From 06e38128c42cd1d6fee7e2b40603d9d04a35689a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Sat, 10 Feb 2018 12:43:10 +0100 Subject: [PATCH] Wire the visitor into SourceCodeProcessor We need a way to configure which visitors are run on nodes obtained from ParserTstUtil --- .../rule/errorprone/ErrorProneRulesTest.java | 3 ++ .../sourceforge/pmd/SourceCodeProcessor.java | 8 ++++ .../sourceforge/pmd/benchmark/Benchmark.java | 42 ++++++++++--------- .../lang/AbstractLanguageVersionHandler.java | 7 ++++ .../pmd/lang/LanguageVersionHandler.java | 9 ++++ .../pmd/lang/java/AbstractJavaHandler.java | 13 ++++++ .../lang/java/ast/QualifiedNameResolver.java | 11 +++-- .../pmd/lang/java/ParserTstUtil.java | 12 ++++-- .../lang/java/ast/JavaQualifiedNameTest.java | 36 +++++++--------- 9 files changed, 95 insertions(+), 46 deletions(-) diff --git a/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/errorprone/ErrorProneRulesTest.java b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/errorprone/ErrorProneRulesTest.java index d8ad2f11dd..7049e805af 100644 --- a/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/errorprone/ErrorProneRulesTest.java +++ b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/errorprone/ErrorProneRulesTest.java @@ -10,6 +10,9 @@ public class ErrorProneRulesTest extends SimpleAggregatorTst { private static final String RULESET = "category/apex/errorprone.xml"; + + + @Override public void setUp() { addRule(RULESET, "AvoidDirectAccessTriggerMap"); diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/SourceCodeProcessor.java b/pmd-core/src/main/java/net/sourceforge/pmd/SourceCodeProcessor.java index 036eb1aaca..78447e8d2b 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/SourceCodeProcessor.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/SourceCodeProcessor.java @@ -121,6 +121,13 @@ public class SourceCodeProcessor { Benchmarker.mark(Benchmark.SymbolTable, end - start, 0); } + private void resolveQualifiedNames(Node rootNode, LanguageVersionHandler handler) { + long start = System.nanoTime(); + handler.getQualifiedNameResolutionFacade().start(rootNode); + long end = System.nanoTime(); + Benchmarker.mark(Benchmark.QualifiedNameResolution, end - start, 0); + } + // private ParserOptions getParserOptions(final LanguageVersionHandler // languageVersionHandler) { // // TODO Handle Rules having different parser options. @@ -171,6 +178,7 @@ public class SourceCodeProcessor { Parser parser = PMD.parserFor(languageVersion, configuration); Node rootNode = parse(ctx, sourceCode, parser); + resolveQualifiedNames(rootNode, languageVersionHandler); symbolFacade(rootNode, languageVersionHandler); Language language = languageVersion.getLanguage(); usesDFA(languageVersion, rootNode, ruleSets, language); diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/benchmark/Benchmark.java b/pmd-core/src/main/java/net/sourceforge/pmd/benchmark/Benchmark.java index 0d9c760699..5e281bed7c 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/benchmark/Benchmark.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/benchmark/Benchmark.java @@ -5,32 +5,36 @@ package net.sourceforge.pmd.benchmark; /** + * Represents an execution phase for benchmarking purposes. * * @author Brian Remedios */ public enum Benchmark { - Rule(0, null), - RuleChainRule(1, null), - CollectFiles(2, "Collect files"), - LoadRules(3, "Load rules"), - Parser(4, "Parser"), - SymbolTable(5, "Symbol table"), - DFA(6, "DFA"), - TypeResolution(7, "Type resolution"), - RuleChainVisit(8, "RuleChain visit"), - Multifile(9, "Multifile analysis"), - Reporting(10, "Reporting"), - RuleTotal(11, "Rule total"), - RuleChainTotal(12, "Rule chain rule total"), - MeasuredTotal(13, "Measured total"), - NonMeasuredTotal(14, "Non-measured total"), - TotalPMD(16, "Total PMD"); + // The constants must be sorted in execution order, + // the index is derived from the ordinal of the constant + Rule(null), + RuleChainRule(null), + CollectFiles("Collect files"), + LoadRules("Load rules"), + Parser("Parser"), + QualifiedNameResolution("Qualified name resolution"), + SymbolTable("Symbol table"), + DFA("DFA"), + TypeResolution("Type resolution"), + RuleChainVisit("RuleChain visit"), + Multifile("Multifile analysis"), + Reporting("Reporting"), + RuleTotal("Rule total"), + RuleChainTotal("Rule chain rule total"), + MeasuredTotal("Measured total"), + NonMeasuredTotal("Non-measured total"), + TotalPMD("Total PMD"); - public final int index; + public final int index = ordinal(); public final String name; - Benchmark(int idx, String theName) { - index = idx; + + Benchmark(String theName) { name = theName; } } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/AbstractLanguageVersionHandler.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/AbstractLanguageVersionHandler.java index 6acbca3de9..e37736795d 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/AbstractLanguageVersionHandler.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/AbstractLanguageVersionHandler.java @@ -60,6 +60,13 @@ public abstract class AbstractLanguageVersionHandler implements LanguageVersionH return VisitorStarter.DUMMY; } + + @Override + public VisitorStarter getQualifiedNameResolutionFacade() { + return VisitorStarter.DUMMY; + } + + @Override public DFAGraphRule getDFAGraphRule() { return null; diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/LanguageVersionHandler.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/LanguageVersionHandler.java index e2ed820391..f881f1d45b 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/LanguageVersionHandler.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/LanguageVersionHandler.java @@ -96,5 +96,14 @@ public interface LanguageVersionHandler { VisitorStarter getMultifileFacade(); + /** + * Gets the visitor that populates the qualified names of the + * nodes. + * + * @return The visitor starter + */ + VisitorStarter getQualifiedNameResolutionFacade(); + + DFAGraphRule getDFAGraphRule(); } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/AbstractJavaHandler.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/AbstractJavaHandler.java index 56d9b1f1dd..bc65baaf08 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/AbstractJavaHandler.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/AbstractJavaHandler.java @@ -17,6 +17,7 @@ import net.sourceforge.pmd.lang.dfa.DFAGraphRule; import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit; import net.sourceforge.pmd.lang.java.ast.DumpFacade; import net.sourceforge.pmd.lang.java.ast.JavaNode; +import net.sourceforge.pmd.lang.java.ast.QualifiedNameResolver; import net.sourceforge.pmd.lang.java.dfa.DataFlowFacade; import net.sourceforge.pmd.lang.java.dfa.JavaDFAGraphRule; import net.sourceforge.pmd.lang.java.multifile.MultifileVisitorFacade; @@ -120,6 +121,18 @@ public abstract class AbstractJavaHandler extends AbstractLanguageVersionHandler }; } + + @Override + public VisitorStarter getQualifiedNameResolutionFacade() { + return new VisitorStarter() { + @Override + public void start(Node rootNode) { + new QualifiedNameResolver().initializeWith(((ASTCompilationUnit) rootNode)); + } + }; + } + + @Override public DFAGraphRule getDFAGraphRule() { return new JavaDFAGraphRule(); diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/QualifiedNameResolver.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/QualifiedNameResolver.java index 874e1c05b5..ea4624f92c 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/QualifiedNameResolver.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/QualifiedNameResolver.java @@ -25,7 +25,7 @@ public class QualifiedNameResolver extends JavaParserVisitorReducedAdapter { // Allows reusing the same list instance for the same packages. // Package prefixes are also shared. private final Map> FOUND_PACKAGES = new HashMap<>(128); - + // The following stacks stack some counter of the // visited classes. A new entry is pushed when // we enter a new class, and popped when we get @@ -75,6 +75,11 @@ public class QualifiedNameResolver extends JavaParserVisitorReducedAdapter { private ImmutableList classNames; + public void initializeWith(ASTCompilationUnit rootNode) { + rootNode.jjtAccept(this, null); + } + + public Object visit(ASTCompilationUnit node, Object data) { // update the package list @@ -99,7 +104,7 @@ public class QualifiedNameResolver extends JavaParserVisitorReducedAdapter { */ private ImmutableList getPackageList(ASTPackageDeclaration pack) { if (pack == null) { - return ListFactory.emptyList(); + return ListFactory.emptyList(); } final String image = pack.getPackageNameImage(); @@ -184,7 +189,7 @@ public class QualifiedNameResolver extends JavaParserVisitorReducedAdapter { } - /** Rollback the context to the state we were in the enclosing class. */ + /** Rollback the context to the state of the enclosing class. */ private void rollbackClassContext() { localIndices = localIndices.tail(); classNames = classNames.tail(); diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ParserTstUtil.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ParserTstUtil.java index b0d3968ebf..8137bbd33c 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ParserTstUtil.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ParserTstUtil.java @@ -24,6 +24,7 @@ import net.sourceforge.pmd.lang.LanguageVersion; import net.sourceforge.pmd.lang.LanguageVersionHandler; import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit; import net.sourceforge.pmd.lang.java.ast.JavaParserVisitor; +import net.sourceforge.pmd.lang.java.ast.QualifiedNameResolver; import net.sourceforge.pmd.lang.java.dfa.DataFlowFacade; import net.sourceforge.pmd.lang.java.symboltable.SymbolFacade; @@ -65,6 +66,9 @@ public class ParserTstUtil { } } + // TODO provide a configurable api to choose which visitors to invoke + // it makes no sense + public static Set getNodes(Class clazz, String javaCode) { return getNodes(LanguageRegistry.getLanguage(JavaLanguageModule.NAME).getDefaultVersion(), clazz, javaCode); } @@ -89,10 +93,9 @@ public class ParserTstUtil { JavaParserVisitor jpv = (JavaParserVisitor) Proxy.newProxyInstance(JavaParserVisitor.class.getClassLoader(), new Class[] { JavaParserVisitor.class }, coll); jpv.visit(cu, null); - SymbolFacade sf = new SymbolFacade(); - sf.initializeWith(cu); - DataFlowFacade dff = new DataFlowFacade(); - dff.initializeWith(languageVersionHandler.getDataFlowHandler(), cu); + new QualifiedNameResolver().initializeWith(cu); + new SymbolFacade().initializeWith(cu); + new DataFlowFacade().initializeWith(languageVersionHandler.getDataFlowHandler(), cu); return (List) coll.getCollection(); } @@ -211,6 +214,7 @@ public class ParserTstUtil { public static ASTCompilationUnit parseJava(LanguageVersionHandler languageVersionHandler, String code) { ASTCompilationUnit rootNode = (ASTCompilationUnit) languageVersionHandler .getParser(languageVersionHandler.getDefaultParserOptions()).parse(null, new StringReader(code)); + languageVersionHandler.getQualifiedNameResolutionFacade().start(rootNode); languageVersionHandler.getSymbolFacade().start(rootNode); return rootNode; } diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/JavaQualifiedNameTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/JavaQualifiedNameTest.java index 84f773af34..61cafac091 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/JavaQualifiedNameTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/JavaQualifiedNameTest.java @@ -4,7 +4,6 @@ package net.sourceforge.pmd.lang.java.ast; -import static net.sourceforge.pmd.lang.java.ParserTstUtil.getNodes; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; @@ -14,7 +13,6 @@ import static org.junit.Assert.assertTrue; import java.util.Arrays; import java.util.List; -import java.util.Set; import org.junit.Before; import org.junit.Test; @@ -40,11 +38,14 @@ public class JavaQualifiedNameTest { } + private List getNodes(Class nodeType, String source) { + return ParserTstUtil.getOrderedNodes(nodeType, source); + } + @Test public void testEmptyPackage() { final String TEST = "class Foo {}"; - Set nodes = getNodes(ASTClassOrInterfaceDeclaration.class, - TEST); + List nodes = getNodes(ASTClassOrInterfaceDeclaration.class, TEST); for (ASTClassOrInterfaceDeclaration coid : nodes) { JavaQualifiedName qname = coid.getQualifiedName(); assertEquals("Foo", qname.toString()); @@ -60,7 +61,7 @@ public class JavaQualifiedNameTest { public void testPackage() { final String TEST = "package foo.bar; class Bzaz{}"; - Set nodes = getNodes(ASTClassOrInterfaceDeclaration.class, + List nodes = getNodes(ASTClassOrInterfaceDeclaration.class, TEST); for (ASTClassOrInterfaceDeclaration coid : nodes) { JavaQualifiedName qname = coid.getQualifiedName(); @@ -76,8 +77,7 @@ public class JavaQualifiedNameTest { public void testNestedClass() { final String TEST = "package foo.bar; class Bzaz{ class Bor{ class Foo{}}}"; - Set nodes = getNodes(ASTClassOrInterfaceDeclaration.class, - TEST); + List nodes = getNodes(ASTClassOrInterfaceDeclaration.class, TEST); for (ASTClassOrInterfaceDeclaration coid : nodes) { JavaQualifiedName qname = coid.getQualifiedName(); @@ -98,7 +98,7 @@ public class JavaQualifiedNameTest { public void testNestedEnum() { final String TEST = "package foo.bar; class Foo { enum Bzaz{HOO;}}"; - Set nodes = getNodes(ASTEnumDeclaration.class, TEST); + List nodes = getNodes(ASTEnumDeclaration.class, TEST); for (ASTEnumDeclaration coid : nodes) { JavaQualifiedName qname = coid.getQualifiedName(); @@ -114,7 +114,7 @@ public class JavaQualifiedNameTest { public void testEnum() { final String TEST = "package foo.bar; enum Bzaz{HOO;}"; - Set nodes = getNodes(ASTEnumDeclaration.class, TEST); + List nodes = getNodes(ASTEnumDeclaration.class, TEST); for (ASTEnumDeclaration coid : nodes) { JavaQualifiedName qname = coid.getQualifiedName(); @@ -130,7 +130,7 @@ public class JavaQualifiedNameTest { public void testEnumMethodMember() { final String TEST = "package foo.bar; enum Bzaz{HOO; void foo(){}}"; - Set nodes = getNodes(ASTMethodDeclaration.class, TEST); + List nodes = getNodes(ASTMethodDeclaration.class, TEST); for (ASTMethodDeclaration coid : nodes) { JavaQualifiedName qname = coid.getQualifiedName(); @@ -146,8 +146,7 @@ public class JavaQualifiedNameTest { public void testNestedEmptyPackage() { final String TEST = "class Bzaz{ class Bor{ class Foo{}}}"; - Set nodes = getNodes(ASTClassOrInterfaceDeclaration.class, - TEST); + List nodes = getNodes(ASTClassOrInterfaceDeclaration.class, TEST); for (ASTClassOrInterfaceDeclaration coid : nodes) { JavaQualifiedName qname = coid.getQualifiedName(); @@ -170,8 +169,7 @@ public class JavaQualifiedNameTest { public void testMethod() { final String TEST = "package bar; class Bzaz{ public void foo(){}}"; - Set nodes = getNodes(ASTMethodDeclaration.class, - TEST); + List nodes = getNodes(ASTMethodDeclaration.class, TEST); for (ASTMethodDeclaration declaration : nodes) { JavaQualifiedName qname = declaration.getQualifiedName(); @@ -187,8 +185,7 @@ public class JavaQualifiedNameTest { public void testConstructor() { final String TEST = "package bar; class Bzaz{ public Bzaz(){}}"; - Set nodes = getNodes(ASTConstructorDeclaration.class, - TEST); + List nodes = getNodes(ASTConstructorDeclaration.class, TEST); for (ASTConstructorDeclaration declaration : nodes) { JavaQualifiedName qname = declaration.getQualifiedName(); @@ -205,8 +202,7 @@ public class JavaQualifiedNameTest { public void testConstructorWithParams() { final String TEST = "package bar; class Bzaz{ public Bzaz(int j, String k){}}"; - Set nodes = getNodes(ASTConstructorDeclaration.class, - TEST); + List nodes = getNodes(ASTConstructorDeclaration.class, TEST); for (ASTConstructorDeclaration declaration : nodes) { JavaQualifiedName qname = declaration.getQualifiedName(); @@ -222,7 +218,7 @@ public class JavaQualifiedNameTest { public void testConstructorOverload() { final String TEST = "package bar; class Bzaz{ public Bzaz(int j) {} public Bzaz(int j, String k){}}"; - Set nodes = getNodes(ASTConstructorDeclaration.class, + List nodes = getNodes(ASTConstructorDeclaration.class, TEST); ASTConstructorDeclaration[] arr = nodes.toArray(new ASTConstructorDeclaration[2]); @@ -235,7 +231,7 @@ public class JavaQualifiedNameTest { final String TEST = "package bar; class Bzaz{ public void foo(String j) {} " + "public void foo(int j){} public void foo(double k){}}"; - Set nodes = getNodes(ASTMethodDeclaration.class, TEST); + List nodes = getNodes(ASTMethodDeclaration.class, TEST); ASTMethodDeclaration[] arr = nodes.toArray(new ASTMethodDeclaration[3]); assertNotEquals(arr[0].getQualifiedName(), arr[1].getQualifiedName());