Keep :: and ?. through decompile instead of collapsing both to a dot

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<CFExpression> expressions;
-
+	/**
+	 * The operator written before a member, keyed by that member's character offset in the source.
+	 * Only the non-default operators are recorded -- <code>::</code> and <code>?.</code> -- 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<Integer, String> memberOperators = new HashMap<Integer, String>();
+
 	public CFFullVarExpression(Token _t, CFExpression _main) {
 		super(_t);
 		// token = _t;
@@ -61,17 +73,40 @@ 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));
 		}
 		return sb.toString();
 	}
 
+	/**
+	 * Records that <code>_member</code> was written after <code>_operator</code> rather than a dot.
+	 * Called for <code>::</code> and <code>?.</code> 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: <code>::</code>, <code>?.</code> 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<CFExpression> getExpressions() {
 		return expressions;
 	}

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,13 +311,84 @@ 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);
 		// }
 		return retval;
 	}
 
+	/**
+	 * Notes which members were reached with <code>::</code> or <code>?.</code> 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()));

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 )         {
 
         };
 

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;