Skip to content

Commit 586a507

Browse files
jfeingold35jfeingold35
authored
Merge pull request #1014 from forcedotcom/d/W-11464344
NEW (GraphEngine): @W-11464344@: UnusedMethodRule supports static methods, tests enabled and passing.
2 parents a56f5be + 5aef9ee commit 586a507

File tree

12 files changed

+525
-69
lines changed

12 files changed

+525
-69
lines changed

‎sfge/src/main/java/com/salesforce/graph/build/CaseSafePropertyUtil.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,17 @@ public static GraphTraversal<Object, Object> hasWithin(
180180
}
181181
}
182182

183+
public static GraphTraversal<Object, Object> hasStartingWith(
184+
String nodeType, String property, String value) {
185+
String caseSafeVariant = toCaseSafeProperty(property).orElse(null);
186+
if (caseSafeVariant != null) {
187+
return __.has(
188+
nodeType, caseSafeVariant, TextP.startingWith(toCaseSafeValue(value)));
189+
} else {
190+
return __.has(nodeType, property, TextP.startingWith(value));
191+
}
192+
}
193+
183194
public static GraphTraversal<Object, Object> hasEndingWith(
184195
String nodeType, String property, String value) {
185196
if (property.equalsIgnoreCase(Schema.DEFINING_TYPE)) {

‎sfge/src/main/java/com/salesforce/graph/ops/ClassUtil.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import com.salesforce.graph.vertex.SFVertexFactory;
1414
import com.salesforce.graph.vertex.Typeable;
1515
import com.salesforce.graph.vertex.UserClassVertex;
16+
import java.util.List;
1617
import java.util.Optional;
1718
import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource;
1819

@@ -152,5 +153,20 @@ public static <V extends BaseSFVertex & Typeable> Optional<UserClassVertex> getU
152153
});
153154
}
154155

156+
/** Finds any inner classes declared within {@code definingType}. */
157+
public static List<UserClassVertex> getInnerClassesOf(
158+
GraphTraversalSource g, String definingType) {
159+
return SFVertexFactory.loadVertices(
160+
g,
161+
g.V()
162+
.where(
163+
H.has(
164+
ASTConstants.NodeType.USER_CLASS,
165+
Schema.DEFINING_TYPE,
166+
definingType))
167+
.out(Schema.CHILD)
168+
.hasLabel(ASTConstants.NodeType.USER_CLASS));
169+
}
170+
155171
private ClassUtil() {}
156172
}

‎sfge/src/main/java/com/salesforce/graph/ops/MethodUtil.java‎

