Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor - Use Fix instance directly in APIs instead of set of locations. #250

Merged
merged 6 commits into from
Oct 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 3 additions & 8 deletions annotator-core/src/main/java/edu/ucr/cs/riple/core/Report.java
Original file line number Diff line number Diff line change
Expand Up @@ -191,23 +191,18 @@ public boolean testEquals(Config config, Report found) {
}
Set<Fix> thisTriggered =
this.triggeredErrors.stream()
.flatMap(error -> error.getResolvingFixes().stream())
.flatMap(Error::getResolvingFixesStream)
.collect(Collectors.toSet());
Set<Fix> otherTriggered =
found.triggeredErrors.stream()
.flatMap(error -> error.getResolvingFixes().stream())
.flatMap(Error::getResolvingFixesStream)
.collect(Collectors.toSet());
return otherTriggered.equals(thisTriggered);
}

@Override
public String toString() {
return "Effect="
+ localEffect
+ ", "
+ root
+ ", "
+ tree.stream().map(Fix::toLocations).collect(Collectors.toSet());
return "Effect=" + localEffect + ", " + root + ", " + tree;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,7 @@
import edu.ucr.cs.riple.core.Report;
import edu.ucr.cs.riple.core.evaluators.BasicEvaluator;
import edu.ucr.cs.riple.core.evaluators.suppliers.DownstreamDependencySupplier;
import edu.ucr.cs.riple.core.registries.index.Error;
import edu.ucr.cs.riple.injector.location.Location;
import java.util.Set;
import java.util.stream.Collectors;

/**
* Evaluator for analyzing downstream dependencies. Used by {@link DownstreamImpactCacheImpl} to
Expand All @@ -47,31 +44,30 @@ public DownstreamImpactEvaluator(DownstreamDependencySupplier supplier) {
@Override
protected void collectGraphResults(ImmutableSet<Report> reports) {
super.collectGraphResults(reports);
// Collect impacted parameters in target module by downstream dependencies.
// Update path for each location instances if declared in target module in the computed
// triggered fixes. These triggered fixes do not have an actual physical path since they are
// provided as a jar file in downstream dependencies.
this.graph
.getNodes()
.forEach(
node -> {
// Impacted locations.
Set<Location> locations =
node.triggeredErrors.stream()
.filter(
error ->
error.isSingleAnnotationFix()
// Method is declared in the target module.
&& context.targetModuleInfo.declaredInModule(
error.toResolvingLocation()))
.map(Error::toResolvingLocation)
.collect(Collectors.toSet());
if (!locations.isEmpty()) {
// Update path for each location. These triggered fixes do not have an
// actual physical path since they are provided as a jar file in downstream
// dependencies.
locations.forEach(
location ->
location.path =
context.targetModuleInfo.getLocationOnClass(location.clazz).path);
}
node.triggeredErrors.forEach(
error ->
error
.getResolvingFixesStream()
.forEach(
fix ->
fix.changes.forEach(
annot -> {
Location location = annot.getLocation();
// check if the location is inside the target module.
if (context.targetModuleInfo.declaredInModule(location)) {
location.path =
context.targetModuleInfo.getLocationOnClass(
location.clazz)
.path;
}
})));
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import edu.ucr.cs.riple.core.module.ModuleConfiguration;
import edu.ucr.cs.riple.core.module.ModuleInfo;
import edu.ucr.cs.riple.core.registries.field.FieldInitializationStore;
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.core.util.Utility;
Expand Down Expand Up @@ -232,8 +233,6 @@ public void suppressRemainingErrors(AnnotationInjector injector) {
// Collect regions with remaining errors.
Utility.buildTarget(context);
Set<NullAwayError> remainingErrors = deserializeErrors(context.targetModuleInfo);
Set<Fix> remainingFixes =
Utility.readFixesFromOutputDirectory(context, context.targetModuleInfo);
// 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.
Expand Down Expand Up @@ -327,7 +326,7 @@ public void suppressRemainingErrors(AnnotationInjector injector) {
.filter(
e ->
e.messageType.equals("METHOD_NO_INIT") || e.messageType.equals("FIELD_NO_INIT"))
.flatMap(e -> e.getResolvingFixes().stream())
.flatMap(Error::getResolvingFixesStream)
.filter(Fix::isOnField)
// Filter nodes annotated with SuppressWarnings("NullAway")
.filter(fix -> !fieldsWithSuppressWarnings.contains(fix.toField()))
Expand Down Expand Up @@ -368,7 +367,7 @@ public void preprocess(AnnotationInjector injector) {
context, context.targetModuleInfo, NullAwayError.class)
.stream()
.filter(e -> e.messageType.equals("FIELD_NO_INIT"))
.flatMap(e -> e.getResolvingFixes().stream())
.flatMap(Error::getResolvingFixesStream)
.filter(Fix::isOnField)
.map(Fix::toField)
.collect(Collectors.toSet());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,9 @@
import edu.ucr.cs.riple.core.evaluators.suppliers.Supplier;
import edu.ucr.cs.riple.core.injectors.AnnotationInjector;
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.ErrorStore;
import edu.ucr.cs.riple.core.registries.index.Fix;
import edu.ucr.cs.riple.injector.changes.AddMarkerAnnotation;
import edu.ucr.cs.riple.injector.location.Location;
import java.util.Set;
import java.util.stream.Collectors;

Expand Down Expand Up @@ -73,19 +72,9 @@ public AbstractConflictGraphProcessor(Context context, CompilerRunner runner, Su
* @param node Node in process.
*/
protected Set<Fix> getTriggeredFixesFromDownstreamErrors(Node node) {
Set<Location> currentLocationsTargetedByTree =
node.tree.stream().flatMap(fix -> fix.toLocations().stream()).collect(Collectors.toSet());
return downstreamImpactCache.getTriggeredErrorsForCollection(node.tree).stream()
.filter(
error ->
error.isSingleAnnotationFix()
&& error.isFixableOnTarget(context)
&& !currentLocationsTargetedByTree.contains(error.toResolvingLocation()))
.map(
error ->
new Fix(
new AddMarkerAnnotation(
error.toResolvingLocation(), context.config.nullableAnnot)))
.filter(error -> error.isFixableOnTarget(context))
.flatMap(Error::getResolvingFixesStream)
.collect(Collectors.toSet());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import com.google.common.collect.ImmutableSet;
import edu.ucr.cs.riple.core.Context;
import edu.ucr.cs.riple.core.registries.field.FieldRegistry;
import edu.ucr.cs.riple.core.registries.index.Fix;
import edu.ucr.cs.riple.core.registries.index.NonnullStore;
import edu.ucr.cs.riple.core.registries.method.MethodRegistry;
import edu.ucr.cs.riple.core.registries.region.CompoundRegionRegistry;
Expand Down Expand Up @@ -161,10 +162,10 @@ public ImmutableSet<ModuleConfiguration> getModuleConfigurations() {
}

/**
* Checks if the passed location is declared in this moduleInfo's modules.
* Checks if the passed location is declared in containing modules.
*
* @param location The location to check.
* @return True if the passed location is declared in this moduleInfo's modules, false otherwise.
* @return True if the passed location is declared in containing modules., false otherwise.
*/
public boolean declaredInModule(Location location) {
if (location.isOnParameter()) {
Expand All @@ -179,6 +180,17 @@ public boolean declaredInModule(Location location) {
return methodRegistry.declaredInModule(location);
}

/**
* Checks if the passed fix contains only annotation changes on elements declared in the
* containing modules.
*
* @param fix The Fix to check.
* @return True if the passed fix contains annotation changes only on containing modules.
*/
public boolean declaredInModule(Fix fix) {
return fix.changes.stream().allMatch(annot -> declaredInModule(annot.getLocation()));
}

/**
* Creates a {@link edu.ucr.cs.riple.injector.location.OnClass} instance targeting the passed
* classes flat name.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import java.util.Collection;
import java.util.Objects;
import java.util.Set;
import java.util.stream.Stream;

/** Represents an error reported by NullAway. */
@SuppressWarnings("JavaLangClash")
Expand Down Expand Up @@ -88,6 +89,15 @@ public ImmutableSet<Fix> getResolvingFixes() {
return this.resolvingFixes;
}

/**
* Returns a stream of resolving fixes for this error.
*
* @return Stream of resolving fixes.
*/
public Stream<Fix> getResolvingFixesStream() {
return this.resolvingFixes.stream();
}

/**
* Checks if error is resolvable with only one annotation.
*
Expand Down Expand Up @@ -170,9 +180,7 @@ public boolean equals(Object o) {
* @return true, if error is resolvable via fixes on target module.
*/
public boolean isFixableOnTarget(Context context) {
return this.resolvingFixes.stream()
.flatMap(fix -> fix.changes.stream())
.allMatch(change -> context.targetModuleInfo.declaredInModule(change.getLocation()));
return this.resolvingFixes.stream().allMatch(context.targetModuleInfo::declaredInModule);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@

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;
Expand Down Expand Up @@ -199,10 +198,7 @@ public boolean isModifyingConstructor() {
if (!(isOnMethod() || isOnParameter())) {
return false;
}
String methodSignature =
isOnMethod() ? toMethod().method : toParameter().enclosingMethod.method;
String clazz = changes.iterator().next().getLocation().clazz;
return Helper.extractCallableName(methodSignature).equals(Helper.simpleName(clazz));
return (isOnMethod() ? toMethod() : toParameter().enclosingMethod).isOnConstructor();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,7 @@ public ImmutableSet<Region> getImpactedRegionsByUse(Location location) {
OnField field = location.toField();
return findRecordsWithHashHint(
candidate ->
candidate.calleeClass.equals(field.clazz)
&& field.isOnFieldWithName(candidate.calleeMember),
candidate.encClass.equals(field.clazz) && field.isOnFieldWithName(candidate.member),
RegionRecord.hash(field.clazz))
.map(regionRecord -> regionRecord.region)
.collect(ImmutableSet.toImmutableSet());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ public ImmutableSet<Region> getImpactedRegionsByUse(Location location) {
// Add callers of method.
return findRecordsWithHashHint(
candidate ->
candidate.calleeClass.equals(onMethod.clazz)
&& candidate.calleeMember.equals(onMethod.method),
candidate.encClass.equals(onMethod.clazz)
&& candidate.member.equals(onMethod.method),
RegionRecord.hash(onMethod.clazz))
.map(node -> node.region)
.collect(ImmutableSet.toImmutableSet());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,21 +26,26 @@

import java.util.Objects;

/** Container class for holding information regarding usage of class members within classes. */
/**
* Records for holding information regarding usage of class members within class regions across the
* program. (E.g. It saves the information field "f" of class "Baz" is used within region
* "a.b.c.Foo#bar()")
*/
public class RegionRecord {

/** Region where the element is used. */
public final Region region;
/** The used member from class {@link RegionRecord#encClass} string representation. */
public final String member;

public final String calleeMember;
/** Fully qualified name of the enclosing class of used member. */
public final String encClass;

/** Fully qualified name of the enclosing class of callee. */
public final String calleeClass;
/** Region where the {@link RegionRecord#member} is used. */
public final Region region;

public RegionRecord(Region region, String calleeMember, String calleeClass) {
public RegionRecord(Region region, String member, String encClass) {
this.region = region;
this.calleeMember = calleeMember;
this.calleeClass = calleeClass;
this.member = member;
this.encClass = encClass;
}

@Override
Expand All @@ -53,15 +58,15 @@ public boolean equals(Object o) {
}
RegionRecord that = (RegionRecord) o;
return region.equals(that.region)
&& calleeMember.equals(that.calleeMember)
&& calleeClass.equals(that.calleeClass);
&& member.equals(that.member)
&& encClass.equals(that.encClass);
}

/**
* Calculates hash. This method is used outside this class to calculate the expected hash based on
* instance's properties value if the actual instance is not available.
*
* @param clazz Full qualified name.
* @param clazz Full qualified name of the enclosing class of the used member.
* @return Expected hash.
*/
public static int hash(String clazz) {
Expand All @@ -70,6 +75,6 @@ public static int hash(String clazz) {

@Override
public int hashCode() {
return hash(calleeClass);
return hash(encClass);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -99,4 +99,13 @@ public int hashCode() {
public String toString() {
return "OnMethod{" + "method='" + method + '\'' + ", clazz='" + clazz + '\'' + '}';
}

/**
* Checks if this location is targeting a constructor.
*
* @return True if this location is targeting a constructor.
*/
public boolean isOnConstructor() {
return Helper.extractCallableName(method).equals(Helper.simpleName(clazz));
}
}
Loading