From 1362b05c1c4ada2fc5f57beef1b7e995661ac39c Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 13 Aug 2026 21:43:01 +0000 Subject: [PATCH] Keep :: and ?. through decompile instead of collapsing both to a dot memberExpression accepts three separators -- DOT, DOUBLECOLUMN and nullSafeOperator -- and none of them reached the AST. DOUBLECOLUMN appeared nowhere in the Java source at all. CFFullVarExpression hardcoded "." between members, so: a::b -> a.b a?.b -> a.b a?.b() -> a.b() ?. is the one that matters. a?.b yields null where a.b throws, so the round trip changed what the code does, and safe navigation is far more common in real code than static references. :: turns a static member reference into an ordinary property access on a variable that may not exist. An existing fixture was pinning the wrong output. acf2016/safenav.cfc parses if(xyz?.bar) and its recorded decompile read if(xyz.bar) -- the operator was being dropped and the expectation had been recorded from the broken result. That section is corrected here by hand, since AutoReplaceFailedTestResults does not cover decompile. The operator is recorded on CFFullVarExpression keyed by the member's character offset, not by its index. Members are gathered through aggregateResult, where one source construct does not reliably yield one element -- a[1].b puts three expressions in the list with a single dot between them -- so index alignment would drift. Source offsets do not. Only the two non-default operators are stored; an absent entry still means a dot, so nothing changes for ordinary member access. Decompile's existing logic for whether to emit a separator is untouched; only which separator it writes. Round trips verified across chains, calls and array members: a[1]?.b, a?.b?.c, a::b::c, a.b?.c.d, a?.b[1].c, a?.b().c. 325 tests, ./gradlew build, differential harness unchanged at 1 with nothing newly broken, CFLint's 675 against a clean build. Both fixtures fail with cfml.parsing/src/main stashed. --- .../parsing/cfscript/CFFullVarExpression.java | 41 ++- .../cfscript/walker/CFExpressionVisitor.java | 75 ++++- .../cfml/tests/acf2016/safenav.expected.txt | 2 +- .../tests/expressions/member_operators.cfc | 9 + .../expressions/member_operators.expected.txt | 304 ++++++++++++++++++ 5 files changed, 426 insertions(+), 5 deletions(-) create mode 100644 cfml.parsing/src/test/resources/cfml/tests/expressions/member_operators.cfc create mode 100644 cfml.parsing/src/test/resources/cfml/tests/expressions/member_operators.expected.txt diff --git a/cfml.parsing/src/main/java/cfml/parsing/cfscript/CFFullVarExpression.java b/cfml.parsing/src/main/java/cfml/parsing/cfscript/CFFullVarExpression.java index 89651c01..b79f8a4f 100644 --- a/cfml.parsing/src/main/java/cfml/parsing/cfscript/CFFullVarExpression.java +++ b/cfml.parsing/src/main/java/cfml/parsing/cfscript/CFFullVarExpression.java @@ -1,7 +1,9 @@ package cfml.parsing.cfscript; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; import org.antlr.v4.runtime.Token; @@ -15,7 +17,17 @@ public class CFFullVarExpression extends CFIdentifier { // private Token token; private List expressions; - + /** + * The operator written before a member, keyed by that member's character offset in the source. + * Only the non-default operators are recorded -- :: and ?. -- so an + * absent entry means an ordinary dot. + * + * Keyed by position rather than by index because the members are gathered through + * aggregateResult, where one source construct does not always yield one element: `a[1].b` puts + * three expressions in the list with a single dot between them. Source offsets survive that. + */ + private Map memberOperators = new HashMap(); + public CFFullVarExpression(Token _t, CFExpression _main) { super(_t); // token = _t; @@ -61,10 +73,10 @@ public String Decompile(int indent) { && expression.getToken().getType() == CFSCRIPTLexer.LEFTBRACKET) { // Array notation [] } else if (expression.getType() == CFExpression.IDENTIFIER || expression.getType() == CFExpression.LITERAL) { - sb.append("."); + sb.append(memberOperator(expression)); } else if (expression instanceof CFFunctionExpression && ((CFFunctionExpression) expression).getIdentifier() != null) { - sb.append("."); + sb.append(memberOperator(expression)); } } sb.append(expression.Decompile(0)); @@ -72,6 +84,29 @@ public String Decompile(int indent) { return sb.toString(); } + /** + * Records that _member was written after _operator rather than a dot. + * Called for :: and ?. only; anything else keeps the default. + */ + public void setMemberOperator(CFExpression _member, String _operator) { + if (_member != null && _member.getToken() != null && _operator != null) { + memberOperators.put(_member.getToken().getStartIndex(), _operator); + } + } + + /** The operator to write before this member: ::, ?. or a dot. */ + public String getMemberOperator(CFExpression _member) { + return memberOperator(_member); + } + + private String memberOperator(CFExpression _member) { + if (_member == null || _member.getToken() == null) { + return "."; + } + String operator = memberOperators.get(_member.getToken().getStartIndex()); + return operator == null ? "." : operator; + } + public List getExpressions() { return expressions; } diff --git a/cfml.parsing/src/main/java/cfml/parsing/cfscript/walker/CFExpressionVisitor.java b/cfml.parsing/src/main/java/cfml/parsing/cfscript/walker/CFExpressionVisitor.java index 20d95203..49f11b5f 100644 --- a/cfml.parsing/src/main/java/cfml/parsing/cfscript/walker/CFExpressionVisitor.java +++ b/cfml.parsing/src/main/java/cfml/parsing/cfscript/walker/CFExpressionVisitor.java @@ -2,6 +2,7 @@ import java.util.Stack; +import org.antlr.v4.runtime.ParserRuleContext; import org.antlr.v4.runtime.Token; import org.antlr.v4.runtime.tree.ParseTree; import org.antlr.v4.runtime.tree.TerminalNode; @@ -37,7 +38,8 @@ import cfml.CFSCRIPTParser.LocalAssignmentExpressionContext; import cfml.CFSCRIPTParser.MemberExpressionContext; import cfml.CFSCRIPTParser.MultipartIdentifierContext; -import cfml.CFSCRIPTParser.NewComponentExpressionContext; +import cfml.CFSCRIPTParser.NewComponentExpressionContext; +import cfml.CFSCRIPTParser.NullSafeOperatorContext; import cfml.CFSCRIPTParser.OtherIdentifiersContext; import cfml.CFSCRIPTParser.ParameterAttributeContext; import cfml.CFSCRIPTParser.ParameterContext; @@ -309,6 +311,7 @@ public CFExpression visitMemberExpression(MemberExpressionContext ctx) { aggregator.push(fullVarExpression); CFExpression retval = visitChildren(ctx); aggregator.pop(); + recordMemberOperators(ctx, retval); // negative if minus present // if (ctx.MINUS() != null) { // retval = new CFUnaryExpression(ctx.MINUS().getSymbol(), retval); @@ -316,6 +319,76 @@ public CFExpression visitMemberExpression(MemberExpressionContext ctx) { return retval; } + /** + * Notes which members were reached with :: or ?. rather than a dot. + * + * Both used to be discarded: the operator is a separator in memberExpression and never became + * part of the AST, so `a::b` and `a?.b` both decompiled to `a.b`. That is not cosmetic -- + * `a?.b` yields null where `a.b` throws, so the round trip changed what the code does. + * + * The association is by source offset rather than by position in the child list, because the + * members are gathered through aggregateResult and one child does not always produce one + * element. Only the two non-default operators are recorded. + */ + private void recordMemberOperators(MemberExpressionContext ctx, CFExpression retval) { + if (!(retval instanceof CFFullVarExpression)) { + return; + } + CFFullVarExpression fullVar = (CFFullVarExpression) retval; + String pending = null; + for (int i = 0; i < ctx.getChildCount(); i++) { + ParseTree child = ctx.getChild(i); + String operator = memberOperatorOf(child); + if (operator != null) { + // DOT carries no information, but it still closes off any pending operator. + pending = operator.equals(".") ? null : operator; + continue; + } + if (pending != null) { + markMember(fullVar, child, pending); + pending = null; + } + } + } + + /** The separator this child represents, or null when it is a member rather than a separator. */ + private String memberOperatorOf(ParseTree child) { + if (child instanceof NullSafeOperatorContext) { + return "?."; + } + if (child instanceof TerminalNode) { + int type = ((TerminalNode) child).getSymbol().getType(); + if (type == CFSCRIPTLexer.DOUBLECOLUMN) { + return "::"; + } + if (type == CFSCRIPTLexer.DOT) { + return "."; + } + } + return null; + } + + /** + * Attaches the operator to whichever member starts at this child's first token. Matching on the + * token keeps this correct when the child produced several expressions, or none. + */ + private void markMember(CFFullVarExpression fullVar, ParseTree child, String operator) { + if (!(child instanceof ParserRuleContext)) { + return; + } + Token start = ((ParserRuleContext) child).getStart(); + if (start == null) { + return; + } + for (CFExpression expression : fullVar.getExpressions()) { + if (expression != null && expression.getToken() != null + && expression.getToken().getStartIndex() == start.getStartIndex()) { + fullVar.setMemberOperator(expression, operator); + return; + } + } + } + @Override public CFExpression visitInnerExpression(InnerExpressionContext ctx) { return new CFNestedExpression(ctx.POUND_SIGN(0).getSymbol(), visit(ctx.anExpression())); diff --git a/cfml.parsing/src/test/resources/cfml/tests/acf2016/safenav.expected.txt b/cfml.parsing/src/test/resources/cfml/tests/acf2016/safenav.expected.txt index c9099257..bb57dae1 100644 --- a/cfml.parsing/src/test/resources/cfml/tests/acf2016/safenav.expected.txt +++ b/cfml.parsing/src/test/resources/cfml/tests/acf2016/safenav.expected.txt @@ -85,7 +85,7 @@ Hidden:NEWLINE <> component { public function foo() { var xyz = {}; - if(xyz.bar ) { + if(xyz?.bar ) { }; diff --git a/cfml.parsing/src/test/resources/cfml/tests/expressions/member_operators.cfc b/cfml.parsing/src/test/resources/cfml/tests/expressions/member_operators.cfc new file mode 100644 index 00000000..1d1a0005 --- /dev/null +++ b/cfml.parsing/src/test/resources/cfml/tests/expressions/member_operators.cfc @@ -0,0 +1,9 @@ +staticRef = Some::myVar; +staticCall = Some::myFunc(); +staticChain = Some::a::b; +nullSafe = obj?.prop; +nullSafeCall = obj?.method(); +nullSafeChain = obj?.a?.b; +mixed = obj.a?.b.c; +withArray = obj?.list[1].name; +plain = obj.a.b; diff --git a/cfml.parsing/src/test/resources/cfml/tests/expressions/member_operators.expected.txt b/cfml.parsing/src/test/resources/cfml/tests/expressions/member_operators.expected.txt new file mode 100644 index 00000000..4b1a272a --- /dev/null +++ b/cfml.parsing/src/test/resources/cfml/tests/expressions/member_operators.expected.txt @@ -0,0 +1,304 @@ +/*===TOKENS===*/ +IDENTIFIER +'=' <=> +IDENTIFIER +'::' <::> +IDENTIFIER +';' <;> +Hidden:NEWLINE <> +IDENTIFIER +'=' <=> +IDENTIFIER +'::' <::> +IDENTIFIER +'(' <(> +')' <)> +';' <;> +Hidden:NEWLINE <> +IDENTIFIER +'=' <=> +IDENTIFIER +'::' <::> +IDENTIFIER +'::' <::> +IDENTIFIER +';' <;> +Hidden:NEWLINE <> +IDENTIFIER +'=' <=> +IDENTIFIER +'?' +'.' <.> +IDENTIFIER +';' <;> +Hidden:NEWLINE <> +IDENTIFIER +'=' <=> +IDENTIFIER +'?' +'.' <.> +IDENTIFIER +'(' <(> +')' <)> +';' <;> +Hidden:NEWLINE <> +IDENTIFIER +'=' <=> +IDENTIFIER +'?' +'.' <.> +IDENTIFIER +'?' +'.' <.> +IDENTIFIER +';' <;> +Hidden:NEWLINE <> +IDENTIFIER +'=' <=> +IDENTIFIER +'.' <.> +IDENTIFIER +'?' +'.' <.> +IDENTIFIER +'.' <.> +IDENTIFIER +';' <;> +Hidden:NEWLINE <> +IDENTIFIER +'=' <=> +IDENTIFIER +'?' +'.' <.> +IDENTIFIER +'[' <[> +INTEGER_LITERAL <1> +']' <]> +'.' <.> +IDENTIFIER +';' <;> +Hidden:NEWLINE <> +IDENTIFIER +'=' <=> +IDENTIFIER +'.' <.> +IDENTIFIER +'.' <.> +IDENTIFIER +';' <;> +Hidden:NEWLINE <> +/*===TREE===*/ +(scriptBlock + (element + (statement + (assignmentExpression + (startExpression + (baseExpression (unaryExpression (memberExpression (identifier staticRef)))) + ) + = + (startExpression + (baseExpression + (unaryExpression (memberExpression (identifier Some) :: (identifier myVar))) + ) + ) + ) + (endOfStatement ;) + ) + ) + (element + (statement + (assignmentExpression + (startExpression + (baseExpression (unaryExpression (memberExpression (identifier staticCall)))) + ) + = + (startExpression + (baseExpression + (unaryExpression + (memberExpression + (identifier Some) + :: + (qualifiedFunctionCall (identifier myFunc) ( argumentList )) + ) + ) + ) + ) + ) + (endOfStatement ;) + ) + ) + (element + (statement + (assignmentExpression + (startExpression + (baseExpression (unaryExpression (memberExpression (identifier staticChain)))) + ) + = + (startExpression + (baseExpression + (unaryExpression + (memberExpression (identifier Some) :: (identifier a) :: (identifier b)) + ) + ) + ) + ) + (endOfStatement ;) + ) + ) + (element + (statement + (assignmentExpression + (startExpression + (baseExpression (unaryExpression (memberExpression (identifier nullSafe)))) + ) + = + (startExpression + (baseExpression + (unaryExpression + (memberExpression (identifier obj) (nullSafeOperator ? .) (identifier prop)) + ) + ) + ) + ) + (endOfStatement ;) + ) + ) + (element + (statement + (assignmentExpression + (startExpression + (baseExpression (unaryExpression (memberExpression (identifier nullSafeCall)))) + ) + = + (startExpression + (baseExpression + (unaryExpression + (memberExpression + (identifier obj) + (nullSafeOperator ? .) + (qualifiedFunctionCall (identifier method) ( argumentList )) + ) + ) + ) + ) + ) + (endOfStatement ;) + ) + ) + (element + (statement + (assignmentExpression + (startExpression + (baseExpression + (unaryExpression (memberExpression (identifier nullSafeChain))) + ) + ) + = + (startExpression + (baseExpression + (unaryExpression + (memberExpression + (identifier obj) + (nullSafeOperator ? .) + (identifier a) + (nullSafeOperator ? .) + (identifier b) + ) + ) + ) + ) + ) + (endOfStatement ;) + ) + ) + (element + (statement + (assignmentExpression + (startExpression + (baseExpression (unaryExpression (memberExpression (identifier mixed)))) + ) + = + (startExpression + (baseExpression + (unaryExpression + (memberExpression + (identifier obj) + . + (identifier a) + (nullSafeOperator ? .) + (identifier b) + . + (identifier c) + ) + ) + ) + ) + ) + (endOfStatement ;) + ) + ) + (element + (statement + (assignmentExpression + (startExpression + (baseExpression (unaryExpression (memberExpression (identifier withArray)))) + ) + = + (startExpression + (baseExpression + (unaryExpression + (memberExpression + (identifier obj) + (nullSafeOperator ? .) + (identifier list) + (arrayMemberExpression + [ + (startExpression + (baseExpression (unaryExpression (primaryExpression (literalExpression 1)))) + ) + ] + ) + . + (identifier name) + ) + ) + ) + ) + ) + (endOfStatement ;) + ) + ) + (element + (statement + (assignmentExpression + (startExpression + (baseExpression (unaryExpression (memberExpression (identifier plain)))) + ) + = + (startExpression + (baseExpression + (unaryExpression + (memberExpression (identifier obj) . (identifier a) . (identifier b)) + ) + ) + ) + ) + (endOfStatement ;) + ) + ) +) +/*======*/ +/*===DECOMPILE===*/ +{ +staticRef = Some::myVar; +staticCall = Some::myFunc(); +staticChain = Some::a::b; +nullSafe = obj?.prop; +nullSafeCall = obj?.method(); +nullSafeChain = obj?.a?.b; +mixed = obj.a?.b.c; +withArray = obj?.list[1].name; +plain = obj.a.b; + +} +/*======*/ \ No newline at end of file