Lines changed: 94 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -22,34 +22,7 @@
2222
import com.salesforce.graph.symbols.apex.ApexStandardValue;
2323
import com.salesforce.graph.symbols.apex.ApexValue;
2424
import com.salesforce.graph.symbols.apex.system.SystemSchema;
25-
import com.salesforce.graph.vertex.AbstractReferenceExpressionVertex;
26-
import com.salesforce.graph.vertex.ArrayLoadExpressionVertex;
27-
import com.salesforce.graph.vertex.AssignmentExpressionVertex;
28-
import com.salesforce.graph.vertex.BaseSFVertex;
29-
import com.salesforce.graph.vertex.BinaryExpressionVertex;
30-
import com.salesforce.graph.vertex.ChainedVertex;
31-
import com.salesforce.graph.vertex.DeclarationVertex;
32-
import com.salesforce.graph.vertex.EmptyReferenceExpressionVertex;
33-
import com.salesforce.graph.vertex.FieldVertex;
34-
import com.salesforce.graph.vertex.InvocableVertex;
35-
import com.salesforce.graph.vertex.InvocableWithParametersVertex;
36-
import com.salesforce.graph.vertex.LiteralExpressionVertex;
37-
import com.salesforce.graph.vertex.MethodCallExpressionVertex;
38-
import com.salesforce.graph.vertex.MethodVertex;
39-
import com.salesforce.graph.vertex.NewCollectionExpressionVertex;
40-
import com.salesforce.graph.vertex.NewObjectExpressionVertex;
41-
import com.salesforce.graph.vertex.ParameterVertex;
42-
import com.salesforce.graph.vertex.PostfixExpressionVertex;
43-
import com.salesforce.graph.vertex.PrefixExpressionVertex;
44-
import com.salesforce.graph.vertex.ReferenceExpressionVertex;
45-
import com.salesforce.graph.vertex.SFVertexFactory;
46-
import com.salesforce.graph.vertex.SoqlExpressionVertex;
47-
import com.salesforce.graph.vertex.SuperMethodCallExpressionVertex;
48-
import com.salesforce.graph.vertex.TernaryExpressionVertex;
49-
import com.salesforce.graph.vertex.ThisMethodCallExpressionVertex;
50-
import com.salesforce.graph.vertex.Typeable;
51-
import com.salesforce.graph.vertex.UserClassVertex;
52-
import com.salesforce.graph.vertex.VariableExpressionVertex;
25+
import com.salesforce.graph.vertex.*;
5326
import com.salesforce.graph.visitor.ApexPathWalker;
5427
import com.salesforce.graph.visitor.DefaultNoOpPathVertexVisitor;
5528
import com.salesforce.messaging.CliMessager;
@@ -75,6 +48,99 @@
7548
public final class MethodUtil {
7649
private static final Logger LOGGER = LogManager.getLogger(MethodUtil.class);
7750

51+
/**
52+
* Seeks the declaration of a method matching {@code signature} on {@code definingType} (or
53+
* optionally on its parent classes/interfaces).
54+
*
55+
* @param g - The graph containing the codebase
56+
* @param definingType - The type on which we should begin our search
57+
* @param signature - The method signature we're looking for
58+
* @param includeInheritedMethods -
59+
* @return - An Optional containing the matching method, if it exists.
60+
*/
61+
public static Optional<MethodVertex> getMethodWithSignature(
62+
GraphTraversalSource g,
63+
String definingType,
64+
String signature,
65+
boolean includeInheritedMethods) {
66+
// First, we need to check the target type itself.
67+
// Start by getting all the methods it defines directly.
68+
List<MethodVertex> methodsOnType =
69+
SFVertexFactory.loadVertices(
70+
g, g.V().where(H.has(NodeType.METHOD, Schema.DEFINING_TYPE, definingType)));
71+
// Check each method's signature and return it if it matches.
72+
for (MethodVertex methodVertex : methodsOnType) {
73+
if (methodVertex.getSignature().equalsIgnoreCase(signature)) {
74+
return Optional.of(methodVertex);
75+
}
76+
}
77+
78+
// If the type itself has no match, but we're asked to check inherited versions, do so.
79+
if (includeInheritedMethods) {
80+
InheritableSFVertex definingTypeVertex =
81+
SFVertexFactory.loadSingleOrNull(
82+
g,
83+
g.V()
84+
.where(
85+
H.has(
86+
Arrays.asList(
87+
NodeType.USER_CLASS,
88+
NodeType.USER_INTERFACE),
89+
Schema.DEFINING_TYPE,
90+
definingType)));
91+
if (definingTypeVertex != null) {
92+
// There are two forms of inheritance we need to check.
93+
// First, if the type has a superclass, then the method could be defined on that
94+
// superclass.
95+
// If the class has a superclass, we need to check that.
96+
Optional<String> superClassOptional = definingTypeVertex.getSuperClassName();
97+
if (superClassOptional.isPresent()) {
98+
// Recursive call on the superclass gets us the superclass's result.
99+
// If the result is present and non-private, then it's inherited by the target
100+
// class.
101+
Optional<MethodVertex> superclassResult =
102+
getMethodWithSignature(g, superClassOptional.get(), signature, true);
103+
if (superclassResult.isPresent() && !superclassResult.get().isPrivate()) {
104+
return superclassResult;
105+
}
106+
}
107+
// Second, if the defining type is an abstract class that implements interfaces,
108+
// then the method could be defined on one of those interfaces but implemented
109+
// on a concrete subclass.
110+
if (definingTypeVertex instanceof UserClassVertex) {
111+
UserClassVertex userClassVertex = (UserClassVertex) definingTypeVertex;
112+
if (userClassVertex.isAbstract()) {
113+
List<String> interfaceNames = userClassVertex.getInterfaceNames();
114+
for (String interfaceName : interfaceNames) {
115+
// Recursive call on the interface gets us the interface's result.
116+
Optional<MethodVertex> interfaceResult =
117+
getMethodWithSignature(g, interfaceName, signature, true);
118+
if (interfaceResult.isPresent()) {
119+
return interfaceResult;
120+
}
121+
}
122+
}
123+
}
124+
}
125+
}
126+
// If we still haven't found anything, we're done. Return an empty optional.
127+
return Optional.empty();
128+
}
129+
130+
/**
131+
* Find all {@link VariableDeclarationVertex} instances representing variables declared in the
132+
* scope of {@code method}.
133+
*/
134+
public static List<VariableDeclarationVertex> getVariableDeclarations(
135+
GraphTraversalSource g, MethodVertex method) {
136+
return SFVertexFactory.loadVertices(
137+
g,
138+
g.V()
139+
.hasId(method.getId())
140+
.repeat(__.out(Schema.CHILD))
141+
.until(__.hasLabel(NodeType.VARIABLE_DECLARATION)));
142+
}
143+
78144
public static List<MethodVertex> getTargetedMethods(
79145
GraphTraversalSource g, List<RuleRunnerTarget> targets) {
80146
// The targets passed into this method should exclusively be ones that target specific

‎sfge/src/main/java/com/salesforce/graph/ops/TraversalUtil.java‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,5 +136,49 @@ public static GraphTraversal<Vertex, Vertex> traverseImplementationsOf(
136136
}
137137
}
138138

139+
/**
140+
* Returns a traversal containing every invocation of {@code method} on something named {@code
141+
* reference} in any file in {@code targetFiles}.
142+
*
143+
* @param targetFiles - Files to target. An empty array implicitly targets the whole graph.
144+
*/
145+
public static GraphTraversal<Vertex, Vertex> traverseInvocationsOf(
146+
GraphTraversalSource g, List<String> targetFiles, String reference, String method) {
147+
// We want to traverse all instances of `reference.method()` occurring in the target files.
148+
GraphTraversal<Vertex, Vertex> traversal =
149+
g.V()
150+
// Starting with `hasLabel()` saves us a ton of time.
151+
.hasLabel(NodeType.METHOD_CALL_EXPRESSION)
152+
// Filter for invocations of the desired method name.
153+
.where(H.has(NodeType.METHOD_CALL_EXPRESSION, Schema.METHOD_NAME, method))
154+
// Filter for references to the desired host.
155+
.where(
156+
__.out(Schema.CHILD)
157+
.where(
158+
H.has(
159+
NodeType.REFERENCE_EXPRESSION,
160+
Schema.NAME,
161+
reference))
162+
.count()
163+
.is(P.gte(1)));
164+
165+
// If there are no target files, we're clear to return this traversal, since it has
166+
// everything we want.
167+
if (targetFiles.isEmpty()) {
168+
return traversal;
169+
} else {
170+
// Otherwise, we need to filter for classes in the target files, which we can do via
171+
// this traversal.
172+
Object[] targetIds =
173+
fileRootTraversal(g, targetFiles)
174+
.union(__.identity(), __.repeat(__.out(Schema.CHILD)).emit())
175+
.hasLabel(NodeType.USER_CLASS)
176+
.id()
177+
.toList()
178+
.toArray();
179+
return traversal.hasId(targetIds);
180+
}
181+
}
182+
139183
private TraversalUtil() {}
140184
}

