Fail closed on ambiguous enum type matching during linking
Pass CodebaseContext into StrictFqnMatchingEngine so import-style OrderEvent.PAY cannot match a.OrderEvent.PAY when OrderEvent is ambiguous across packages. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -32,7 +32,6 @@ import click.kamil.springstatemachineexporter.analysis.resolver.StateMachineType
|
|||||||
public class TransitionLinkerEnricher implements AnalysisEnricher {
|
public class TransitionLinkerEnricher implements AnalysisEnricher {
|
||||||
|
|
||||||
private final BeanResolutionEngine routingEngine = new HeuristicBeanResolutionEngine();
|
private final BeanResolutionEngine routingEngine = new HeuristicBeanResolutionEngine();
|
||||||
private final EventMatchingEngine matchingEngine = new StrictFqnMatchingEngine();
|
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public void enrich(AnalysisResult result, CodebaseContext context, CodebaseIntelligenceProvider intelligence) {
|
public void enrich(AnalysisResult result, CodebaseContext context, CodebaseIntelligenceProvider intelligence) {
|
||||||
@@ -40,6 +39,8 @@ public class TransitionLinkerEnricher implements AnalysisEnricher {
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
EventMatchingEngine matchingEngine = new StrictFqnMatchingEngine(context);
|
||||||
|
|
||||||
List<CallChain> updatedChains = new ArrayList<>();
|
List<CallChain> updatedChains = new ArrayList<>();
|
||||||
List<Transition> stateMachineTransitions = result.getTransitions();
|
List<Transition> stateMachineTransitions = result.getTransitions();
|
||||||
StateMachineTypeResolver.MachineTypes machineTypes =
|
StateMachineTypeResolver.MachineTypes machineTypes =
|
||||||
|
|||||||
@@ -2,11 +2,22 @@ package click.kamil.springstatemachineexporter.analysis.enricher.matching;
|
|||||||
|
|
||||||
import click.kamil.springstatemachineexporter.analysis.model.TriggerPoint;
|
import click.kamil.springstatemachineexporter.analysis.model.TriggerPoint;
|
||||||
import click.kamil.springstatemachineexporter.analysis.resolver.MachineEnumCanonicalizer;
|
import click.kamil.springstatemachineexporter.analysis.resolver.MachineEnumCanonicalizer;
|
||||||
|
import click.kamil.springstatemachineexporter.ast.common.CodebaseContext;
|
||||||
import click.kamil.springstatemachineexporter.model.Event;
|
import click.kamil.springstatemachineexporter.model.Event;
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
|
|
||||||
public class StrictFqnMatchingEngine implements EventMatchingEngine {
|
public class StrictFqnMatchingEngine implements EventMatchingEngine {
|
||||||
|
|
||||||
|
private final CodebaseContext context;
|
||||||
|
|
||||||
|
public StrictFqnMatchingEngine() {
|
||||||
|
this(null);
|
||||||
|
}
|
||||||
|
|
||||||
|
public StrictFqnMatchingEngine(CodebaseContext context) {
|
||||||
|
this.context = context;
|
||||||
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public boolean matches(Event stateMachineEvent, TriggerPoint triggerPoint) {
|
public boolean matches(Event stateMachineEvent, TriggerPoint triggerPoint) {
|
||||||
if (stateMachineEvent == null || triggerPoint == null || triggerPoint.getEvent() == null) {
|
if (stateMachineEvent == null || triggerPoint == null || triggerPoint.getEvent() == null) {
|
||||||
@@ -52,7 +63,7 @@ public class StrictFqnMatchingEngine implements EventMatchingEngine {
|
|||||||
if (pe.equals(smConst) && triggerPoint.getEventTypeFqn() != null
|
if (pe.equals(smConst) && triggerPoint.getEventTypeFqn() != null
|
||||||
&& !isStringTypeOrPrimitive(triggerPoint.getEventTypeFqn())) {
|
&& !isStringTypeOrPrimitive(triggerPoint.getEventTypeFqn())) {
|
||||||
String smEnumType = smEventRaw.substring(0, smEventRaw.lastIndexOf('.'));
|
String smEnumType = smEventRaw.substring(0, smEventRaw.lastIndexOf('.'));
|
||||||
return MachineEnumCanonicalizer.enumTypesMatch(triggerPoint.getEventTypeFqn(), smEnumType);
|
return typesMatch(triggerPoint.getEventTypeFqn(), smEnumType);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -103,12 +114,12 @@ public class StrictFqnMatchingEngine implements EventMatchingEngine {
|
|||||||
String smEnumType = smEvent.substring(0, smEvent.lastIndexOf('.'));
|
String smEnumType = smEvent.substring(0, smEvent.lastIndexOf('.'));
|
||||||
|
|
||||||
if (eventTypeFqn != null) {
|
if (eventTypeFqn != null) {
|
||||||
return MachineEnumCanonicalizer.enumTypesMatch(eventTypeFqn, smEnumType);
|
return typesMatch(eventTypeFqn, smEnumType);
|
||||||
}
|
}
|
||||||
|
|
||||||
if (triggerEvent.contains(".")) {
|
if (triggerEvent.contains(".")) {
|
||||||
String triggerEnumType = triggerEvent.substring(0, triggerEvent.lastIndexOf('.'));
|
String triggerEnumType = triggerEvent.substring(0, triggerEvent.lastIndexOf('.'));
|
||||||
return MachineEnumCanonicalizer.enumTypesMatch(triggerEnumType, smEnumType);
|
return typesMatch(triggerEnumType, smEnumType);
|
||||||
}
|
}
|
||||||
|
|
||||||
// Bare constant name (e.g. after Enum.name()) with no type context must not match every enum
|
// Bare constant name (e.g. after Enum.name()) with no type context must not match every enum
|
||||||
@@ -119,6 +130,10 @@ public class StrictFqnMatchingEngine implements EventMatchingEngine {
|
|||||||
return !triggerEvent.contains(".");
|
return !triggerEvent.contains(".");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private boolean typesMatch(String type1, String type2) {
|
||||||
|
return MachineEnumCanonicalizer.enumTypesMatch(type1, type2, context);
|
||||||
|
}
|
||||||
|
|
||||||
private String constantName(String event) {
|
private String constantName(String event) {
|
||||||
return event.contains(".") ? event.substring(event.lastIndexOf('.') + 1) : event;
|
return event.contains(".") ? event.substring(event.lastIndexOf('.') + 1) : event;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,5 +1,6 @@
|
|||||||
package click.kamil.springstatemachineexporter.analysis.enricher.routing;
|
package click.kamil.springstatemachineexporter.analysis.enricher.routing;
|
||||||
|
|
||||||
|
import click.kamil.springstatemachineexporter.analysis.enricher.matching.EventMatchingEngine;
|
||||||
import click.kamil.springstatemachineexporter.analysis.enricher.matching.StrictFqnMatchingEngine;
|
import click.kamil.springstatemachineexporter.analysis.enricher.matching.StrictFqnMatchingEngine;
|
||||||
import click.kamil.springstatemachineexporter.analysis.model.CallChain;
|
import click.kamil.springstatemachineexporter.analysis.model.CallChain;
|
||||||
import click.kamil.springstatemachineexporter.analysis.model.TriggerPoint;
|
import click.kamil.springstatemachineexporter.analysis.model.TriggerPoint;
|
||||||
@@ -14,8 +15,6 @@ import java.util.Set;
|
|||||||
|
|
||||||
public class HeuristicBeanResolutionEngine implements BeanResolutionEngine {
|
public class HeuristicBeanResolutionEngine implements BeanResolutionEngine {
|
||||||
|
|
||||||
private final StrictFqnMatchingEngine matchingEngine = new StrictFqnMatchingEngine();
|
|
||||||
|
|
||||||
private enum DistinctTypeAffinity {
|
private enum DistinctTypeAffinity {
|
||||||
FOR_MACHINE,
|
FOR_MACHINE,
|
||||||
AGAINST_MACHINE,
|
AGAINST_MACHINE,
|
||||||
@@ -64,7 +63,7 @@ public class HeuristicBeanResolutionEngine implements BeanResolutionEngine {
|
|||||||
TriggerPoint trigger = chain.getTriggerPoint();
|
TriggerPoint trigger = chain.getTriggerPoint();
|
||||||
if (trigger != null
|
if (trigger != null
|
||||||
&& SharedServiceRoutingPolicy.isProvablySharedInfrastructure(chain)
|
&& SharedServiceRoutingPolicy.isProvablySharedInfrastructure(chain)
|
||||||
&& matchesConfiguredTransitionEvent(trigger, machineTransitions)) {
|
&& matchesConfiguredTransitionEvent(trigger, machineTransitions, context)) {
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -189,10 +188,14 @@ public class HeuristicBeanResolutionEngine implements BeanResolutionEngine {
|
|||||||
return DistinctTypeAffinity.UNKNOWN;
|
return DistinctTypeAffinity.UNKNOWN;
|
||||||
}
|
}
|
||||||
|
|
||||||
private boolean matchesConfiguredTransitionEvent(TriggerPoint trigger, List<Transition> machineTransitions) {
|
private boolean matchesConfiguredTransitionEvent(
|
||||||
|
TriggerPoint trigger,
|
||||||
|
List<Transition> machineTransitions,
|
||||||
|
CodebaseContext context) {
|
||||||
if (trigger == null || machineTransitions == null || machineTransitions.isEmpty()) {
|
if (trigger == null || machineTransitions == null || machineTransitions.isEmpty()) {
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
EventMatchingEngine matchingEngine = new StrictFqnMatchingEngine(context);
|
||||||
for (Transition transition : machineTransitions) {
|
for (Transition transition : machineTransitions) {
|
||||||
if (transition.getEvent() != null && matchingEngine.matches(transition.getEvent(), trigger)) {
|
if (transition.getEvent() != null && matchingEngine.matches(transition.getEvent(), trigger)) {
|
||||||
return true;
|
return true;
|
||||||
|
|||||||
@@ -809,6 +809,17 @@ public final class MachineEnumCanonicalizer {
|
|||||||
return !type1.contains(".") || !type2.contains(".");
|
return !type1.contains(".") || !type2.contains(".");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public static boolean enumTypesMatch(String type1, String type2, CodebaseContext context) {
|
||||||
|
if (context != null && type1 != null && type2 != null) {
|
||||||
|
String simple1 = simpleName(type1);
|
||||||
|
String simple2 = simpleName(type2);
|
||||||
|
if (simple1.equals(simple2) && context.isAmbiguousSimpleName(simple1)) {
|
||||||
|
return type1.equals(type2);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return enumTypesMatch(type1, type2);
|
||||||
|
}
|
||||||
|
|
||||||
private static String simpleName(String fqn) {
|
private static String simpleName(String fqn) {
|
||||||
return fqn.contains(".") ? fqn.substring(fqn.lastIndexOf('.') + 1) : fqn;
|
return fqn.contains(".") ? fqn.substring(fqn.lastIndexOf('.') + 1) : fqn;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -3,6 +3,7 @@ package click.kamil.springstatemachineexporter.analysis.enricher.matching;
|
|||||||
import click.kamil.springstatemachineexporter.analysis.model.TriggerPoint;
|
import click.kamil.springstatemachineexporter.analysis.model.TriggerPoint;
|
||||||
import click.kamil.springstatemachineexporter.model.Event;
|
import click.kamil.springstatemachineexporter.model.Event;
|
||||||
import org.junit.jupiter.api.Test;
|
import org.junit.jupiter.api.Test;
|
||||||
|
import org.junit.jupiter.api.io.TempDir;
|
||||||
|
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
|
|
||||||
@@ -337,4 +338,27 @@ class StrictFqnMatchingEngineTest {
|
|||||||
|
|
||||||
assertThat(engine.matches(smEvent, triggerPoint)).isFalse();
|
assertThat(engine.matches(smEvent, triggerPoint)).isFalse();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void shouldNotMatchImportStyleEnumWhenSimpleNameIsAmbiguous(@TempDir java.nio.file.Path tempDir) throws Exception {
|
||||||
|
java.nio.file.Files.createDirectories(tempDir.resolve("a"));
|
||||||
|
java.nio.file.Files.createDirectories(tempDir.resolve("b"));
|
||||||
|
java.nio.file.Files.writeString(tempDir.resolve("a/OrderEvent.java"),
|
||||||
|
"package a; public enum OrderEvent { PAY }");
|
||||||
|
java.nio.file.Files.writeString(tempDir.resolve("b/OrderEvent.java"),
|
||||||
|
"package b; public enum OrderEvent { PAY }");
|
||||||
|
|
||||||
|
click.kamil.springstatemachineexporter.ast.common.CodebaseContext context =
|
||||||
|
new click.kamil.springstatemachineexporter.ast.common.CodebaseContext();
|
||||||
|
context.scan(tempDir);
|
||||||
|
|
||||||
|
StrictFqnMatchingEngine engine = new StrictFqnMatchingEngine(context);
|
||||||
|
Event smEvent = Event.of("PAY", "a.OrderEvent.PAY");
|
||||||
|
TriggerPoint triggerPoint = TriggerPoint.builder()
|
||||||
|
.event("OrderEvent.PAY")
|
||||||
|
.polymorphicEvents(List.of("OrderEvent.PAY"))
|
||||||
|
.build();
|
||||||
|
|
||||||
|
assertThat(engine.matches(smEvent, triggerPoint)).isFalse();
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user