diff --git a/.github/workflows/continuous-integration.yml b/.github/workflows/continuous-integration.yml index e7938276d..85316f26c 100644 --- a/.github/workflows/continuous-integration.yml +++ b/.github/workflows/continuous-integration.yml @@ -56,4 +56,15 @@ jobs: uses: actions/upload-artifact@v2 with: name: Test report - path: build/reports/tests/test \ No newline at end of file + path: build/reports/tests/test + build-with-nullaway-serialization-0: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - name: Set up JDK 11 + uses: actions/setup-java@v2 + with: + java-version: 11 + distribution: 'temurin' + - name: Build with Gradle + run: ./gradlew :core:test --scan -Pnullaway-serialization-format-version=0 diff --git a/core/build.gradle b/core/build.gradle index 578b7aefb..4054b6eb9 100644 --- a/core/build.gradle +++ b/core/build.gradle @@ -48,12 +48,15 @@ dependencies { testImplementation deps.build.commonsio } +// Should be the latest supporting version of NullAway. +def NULLAWAY_TEST = "0.10.5" + tasks.test.dependsOn(':type-annotator-scanner:publishToMavenLocal') tasks.test.dependsOn(':qual:publishToMavenLocal') // Set up environment variables for test configuration tu run with the latest development version. tasks.test.doFirst { - environment "NULLAWAY_TEST_VERSION", "0.10.0" + environment "NULLAWAY_TEST_VERSION", NULLAWAY_TEST environment "ANNOTATOR_VERSION", project.version } @@ -83,3 +86,24 @@ jar { shadowJar { archiveClassifier = null } + +// Configure test environment +def nullawayVersionMap = [0:"0.10.4", 1:"0.10.5"] +tasks.create("configureNullAwayVersion"){ + if(!project.hasProperty("nullaway-serialization-format-version")){ + return + } + NULLAWAY_TEST = nullawayVersionMap.get((project.getProperty("nullaway-serialization-format-version")) as int) + println "NullAway Test version changed to: " + NULLAWAY_TEST + + switch (NULLAWAY_TEST){ + case "0.10.4": + test { + filter { + excludeTest "edu.ucr.cs.riple.core.CoreTest", "errorInFieldDeclarationForceResolveTest" + } + } + break + } +} +tasks.test.dependsOn("configureNullAwayVersion") diff --git a/core/src/main/java/edu/ucr/cs/riple/core/Annotator.java b/core/src/main/java/edu/ucr/cs/riple/core/Annotator.java index 690c13831..65e61e6c1 100644 --- a/core/src/main/java/edu/ucr/cs/riple/core/Annotator.java +++ b/core/src/main/java/edu/ucr/cs/riple/core/Annotator.java @@ -38,9 +38,11 @@ import edu.ucr.cs.riple.core.injectors.PhysicalInjector; import edu.ucr.cs.riple.core.metadata.field.FieldDeclarationAnalysis; import edu.ucr.cs.riple.core.metadata.field.FieldInitializationAnalysis; +import edu.ucr.cs.riple.core.metadata.index.Bank; import edu.ucr.cs.riple.core.metadata.index.Error; import edu.ucr.cs.riple.core.metadata.index.Fix; import edu.ucr.cs.riple.core.metadata.method.MethodDeclarationTree; +import edu.ucr.cs.riple.core.metadata.method.MethodNode; import edu.ucr.cs.riple.core.metadata.trackers.CompoundTracker; import edu.ucr.cs.riple.core.metadata.trackers.RegionTracker; import edu.ucr.cs.riple.core.util.Utility; @@ -49,8 +51,10 @@ import edu.ucr.cs.riple.injector.changes.AddSingleElementAnnotation; import edu.ucr.cs.riple.injector.location.OnField; import edu.ucr.cs.riple.injector.location.OnMethod; -import edu.ucr.cs.riple.scanner.Serializer; +import edu.ucr.cs.riple.injector.location.OnParameter; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.Set; import java.util.stream.Collectors; @@ -100,13 +104,13 @@ private void preprocess() { Utility.setScannerCheckerActivation(config.target, true); System.out.println("Making the first build..."); Utility.buildTarget(config, true); + config.initializeAdapter(); Set uninitializedFields = Utility.readFixesFromOutputDirectory(config.target, Fix.factory(config, null)).stream() .filter(fix -> fix.isOnField() && fix.reasons.contains("FIELD_NO_INIT")) .map(Fix::toField) .collect(Collectors.toSet()); - FieldInitializationAnalysis analysis = - new FieldInitializationAnalysis(config.target.dir.resolve("field_init.tsv")); + FieldInitializationAnalysis analysis = new FieldInitializationAnalysis(config); Set initializers = analysis .findInitializers(uninitializedFields) @@ -120,9 +124,9 @@ private void annotate() { Utility.setScannerCheckerActivation(config.target, true); Utility.buildTarget(config); Utility.setScannerCheckerActivation(config.target, false); - FieldDeclarationAnalysis fieldDeclarationAnalysis = new FieldDeclarationAnalysis(config.target); - MethodDeclarationTree tree = - new MethodDeclarationTree(config.target.dir.resolve(Serializer.METHOD_INFO_FILE_NAME)); + FieldDeclarationAnalysis fieldDeclarationAnalysis = + new FieldDeclarationAnalysis(config, config.target); + MethodDeclarationTree tree = new MethodDeclarationTree(config); // globalAnalyzer analyzes effects of all public APIs on downstream dependencies. // Through iterations, since the source code for downstream dependencies does not change and the // computation does not depend on the changes in the target module, it will compute the same @@ -152,7 +156,7 @@ private void annotate() { } if (config.forceResolveActivated) { - forceResolveRemainingErrors(fieldDeclarationAnalysis, tree); + forceResolveRemainingErrors(fieldDeclarationAnalysis, tree, globalAnalyzer); } System.out.println("\nFinished annotating."); @@ -220,9 +224,8 @@ private ImmutableSet processTriggeredFixes( .collect(ImmutableSet.toImmutableSet()); // Initializing required explorer instances. - MethodDeclarationTree tree = - new MethodDeclarationTree(config.target.dir.resolve(Serializer.METHOD_INFO_FILE_NAME)); - RegionTracker tracker = new CompoundTracker(config.target, tree); + MethodDeclarationTree tree = new MethodDeclarationTree(config); + RegionTracker tracker = new CompoundTracker(config, config.target, tree); TargetModuleSupplier supplier = new TargetModuleSupplier(config, tree); Explorer explorer = config.exhaustiveSearch @@ -248,63 +251,122 @@ private ImmutableSet processTriggeredFixes( * * @param fieldDeclarationAnalysis Field declaration analysis. * @param tree Method Declaration analysis. + * @param analyzer Global analyzer instance, used to compute the effect of {@code @NullUnmarked} + * injections globally */ private void forceResolveRemainingErrors( - FieldDeclarationAnalysis fieldDeclarationAnalysis, MethodDeclarationTree tree) { + FieldDeclarationAnalysis fieldDeclarationAnalysis, + MethodDeclarationTree tree, + GlobalAnalyzer analyzer) { // Collect regions with remaining errors. Utility.buildTarget(config); - List remainingErrors = Utility.readErrorsFromOutputDirectory(config.target); + List remainingErrors = Utility.readErrorsFromOutputDirectory(config, config.target); Set remainingFixes = Utility.readFixesFromOutputDirectory( config.target, Fix.factory(config, fieldDeclarationAnalysis)); + Bank errorBank = + new Bank<>(config.target.dir.resolve("errors.tsv"), Error.factory(config)); + // Collect all regions for NullUnmarked. // For all errors in regions which correspond to a method's body, we can add @NullUnmarked at // the method level. - Set nullUnMarkedAnnotations = + Map methodCommentMap = new HashMap<>(); + Set nullUnmarkedMethods = remainingErrors.stream() // find the corresponding method nodes. .map( error -> { - if (!error.encMethod().equals("null")) { - return tree.findNode(error.encMethod(), error.encClass()); - } - if (error.nonnullTarget == null) { - // Just a sanity check. - return null; + if (error.getRegion().isOnMethod()) { + return tree.findNode(error.encMember(), error.encClass()); } // For methods invoked in an initialization region, where the error is that // `@Nullable` is being passed as an argument, we add a `@NullUnmarked` annotation // to the called method. - if (error.messageType.equals("PASS_NULLABLE")) { - OnMethod calledMethod = error.nonnullTarget.toMethod(); + if (error.messageType.equals("PASS_NULLABLE") + && error.nonnullTarget != null + && error.nonnullTarget.isOnParameter()) { + OnParameter calledMethod = error.nonnullTarget.toParameter(); return tree.findNode(calledMethod.method, calledMethod.clazz); } return null; }) + // Filter null values from map above. .filter(Objects::nonNull) - .map(node -> new AddMarkerAnnotation(node.location, config.nullUnMarkedAnnotation)) + .map( + node -> { + methodCommentMap.put( + node.location, + "//local " + + errorBank + .getEntriesByRegion(node.location.clazz, node.location.method) + .size()); + return node.location; + }) + .collect(Collectors.toSet()); + + if (config.commentGenerationEnabled) { + // Analyze impacts of the annotation locally and globally for comment generation. + remainingFixes.forEach( + fix -> { + if (fix.isOnMethod()) { + MethodNode node = tree.findNode(fix.toMethod().method, fix.toMethod().clazz); + if (node != null && !methodCommentMap.containsKey(node.location)) { + String comment = ""; + if (config.downStreamDependenciesAnalysisActivated + && node.isPublicMethodWithNonPrimitiveReturnType()) { + comment += " //dependent: " + analyzer.getTriggeredErrors(fix).size(); + } + Report report = cache.getReportForFix(fix); + if (report != null) { + comment += " //local: " + report.localEffect; + } + methodCommentMap.put(node.location, methodCommentMap.get(node.location) + comment); + } + } + }); + } + + Set nullUnmarkedInjections = + nullUnmarkedMethods.stream() + .map( + onMethod -> + new AddMarkerAnnotation( + onMethod, + config.nullUnMarkedAnnotation, + config.commentGenerationEnabled + ? config.commentPrefix + ":" + methodCommentMap.get(onMethod) + : null)) .collect(Collectors.toSet()); - injector.injectAnnotations(nullUnMarkedAnnotations); + injector.injectAnnotations(nullUnmarkedInjections); // Update log. - config.log.updateInjectedAnnotations(nullUnMarkedAnnotations); + config.log.updateInjectedAnnotations(nullUnmarkedInjections); - // Collect explicit Nullable initialization to fields - Set suppressWarningsAnnotations = - remainingFixes.stream() + // Collect suppress warnings, errors on field declaration regions. + Set fieldsWithSuppressWarnings = + remainingErrors.stream() .filter( - fix -> { - if (!(fix.isOnField() && fix.reasons.contains("ASSIGN_FIELD_NULLABLE"))) { + error -> { + if (!error.getRegion().isOnField()) { return false; } - OnField onField = fix.toField(); - return onField.clazz.equals(fix.encClass()) && fix.encMethod().equals(""); + // We can silence them by SuppressWarnings("NullAway.Init") + return !error.messageType.equals("METHOD_NO_INIT") + && !error.messageType.equals("FIELD_NO_INIT"); }) .map( - fix -> - new AddSingleElementAnnotation( - fix.toField(), "SuppressWarnings", "NullAway", false)) + error -> + fieldDeclarationAnalysis.getLocationOnField( + error.getRegion().clazz, error.getRegion().member)) + .filter(Objects::nonNull) + .collect(Collectors.toSet()); + + Set suppressWarningsAnnotations = + fieldsWithSuppressWarnings.stream() + .map( + onField -> + new AddSingleElementAnnotation(onField, "SuppressWarnings", "NullAway", false)) .collect(Collectors.toSet()); injector.injectAnnotations(suppressWarningsAnnotations); // Update log. @@ -318,12 +380,12 @@ private void forceResolveRemainingErrors( fix.isOnField() && (fix.reasons.contains("METHOD_NO_INIT") || fix.reasons.contains("FIELD_NO_INIT"))) + // Filter nodes annotated with SuppressWarnings("NullAway") + .filter(fix -> !fieldsWithSuppressWarnings.contains(fix.toField())) .map( fix -> new AddSingleElementAnnotation( fix.toField(), "SuppressWarnings", "NullAway.Init", false)) - // Exclude already annotated fields with a general NullAway suppress warning. - .filter(f -> !suppressWarningsAnnotations.contains(f)) .collect(Collectors.toSet()); injector.injectAnnotations(initializationSuppressWarningsAnnotations); // Update log. diff --git a/core/src/main/java/edu/ucr/cs/riple/core/Config.java b/core/src/main/java/edu/ucr/cs/riple/core/Config.java index 3a8bfbca9..27e5a78af 100644 --- a/core/src/main/java/edu/ucr/cs/riple/core/Config.java +++ b/core/src/main/java/edu/ucr/cs/riple/core/Config.java @@ -26,6 +26,9 @@ import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableSet; +import edu.ucr.cs.riple.core.adapters.NullAwayV0Adapter; +import edu.ucr.cs.riple.core.adapters.NullAwayV1Adapter; +import edu.ucr.cs.riple.core.adapters.NullAwayVersionAdapter; import edu.ucr.cs.riple.core.log.Log; import edu.ucr.cs.riple.core.util.Utility; import java.io.BufferedWriter; @@ -138,6 +141,21 @@ public class Config { */ public final boolean inferenceActivated; + /** + * This adapter is initialized lazily as it requires build of target to serialize its using + * NullAway serialization version. + */ + private NullAwayVersionAdapter adapter; + + /** + * If enabled, an informative comment explaining the impact of the annotation locally and globally + * will be injected along all {@code @NullUnmarked} injections. + */ + public final boolean commentGenerationEnabled; + + /** Prefix to all comments added by annotator; */ + public final String commentPrefix; + /** * Builds config from command line arguments. * @@ -304,6 +322,12 @@ public Config(String[] args) { deactivateInference.setRequired(false); options.addOption(deactivateInference); + // Comment generation activation + Option commentGenerationActivation = + new Option("acg", "activate-comment-generation", true, "Activates comment generation."); + commentGenerationActivation.setRequired(false); + options.addOption(commentGenerationActivation); + HelpFormatter formatter = new HelpFormatter(); CommandLineParser parser = new DefaultParser(); CommandLine cmd; @@ -397,6 +421,11 @@ public Config(String[] args) { this.forceResolveActivated ? cmd.getOptionValue(activateForceResolveOption) : "org.jspecify.nullness.NullUnmarked"; + this.commentGenerationEnabled = cmd.hasOption(commentGenerationActivation); + this.commentPrefix = + this.commentGenerationEnabled + ? cmd.getOptionValue(commentGenerationActivation) + : "Annotator"; this.moduleCounterID = 0; this.log = new Log(); this.log.reset(); @@ -479,10 +508,57 @@ public Config(Path configPath) { this.nullUnMarkedAnnotation = getValueFromKey(jsonObject, "ANNOTATION:NULL_UNMARKED", String.class) .orElse("org.jspecify.nullness.NullUnmarked"); + this.commentGenerationEnabled = + getValueFromKey(jsonObject, "COMMENT:ACTIVE", Boolean.class).orElse(false); + this.commentPrefix = + getValueFromKey(jsonObject, "COMMENT:PREFIX", String.class).orElse("Annotator"); this.log = new Log(); log.reset(); } + /** Initializes NullAway serialization adapter according to the serialized version. */ + public void initializeAdapter() { + if (adapter != null) { + // adapter is already initialized. + return; + } + Path serializationVersionPath = target.dir.resolve("serialization_version.txt"); + if (!serializationVersionPath.toFile().exists()) { + // Older versions of NullAway. + this.adapter = new NullAwayV0Adapter(this); + return; + } + try { + List lines = Files.readAllLines(serializationVersionPath); + int version = Integer.parseInt(lines.get(0)); + switch (version) { + case 0: + this.adapter = new NullAwayV0Adapter(this); + break; + case 1: + this.adapter = new NullAwayV1Adapter(this); + break; + default: + throw new RuntimeException("Unrecognized NullAway serialization version: " + version); + } + } catch (IOException e) { + throw new RuntimeException( + "Could not read serialization version at path: " + serializationVersionPath, e); + } + } + + /** + * Getter for adapter. + * + * @return adapter. + */ + public NullAwayVersionAdapter getAdapter() { + if (adapter == null) { + throw new IllegalStateException("Adapter is not initialized."); + } + return adapter; + } + /** * Returns the latest id associated to a module, used to create unique ids for each module and * increments it. diff --git a/core/src/main/java/edu/ucr/cs/riple/core/ReportCache.java b/core/src/main/java/edu/ucr/cs/riple/core/ReportCache.java index 4e716c9c8..428913458 100644 --- a/core/src/main/java/edu/ucr/cs/riple/core/ReportCache.java +++ b/core/src/main/java/edu/ucr/cs/riple/core/ReportCache.java @@ -106,4 +106,14 @@ public void disable() { public ImmutableSet reports() { return ImmutableSet.copyOf(store.values()); } + + /** + * Gets the corresponding report for the given fix. + * + * @param fix Given fix. + * @return Report of the processing of the fix. + */ + public Report getReportForFix(Fix fix) { + return store.get(fix); + } } diff --git a/core/src/main/java/edu/ucr/cs/riple/core/adapters/NullAwayV0Adapter.java b/core/src/main/java/edu/ucr/cs/riple/core/adapters/NullAwayV0Adapter.java new file mode 100644 index 000000000..d75dd626f --- /dev/null +++ b/core/src/main/java/edu/ucr/cs/riple/core/adapters/NullAwayV0Adapter.java @@ -0,0 +1,101 @@ +/* + * MIT License + * + * Copyright (c) 2022 Nima Karimipour + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +package edu.ucr.cs.riple.core.adapters; + +import com.google.common.base.Preconditions; +import edu.ucr.cs.riple.core.Config; +import edu.ucr.cs.riple.core.metadata.index.Error; +import edu.ucr.cs.riple.core.metadata.index.Fix; +import edu.ucr.cs.riple.core.metadata.trackers.Region; +import edu.ucr.cs.riple.core.metadata.trackers.TrackerNode; +import edu.ucr.cs.riple.injector.changes.AddMarkerAnnotation; +import edu.ucr.cs.riple.injector.location.Location; +import edu.ucr.cs.riple.injector.location.OnField; +import java.util.Arrays; +import java.util.Collections; +import java.util.Set; + +/** + * Adapter working with versions below: + * + *
    + *
  • NullAway: Serialization version 0 + *
  • Type Annotator Scanner: 1.3.3 or below + *
