468396 - Some of the position information is incorrectly set on the AST nodes.

Status: RESOLVED INACTIVE

(Rhino Graveyard :: Core, defect)

Description

[Ryan Gustafson]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.0.4) Gecko/2008102920 Firefox/3.0.4
Build Identifier: Trunk circa 12/07/2008

The Parser produces incorrect length information for the SwitchCase and VariableInitializer AST nodes.

I am submitting both the changes, and a "verifier" which should likely be included in some form into the Rhino test cases.  The verifier merely visits the AST recording parent/child relationships, and the revisits, checking that certain constraints are met between the start and end positions between the parent and child, and the child and siblings.

This is problematic when some interactive GUI wishes to highlight a section of code based upon this AST information.


Reproducible: Always

Steps to Reproduce:
1.
2.
3.

Comment 1

[Ryan Gustafson]

Attachment: Parser patch to fix positions

Comment 2

[Ryan Gustafson]

Below is a NodeVisitor which can be used to verify positions.  If Rhino was my project, I would incorporate it into the common path of every JUnit test against any .js code.

I've not spent the time yet to figure out how Rhino handles JUnit tests, I merely kludged this class into the stuff I am working on (e.g. new PositionVerifier().verify(astNode);)

I wouldn't be surprised if the verifier could validate even more constraints about the expected structure of the AST than it currently does.

Code follows...........................


    public class PositionVerifier implements NodeVisitor {
	private final Map<AstNode, List<AstNode>> nodeToChildren = new HashMap<AstNode, List<AstNode>>();
	private boolean verify = false;

	public boolean visit(final AstNode node) {
	    if (!verify) {
		// Register this node as a child of it's parent.
		final AstNode parent = node.getParent();
		List<AstNode> children = nodeToChildren.get(parent);
		if (children == null) {
		    children = new ArrayList<AstNode>(1);
		    nodeToChildren.put(parent, children);
		}
		children.add(node);
	    } else {
		final int parentStartPosition = node.getAbsolutePosition();
		final int parentEndPosition = parentStartPosition + node.getLength();
		verify(node, parentStartPosition, parentEndPosition);

		// Verify position of children...
		final List<AstNode> children = nodeToChildren.get(node);
		if (children != null) {
		    int previousChildEndPosition = Integer.MIN_VALUE;
		    for (AstNode child : children) {
			// Child must be in range of the parent
			final int childStartPosition = child.getAbsolutePosition();
			final int childEndPosition = childStartPosition + child.getLength();
			verify(node, childStartPosition, childEndPosition);
			if (!(childStartPosition >= parentStartPosition && childStartPosition <= parentEndPosition)) {
			    throw new RuntimeException("Child start position(" + childStartPosition
				    + ") must be between parent [" + parentStartPosition + ", " + parentEndPosition
				    + "].");
			}
			if (!(childEndPosition >= parentStartPosition && childEndPosition <= parentEndPosition)) {
			    throw new RuntimeException("Child end position(" + childEndPosition
				    + ") must be between parent [" + parentStartPosition + ", " + parentEndPosition
				    + "].");
			}
			if (previousChildEndPosition != Integer.MIN_VALUE) {
			    if (childStartPosition < previousChildEndPosition) {
				throw new RuntimeException("Child start position(" + childStartPosition
					+ ") must after previous child's end position(" + previousChildEndPosition
					+ ").");
			    }
			}
			previousChildEndPosition = childEndPosition;
		    }
		}
	    }
	    return true;
	}

	public void verify(final AstNode root) {
	    root.visit(this);
	    this.verify = true;
	    root.visit(this);
	}

	private void verify(final AstNode node, final int start, final int end) {
	    if (end <= start) {
		throw new RuntimeException("Node(" + node.toString() + "/" + node.getClass().getSimpleName()
			+ ") end position (" + end + ") must be > start position (" + start + ").");
	    }
	}
    }

Comment 3

[Sylvestre Ledru [:Sylvestre]]

Closing. Bug management is now done here:
https://github.com/mozilla/rhino