Fail closed on ambiguous polymorphic widening to prevent over-linking transitions.
Tighten linker, matcher, and canonicalizer guards for null sourceState inference, dynamic getter wildcards, predicate ceiling, and interface accessor ENUM_SET widen; update goldens for ambiguous internal triggers. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -14,7 +14,11 @@ import click.kamil.springstatemachineexporter.model.State;
|
||||
import click.kamil.springstatemachineexporter.model.Transition;
|
||||
|
||||
import java.util.ArrayList;
|
||||
import java.util.LinkedHashMap;
|
||||
import java.util.LinkedHashSet;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.Set;
|
||||
|
||||
import click.kamil.springstatemachineexporter.analysis.enricher.matching.EventMatchingEngine;
|
||||
import click.kamil.springstatemachineexporter.analysis.enricher.matching.StrictFqnMatchingEngine;
|
||||
@@ -61,9 +65,50 @@ public class TransitionLinkerEnricher implements AnalysisEnricher {
|
||||
}
|
||||
|
||||
String triggerSource = tp.getSourceState() != null ? simplify(tp.getSourceState()) : null;
|
||||
if (shouldFailClosedOnAmbiguousCallGraphWiden(tp)) {
|
||||
updatedChains.add(chain.toBuilder()
|
||||
.triggerPoint(tp)
|
||||
.matchedTransitions(null)
|
||||
.build());
|
||||
continue;
|
||||
}
|
||||
|
||||
boolean ambiguousSource = false;
|
||||
|
||||
if (triggerSource == null) {
|
||||
Map<String, Set<String>> sourcesByEvent = new LinkedHashMap<>();
|
||||
Map<String, String> canonicalSourceBySimplified = new LinkedHashMap<>();
|
||||
for (Transition t : stateMachineTransitions) {
|
||||
if (t.getEvent() != null && matchingEngine.matches(t.getEvent(), tp)
|
||||
&& isRoutedToCorrectMachine(chain, result.getName(), context)
|
||||
&& isConstraintCompatible(tp.getConstraint(), result.getName())) {
|
||||
String smEventForLink = canonicalEvent(t.getEvent());
|
||||
for (State smSourceState : t.getSourceStates()) {
|
||||
String smSourceForLink = canonicalState(smSourceState);
|
||||
String smSource = simplify(smSourceForLink);
|
||||
sourcesByEvent.computeIfAbsent(smEventForLink, ignored -> new LinkedHashSet<>())
|
||||
.add(smSource);
|
||||
canonicalSourceBySimplified.putIfAbsent(smSource, smSourceForLink);
|
||||
}
|
||||
}
|
||||
}
|
||||
if (sourcesByEvent.values().stream().anyMatch(sources -> sources.size() > 1)) {
|
||||
ambiguousSource = true;
|
||||
} else {
|
||||
Set<String> allDistinctSources = new LinkedHashSet<>();
|
||||
sourcesByEvent.values().forEach(allDistinctSources::addAll);
|
||||
if (allDistinctSources.size() == 1) {
|
||||
String inferredSimplifiedSource = allDistinctSources.iterator().next();
|
||||
String inferredCanonicalSource = canonicalSourceBySimplified.get(inferredSimplifiedSource);
|
||||
tp = tp.toBuilder().sourceState(inferredCanonicalSource).build();
|
||||
triggerSource = inferredSimplifiedSource;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
List<MatchedTransition> matched = new ArrayList<>();
|
||||
|
||||
if (!ambiguousSource) {
|
||||
for (Transition t : stateMachineTransitions) {
|
||||
if (t.getEvent() != null && matchingEngine.matches(t.getEvent(), tp)) {
|
||||
String smEventForLink = canonicalEvent(t.getEvent());
|
||||
@@ -99,6 +144,11 @@ public class TransitionLinkerEnricher implements AnalysisEnricher {
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (ambiguousSource) {
|
||||
tp = tp.toBuilder().ambiguous(true).build();
|
||||
}
|
||||
|
||||
if (!matched.isEmpty()) {
|
||||
CallChain newChain = chain.toBuilder()
|
||||
@@ -176,6 +226,23 @@ public class TransitionLinkerEnricher implements AnalysisEnricher {
|
||||
return event.fullIdentifier() != null ? event.fullIdentifier() : event.rawName();
|
||||
}
|
||||
|
||||
private boolean shouldFailClosedOnAmbiguousCallGraphWiden(TriggerPoint tp) {
|
||||
if (tp == null || tp.isExternal() || !tp.isAmbiguous()) {
|
||||
return false;
|
||||
}
|
||||
List<String> poly = tp.getPolymorphicEvents();
|
||||
if (poly == null || poly.size() <= 1) {
|
||||
return false;
|
||||
}
|
||||
String event = tp.getEvent();
|
||||
if (event != null && event.startsWith("ENUM_SET:")) {
|
||||
return true;
|
||||
}
|
||||
return MachineEnumCanonicalizer.isDynamicTriggerExpression(event)
|
||||
&& event != null
|
||||
&& !event.contains("valueOf");
|
||||
}
|
||||
|
||||
private TriggerPoint markRestEndpointExternal(TriggerPoint trigger, CallChain chain) {
|
||||
if (trigger == null || trigger.isExternal() || chain.getEntryPoint() == null) {
|
||||
return trigger;
|
||||
|
||||
@@ -65,6 +65,11 @@ public class StrictFqnMatchingEngine implements EventMatchingEngine {
|
||||
if (triggerPoint.getEventTypeFqn() != null && !isStringTypeOrPrimitive(triggerPoint.getEventTypeFqn())) {
|
||||
return false;
|
||||
}
|
||||
// Getter/method-call expressions (e.g. richEvent.getId(), getType()) without resolved
|
||||
// polymorphic events must not wildcard-match bare or enum transition events.
|
||||
if (rawTriggerEvent.contains(".") || rawTriggerEvent.endsWith("()")) {
|
||||
return false;
|
||||
}
|
||||
if (triggerPoint.getEventTypeFqn() != null && isStringTypeOrPrimitive(triggerPoint.getEventTypeFqn())) {
|
||||
return !smEventRaw.contains(".");
|
||||
}
|
||||
|
||||
@@ -128,10 +128,14 @@ public final class MachineEnumCanonicalizer {
|
||||
if (!enumTypesMatch(machineEventTypeFqn, symbolicType)) {
|
||||
continue;
|
||||
}
|
||||
if (!transitionEvents.isEmpty()) {
|
||||
if (transitionEvents.size() == 1) {
|
||||
expanded.addAll(transitionEvents);
|
||||
continue;
|
||||
}
|
||||
if (!transitionEvents.isEmpty()) {
|
||||
// Multiple configured transitions: do not widen symbolic placeholders to all events.
|
||||
continue;
|
||||
}
|
||||
if (context != null) {
|
||||
List<String> enumValues = context.getEnumValues(stripGenerics(machineEventTypeFqn));
|
||||
if (enumValues != null) {
|
||||
@@ -246,7 +250,7 @@ public final class MachineEnumCanonicalizer {
|
||||
if (!narrowed.equals(expanded.getPolymorphicEvents())) {
|
||||
return expanded.toBuilder()
|
||||
.polymorphicEvents(narrowed)
|
||||
.ambiguous(narrowed.size() > 1)
|
||||
.ambiguous(narrowed.size() > 1 && expanded.isAmbiguous())
|
||||
.build();
|
||||
}
|
||||
return expanded;
|
||||
@@ -334,18 +338,15 @@ public final class MachineEnumCanonicalizer {
|
||||
if (transitionEvents.isEmpty()) {
|
||||
return List.of();
|
||||
}
|
||||
List<String> filtered = EnumMemberPredicateEvaluator.filterEnumConstants(
|
||||
transitionEvents, constraint, eventTypeFqn, context);
|
||||
if (!filtered.isEmpty()) {
|
||||
return filtered;
|
||||
}
|
||||
if (EnumMemberPredicateEvaluator.hasEnumMemberPredicates(constraint)) {
|
||||
if (EnumMemberPredicateEvaluator.hasResolvablePredicateMethods(constraint, eventTypeFqn, context)) {
|
||||
return List.of();
|
||||
List<String> filtered = EnumMemberPredicateEvaluator.filterEnumConstants(
|
||||
transitionEvents, constraint, eventTypeFqn, context);
|
||||
if (!filtered.isEmpty()) {
|
||||
return filtered;
|
||||
}
|
||||
return transitionEvents;
|
||||
return List.of();
|
||||
}
|
||||
return transitionEvents;
|
||||
return transitionEvents.size() == 1 ? transitionEvents : List.of();
|
||||
}
|
||||
|
||||
private static List<String> narrowPolymorphicCandidates(
|
||||
@@ -367,11 +368,8 @@ public final class MachineEnumCanonicalizer {
|
||||
transitionEvents, constraint, eventTypeFqn, context);
|
||||
if (!filtered.isEmpty()) {
|
||||
result = filtered;
|
||||
} else if (EnumMemberPredicateEvaluator.hasResolvablePredicateMethods(
|
||||
constraint, eventTypeFqn, context)) {
|
||||
result = List.of();
|
||||
} else {
|
||||
result = transitionEvents;
|
||||
result = List.of();
|
||||
}
|
||||
} else {
|
||||
result = List.of();
|
||||
|
||||
@@ -390,21 +390,7 @@ public abstract class AbstractCallGraphEngine implements CallGraphEngine {
|
||||
if (debug) {
|
||||
log.debug("Early return 2: getterEvents = {}", getterEvents);
|
||||
}
|
||||
return TriggerPoint.builder()
|
||||
.event(resolvedValue)
|
||||
.className(tp.getClassName())
|
||||
.methodName(tp.getMethodName())
|
||||
.sourceFile(tp.getSourceFile())
|
||||
.sourceModule(tp.getSourceModule())
|
||||
.stateMachineId(tp.getStateMachineId())
|
||||
.sourceState(tp.getSourceState())
|
||||
.lineNumber(tp.getLineNumber())
|
||||
.polymorphicEvents(getterEvents)
|
||||
.constraint(tp.getConstraint())
|
||||
.stateTypeFqn(tp.getStateTypeFqn())
|
||||
.eventTypeFqn(tp.getEventTypeFqn())
|
||||
.external(tp.isExternal())
|
||||
.build();
|
||||
return buildTriggerPointWithPolymorphicEvents(tp, resolvedValue, getterEvents);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -443,6 +429,7 @@ public abstract class AbstractCallGraphEngine implements CallGraphEngine {
|
||||
.stateTypeFqn(tp.getStateTypeFqn())
|
||||
.eventTypeFqn(tp.getEventTypeFqn())
|
||||
.external(tp.isExternal())
|
||||
.ambiguous(polymorphicEvents.size() > 1)
|
||||
.build();
|
||||
}
|
||||
|
||||
@@ -924,6 +911,9 @@ public abstract class AbstractCallGraphEngine implements CallGraphEngine {
|
||||
}
|
||||
}
|
||||
}
|
||||
if (polymorphicEvents.size() > 1) {
|
||||
isAmbiguous = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -1079,6 +1069,7 @@ public abstract class AbstractCallGraphEngine implements CallGraphEngine {
|
||||
.stateTypeFqn(tp.getStateTypeFqn())
|
||||
.eventTypeFqn(tp.getEventTypeFqn())
|
||||
.external(tp.isExternal())
|
||||
.ambiguous(isAmbiguous || polymorphicEvents.size() > 1)
|
||||
.build();
|
||||
}
|
||||
|
||||
@@ -1633,6 +1624,8 @@ public abstract class AbstractCallGraphEngine implements CallGraphEngine {
|
||||
* and, when possible, a {@code new ...()} instance from the getter body.
|
||||
*/
|
||||
private TriggerPoint buildTriggerPointWithPolymorphicEvents(TriggerPoint tp, String resolvedValue, List<String> polymorphicEvents) {
|
||||
boolean ambiguous = tp.isAmbiguous()
|
||||
|| (polymorphicEvents != null && polymorphicEvents.size() > 1);
|
||||
return TriggerPoint.builder()
|
||||
.event(resolvedValue)
|
||||
.className(tp.getClassName())
|
||||
@@ -1647,6 +1640,7 @@ public abstract class AbstractCallGraphEngine implements CallGraphEngine {
|
||||
.stateTypeFqn(tp.getStateTypeFqn())
|
||||
.eventTypeFqn(tp.getEventTypeFqn())
|
||||
.external(tp.isExternal())
|
||||
.ambiguous(ambiguous)
|
||||
.build();
|
||||
}
|
||||
|
||||
|
||||
@@ -419,6 +419,9 @@ public final class AnalysisCanonicalFormValidator {
|
||||
if (LifecycleTriggerMarkers.isLifecycle(trigger.getEvent())) {
|
||||
return;
|
||||
}
|
||||
if (trigger.isAmbiguous() && !trigger.isExternal()) {
|
||||
return;
|
||||
}
|
||||
if (!MachineEnumCanonicalizer.hasOnlyConcretePolymorphicEvents(trigger.getPolymorphicEvents())) {
|
||||
return;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user