diff --git a/.github/workflows/continuous-integration.yml b/.github/workflows/continuous-integration.yml index 517ebe3c5e..aeb938f2ce 100644 --- a/.github/workflows/continuous-integration.yml +++ b/.github/workflows/continuous-integration.yml @@ -16,6 +16,9 @@ jobs: - os: ubuntu-latest java: 11 epVersion: 2.4.0 + - os: ubuntu-latest + java: 17 + epVersion: 2.4.0 - os: macos-latest java: 11 epVersion: 2.20.0 @@ -44,20 +47,12 @@ jobs: with: java-version: ${{ matrix.java }} distribution: 'temurin' - - name: Build and test using Java 11 and Error Prone ${{ matrix.epVersion }} - env: - ORG_GRADLE_PROJECT_epApiVersion: ${{ matrix.epVersion }} - uses: gradle/gradle-build-action@v2 - with: - arguments: verGJF build - if: matrix.java == '11' - - name: Build and test using Java 17 and Error Prone ${{ matrix.epVersion }} + - name: Build and test using Java ${{ matrix.java }} and Error Prone ${{ matrix.epVersion }} env: ORG_GRADLE_PROJECT_epApiVersion: ${{ matrix.epVersion }} uses: gradle/gradle-build-action@v2 with: arguments: build - if: matrix.java == '17' - name: Report jacoco coverage uses: gradle/gradle-build-action@v2 env: diff --git a/build.gradle b/build.gradle index e5edbcdb97..f1f0717b6c 100644 --- a/build.gradle +++ b/build.gradle @@ -35,7 +35,7 @@ buildscript { } } plugins { - id "com.github.sherter.google-java-format" version "0.9" + id "com.diffplug.spotless" version "6.19.0" id "net.ltgt.errorprone" version "3.0.1" apply false id "com.github.johnrengelman.shadow" version "8.1.0" apply false id "com.github.kt3k.coveralls" version "2.12.0" apply false @@ -103,14 +103,24 @@ subprojects { project -> google() } + // For some reason, spotless complains when applied to the jar-infer folder itself, even + // though there is no top-level :jar-infer project + if (project.name != "jar-infer") { + project.apply plugin: "com.diffplug.spotless" + spotless { + java { + googleJavaFormat() + } + } + } } -googleJavaFormat { - toolVersion = "1.14.0" - // don't enforce formatting for generated Java code under buildSrc - exclude 'buildSrc/build/**/*.java' +spotless { + predeclareDeps() +} +spotlessPredeclare { + java { googleJavaFormat('1.17.0') } } - //////////////////////////////////////////////////////////////////////// // // Google Java Format pre-commit hook installation diff --git a/config/hooks/pre-commit b/config/hooks/pre-commit index f31a3cf79c..ffdb168862 100755 --- a/config/hooks/pre-commit +++ b/config/hooks/pre-commit @@ -7,6 +7,6 @@ REPO_ROOT_DIR="$(git rev-parse --show-toplevel)" files=$((git diff --cached --name-only --diff-filter=ACMR | grep -Ei "\.java$") || true) if [ ! -z "${files}" ]; then comma_files=$(echo "$files" | paste -s -d "," -) - "${REPO_ROOT_DIR}/gradlew" goJF -DgoogleJavaFormat.include="$comma_files" &>/dev/null + "${REPO_ROOT_DIR}/gradlew" spotlessApply -Pspotless.ratchet.from=HEAD >/dev/null 2>&1 git add $(echo "$files" | paste -s -d " " -) fi diff --git a/jar-infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/DefinitelyDerefedParamsDriver.java b/jar-infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/DefinitelyDerefedParamsDriver.java index 5deb5cf229..6eb0c10c24 100644 --- a/jar-infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/DefinitelyDerefedParamsDriver.java +++ b/jar-infer/jar-infer-lib/src/main/java/com/uber/nullaway/jarinfer/DefinitelyDerefedParamsDriver.java @@ -510,6 +510,7 @@ private static String getAstubxSignature(IMethod mtd) { + strArgTypes + ")"; } + /** * Get simple unqualified type name. * diff --git a/nullaway/build.gradle b/nullaway/build.gradle index 8e3f3ce54e..af27fcf69b 100644 --- a/nullaway/build.gradle +++ b/nullaway/build.gradle @@ -92,6 +92,13 @@ test { // Accessed by Lombok tests "--add-opens=jdk.compiler/com.sun.tools.javac.jvm=ALL-UNNAMED", ] + if (deps.versions.errorProneApi == "2.4.0" && JavaVersion.current() >= JavaVersion.VERSION_17) { + // This test does not pass on JDK 17 with Error Prone 2.4.0 due to a Mockito incompatibility. Skip it (the + // test passes with more recent Error Prone versions on JDK 17) + filter { + excludeTestsMatching "com.uber.nullaway.NullAwaySerializationTest.suggestNullableArgumentOnBytecodeNoFileInfo" + } + } } apply plugin: 'com.vanniktech.maven.publish' diff --git a/nullaway/src/main/java/com/uber/nullaway/ErrorProneCLIFlagsConfig.java b/nullaway/src/main/java/com/uber/nullaway/ErrorProneCLIFlagsConfig.java index fe3cf8a3b7..6e30e4e43f 100644 --- a/nullaway/src/main/java/com/uber/nullaway/ErrorProneCLIFlagsConfig.java +++ b/nullaway/src/main/java/com/uber/nullaway/ErrorProneCLIFlagsConfig.java @@ -83,6 +83,7 @@ final class ErrorProneCLIFlagsConfig extends AbstractConfig { static final String FL_JI_REGEX_MODEL_PATH = EP_FL_NAMESPACE + ":JarInferRegexStripModelJar"; static final String FL_JI_REGEX_CODE_PATH = EP_FL_NAMESPACE + ":JarInferRegexStripCodeJar"; static final String FL_ERROR_URL = EP_FL_NAMESPACE + ":ErrorURL"; + /** --- Serialization configs --- */ static final String FL_FIX_SERIALIZATION = EP_FL_NAMESPACE + ":SerializeFixMetadata"; diff --git a/nullaway/src/main/java/com/uber/nullaway/LibraryModels.java b/nullaway/src/main/java/com/uber/nullaway/LibraryModels.java index a69bbcd846..d801d40036 100644 --- a/nullaway/src/main/java/com/uber/nullaway/LibraryModels.java +++ b/nullaway/src/main/java/com/uber/nullaway/LibraryModels.java @@ -151,6 +151,7 @@ public interface LibraryModels { final class MethodRef { public final String enclosingClass; + /** * we store the method name separately to enable fast comparison with MethodSymbols. See {@link * com.uber.nullaway.handlers.LibraryModelsHandler.OptimizedLibraryModels} diff --git a/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java b/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java index 54a4c37710..880909213f 100644 --- a/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java +++ b/nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java @@ -176,6 +176,18 @@ public static TreePath findEnclosingMethodOrLambdaOrInitializer(TreePath path) { return findEnclosingMethodOrLambdaOrInitializer(path, ImmutableSet.of()); } + /** + * A wrapper for {@link Symbol#getEnclosedElements} to avoid binary compatibility issues for + * covariant overrides in subtypes of {@link Symbol}. + * + *

Same as this ASTHelpers method in Error Prone: + * https://github.com/google/error-prone/blame/a1318e4b0da4347dff7508108835d77c470a7198/check_api/src/main/java/com/google/errorprone/util/ASTHelpers.java#L1148 + * TODO: delete this method and switch to ASTHelpers once we can require Error Prone 2.20.0 + */ + public static List getEnclosedElements(Symbol symbol) { + return symbol.getEnclosedElements(); + } + /** * NOTE: this method does not work for getting all annotations of parameters of methods from class * files. For that case, use {@link #getAllAnnotationsForParameter(Symbol.MethodSymbol, int)} diff --git a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.java b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.java index 4a90c1fbf5..056344a53f 100644 --- a/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.java +++ b/nullaway/src/main/java/com/uber/nullaway/dataflow/AccessPath.java @@ -191,6 +191,7 @@ private static AccessPath fromVanillaMethodCall( static AccessPath switchRoot(AccessPath origAP, Element newRoot) { return new AccessPath(newRoot, origAP.elements, origAP.mapGetArg); } + /** * Construct the access path given a {@code base.element} structure. * diff --git a/nullaway/src/main/java/com/uber/nullaway/dataflow/NullnessStore.java b/nullaway/src/main/java/com/uber/nullaway/dataflow/NullnessStore.java index 3d82d7763e..732ed01920 100644 --- a/nullaway/src/main/java/com/uber/nullaway/dataflow/NullnessStore.java +++ b/nullaway/src/main/java/com/uber/nullaway/dataflow/NullnessStore.java @@ -50,6 +50,7 @@ public class NullnessStore implements Store { private NullnessStore(Map contents) { this.contents = ImmutableMap.copyOf(contents); } + /** * Produce an empty store. * @@ -139,6 +140,7 @@ public AccessPath getMapGetIteratorContentsAccessPath(LocalVariableNode iterator } return null; } + /** * Gets the {@link Nullness} value of an access path. * diff --git a/nullaway/src/main/java/com/uber/nullaway/fixserialization/FixSerializationConfig.java b/nullaway/src/main/java/com/uber/nullaway/fixserialization/FixSerializationConfig.java index 2e2df8faee..5ffca638ce 100644 --- a/nullaway/src/main/java/com/uber/nullaway/fixserialization/FixSerializationConfig.java +++ b/nullaway/src/main/java/com/uber/nullaway/fixserialization/FixSerializationConfig.java @@ -49,6 +49,7 @@ public class FixSerializationConfig { * untouched. */ public final boolean suggestEnabled; + /** * If enabled, serialized information of a fix suggest will also include the enclosing method and * class of the element involved in error. Finding enclosing elements is costly and will only be diff --git a/nullaway/src/main/java/com/uber/nullaway/fixserialization/Serializer.java b/nullaway/src/main/java/com/uber/nullaway/fixserialization/Serializer.java index db6d8c7250..3adb94bbb7 100644 --- a/nullaway/src/main/java/com/uber/nullaway/fixserialization/Serializer.java +++ b/nullaway/src/main/java/com/uber/nullaway/fixserialization/Serializer.java @@ -46,10 +46,13 @@ public class Serializer { /** Path to write errors. */ private final Path errorOutputPath; + /** Path to write suggested fix metadata. */ private final Path suggestedFixesOutputPath; + /** Path to write suggested fix metadata. */ private final Path fieldInitializationOutputPath; + /** * Adapter used to serialize outputs. This adapter is capable of serializing outputs according to * the requested serilization version and maintaining backward compatibility with previous @@ -66,6 +69,7 @@ public Serializer(FixSerializationConfig config, SerializationAdapter serializat serializeVersion(outputDirectory); initializeOutputFiles(config); } + /** * Appends the string representation of the {@link SuggestedNullableFixInfo}. * diff --git a/nullaway/src/main/java/com/uber/nullaway/fixserialization/location/AbstractSymbolLocation.java b/nullaway/src/main/java/com/uber/nullaway/fixserialization/location/AbstractSymbolLocation.java index 75a9a455b9..319c636e48 100644 --- a/nullaway/src/main/java/com/uber/nullaway/fixserialization/location/AbstractSymbolLocation.java +++ b/nullaway/src/main/java/com/uber/nullaway/fixserialization/location/AbstractSymbolLocation.java @@ -38,8 +38,10 @@ public abstract class AbstractSymbolLocation implements SymbolLocation { /** Element kind of the targeted symbol */ protected final ElementKind type; + /** Path of the file containing the symbol, if available. */ @Nullable protected final Path path; + /** Enclosing class of the symbol. */ protected final Symbol.ClassSymbol enclosingClass; diff --git a/nullaway/src/main/java/com/uber/nullaway/fixserialization/location/MethodParameterLocation.java b/nullaway/src/main/java/com/uber/nullaway/fixserialization/location/MethodParameterLocation.java index ff4f0a542e..7a1c22f9f4 100644 --- a/nullaway/src/main/java/com/uber/nullaway/fixserialization/location/MethodParameterLocation.java +++ b/nullaway/src/main/java/com/uber/nullaway/fixserialization/location/MethodParameterLocation.java @@ -33,8 +33,10 @@ public class MethodParameterLocation extends AbstractSymbolLocation { /** Symbol of the targeted method. */ private final Symbol.MethodSymbol enclosingMethod; + /** Symbol of the targeted method parameter. */ private final Symbol.VarSymbol paramSymbol; + /** Index of the method parameter in the containing method's argument list. */ private final int index; diff --git a/nullaway/src/main/java/com/uber/nullaway/fixserialization/out/ErrorInfo.java b/nullaway/src/main/java/com/uber/nullaway/fixserialization/out/ErrorInfo.java index e6311c2d94..eb4b21fd57 100644 --- a/nullaway/src/main/java/com/uber/nullaway/fixserialization/out/ErrorInfo.java +++ b/nullaway/src/main/java/com/uber/nullaway/fixserialization/out/ErrorInfo.java @@ -45,6 +45,7 @@ public class ErrorInfo { * target, and this field is the Symbol for that target. */ @Nullable private final Symbol nonnullTarget; + /** * In cases where {@link ErrorInfo#nonnullTarget} is {@code null}, we serialize this value at its * placeholder in the output tsv file. @@ -54,6 +55,7 @@ public class ErrorInfo { /** Offset of program point where this error is reported. */ private final int offset; + /** Path to the containing source file where this error is reported. */ @Nullable private final Path path; diff --git a/nullaway/src/main/java/com/uber/nullaway/fixserialization/out/FieldInitializationInfo.java b/nullaway/src/main/java/com/uber/nullaway/fixserialization/out/FieldInitializationInfo.java index dac0c6d117..1a23c81d36 100644 --- a/nullaway/src/main/java/com/uber/nullaway/fixserialization/out/FieldInitializationInfo.java +++ b/nullaway/src/main/java/com/uber/nullaway/fixserialization/out/FieldInitializationInfo.java @@ -35,6 +35,7 @@ public class FieldInitializationInfo { /** Symbol of the initializer method. */ private final SymbolLocation initializerMethodLocation; + /** Symbol of the initialized class field. */ private final Symbol field; diff --git a/nullaway/src/main/java/com/uber/nullaway/fixserialization/out/SuggestedNullableFixInfo.java b/nullaway/src/main/java/com/uber/nullaway/fixserialization/out/SuggestedNullableFixInfo.java index fb271cd644..3c5c661239 100644 --- a/nullaway/src/main/java/com/uber/nullaway/fixserialization/out/SuggestedNullableFixInfo.java +++ b/nullaway/src/main/java/com/uber/nullaway/fixserialization/out/SuggestedNullableFixInfo.java @@ -34,6 +34,7 @@ public class SuggestedNullableFixInfo { /** SymbolLocation of the target element in source code. */ private final SymbolLocation symbolLocation; + /** Error which will be resolved by this type change. */ private final ErrorMessage errorMessage; diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/AbstractFieldContractHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/AbstractFieldContractHandler.java index 20ad5cdc62..c458f3200a 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/AbstractFieldContractHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/AbstractFieldContractHandler.java @@ -48,6 +48,7 @@ public abstract class AbstractFieldContractHandler extends BaseNoOpHandler { protected static final String THIS_NOTATION = "this."; + /** Simple name of the annotation in {@code String} */ protected final String annotName; @@ -221,7 +222,7 @@ protected boolean validateAnnotationSyntax( public static @Nullable VariableElement getInstanceFieldOfClass( Symbol.ClassSymbol classSymbol, String name) { Preconditions.checkNotNull(classSymbol); - for (Element member : classSymbol.getEnclosedElements()) { + for (Element member : NullabilityUtil.getEnclosedElements(classSymbol)) { if (member.getKind().isField() && !member.getModifiers().contains(Modifier.STATIC)) { if (member.getSimpleName().toString().equals(name)) { return (VariableElement) member; diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/ApacheThriftIsSetHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/ApacheThriftIsSetHandler.java index 2b36cdeb9a..25c4210591 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/ApacheThriftIsSetHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/ApacheThriftIsSetHandler.java @@ -30,6 +30,7 @@ import com.sun.tools.javac.code.Type; import com.sun.tools.javac.code.Types; import com.uber.nullaway.NullAway; +import com.uber.nullaway.NullabilityUtil; import com.uber.nullaway.Nullness; import com.uber.nullaway.dataflow.AccessPath; import com.uber.nullaway.dataflow.AccessPathNullnessPropagation; @@ -142,7 +143,7 @@ private FieldAndGetterElements getFieldAndGetterForProperty( Element getter = null; String fieldName = decapitalize(capPropName); String getterName = "get" + capPropName; - for (Symbol elem : symbol.owner.getEnclosedElements()) { + for (Symbol elem : NullabilityUtil.getEnclosedElements(symbol.owner)) { if (elem.getKind().isField() && elem.getSimpleName().toString().equals(fieldName)) { if (field != null) { throw new RuntimeException("already found field " + fieldName); diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/GrpcHandler.java b/nullaway/src/main/java/com/uber/nullaway/handlers/GrpcHandler.java index 0e27408718..ed5e620db6 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/GrpcHandler.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/GrpcHandler.java @@ -35,6 +35,7 @@ import com.sun.tools.javac.code.Type; import com.sun.tools.javac.code.Types; import com.uber.nullaway.NullAway; +import com.uber.nullaway.NullabilityUtil; import com.uber.nullaway.Nullness; import com.uber.nullaway.dataflow.AccessPath; import com.uber.nullaway.dataflow.AccessPathNullnessPropagation; @@ -122,7 +123,7 @@ public ImmutableSet onRegisterImmutableTypes() { private Symbol.MethodSymbol getGetterForMetadataSubtype( Symbol.ClassSymbol classSymbol, Types types) { // Is there a better way than iteration? - for (Symbol elem : classSymbol.getEnclosedElements()) { + for (Symbol elem : NullabilityUtil.getEnclosedElements(classSymbol)) { if (elem.getKind().equals(ElementKind.METHOD)) { Symbol.MethodSymbol methodSymbol = (Symbol.MethodSymbol) elem; if (grpcIsMetadataGetCall(methodSymbol, types)) { diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/StreamNullabilityPropagatorFactory.java b/nullaway/src/main/java/com/uber/nullaway/handlers/StreamNullabilityPropagatorFactory.java index 2fb0e5a9c2..0642296b8e 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/StreamNullabilityPropagatorFactory.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/StreamNullabilityPropagatorFactory.java @@ -1,4 +1,5 @@ package com.uber.nullaway.handlers; + /* * Copyright (c) 2017 Uber Technologies, Inc. * diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/stream/MaplikeMethodRecord.java b/nullaway/src/main/java/com/uber/nullaway/handlers/stream/MaplikeMethodRecord.java index a2abfd0a1b..cb2a34e7e0 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/stream/MaplikeMethodRecord.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/stream/MaplikeMethodRecord.java @@ -1,4 +1,5 @@ package com.uber.nullaway.handlers.stream; + /* * Copyright (c) 2017 Uber Technologies, Inc. * diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/stream/MaplikeToFilterInstanceRecord.java b/nullaway/src/main/java/com/uber/nullaway/handlers/stream/MaplikeToFilterInstanceRecord.java index 908931cc56..74c932e33c 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/stream/MaplikeToFilterInstanceRecord.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/stream/MaplikeToFilterInstanceRecord.java @@ -1,4 +1,5 @@ package com.uber.nullaway.handlers.stream; + /* * Copyright (c) 2017 Uber Technologies, Inc. * diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/stream/StreamModelBuilder.java b/nullaway/src/main/java/com/uber/nullaway/handlers/stream/StreamModelBuilder.java index 2423966229..e0381832d3 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/stream/StreamModelBuilder.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/stream/StreamModelBuilder.java @@ -1,4 +1,5 @@ package com.uber.nullaway.handlers.stream; + /* * Copyright (c) 2017 Uber Technologies, Inc. * diff --git a/nullaway/src/main/java/com/uber/nullaway/handlers/stream/StreamTypeRecord.java b/nullaway/src/main/java/com/uber/nullaway/handlers/stream/StreamTypeRecord.java index 4167ec9c8b..fc1caf696e 100644 --- a/nullaway/src/main/java/com/uber/nullaway/handlers/stream/StreamTypeRecord.java +++ b/nullaway/src/main/java/com/uber/nullaway/handlers/stream/StreamTypeRecord.java @@ -1,4 +1,5 @@ package com.uber.nullaway.handlers.stream; + /* * Copyright (c) 2017 Uber Technologies, Inc. * diff --git a/sample-app/build.gradle b/sample-app/build.gradle index 7b64db306e..595a305694 100644 --- a/sample-app/build.gradle +++ b/sample-app/build.gradle @@ -72,3 +72,9 @@ dependencies { testImplementation deps.test.junit4 } + +spotless { + java { + target 'src/*/java/**/*.java' + } +} diff --git a/settings.gradle b/settings.gradle index 3f7ee39b83..b1fe598c56 100644 --- a/settings.gradle +++ b/settings.gradle @@ -13,7 +13,6 @@ include ':sample' include ':test-java-lib' include ':test-java-lib-lombok' include ':test-library-models' -include ':compile-bench' include ':jar-infer:android-jarinfer-models-sdk28' include ':jar-infer:android-jarinfer-models-sdk29' include ':jar-infer:android-jarinfer-models-sdk30'