diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 5a51842..98500a2 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -9,11 +9,14 @@ env: jobs: build-and-test: - name: Build and Test + name: Build and Test on JDK ${{ matrix.java_version }} runs-on: ubuntu-latest defaults: run: working-directory: jspecify-reference-checker + strategy: + matrix: + java_version: [17, 21, 25] steps: - name: Check out jspecify-reference checker uses: actions/checkout@v6 @@ -47,20 +50,20 @@ jobs: uses: actions/setup-java@v5 with: distribution: temurin - java-version: 17 + java-version: ${{ matrix.java_version }} - name: Set up Gradle uses: gradle/actions/setup-gradle@v5 - name: Build and Test (EISOP release) if: ${{ env.EISOP_RELEASE == 'true' }} # If a released CF is needed, use the following: - run: ./gradlew build conformanceTests demoTest --include-build ../jspecify + run: ./gradlew build conformanceTests demoTest --include-build ../jspecify --warning-mode all env: SHALLOW: 1 JSPECIFY_CONFORMANCE_TEST_MODE: details - name: Build and Test (EISOP build) if: ${{ env.EISOP_RELEASE != 'true' }} # If a cloned EISOP CF is needed, use the following: - run: ./gradlew build conformanceTests demoTest --include-build ../jspecify --include-build ../checker-framework + run: ./gradlew build conformanceTests demoTest --include-build ../jspecify --include-build ../checker-framework --warning-mode all env: SHALLOW: 1 JSPECIFY_CONFORMANCE_TEST_MODE: details @@ -73,11 +76,11 @@ jobs: - name: Run Samples Tests (EISOP release) if: ${{ always() && env.EISOP_RELEASE == 'true' }} # If a released CF is needed, use the following: - run: ./gradlew jspecifySamplesTest --include-build ../jspecify + run: ./gradlew jspecifySamplesTest --include-build ../jspecify --warning-mode all - name: Run Samples Tests (EISOP build) if: ${{ always() && env.EISOP_RELEASE != 'true' }} # If a cloned EISOP CF is needed, use the following: - run: ./gradlew jspecifySamplesTest --include-build ../jspecify --include-build ../checker-framework + run: ./gradlew jspecifySamplesTest --include-build ../jspecify --include-build ../checker-framework --warning-mode all publish-snapshot: name: Publish Conformance Test Framework Snapshot @@ -95,7 +98,7 @@ jobs: - name: Set up Gradle uses: gradle/actions/setup-gradle@v5 - name: Publish snapshot - run: ./gradlew publishConformanceTestFrameworkPublicationToSonatypeRepository + run: ./gradlew publishConformanceTestFrameworkPublicationToSonatypeRepository --warning-mode all --no-configuration-cache env: ORG_GRADLE_PROJECT_sonatypeUsername: ${{ secrets.sonatype_username }} ORG_GRADLE_PROJECT_sonatypePassword: ${{ secrets.sonatype_password }} diff --git a/build.gradle b/build.gradle index 2870c5c..be70c42 100644 --- a/build.gradle +++ b/build.gradle @@ -1,3 +1,4 @@ +import java.nio.file.Files import org.gradle.plugins.ide.eclipse.model.Output import org.gradle.plugins.ide.eclipse.model.SourceFolder @@ -6,11 +7,11 @@ plugins { id 'java' id 'maven-publish' - id 'com.diffplug.spotless' version '6.25.0' + id 'com.diffplug.spotless' version '8.2.1' id 'io.github.gradle-nexus.publish-plugin' version '2.0.0' - id 'net.ltgt.errorprone' version '4.1.0' + id 'net.ltgt.errorprone' version '5.0.0' // To show task list as a tree, run: ./gradlew taskTree - id 'com.dorongold.task-tree' version '4.0.0' + id 'com.dorongold.task-tree' version '4.0.1' } // Nexus Publish plugin requires a group/version at the root project. @@ -59,6 +60,8 @@ ext { // Location of the jspecify/jdk clone, relative to this directory jspecifyJdkHome = '../jdk' + + errorproneSupportsCurrentJdk = Integer.valueOf(JavaVersion.current().getMajorVersion()) >= 21 } configurations { @@ -96,8 +99,18 @@ dependencies { conformanceTestSuite libs.jspecify.conformanceTests - errorproneJavac libs.errorProne.javac - errorprone libs.errorProne.core + if (errorproneSupportsCurrentJdk) { + errorproneJavac libs.errorProne.javac + errorprone libs.errorProne.core + } +} + +configurations { + checkerFrameworkRuntimeClasspath { + canBeResolved = true + canBeConsumed = false + extendsFrom implementation + } } // If built with `--include-build path/to/checker-framework` then @@ -112,17 +125,12 @@ if (jspecify != null) { assemble.dependsOn(jspecify.task(':assemble')) } -// Enable exec/javaexec -interface InjectedExecOps { - @Inject - ExecOperations getExecOps() -} - tasks.withType(JavaCompile).configureEach { options.compilerArgs.add("-Xlint:all") // ErrorProne makes suppressing these easier options.compilerArgs.add("-Xlint:-fallthrough") + options.errorprone.enabled = errorproneSupportsCurrentJdk options.errorprone.disable("BadImport") options.errorprone.disable("VoidUsed") @@ -143,10 +151,7 @@ tasks.withType(JavaCompile).configureEach { .collect { "--add-exports=jdk.compiler/com.sun.tools.javac.$it=ALL-UNNAMED" }) } -tasks.register('includeJSpecifyJDK') { - group = 'Build' - dependsOn 'compileJava' - +tasks.register('copyJSpecifyJDK', Copy) { def srcDir = "${jspecifyJdkHome}/src" // This directory needs to be stored at the top-level of the resulting .jar file. // org.checkerframework.framework.stub.AnnotationFileElementTypes will then load @@ -156,39 +161,50 @@ tasks.register('includeJSpecifyJDK') { inputs.dir file(srcDir) outputs.dir file(dstDir) - def injected = project.objects.newInstance(InjectedExecOps) - - doLast { - FileTree srcTree = fileTree(dir: srcDir) - NavigableSet specFiles = new TreeSet<>(); - srcTree.visit { FileVisitDetails fvd -> - if (!fvd.file.isDirectory() && fvd.file.name.matches('.*\\.java')) { - fvd.getFile().readLines().any { line -> - if (line.contains('org.jspecify')) { - specFiles.add(fvd.file.absolutePath) - return true; - } + FileTree srcTree = fileTree(dir: srcDir) + NavigableSet specFiles = new TreeSet<>(); + srcTree.visit { FileVisitDetails fvd -> + if (!fvd.file.isDirectory() && fvd.file.name.matches('.*\\.java')) { + fvd.getFile().readLines().any { line -> + if (line.contains('org.jspecify')) { + specFiles.add(fvd.file.absolutePath) + return true; } } } - String absoluteSrcDir = file(srcDir).absolutePath - int srcPrefixSize = absoluteSrcDir.size() - copy { - from(srcDir) - into(dstDir) - for (String specFile : specFiles) { - include specFile.substring(srcPrefixSize) - } - } - injected.execOps.javaexec { - classpath = sourceSets.main.runtimeClasspath - standardOutput = System.out - errorOutput = System.err + } + String absoluteSrcDir = file(srcDir).absolutePath + int srcPrefixSize = absoluteSrcDir.size() - mainClass = 'org.checkerframework.framework.stubifier.JavaStubifier' - args dstDir - } + from(srcDir) + into(dstDir) + for (String specFile : specFiles) { + include specFile.substring(srcPrefixSize) + } +} + +tasks.register('includeJSpecifyJDK', JavaExec) { + group = 'Build' + + if (checkerFramework != null) { + dependsOn(checkerFramework.task(":checker:assembleForJavac")) } + dependsOn 'copyJSpecifyJDK' + + // See comment in copyJSpecifyJDK. + def dstDir = "${buildDir}/generated/resources/annotated-jdk/src/" + + inputs.dir file(dstDir) + outputs.dir file(dstDir) + + // We only need the CF on the classpath. + classpath = configurations.checkerFrameworkRuntimeClasspath + + standardOutput = System.out + errorOutput = System.err + + mainClass = 'org.checkerframework.framework.stubifier.JavaStubifier' + args dstDir } processResources.dependsOn(includeJSpecifyJDK) @@ -230,10 +246,16 @@ tasks.register('jspecifySamplesTest', Test) { description = 'Run the checker against the JSpecify samples.' group = 'verification' shouldRunAfter test + outputs.upToDateWhen { false } include '**/NullSpecTest$Lenient.class' include '**/NullSpecTest$Strict.class' - inputs.files(unzipConformanceTestSuite) + testClassesDirs = files('build/classes/java/test') + classpath = sourceSets.test.runtimeClasspath + + // TODO: does this actually run on the correct tests? Should this unzip again?? + def conformanceTests = layout.buildDirectory.dir("conformanceTests").get() + inputs.dir(conformanceTests) } tasks.register('unzipConformanceTestSuite', Copy) { @@ -245,16 +267,25 @@ tasks.register('unzipConformanceTestSuite', Copy) { tasks.register('conformanceTests', Test) { group = 'verification' include '**/ConformanceTest.class' + dependsOn 'unzipConformanceTestSuite' + shouldRunAfter test + outputs.upToDateWhen { false } + + testClassesDirs = files('build/classes/java/test') + classpath = sourceSets.test.runtimeClasspath + + def conformanceTests = layout.buildDirectory.dir("conformanceTests").get() + def deps = fileTree("${conformanceTests}/deps").join(":") // Conformance tests - inputs.files(unzipConformanceTestSuite) + inputs.dir(conformanceTests) inputs.files("tests/ConformanceTest-report.txt") doFirst { systemProperties([ - "JSpecifyConformanceTest.inputs": "${unzipConformanceTestSuite.destinationDir}/assertions/org/jspecify/conformance/tests", + "JSpecifyConformanceTest.inputs": "${conformanceTests}/assertions/org/jspecify/conformance/tests", "JSpecifyConformanceTest.report": "tests/ConformanceTest-report.txt", - "JSpecifyConformanceTest.deps" : fileTree("${unzipConformanceTestSuite.destinationDir}/deps").join(":") + "JSpecifyConformanceTest.deps" : deps ]) } @@ -262,7 +293,7 @@ tasks.register('conformanceTests', Test) { inputs.files("tests/ConformanceTestOnSamples-report.txt") doFirst { systemProperties([ - "JSpecifyConformanceTest.samples.inputs": "${unzipConformanceTestSuite.destinationDir}/samples", + "JSpecifyConformanceTest.samples.inputs": "${conformanceTests}/samples", "JSpecifyConformanceTest.samples.report": "tests/ConformanceTestOnSamples-report.txt" ]) } @@ -272,9 +303,10 @@ tasks.named('check').configure { dependsOn('conformanceTests') } -clean.doFirst { +tasks.register('deleteTestsBuild') { delete "${rootDir}/tests/build/" } +clean.dependsOn('deleteTestsBuild') tasks.register('demoTest', Exec) { group = 'verification' @@ -284,10 +316,15 @@ tasks.register('demoTest', Exec) { executable '/bin/sh' args 'demo', 'SimpleSample.java' ignoreExitValue = true - errorOutput = new ByteArrayOutputStream() + def outputFile = layout.buildDirectory.file('demoTest-output.txt') + outputs.file(outputFile) + doFirst { + errorOutput = Files.newOutputStream(outputFile.get().asFile.toPath()) + } doLast { - if (!errorOutput.toString().contains("SimpleSample.java:7: error:")) { - throw new AssertionError("`./demo SimpleSample.java` did not run correctly. Error output:\n$errorOutput") + def outputFileText = outputFile.get().asFile.text + if (!outputFileText.contains("SimpleSample.java:7: error:")) { + throw new AssertionError("`./demo SimpleSample.java` did not contain the expected error. Error output:\n${outputFileText}") } } } @@ -297,28 +334,31 @@ tasks.register('demoTest', Exec) { google-java-format depends on checker-qual, which is built by a subproject. On a clean build, the checker-qual JAR file doesn't exist yet, so Spotless throws an error. The file doesn't have to be correct; it just has to be a JAR file. - So here, before the spotless block, we create a meaningless JAR file at that location if it doesn't already exist. + So here, before the spotless block, we create a meaningless JAR file at that location if it doesn't already exist. See https://github.com/jspecify/jspecify-reference-checker/issues/81 */ - -if (checkerFramework != null) { - def cfQualJar = - checkerFramework.projectDir.toPath() - .resolve("checker-qual/build/libs/checker-qual-${libs.versions.checkerFramework.get()}.jar") - def injected = project.objects.newInstance(InjectedExecOps) - - if (!cfQualJar.toFile().exists()) { - mkdir(cfQualJar.parent) - injected.execOps.exec { - executable 'jar' - args = [ - 'cf', - cfQualJar, - buildFile.path // Use this build script file! - ] - } - } -} +/* This work-around isn't compatible with the configuration cache. + We don't need it, b/c we depend on EISOP CF and Spotless depends on typetools CF. + */ +/* + if (checkerFramework != null) { + def cfQualJar = + checkerFramework.projectDir.toPath() + .resolve("checker-qual/build/libs/checker-qual-${libs.versions.checkerFramework.get()}.jar") + def injected = project.objects.newInstance(InjectedExecOps) + if (!cfQualJar.toFile().exists()) { + mkdir(cfQualJar.parent) + injected.execOps.exec { + executable 'jar' + args = [ + 'cf', + cfQualJar, + buildFile.path // Use this build script file! + ] + } + } + } + */ spotless { java { @@ -329,7 +369,7 @@ spotless { groovyGradle { target '**/*.gradle' greclipse() - indentWithSpaces(4) + leadingTabsToSpaces(4) trimTrailingWhitespace() } } diff --git a/conformance-test-framework/src/main/java/org/jspecify/conformance/ConformanceTestRunner.java b/conformance-test-framework/src/main/java/org/jspecify/conformance/ConformanceTestRunner.java index ca47664..9e9174c 100644 --- a/conformance-test-framework/src/main/java/org/jspecify/conformance/ConformanceTestRunner.java +++ b/conformance-test-framework/src/main/java/org/jspecify/conformance/ConformanceTestRunner.java @@ -126,7 +126,7 @@ public void checkConformance(Path testDirectory, ImmutableList testDeps, P switch (Mode.fromEnvironment()) { case DETAILS: System.out.print(testResults.report(true)); - // fall-through + // fall-through case COMPARE: assertThat(testResults.report(false)).isEqualTo(asCharSource(testReport, UTF_8).read()); diff --git a/gradle.properties b/gradle.properties index 7315fa5..59cc08a 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,3 +1,8 @@ org.jspecify\:jspecify=1.0.0 org.jspecify.conformance\:conformance-test-framework=0.0.0-SNAPSHOT org.jspecify.conformance\:conformance-tests=0.0.0-SNAPSHOT + +org.gradle.parallel=true +org.gradle.jvmargs=-Xmx4g -Dfile.encoding=UTF-8 +org.gradle.caching=true +org.gradle.configuration-cache=true diff --git a/gradle/wrapper/gradle-wrapper.jar b/gradle/wrapper/gradle-wrapper.jar index 1b33c55..61285a6 100644 Binary files a/gradle/wrapper/gradle-wrapper.jar and b/gradle/wrapper/gradle-wrapper.jar differ diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties index d4081da..37f78a6 100644 --- a/gradle/wrapper/gradle-wrapper.properties +++ b/gradle/wrapper/gradle-wrapper.properties @@ -1,6 +1,6 @@ distributionBase=GRADLE_USER_HOME distributionPath=wrapper/dists -distributionUrl=https\://services.gradle.org/distributions/gradle-8.14.3-bin.zip +distributionUrl=https\://services.gradle.org/distributions/gradle-9.3.1-bin.zip networkTimeout=10000 validateDistributionUrl=true zipStoreBase=GRADLE_USER_HOME diff --git a/gradlew b/gradlew index 23d15a9..adff685 100755 --- a/gradlew +++ b/gradlew @@ -1,7 +1,7 @@ #!/bin/sh # -# Copyright © 2015-2021 the original authors. +# Copyright © 2015 the original authors. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -114,7 +114,6 @@ case "$( uname )" in #( NONSTOP* ) nonstop=true ;; esac -CLASSPATH="\\\"\\\"" # Determine the Java command to use to start the JVM. @@ -172,7 +171,6 @@ fi # For Cygwin or MSYS, switch paths to Windows format before running java if "$cygwin" || "$msys" ; then APP_HOME=$( cygpath --path --mixed "$APP_HOME" ) - CLASSPATH=$( cygpath --path --mixed "$CLASSPATH" ) JAVACMD=$( cygpath --unix "$JAVACMD" ) @@ -212,7 +210,6 @@ DEFAULT_JVM_OPTS='"-Xmx64m" "-Xms64m"' set -- \ "-Dorg.gradle.appname=$APP_BASE_NAME" \ - -classpath "$CLASSPATH" \ -jar "$APP_HOME/gradle/wrapper/gradle-wrapper.jar" \ "$@" diff --git a/gradlew.bat b/gradlew.bat index 5eed7ee..e509b2d 100644 --- a/gradlew.bat +++ b/gradlew.bat @@ -70,11 +70,10 @@ goto fail :execute @rem Setup the command line -set CLASSPATH= @rem Execute Gradle -"%JAVA_EXE%" %DEFAULT_JVM_OPTS% %JAVA_OPTS% %GRADLE_OPTS% "-Dorg.gradle.appname=%APP_BASE_NAME%" -classpath "%CLASSPATH%" -jar "%APP_HOME%\gradle\wrapper\gradle-wrapper.jar" %* +"%JAVA_EXE%" %DEFAULT_JVM_OPTS% %JAVA_OPTS% %GRADLE_OPTS% "-Dorg.gradle.appname=%APP_BASE_NAME%" -jar "%APP_HOME%\gradle\wrapper\gradle-wrapper.jar" %* :end @rem End local scope for the variables with windows NT shell diff --git a/settings.gradle b/settings.gradle index 265a4d4..c5a0673 100644 --- a/settings.gradle +++ b/settings.gradle @@ -6,13 +6,6 @@ include 'conformance-test-framework' // See https://docs.gradle.org/current/userguide/composite_builds.html#included_build_declaring_substitutions includeBuild(".") -def proc = "./initialize-project".execute() -proc.waitForProcessOutput(System.out, System.err) - -if (proc.exitValue() != 0) { - throw new GradleException("Execution of ./initialize-project failed with exit code ${proc.exitValue()}") -} - dependencyResolutionManagement { versionCatalogs { libs { @@ -24,7 +17,7 @@ dependencyResolutionManagement { library("checkerFramework-framework", "io.github.eisop", "framework").versionRef("checkerFramework") library("checkerFramework-framework-test", "io.github.eisop", "framework-test").versionRef("checkerFramework") library("checkerFramework-javacutil", "io.github.eisop", "javacutil").versionRef("checkerFramework") - library("errorProne-core", "com.google.errorprone:error_prone_core:2.36.0") + library("errorProne-core", "com.google.errorprone:error_prone_core:2.46.0") library("errorProne-javac", "com.google.errorprone:javac:9+181-r4173-1") library("guava", "com.google.guava:guava:31.1-jre") diff --git a/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java b/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java index 23981f6..fa25f35 100644 --- a/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java +++ b/src/main/java/com/google/jspecify/nullness/NullSpecAnnotatedTypeFactory.java @@ -16,8 +16,6 @@ import static com.google.jspecify.nullness.NullSpecAnnotatedTypeFactory.IsDeclaredOrArrayOrNull.IS_DECLARED_OR_ARRAY_OR_NULL; import static com.google.jspecify.nullness.Util.nameMatches; -import static com.sun.source.tree.Tree.Kind.IDENTIFIER; -import static com.sun.source.tree.Tree.Kind.MEMBER_SELECT; import static com.sun.source.tree.Tree.Kind.NOT_EQUAL_TO; import static com.sun.source.tree.Tree.Kind.NULL_LITERAL; import static java.util.Arrays.asList; @@ -816,16 +814,16 @@ private List getUpperBounds(AnnotatedTypeMirror t case TYPEVAR: return singletonList(((AnnotatedTypeVariable) type).getUpperBound()); - /* - * We used to have a case here for WILDCARD. It shouldn't be necessary now that we've merged - * the CF implementation capture conversion. That said, we shouldn't need wildcard handling - * in isNullnessSubtype, either, and yet we do. - * - * So we could consider restoring wildcard handling here, too. But for now, we've left it - * out, since its implementation required some digging into javac internal types and calling - * getAnnotatedType (always a little scary, as discussed in - * NullSpecTreeAnnotator.visitMethodInvocation and elsewhere). - */ + /* + * We used to have a case here for WILDCARD. It shouldn't be necessary now that we've merged + * the CF implementation capture conversion. That said, we shouldn't need wildcard handling + * in isNullnessSubtype, either, and yet we do. + * + * So we could consider restoring wildcard handling here, too. But for now, we've left it + * out, since its implementation required some digging into javac internal types and calling + * getAnnotatedType (always a little scary, as discussed in + * NullSpecTreeAnnotator.visitMethodInvocation and elsewhere). + */ default: return emptyList(); @@ -1256,7 +1254,7 @@ private boolean isGetOrDefaultWithNonnullMapValuesAndDefault(MethodInvocationTre if (!util.isOrOverrides(elementFromUse(tree), util.mapGetOrDefaultElement)) { return false; } - if (tree.getMethodSelect().getKind() != MEMBER_SELECT) { + if (!(tree.getMethodSelect() instanceof MemberSelectTree)) { /* * We don't care much about handling IDENTIFIER, since we're not likely to be analyzing a * Map implementation with a call to getOrDefault on itself, at least not with a @@ -1292,7 +1290,7 @@ private boolean isGetEnumConstantsOnEnumClass(MethodInvocationTree tree) { || elementFromUse(tree) != util.classGetEnumConstantsElement.get()) { return false; } - if (tree.getMethodSelect().getKind() != MEMBER_SELECT) { + if (!(tree.getMethodSelect() instanceof MemberSelectTree)) { /* * We don't care much about handling IDENTIFIER, since we're not likely to be analyzing * Class itself. But see the TODO in upperBoundOnToArrayElementType. @@ -1305,7 +1303,7 @@ private boolean isGetEnumConstantsOnEnumClass(MethodInvocationTree tree) { } private boolean isGetCauseOnExecutionException(MethodInvocationTree tree) { - if (tree.getMethodSelect().getKind() != MEMBER_SELECT) { + if (!(tree.getMethodSelect() instanceof MemberSelectTree)) { /* * We don't care much about handling IDENTIFIER, since we're not likely to be analyzing * ExecutionException itself (nor a subclass of it). But see the TODO in @@ -1395,9 +1393,9 @@ private AnnotationMirror upperBoundOnToArrayElementType(MethodInvocationTree tre * TypeMirror, as in isGetCauseOnExecutionException. */ Tree receiver; - if (tree.getMethodSelect().getKind() == MEMBER_SELECT) { + if (tree.getMethodSelect() instanceof MemberSelectTree) { receiver = ((MemberSelectTree) tree.getMethodSelect()).getExpression(); - } else if (tree.getMethodSelect().getKind() == IDENTIFIER) { + } else if (tree.getMethodSelect() instanceof IdentifierTree) { /* * TODO(cpovirk): We need to figure out whether the call is being made on the instance of * the enclosing class or on another class that encloses that. diff --git a/src/main/java/com/google/jspecify/nullness/NullSpecVisitor.java b/src/main/java/com/google/jspecify/nullness/NullSpecVisitor.java index ff22136..41d8b69 100644 --- a/src/main/java/com/google/jspecify/nullness/NullSpecVisitor.java +++ b/src/main/java/com/google/jspecify/nullness/NullSpecVisitor.java @@ -16,11 +16,7 @@ import static com.google.jspecify.nullness.Util.IMPLEMENTATION_VARIABLE_KINDS; import static com.google.jspecify.nullness.Util.nameMatches; -import static com.sun.source.tree.Tree.Kind.ANNOTATED_TYPE; -import static com.sun.source.tree.Tree.Kind.ARRAY_TYPE; import static com.sun.source.tree.Tree.Kind.EXTENDS_WILDCARD; -import static com.sun.source.tree.Tree.Kind.MEMBER_SELECT; -import static com.sun.source.tree.Tree.Kind.PARAMETERIZED_TYPE; import static com.sun.source.tree.Tree.Kind.PRIMITIVE_TYPE; import static com.sun.source.tree.Tree.Kind.SUPER_WILDCARD; import static com.sun.source.tree.Tree.Kind.UNBOUNDED_WILDCARD; @@ -54,6 +50,7 @@ import com.sun.source.tree.NewClassTree; import com.sun.source.tree.ParameterizedTypeTree; import com.sun.source.tree.ParenthesizedTree; +import com.sun.source.tree.PrimitiveTypeTree; import com.sun.source.tree.SynchronizedTree; import com.sun.source.tree.ThrowTree; import com.sun.source.tree.Tree; @@ -257,10 +254,10 @@ public Void visitMemberSelect(MemberSelectTree tree, Void p) { * I would not be at all surprised if there are additional cases that we still haven't covered. */ Tree typeToCheckForAnnotations = - expression.getKind() == PARAMETERIZED_TYPE + expression instanceof ParameterizedTypeTree ? ((ParameterizedTypeTree) expression).getType() : expression; - if (typeToCheckForAnnotations.getKind() == ANNOTATED_TYPE) { + if (typeToCheckForAnnotations instanceof AnnotatedTypeTree) { /* * In all cases in which we report outer.annotated, we know that we're dealing with a true * inner class (`@Nullable Outer.Inner`), not a static nested class (`@Nullable Map.Entry`). @@ -474,7 +471,7 @@ public Void visitVariable(VariableTree tree, Void p) { if (isPrimitiveOrArrayOfPrimitive(tree.getType())) { checkNoNullnessAnnotations(tree, annotations, "primitive.annotated"); - } else if (tree.getType().getKind() == MEMBER_SELECT) { + } else if (tree.getType() instanceof MemberSelectTree) { checkNoNullnessAnnotations(tree, annotations, "outer.annotated"); } @@ -488,9 +485,9 @@ public Void visitVariable(VariableTree tree, Void p) { * * I have an alternative that appears to work, but it could probably be simplified. */ - if (tree.getType().getKind() == ARRAY_TYPE) { + if (tree.getType() instanceof ArrayTypeTree) { checkNoNullnessAnnotationsOnArrayItself(tree, "local.variable.annotated"); - } else if (tree.getType().getKind() == ANNOTATED_TYPE) { + } else if (tree.getType() instanceof AnnotatedTypeTree) { checkNoNullnessAnnotations( tree, ((AnnotatedTypeTree) tree.getType()).getAnnotations(), @@ -514,7 +511,7 @@ public void processMethodTree(String className, MethodTree tree) { if (returnType != null) { if (isPrimitiveOrArrayOfPrimitive(returnType)) { checkNoNullnessAnnotations(tree, annotations, "primitive.annotated"); - } else if (returnType.getKind() == MEMBER_SELECT) { + } else if (returnType instanceof MemberSelectTree) { checkNoNullnessAnnotations(tree, annotations, "outer.annotated"); } } @@ -556,18 +553,18 @@ public void processClassTree(ClassTree tree) { } private boolean isPrimitiveOrArrayOfPrimitive(Tree type) { - return type.getKind() == PRIMITIVE_TYPE - || (type.getKind() == ARRAY_TYPE + return type instanceof PrimitiveTypeTree + || (type instanceof ArrayTypeTree && isPrimitiveOrArrayOfPrimitive(((ArrayTypeTree) type).getType())); } private void checkNoNullnessAnnotationsOnArrayItself( VariableTree treeToReportOn, String messageKey) { Tree tree = treeToReportOn.getType(); - while (tree.getKind() == ARRAY_TYPE) { + while (tree instanceof ArrayTypeTree) { tree = ((ArrayTypeTree) tree).getType(); } - if (tree.getKind() == ANNOTATED_TYPE) { + if (tree instanceof AnnotatedTypeTree) { checkNoNullnessAnnotations( treeToReportOn, ((AnnotatedTypeTree) tree).getAnnotations(), messageKey); } diff --git a/src/test/java/tests/NullSpecTest.java b/src/test/java/tests/NullSpecTest.java index 84241f5..9a2e49c 100644 --- a/src/test/java/tests/NullSpecTest.java +++ b/src/test/java/tests/NullSpecTest.java @@ -202,14 +202,14 @@ private boolean corresponds(TestDiagnostic missing, TestDiagnostic unexpected) { } case "jspecify_unrecognized_location": switch (unexpected.getMessageKey()) { - /* - * We'd rather avoid this `bound` error (in part because it suggests that the annotation - * is having some effect, which we don't want!), but the most important thing is that the - * checker is issuing one or more errors when someone annotates a type-parameter - * declaration. The second most important thing is that the errors issued include our - * custom `*.annotated` error. This test probably doesn't confirm that second thing - * anymore, but I did manually confirm that it is true as of this writing. - */ + /* + * We'd rather avoid this `bound` error (in part because it suggests that the annotation + * is having some effect, which we don't want!), but the most important thing is that the + * checker is issuing one or more errors when someone annotates a type-parameter + * declaration. The second most important thing is that the errors issued include our + * custom `*.annotated` error. This test probably doesn't confirm that second thing + * anymore, but I did manually confirm that it is true as of this writing. + */ case "bound.type.incompatible": case "local.variable.annotated": case "type.parameter.annotated": diff --git a/usage-demo/build.gradle b/usage-demo/build.gradle index e24b982..13b1ca0 100644 --- a/usage-demo/build.gradle +++ b/usage-demo/build.gradle @@ -2,14 +2,14 @@ plugins { id 'java' - id 'com.diffplug.spotless' version '6.25.0' - id 'net.ltgt.errorprone' version '4.1.0' + id 'com.diffplug.spotless' version '7.2.1' + id 'net.ltgt.errorprone' version '4.3.0' id 'org.checkerframework' version '0.6.48' apply false } ext { versions = [ - eisopVersion: '3.42.0-eisop5', + eisopVersion: '3.49.3-eisop1', jspecifyVersion: '1.0.0', jspecifyReferenceCheckerVersion: '0.0.0-SNAPSHOT' ] @@ -22,7 +22,7 @@ repositories { // Project dependencies, e.g. error prone. dependencies { - errorprone 'com.google.errorprone:error_prone_core:2.36.0' + errorprone 'com.google.errorprone:error_prone_core:2.42.0' } // Dependency on JSpecify annotations. @@ -65,7 +65,7 @@ spotless { groovyGradle { target '**/*.gradle' greclipse() - indentWithSpaces(4) + leadingTabsToSpaces(4) trimTrailingWhitespace() } }