diff --git a/annotator-core/src/main/java/edu/ucr/cs/riple/core/Annotator.java b/annotator-core/src/main/java/edu/ucr/cs/riple/core/Annotator.java index 2c15c4833..29c15e2a8 100644 --- a/annotator-core/src/main/java/edu/ucr/cs/riple/core/Annotator.java +++ b/annotator-core/src/main/java/edu/ucr/cs/riple/core/Annotator.java @@ -157,7 +157,7 @@ private void executeNextIteration( injector.injectFixes(selectedFixes); // Update log. context.log.updateInjectedAnnotations( - selectedFixes.stream().map(fix -> fix.change).collect(Collectors.toSet())); + selectedFixes.stream().flatMap(fix -> fix.changes.stream()).collect(Collectors.toSet())); // Update impact saved state. downstreamImpactCache.updateImpactsAfterInjection(selectedFixes); targetModuleCache.updateImpactsAfterInjection(selectedFixes); diff --git a/annotator-core/src/main/java/edu/ucr/cs/riple/core/Report.java b/annotator-core/src/main/java/edu/ucr/cs/riple/core/Report.java index d5bdd0059..edcdd4d91 100644 --- a/annotator-core/src/main/java/edu/ucr/cs/riple/core/Report.java +++ b/annotator-core/src/main/java/edu/ucr/cs/riple/core/Report.java @@ -30,7 +30,6 @@ import edu.ucr.cs.riple.core.module.ModuleInfo; import edu.ucr.cs.riple.core.registries.index.Error; import edu.ucr.cs.riple.core.registries.index.Fix; -import edu.ucr.cs.riple.injector.location.Location; import java.util.HashSet; import java.util.Objects; import java.util.Set; @@ -180,22 +179,16 @@ public boolean testEquals(Config config, Report found) { } this.tree.add(this.root); found.tree.add(found.root); - Set thisTree = this.tree.stream().map(Fix::toLocation).collect(Collectors.toSet()); - Set otherTree = found.tree.stream().map(Fix::toLocation).collect(Collectors.toSet()); - if (!thisTree.equals(otherTree)) { + if (!this.tree.equals(found.tree)) { return false; } - Set thisTriggered = + Set thisTriggered = this.triggeredErrors.stream() - .filter(Error::hasFix) .flatMap(error -> error.getResolvingFixes().stream()) - .map(Fix::toLocation) .collect(Collectors.toSet()); - Set otherTriggered = + Set otherTriggered = found.triggeredErrors.stream() - .filter(Error::hasFix) .flatMap(error -> error.getResolvingFixes().stream()) - .map(Fix::toLocation) .collect(Collectors.toSet()); return otherTriggered.equals(thisTriggered); } @@ -207,7 +200,7 @@ public String toString() { + ", " + root + ", " - + tree.stream().map(Fix::toLocation).collect(Collectors.toSet()); + + tree.stream().map(Fix::toLocations).collect(Collectors.toSet()); } /** diff --git a/annotator-core/src/main/java/edu/ucr/cs/riple/core/cache/BaseCache.java b/annotator-core/src/main/java/edu/ucr/cs/riple/core/cache/BaseCache.java index a14378b02..f5227a85a 100644 --- a/annotator-core/src/main/java/edu/ucr/cs/riple/core/cache/BaseCache.java +++ b/annotator-core/src/main/java/edu/ucr/cs/riple/core/cache/BaseCache.java @@ -27,7 +27,6 @@ import com.google.common.collect.ImmutableSet; import edu.ucr.cs.riple.core.registries.index.Error; import edu.ucr.cs.riple.core.registries.index.Fix; -import edu.ucr.cs.riple.injector.location.Location; import java.util.Collection; import java.util.Map; import java.util.Objects; @@ -39,8 +38,7 @@ * @param type of impacts saved in this model. * @param type of the map used to store impacts. */ -public abstract class BaseCache> - implements ImpactCache { +public abstract class BaseCache> implements ImpactCache { /** Container holding cache entries. */ protected final S store; @@ -51,19 +49,19 @@ public BaseCache(S store) { @Override public boolean isUnknown(Fix fix) { - return !this.store.containsKey(fix.toLocation()); + return !this.store.containsKey(fix); } @Nullable @Override public T fetchImpact(Fix fix) { - return store.get(fix.toLocation()); + return store.get(fix); } @Override public ImmutableSet getTriggeredErrorsForCollection(Collection fixes) { return fixes.stream() - .map(fix -> store.get(fix.toLocation())) + .map(store::get) .filter(Objects::nonNull) .flatMap(impact -> impact.triggeredErrors.stream()) // filter errors that will be resolved with the existing collection of fixes. @@ -74,7 +72,7 @@ public ImmutableSet getTriggeredErrorsForCollection(Collection fixes @Override public ImmutableSet getTriggeredFixesFromDownstreamForCollection(Collection fixTree) { return fixTree.stream() - .map(fix -> store.get(fix.toLocation())) + .map(store::get) .filter(Objects::nonNull) .flatMap(impact -> impact.getTriggeredFixesFromDownstreamErrors().stream()) // filter fixes that are already inside tree. @@ -95,6 +93,6 @@ public void updateImpactsAfterInjection(Collection fixes) { @Override public int size() { - return this.store.values().size(); + return this.store.size(); } } diff --git a/annotator-core/src/main/java/edu/ucr/cs/riple/core/cache/Impact.java b/annotator-core/src/main/java/edu/ucr/cs/riple/core/cache/Impact.java index 62d91963b..363336030 100644 --- a/annotator-core/src/main/java/edu/ucr/cs/riple/core/cache/Impact.java +++ b/annotator-core/src/main/java/edu/ucr/cs/riple/core/cache/Impact.java @@ -27,7 +27,6 @@ import com.google.common.collect.ImmutableSet; import edu.ucr.cs.riple.core.registries.index.Error; import edu.ucr.cs.riple.core.registries.index.Fix; -import edu.ucr.cs.riple.injector.location.Location; import java.util.Collection; import java.util.Objects; import java.util.Set; @@ -90,15 +89,6 @@ public ImmutableSet getTriggeredFixesFromDownstreamErrors() { return triggeredFixesFromDownstreamErrors; } - /** - * Gets the containing location. - * - * @return Containing fix location. - */ - public Location toLocation() { - return fix.toLocation(); - } - @Override public int hashCode() { return fix.hashCode(); diff --git a/annotator-core/src/main/java/edu/ucr/cs/riple/core/cache/TargetModuleCache.java b/annotator-core/src/main/java/edu/ucr/cs/riple/core/cache/TargetModuleCache.java index 0fb7a12fc..74bff2d29 100644 --- a/annotator-core/src/main/java/edu/ucr/cs/riple/core/cache/TargetModuleCache.java +++ b/annotator-core/src/main/java/edu/ucr/cs/riple/core/cache/TargetModuleCache.java @@ -24,7 +24,7 @@ package edu.ucr.cs.riple.core.cache; -import edu.ucr.cs.riple.injector.location.Location; +import edu.ucr.cs.riple.core.registries.index.Fix; import java.util.HashMap; import java.util.Set; @@ -32,7 +32,7 @@ * Cache for storing impacts of fixes on target module. This cache's state is not immutable and can * be updated. */ -public class TargetModuleCache extends BaseCache> { +public class TargetModuleCache extends BaseCache> { public TargetModuleCache() { super(new HashMap<>()); @@ -44,6 +44,6 @@ public TargetModuleCache() { * @param newData New given impacts. */ public void updateCacheState(Set newData) { - newData.forEach(t -> store.put(t.toLocation(), t)); + newData.forEach(t -> store.put(t.fix, t)); } } diff --git a/annotator-core/src/main/java/edu/ucr/cs/riple/core/cache/downstream/DownstreamImpactCacheImpl.java b/annotator-core/src/main/java/edu/ucr/cs/riple/core/cache/downstream/DownstreamImpactCacheImpl.java index 1ca604d10..343a22736 100644 --- a/annotator-core/src/main/java/edu/ucr/cs/riple/core/cache/downstream/DownstreamImpactCacheImpl.java +++ b/annotator-core/src/main/java/edu/ucr/cs/riple/core/cache/downstream/DownstreamImpactCacheImpl.java @@ -51,7 +51,7 @@ * once created, cannot be updated. */ public class DownstreamImpactCacheImpl - extends BaseCache> + extends BaseCache> implements DownstreamImpactCache { /** Annotator context instance. */ @@ -130,7 +130,7 @@ public void analyzeDownstreamDependencies() { reports.forEach( report -> { DownstreamImpact impact = new DownstreamImpact(report); - store.put(report.root.toLocation(), impact); + store.put(report.root, impact); }); System.out.println("Analyzing downstream dependencies completed!"); } diff --git a/annotator-core/src/main/java/edu/ucr/cs/riple/core/cache/downstream/DownstreamImpactEvaluator.java b/annotator-core/src/main/java/edu/ucr/cs/riple/core/cache/downstream/DownstreamImpactEvaluator.java index aa13d73a8..94249a97e 100644 --- a/annotator-core/src/main/java/edu/ucr/cs/riple/core/cache/downstream/DownstreamImpactEvaluator.java +++ b/annotator-core/src/main/java/edu/ucr/cs/riple/core/cache/downstream/DownstreamImpactEvaluator.java @@ -57,7 +57,7 @@ protected void collectGraphResults(ImmutableSet reports) { node.triggeredErrors.stream() .filter( error -> - error.isSingleFix() + error.isSingleAnnotationFix() // Method is declared in the target module. && context.targetModuleInfo.declaredInModule( error.toResolvingLocation())) diff --git a/annotator-core/src/main/java/edu/ucr/cs/riple/core/checkers/nullaway/NullAway.java b/annotator-core/src/main/java/edu/ucr/cs/riple/core/checkers/nullaway/NullAway.java index a52f433bf..3d004a987 100644 --- a/annotator-core/src/main/java/edu/ucr/cs/riple/core/checkers/nullaway/NullAway.java +++ b/annotator-core/src/main/java/edu/ucr/cs/riple/core/checkers/nullaway/NullAway.java @@ -114,29 +114,30 @@ private NullAwayError deserializeErrorFromTSVLine(ModuleInfo moduleInfo, String Location nonnullTarget = Location.createLocationFromArrayInfo(Arrays.copyOfRange(values, 6, 12)); if (nonnullTarget == null && errorType.equals(NullAwayError.METHOD_INITIALIZER_ERROR)) { - ImmutableSet resolvingFixes = - generateFixesForUninitializedFields(errorMessage, region, moduleInfo); + Set annotationsOnField = + computeAddAnnotationInstancesForUninitializedFields( + errorMessage, region.clazz, moduleInfo); return createError( errorType, errorMessage, region, context.offsetHandler.getOriginalOffset(path, offset), - resolvingFixes, + annotationsOnField, moduleInfo); } if (nonnullTarget != null && nonnullTarget.isOnField()) { nonnullTarget = extendVariableList(nonnullTarget.toField(), moduleInfo); } - Fix resolvingFix = + Set annotations = nonnullTarget == null - ? null - : new Fix(new AddMarkerAnnotation(nonnullTarget, config.nullableAnnot)); + ? Set.of() + : Set.of(new AddMarkerAnnotation(nonnullTarget, config.nullableAnnot)); return createError( errorType, errorMessage, region, context.offsetHandler.getOriginalOffset(path, offset), - resolvingFix == null ? ImmutableSet.of() : ImmutableSet.of(resolvingFix), + annotations, moduleInfo); } @@ -181,25 +182,31 @@ private Set extractUninitializedFieldNames(String errorMessage) { } /** - * Generates a set of fixes for uninitialized fields from the given error message. + * Computes a set of {@link AddAnnotation} instances for fields that are uninitialized. This + * method extracts field names from the provided error message, and for each uninitialized field, + * it attempts to find the location of the field within the specified class. If a field's location + * is found, an {@link AddMarkerAnnotation} is created with the appropriate nullable annotation + * and added to the result set. * - * @param errorMessage Given error message. - * @param region Region where the error is reported. - * @return Set of fixes for uninitialized fields to resolve the given error. + * @param errorMessage the error message containing the details about uninitialized fields. + * @param encClass The class where this error is reported. + * @param module the {@link ModuleInfo} containing the field registry and configuration + * information. + * @return an {@link ImmutableSet} of {@link AddAnnotation} instances representing the fields that + * should have annotations added, based on their uninitialized status. */ - protected ImmutableSet generateFixesForUninitializedFields( - String errorMessage, Region region, ModuleInfo module) { + private ImmutableSet computeAddAnnotationInstancesForUninitializedFields( + String errorMessage, String encClass, ModuleInfo module) { return extractUninitializedFieldNames(errorMessage).stream() .map( field -> { OnField locationOnField = - module.getFieldRegistry().getLocationOnField(region.clazz, field); + module.getFieldRegistry().getLocationOnField(encClass, field); if (locationOnField == null) { return null; } - return new Fix( - new AddMarkerAnnotation( - extendVariableList(locationOnField, module), config.nullableAnnot)); + return new AddMarkerAnnotation( + extendVariableList(locationOnField, module), config.nullableAnnot); }) .filter(Objects::nonNull) .collect(ImmutableSet.toImmutableSet()); @@ -249,7 +256,7 @@ public void suppressRemainingErrors(AnnotationInjector injector) { // `@Nullable` is being passed as an argument, we add a `@NullUnmarked` annotation // to the called method. if (error.messageType.equals("PASS_NULLABLE") - && error.isSingleFix() + && error.isSingleAnnotationFix() && error.toResolvingLocation().isOnParameter()) { OnParameter nullableParameter = error.toResolvingParameter(); return context @@ -382,7 +389,7 @@ public void preprocess(AnnotationInjector injector) { * @param errorMessage Error Message from NullAway. * @param region Region where the error is reported. * @param offset Offset of program point in the source file where the error is reported. - * @param resolvingFixes Resolving fixes that can fix the error if all applied to source code. + * @param annotations Annotations that should be added source file to resolve the error. * @param module Module where this error is reported. * @return Creates and returns the corresponding {@link NullAwayError} instance using the provided * information. @@ -392,14 +399,16 @@ private NullAwayError createError( String errorMessage, Region region, int offset, - ImmutableSet resolvingFixes, + Set annotations, ModuleInfo module) { // Filter fixes on elements with explicit nonnull annotations. - ImmutableSet cleanedResolvingFixes = - resolvingFixes.stream() - .filter(f -> !module.getNonnullStore().hasExplicitNonnullAnnotation(f.toLocation())) + ImmutableSet cleanedAnnotations = + annotations.stream() + .filter( + annot -> + !module.getNonnullStore().hasExplicitNonnullAnnotation(annot.getLocation())) .collect(ImmutableSet.toImmutableSet()); - return new NullAwayError(errorType, errorMessage, region, offset, cleanedResolvingFixes); + return new NullAwayError(errorType, errorMessage, region, offset, cleanedAnnotations); } @Override diff --git a/annotator-core/src/main/java/edu/ucr/cs/riple/core/checkers/nullaway/NullAwayError.java b/annotator-core/src/main/java/edu/ucr/cs/riple/core/checkers/nullaway/NullAwayError.java index a5fcc9922..d6ac5758e 100644 --- a/annotator-core/src/main/java/edu/ucr/cs/riple/core/checkers/nullaway/NullAwayError.java +++ b/annotator-core/src/main/java/edu/ucr/cs/riple/core/checkers/nullaway/NullAwayError.java @@ -28,7 +28,9 @@ import edu.ucr.cs.riple.core.registries.index.Error; import edu.ucr.cs.riple.core.registries.index.Fix; import edu.ucr.cs.riple.core.registries.region.Region; +import edu.ucr.cs.riple.injector.changes.AddAnnotation; import java.util.Objects; +import java.util.Set; /** Represents an error reported by {@link NullAway}. */ public class NullAwayError extends Error { @@ -43,8 +45,15 @@ public NullAwayError( String message, Region region, int offset, - ImmutableSet resolvingFixes) { - super(messageType, message, region, offset, resolvingFixes); + Set annotations) { + super(messageType, message, region, offset, annotations); + } + + @Override + protected ImmutableSet computeFixesFromAnnotations(Set annotations) { + // In NullAway inference, each annotation is examined individually. Thus, we create a separate + // fix instance for each annotation. + return annotations.stream().map(Fix::new).collect(ImmutableSet.toImmutableSet()); } @Override diff --git a/annotator-core/src/main/java/edu/ucr/cs/riple/core/evaluators/graph/Node.java b/annotator-core/src/main/java/edu/ucr/cs/riple/core/evaluators/graph/Node.java index 52cbfd5ca..f94b31ddc 100644 --- a/annotator-core/src/main/java/edu/ucr/cs/riple/core/evaluators/graph/Node.java +++ b/annotator-core/src/main/java/edu/ucr/cs/riple/core/evaluators/graph/Node.java @@ -39,6 +39,7 @@ import java.util.HashSet; import java.util.Objects; import java.util.Set; +import java.util.stream.Collectors; /** * Vertex in {@link ConflictGraph} graph. It stores a fix tree (starting from a root) and all it's @@ -109,11 +110,15 @@ public void reCollectPotentiallyImpactedRegions(RegionRegistry regionRegistry) { // Add origins. this.regions.addAll(this.origins); this.tree.forEach( - fix -> this.regions.addAll(regionRegistry.getImpactedRegions(fix.toLocation()))); + fix -> + this.regions.addAll( + fix.toLocations().stream() + .flatMap(location -> regionRegistry.getImpactedRegions(location).stream()) + .collect(Collectors.toSet()))); // Add class initialization region, if a fix is modifying a parameter on constructor. this.tree.stream() .filter(fix -> fix.isOnParameter() && fix.isModifyingConstructor()) - .forEach(fix -> regions.add(new Region(fix.change.getLocation().clazz, "null"))); + .forEach(fix -> regions.add(new Region(fix.toParameter().clazz, "null"))); } /** diff --git a/annotator-core/src/main/java/edu/ucr/cs/riple/core/evaluators/graph/processors/AbstractConflictGraphProcessor.java b/annotator-core/src/main/java/edu/ucr/cs/riple/core/evaluators/graph/processors/AbstractConflictGraphProcessor.java index 9c2dad2d9..86df296d4 100644 --- a/annotator-core/src/main/java/edu/ucr/cs/riple/core/evaluators/graph/processors/AbstractConflictGraphProcessor.java +++ b/annotator-core/src/main/java/edu/ucr/cs/riple/core/evaluators/graph/processors/AbstractConflictGraphProcessor.java @@ -69,11 +69,11 @@ public AbstractConflictGraphProcessor(Context context, CompilerRunner runner, Su */ protected Set getTriggeredFixesFromDownstreamErrors(Node node) { Set currentLocationsTargetedByTree = - node.tree.stream().map(Fix::toLocation).collect(Collectors.toSet()); + node.tree.stream().flatMap(fix -> fix.toLocations().stream()).collect(Collectors.toSet()); return downstreamImpactCache.getTriggeredErrorsForCollection(node.tree).stream() .filter( error -> - error.isSingleFix() + error.isSingleAnnotationFix() && error.isFixableOnTarget(context) && !currentLocationsTargetedByTree.contains(error.toResolvingLocation())) .map( diff --git a/annotator-core/src/main/java/edu/ucr/cs/riple/core/injectors/AnnotationInjector.java b/annotator-core/src/main/java/edu/ucr/cs/riple/core/injectors/AnnotationInjector.java index 57786dd3a..82505b625 100644 --- a/annotator-core/src/main/java/edu/ucr/cs/riple/core/injectors/AnnotationInjector.java +++ b/annotator-core/src/main/java/edu/ucr/cs/riple/core/injectors/AnnotationInjector.java @@ -50,7 +50,9 @@ public void removeFixes(Set fixes) { return; } Set toRemove = - fixes.stream().map(fix -> fix.change.getReverse()).collect(Collectors.toSet()); + fixes.stream() + .flatMap(fix -> fix.changes.stream().map(AddAnnotation::getReverse)) + .collect(Collectors.toSet()); removeAnnotations(toRemove); } @@ -63,7 +65,8 @@ public void injectFixes(Set fixes) { if (fixes == null || fixes.size() == 0) { return; } - injectAnnotations(fixes.stream().map(fix -> fix.change).collect(Collectors.toSet())); + injectAnnotations( + fixes.stream().flatMap(fix -> fix.changes.stream()).collect(Collectors.toSet())); } /** diff --git a/annotator-core/src/main/java/edu/ucr/cs/riple/core/registries/index/Error.java b/annotator-core/src/main/java/edu/ucr/cs/riple/core/registries/index/Error.java index 86e779419..a1ebd16bd 100644 --- a/annotator-core/src/main/java/edu/ucr/cs/riple/core/registries/index/Error.java +++ b/annotator-core/src/main/java/edu/ucr/cs/riple/core/registries/index/Error.java @@ -27,11 +27,12 @@ import com.google.common.collect.ImmutableSet; import edu.ucr.cs.riple.core.Context; import edu.ucr.cs.riple.core.registries.region.Region; +import edu.ucr.cs.riple.injector.changes.AddAnnotation; import edu.ucr.cs.riple.injector.location.Location; import edu.ucr.cs.riple.injector.location.OnParameter; import java.util.Collection; import java.util.Objects; -import javax.annotation.Nullable; +import java.util.Set; /** Represents an error reported by NullAway. */ @SuppressWarnings("JavaLangClash") @@ -47,50 +48,49 @@ public abstract class Error { protected final int offset; /** Containing region. */ protected final Region region; + /** Error type for method initialization errors from NullAway in {@code String}. */ public Error( String messageType, String message, Region region, int offset, - ImmutableSet resolvingFixes) { + Set annotations) { this.region = region; this.messageType = messageType; this.message = message; this.offset = offset; - this.resolvingFixes = resolvingFixes; - } - - public Error( - String messageType, String message, Region region, int offset, @Nullable Fix resolvingFix) { - this( - messageType, - message, - region, - offset, - resolvingFix == null ? ImmutableSet.of() : ImmutableSet.of(resolvingFix)); + this.resolvingFixes = computeFixesFromAnnotations(annotations); } /** - * Checks if error is resolvable. + * Creates a set of {@link Fix} instances from the provided set of annotations that resolve the + * error. A fix instance can contain multiple annotations, which are grouped for evaluation. A fix + * is an input to the search algorithm, and if approved, all its contained annotations will be + * applied to the source code. * - * @return true if error is resolvable and false otherwise. + * @param annotations A set of annotations that, if fully applied, resolve the error. Each fix can + * contain a subset of these annotations. + * @return A set of fix instances, each representing a possible group of annotations. */ - public boolean hasFix() { - return this.resolvingFixes.size() > 0; - } + protected abstract ImmutableSet computeFixesFromAnnotations(Set annotations); + /** + * Getter for the set of fixes that resolves this error. + * + * @return Set of fixes that resolves this error. + */ public ImmutableSet getResolvingFixes() { return this.resolvingFixes; } /** - * Checks if error is resolvable with only one fix. + * Checks if error is resolvable with only one annotation. * - * @return true if error is resolvable with only one fix and false otherwise. + * @return true if error is resolvable with only one annotation and false otherwise. */ - public boolean isSingleFix() { - return this.resolvingFixes.size() == 1; + public boolean isSingleAnnotationFix() { + return !resolvingFixes.isEmpty() && resolvingFixes.iterator().next().changes.size() == 1; } /** @@ -99,9 +99,9 @@ public boolean isSingleFix() { * @return Location of the fix resolving this error. */ public Location toResolvingLocation() { - Preconditions.checkArgument(resolvingFixes.size() == 1); + Preconditions.checkArgument(isSingleAnnotationFix()); // no get() method, have to use iterator. - return resolvingFixes.iterator().next().toLocation(); + return resolvingFixes.iterator().next().changes.iterator().next().getLocation(); } /** @@ -166,9 +166,9 @@ public boolean equals(Object o) { * @return true, if error is resolvable via fixes on target module. */ public boolean isFixableOnTarget(Context context) { - return !resolvingFixes.isEmpty() - && this.resolvingFixes.stream() - .allMatch(fix -> context.targetModuleInfo.declaredInModule(fix.toLocation())); + return this.resolvingFixes.stream() + .flatMap(fix -> fix.changes.stream()) + .allMatch(change -> context.targetModuleInfo.declaredInModule(change.getLocation())); } @Override @@ -209,7 +209,7 @@ public static ImmutableSet getResolvingFixesOfErrors( * @return true, if this error is resolvable. */ public boolean isResolvableWith(Collection fixes) { - if (resolvingFixes.size() == 0) { + if (this.resolvingFixes.isEmpty()) { return false; } return fixes.containsAll(this.resolvingFixes); diff --git a/annotator-core/src/main/java/edu/ucr/cs/riple/core/registries/index/Fix.java b/annotator-core/src/main/java/edu/ucr/cs/riple/core/registries/index/Fix.java index cafe3f1b4..4fbaf9fe6 100644 --- a/annotator-core/src/main/java/edu/ucr/cs/riple/core/registries/index/Fix.java +++ b/annotator-core/src/main/java/edu/ucr/cs/riple/core/registries/index/Fix.java @@ -24,7 +24,10 @@ package edu.ucr.cs.riple.core.registries.index; +import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableSet; import edu.ucr.cs.riple.injector.Helper; +import edu.ucr.cs.riple.injector.changes.ASTChange; import edu.ucr.cs.riple.injector.changes.AddAnnotation; import edu.ucr.cs.riple.injector.location.Location; import edu.ucr.cs.riple.injector.location.LocationKind; @@ -33,7 +36,9 @@ import edu.ucr.cs.riple.injector.location.OnMethod; import edu.ucr.cs.riple.injector.location.OnParameter; import java.util.Objects; +import java.util.Set; import java.util.function.Consumer; +import java.util.stream.Collectors; import org.json.simple.JSONObject; /** @@ -42,38 +47,45 @@ */ public class Fix { - /** Suggested change. */ - public final AddAnnotation change; + /** The set of suggested changes that should be evaluated together by this fix instance. */ + public final Set changes; public Fix(AddAnnotation change) { - this.change = change; + this(ImmutableSet.of(change)); + } + + public Fix(ImmutableSet change) { + this.changes = change; } /** - * Returns the targeted location information. + * Returns the set of locations targeted by this fix instance. * - * @return Location information. + * @return Set of locations targeted by this fix instance. */ - public Location toLocation() { - return change.getLocation(); + public Set toLocations() { + return changes.stream().map(ASTChange::getLocation).collect(Collectors.toSet()); } /** - * Checks if fix is targeting a method. + * Checks if fix contains only one change and the change is on a method. * * @return true, if fix is targeting a method. */ public boolean isOnMethod() { - return change.getLocation().isOnMethod(); + return changes.size() == 1 && changes.iterator().next().getLocation().isOnMethod(); } /** - * Returns the targeted method information. + * Returns the targeted method location if this fix contains only one change and that change is on + * method. * * @return Target method information. */ public OnMethod toMethod() { - return change.getLocation().toMethod(); + Preconditions.checkArgument( + isOnMethod(), "This fix contains more than one change or the change is not on method."); + return changes.iterator().next().getLocation().toMethod(); } /** @@ -82,25 +94,31 @@ public OnMethod toMethod() { * @param consumer Consumer instance. */ public void ifOnMethod(Consumer consumer) { - change.getLocation().ifMethod(consumer); + if (isOnMethod()) { + changes.iterator().next().getLocation().ifMethod(consumer); + } } /** - * Checks if fix is targeting a parameter. + * Checks if fix contains only one change and the change is on a parameter. * * @return true, if fix is targeting a parameter. */ public boolean isOnParameter() { - return change.getLocation().isOnParameter(); + return changes.size() == 1 && changes.iterator().next().getLocation().isOnParameter(); } /** - * Returns the targeted parameter information. + * Returns the targeted parameter location if this fix contains only one change and that change is + * on a parameter. * * @return Target method parameter. */ public OnParameter toParameter() { - return change.getLocation().toParameter(); + Preconditions.checkArgument( + isOnParameter(), + "This fix contains more than one change or the change is not on parameter."); + return changes.iterator().next().getLocation().toParameter(); } /** @@ -109,25 +127,30 @@ public OnParameter toParameter() { * @param consumer Consumer instance. */ public void ifOnParameter(Consumer consumer) { - change.getLocation().ifParameter(consumer); + if (isOnParameter()) { + changes.iterator().next().getLocation().ifParameter(consumer); + } } /** - * Checks if fix is targeting a field. + * Checks if fix contains only one change and the change is on a field. * * @return true, if fix is targeting a field. */ public boolean isOnField() { - return change.getLocation().isOnField(); + return changes.size() == 1 && changes.iterator().next().getLocation().isOnField(); } /** - * Returns the targeted field information. + * Returns the targeted field location if this fix contains only one change and that change is on + * a field. * * @return Target field information. */ public OnField toField() { - return change.getLocation().toField(); + Preconditions.checkArgument( + isOnField(), "This fix contains more than one change or the change is not on field."); + return changes.iterator().next().getLocation().toField(); } /** @@ -136,7 +159,9 @@ public OnField toField() { * @param consumer Consumer instance. */ public void ifOnField(Consumer consumer) { - change.getLocation().ifField(consumer); + if (isOnField()) { + changes.iterator().next().getLocation().ifField(consumer); + } } @Override @@ -148,12 +173,12 @@ public boolean equals(Object o) { return false; } Fix fix = (Fix) o; - return change.equals(fix.change); + return changes.equals(fix.changes); } @Override public int hashCode() { - return Objects.hash(change); + return Objects.hash(changes); } /** @@ -162,26 +187,26 @@ public int hashCode() { * @return Json instance. */ public JSONObject getJson() { - return change.getLocation().accept(new LocationToJsonVisitor(), null); + return changes.iterator().next().getLocation().accept(new LocationToJsonVisitor(), null); } /** - * Checks if fix is modifying constructor (parameter or method). + * Checks if fix is a single change and is modifying constructor (parameter or method). * * @return true, if fix is modifying constructor. */ public boolean isModifyingConstructor() { - if (isOnField()) { + if (!(isOnMethod() || isOnParameter())) { return false; } String methodSignature = isOnMethod() ? toMethod().method : toParameter().enclosingMethod.method; - return Helper.extractCallableName(methodSignature) - .equals(Helper.simpleName(change.getLocation().clazz)); + String clazz = changes.iterator().next().getLocation().clazz; + return Helper.extractCallableName(methodSignature).equals(Helper.simpleName(clazz)); } @Override public String toString() { - return change.toString(); + return changes.toString(); } } diff --git a/annotator-core/src/main/java/edu/ucr/cs/riple/core/registries/region/generatedcode/LombokHandler.java b/annotator-core/src/main/java/edu/ucr/cs/riple/core/registries/region/generatedcode/LombokHandler.java index 12ed10524..6a9e7f050 100644 --- a/annotator-core/src/main/java/edu/ucr/cs/riple/core/registries/region/generatedcode/LombokHandler.java +++ b/annotator-core/src/main/java/edu/ucr/cs/riple/core/registries/region/generatedcode/LombokHandler.java @@ -87,42 +87,42 @@ public ImmutableSet extendForGeneratedFixes(Set fixes) { ImmutableSet.Builder builder = ImmutableSet.builder(); fixes.forEach( fix -> - fix.ifOnField( - onField -> - onField.variables.forEach( - name -> { - // Expected getter method signature. - String getterSignature = - "get" - + Character.toUpperCase(name.charAt(0)) - + name.substring(1) - + "()"; - // Check if method is lombok generated. - MethodRecord getterMethod = - moduleInfo - .getMethodRegistry() - .findMethodByName(onField.clazz, getterSignature); - if (getterMethod == null) { - // Getter method is not declared. skip. - // Note: If the getter method is generated, it should still be in the - // registry. - return; - } - if (isLombokGenerated(getterMethod.annotations)) { - // Method is lombok generated, add a fix to add the annotation on the - // method. - if (!(fix.change instanceof AnnotationChange)) { - // Only annotation changes are supported for now. - return; - } - AnnotationChange change = (AnnotationChange) fix.change; - builder.add( - new Fix( - new AddMarkerAnnotation( - getterMethod.location, - change.getAnnotationName().fullName))); - } - }))); + fix.changes.forEach( + addAnnotation -> + addAnnotation + .getLocation() + .ifField( + onField -> + onField.variables.forEach( + name -> { + // Expected getter method signature. + String getterSignature = + "get" + + Character.toUpperCase(name.charAt(0)) + + name.substring(1) + + "()"; + // Check if method is lombok generated. + MethodRecord getterMethod = + moduleInfo + .getMethodRegistry() + .findMethodByName(onField.clazz, getterSignature); + if (getterMethod == null) { + // Getter method is not declared. skip. + // Note: If the getter method is generated, it should still + // be in the registry. + return; + } + if (isLombokGenerated(getterMethod.annotations)) { + // Method is lombok generated, add a fix to add the + // annotation on the method. + AnnotationChange change = (AnnotationChange) addAnnotation; + builder.add( + new Fix( + new AddMarkerAnnotation( + getterMethod.location, + change.getAnnotationName().fullName))); + } + })))); return builder.build(); } diff --git a/annotator-core/src/test/java/edu/ucr/cs/riple/core/tools/CoreTestHelper.java b/annotator-core/src/test/java/edu/ucr/cs/riple/core/tools/CoreTestHelper.java index a3f3bffb0..f24b51fdc 100644 --- a/annotator-core/src/test/java/edu/ucr/cs/riple/core/tools/CoreTestHelper.java +++ b/annotator-core/src/test/java/edu/ucr/cs/riple/core/tools/CoreTestHelper.java @@ -479,7 +479,7 @@ private DEFAULT_PREDICATE(Config config) { @Override public boolean test(TReport expected, Report found) { - return expected.root.change.getLocation().equals(found.root.change.getLocation()) + return expected.root.equals(found.root) && expected.getExpectedValue() == found.getOverallEffect(config); } diff --git a/annotator-core/src/test/java/edu/ucr/cs/riple/core/tools/TError.java b/annotator-core/src/test/java/edu/ucr/cs/riple/core/tools/TError.java index 1f9444011..7f9dd883e 100644 --- a/annotator-core/src/test/java/edu/ucr/cs/riple/core/tools/TError.java +++ b/annotator-core/src/test/java/edu/ucr/cs/riple/core/tools/TError.java @@ -24,9 +24,13 @@ package edu.ucr.cs.riple.core.tools; +import com.google.common.collect.ImmutableSet; import edu.ucr.cs.riple.core.registries.index.Error; +import edu.ucr.cs.riple.core.registries.index.Fix; import edu.ucr.cs.riple.core.registries.region.Region; +import edu.ucr.cs.riple.injector.changes.AddAnnotation; import edu.ucr.cs.riple.injector.location.Location; +import java.util.Set; /** * Wrapper class for {@link Error} used to create dummy errors (with default values) as part of @@ -36,6 +40,11 @@ public class TError extends Error { public TError(Location location) { - super("null", "null", new Region("null", "null"), 0, new TFix(location)); + super("null", "null", new Region("null", "null"), 0, Set.of(new DefaultAnnotation(location))); + } + + @Override + protected ImmutableSet computeFixesFromAnnotations(Set annotations) { + return annotations.stream().map(Fix::new).collect(ImmutableSet.toImmutableSet()); } }