diff --git a/.github/workflows/archive-docs.yml b/.github/workflows/archive-docs.yml new file mode 100644 index 00000000000..e183f748d78 --- /dev/null +++ b/.github/workflows/archive-docs.yml @@ -0,0 +1,45 @@ +name: Archive documentation + +on: + release: + types: [published] + workflow_dispatch: + +jobs: + archive-docs: + if: "! contains(toJSON(github.event.commits.*.message), '[ci skip]')" + needs: release-docs + runs-on: ubuntu-latest + steps: + - name: Configure workflow + id: configuration + run: | + echo "BRANCH_NAME=${GITHUB_REF#refs/*/}" >> $GITHUB_OUTPUT + echo "DOCS_OUTPUT_DIR=${GITHUB_WORKSPACE}/skript-docs/docs/archives/${GITHUB_REF#refs/tags/}" >> $GITHUB_OUTPUT + echo "DOCS_REPO_DIR=${GITHUB_WORKSPACE}/skript-docs" >> $GITHUB_OUTPUT + echo "SKRIPT_REPO_DIR=${GITHUB_WORKSPACE}/skript" >> $GITHUB_OUTPUT + - name: Checkout Skript + uses: actions/checkout@v4 + with: + submodules: recursive + path: skript + - name: Setup documentation environment + uses: ./skript/.github/workflows/docs/setup-docs + with: + docs_deploy_key: ${{ secrets.DOCS_DEPLOY_KEY }} + docs_output_dir: ${{ steps.configuration.outputs.DOCS_OUTPUT_DIR }} + - name: Generate documentation + uses: ./skript/.github/workflows/docs/generate-docs + with: + docs_output_dir: ${{ steps.configuration.outputs.DOCS_OUTPUT_DIR }} + docs_repo_dir: ${{ steps.configuration.outputs.DOCS_REPO_DIR }} + skript_repo_dir: ${{ steps.configuration.outputs.SKRIPT_REPO_DIR }} + is_release: true + generate_javadocs: true + - name: Push archive documentation + uses: ./skript/.github/workflows/docs/push-docs + with: + docs_repo_dir: ${{ steps.configuration.outputs.DOCS_REPO_DIR }} + git_name: Archive Docs Bot + git_email: archivedocs@skriptlang.org + git_commit_message: "Update ${{ steps.configuration.outputs.BRANCH_NAME }} archive docs" diff --git a/.github/workflows/docs/generate-docs/action.yml b/.github/workflows/docs/generate-docs/action.yml index e033996868e..2470f852450 100644 --- a/.github/workflows/docs/generate-docs/action.yml +++ b/.github/workflows/docs/generate-docs/action.yml @@ -69,7 +69,7 @@ runs: cd $SKRIPT_REPO_DIR if [[ "${IS_RELEASE}" == "true" ]]; then - ./gradlew genReleaseDocs releaseJavadoc + ./gradlew genReleaseDocs javadoc elif [[ "${GENERATE_JAVADOCS}" == "true" ]]; then ./gradlew genNightlyDocs javadoc else @@ -77,7 +77,7 @@ runs: fi if [ -d "${DOCS_OUTPUT_DIR}" ]; then - if [[ "${GENERATE_JAVADOCS}" == "true" ]]; then + if [[ "${GENERATE_JAVADOCS}" == "true" ]] || [[ "${IS_RELEASE}" == "true" ]] ; then mkdir -p "${SKRIPT_DOCS_OUTPUT_DIR}/javadocs" && cp -a "./build/docs/javadoc/." "$_" fi diff --git a/.github/workflows/release-docs.yml b/.github/workflows/release-docs.yml index f6f7ebc8a27..9cfe198ba93 100644 --- a/.github/workflows/release-docs.yml +++ b/.github/workflows/release-docs.yml @@ -3,6 +3,7 @@ name: Release documentation on: release: types: [published] + workflow_dispatch: jobs: release-docs: @@ -33,6 +34,7 @@ jobs: docs_repo_dir: ${{ steps.configuration.outputs.DOCS_REPO_DIR }} skript_repo_dir: ${{ steps.configuration.outputs.SKRIPT_REPO_DIR }} is_release: true + generate_javadocs: true cleanup_pattern: "!(nightly|archives|templates)" - name: Push release documentation uses: ./skript/.github/workflows/docs/push-docs @@ -41,40 +43,3 @@ jobs: git_name: Release Docs Bot git_email: releasedocs@skriptlang.org git_commit_message: "Update release docs to ${{ steps.configuration.outputs.BRANCH_NAME }}" - - archive-docs: - if: "! contains(toJSON(github.event.commits.*.message), '[ci skip]')" - needs: release-docs - runs-on: ubuntu-latest - steps: - - name: Configure workflow - id: configuration - run: | - echo "BRANCH_NAME=${GITHUB_REF#refs/*/}" >> $GITHUB_OUTPUT - echo "DOCS_OUTPUT_DIR=${GITHUB_WORKSPACE}/skript-docs/docs/archives/${GITHUB_REF#refs/tags/}" >> $GITHUB_OUTPUT - echo "DOCS_REPO_DIR=${GITHUB_WORKSPACE}/skript-docs" >> $GITHUB_OUTPUT - echo "SKRIPT_REPO_DIR=${GITHUB_WORKSPACE}/skript" >> $GITHUB_OUTPUT - - name: Checkout Skript - uses: actions/checkout@v4 - with: - submodules: recursive - path: skript - - name: Setup documentation environment - uses: ./skript/.github/workflows/docs/setup-docs - with: - docs_deploy_key: ${{ secrets.DOCS_DEPLOY_KEY }} - docs_output_dir: ${{ steps.configuration.outputs.DOCS_OUTPUT_DIR }} - - name: Generate documentation - uses: ./skript/.github/workflows/docs/generate-docs - with: - docs_repo_dir: ${{ steps.configuration.outputs.DOCS_REPO_DIR }} - docs_output_dir: ${{ steps.configuration.outputs.DOCS_OUTPUT_DIR }} - skript_repo_dir: ${{ steps.configuration.outputs.SKRIPT_REPO_DIR }} - is_release: true - - name: Push archive documentation - uses: ./skript/.github/workflows/docs/push-docs - with: - docs_repo_dir: ${{ steps.configuration.outputs.DOCS_REPO_DIR }} - git_name: Archive Docs Bot - git_email: archivedocs@skriptlang.org - git_commit_message: "Update ${{ steps.configuration.outputs.BRANCH_NAME }} archive docs" diff --git a/build.gradle b/build.gradle index fcd904d9863..161d3dd3daf 100644 --- a/build.gradle +++ b/build.gradle @@ -5,7 +5,6 @@ import java.time.LocalTime plugins { id 'com.github.johnrengelman.shadow' version '8.1.1' - id 'com.github.hierynomus.license' version '0.16.1' id 'maven-publish' id 'java' } @@ -55,7 +54,7 @@ task checkAliases { } task testJar(type: ShadowJar) { - dependsOn(compileTestJava, licenseTest) + dependsOn(compileTestJava) archiveFileName = 'Skript-JUnit.jar' from sourceSets.test.output, sourceSets.main.output, project.configurations.testShadow } @@ -141,24 +140,6 @@ publishing { } } -license { - header file('licenseheader.txt') - exclude('**/Metrics.java') // Not under GPLv3 - exclude('**/BurgerHelper.java') // Not exclusively GPLv3 - exclude('**/*.sk') // Sample scripts and maybe aliases - exclude('**/*.lang') // Language files do not have headers (still under GPLv3) - exclude('**/*.json') // JSON files do not have headers -} - -task releaseJavadoc(type: Javadoc) { - title = project.name + ' ' + project.property('version') - source = sourceSets.main.allJava - classpath = configurations.compileClasspath - options.encoding = 'UTF-8' - // currently our javadoc has a lot of errors, so we need to suppress the linter - options.addStringOption('Xdoclint:none', '-quiet') -} - // Task to check that test scripts are named correctly tasks.register('testNaming') { doLast { @@ -198,13 +179,12 @@ void createTestTask(String name, String desc, String environments, int javaVersi if (junit) { artifact += 'Skript-JUnit.jar' } else if (releaseDocs) { - artifact += 'Skript-github.jar' + artifact += 'Skript-' + version + '.jar' } else { artifact += 'Skript-nightly.jar' } tasks.register(name, JavaExec) { description = desc - dependsOn licenseTest if (junit) { dependsOn testJar } else if (releaseDocs) { @@ -307,7 +287,7 @@ task githubResources(type: ProcessResources) { include '**' version = project.property('version') def channel = 'stable' - if (version.contains('pre')) + if (version.contains('-')) channel = 'prerelease' filter ReplaceTokens, tokens: [ 'version' : version, @@ -340,7 +320,7 @@ task spigotResources(type: ProcessResources) { include '**' version = project.property('version') def channel = 'stable' - if (version.contains('pre')) + if (version.contains('-')) channel = 'prerelease' filter ReplaceTokens, tokens: [ 'version' : version, @@ -378,7 +358,7 @@ task nightlyResources(type: ProcessResources) { 'today' : '' + LocalTime.now(), 'release-flavor' : 'skriptlang-nightly', // SkriptLang build, automatically done by CI 'release-channel' : 'prerelease', // No update checking, but these are VERY unstable - 'release-updater' : 'ch.njol.skript.update.NoUpdateChecker', // No autoupdates for now + 'release-updater' : 'ch.njol.skript.update.NoUpdateChecker', // No auto updates for now 'release-source' : '', 'release-download': 'null' ] @@ -388,7 +368,7 @@ task nightlyResources(type: ProcessResources) { task nightlyRelease(type: ShadowJar) { from sourceSets.main.output - dependsOn nightlyResources, licenseMain + dependsOn nightlyResources archiveFileName = 'Skript-nightly.jar' manifest { attributes( @@ -400,8 +380,8 @@ task nightlyRelease(type: ShadowJar) { } javadoc { - dependsOn nightlyResources - + mustRunAfter(tasks.withType(ProcessResources)) + title = 'Skript ' + project.property('version') source = sourceSets.main.allJava exclude("ch/njol/skript/conditions/**") @@ -420,4 +400,3 @@ javadoc { // currently our javadoc has a lot of errors, so we need to suppress the linter options.addStringOption('Xdoclint:none', '-quiet') } - diff --git a/code-conventions.md b/code-conventions.md index f571429ddb7..25c6d5c4417 100644 --- a/code-conventions.md +++ b/code-conventions.md @@ -66,6 +66,9 @@ With the exception of contacting our own resources (e.g. to check for updates) c Code contributed must be licensed under GPLv3, by **you**. We expect that any code you contribute is either owned by you or you have explicit permission to provide and license it to us. +Licenses do not need to be printed in individual files (or packages) unless the licence applying to the code in +that file (or package) deviates from the licence scope of its containing package. + Third party code (under a compatible licence) _may_ be accepted in the following cases: - It is part of a public, freely-available library or resource. - It is somehow necessary to your contribution, and you have been given permission to include it. @@ -75,34 +78,61 @@ If we receive complaints regarding the licensing of a contribution we will forwa If you have questions or complaints regarding the licensing or reproduction of a contribution you may contact us (the organisation) or the contributor of that code directly. -If, in the future, we need to relicense contributed code, we will contact all contributors involved. +If, in the future, we need to re-license contributed code, we will contact all contributors involved. If we need to remove or alter contributed code due to a licensing issue we will attempt to notify its contributor. ## Code Style ### Formatting +* Imports should be grouped together by type (e.g. all `java.lang...` imports together) + * Following the style of existing imports in a class is encouraged, but not required + * Wildcard `*` imports are permitted (as long as they do not interfere with existing imports), e.g. `java.lang.*`. * Tabs, no spaces (unless in code imported from other projects) -** No tabs/spaces in empty lines + - No tabs/spaces in empty lines * No trailing whitespace * At most 120 characters per line - - In Javadoc/multiline comments, at most 80 characters per line -* When statements consume multiple lines, all lines but first have two tabs of additional indentation + - In Javadoc/multiline comments, at most 80 characters per line +* When statements consume multiple lines, all lines but the first have two tabs of additional indentation + - The exception to this is breaking up conditional statements (e.g. `if (x || y)`) where the + condition starts may be aligned * Each class begins with an empty line * No squeezing of multiple lines of code on a single line * Separate method declarations with empty lines - - Empty line after last method in a class is *not* required - - Otherwise, empty line before and after method is a good rule of thumb + - Empty line after last method in a class is *not* required + - Otherwise, empty line before and after method is a good rule of thumb * If fields have Javadoc, separate them with empty lines * Use empty lines liberally inside methods to improve readability * Use curly brackets to start and end most blocks - - Only when a conditional block (if or else) contains a single statement, they may be omitted - - When omitting brackets, still indent as if the code had brackets - - Avoid omitting brackets if it produces hard-to-read code + - When a block contains a single statement, they may be omitted + - Brackets may not be omitted in a chain of other blocks that require brackets, e.g `if ... else {}` + - When omitting brackets, still indent as if the code had brackets + - Avoid omitting brackets if it produces hard-to-read or ambiguous code +* Ternaries should be avoided where it makes the code complex or difficult to read * Annotations for methods and classes are placed in lines before their declarations, one per line -* When there are multiple annotations, place them in order: - - @Override -> @Nullable -> @SuppressWarnings - - For other annotations, doesn't matter; let your IDE decide + - Annotations for a structure go on the line before that structure + ```java + @Override + @SuppressWarnings("xyz") + public void myMethod() { + // Override goes above method because method is overriding + } + ``` + + - Annotations for the _value_ of a thing go before that value's type declaration + ```java + @Override + public @Nullable Object myMethod() { + // Nullable goes before Object because Object is Nullable + } + ``` +* When there are multiple annotations, it looks nicer to place them in length order (longest last) +but this is not strictly required: + ```java + @Override + @Deprecated + @SuppressWarnings("xyz") + ``` * When splitting Strings into multiple lines the last part of the string must be (space character included) " " + ```java String string = "example string " + @@ -111,6 +141,7 @@ If we need to remove or alter contributed code due to a licensing issue we will * When extending one of following classes: SimpleExpression, SimplePropertyExpression, Effect, Condition... - Put overridden methods in order + - Put static registration before all methods - SimpleExpression: init -> get/getAll -> acceptChange -> change -> setTime -> getTime -> isSingle -> getReturnType -> toString - SimplePropertyExpression: -> init -> convert -> acceptChange -> change -> setTime -> getTime -> getReturnType -> getPropertyName - Effect: init -> execute -> toString @@ -130,8 +161,8 @@ If we need to remove or alter contributed code due to a licensing issue we will * Use prefixes only where their use has been already established (such as `ExprSomeRandomThing`) - Otherwise, use postfixes where necessary - Common occurrences include: Struct (Structure), Sec (Section), EffSec (EffectSection), Eff (Effect), Cond (Condition), Expr (Expression) -* Ensure variable/field names are descriptive. Avoid using shorthand names like `e`, or `c`. - - e.g. Event should be `event`, not `e`. `e` is ambiguous and could mean a number of things. +* Ensure variable/field names are descriptive. Avoid using shorthand names like `e`, or `c` + - e.g. Event should be `event`, not `e`. `e` is ambiguous and could mean a number of things ### Comments * Prefer to comment *why* you're doing things instead of how you're doing them @@ -163,33 +194,79 @@ Your comments should look something like these: ## Language Features ### Compatibility -* Contributions should maintain Java 8 source/binary compatibility, even though compiling Skript requires Java 21 - - Users must not need JRE newer than version 8 +[//]: # (To be updated after feature/2.9 for Java 17) +* Contributions should maintain Java 11 source/binary compatibility, even though compiling Skript requires Java 21 + - Users must not need JRE newer than version 11 * Versions up to and including Java 21 should work too - Please avoid using unsafe reflection * It is recommended to make fields final, if they are effectively final -* Local variables and method parameters should not be declared final +* Local variables and method parameters should not be declared final unless used in anonymous classes, lambdas +or try-with-resources sections where their immutability is necessary * Methods should be declared final only where necessary * Use `@Override` whenever applicable - -### Nullness + - They may be omitted to prevent compilation errors when something overrides only + on a version-dependent basis (e.g. in Library XYZ version 2 we override `getX()` but in version 3 it's + gone, and we call it ourselves) + +### null-ness +* We use **JetBrains** Annotations for specifying null-ness and method contracts. + * If editing a file using a different annotation set (e.g. Javax, Eclipse Sisu, Bukkit) + these should be replaced with their JetBrains equivalent. + * The semantics for JetBrains Annotations are strict _and should be observed!_ + * Many IDEs have built-in compiler-level support for these, and can even be set to produce strict + errors when an annotation is misused; do not misuse them. + * **`@NotNull`** + * > An element annotated with NotNull claims null value is forbidden to return (for methods), + pass to (parameters) and hold (local variables and fields). + * Something is `@NotNull` iff it is never null from its inception (new X) to its garbage collection, + i.e. there is no point in time at which the value could ever be null. + * **`@Nullable`** + * > An element annotated with Nullable claims null value is perfectly valid to return (for methods), + > pass to (parameters) or hold in (local variables and fields). + > + > By convention, this annotation applied only when the value should always be checked + > against null because the developer could do nothing to prevent null from happening. + * Something is `@Nullable` iff there is _absolutely no way of determining_ (other than checking its + value `!= null`) whether it is null. + * In other words, if there is another way of knowing (e.g. you set it yourself, an `isPresent` method, etc.) + then it should not be marked nullable. + * **`@Contract`** + * The contract annotation should be used to express other behaviour (e.g. null depending on parameters). * All fields, method parameters and their return values are non-null by default - - Exceptions: Github API JSON mappings, Metrics -* When something is nullable, mark it as so -* Only ignore nullness errors when a variable is effectively non-null - if in doubt: check - - Most common example is syntax elements, which are not initialised using a constructor + - Exceptions: GitHub API JSON mappings, Metrics +* When ignoring warnings, use the no-inspection comment rather than a blanket suppression annotation * Use assertions liberally: if you're sure something is not null, assert so to the compiler - Makes finding bugs easier for developers -* Assertions must **not** have side-effects - they may be skipped in real environments +* Assertions must **not** have side-effects in non-test packages - they may be skipped in real environments * Avoid checking non-null values for nullability - Unless working around buggy addons, it is rarely necessary - - This is why ignoring nullness errors is particularly dangerous + - This is why ignoring null-ness errors is particularly dangerous +* Annotations on array types **must** be placed properly: + * Annotations on the array itself go before the array brackets + ```java + @Nullable Object @NotNull [] + // a not-null array of nullable objects + ``` + * Annotations on values inside the array go before the value declaration + ```java + @NotNull Object @Nullable [] + // a nullable array of not-null objects + ``` + * If this is not adhered to, an IDE may provide incorrect feedback. ### Assertions Skript must run with assertations enabled; use them in your development environment. \ The JVM flag -ea is used to enable them. +## Code Complexity + +Dense, highly-complex code should be avoided to preserve readability and to help with future maintenance, +especially within a single method body. + +There are many available metrics for measuring code complexity (for different purposes); [we have our own](https://stable.skriptlang.org/Radical_Complexity.pdf). +There are no strict limits for code complexity, but you may be encouraged (or required) to reformat or break up methods +into smaller, more manageable chunks. If in doubt, keep things simple. ## Minecraft Features diff --git a/gradle.properties b/gradle.properties index 7753116b0ea..f78ae1766eb 100644 --- a/gradle.properties +++ b/gradle.properties @@ -5,7 +5,7 @@ org.gradle.parallel=true groupid=ch.njol name=skript -version=2.8.5 +version=2.8.7 jarName=Skript.jar testEnv=java21/paper-1.20.6 testEnvJavaVersion=21 diff --git a/licenseheader.txt b/licenseheader.txt deleted file mode 100644 index 15be760be16..00000000000 --- a/licenseheader.txt +++ /dev/null @@ -1,16 +0,0 @@ - This file is part of Skript. - - Skript is free software: you can redistribute it and/or modify - it under the terms of the GNU General Public License as published by - the Free Software Foundation, either version 3 of the License, or - (at your option) any later version. - - Skript is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - GNU General Public License for more details. - - You should have received a copy of the GNU General Public License - along with Skript. If not, see . - -Copyright Peter Güttinger, SkriptLang team and contributors \ No newline at end of file diff --git a/src/main/java/ch/njol/skript/SkriptCommand.java b/src/main/java/ch/njol/skript/SkriptCommand.java index 62261d0e426..0bc25391267 100644 --- a/src/main/java/ch/njol/skript/SkriptCommand.java +++ b/src/main/java/ch/njol/skript/SkriptCommand.java @@ -62,19 +62,19 @@ public class SkriptCommand implements CommandExecutor { // TODO /skript scripts show/list - lists all enabled and/or disabled scripts in the scripts folder and/or subfolders (maybe add a pattern [using * and **]) // TODO document this command on the website private static final CommandHelp SKRIPT_COMMAND_HELP = new CommandHelp("/skript", SkriptColor.LIGHT_CYAN, CONFIG_NODE + ".help") - .add(new CommandHelp("reload", SkriptColor.DARK_RED) + .add(new CommandHelp("reload", SkriptColor.DARK_CYAN) .add("all") .add("config") .add("aliases") .add("scripts") .add("