‎sfge/src/main/java/com/salesforce/graph/vertex/MethodCallExpressionVertex.java‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,15 @@ public Optional<String> getKeyName() {
8888
}
8989
}
9090

91+
/**
92+
* If this method call occurs in the context of a field declaration, return that declaration.
93+
*
94+
* @return - Optional containing the field declaration, if it exists.
95+
*/
96+
public Optional<FieldDeclarationVertex> getFieldDeclaration() {
97+
return getFirstParentOfType(NodeType.FIELD_DECLARATION);
98+
}
99+
91100
public Optional<ClassRefExpressionVertex> getClassRefExpression() {
92101
AbstractReferenceExpressionVertex abstractReferenceExpression = referenceExpression.get();
93102
if (abstractReferenceExpression instanceof ReferenceExpressionVertex) {

‎sfge/src/main/java/com/salesforce/rules/UnusedMethodRule.java‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -144,10 +144,21 @@ private List<Violation> convertMethodsToViolations() {
144144
* for filtering the list of all possible candidates into just the eligible ones.
145145
*/
146146
private boolean methodIsIneligible(MethodVertex vertex) {
147-
// TODO: At this time, only private instance methods and private/protected constructors are
148-
// supported. This limit will be loosened over time, and eventually removed entirely.
149-
if (!(vertex.isPrivate() || (vertex.isProtected() && vertex.isConstructor()))
150-
|| vertex.isStatic()) {
147+
// TODO: At this time, only static methods, private instance methods, and private/protected
148+
// constructors are supported. This limit will be loosened over time, and eventually
149+
// removed entirely.
150+
if (!vertex.isStatic()
151+
&& !(vertex.isPrivate() || (vertex.isProtected() && vertex.isConstructor()))) {
152+
return true;
153+
}
154+
155+
// Test methods are ineligible.
156+
if (vertex.isTest()) {
157+
return true;
158+
}
159+
160+
// The "<clinit>" method is inherently ineligible.
161+
if (vertex.getName().equalsIgnoreCase("<clinit>")) {
151162
return true;
152163
}
153164

‎sfge/src/main/java/com/salesforce/rules/unusedmethod/BaseMethodCallValidator.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@
33
import com.salesforce.graph.vertex.ChainedVertex;
44
import com.salesforce.graph.vertex.InvocableWithParametersVertex;
55
import com.salesforce.graph.vertex.MethodVertex;
6+
import com.salesforce.graph.vertex.UserClassVertex;
67
import java.util.List;
8+
import java.util.Optional;
79

810
/**
911
* Abstract base class for validating that a given method is actually invoked. Used as a helper
@@ -63,4 +65,16 @@ protected boolean parametersAreValid(InvocableWithParametersVertex vertex) {
6365
List<ChainedVertex> parameters = vertex.getParameters();
6466
return parameters.size() == targetMethod.getArity();
6567
}
68+
69+
/** Indicates whether {@link #targetMethod} can be inherited by subclasses. */
70+
protected boolean methodIsHeritable() {
71+
Optional<UserClassVertex> classOptional = targetMethod.getParentClass();
72+
// A method is heritable if the following criteria are met:
73+
return
74+
// The method is non-private...
75+
!targetMethod.isPrivate()
76+
// ...and the host class is extensible (virtual or abstract).
77+
&& classOptional.isPresent()
78+
&& (classOptional.get().isAbstract() || classOptional.get().isVirtual());
79+
}
6680
}

‎sfge/src/main/java/com/salesforce/rules/unusedmethod/ConstructorMethodCallValidator.java‎

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
import com.salesforce.graph.vertex.*;
44
import java.util.List;
5-
import java.util.Optional;
65

76
/**
87
* Helper class for {@link com.salesforce.rules.UnusedMethodRule}. Used for determining whether a
@@ -42,20 +41,12 @@ protected boolean internalUsageDetected() {
4241
}
4342
}
4443

45-
// Before checking subclasses, verify that it even makes sense to do so.
46-
// A private constructor cannot be called by a subclass.
47-
if (targetMethod.isPrivate()) {
44+
// If the constructor isn't heritable, we're done.
45+
if (!methodIsHeritable()) {
4846
return false;
4947
}
50-
// If the parent class is neither virtual nor abstract, it can't have any subclasses.
51-
Optional<UserClassVertex> classOptional = targetMethod.getParentClass();
52-
if (!classOptional.isPresent()) {
53-
return false;
54-
} else if (!classOptional.get().isAbstract() && !classOptional.get().isVirtual()) {
55-
return false;
56-
}
57-
// If it's possible for this constructor to be called in a subclass, get those subclasses
58-
// and check for calls.
48+
// If the constructor is heritable, then we have to check for usage by subclasses via
49+
// `super()`.
5950
List<String> subclassNames = ruleStateTracker.getSubclasses(targetMethod.getDefiningType());
6051
for (String subclassName : subclassNames) {
6152
List<SuperMethodCallExpressionVertex> subclassPotentialCalls =

0 commit comments

Comments
 (0)