diff --git a/pmd/etc/changelog.txt b/pmd/etc/changelog.txt index bac7e7c936..33e2d570b0 100644 --- a/pmd/etc/changelog.txt +++ b/pmd/etc/changelog.txt @@ -1,5 +1,6 @@ ???? ??, 2012 - 5.0.2: +Fixed bug 1043: node.getEndLine() always returns 0 (ECMAscript) Fixed bug 1044: Unknown option: -excludemarker Fixed bug 1047: False Positive in 'for' loops for LocalVariableCouldBeFinal in 5.0.1 Fixed bug 1048: CommentContent Rule, String Index out of range Exception diff --git a/pmd/pom.xml b/pmd/pom.xml index ec34c2b603..5797b0dcc6 100644 --- a/pmd/pom.xml +++ b/pmd/pom.xml @@ -605,6 +605,11 @@ javacc ${javacc.version} + + commons-io + commons-io + 2.2 + diff --git a/pmd/src/main/java/net/sourceforge/pmd/lang/ast/AbstractNode.java b/pmd/src/main/java/net/sourceforge/pmd/lang/ast/AbstractNode.java index 196d92be01..bbdf207026 100644 --- a/pmd/src/main/java/net/sourceforge/pmd/lang/ast/AbstractNode.java +++ b/pmd/src/main/java/net/sourceforge/pmd/lang/ast/AbstractNode.java @@ -119,7 +119,7 @@ public abstract class AbstractNode implements Node { if (children != null && children.length > 0) { return children[0].getBeginColumn(); } else { - throw new RuntimeException("Unable to determine begining line of Node."); + throw new RuntimeException("Unable to determine beginning line of Node."); } } } diff --git a/pmd/src/main/java/net/sourceforge/pmd/lang/ecmascript/ast/AbstractEcmascriptNode.java b/pmd/src/main/java/net/sourceforge/pmd/lang/ecmascript/ast/AbstractEcmascriptNode.java index cc4c552451..3c7d2f281e 100644 --- a/pmd/src/main/java/net/sourceforge/pmd/lang/ecmascript/ast/AbstractEcmascriptNode.java +++ b/pmd/src/main/java/net/sourceforge/pmd/lang/ecmascript/ast/AbstractEcmascriptNode.java @@ -7,18 +7,27 @@ import net.sourceforge.pmd.lang.ast.AbstractNode; import org.mozilla.javascript.ast.AstNode; -public abstract class AbstractEcmascriptNode extends AbstractNode implements EcmascriptNode { +public abstract class AbstractEcmascriptNode extends AbstractNode implements EcmascriptNode { protected final T node; public AbstractEcmascriptNode(T node) { super(node.getType()); this.node = node; - this.beginLine = node.getLineno() + 1; - this.beginLine = node.getLineno() + 1; - // TODO Implement positions, or figure out how to do begin/end lines/column - //this.beginPosition = node.getAbsolutePosition(); - //this.endPosition = this.beginPosition + node.getLength(); + } + + /* package private */ + void calculateLineNumbers(SourceCodePositioner positioner) { + int startOffset = node.getAbsolutePosition(); + int endOffset = startOffset + node.getLength(); + + this.beginLine = positioner.lineNumberFromOffset(startOffset); + this.beginColumn = positioner.columnFromOffset(startOffset); + this.endLine = positioner.lineNumberFromOffset(endOffset); + this.endColumn = positioner.columnFromOffset(endOffset) - 1; // end column is inclusive + if (this.endColumn < 0) { + this.endColumn = 0; + } } /** @@ -34,7 +43,9 @@ public abstract class AbstractEcmascriptNode extends Abstract public Object childrenAccept(EcmascriptParserVisitor visitor, Object data) { if (children != null) { for (int i = 0; i < children.length; ++i) { - ((EcmascriptNode) children[i]).jjtAccept(visitor, data); + @SuppressWarnings("unchecked") // we know that the children here are all EcmascriptNodes + EcmascriptNode ecmascriptNode = (EcmascriptNode) children[i]; + ecmascriptNode.jjtAccept(visitor, data); } } return data; @@ -52,11 +63,6 @@ public abstract class AbstractEcmascriptNode extends Abstract return node.hasSideEffects(); } - @Override - public int getBeginColumn() { - return -1; - } - @Override public String toString() { return node.shortName(); diff --git a/pmd/src/main/java/net/sourceforge/pmd/lang/ecmascript/ast/EcmascriptParser.java b/pmd/src/main/java/net/sourceforge/pmd/lang/ecmascript/ast/EcmascriptParser.java index 9160d38b1b..c2d1c6bf1e 100644 --- a/pmd/src/main/java/net/sourceforge/pmd/lang/ecmascript/ast/EcmascriptParser.java +++ b/pmd/src/main/java/net/sourceforge/pmd/lang/ecmascript/ast/EcmascriptParser.java @@ -11,6 +11,7 @@ import java.util.List; import net.sourceforge.pmd.lang.ast.ParseException; import net.sourceforge.pmd.lang.ecmascript.EcmascriptParserOptions; +import org.apache.commons.io.IOUtils; import org.mozilla.javascript.CompilerEnvirons; import org.mozilla.javascript.Parser; import org.mozilla.javascript.ast.AstRoot; @@ -24,7 +25,7 @@ public class EcmascriptParser { this.parserOptions = parserOptions; } - protected AstRoot parseEcmascript(final Reader reader, final List parseProblems) throws ParseException { + protected AstRoot parseEcmascript(final String sourceCode, final List parseProblems) throws ParseException { final CompilerEnvirons compilerEnvirons = new CompilerEnvirons(); compilerEnvirons.setRecordingComments(parserOptions.isRecordingComments()); compilerEnvirons.setRecordingLocalJsDocComments(parserOptions.isRecordingLocalJsDocComments()); @@ -35,23 +36,23 @@ public class EcmascriptParser { // TODO We should do something with Rhino errors... final ErrorCollector errorCollector = new ErrorCollector(); final Parser parser = new Parser(compilerEnvirons, errorCollector); + // TODO Fix hardcode + final String sourceURI = "unknown"; + final int beginLineno = 1; + AstRoot astRoot = parser.parse(sourceCode, sourceURI, beginLineno); + parseProblems.addAll(errorCollector.getErrors()); + return astRoot; + } + + public EcmascriptNode parse(final Reader reader) { try { - // TODO Fix hardcode - final String sourceURI = "unknown"; - // TODO Fix hardcode - final int lineno = 0; - AstRoot astRoot = parser.parse(reader, sourceURI, lineno); - parseProblems.addAll(errorCollector.getErrors()); - return astRoot; - } catch (final IOException e) { + final List parseProblems = new ArrayList(); + final String sourceCode = IOUtils.toString(reader); + final AstRoot astRoot = parseEcmascript(sourceCode, parseProblems); + final EcmascriptTreeBuilder treeBuilder = new EcmascriptTreeBuilder(sourceCode, parseProblems); + return treeBuilder.build(astRoot); + } catch (IOException e) { throw new ParseException(e); } } - - public EcmascriptNode parse(final Reader reader) { - final List parseProblems = new ArrayList(); - final AstRoot astRoot = parseEcmascript(reader, parseProblems); - final EcmascriptTreeBuilder treeBuilder = new EcmascriptTreeBuilder(parseProblems); - return treeBuilder.build(astRoot); - } } diff --git a/pmd/src/main/java/net/sourceforge/pmd/lang/ecmascript/ast/EcmascriptTreeBuilder.java b/pmd/src/main/java/net/sourceforge/pmd/lang/ecmascript/ast/EcmascriptTreeBuilder.java index fd9b64519b..9975a8ee6a 100644 --- a/pmd/src/main/java/net/sourceforge/pmd/lang/ecmascript/ast/EcmascriptTreeBuilder.java +++ b/pmd/src/main/java/net/sourceforge/pmd/lang/ecmascript/ast/EcmascriptTreeBuilder.java @@ -65,9 +65,9 @@ import org.mozilla.javascript.ast.XmlExpression; import org.mozilla.javascript.ast.XmlMemberGet; import org.mozilla.javascript.ast.XmlString; -public class EcmascriptTreeBuilder implements NodeVisitor { +public final class EcmascriptTreeBuilder implements NodeVisitor { - protected static final Map, Constructor> NODE_TYPE_TO_NODE_ADAPTER_TYPE = new HashMap, Constructor>(); + private static final Map, Constructor>> NODE_TYPE_TO_NODE_ADAPTER_TYPE = new HashMap, Constructor>>(); static { register(ArrayComprehension.class, ASTArrayComprehension.class); register(ArrayComprehensionLoop.class, ASTArrayComprehensionLoop.class); @@ -120,7 +120,7 @@ public class EcmascriptTreeBuilder implements NodeVisitor { register(XmlString.class, ASTXmlString.class); } - protected static void register(Class nodeType, Class nodeAdapterType) { + private static void register(Class nodeType, Class> nodeAdapterType) { try { NODE_TYPE_TO_NODE_ADAPTER_TYPE.put(nodeType, nodeAdapterType.getConstructor(nodeType)); } catch (SecurityException e) { @@ -139,13 +139,18 @@ public class EcmascriptTreeBuilder implements NodeVisitor { // The Rhino nodes with children to build. protected Stack parents = new Stack(); - public EcmascriptTreeBuilder(List parseProblems) { + private final SourceCodePositioner sourceCodePositioner; + + public EcmascriptTreeBuilder(String sourceCode, List parseProblems) { + this.sourceCodePositioner = new SourceCodePositioner(sourceCode); this.parseProblems = parseProblems; } - protected EcmascriptNode createNodeAdapter(AstNode node) { + private EcmascriptNode createNodeAdapter(T node) { try { - Constructor constructor = NODE_TYPE_TO_NODE_ADAPTER_TYPE.get(node.getClass()); + @SuppressWarnings("unchecked") // the register function makes sure only EcmascriptNode can be added, + // where T is "T extends AstNode". + Constructor> constructor = (Constructor>) NODE_TYPE_TO_NODE_ADAPTER_TYPE.get(node.getClass()); if (constructor == null) { throw new IllegalArgumentException("There is no Node adapter class registered for the Node class: " + node.getClass()); @@ -160,8 +165,10 @@ public class EcmascriptTreeBuilder implements NodeVisitor { } } - public EcmascriptNode build(AstNode astNode) { - EcmascriptNode node = buildInternal(astNode); + public EcmascriptNode build(T astNode) { + EcmascriptNode node = buildInternal(astNode); + + calculateLineNumbers(node); // Set all the trailing comma nodes for (TrailingCommaNode trailingCommaNode : parseProblemToNode.values()) { @@ -171,9 +178,9 @@ public class EcmascriptTreeBuilder implements NodeVisitor { return node; } - protected EcmascriptNode buildInternal(AstNode astNode) { + private EcmascriptNode buildInternal(T astNode) { // Create a Node - EcmascriptNode node = createNodeAdapter(astNode); + EcmascriptNode node = createNodeAdapter(astNode); // Append to parent Node parent = nodes.isEmpty() ? null : nodes.peek(); @@ -203,7 +210,7 @@ public class EcmascriptTreeBuilder implements NodeVisitor { } } - private void handleParseProblems(EcmascriptNode node) { + private void handleParseProblems(EcmascriptNode node) { if (node instanceof TrailingCommaNode) { TrailingCommaNode trailingCommaNode = (TrailingCommaNode) node; int nodeStart = node.getNode().getAbsolutePosition(); @@ -216,7 +223,7 @@ public class EcmascriptTreeBuilder implements NodeVisitor { if ("Trailing comma is not legal in an ECMA-262 object initializer".equals(parseProblem.getMessage())) { // Report on the shortest code block containing the // problem (i.e. inner most code in nested structures). - EcmascriptNode currentNode = (EcmascriptNode) parseProblemToNode.get(parseProblem); + EcmascriptNode currentNode = (EcmascriptNode) parseProblemToNode.get(parseProblem); if (currentNode == null || node.getNode().getLength() < currentNode.getNode().getLength()) { parseProblemToNode.put(parseProblem, trailingCommaNode); } @@ -225,4 +232,15 @@ public class EcmascriptTreeBuilder implements NodeVisitor { } } } + + private void calculateLineNumbers(EcmascriptNode node) { + EcmascriptParserVisitorAdapter visitor = new EcmascriptParserVisitorAdapter() { + @Override + public Object visit(EcmascriptNode node, Object data) { + ((AbstractEcmascriptNode)node).calculateLineNumbers(sourceCodePositioner); + return super.visit(node, data); // also visit the children + } + }; + node.jjtAccept(visitor, null); + } } diff --git a/pmd/src/main/java/net/sourceforge/pmd/lang/ecmascript/ast/SourceCodePositioner.java b/pmd/src/main/java/net/sourceforge/pmd/lang/ecmascript/ast/SourceCodePositioner.java new file mode 100644 index 0000000000..c4ac16b5a6 --- /dev/null +++ b/pmd/src/main/java/net/sourceforge/pmd/lang/ecmascript/ast/SourceCodePositioner.java @@ -0,0 +1,56 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ +package net.sourceforge.pmd.lang.ecmascript.ast; + +import java.util.Arrays; + +/** + * Calculates from an absolute offset in the source file the line/column coordinate. + * This is needed as Rhino only offers absolute positions for each node. + * + * Idea from: http://code.google.com/p/closure-compiler/source/browse/trunk/src/com/google/javascript/jscomp/SourceFile.java + */ +public class SourceCodePositioner { + + private int[] lineOffsets; + + public SourceCodePositioner(String sourceCode) { + analyzeLineOffsets(sourceCode); + } + + private void analyzeLineOffsets(String sourceCode) { + String[] lines = sourceCode.split("\n"); + + int startOffset = 0; + int lineNumber = 0; + + lineOffsets = new int[lines.length]; + + for (String line : lines) { + lineOffsets[lineNumber] = startOffset; + lineNumber++; + startOffset += line.length() + 1; // +1 for the "\n" character + } + } + + public int lineNumberFromOffset(int offset) { + int search = Arrays.binarySearch(lineOffsets, offset); + int lineNumber; + if (search >= 0) { + lineNumber = search; + } else { + int insertionPoint = search; + insertionPoint += 1; + insertionPoint *= -1; + lineNumber = insertionPoint - 1; // take the insertion point one before + } + return lineNumber + 1; // 1-based line numbers + } + + public int columnFromOffset(int offset) { + int lineNumber = lineNumberFromOffset(offset); + int columnOffset = offset - lineOffsets[lineNumber - 1]; + return columnOffset + 1; // 1-based column offsets + } +} diff --git a/pmd/src/test/java/net/sourceforge/pmd/lang/ecmascript/ast/EcmascriptParserTest.java b/pmd/src/test/java/net/sourceforge/pmd/lang/ecmascript/ast/EcmascriptParserTest.java new file mode 100644 index 0000000000..fae93cabd3 --- /dev/null +++ b/pmd/src/test/java/net/sourceforge/pmd/lang/ecmascript/ast/EcmascriptParserTest.java @@ -0,0 +1,49 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ +package net.sourceforge.pmd.lang.ecmascript.ast; + +import static org.junit.Assert.assertEquals; + +import java.io.Reader; +import java.io.StringReader; + +import net.sourceforge.pmd.PMD; +import net.sourceforge.pmd.lang.ast.Node; +import net.sourceforge.pmd.lang.ecmascript.EcmascriptParserOptions; + +import org.junit.Test; +import org.mozilla.javascript.ast.AstRoot; + +public class EcmascriptParserTest { + + @Test + public void testLineNumbers() { + + EcmascriptParser parser = new EcmascriptParser(new EcmascriptParserOptions()); + Reader sourceCode = new StringReader(SOURCE_CODE); + EcmascriptNode node = parser.parse(sourceCode); + + assertEquals(1, node.getBeginLine()); + assertEquals(1, node.getBeginColumn()); + assertEquals(3, node.getEndLine()); + assertEquals(1, node.getEndColumn()); + + Node child = node.getFirstChildOfType(ASTFunctionNode.class); + assertEquals(1, child.getBeginLine()); + assertEquals(1, child.getBeginColumn()); + assertEquals(3, child.getEndLine()); + assertEquals(1, child.getEndColumn()); + + child = node.getFirstDescendantOfType(ASTFunctionCall.class); + assertEquals(2, child.getBeginLine()); + assertEquals(3, child.getBeginColumn()); + assertEquals(2, child.getEndLine()); + assertEquals(16, child.getEndColumn()); + } + + private static final String SOURCE_CODE = + "function a() {" + PMD.EOL + + " alert('hello');" + PMD.EOL + + "}" + PMD.EOL; +} diff --git a/pmd/src/test/java/net/sourceforge/pmd/lang/ecmascript/ast/SourceCodePositionerTest.java b/pmd/src/test/java/net/sourceforge/pmd/lang/ecmascript/ast/SourceCodePositionerTest.java new file mode 100644 index 0000000000..b86f3d4477 --- /dev/null +++ b/pmd/src/test/java/net/sourceforge/pmd/lang/ecmascript/ast/SourceCodePositionerTest.java @@ -0,0 +1,36 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ +package net.sourceforge.pmd.lang.ecmascript.ast; + +import static org.junit.Assert.*; + +import org.junit.Test; + +public class SourceCodePositionerTest { + + private final String SOURCE_CODE = "abcd\ndefghi\n\njklmn\nopq"; + + @Test + public void testLineNumberFromOffset() { + SourceCodePositioner positioner = new SourceCodePositioner(SOURCE_CODE); + + int offset; + + offset = SOURCE_CODE.indexOf('a'); + assertEquals(1, positioner.lineNumberFromOffset(offset)); + assertEquals(1, positioner.columnFromOffset(offset)); + + offset = SOURCE_CODE.indexOf('b'); + assertEquals(1, positioner.lineNumberFromOffset(offset)); + assertEquals(2, positioner.columnFromOffset(offset)); + + offset = SOURCE_CODE.indexOf('e'); + assertEquals(2, positioner.lineNumberFromOffset(offset)); + assertEquals(2, positioner.columnFromOffset(offset)); + + offset = SOURCE_CODE.indexOf('q'); + assertEquals(5, positioner.lineNumberFromOffset(offset)); + assertEquals(3, positioner.columnFromOffset(offset)); + } +}