+ */ +public class NullAwayV0Adapter implements NullAwayVersionAdapter { + + /** Annotator config. */ + private final Config config; + + public NullAwayV0Adapter(Config config) { + this.config = config; + } + + @Override + public Fix deserializeFix(Location location, String[] values) { + Preconditions.checkArgument( + values.length == 10, + "Expected 10 values to create Fix instance in NullAway serialization version 0 but found: " + + values.length); + Preconditions.checkArgument( + values[7].equals("nullable"), "unsupported annotation: " + values[7]); + String encMember = !Region.getType(values[9]).equals(Region.Type.METHOD) ? "null" : values[9]; + return new Fix( + new AddMarkerAnnotation(location, config.nullableAnnot), + values[6], + new Region(values[8], encMember), + true); + } + + @Override + public Error deserializeError(String[] values) { + Preconditions.checkArgument( + values.length == 10, + "Expected 10 values to create Error instance in NullAway serialization version 0 but found: " + + values.length); + String encMember = !Region.getType(values[3]).equals(Region.Type.METHOD) ? "null" : values[3]; + return new Error( + values[0], + values[1], + new Region(values[2], encMember), + Location.createLocationFromArrayInfo(Arrays.copyOfRange(values, 4, 10))); + } + + @Override + public TrackerNode deserializeTrackerNode(String[] values) { + Preconditions.checkArgument( + values.length == 4, + "Expected 4 values to create TrackerNode instance in NullAway serialization version 0 but found: " + + values.length); + String encMember = !Region.getType(values[1]).equals(Region.Type.METHOD) ? "null" : values[1]; + return new TrackerNode(values[0], encMember, values[2], values[3]); + } + + @Override + public Set getFieldRegionScope(OnField onField) { + return Collections.singleton(new Region(onField.clazz, "null")); + } +} diff --git a/core/src/main/java/edu/ucr/cs/riple/core/adapters/NullAwayV1Adapter.java b/core/src/main/java/edu/ucr/cs/riple/core/adapters/NullAwayV1Adapter.java new file mode 100644 index 000000000..5bc3821b9 --- /dev/null +++ b/core/src/main/java/edu/ucr/cs/riple/core/adapters/NullAwayV1Adapter.java @@ -0,0 +1,100 @@ +/* + * MIT License + * + * Copyright (c) 2022 Nima Karimipour + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +package edu.ucr.cs.riple.core.adapters; + +import com.google.common.base.Preconditions; +import edu.ucr.cs.riple.core.Config; +import edu.ucr.cs.riple.core.metadata.index.Error; +import edu.ucr.cs.riple.core.metadata.index.Fix; +import edu.ucr.cs.riple.core.metadata.trackers.Region; +import edu.ucr.cs.riple.core.metadata.trackers.TrackerNode; +import edu.ucr.cs.riple.injector.changes.AddMarkerAnnotation; +import edu.ucr.cs.riple.injector.location.Location; +import edu.ucr.cs.riple.injector.location.OnField; +import java.util.Arrays; +import java.util.Set; +import java.util.stream.Collectors; + +/** + * Adapter working with versions below: + * + *
    + *
  • NullAway: Serialization version 1 + *
  • Type Annotator Scanner: 1.3.4 or above + *
+ */ +public class NullAwayV1Adapter implements NullAwayVersionAdapter { + + /** Annotator config. */ + private final Config config; + + public NullAwayV1Adapter(Config config) { + this.config = config; + } + + @Override + public Fix deserializeFix(Location location, String[] values) { + Preconditions.checkArgument( + values.length == 10, + "Expected 10 values to create Fix instance in NullAway serialization version 1 but found: " + + values.length); + Preconditions.checkArgument( + values[7].equals("nullable"), "unsupported annotation: " + values[7]); + return new Fix( + new AddMarkerAnnotation(location, config.nullableAnnot), + values[6], + new Region(values[8], values[9]), + true); + } + + @Override + public Error deserializeError(String[] values) { + Preconditions.checkArgument( + values.length == 10, + "Expected 10 values to create Error instance in NullAway serialization version 1 but found: " + + values.length); + return new Error( + values[0], + values[1], + new Region(values[2], values[3]), + Location.createLocationFromArrayInfo(Arrays.copyOfRange(values, 4, 10))); + } + + @Override + public TrackerNode deserializeTrackerNode(String[] values) { + Preconditions.checkArgument( + values.length == 4, + "Expected 4 values to create TrackerNode instance in NullAway serialization version 1 but found: " + + values.length); + return new TrackerNode(values[0], values[1], values[2], values[3]); + } + + @Override + public Set getFieldRegionScope(OnField onField) { + return onField.variables.stream() + .map(fieldName -> new Region(onField.clazz, fieldName)) + .collect(Collectors.toSet()); + } +} diff --git a/core/src/main/java/edu/ucr/cs/riple/core/adapters/NullAwayVersionAdapter.java b/core/src/main/java/edu/ucr/cs/riple/core/adapters/NullAwayVersionAdapter.java new file mode 100644 index 000000000..e78ccbd48 --- /dev/null +++ b/core/src/main/java/edu/ucr/cs/riple/core/adapters/NullAwayVersionAdapter.java @@ -0,0 +1,77 @@ +/* + * MIT License + * + * Copyright (c) 2022 Nima Karimipour + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +package edu.ucr.cs.riple.core.adapters; + +import edu.ucr.cs.riple.core.metadata.index.Error; +import edu.ucr.cs.riple.core.metadata.index.Fix; +import edu.ucr.cs.riple.core.metadata.trackers.Region; +import edu.ucr.cs.riple.core.metadata.trackers.TrackerNode; +import edu.ucr.cs.riple.injector.location.Location; +import edu.ucr.cs.riple.injector.location.OnField; +import java.util.Set; + +/** + * Responsible for performing tasks related to NullAway / Type Annotator Scanner serialization + * features. + */ +public interface NullAwayVersionAdapter { + + /** + * Deserializes values produced by NullAway in a tsv file and creates a corresponding {@link Fix} + * instance. + * + * @param location Location of the targeted element. + * @param values Values in row of a TSV file. + * @return Corresponding Fix instance with the passed values. + */ + Fix deserializeFix(Location location, String[] values); + + /** + * Deserializes values produced by NullAway in a tsv file and creates a corresponding {@link + * Error} instance. + * + * @param values Values in row of a TSV file. + * @return Corresponding Error instance with the passed values. + */ + Error deserializeError(String[] values); + + /** + * Deserializes values produced by Type Annotator Scanner in a tsv file and creates a + * corresponding {@link TrackerNode} instance. + * + * @param values Values in row of a TSV file. + * @return Corresponding TrackerNode instance with the passed values. + */ + TrackerNode deserializeTrackerNode(String[] values); + + /** + * Returns a set of regions enclosed by a field. Returns a set since there can multiple inline + * field declarations. + * + * @param onField The target field. + * @return Set of regions enclosed by the passed location. + */ + Set getFieldRegionScope(OnField onField); +} diff --git a/core/src/main/java/edu/ucr/cs/riple/core/explorers/BasicExplorer.java b/core/src/main/java/edu/ucr/cs/riple/core/explorers/BasicExplorer.java index 9da754c63..dafbdb481 100644 --- a/core/src/main/java/edu/ucr/cs/riple/core/explorers/BasicExplorer.java +++ b/core/src/main/java/edu/ucr/cs/riple/core/explorers/BasicExplorer.java @@ -31,6 +31,7 @@ import edu.ucr.cs.riple.core.metadata.index.Error; import edu.ucr.cs.riple.core.metadata.index.Fix; import edu.ucr.cs.riple.core.metadata.index.Result; +import edu.ucr.cs.riple.core.metadata.trackers.Region; import edu.ucr.cs.riple.core.util.Utility; import edu.ucr.cs.riple.injector.changes.AddMarkerAnnotation; import edu.ucr.cs.riple.injector.location.Location; @@ -92,8 +93,7 @@ public void addTriggeredFixesFromDownstream(Node node, Collection localTrig new Fix( new AddMarkerAnnotation(onParameter, config.nullableAnnot), "PASSING_NULLABLE", - onParameter.clazz, - onParameter.method, + new Region(onParameter.clazz, onParameter.method), false)) .collect(Collectors.toList())); } diff --git a/core/src/main/java/edu/ucr/cs/riple/core/explorers/OptimizedExplorer.java b/core/src/main/java/edu/ucr/cs/riple/core/explorers/OptimizedExplorer.java index 9cf45816f..3939e1ab0 100644 --- a/core/src/main/java/edu/ucr/cs/riple/core/explorers/OptimizedExplorer.java +++ b/core/src/main/java/edu/ucr/cs/riple/core/explorers/OptimizedExplorer.java @@ -85,11 +85,11 @@ protected void executeNextCycle() { List triggeredErrors = new ArrayList<>(); for (Region region : node.regions) { Result errorComparisonResult = - errorBank.compareByMethod(region.clazz, region.method, false); + errorBank.compareByMember(region.clazz, region.member, false); localEffect += errorComparisonResult.size; triggeredErrors.addAll(errorComparisonResult.dif); triggeredFixes.addAll( - fixBank.compareByMethod(region.clazz, region.method, false).dif); + fixBank.compareByMember(region.clazz, region.member, false).dif); } addTriggeredFixesFromDownstream(node, triggeredFixes); node.updateStatus( diff --git a/core/src/main/java/edu/ucr/cs/riple/core/explorers/suppliers/AbstractSupplier.java b/core/src/main/java/edu/ucr/cs/riple/core/explorers/suppliers/AbstractSupplier.java index 0b74a13d0..ed98cba73 100644 --- a/core/src/main/java/edu/ucr/cs/riple/core/explorers/suppliers/AbstractSupplier.java +++ b/core/src/main/java/edu/ucr/cs/riple/core/explorers/suppliers/AbstractSupplier.java @@ -55,7 +55,7 @@ public abstract class AbstractSupplier implements Supplier { public AbstractSupplier( ImmutableSet modules, Config config, MethodDeclarationTree tree) { this.config = config; - this.fieldDeclarationAnalysis = new FieldDeclarationAnalysis(modules); + this.fieldDeclarationAnalysis = new FieldDeclarationAnalysis(config, modules); this.tree = tree; this.fixBank = initializeFixBank(modules); this.errorBank = initializeErrorBank(modules); @@ -88,7 +88,7 @@ protected Bank initializeErrorBank(ImmutableSet modules) { modules.stream() .map(info -> info.dir.resolve("errors.tsv")) .collect(ImmutableSet.toImmutableSet()), - Error::new); + Error.factory(config)); } /** diff --git a/core/src/main/java/edu/ucr/cs/riple/core/global/GlobalAnalyzerImpl.java b/core/src/main/java/edu/ucr/cs/riple/core/global/GlobalAnalyzerImpl.java index c1a41ba2c..aa63ee0f7 100644 --- a/core/src/main/java/edu/ucr/cs/riple/core/global/GlobalAnalyzerImpl.java +++ b/core/src/main/java/edu/ucr/cs/riple/core/global/GlobalAnalyzerImpl.java @@ -36,6 +36,7 @@ import edu.ucr.cs.riple.core.metadata.method.MethodDeclarationTree; import edu.ucr.cs.riple.core.metadata.method.MethodNode; import edu.ucr.cs.riple.core.metadata.trackers.MethodRegionTracker; +import edu.ucr.cs.riple.core.metadata.trackers.Region; import edu.ucr.cs.riple.core.util.Utility; import edu.ucr.cs.riple.injector.changes.AddMarkerAnnotation; import edu.ucr.cs.riple.injector.location.Location; @@ -81,7 +82,7 @@ public void analyzeDownstreamDependencies() { Utility.buildDownstreamDependencies(config); Utility.setScannerCheckerActivation(downstreamModules, false); // Collect callers of public APIs in module. - MethodRegionTracker tracker = new MethodRegionTracker(config.downstreamInfo, tree); + MethodRegionTracker tracker = new MethodRegionTracker(config, config.downstreamInfo, tree); // Generate fixes corresponding methods. ImmutableSet fixes = methods.values().stream() @@ -100,8 +101,7 @@ public void analyzeDownstreamDependencies() { methodImpact.node.location.method), config.nullableAnnot), "null", - "null", - "null", + new Region("null", "null"), false)) .collect(ImmutableSet.toImmutableSet()); DownstreamImpactExplorer analyzer = diff --git a/core/src/main/java/edu/ucr/cs/riple/core/metadata/MetaData.java b/core/src/main/java/edu/ucr/cs/riple/core/metadata/MetaData.java index 07ca09608..e03a06182 100644 --- a/core/src/main/java/edu/ucr/cs/riple/core/metadata/MetaData.java +++ b/core/src/main/java/edu/ucr/cs/riple/core/metadata/MetaData.java @@ -27,6 +27,7 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.Multimap; import com.google.common.collect.MultimapBuilder; +import edu.ucr.cs.riple.core.Config; import java.io.BufferedReader; import java.io.IOException; import java.nio.charset.Charset; @@ -50,14 +51,19 @@ public abstract class MetaData { */ protected final Multimap contents; + /** Annotator config. */ + protected final Config config; + /** * Constructor for this container. Once this constructor is invoked, all data will be loaded from * the file. * + * @param config Annotator config. * @param path Path to the file containing the data. */ - public MetaData(Path path) { + public MetaData(Config config, Path path) { contents = MultimapBuilder.hashKeys().arrayListValues().build(); + this.config = config; setup(); try { fillNodes(path); @@ -70,10 +76,12 @@ public MetaData(Path path) { * Constructor for this container. Contents are accumulated from multiple sources. Once this * constructor is invoked, all data will be loaded from the file. * + * @param config Annotator config. * @param paths Paths to all files containing data. */ - public MetaData(ImmutableSet paths) { + public MetaData(Config config, ImmutableSet paths) { contents = MultimapBuilder.hashKeys().arrayListValues().build(); + this.config = config; setup(); paths.forEach( path -> { diff --git a/core/src/main/java/edu/ucr/cs/riple/core/metadata/field/FieldDeclarationAnalysis.java b/core/src/main/java/edu/ucr/cs/riple/core/metadata/field/FieldDeclarationAnalysis.java index 674d32ac4..8621dd596 100644 --- a/core/src/main/java/edu/ucr/cs/riple/core/metadata/field/FieldDeclarationAnalysis.java +++ b/core/src/main/java/edu/ucr/cs/riple/core/metadata/field/FieldDeclarationAnalysis.java @@ -7,10 +7,12 @@ import com.github.javaparser.ast.body.VariableDeclarator; import com.github.javaparser.ast.nodeTypes.NodeWithSimpleName; import com.google.common.collect.ImmutableSet; +import edu.ucr.cs.riple.core.Config; import edu.ucr.cs.riple.core.ModuleInfo; import edu.ucr.cs.riple.core.metadata.MetaData; import edu.ucr.cs.riple.injector.Helper; import edu.ucr.cs.riple.injector.exceptions.TargetClassNotFound; +import edu.ucr.cs.riple.injector.location.OnField; import edu.ucr.cs.riple.scanner.TypeAnnotatorScanner; import java.io.File; import java.io.FileNotFoundException; @@ -38,20 +40,23 @@ public class FieldDeclarationAnalysis extends MetaData { /** * Constructor for {@link FieldDeclarationAnalysis}. * + * @param config Annotator config. * @param module Information of the target module. */ - public FieldDeclarationAnalysis(ModuleInfo module) { - super(module.dir.resolve(FILE_NAME)); + public FieldDeclarationAnalysis(Config config, ModuleInfo module) { + super(config, module.dir.resolve(FILE_NAME)); } /** * Constructor for {@link FieldDeclarationAnalysis}. Contents are accumulated from multiple * sources. * + * @param config Annotator config. * @param modules Information of set of modules. */ - public FieldDeclarationAnalysis(ImmutableSet modules) { + public FieldDeclarationAnalysis(Config config, ImmutableSet modules) { super( + config, modules.stream() .map(info -> info.dir.resolve(FILE_NAME)) .collect(ImmutableSet.toImmutableSet())); @@ -73,7 +78,7 @@ protected FieldDeclarationInfo addNodeByLine(String[] values) { System.err.println(notFound.getMessage()); return null; } - FieldDeclarationInfo info = new FieldDeclarationInfo(clazz); + FieldDeclarationInfo info = new FieldDeclarationInfo(path, clazz); members.forEach( bodyDeclaration -> bodyDeclaration.ifFieldDeclaration( @@ -112,4 +117,20 @@ public ImmutableSet getInLineMultipleFieldDeclarationsOnField( candidate.fields.stream().filter(group -> !Collections.disjoint(group, fields)).findFirst(); return inLineGroupFieldDeclaration.orElse(ImmutableSet.copyOf(fields)); } + + /** + * Creates a {@link OnField} instance targeting the passed field and class. + * + * @param clazz Enclosing class of the field. + * @param field Field name. + * @return {@link OnField} instance targeting the passed field and class. + */ + public OnField getLocationOnField(String clazz, String field) { + FieldDeclarationInfo candidate = + findNodeWithHashHint(node -> node.clazz.equals(clazz), FieldDeclarationInfo.hash(clazz)); + if (candidate == null) { + return null; + } + return new OnField(candidate.pathToSourceFile, candidate.clazz, Collections.singleton(field)); + } } diff --git a/core/src/main/java/edu/ucr/cs/riple/core/metadata/field/FieldDeclarationInfo.java b/core/src/main/java/edu/ucr/cs/riple/core/metadata/field/FieldDeclarationInfo.java index 74852e319..fd53bf15c 100644 --- a/core/src/main/java/edu/ucr/cs/riple/core/metadata/field/FieldDeclarationInfo.java +++ b/core/src/main/java/edu/ucr/cs/riple/core/metadata/field/FieldDeclarationInfo.java @@ -12,9 +12,12 @@ public class FieldDeclarationInfo implements Hashable { public final Set> fields; /** Flat name of the containing class. */ public final String clazz; + /** Path to source file containing this class. */ + public final String pathToSourceFile; - public FieldDeclarationInfo(String clazz) { + public FieldDeclarationInfo(String path, String clazz) { this.clazz = clazz; + this.pathToSourceFile = path; this.fields = new HashSet<>(); } @@ -26,8 +29,8 @@ public boolean equals(Object o) { if (!(o instanceof FieldDeclarationInfo)) { return false; } - FieldDeclarationInfo info = (FieldDeclarationInfo) o; - return clazz.equals(info.clazz); + FieldDeclarationInfo other = (FieldDeclarationInfo) o; + return clazz.equals(other.clazz); } /** diff --git a/core/src/main/java/edu/ucr/cs/riple/core/metadata/field/FieldInitializationAnalysis.java b/core/src/main/java/edu/ucr/cs/riple/core/metadata/field/FieldInitializationAnalysis.java index 3ecdcd26d..8a8f0c9ee 100644 --- a/core/src/main/java/edu/ucr/cs/riple/core/metadata/field/FieldInitializationAnalysis.java +++ b/core/src/main/java/edu/ucr/cs/riple/core/metadata/field/FieldInitializationAnalysis.java @@ -1,11 +1,11 @@ package edu.ucr.cs.riple.core.metadata.field; import com.google.common.base.Preconditions; +import edu.ucr.cs.riple.core.Config; import edu.ucr.cs.riple.core.metadata.MetaData; import edu.ucr.cs.riple.injector.location.Location; import edu.ucr.cs.riple.injector.location.OnField; import edu.ucr.cs.riple.injector.location.OnMethod; -import java.nio.file.Path; import java.util.Arrays; import java.util.HashMap; import java.util.HashSet; @@ -20,14 +20,22 @@ */ public class FieldInitializationAnalysis extends MetaData { + /** + * Output file name. It contains information about initializations of fields via methods. On each + * row, an initializer method along the initialized field is written. A method is considered to be + * an initializer for a filed, if the method writes a {@code @Nonnull} value to the field and + * leaves it {@code @Nonnull} at exit. + */ + public static final String FILE_NAME = "field_init.tsv"; + /** * Constructs an {@link FieldInitializationAnalysis} instance. After this call, all serialized * information from NullAway has been processed. * - * @param path Path to field initialization info coming from NullAway. + * @param config Annotator config. */ - public FieldInitializationAnalysis(Path path) { - super(path); + public FieldInitializationAnalysis(Config config) { + super(config, config.target.dir.resolve(FILE_NAME)); } @Override diff --git a/core/src/main/java/edu/ucr/cs/riple/core/metadata/index/Bank.java b/core/src/main/java/edu/ucr/cs/riple/core/metadata/index/Bank.java index 4baceecb8..e8e1442ed 100644 --- a/core/src/main/java/edu/ucr/cs/riple/core/metadata/index/Bank.java +++ b/core/src/main/java/edu/ucr/cs/riple/core/metadata/index/Bank.java @@ -43,11 +43,11 @@ public class Bank { /** Initial state indexed by enclosing class. */ private final Index rootInClass; /** Initial state indexed by enclosing class and method. */ - private final Index rootInMethod; + private final Index rootInMember; /** Current state indexed by enclosing class. */ private Index currentInClass; /** Current state indexed by enclosing class and method. */ - private Index currentInMethod; + private Index currentInMember; /** Factory instance to parse outputs. */ private final Factory factory; /** Path to file where outputs are stored. */ @@ -57,10 +57,10 @@ public Bank(ImmutableSet paths, Factory factory) { this.factory = factory; this.paths = paths; rootInClass = new Index<>(this.paths, Index.Type.BY_CLASS, factory); - rootInMethod = new Index<>(this.paths, Index.Type.BY_METHOD, factory); - rootInMethod.index(); + rootInMember = new Index<>(this.paths, Index.Type.BY_MEMBER, factory); + rootInMember.index(); rootInClass.index(); - Preconditions.checkArgument(rootInClass.getTotal() == rootInMethod.getTotal()); + Preconditions.checkArgument(rootInClass.getTotal() == rootInMember.getTotal()); } public Bank(Path path, Factory factory) { @@ -71,16 +71,16 @@ public Bank(Path path, Factory factory) { * Overwrites the current state with the new generated output, * * @param saveClass If true, current index by class will be updated. - * @param saveMethod if true, current index by class and method will be updated. + * @param saveMember if true, current index by class and member will be updated. */ - public void saveState(boolean saveClass, boolean saveMethod) { + public void saveState(boolean saveClass, boolean saveMember) { if (saveClass) { currentInClass = new Index<>(paths, Index.Type.BY_CLASS, factory); currentInClass.index(); } - if (saveMethod) { - currentInMethod = new Index<>(paths, Index.Type.BY_METHOD, factory); - currentInMethod.index(); + if (saveMember) { + currentInMember = new Index<>(paths, Index.Type.BY_MEMBER, factory); + currentInMember.index(); } } @@ -98,18 +98,18 @@ private Result compareByList(Collection previousItems, Collection curre } /** - * Computes the difference in items enclosed by the given enclosing class and method in current + * Computes the difference in items enclosed by the given enclosing class and member in current * state and root state. * * @param encClass Enclosing fully qualified class name. - * @param encMethod Enclosing method signature. + * @param encMember Enclosing member symbol. * @return Corresponding {@link Result}. */ - public Result compareByMethod(String encClass, String encMethod, boolean fresh) { + public Result compareByMember(String encClass, String encMember, boolean fresh) { saveState(false, fresh); return compareByList( - rootInMethod.getByMethod(encClass, encMethod), - currentInMethod.getByMethod(encClass, encMethod)); + rootInMember.getByMember(encClass, encMember), + currentInMember.getByMember(encClass, encMember)); } /** @@ -118,7 +118,7 @@ public Result compareByMethod(String encClass, String encMethod, boolean fres * @return Corresponding {@link Result} instance. */ public Result compare() { - return compareByList(rootInMethod.values(), currentInMethod.values()); + return compareByList(rootInMember.values(), currentInMember.values()); } /** @@ -130,4 +130,15 @@ public Result compare() { public Set getRegionsForFixes(Predicate predicate) { return rootInClass.getRegionsOfMatchingItems(predicate); } + + /** + * Fetches all ths index entries in the given region. + * + * @param regionClass Region class. + * @param regionMember Region member. + * @return Collection of items enclosed in the given region. + */ + public Collection getEntriesByRegion(String regionClass, String regionMember) { + return rootInMember.getByMember(regionClass, regionMember); + } } diff --git a/core/src/main/java/edu/ucr/cs/riple/core/metadata/index/Enclosed.java b/core/src/main/java/edu/ucr/cs/riple/core/metadata/index/Enclosed.java index fe00beeb2..af0d20181 100644 --- a/core/src/main/java/edu/ucr/cs/riple/core/metadata/index/Enclosed.java +++ b/core/src/main/java/edu/ucr/cs/riple/core/metadata/index/Enclosed.java @@ -35,8 +35,8 @@ public abstract class Enclosed { /** Containing region. */ protected final Region region; - public Enclosed(String encClass, String encMethod) { - this.region = new Region(encClass, encMethod); + public Enclosed(Region region) { + this.region = region; } /** @@ -49,11 +49,20 @@ public String encClass() { } /** - * Method signature of the containing region. + * Representative member of the containing region as {@code String}. * - * @return Method signature. + * @return Member symbol in {@code String}. */ - public String encMethod() { - return this.region.method; + public String encMember() { + return this.region.member; + } + + /** + * Getter for region. + * + * @return region instance. + */ + public Region getRegion() { + return this.region; } } diff --git a/core/src/main/java/edu/ucr/cs/riple/core/metadata/index/Error.java b/core/src/main/java/edu/ucr/cs/riple/core/metadata/index/Error.java index e84a41943..5fb9dd454 100644 --- a/core/src/main/java/edu/ucr/cs/riple/core/metadata/index/Error.java +++ b/core/src/main/java/edu/ucr/cs/riple/core/metadata/index/Error.java @@ -23,8 +23,9 @@ */ package edu.ucr.cs.riple.core.metadata.index; +import edu.ucr.cs.riple.core.Config; +import edu.ucr.cs.riple.core.metadata.trackers.Region; import edu.ucr.cs.riple.injector.location.Location; -import java.util.Arrays; import java.util.Objects; import javax.annotation.Nullable; @@ -42,32 +43,25 @@ public class Error extends Enclosed { */ @Nullable public final Location nonnullTarget; - /** - * NullAway serializes error on TSV file, this constructor is called for each line of that file. - * - * @param values Values in row of a TSV file. - */ - public Error(String[] values) { - this( - values[0], - values[1], - values[2], - values[3], - Location.createLocationFromArrayInfo(Arrays.copyOfRange(values, 4, 10))); - } - public Error( - String messageType, - String message, - String encClass, - String encMethod, - @Nullable Location nonnullTargetLocation) { - super(encClass, encMethod); + String messageType, String message, Region region, @Nullable Location nonnullTargetLocation) { + super(region); this.messageType = messageType; this.message = message; this.nonnullTarget = nonnullTargetLocation; } + /** + * Returns a factory that can create instances of Error object based on the given values. These + * values correspond to a row in a TSV file generated by NullAway. + * + * @param config Config instance. + * @return Factory instance. + */ + public static Factory factory(Config config) { + return config.getAdapter()::deserializeError; + } + @Override public boolean equals(Object o) { if (this == o) { diff --git a/core/src/main/java/edu/ucr/cs/riple/core/metadata/index/Fix.java b/core/src/main/java/edu/ucr/cs/riple/core/metadata/index/Fix.java index 7d98b842c..daf342d0d 100644 --- a/core/src/main/java/edu/ucr/cs/riple/core/metadata/index/Fix.java +++ b/core/src/main/java/edu/ucr/cs/riple/core/metadata/index/Fix.java @@ -28,9 +28,9 @@ import com.google.common.collect.Sets; import edu.ucr.cs.riple.core.Config; import edu.ucr.cs.riple.core.metadata.field.FieldDeclarationAnalysis; +import edu.ucr.cs.riple.core.metadata.trackers.Region; import edu.ucr.cs.riple.injector.Helper; import edu.ucr.cs.riple.injector.changes.AddAnnotation; -import edu.ucr.cs.riple.injector.changes.AddMarkerAnnotation; import edu.ucr.cs.riple.injector.location.Location; import edu.ucr.cs.riple.injector.location.OnField; import edu.ucr.cs.riple.injector.location.OnMethod; @@ -59,13 +59,8 @@ public class Fix extends Enclosed { */ public boolean fixSourceIsInTarget; - public Fix( - AddAnnotation change, - String reason, - String encClass, - String encMethod, - boolean fixSourceIsInTarget) { - super(encClass, encMethod); + public Fix(AddAnnotation change, String reason, Region region, boolean fixSourceIsInTarget) { + super(region); this.change = change; this.reasons = reason != null ? Sets.newHashSet(reason) : new HashSet<>(); this.fixSourceIsInTarget = fixSourceIsInTarget; @@ -81,9 +76,10 @@ public Fix( * @return Factory instance. */ public static Factory factory(Config config, FieldDeclarationAnalysis analysis) { - return info -> { - Location location = Location.createLocationFromArrayInfo(info); - Preconditions.checkNotNull(location, "Fix Location cannot be null: " + Arrays.toString(info)); + return values -> { + Location location = Location.createLocationFromArrayInfo(values); + Preconditions.checkNotNull( + location, "Fix Location cannot be null: " + Arrays.toString(values)); if (analysis != null) { location.ifField( field -> { @@ -92,9 +88,7 @@ public static Factory factory(Config config, FieldDeclarationAnalysis analy field.variables.addAll(variables); }); } - Preconditions.checkArgument(info[7].equals("nullable"), "unsupported annotation: " + info[7]); - return new Fix( - new AddMarkerAnnotation(location, config.nullableAnnot), info[6], info[8], info[9], true); + return config.getAdapter().deserializeFix(location, values); }; } diff --git a/core/src/main/java/edu/ucr/cs/riple/core/metadata/index/Index.java b/core/src/main/java/edu/ucr/cs/riple/core/metadata/index/Index.java index be07164f1..c83fad691 100644 --- a/core/src/main/java/edu/ucr/cs/riple/core/metadata/index/Index.java +++ b/core/src/main/java/edu/ucr/cs/riple/core/metadata/index/Index.java @@ -61,7 +61,7 @@ public class Index { /** Index type. */ public enum Type { - BY_METHOD, + BY_MEMBER, BY_CLASS } @@ -99,7 +99,7 @@ public void index() { if (type.equals(Type.BY_CLASS)) { hash = Objects.hash(item.encClass()); } else { - hash = Objects.hash(item.encClass(), item.encMethod()); + hash = Objects.hash(item.encClass(), item.encMember()); } items.put(hash, item); line = br.readLine(); @@ -123,15 +123,15 @@ public Collection getByClass(String clazz) { } /** - * Returns all contents which are enclosed by the given class and method. + * Returns all contents which are enclosed by the given class and member. * * @param clazz Fully qualified name of the class. - * @param method Method signature. - * @return Stored contents that are enclosed by the given class and method. + * @param member member symbol. + * @return Stored contents that are enclosed by the given class and member. */ - public Collection getByMethod(String clazz, String method) { - return items.get(Objects.hash(clazz, method)).stream() - .filter(item -> item.encClass().equals(clazz) && item.encMethod().equals(method)) + public Collection getByMember(String clazz, String member) { + return items.get(Objects.hash(clazz, member)).stream() + .filter(item -> item.encClass().equals(clazz) && item.encMember().equals(member)) .collect(Collectors.toList()); } diff --git a/core/src/main/java/edu/ucr/cs/riple/core/metadata/method/MethodDeclarationTree.java b/core/src/main/java/edu/ucr/cs/riple/core/metadata/method/MethodDeclarationTree.java index f43e7d9a7..a96c2e493 100644 --- a/core/src/main/java/edu/ucr/cs/riple/core/metadata/method/MethodDeclarationTree.java +++ b/core/src/main/java/edu/ucr/cs/riple/core/metadata/method/MethodDeclarationTree.java @@ -25,10 +25,11 @@ package edu.ucr.cs.riple.core.metadata.method; import com.google.common.collect.ImmutableSet; +import edu.ucr.cs.riple.core.Config; import edu.ucr.cs.riple.core.metadata.MetaData; import edu.ucr.cs.riple.injector.location.Location; import edu.ucr.cs.riple.injector.location.OnMethod; -import java.nio.file.Path; +import edu.ucr.cs.riple.scanner.Serializer; import java.util.HashMap; import java.util.HashSet; import java.util.Set; @@ -46,8 +47,8 @@ public class MethodDeclarationTree extends MetaData { /** Set of all classes flat name declared in module. */ private HashSet classNames; - public MethodDeclarationTree(Path path) { - super(path); + public MethodDeclarationTree(Config config) { + super(config, config.target.dir.resolve(Serializer.METHOD_INFO_FILE_NAME)); } @Override diff --git a/core/src/main/java/edu/ucr/cs/riple/core/metadata/trackers/CompoundTracker.java b/core/src/main/java/edu/ucr/cs/riple/core/metadata/trackers/CompoundTracker.java index 6a664db38..c9eaed351 100644 --- a/core/src/main/java/edu/ucr/cs/riple/core/metadata/trackers/CompoundTracker.java +++ b/core/src/main/java/edu/ucr/cs/riple/core/metadata/trackers/CompoundTracker.java @@ -25,6 +25,7 @@ package edu.ucr.cs.riple.core.metadata.trackers; import com.google.common.collect.ImmutableSet; +import edu.ucr.cs.riple.core.Config; import edu.ucr.cs.riple.core.ModuleInfo; import edu.ucr.cs.riple.core.metadata.index.Fix; import edu.ucr.cs.riple.core.metadata.method.MethodDeclarationTree; @@ -39,18 +40,19 @@ public class CompoundTracker implements RegionTracker { /** List of all trackers. */ private final List trackers; - public CompoundTracker(ModuleInfo info, MethodDeclarationTree tree) { + public CompoundTracker(Config config, ModuleInfo info, MethodDeclarationTree tree) { this.trackers = new ArrayList<>(); - MethodRegionTracker methodRegionTracker = new MethodRegionTracker(info, tree); - this.trackers.add(new FieldRegionTracker(info)); + MethodRegionTracker methodRegionTracker = new MethodRegionTracker(config, info, tree); + this.trackers.add(new FieldRegionTracker(config, info)); this.trackers.add(methodRegionTracker); this.trackers.add(new ParameterRegionTracker(tree, methodRegionTracker)); } - public CompoundTracker(ImmutableSet modules, MethodDeclarationTree tree) { + public CompoundTracker( + Config config, ImmutableSet modules, MethodDeclarationTree tree) { this.trackers = new ArrayList<>(); - MethodRegionTracker methodRegionTracker = new MethodRegionTracker(modules, tree); - this.trackers.add(new FieldRegionTracker(modules)); + MethodRegionTracker methodRegionTracker = new MethodRegionTracker(config, modules, tree); + this.trackers.add(new FieldRegionTracker(config, modules)); this.trackers.add(methodRegionTracker); this.trackers.add(new ParameterRegionTracker(tree, methodRegionTracker)); } diff --git a/core/src/main/java/edu/ucr/cs/riple/core/metadata/trackers/FieldRegionTracker.java b/core/src/main/java/edu/ucr/cs/riple/core/metadata/trackers/FieldRegionTracker.java index 416b2bd61..93f41f422 100644 --- a/core/src/main/java/edu/ucr/cs/riple/core/metadata/trackers/FieldRegionTracker.java +++ b/core/src/main/java/edu/ucr/cs/riple/core/metadata/trackers/FieldRegionTracker.java @@ -25,6 +25,7 @@ package edu.ucr.cs.riple.core.metadata.trackers; import com.google.common.collect.ImmutableSet; +import edu.ucr.cs.riple.core.Config; import edu.ucr.cs.riple.core.ModuleInfo; import edu.ucr.cs.riple.core.metadata.MetaData; import edu.ucr.cs.riple.core.metadata.index.Fix; @@ -37,12 +38,13 @@ /** Tracker for Fields. */ public class FieldRegionTracker extends MetaData implements RegionTracker { - public FieldRegionTracker(ModuleInfo info) { - super(info.dir.resolve(Serializer.FIELD_GRAPH_FILE_NAME)); + public FieldRegionTracker(Config config, ModuleInfo info) { + super(config, info.dir.resolve(Serializer.FIELD_GRAPH_FILE_NAME)); } - public FieldRegionTracker(ImmutableSet modules) { + public FieldRegionTracker(Config config, ImmutableSet modules) { super( + config, modules.stream() .map(info -> info.dir.resolve(Serializer.FIELD_GRAPH_FILE_NAME)) .collect(ImmutableSet.toImmutableSet())); @@ -50,7 +52,7 @@ public FieldRegionTracker(ImmutableSet modules) { @Override protected TrackerNode addNodeByLine(String[] values) { - return new TrackerNode(values[0], values[1], values[2], values[3]); + return config.getAdapter().deserializeTrackerNode(values); } @Override @@ -66,9 +68,9 @@ public Optional> getRegions(Fix fix) { candidate.calleeClass.equals(field.clazz) && field.isOnFieldWithName(candidate.calleeMember), TrackerNode.hash(field.clazz)) - .map(trackerNode -> new Region(trackerNode.callerClass, trackerNode.callerMethod)) + .map(trackerNode -> new Region(trackerNode.callerClass, trackerNode.callerMember)) .collect(Collectors.toSet()); - ans.add(new Region(field.clazz, "null")); + ans.addAll(config.getAdapter().getFieldRegionScope(field)); return Optional.of(ans); } } diff --git a/core/src/main/java/edu/ucr/cs/riple/core/metadata/trackers/MethodRegionTracker.java b/core/src/main/java/edu/ucr/cs/riple/core/metadata/trackers/MethodRegionTracker.java index 221a4a688..07942b62f 100644 --- a/core/src/main/java/edu/ucr/cs/riple/core/metadata/trackers/MethodRegionTracker.java +++ b/core/src/main/java/edu/ucr/cs/riple/core/metadata/trackers/MethodRegionTracker.java @@ -25,6 +25,7 @@ package edu.ucr.cs.riple.core.metadata.trackers; import com.google.common.collect.ImmutableSet; +import edu.ucr.cs.riple.core.Config; import edu.ucr.cs.riple.core.ModuleInfo; import edu.ucr.cs.riple.core.metadata.MetaData; import edu.ucr.cs.riple.core.metadata.index.Fix; @@ -45,13 +46,15 @@ public class MethodRegionTracker extends MetaData implements Region */ private final MethodDeclarationTree tree; - public MethodRegionTracker(ModuleInfo info, MethodDeclarationTree tree) { - super(info.dir.resolve(Serializer.CALL_GRAPH_FILE_NAME)); + public MethodRegionTracker(Config config, ModuleInfo info, MethodDeclarationTree tree) { + super(config, info.dir.resolve(Serializer.CALL_GRAPH_FILE_NAME)); this.tree = tree; } - public MethodRegionTracker(ImmutableSet modules, MethodDeclarationTree tree) { + public MethodRegionTracker( + Config config, ImmutableSet modules, MethodDeclarationTree tree) { super( + config, modules.stream() .map(info -> info.dir.resolve(Serializer.CALL_GRAPH_FILE_NAME)) .collect(ImmutableSet.toImmutableSet())); @@ -60,7 +63,7 @@ public MethodRegionTracker(ImmutableSet modules, MethodDeclarationTr @Override protected TrackerNode addNodeByLine(String[] values) { - return new TrackerNode(values[0], values[1], values[2], values[3]); + return config.getAdapter().deserializeTrackerNode(values); } @Override @@ -91,7 +94,7 @@ public Set getCallersOfMethod(String clazz, String method) { candidate -> candidate.calleeClass.equals(clazz) && candidate.calleeMember.equals(method), TrackerNode.hash(clazz)) - .map(node -> new Region(node.callerClass, node.callerMethod)) + .map(node -> new Region(node.callerClass, node.callerMember)) .collect(Collectors.toSet()); } } diff --git a/core/src/main/java/edu/ucr/cs/riple/core/metadata/trackers/Region.java b/core/src/main/java/edu/ucr/cs/riple/core/metadata/trackers/Region.java index c43f75649..680f39426 100644 --- a/core/src/main/java/edu/ucr/cs/riple/core/metadata/trackers/Region.java +++ b/core/src/main/java/edu/ucr/cs/riple/core/metadata/trackers/Region.java @@ -32,16 +32,60 @@ */ public class Region { /** - * Signature of the method where this region is enclosed. If region is a class field - * initialization region, method value will be String of {@code "null"} not {@code null}. + * Symbol of the region representative. If region is a class static initialization region, member + * value will be String of {@code "null"} not {@code null}. */ - public final String method; + public final String member; /** Fully qualified name of the enclosing class of the region. */ public final String clazz; - public Region(String encClass, String encMethod) { + public final Type type; + + /** Different types of code segments for a region. */ + public enum Type { + METHOD, + FIELD, + STATIC_BLOCK + } + + public Region(String encClass, String encMember) { this.clazz = encClass == null ? "null" : encClass; - this.method = encMethod == null ? "null" : encMethod; + this.member = encMember == null ? "null" : encMember; + this.type = getType(member); + } + + /** + * Initializes {@link Region#type} based on the string representation of member. + * + * @param member Symbol of the region representative. + * @return The corresponding Type. + */ + public static Type getType(String member) { + if (member.equals("null")) { + return Type.STATIC_BLOCK; + } + if (member.contains("(")) { + return Type.METHOD; + } + return Type.FIELD; + } + + /** + * Checks if region targets a method body. + * + * @return true, if region is targeting a method body. + */ + public boolean isOnMethod() { + return type.equals(Type.METHOD); + } + + /** + * Checks if region targets a field declaration. + * + * @return true, if region is targeting a field declaration. + */ + public boolean isOnField() { + return type.equals(Type.FIELD); } @Override @@ -53,16 +97,16 @@ public boolean equals(Object o) { return false; } Region region = (Region) o; - return Objects.equals(method, region.method) && Objects.equals(clazz, region.clazz); + return Objects.equals(member, region.member) && Objects.equals(clazz, region.clazz); } @Override public int hashCode() { - return Objects.hash(method, clazz); + return Objects.hash(member, clazz); } @Override public String toString() { - return "class='" + clazz + '\'' + ", method='" + method; + return "class='" + clazz + '\'' + ", member='" + member; } } diff --git a/core/src/main/java/edu/ucr/cs/riple/core/metadata/trackers/TrackerNode.java b/core/src/main/java/edu/ucr/cs/riple/core/metadata/trackers/TrackerNode.java index 17f515eb2..3b48ef3bc 100644 --- a/core/src/main/java/edu/ucr/cs/riple/core/metadata/trackers/TrackerNode.java +++ b/core/src/main/java/edu/ucr/cs/riple/core/metadata/trackers/TrackerNode.java @@ -32,17 +32,17 @@ public class TrackerNode implements Hashable { /** Fully qualified names of the caller class. */ public final String callerClass; - /** Method signature of the caller. "null" if used in class initialization region, not null. */ - public final String callerMethod; + /** Symbol the target region. */ + public final String callerMember; /** Callee field name if field and method signature if method. */ public final String calleeMember; /** Fully qualified name of the enclosing class of callee. */ public final String calleeClass; public TrackerNode( - String callerClass, String callerMethod, String calleeMember, String calleeClass) { + String callerClass, String callerMember, String calleeMember, String calleeClass) { this.callerClass = callerClass; - this.callerMethod = callerMethod; + this.callerMember = callerMember; this.calleeMember = calleeMember; this.calleeClass = calleeClass; } @@ -59,7 +59,7 @@ public boolean equals(Object o) { return callerClass.equals(that.callerClass) && calleeMember.equals(that.calleeMember) && calleeClass.equals(that.calleeClass) - && callerMethod.equals(that.callerMethod); + && callerMember.equals(that.callerMember); } /** diff --git a/core/src/main/java/edu/ucr/cs/riple/core/util/Utility.java b/core/src/main/java/edu/ucr/cs/riple/core/util/Utility.java index cd3f185da..b45d02a92 100644 --- a/core/src/main/java/edu/ucr/cs/riple/core/util/Utility.java +++ b/core/src/main/java/edu/ucr/cs/riple/core/util/Utility.java @@ -174,7 +174,7 @@ public static Set readFixesFromOutputDirectory(ModuleInfo info, Factory readErrorsFromOutputDirectory(ModuleInfo info) { + public static List readErrorsFromOutputDirectory(Config config, ModuleInfo info) { Path errorsPath = info.dir.resolve("errors.tsv"); List errors = new ArrayList<>(); try { @@ -184,7 +184,7 @@ public static List readErrorsFromOutputDirectory(ModuleInfo info) { // Skip header. br.readLine(); while ((line = br.readLine()) != null) { - errors.add(new Error(line.split("\t"))); + errors.add(config.getAdapter().deserializeError(line.split("\t"))); } } } catch (IOException e) { diff --git a/core/src/test/java/edu/ucr/cs/riple/core/ConfigurationTest.java b/core/src/test/java/edu/ucr/cs/riple/core/ConfigurationTest.java index d96457fe7..7a9e5ecd5 100644 --- a/core/src/test/java/edu/ucr/cs/riple/core/ConfigurationTest.java +++ b/core/src/test/java/edu/ucr/cs/riple/core/ConfigurationTest.java @@ -260,6 +260,23 @@ public void testForceResolveFlag() { assertEquals(config.nullUnMarkedAnnotation, "edu.ucr.example.NullUnmarked"); } + @Test + public void testCommentGenerationFlag() { + Config config; + + List baseFlags = new ArrayList<>(requiredFlagsCli); + baseFlags.addAll(requiredDownsStreamDependencyFlagsCli); + + // Check default mode. + config = new Config(makeCommandLineArguments(baseFlags)); + assertFalse(config.commentGenerationEnabled); + + baseFlags.add(new CLIFlagWithValue("acg", "CustomPrefix")); + config = new Config(makeCommandLineArguments(baseFlags)); + assertTrue(config.commentGenerationEnabled); + assertEquals(config.commentPrefix, "CustomPrefix"); + } + /** * Helper method for reading value of a node located at /key_1/key_2/.../key_n (in the form of * {@code Xpath} query) from a xml document at the given path. diff --git a/core/src/test/java/edu/ucr/cs/riple/core/CoreTest.java b/core/src/test/java/edu/ucr/cs/riple/core/CoreTest.java index f694586cf..eaa225394 100644 --- a/core/src/test/java/edu/ucr/cs/riple/core/CoreTest.java +++ b/core/src/test/java/edu/ucr/cs/riple/core/CoreTest.java @@ -29,10 +29,16 @@ import com.google.common.base.Preconditions; import edu.ucr.cs.riple.core.tools.TReport; +import edu.ucr.cs.riple.injector.changes.AddAnnotation; +import edu.ucr.cs.riple.injector.changes.AddMarkerAnnotation; +import edu.ucr.cs.riple.injector.changes.AddSingleElementAnnotation; import edu.ucr.cs.riple.injector.location.OnField; import edu.ucr.cs.riple.injector.location.OnMethod; import edu.ucr.cs.riple.injector.location.OnParameter; +import java.nio.file.Path; import java.util.List; +import java.util.Set; +import org.junit.Assert; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -296,4 +302,63 @@ public void deactivateInferenceTest() { .annotation .equals("org.jspecify.nullness.NullUnmarked")); } + + @Test + public void errorInFieldDeclarationForceResolveTest() { + coreTestHelper + .addInputDirectory("test", "fielderrorregion") + .addExpectedReports( + new TReport(new OnField("Foo.java", "test.Foo", Set.of("f1")), 2), + new TReport(new OnField("Foo.java", "test.Foo", Set.of("f2", "f3")), 2), + new TReport(new OnField("Foo.java", "test.Foo", Set.of("f0")), -1), + new TReport(new OnParameter("Bar.java", "test.Bar", "process(java.lang.Object)", 0), 1), + new TReport(new OnField("Foo.java", "test.Foo", Set.of("f4")), 1)) + .toDepth(1) + .enableForceResolve() + .start(); + Path srcRoot = + coreTestHelper + .getConfig() + .globalDir + .resolve("unittest") + .resolve("src") + .resolve("main") + .resolve("java") + .resolve("test"); + Set expectedAnnotations = + Set.of( + new AddMarkerAnnotation( + new OnField(srcRoot.resolve("Foo.java").toString(), "test.Foo", Set.of("f0")), + "javax.annotation.Nullable"), + new AddMarkerAnnotation( + new OnMethod( + srcRoot.resolve("Bar.java").toString(), + "test.Bar", + "process(java.lang.Object)"), + "org.jspecify.nullness.NullUnmarked"), + new AddSingleElementAnnotation( + new OnField(srcRoot.resolve("Foo.java").toString(), "test.Foo", Set.of("f4")), + "SuppressWarnings", + "NullAway", + false), + new AddSingleElementAnnotation( + new OnField(srcRoot.resolve("Foo.java").toString(), "test.Foo", Set.of("f0")), + "SuppressWarnings", + "NullAway", + false), + new AddSingleElementAnnotation( + new OnField(srcRoot.resolve("Foo.java").toString(), "test.Foo", Set.of("f2", "f3")), + "SuppressWarnings", + "NullAway.Init", + false), + new AddSingleElementAnnotation( + new OnField(srcRoot.resolve("Foo.java").toString(), "test.Foo", Set.of("f1")), + "SuppressWarnings", + "NullAway.Init", + false)); + // todo: change test infrastructure to do the expected added annotations comparison internally + // in the upcoming refactoring cycle. + Assert.assertEquals( + expectedAnnotations, Set.copyOf(coreTestHelper.getConfig().log.getInjectedAnnotations())); + } } diff --git a/core/src/test/java/edu/ucr/cs/riple/core/tools/CoreTestHelper.java b/core/src/test/java/edu/ucr/cs/riple/core/tools/CoreTestHelper.java index f5aa9ccf6..f0a35145f 100644 --- a/core/src/test/java/edu/ucr/cs/riple/core/tools/CoreTestHelper.java +++ b/core/src/test/java/edu/ucr/cs/riple/core/tools/CoreTestHelper.java @@ -182,7 +182,7 @@ private void checkBuildsStatus() { // Verify no error is reported in downstream dependencies. for (int i = 1; i < modules.size(); i++) { Path path = outDirPath.resolve(i + "").resolve("errors.tsv"); - List errors = Utility.readErrorsFromOutputDirectory(path); + List errors = Utility.readErrorsFromOutputDirectory(config, path); if (errors.size() != 0) { fail( "Strict mode introduced errors in downstream dependency module: " @@ -196,7 +196,7 @@ private void checkBuildsStatus() { // Check no error will be reported in Target module Utility.executeCommand(config.buildCommand); Path path = outDirPath.resolve("0").resolve("errors.tsv"); - List errors = Utility.readErrorsFromOutputDirectory(path); + List errors = Utility.readErrorsFromOutputDirectory(config, path); if (errors.size() != 0) { fail( "Force Resolve Mode did not resolve all errors:\n" diff --git a/core/src/test/java/edu/ucr/cs/riple/core/tools/TFix.java b/core/src/test/java/edu/ucr/cs/riple/core/tools/TFix.java index 4a0ddcfa6..9bc99ae45 100644 --- a/core/src/test/java/edu/ucr/cs/riple/core/tools/TFix.java +++ b/core/src/test/java/edu/ucr/cs/riple/core/tools/TFix.java @@ -25,6 +25,7 @@ package edu.ucr.cs.riple.core.tools; import edu.ucr.cs.riple.core.metadata.index.Fix; +import edu.ucr.cs.riple.core.metadata.trackers.Region; import edu.ucr.cs.riple.injector.changes.Change; import edu.ucr.cs.riple.injector.location.Location; @@ -32,6 +33,6 @@ public class TFix extends Fix { public TFix(Location location) { - super(new DefaultAnnotation(location), null, null, null, true); + super(new DefaultAnnotation(location), null, new Region("null", "null"), true); } } diff --git a/core/src/test/java/edu/ucr/cs/riple/core/tools/TReport.java b/core/src/test/java/edu/ucr/cs/riple/core/tools/TReport.java index 0cbbd77af..f0de57249 100644 --- a/core/src/test/java/edu/ucr/cs/riple/core/tools/TReport.java +++ b/core/src/test/java/edu/ucr/cs/riple/core/tools/TReport.java @@ -28,6 +28,7 @@ import edu.ucr.cs.riple.core.Config; import edu.ucr.cs.riple.core.Report; import edu.ucr.cs.riple.core.metadata.index.Fix; +import edu.ucr.cs.riple.core.metadata.trackers.Region; import edu.ucr.cs.riple.injector.changes.AddMarkerAnnotation; import edu.ucr.cs.riple.injector.location.Location; import java.util.Set; @@ -48,13 +49,12 @@ public TReport(Location root, int effect, Tag tag) { this(root, effect, "", "", tag); } - public TReport(Location root, int effect, String encClass, String encMethod, Tag tag) { + public TReport(Location root, int effect, String encClass, String encMember, Tag tag) { super( new Fix( new AddMarkerAnnotation(root, "javax.annotation.Nullable"), null, - encClass, - encMethod, + new Region(encClass, encMember), true), effect); this.expectedValue = effect; diff --git a/core/src/test/java/edu/ucr/cs/riple/core/tools/Utility.java b/core/src/test/java/edu/ucr/cs/riple/core/tools/Utility.java index 0b386d51e..a8cc348ef 100644 --- a/core/src/test/java/edu/ucr/cs/riple/core/tools/Utility.java +++ b/core/src/test/java/edu/ucr/cs/riple/core/tools/Utility.java @@ -24,6 +24,7 @@ package edu.ucr.cs.riple.core.tools; +import edu.ucr.cs.riple.core.Config; import edu.ucr.cs.riple.core.metadata.index.Error; import java.io.BufferedReader; import java.io.IOException; @@ -65,7 +66,7 @@ public static void executeCommand(String command) { * @param path Path to errors.tsv. * @return List of serialized errors. */ - public static List readErrorsFromOutputDirectory(Path path) { + public static List readErrorsFromOutputDirectory(Config config, Path path) { List errors = new ArrayList<>(); try { try (BufferedReader br = @@ -74,7 +75,7 @@ public static List readErrorsFromOutputDirectory(Path path) { // Skip headers. br.readLine(); while ((line = br.readLine()) != null) { - errors.add(new Error(line.split("\t"))); + errors.add(config.getAdapter().deserializeError(line.split("\t"))); } } } catch (IOException e) { diff --git a/core/src/test/resources/fielderrorregion/Bar.java b/core/src/test/resources/fielderrorregion/Bar.java new file mode 100644 index 000000000..eeccac532 --- /dev/null +++ b/core/src/test/resources/fielderrorregion/Bar.java @@ -0,0 +1,22 @@ +package test; + +import javax.annotation.Nullable; + +public class Bar { + + Object foo = new Object(); + + public void deref() {} + + @Nullable + public Bar process(Object foo) { + this.foo = foo; + receiveNonnull(foo); + return null; + } + + public Object receiveNonnull(Object o) { + // just to keep Bar#process:foo @Nonnull + return o; + } +} diff --git a/core/src/test/resources/fielderrorregion/Foo.java b/core/src/test/resources/fielderrorregion/Foo.java new file mode 100644 index 000000000..0ceb3f588 --- /dev/null +++ b/core/src/test/resources/fielderrorregion/Foo.java @@ -0,0 +1,29 @@ +package test; + +import javax.annotation.Nullable; + +public class Foo { + Bar f0 = new Bar().process(null); + Bar f1; + Bar f2, f3; + @Nullable Bar nullableBar; + Bar f4 = nullableBar.process(new Object()); + + Foo() { + this.f2 = new Bar(); + } + + public void run1() { + // to prevent annotator making f1, f3 and f4 @Nullable. + f1.deref(); + f3.deref(); + f4.deref(); + } + + public void run2() { + // to prevent annotator making f1, f3 and f4 @Nullable. + f1.deref(); + f3.deref(); + f4.deref(); + } +} diff --git a/injector/src/main/java/edu/ucr/cs/riple/injector/changes/AddMarkerAnnotation.java b/injector/src/main/java/edu/ucr/cs/riple/injector/changes/AddMarkerAnnotation.java index 51f43cc63..1ad68cd53 100644 --- a/injector/src/main/java/edu/ucr/cs/riple/injector/changes/AddMarkerAnnotation.java +++ b/injector/src/main/java/edu/ucr/cs/riple/injector/changes/AddMarkerAnnotation.java @@ -40,8 +40,24 @@ */ public class AddMarkerAnnotation extends AddAnnotation { - public AddMarkerAnnotation(Location location, String annotation) { + /** + * Comment to be added on the added annotation. If {@code null} no comment will be added. Comment + * cannot contain any consecutive characters of {@code *} and {@code /} since this comment will be + * added as a trailing comment surrounded by "/* {@literal *}/". + */ + @Nullable public final String comment; + + public AddMarkerAnnotation(Location location, String annotation, @Nullable String comment) { super(location, annotation); + this.comment = comment; + if (comment != null && comment.contains("*/")) { + throw new IllegalArgumentException( + "Comment cannot contain pair of \"*/\" characters: " + comment); + } + } + + public AddMarkerAnnotation(Location location, String annotation) { + this(location, annotation, null); } @Override @@ -56,11 +72,28 @@ public Modification visit(ElementKind kind, NodeWithAnnotations node, Range r if (annotAlreadyExists) { return null; } - return new Insertion(annotationExpr.toString(), range.begin, kind); + String contentToAdd = + this.comment == null + ? annotationExpr.toString() + : annotationExpr + " " + getCommentRepresentationInSourceCode(); + return new Insertion(contentToAdd, range.begin, kind); } @Override public Change duplicate() { return new AddMarkerAnnotation(location, annotation); } + + /** + * Returns string representation of comment as trailing comment according to java comment style. + * + * @return Comment according to java comment style. + */ + private String getCommentRepresentationInSourceCode() { + if (this.comment == null) { + // to make this method return non-null. + throw new RuntimeException("comment is not initialized"); + } + return "/* " + this.comment + " */"; + } } diff --git a/injector/src/test/java/edu/ucr/cs/riple/injector/OnMethodInjectionTest.java b/injector/src/test/java/edu/ucr/cs/riple/injector/OnMethodInjectionTest.java index 1c9f34580..adb968971 100644 --- a/injector/src/test/java/edu/ucr/cs/riple/injector/OnMethodInjectionTest.java +++ b/injector/src/test/java/edu/ucr/cs/riple/injector/OnMethodInjectionTest.java @@ -432,4 +432,30 @@ public void methodWithMultipleLineDeclaration() { "javax.annotation.Nullable")) .start(); } + + @Test + public void annotWithCommentOnMethod() { + injectorTestHelper + .addInput( + "Main.java", + "package edu.ucr;", + "public class Main {", + " @Foo(clazz = String.class, value = \"Some description\")", + " public void foo() { }", + "}") + .expectOutput( + "package edu.ucr;", + "import edu.ucr.Custom;", + "public class Main {", + " @Custom /* some comment */", + " @Foo(clazz = String.class, value = \"Some description\")", + " public void foo() { }", + "}") + .addChanges( + new AddMarkerAnnotation( + new OnMethod("Main.java", "edu.ucr.Main", "foo()"), + "edu.ucr.Custom", + "some comment")) + .start(); + } } diff --git a/injector/src/test/java/edu/ucr/cs/riple/injector/OnParameterInjectionTest.java b/injector/src/test/java/edu/ucr/cs/riple/injector/OnParameterInjectionTest.java index 9f5eab433..2426b9893 100644 --- a/injector/src/test/java/edu/ucr/cs/riple/injector/OnParameterInjectionTest.java +++ b/injector/src/test/java/edu/ucr/cs/riple/injector/OnParameterInjectionTest.java @@ -763,4 +763,29 @@ public void methodWithMultipleLineInlineMultiParameterDeclarationAllParamRemoval "javax.annotation.Nullable")) .start(); } + + @Test + public void annotWithCommentOnParameter() { + injectorTestHelper + .addInput( + "Main.java", + "package edu.ucr;", + "public class Main {", + " @Foo(clazz = String.class, value = \"Some description\")", + " public void foo(Object a, Object b, Object c) { }", + "}") + .expectOutput( + "package edu.ucr;", + "import edu.ucr.Custom;", + "public class Main {", + " @Foo(clazz = String.class, value = \"Some description\")", + " public void foo(Object a, Object b, @Custom /* some comment */ Object c) { }", + "}") + .addChanges( + new AddMarkerAnnotation( + new OnParameter("Main.java", "edu.ucr.Main", "foo(Object, Object, Object)", 2), + "edu.ucr.Custom", + "some comment")) + .start(); + } } diff --git a/runner/config.json b/runner/config.json index 634ca5cf8..c657f54af 100644 --- a/runner/config.json +++ b/runner/config.json @@ -1,10 +1,11 @@ { - "BUILD_COMMAND": "cd /path/to/target && command to run javac with analysis (e.g. ./gradlew :p:compileJava)", + "BUILD_COMMAND": "cd /Users/nima/Developer/NullAwayFixer/Projects/libgdx && ./gradlew :gdx:compileJava", "ANNOTATION": { "INITIALIZER": "com.badlogic.gdx.Initializer", - "NULLABLE": "javax.annotation.Nullable" + "NULLABLE": "javax.annotation.Nullable", + "NULL_UNMARKED": "com.badlogic.gdx.NullUnmarked" }, - "LEXICAL_PRESERVATION": true, + "LEXICAL_PRESERVATION": false, "OUTPUT_DIR": "/tmp/NullAwayFix", "CHAIN": false, "OPTIMIZED": true, @@ -13,15 +14,17 @@ "REDIRECT_BUILD_OUTPUT_TO_STDERR": false, "EXHAUSTIVE_SEARCH": false, "OUTER_LOOP": true, + "FORCE_RESOLVE": true, + "INFERENCE_ACTIVATION": false, "CONFIG_PATHS": [ { - "NULLAWAY": "path_to_nullaway_config.xml", - "SCANNER": "path_to_scanner_config.xml" + "NULLAWAY": "/tmp/NullAwayFix/nullaway.xml", + "SCANNER": "/tmp/NullAwayFix/scanner.xml" } ], - "DEPTH": 1, + "DEPTH": 5, "DOWNSTREAM_DEPENDENCY": { - "ACTIVATION":true, + "ACTIVATION":false, "BUILD_COMMAND": "cd /path/to/dependencies && command to run javac with analysis (e.g. ./gradlew :p:compileJava)", "LIBRARY_MODEL_LOADER_PATH": "path to nullaway library model loader jar" } diff --git a/runner/update.sh b/runner/update.sh index ddb33c9c4..aa6e9cf4d 100755 --- a/runner/update.sh +++ b/runner/update.sh @@ -1,9 +1,9 @@ set -exu -CURRENT_VERSION="core-1.3.3-LOCAL.jar" +CURRENT_VERSION="core-1.3.4-SNAPSHOT.jar" PROJECT_ROOT=${PROJECT_ROOT:-$(git rev-parse --show-toplevel)} pushd "$PROJECT_ROOT" - ./gradlew publishToMavenLocal -x signMavenPublication --rerun-tasks + ./gradlew publishToMavenLocal --rerun-tasks mv core/build/libs/"$CURRENT_VERSION" runner/jars/core.jar popd diff --git a/type-annotator-scanner/src/main/java/edu/ucr/cs/riple/scanner/TypeAnnotatorScanner.java b/type-annotator-scanner/src/main/java/edu/ucr/cs/riple/scanner/TypeAnnotatorScanner.java index b07a0ab80..32afe8f33 100644 --- a/type-annotator-scanner/src/main/java/edu/ucr/cs/riple/scanner/TypeAnnotatorScanner.java +++ b/type-annotator-scanner/src/main/java/edu/ucr/cs/riple/scanner/TypeAnnotatorScanner.java @@ -95,7 +95,7 @@ public Description matchMethodInvocation(MethodInvocationTree tree, VisitorState } config .getSerializer() - .serializeCallGraphNode(new TrackerNode(ASTHelpers.getSymbol(tree), state.getPath(), true)); + .serializeCallGraphNode(new TrackerNode(ASTHelpers.getSymbol(tree), state.getPath())); return Description.NO_MATCH; } @@ -107,6 +107,7 @@ public Description matchMethod(MethodTree tree, VisitorState state) { } Symbol.MethodSymbol methodSymbol = ASTHelpers.getSymbol(tree); MethodInfo methodInfo = MethodInfo.findOrCreate(methodSymbol, context); + methodInfo.setURI(state); methodInfo.findParent(state, context); methodInfo.setAnnotation(config); methodInfo.setURI(state); @@ -124,7 +125,7 @@ public Description matchVariable(VariableTree tree, VisitorState state) { if (!context.getConfig().fieldTrackerIsActive()) { return Description.NO_MATCH; } - serializeField(ASTHelpers.getSymbol(tree.getInitializer()), state, true); + serializeSymIfField(ASTHelpers.getSymbol(tree.getInitializer()), state); return Description.NO_MATCH; } @@ -133,7 +134,7 @@ public Description matchIdentifier(IdentifierTree tree, VisitorState state) { if (!context.getConfig().fieldTrackerIsActive()) { return Description.NO_MATCH; } - serializeField(ASTHelpers.getSymbol(tree), state, false); + serializeSymIfField(ASTHelpers.getSymbol(tree), state); return Description.NO_MATCH; } @@ -142,16 +143,16 @@ public Description matchMemberSelect(MemberSelectTree tree, VisitorState state) if (!context.getConfig().fieldTrackerIsActive()) { return Description.NO_MATCH; } - serializeField(ASTHelpers.getSymbol(tree), state, false); + serializeSymIfField(ASTHelpers.getSymbol(tree), state); return Description.NO_MATCH; } - private void serializeField(Symbol symbol, VisitorState state, boolean force) { + private void serializeSymIfField(Symbol symbol, VisitorState state) { if (symbol != null && symbol.getKind() == ElementKind.FIELD) { context .getConfig() .getSerializer() - .serializeFieldGraphNode(new TrackerNode(symbol, state.getPath(), force)); + .serializeFieldGraphNode(new TrackerNode(symbol, state.getPath())); } } } diff --git a/type-annotator-scanner/src/main/java/edu/ucr/cs/riple/scanner/out/MethodInfo.java b/type-annotator-scanner/src/main/java/edu/ucr/cs/riple/scanner/out/MethodInfo.java index ebb64e916..6e285a4d2 100644 --- a/type-annotator-scanner/src/main/java/edu/ucr/cs/riple/scanner/out/MethodInfo.java +++ b/type-annotator-scanner/src/main/java/edu/ucr/cs/riple/scanner/out/MethodInfo.java @@ -52,7 +52,7 @@ public class MethodInfo { private MethodInfo(Symbol.MethodSymbol method, ScannerContext context) { this.id = context.getNextMethodId(); this.symbol = method; - this.clazz = (method != null) ? method.enclClass() : null; + this.clazz = method.enclClass(); this.parent = 0; context.visitMethod(this); } diff --git a/type-annotator-scanner/src/main/java/edu/ucr/cs/riple/scanner/out/TrackerNode.java b/type-annotator-scanner/src/main/java/edu/ucr/cs/riple/scanner/out/TrackerNode.java index bf5825c0b..f393ba7d7 100644 --- a/type-annotator-scanner/src/main/java/edu/ucr/cs/riple/scanner/out/TrackerNode.java +++ b/type-annotator-scanner/src/main/java/edu/ucr/cs/riple/scanner/out/TrackerNode.java @@ -27,46 +27,80 @@ import com.google.errorprone.util.ASTHelpers; import com.sun.source.tree.ClassTree; import com.sun.source.tree.MethodTree; +import com.sun.source.tree.VariableTree; import com.sun.source.util.TreePath; import com.sun.tools.javac.code.Symbol; public class TrackerNode { - private final Symbol member; - private final Symbol.ClassSymbol callerClass; - private final Symbol.MethodSymbol callerMethod; - private final boolean force; + private final Symbol usedMember; + private final Symbol.ClassSymbol regionClass; + private final Symbol regionMember; - public TrackerNode(Symbol member, TreePath path, boolean force) { - ClassTree callerClass = ASTHelpers.findEnclosingNode(path, ClassTree.class); - MethodTree callerMethod = ASTHelpers.findEnclosingNode(path, MethodTree.class); - this.member = member; - this.callerClass = (callerClass != null) ? ASTHelpers.getSymbol(callerClass) : null; - this.callerMethod = (callerMethod != null) ? ASTHelpers.getSymbol(callerMethod) : null; - this.force = force; - } - - public TrackerNode(Symbol member, TreePath path) { - this(member, path, false); + public TrackerNode(Symbol usedMember, TreePath path) { + this.usedMember = usedMember; + ClassTree enclosingClass; + MethodTree enclosingMethod; + enclosingMethod = + path.getLeaf() instanceof MethodTree + ? (MethodTree) path.getLeaf() + : ASTHelpers.findEnclosingNode(path, MethodTree.class); + enclosingClass = + path.getLeaf() instanceof ClassTree + ? (ClassTree) path.getLeaf() + : ASTHelpers.findEnclosingNode(path, ClassTree.class); + if (enclosingClass != null) { + Symbol.ClassSymbol classSymbol = ASTHelpers.getSymbol(enclosingClass); + regionClass = classSymbol; + if (enclosingMethod != null) { + Symbol.MethodSymbol methodSymbol = ASTHelpers.getSymbol(enclosingMethod); + if (!methodSymbol.isEnclosedBy(classSymbol)) { + enclosingMethod = null; + } + } + if (enclosingMethod != null) { + regionMember = ASTHelpers.getSymbol(enclosingMethod); + } else { + // Node is not enclosed by any method, can be a field declaration or enclosed by it. + Symbol sym = ASTHelpers.getSymbol(path.getLeaf()); + Symbol.VarSymbol fieldSymbol = null; + if (sym != null && sym.getKind().isField() && sym.isEnclosedBy(classSymbol)) { + // Directly on a field declaration. + fieldSymbol = (Symbol.VarSymbol) sym; + } else { + // Can be enclosed by a field declaration tree. + VariableTree fieldDeclTree = ASTHelpers.findEnclosingNode(path, VariableTree.class); + if (fieldDeclTree != null) { + fieldSymbol = ASTHelpers.getSymbol(fieldDeclTree); + } + } + if (fieldSymbol != null && fieldSymbol.isEnclosedBy(classSymbol)) { + regionMember = fieldSymbol; + } else { + regionMember = null; + } + } + } else { + // just to make them final in declaration. + regionMember = null; + regionClass = null; + } } @Override public String toString() { - if (callerClass == null) { - return ""; - } - if (!force && callerMethod == null) { + if (regionClass == null) { return ""; } - Symbol enclosingClass = member.enclClass(); + Symbol enclosingClass = usedMember.enclClass(); return String.join( "\t", - callerClass.flatName(), - ((callerMethod == null) ? "null" : callerMethod.toString()), - member.toString(), + regionClass.flatName(), + ((regionMember == null) ? "null" : regionMember.toString()), + usedMember.toString(), ((enclosingClass == null) ? "null" : enclosingClass.flatName())); } public static String header() { - return "CALLER_CLASS" + '\t' + "CALLER_METHOD" + '\t' + "MEMBER" + '\t' + "CALLEE_CLASS"; + return "REGION_CLASS" + '\t' + "REGION_MEMBER" + '\t' + "USED_MEMBER" + '\t' + "USED_CLASS"; } } diff --git a/type-annotator-scanner/src/test/java/edu/ucr/cs/riple/scanner/FieldTrackerTest.java b/type-annotator-scanner/src/test/java/edu/ucr/cs/riple/scanner/FieldTrackerTest.java index d5638ec78..70f920be2 100644 --- a/type-annotator-scanner/src/test/java/edu/ucr/cs/riple/scanner/FieldTrackerTest.java +++ b/type-annotator-scanner/src/test/java/edu/ucr/cs/riple/scanner/FieldTrackerTest.java @@ -40,7 +40,7 @@ public class FieldTrackerTest extends TypeAnnotatorScannerBaseTest