Skip to content

Commit

Permalink
Merge branch 'master' into try-method-match
Browse files Browse the repository at this point in the history
  • Loading branch information
msridhar authored Jul 15, 2023
2 parents bc3b181 + 61abe56 commit df25816
Show file tree
Hide file tree
Showing 27 changed files with 76 additions and 20 deletions.
13 changes: 4 additions & 9 deletions .github/workflows/continuous-integration.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down
22 changes: 16 additions & 6 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion config/hooks/pre-commit
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,7 @@ private static String getAstubxSignature(IMethod mtd) {
+ strArgTypes
+ ")";
}

/**
* Get simple unqualified type name.
*
Expand Down
7 changes: 7 additions & 0 deletions nullaway/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
12 changes: 12 additions & 0 deletions nullaway/src/main/java/com/uber/nullaway/NullabilityUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -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}.
*
* <p>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<Symbol> 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)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ public class NullnessStore implements Store<NullnessStore> {
private NullnessStore(Map<AccessPath, Nullness> contents) {
this.contents = ImmutableMap.copyOf(contents);
}

/**
* Produce an empty store.
*
Expand Down Expand Up @@ -139,6 +140,7 @@ public AccessPath getMapGetIteratorContentsAccessPath(LocalVariableNode iterator
}
return null;
}

/**
* Gets the {@link Nullness} value of an access path.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -66,6 +69,7 @@ public Serializer(FixSerializationConfig config, SerializationAdapter serializat
serializeVersion(outputDirectory);
initializeOutputFiles(config);
}

/**
* Appends the string representation of the {@link SuggestedNullableFixInfo}.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -122,7 +123,7 @@ public ImmutableSet<String> 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)) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
package com.uber.nullaway.handlers;

/*
* Copyright (c) 2017 Uber Technologies, Inc.
*
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
package com.uber.nullaway.handlers.stream;

/*
* Copyright (c) 2017 Uber Technologies, Inc.
*
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
package com.uber.nullaway.handlers.stream;

/*
* Copyright (c) 2017 Uber Technologies, Inc.
*
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
package com.uber.nullaway.handlers.stream;

/*
* Copyright (c) 2017 Uber Technologies, Inc.
*
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
package com.uber.nullaway.handlers.stream;

/*
* Copyright (c) 2017 Uber Technologies, Inc.
*
Expand Down
6 changes: 6 additions & 0 deletions sample-app/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -72,3 +72,9 @@ dependencies {
testImplementation deps.test.junit4

}

spotless {
java {
target 'src/*/java/**/*.java'
}
}
1 change: 0 additions & 1 deletion settings.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down

0 comments on commit df25816

Please sign in to comment.