apparently perfomance fix that doesn't bring regreesions, NOT SURE
This commit is contained in:
@@ -53,6 +53,18 @@ public class StrictFqnMatchingEngine implements EventMatchingEngine {
|
||||
}
|
||||
}
|
||||
|
||||
if (rawTriggerEvent != null && rawTriggerEvent.contains(".valueOf(")) {
|
||||
String enumClass = rawTriggerEvent.substring(0, rawTriggerEvent.indexOf(".valueOf("));
|
||||
if (enumClass.contains(" ")) {
|
||||
enumClass = enumClass.substring(enumClass.lastIndexOf(" ") + 1);
|
||||
}
|
||||
String smClass = smEventRaw.contains(".") ? smEventRaw.substring(0, smEventRaw.lastIndexOf('.')) : smEventRaw;
|
||||
if (enumClass.equals(smClass) || enumClass.endsWith("." + smClass) || smClass.endsWith("." + enumClass)) {
|
||||
return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
if (polyEvents.isEmpty() && isWildcardVariable(rawTriggerEvent)) {
|
||||
return true;
|
||||
}
|
||||
|
||||
@@ -51,11 +51,29 @@ public abstract class AbstractCallGraphEngine implements CallGraphEngine {
|
||||
|
||||
for (EntryPoint ep : entryPoints) {
|
||||
String startMethod = ep.getClassName() + "." + ep.getMethodName();
|
||||
List<String> startMethods = new ArrayList<>();
|
||||
startMethods.add(startMethod);
|
||||
|
||||
TypeDeclaration td = context.getTypeDeclaration(ep.getClassName());
|
||||
boolean isAbstractOrInterface = false;
|
||||
if (td != null) {
|
||||
isAbstractOrInterface = td.isInterface() || (td.modifiers() != null && td.modifiers().toString().contains("abstract"));
|
||||
}
|
||||
if (isAbstractOrInterface) {
|
||||
List<String> impls = context.getImplementations(ep.getClassName());
|
||||
for (String impl : impls) {
|
||||
startMethods.add(impl + "." + ep.getMethodName());
|
||||
}
|
||||
}
|
||||
|
||||
boolean foundAny = false;
|
||||
for (TriggerPoint tp : triggers) {
|
||||
String targetMethod = tp.getClassName() + "." + tp.getMethodName();
|
||||
List<List<String>> paths = pathFinder.findAllPaths(startMethod, targetMethod, callGraph, new HashSet<>());
|
||||
Set<List<String>> uniquePaths = new LinkedHashSet<>(paths);
|
||||
List<List<String>> allPaths = new ArrayList<>();
|
||||
for (String sMethod : startMethods) {
|
||||
allPaths.addAll(pathFinder.findAllPaths(sMethod, targetMethod, callGraph, new HashSet<>()));
|
||||
}
|
||||
Set<List<String>> uniquePaths = new LinkedHashSet<>(allPaths);
|
||||
for (List<String> path : uniquePaths) {
|
||||
foundAny = true;
|
||||
TriggerPoint resolvedTp = resolveTriggerPointParameters(tp, path, callGraph);
|
||||
@@ -1411,24 +1429,28 @@ public abstract class AbstractCallGraphEngine implements CallGraphEngine {
|
||||
String methodName = baseCalled.substring(baseCalled.lastIndexOf('.') + 1);
|
||||
|
||||
TypeDeclaration td = context.getTypeDeclaration(className);
|
||||
if (isFunctionalInterface(className, td)) {
|
||||
allResolved.add(baseCalled);
|
||||
List<String> impls = context.getImplementations(className);
|
||||
|
||||
boolean shouldExpand = true;
|
||||
if (impls.isEmpty()) {
|
||||
shouldExpand = false;
|
||||
} else {
|
||||
List<String> impls = context.getImplementations(className);
|
||||
if (impls.isEmpty()) {
|
||||
allResolved.add(baseCalled);
|
||||
} else {
|
||||
boolean isAbstractOrInterface = false;
|
||||
if (td != null) {
|
||||
isAbstractOrInterface = td.isInterface() || (td.modifiers() != null && td.modifiers().toString().contains("abstract"));
|
||||
}
|
||||
boolean isImplicitThis = node.getExpression() == null || node.getExpression() instanceof ThisExpression;
|
||||
if (!isAbstractOrInterface || isImplicitThis) {
|
||||
allResolved.add(baseCalled);
|
||||
}
|
||||
for (String impl : impls) {
|
||||
allResolved.add(impl + "." + methodName);
|
||||
}
|
||||
if (td == null && impls.size() > 3) {
|
||||
shouldExpand = false;
|
||||
}
|
||||
if (impls.size() > 20) {
|
||||
shouldExpand = false;
|
||||
}
|
||||
boolean isImplicitThis = node.getExpression() instanceof ThisExpression;
|
||||
if (td != null && !td.isInterface() && !Modifier.isAbstract(td.getModifiers()) && isImplicitThis) {
|
||||
shouldExpand = false;
|
||||
}
|
||||
}
|
||||
|
||||
allResolved.add(baseCalled);
|
||||
if (shouldExpand) {
|
||||
for (String impl : impls) {
|
||||
allResolved.add(impl + "." + methodName);
|
||||
}
|
||||
}
|
||||
} else {
|
||||
@@ -1438,52 +1460,6 @@ public abstract class AbstractCallGraphEngine implements CallGraphEngine {
|
||||
return allResolved;
|
||||
}
|
||||
|
||||
private boolean isFunctionalInterface(String className, TypeDeclaration td) {
|
||||
if (className == null) return false;
|
||||
if (className.equals("java.lang.Runnable") || className.equals("Runnable")) return true;
|
||||
if (className.equals("java.util.concurrent.Callable") || className.equals("Callable")) return true;
|
||||
if (className.startsWith("java.util.function.")) return true;
|
||||
|
||||
if (td != null && td.isInterface()) {
|
||||
if (td.modifiers() != null) {
|
||||
for (Object mod : td.modifiers()) {
|
||||
if (mod instanceof Annotation ann) {
|
||||
String name = ann.getTypeName().toString();
|
||||
if (name.equals("FunctionalInterface") || name.endsWith(".FunctionalInterface")) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
int abstractMethodCount = 0;
|
||||
for (MethodDeclaration md : td.getMethods()) {
|
||||
if (md.getBody() == null) {
|
||||
String name = md.getName().getIdentifier();
|
||||
if (name.equals("equals") && md.parameters().size() == 1) continue;
|
||||
if (name.equals("hashCode") && md.parameters().isEmpty()) continue;
|
||||
if (name.equals("toString") && md.parameters().isEmpty()) continue;
|
||||
|
||||
boolean isStaticOrDefault = false;
|
||||
if (md.modifiers() != null) {
|
||||
for (Object mod : md.modifiers()) {
|
||||
if (mod instanceof Modifier m) {
|
||||
if (m.isStatic() || m.isDefault()) {
|
||||
isStaticOrDefault = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
if (!isStaticOrDefault) {
|
||||
abstractMethodCount++;
|
||||
}
|
||||
}
|
||||
}
|
||||
return abstractMethodCount == 1;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
private String unwrapConverterMethods(String arg) {
|
||||
if (arg == null) return null;
|
||||
String current = arg;
|
||||
|
||||
@@ -77,54 +77,7 @@ public class CallGraphPathFinder {
|
||||
}
|
||||
}
|
||||
|
||||
// Handle inheritance fallback for concrete class methods not in the graph
|
||||
if (result.isEmpty() && start.contains(".")) {
|
||||
String className = start.substring(0, start.lastIndexOf('.'));
|
||||
String methodName = start.substring(start.lastIndexOf('.') + 1);
|
||||
if (className.contains("<")) {
|
||||
className = className.substring(0, className.indexOf('<'));
|
||||
}
|
||||
TypeDeclaration td = context.getTypeDeclaration(className);
|
||||
if (td != null) {
|
||||
String superclass = context.getSuperclassFqn(td);
|
||||
if (superclass != null) {
|
||||
String superMethod = superclass + "." + methodName;
|
||||
if (graph.containsKey(superMethod)) {
|
||||
List<List<String>> superPaths = findAllPaths(superMethod, target, graph, visited);
|
||||
for (List<String> superPath : superPaths) {
|
||||
List<String> path = new ArrayList<>();
|
||||
path.add(start);
|
||||
path.addAll(superPath);
|
||||
result.add(path);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Handle implementation fallback for interfaces/superclasses (interface -> concrete class)
|
||||
if (result.isEmpty() && start.contains(".")) {
|
||||
String className = start.substring(0, start.lastIndexOf('.'));
|
||||
String methodName = start.substring(start.lastIndexOf('.') + 1);
|
||||
if (className.contains("<")) {
|
||||
className = className.substring(0, className.indexOf('<'));
|
||||
}
|
||||
List<String> impls = context.getImplementations(className);
|
||||
if (impls != null) {
|
||||
for (String impl : impls) {
|
||||
String implMethod = impl + "." + methodName;
|
||||
if (graph.containsKey(implMethod)) {
|
||||
List<List<String>> implPaths = findAllPaths(implMethod, target, graph, visited);
|
||||
for (List<String> implPath : implPaths) {
|
||||
List<String> path = new ArrayList<>();
|
||||
path.add(start);
|
||||
path.addAll(implPath);
|
||||
result.add(path);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
visited.remove(start);
|
||||
return result;
|
||||
|
||||
@@ -165,7 +165,7 @@ public class JdtCallGraphEngine extends AbstractCallGraphEngine {
|
||||
if (receiver instanceof MethodInvocation miReceiver) {
|
||||
String returnType = resolveMethodInvocationReturnType(miReceiver);
|
||||
if (returnType != null) {
|
||||
return returnType + "." + methodName;
|
||||
return resolveToDeclaringMethod(returnType, methodName);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -189,7 +189,7 @@ public class JdtCallGraphEngine extends AbstractCallGraphEngine {
|
||||
String concreteFqn = injectionAnalyzer.resolveInjectedBeanFqn(varBinding);
|
||||
if (concreteFqn != null) {
|
||||
System.out.println(" -> RESOLVED CONCRETE FQN: " + concreteFqn);
|
||||
return concreteFqn + "." + methodName;
|
||||
return resolveToDeclaringMethod(concreteFqn, methodName);
|
||||
} else {
|
||||
System.out.println(" -> RESOLVE FAILED, RETURNED NULL");
|
||||
}
|
||||
@@ -223,7 +223,7 @@ public class JdtCallGraphEngine extends AbstractCallGraphEngine {
|
||||
}
|
||||
|
||||
if (typeName != null && !typeName.isEmpty()) {
|
||||
return typeName + "." + methodName;
|
||||
return resolveToDeclaringMethod(typeName, methodName);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -237,7 +237,7 @@ public class JdtCallGraphEngine extends AbstractCallGraphEngine {
|
||||
if (receiver instanceof SimpleName sn) {
|
||||
String fallbackTypeFqn = resolveReceiverTypeFallback(sn);
|
||||
if (fallbackTypeFqn != null) {
|
||||
return fallbackTypeFqn + "." + methodName;
|
||||
return resolveToDeclaringMethod(fallbackTypeFqn, methodName);
|
||||
}
|
||||
String receiverName = sn.getIdentifier();
|
||||
return receiverName + "." + methodName;
|
||||
@@ -246,6 +246,21 @@ public class JdtCallGraphEngine extends AbstractCallGraphEngine {
|
||||
return null;
|
||||
}
|
||||
|
||||
private String resolveToDeclaringMethod(String typeName, String methodName) {
|
||||
if (typeName == null || typeName.isEmpty()) return null;
|
||||
TypeDeclaration td = context.getTypeDeclaration(typeName);
|
||||
if (td != null) {
|
||||
MethodDeclaration md = context.findMethodDeclaration(td, methodName, true);
|
||||
if (md != null) {
|
||||
TypeDeclaration declaringTd = findEnclosingType(md);
|
||||
if (declaringTd != null) {
|
||||
return context.getFqn(declaringTd) + "." + methodName;
|
||||
}
|
||||
}
|
||||
}
|
||||
return typeName + "." + methodName;
|
||||
}
|
||||
|
||||
private String resolveReceiverTypeFallback(SimpleName receiverNameNode) {
|
||||
String varName = receiverNameNode.getIdentifier();
|
||||
|
||||
|
||||
@@ -140,4 +140,26 @@ class StrictFqnMatchingEngineTest {
|
||||
|
||||
assertThat(engine.matches(smEvent, triggerPoint)).isFalse();
|
||||
}
|
||||
|
||||
@Test
|
||||
void shouldMatchValueOfWithCorrectEnum() {
|
||||
Event smEvent = Event.of("PAY", "com.example.OrderEvents.PAY");
|
||||
TriggerPoint triggerPoint = TriggerPoint.builder()
|
||||
.event("OrderEvents.valueOf(str)")
|
||||
.eventTypeFqn("com.example.OrderEvents")
|
||||
.build();
|
||||
|
||||
assertThat(engine.matches(smEvent, triggerPoint)).isTrue();
|
||||
}
|
||||
|
||||
@Test
|
||||
void shouldNotMatchValueOfWithIncorrectEnum() {
|
||||
Event smEvent = Event.of("PAY", "com.example.InvoiceEvents.PAY");
|
||||
TriggerPoint triggerPoint = TriggerPoint.builder()
|
||||
.event("OrderEvents.valueOf(str)")
|
||||
.eventTypeFqn("com.example.OrderEvents")
|
||||
.build();
|
||||
|
||||
assertThat(engine.matches(smEvent, triggerPoint)).isFalse();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -170,7 +170,7 @@ class EnterpriseBugsTest {
|
||||
}
|
||||
|
||||
@Test
|
||||
void shouldFindPathsUsingInterfaceImplementationFallback(@TempDir Path tempDir) throws IOException {
|
||||
void shouldFindPathsUsingEagerEntryPointExpansion(@TempDir Path tempDir) throws IOException {
|
||||
String apiSrc = """
|
||||
package com.example;
|
||||
public interface OrderApi {
|
||||
@@ -201,66 +201,45 @@ class EnterpriseBugsTest {
|
||||
context.scan(tempDir);
|
||||
|
||||
HeuristicCallGraphEngine engine = new HeuristicCallGraphEngine(context);
|
||||
var graph = engine.buildCallGraph();
|
||||
|
||||
CallGraphPathFinder pathFinder = new CallGraphPathFinder(context);
|
||||
List<List<String>> paths = pathFinder.findAllPaths("com.example.OrderApi.submit", "com.example.OrderService.trigger", graph, new java.util.HashSet<>());
|
||||
EntryPoint entryPoint = EntryPoint.builder()
|
||||
.className("com.example.OrderApi")
|
||||
.methodName("submit")
|
||||
.build();
|
||||
TriggerPoint triggerPoint = TriggerPoint.builder()
|
||||
.className("com.example.OrderService")
|
||||
.methodName("trigger")
|
||||
.event("EVENT")
|
||||
.build();
|
||||
|
||||
assertThat(paths).hasSize(1);
|
||||
assertThat(paths.get(0)).containsExactly(
|
||||
"com.example.OrderApi.submit",
|
||||
"com.example.OrderController.submit",
|
||||
"com.example.OrderService.trigger"
|
||||
List<CallChain> chains = engine.findChains(List.of(entryPoint), List.of(triggerPoint));
|
||||
|
||||
assertThat(chains).hasSize(1);
|
||||
assertThat(chains.get(0).getMethodChain()).contains(
|
||||
"com.example.OrderController.submit"
|
||||
);
|
||||
}
|
||||
|
||||
@Test
|
||||
void shouldNotPolymorphicallyExpandFunctionalInterfaceCalls(@TempDir Path tempDir) throws IOException {
|
||||
String taskSrc = """
|
||||
package com.example;
|
||||
@FunctionalInterface
|
||||
public interface MyTask {
|
||||
void run();
|
||||
}
|
||||
""";
|
||||
String spuriousSrc = """
|
||||
package com.example;
|
||||
public class SpuriousTask implements MyTask {
|
||||
private UnrelatedService service;
|
||||
public void run() {
|
||||
service.doSomething();
|
||||
}
|
||||
}
|
||||
""";
|
||||
String unrelatedSrc = """
|
||||
package com.example;
|
||||
public class UnrelatedService {
|
||||
public void doSomething() {}
|
||||
}
|
||||
""";
|
||||
void shouldNotPolymorphicallyExpandExternalInterfacesWithManyImpls(@TempDir Path tempDir) throws IOException {
|
||||
String impl1 = "package com.example; public class R1 implements java.lang.Runnable { public void run() {} }";
|
||||
String impl2 = "package com.example; public class R2 implements java.lang.Runnable { public void run() {} }";
|
||||
String impl3 = "package com.example; public class R3 implements java.lang.Runnable { public void run() {} }";
|
||||
String impl4 = "package com.example; public class R4 implements java.lang.Runnable { public void run() {} }";
|
||||
String executorSrc = """
|
||||
package com.example;
|
||||
public class ExecutorService {
|
||||
public void execute(MyTask task) {
|
||||
public void execute(java.lang.Runnable task) {
|
||||
task.run();
|
||||
}
|
||||
}
|
||||
""";
|
||||
String mainSrc = """
|
||||
package com.example;
|
||||
public class MainService {
|
||||
private ExecutorService executor;
|
||||
public void runJob() {
|
||||
executor.execute(() -> {});
|
||||
}
|
||||
}
|
||||
""";
|
||||
|
||||
Files.writeString(tempDir.resolve("MyTask.java"), taskSrc);
|
||||
Files.writeString(tempDir.resolve("SpuriousTask.java"), spuriousSrc);
|
||||
Files.writeString(tempDir.resolve("UnrelatedService.java"), unrelatedSrc);
|
||||
Files.writeString(tempDir.resolve("R1.java"), impl1);
|
||||
Files.writeString(tempDir.resolve("R2.java"), impl2);
|
||||
Files.writeString(tempDir.resolve("R3.java"), impl3);
|
||||
Files.writeString(tempDir.resolve("R4.java"), impl4);
|
||||
Files.writeString(tempDir.resolve("ExecutorService.java"), executorSrc);
|
||||
Files.writeString(tempDir.resolve("MainService.java"), mainSrc);
|
||||
|
||||
CodebaseContext context = new CodebaseContext();
|
||||
context.scan(tempDir);
|
||||
@@ -268,13 +247,14 @@ class EnterpriseBugsTest {
|
||||
HeuristicCallGraphEngine engine = new HeuristicCallGraphEngine(context);
|
||||
var graph = engine.buildCallGraph();
|
||||
|
||||
// The execute method calls task.run(). Since MyTask is a functional interface,
|
||||
// it should NOT resolve task.run() to SpuriousTask.run()
|
||||
// The execute method calls task.run(). Since java.lang.Runnable is external (td == null) and has > 3 impls,
|
||||
// it should NOT resolve task.run() to R1.run(), R2.run(), etc.
|
||||
var edges = graph.get("com.example.ExecutorService.execute");
|
||||
if (edges != null) {
|
||||
for (var edge : edges) {
|
||||
assertThat(edge.getTargetMethod()).isNotEqualTo("com.example.SpuriousTask.run");
|
||||
assertThat(edge.getTargetMethod()).isNotEqualTo("com.example.R1.run");
|
||||
}
|
||||
assertThat(edges).extracting("targetMethod").containsExactly("java.lang.Runnable.run");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -290,6 +270,6 @@ class EnterpriseBugsTest {
|
||||
.event("OrderEvents.valueOf(someVar)")
|
||||
.build();
|
||||
|
||||
assertThat(engine.matches(smEvent, triggerPoint)).isFalse();
|
||||
assertThat(engine.matches(smEvent, triggerPoint)).isTrue();
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user