Skip to content

Commit

Permalink
Eliminate redundant line number (#1220)
Browse files Browse the repository at this point in the history
Some classes store a `int lineNumber` field next to a
`SourceCodeLocation`.
This PR eliminates the redundancy, which suggests more degrees of
freedom than there actually are.
  • Loading branch information
codecholeric authored Mar 11, 2024
2 parents 5639421 + ef726a4 commit c4785e5
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -58,30 +58,28 @@
public final class Dependency implements HasDescription, Comparable<Dependency>, HasSourceCodeLocation {
private final JavaClass originClass;
private final JavaClass targetClass;
private final int lineNumber;
private final String description;
private final SourceCodeLocation sourceCodeLocation;
private final int hashCode;

private Dependency(JavaClass originClass, JavaClass targetClass, int lineNumber, String description) {
private Dependency(JavaClass originClass, JavaClass targetClass, SourceCodeLocation sourceCodeLocation, String description) {
checkArgument(!originClass.equals(targetClass) || targetClass.isPrimitive(),
"Tried to create illegal dependency '%s' (%s -> %s), this is likely a bug!",
description, originClass.getSimpleName(), targetClass.getSimpleName());

this.originClass = originClass;
this.targetClass = targetClass;
this.lineNumber = lineNumber;
this.description = description;
this.sourceCodeLocation = SourceCodeLocation.of(originClass, lineNumber);
hashCode = Objects.hash(originClass, targetClass, lineNumber, description);
this.sourceCodeLocation = sourceCodeLocation;
hashCode = Objects.hash(originClass, targetClass, sourceCodeLocation, description);
}

static Set<Dependency> tryCreateFromAccess(JavaAccess<?> access) {
JavaClass originOwner = access.getOriginOwner();
JavaClass targetOwner = access.getTargetOwner();
ImmutableSet.Builder<Dependency> dependencies = ImmutableSet.<Dependency>builder()
.addAll(createComponentTypeDependencies(originOwner, access.getOrigin().getDescription(), targetOwner, access.getSourceCodeLocation()));
dependencies.addAll(asSet(tryCreateDependency(originOwner, targetOwner, access.getDescription(), access.getLineNumber())));
dependencies.addAll(asSet(tryCreateDependency(originOwner, targetOwner, access.getDescription(), access.getSourceCodeLocation())));
return dependencies.build();
}

Expand All @@ -97,7 +95,7 @@ static Dependency fromInheritance(JavaClass origin, JavaClass targetSupertype) {
String dependencyDescription = originDescription + " " + dependencyType + " " + targetType + " " + targetDescription;

String description = dependencyDescription + " in " + origin.getSourceCodeLocation();
Optional<Dependency> result = tryCreateDependency(origin, targetSupertype, description, 0);
Optional<Dependency> result = tryCreateDependency(origin, targetSupertype, description, origin.getSourceCodeLocation());

if (!result.isPresent()) {
throw new IllegalStateException(String.format("Tried to create illegal inheritance dependency '%s' (%s -> %s), this is likely a bug!",
Expand Down Expand Up @@ -214,7 +212,7 @@ private static Set<Dependency> tryCreateDependency(
String targetDescription = bracketFormat(targetClass.getName());
String dependencyDescription = originDescription + " " + dependencyType + " " + targetDescription;
String description = dependencyDescription + " in " + sourceCodeLocation;
dependencies.addAll(asSet(tryCreateDependency(originClass, targetClass, description, sourceCodeLocation.getLineNumber())));
dependencies.addAll(asSet(tryCreateDependency(originClass, targetClass, description, sourceCodeLocation)));
return dependencies.build();
}

Expand All @@ -227,17 +225,17 @@ private static Set<Dependency> createComponentTypeDependencies(
String componentTypeTargetDescription = bracketFormat(componentType.get().getName());
String componentTypeDependencyDescription = originDescription + " depends on component type " + componentTypeTargetDescription;
String componentTypeDescription = componentTypeDependencyDescription + " in " + sourceCodeLocation;
result.addAll(asSet(tryCreateDependency(originClass, componentType.get(), componentTypeDescription, sourceCodeLocation.getLineNumber())));
result.addAll(asSet(tryCreateDependency(originClass, componentType.get(), componentTypeDescription, sourceCodeLocation)));
componentType = componentType.get().tryGetComponentType();
}
return result.build();
}

private static Optional<Dependency> tryCreateDependency(JavaClass originClass, JavaClass targetClass, String description, int lineNumber) {
private static Optional<Dependency> tryCreateDependency(JavaClass originClass, JavaClass targetClass, String description, SourceCodeLocation sourceCodeLocation) {
if (originClass.equals(targetClass) || targetClass.isPrimitive()) {
return Optional.empty();
}
return Optional.of(new Dependency(originClass, targetClass, lineNumber, description));
return Optional.of(new Dependency(originClass, targetClass, sourceCodeLocation, description));
}

private static String bracketFormat(String name) {
Expand Down Expand Up @@ -276,7 +274,7 @@ public SourceCodeLocation getSourceCodeLocation() {
@PublicAPI(usage = ACCESS)
public int compareTo(Dependency o) {
return ComparisonChain.start()
.compare(lineNumber, o.lineNumber)
.compare(sourceCodeLocation.getLineNumber(), o.sourceCodeLocation.getLineNumber())
.compare(getDescription(), o.getDescription())
.result();
}
Expand All @@ -297,7 +295,7 @@ public boolean equals(Object obj) {
Dependency other = (Dependency) obj;
return Objects.equals(this.originClass, other.originClass)
&& Objects.equals(this.targetClass, other.targetClass)
&& Objects.equals(this.lineNumber, other.lineNumber)
&& Objects.equals(this.sourceCodeLocation.getLineNumber(), other.sourceCodeLocation.getLineNumber())
&& Objects.equals(this.description, other.description);
}

Expand All @@ -306,7 +304,7 @@ public String toString() {
return MoreObjects.toStringHelper(this)
.add("originClass", originClass)
.add("targetClass", targetClass)
.add("lineNumber", lineNumber)
.add("sourceCodeLocation", sourceCodeLocation)
.add("description", description)
.toString();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,12 @@ public final class InstanceofCheck implements HasType, HasOwner<JavaCodeUnit>, H

private final JavaCodeUnit owner;
private final JavaClass type;
private final int lineNumber;
private final boolean declaredInLambda;
private final SourceCodeLocation sourceCodeLocation;

private InstanceofCheck(JavaCodeUnit owner, JavaClass type, int lineNumber, boolean declaredInLambda) {
this.owner = checkNotNull(owner);
this.type = checkNotNull(type);
this.lineNumber = lineNumber;
this.declaredInLambda = declaredInLambda;
sourceCodeLocation = SourceCodeLocation.of(owner.getOwner(), lineNumber);
}
Expand All @@ -61,7 +59,7 @@ public JavaCodeUnit getOwner() {

@PublicAPI(usage = ACCESS)
public int getLineNumber() {
return lineNumber;
return sourceCodeLocation.getLineNumber();
}

@PublicAPI(usage = ACCESS)
Expand All @@ -79,7 +77,7 @@ public String toString() {
return toStringHelper(this)
.add("owner", owner)
.add("type", type)
.add("lineNumber", lineNumber)
.add("sourceCodeLocation", sourceCodeLocation)
.add("declaredInLambda", declaredInLambda)
.toString();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,13 @@ public abstract class JavaAccess<TARGET extends AccessTarget>

private final JavaCodeUnit origin;
private final TARGET target;
private final int lineNumber;
private final SourceCodeLocation sourceCodeLocation;
private final boolean declaredInLambda;

JavaAccess(JavaAccessBuilder<TARGET, ?> builder) {
this.origin = checkNotNull(builder.getOrigin());
this.target = checkNotNull(builder.getTarget());
this.lineNumber = builder.getLineNumber();
this.sourceCodeLocation = SourceCodeLocation.of(getOriginOwner(), lineNumber);
this.sourceCodeLocation = SourceCodeLocation.of(getOriginOwner(), builder.getLineNumber());
this.declaredInLambda = builder.isDeclaredInLambda();
}

Expand Down Expand Up @@ -77,7 +75,7 @@ public TARGET getTarget() {

@PublicAPI(usage = ACCESS)
public int getLineNumber() {
return lineNumber;
return sourceCodeLocation.getLineNumber();
}

@Override
Expand All @@ -100,7 +98,7 @@ public boolean isDeclaredInLambda() {
@Override
public String toString() {
return getClass().getSimpleName() +
"{origin=" + origin + ", target=" + target + ", lineNumber=" + lineNumber + additionalToStringFields() + '}';
"{origin=" + origin + ", target=" + target + ", lineNumber=" + getLineNumber() + additionalToStringFields() + '}';
}

String additionalToStringFields() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ public boolean isMethod() {
return true;
}

@Override
@PublicAPI(usage = ACCESS)
public Set<JavaMethodCall> getCallsOfSelf() {
return getReverseDependencies().getCallsTo(this);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,12 @@
public final class ReferencedClassObject implements HasType, HasOwner<JavaCodeUnit>, HasSourceCodeLocation {
private final JavaCodeUnit owner;
private final JavaClass value;
private final int lineNumber;
private final SourceCodeLocation sourceCodeLocation;
private final boolean declaredInLambda;

private ReferencedClassObject(JavaCodeUnit owner, JavaClass value, int lineNumber, boolean declaredInLambda) {
this.owner = checkNotNull(owner);
this.value = checkNotNull(value);
this.lineNumber = lineNumber;
sourceCodeLocation = SourceCodeLocation.of(owner.getOwner(), lineNumber);
this.declaredInLambda = declaredInLambda;
}
Expand Down Expand Up @@ -66,7 +64,7 @@ public JavaClass getValue() {

@PublicAPI(usage = ACCESS)
public int getLineNumber() {
return lineNumber;
return sourceCodeLocation.getLineNumber();
}

@Override
Expand Down

0 comments on commit c4785e5

Please sign in to comment.