From 12afff4f5b022afedff555e8a8489f5d8f23ff8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Fri, 12 Apr 2019 14:23:13 -0300 Subject: [PATCH 01/34] [java] Fix race conditions in JavaTypeDefinitionSimple - The data is now a fixed-size array instead of a fixed-size arraylist, this removes the need to manually add null elements all over. - This in turn avoids the resizing / runtime exceptions under multithreaded accesses. - Take the change to fix the way `isGeneric` is resolved to be consistent over time and correct. - Fixes #1691 --- .../JavaTypeDefinitionSimple.java | 37 +++++++------------ 1 file changed, 14 insertions(+), 23 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/JavaTypeDefinitionSimple.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/JavaTypeDefinitionSimple.java index 1bed9a9f64..ba9d87d159 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/JavaTypeDefinitionSimple.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/JavaTypeDefinitionSimple.java @@ -16,8 +16,7 @@ import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; import java.lang.reflect.TypeVariable; import java.lang.reflect.WildcardType; -import java.util.ArrayList; -import java.util.Collections; +import java.util.Arrays; import java.util.HashSet; import java.util.List; import java.util.Objects; @@ -28,7 +27,7 @@ import java.util.logging.Logger; /* default */ class JavaTypeDefinitionSimple extends JavaTypeDefinition { private final Class clazz; - private final List genericArgs; + private final JavaTypeDefinition[] genericArgs; // cached because calling clazz.getTypeParameters().length create a new array every time private final int typeParameterCount; private final boolean isGeneric; @@ -36,6 +35,7 @@ import java.util.logging.Logger; private final JavaTypeDefinition enclosingClass; private static final Logger LOG = Logger.getLogger(JavaTypeDefinitionSimple.class.getName()); + private static final JavaTypeDefinition[] NO_GENERICS = {}; protected JavaTypeDefinitionSimple(Class clazz, JavaTypeDefinition... boundGenerics) { super(EXACT); @@ -59,12 +59,10 @@ import java.util.logging.Logger; isRawType = isGeneric && boundGenerics.length == 0; if (isGeneric) { - // Generics will be lazily loaded - this.genericArgs = new ArrayList<>(typeParameters.length); - // boundGenerics would be empty if this is a raw type, hence the lazy loading - Collections.addAll(this.genericArgs, boundGenerics); + // Generics will be lazily loaded if not already known + this.genericArgs = Arrays.copyOf(boundGenerics, typeParameterCount); } else { - this.genericArgs = Collections.emptyList(); + this.genericArgs = NO_GENERICS; } enclosingClass = forClass(clazz.getEnclosingClass()); @@ -82,7 +80,7 @@ import java.util.logging.Logger; @Override public boolean isGeneric() { - return !genericArgs.isEmpty(); + return isGeneric; } private JavaTypeDefinition getGenericType(final String parameterName, Method method, @@ -126,29 +124,22 @@ import java.util.logging.Logger; @Override public JavaTypeDefinition getGenericType(final int index) { // Check if it has been lazily initialized first - if (genericArgs.size() > index) { - final JavaTypeDefinition cachedDefinition = genericArgs.get(index); - if (cachedDefinition != null) { - return cachedDefinition; - } - } - - // Force the list to have enough elements - for (int i = genericArgs.size(); i <= index; i++) { - genericArgs.add(null); + final JavaTypeDefinition cachedDefinition = genericArgs[index]; + if (cachedDefinition != null) { + return cachedDefinition; } /* * Set a default to circuit-brake any recursions (ie: raw types with no generic info) * Object.class is a right answer in those scenarios */ - genericArgs.set(index, forClass(Object.class)); + genericArgs[index] = forClass(Object.class); final TypeVariable typeVariable = clazz.getTypeParameters()[index]; final JavaTypeDefinition typeDefinition = resolveTypeDefinition(typeVariable.getBounds()[0]); // cache result - genericArgs.set(index, typeDefinition); + genericArgs[index] = typeDefinition; return typeDefinition; } @@ -279,7 +270,7 @@ import java.util.logging.Logger; .append(", genericArgs=["); // Forcefully resolve all generic types - for (int i = 0; i < genericArgs.size(); i++) { + for (int i = 0; i < genericArgs.length; i++) { getGenericType(i); } @@ -287,7 +278,7 @@ import java.util.logging.Logger; sb.append(jtd.shallowString()).append(", "); } - if (!genericArgs.isEmpty()) { + if (genericArgs.length != 0) { sb.replace(sb.length() - 3, sb.length() - 1, ""); // remove last comma } From bb1ede559805e2fd18c36c55e3e4f6739dee89f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Fri, 12 Apr 2019 14:28:53 -0300 Subject: [PATCH 02/34] Update changelog, refs #1691 --- docs/pages/release_notes.md | 1 + .../typeresolution/typedefinition/JavaTypeDefinitionSimple.java | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index dcec3b34b5..2336c88a99 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -29,6 +29,7 @@ Being based on a proper Antlr grammar, CPD can: * go * [#1751](https://github.com/pmd/pmd/issues/1751): \[go] Parsing errors encountered with escaped backslash * java + * [#1691](https://github.com/pmd/pmd/issues/1691): \[java] Possible Data Race in JavaTypeDefinitionSimple.getGenericType * [#1729](https://github.com/pmd/pmd/issues/1729): \[java] JavaRuleViolation loses information in `className` field when class has package-private access level * java-bestpractices * [#1720](https://github.com/pmd/pmd/issues/1720): \[java] UnusedImports false positive for Javadoc link with array type diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/JavaTypeDefinitionSimple.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/JavaTypeDefinitionSimple.java index ba9d87d159..b1eca9da7f 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/JavaTypeDefinitionSimple.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/JavaTypeDefinitionSimple.java @@ -130,7 +130,7 @@ import java.util.logging.Logger; } /* - * Set a default to circuit-brake any recursions (ie: raw types with no generic info) + * Set a default to circuit-break any recursions (ie: raw types with no generic info) * Object.class is a right answer in those scenarios */ genericArgs[index] = forClass(Object.class); From 23185f9e6dd64788f95b1f34a79f4db878a203df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Fri, 12 Apr 2019 14:45:53 -0300 Subject: [PATCH 03/34] [java] Fix NPE in type resolution - Fixes #1532 --- .../pmd/lang/java/typeresolution/ClassTypeResolver.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java index 4883668d0a..324452245d 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java @@ -1407,7 +1407,9 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { } } else { JavaTypeDefinition def = JavaTypeDefinition.forClass(myType); - node.setTypeDefinition(def.withDimensions(arrayDimens)); + if (def != null) { + node.setTypeDefinition(def.withDimensions(arrayDimens)); + } } } From 72a2b7baab9cb64cea7600eaf48a8643a8811599 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Fri, 12 Apr 2019 14:46:48 -0300 Subject: [PATCH 04/34] Update changelog, refs #1532 --- docs/pages/release_notes.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index dcec3b34b5..9c0a4deb29 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -29,6 +29,7 @@ Being based on a proper Antlr grammar, CPD can: * go * [#1751](https://github.com/pmd/pmd/issues/1751): \[go] Parsing errors encountered with escaped backslash * java + * [#1532](https://github.com/pmd/pmd/issues/1532): \[java] NPE with incomplete auxclasspath * [#1729](https://github.com/pmd/pmd/issues/1729): \[java] JavaRuleViolation loses information in `className` field when class has package-private access level * java-bestpractices * [#1720](https://github.com/pmd/pmd/issues/1720): \[java] UnusedImports false positive for Javadoc link with array type From 0dd0ce1b065e3864cc120aa4ba7a981f7cfd522b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Fri, 12 Apr 2019 16:10:53 -0300 Subject: [PATCH 05/34] [apex] Add test case for #702 --- .../codestyle/xml/VariableNamingConventions.xml | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/codestyle/xml/VariableNamingConventions.xml b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/codestyle/xml/VariableNamingConventions.xml index a337efeaad..69441b8b3b 100644 --- a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/codestyle/xml/VariableNamingConventions.xml +++ b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/codestyle/xml/VariableNamingConventions.xml @@ -303,4 +303,21 @@ public class SerializerException extends Exception { ]]> + + Enum members + 0 + + From 44573d8bbed4a16564b430f9a87057acd1ebb644 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Fri, 12 Apr 2019 16:57:03 -0300 Subject: [PATCH 06/34] [java] JUnitTestContainsTooManyAsserts uses XPath 2 - We are already using `pmd-java:typeIs`, so it's the correct thing to do. - Plus, this way it's 6X faster --- pmd-java/src/main/resources/category/java/bestpractices.xml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pmd-java/src/main/resources/category/java/bestpractices.xml b/pmd-java/src/main/resources/category/java/bestpractices.xml index 92e683fd9a..1a8dfe3948 100644 --- a/pmd-java/src/main/resources/category/java/bestpractices.xml +++ b/pmd-java/src/main/resources/category/java/bestpractices.xml @@ -699,7 +699,7 @@ This rule checks for JUnit4, JUnit5 and TestNG Tests, as well as methods startin $maximumAsserts] + [count(..//PrimaryPrefix/Name[@Image[matches(.,'^assert')]]) > $maximumAsserts] ]]> + Date: Fri, 12 Apr 2019 17:27:27 -0300 Subject: [PATCH 07/34] [java] Add failing enum case for NoPackage --- .../pmd/lang/java/rule/codestyle/xml/NoPackage.xml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/NoPackage.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/NoPackage.xml index 67c418e021..95b3f865b7 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/NoPackage.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/NoPackage.xml @@ -32,6 +32,17 @@ nested package + + + + 1 + From eb7903d96355fc503191fb691e5dcb03edc44821 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Fri, 12 Apr 2019 17:27:45 -0300 Subject: [PATCH 08/34] [java] Fix NoPackage rule - Also make it's search much more precise, making it 10X faster --- pmd-java/src/main/resources/category/java/codestyle.xml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pmd-java/src/main/resources/category/java/codestyle.xml b/pmd-java/src/main/resources/category/java/codestyle.xml index 8421462853..9643356624 100644 --- a/pmd-java/src/main/resources/category/java/codestyle.xml +++ b/pmd-java/src/main/resources/category/java/codestyle.xml @@ -1359,8 +1359,9 @@ Detects when a class or interface does not have a package definition. 3 - //ClassOrInterfaceDeclaration[count(preceding::PackageDeclaration) = 0] + /CompilationUnit[not(./PackageDeclaration)]/TypeDeclaration[1] + Date: Sat, 13 Apr 2019 19:28:55 +0200 Subject: [PATCH 09/34] Declare fn prefix in XPath queries The namespace is standard and contains the XPath functions specified by the spec. Making it available on saxon improves the compatibility between Jaxen and Saxon queries --- .../sourceforge/pmd/lang/rule/xpath/SaxonXPathRuleQuery.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/SaxonXPathRuleQuery.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/SaxonXPathRuleQuery.java index b369007c68..5dcc9e4364 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/SaxonXPathRuleQuery.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/SaxonXPathRuleQuery.java @@ -18,6 +18,7 @@ import net.sourceforge.pmd.lang.xpath.Initializer; import net.sourceforge.pmd.properties.PropertyDescriptor; import net.sf.saxon.om.Item; +import net.sf.saxon.om.NamespaceConstant; import net.sf.saxon.om.ValueRepresentation; import net.sf.saxon.sxpath.AbstractStaticContext; import net.sf.saxon.sxpath.IndependentContext; @@ -193,6 +194,8 @@ public class SaxonXPathRuleQuery extends AbstractXPathRuleQuery { ((AbstractStaticContext) xpathStaticContext).setBackwardsCompatibilityMode(true); } + ((IndependentContext) xpathEvaluator.getStaticContext()).declareNamespace("fn", NamespaceConstant.FN); + // Register PMD functions Initializer.initialize((IndependentContext) xpathStaticContext); From 381506671042ba77495924d149b9f9c6f949cabe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Sat, 13 Apr 2019 19:39:28 +0200 Subject: [PATCH 10/34] Add tests --- .../pmd/lang/java/rule/XPathRuleTest.java | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/XPathRuleTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/XPathRuleTest.java index 077e1b5db7..88245a5554 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/XPathRuleTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/XPathRuleTest.java @@ -99,6 +99,24 @@ public class XPathRuleTest extends RuleTst { assertEquals(3, rv.getBeginLine()); } + @Test + public void testFnPrefixOnSaxon() throws Exception { + rule.setXPath("//VariableDeclaratorId[fn:matches(@Image, 'fiddle')]"); + rule.setVersion(XPathRuleQuery.XPATH_2_0); + Report report = getReportForTestString(rule, TEST2); + RuleViolation rv = report.iterator().next(); + assertEquals(3, rv.getBeginLine()); + } + + @Test + public void testNoFnPrefixOnSaxon() throws Exception { + rule.setXPath("//VariableDeclaratorId[matches(@Image, 'fiddle')]"); + rule.setVersion(XPathRuleQuery.XPATH_2_0); + Report report = getReportForTestString(rule, TEST2); + RuleViolation rv = report.iterator().next(); + assertEquals(3, rv.getBeginLine()); + } + /** * Test for problem reported in bug #1219 PrimarySuffix/@Image does not work From 94545b6049c17d701ba41131c4590e5bc3c1477b Mon Sep 17 00:00:00 2001 From: "Travis CI (pmd-bot)" Date: Sat, 13 Apr 2019 17:43:22 +0000 Subject: [PATCH 11/34] Update documentation TRAVIS_JOB_NUMBER=3650.1 TRAVIS_COMMIT_RANGE=0dd0ce1b065e...d6f24afa30d0 --- docs/pages/pmd/rules/java/bestpractices.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/pages/pmd/rules/java/bestpractices.md b/docs/pages/pmd/rules/java/bestpractices.md index 68feb9d383..4023ea302e 100644 --- a/docs/pages/pmd/rules/java/bestpractices.md +++ b/docs/pages/pmd/rules/java/bestpractices.md @@ -806,7 +806,7 @@ This rule checks for JUnit4, JUnit5 and TestNG Tests, as well as methods startin **This rule is defined by the following XPath expression:** ``` xpath -//MethodDeclarator[@Image[fn:matches(.,'^test')] or ../../Annotation/MarkerAnnotation/Name[ +//MethodDeclarator[@Image[matches(.,'^test')] or ../../Annotation/MarkerAnnotation/Name[ pmd-java:typeIs('org.junit.Test') or pmd-java:typeIs('org.junit.jupiter.api.Test') or pmd-java:typeIs('org.junit.jupiter.api.RepeatedTest') @@ -815,7 +815,7 @@ This rule checks for JUnit4, JUnit5 and TestNG Tests, as well as methods startin or pmd-java:typeIs('org.junit.jupiter.params.ParameterizedTest') or pmd-java:typeIs('org.testng.annotations.Test') ]] - [count(..//PrimaryPrefix/Name[@Image[fn:matches(.,'^assert')]]) > $maximumAsserts] + [count(..//PrimaryPrefix/Name[@Image[matches(.,'^assert')]]) > $maximumAsserts] ``` **Example(s):** From ee2d03a733d8d33b43c9dfc82cb6c93030f26226 Mon Sep 17 00:00:00 2001 From: "Travis CI (pmd-bot)" Date: Sat, 13 Apr 2019 18:27:25 +0000 Subject: [PATCH 12/34] Update documentation TRAVIS_JOB_NUMBER=3653.1 TRAVIS_COMMIT_RANGE=94545b6049c1...893119515677 --- docs/pages/pmd/rules/java/codestyle.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/pages/pmd/rules/java/codestyle.md b/docs/pages/pmd/rules/java/codestyle.md index 1e267004bc..88aab345da 100644 --- a/docs/pages/pmd/rules/java/codestyle.md +++ b/docs/pages/pmd/rules/java/codestyle.md @@ -1693,7 +1693,7 @@ Detects when a class or interface does not have a package definition. **This rule is defined by the following XPath expression:** ``` xpath -//ClassOrInterfaceDeclaration[count(preceding::PackageDeclaration) = 0] +/CompilationUnit[not(./PackageDeclaration)]/TypeDeclaration[1] ``` **Example(s):** From f121b102a46a3e651ded9edc252ec9867d4411a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=A4=D0=BE=D0=BC=D0=B8=D0=BD=20=D0=90=D0=BD=D0=B4=D1=80?= =?UTF-8?q?=D0=B5=D0=B9=20=D0=9D=D0=B8=D0=BA=D0=BE=D0=BB=D0=B0=D0=B5=D0=B2?= =?UTF-8?q?=D0=B8=D1=87?= Date: Mon, 15 Apr 2019 16:11:26 +0700 Subject: [PATCH 13/34] [java] Show more detailed message when can't resolve field type --- .../pmd/lang/java/typeresolution/ClassTypeResolver.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java index 4883668d0a..76668f7822 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java @@ -504,9 +504,10 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { // swallow } catch (final LinkageError e) { if (LOG.isLoggable(Level.WARNING)) { - LOG.log(Level.WARNING, "Error during type resolution due to: " + e); + String message = "Error during type resolution of field '" + fieldImage + "' in " + + typeToSearch.getType() + " due to: " + e; + LOG.log(Level.WARNING, message); } - // TODO : report a missing class once we start doing that... return null; } From 28c3752088331b88036aa6134220d2b6d90868fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Tue, 16 Apr 2019 14:04:46 -0300 Subject: [PATCH 14/34] [java] Fix FN in UnnecessaryLocalBeforeReturn - Fixes #1775 --- .../UnnecessaryLocalBeforeReturnRule.java | 12 +++++++++--- .../xml/UnnecessaryLocalBeforeReturn.xml | 15 +++++++++++++++ 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryLocalBeforeReturnRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryLocalBeforeReturnRule.java index 2890b968ca..dde6504241 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryLocalBeforeReturnRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryLocalBeforeReturnRule.java @@ -24,7 +24,6 @@ import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; import net.sourceforge.pmd.lang.java.symboltable.VariableNameDeclaration; import net.sourceforge.pmd.lang.symboltable.NameOccurrence; import net.sourceforge.pmd.lang.symboltable.Scope; -import net.sourceforge.pmd.lang.symboltable.ScopedNode; import net.sourceforge.pmd.properties.PropertyDescriptor; @@ -115,6 +114,12 @@ public class UnnecessaryLocalBeforeReturnRule extends AbstractJavaRule { final ASTReturnStatement rtn) { final ASTVariableInitializer initializer = variableDeclaration.getAccessNodeParent() .getFirstDescendantOfType(ASTVariableInitializer.class); + + // Get the block statements for each, so we can compare apples to apples + final ASTBlockStatement initializerStmt = variableDeclaration.getAccessNodeParent() + .getFirstParentOfType(ASTBlockStatement.class); + final ASTBlockStatement rtnStmt = rtn.getFirstParentOfType(ASTBlockStatement.class); + if (initializer != null) { final List referencedNames = initializer.findDescendantsOfType(ASTName.class); for (final ASTName refName : referencedNames) { @@ -128,9 +133,10 @@ public class UnnecessaryLocalBeforeReturnRule extends AbstractJavaRule { if (entry.getKey().getName().equals(refName.getImage())) { // Variable found! Check usage locations for (final NameOccurrence occ : entry.getValue()) { - final ScopedNode location = occ.getLocation(); + final ASTBlockStatement location = occ.getLocation().getFirstParentOfType(ASTBlockStatement.class); + // Is it used after initializing our "unnecessary" local but before the return statement? - if (isAfter(location, initializer) && isAfter(rtn, location)) { + if (isAfter(location, initializerStmt) && isAfter(rtnStmt, location)) { return true; } } diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/UnnecessaryLocalBeforeReturn.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/UnnecessaryLocalBeforeReturn.xml index 34310160d2..a97b4efc4d 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/UnnecessaryLocalBeforeReturn.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/UnnecessaryLocalBeforeReturn.xml @@ -229,6 +229,21 @@ public class UnnecessaryLocalBeforeReturnFP { sideEffect(m); return i; } +} + ]]> + + + + #1775 [java] False negative in UnnecessaryLocalBeforeReturn when splitting statements across multiple lines + 1 + From f004ff063420f5fb21a72d5f9836150c99f9e281 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Tue, 16 Apr 2019 14:07:37 -0300 Subject: [PATCH 15/34] Update changelog, refs #1775 --- docs/pages/release_notes.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index dcec3b34b5..17404057b2 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -32,6 +32,8 @@ Being based on a proper Antlr grammar, CPD can: * [#1729](https://github.com/pmd/pmd/issues/1729): \[java] JavaRuleViolation loses information in `className` field when class has package-private access level * java-bestpractices * [#1720](https://github.com/pmd/pmd/issues/1720): \[java] UnusedImports false positive for Javadoc link with array type +* java-codestyle + * [#1755](https://github.com/pmd/pmd/issues/1775): \[java] False negative in UnnecessaryLocalBeforeReturn when splitting statements across multiple lines * java-design * [#1760](https://github.com/pmd/pmd/issues/1760): \[java] UseObjectForClearerAPI flags private methods From 7c6d8be1d21a5248cf0502f03b485a33bd1ae977 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Tue, 16 Apr 2019 14:09:30 -0300 Subject: [PATCH 16/34] Delay searching statements --- .../codestyle/UnnecessaryLocalBeforeReturnRule.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryLocalBeforeReturnRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryLocalBeforeReturnRule.java index dde6504241..056878a58f 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryLocalBeforeReturnRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryLocalBeforeReturnRule.java @@ -115,12 +115,12 @@ public class UnnecessaryLocalBeforeReturnRule extends AbstractJavaRule { final ASTVariableInitializer initializer = variableDeclaration.getAccessNodeParent() .getFirstDescendantOfType(ASTVariableInitializer.class); - // Get the block statements for each, so we can compare apples to apples - final ASTBlockStatement initializerStmt = variableDeclaration.getAccessNodeParent() - .getFirstParentOfType(ASTBlockStatement.class); - final ASTBlockStatement rtnStmt = rtn.getFirstParentOfType(ASTBlockStatement.class); - if (initializer != null) { + // Get the block statements for each, so we can compare apples to apples + final ASTBlockStatement initializerStmt = variableDeclaration.getAccessNodeParent() + .getFirstParentOfType(ASTBlockStatement.class); + final ASTBlockStatement rtnStmt = rtn.getFirstParentOfType(ASTBlockStatement.class); + final List referencedNames = initializer.findDescendantsOfType(ASTName.class); for (final ASTName refName : referencedNames) { // TODO : Shouldn't the scope allow us to search for a var name occurrences directly, moving up through parent scopes? From cb1b5c16bffd63c954702907ae2e6fbf2193cfa4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Tue, 16 Apr 2019 16:51:15 -0300 Subject: [PATCH 17/34] [java] Have symbol table recognize concise resources - Concise resources are valid usages. - Fixes #1190 --- .../java/symboltable/JavaNameOccurrence.java | 6 ++++- .../java/symboltable/OccurrenceFinder.java | 17 ++++++++++++++ .../bestpractices/xml/UnusedLocalVariable.xml | 19 +++++++++++++++ .../bestpractices/xml/UnusedPrivateField.xml | 23 +++++++++++++++++++ 4 files changed, 64 insertions(+), 1 deletion(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/JavaNameOccurrence.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/JavaNameOccurrence.java index 7dcc57fe91..4e7033de52 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/JavaNameOccurrence.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/JavaNameOccurrence.java @@ -14,6 +14,7 @@ import net.sourceforge.pmd.lang.java.ast.ASTPreDecrementExpression; import net.sourceforge.pmd.lang.java.ast.ASTPreIncrementExpression; import net.sourceforge.pmd.lang.java.ast.ASTPrimaryExpression; import net.sourceforge.pmd.lang.java.ast.ASTPrimaryPrefix; +import net.sourceforge.pmd.lang.java.ast.ASTResource; import net.sourceforge.pmd.lang.java.ast.ASTStatementExpression; import net.sourceforge.pmd.lang.java.ast.JavaNode; import net.sourceforge.pmd.lang.symboltable.NameOccurrence; @@ -87,9 +88,12 @@ public class JavaNameOccurrence implements NameOccurrence { primaryExpression = location.jjtGetParent().jjtGetParent(); } else if (location.jjtGetParent().jjtGetParent() instanceof ASTPrimaryExpression) { primaryExpression = location.jjtGetParent().jjtGetParent().jjtGetParent(); + } else if (location.jjtGetParent() instanceof ASTResource) { + return false; } else { throw new RuntimeException( - "Found a NameOccurrence (" + location + ") that didn't have an ASTPrimary Expression as parent or grandparent. Parent = " + "Found a NameOccurrence (" + location + ") that didn't have an ASTPrimary Expression" + + " as parent or grandparent nor is a concise resource. Parent = " + location.jjtGetParent() + " and grandparent = " + location.jjtGetParent().jjtGetParent() + " (location line " + location.getBeginLine() + " col " + location.getBeginColumn() + ")"); } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/OccurrenceFinder.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/OccurrenceFinder.java index 37b6b819a5..13c770ab7e 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/OccurrenceFinder.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/OccurrenceFinder.java @@ -7,8 +7,11 @@ package net.sourceforge.pmd.lang.java.symboltable; import java.util.HashSet; import java.util.List; import java.util.Set; +import java.util.StringTokenizer; +import net.sourceforge.pmd.lang.java.ast.ASTName; import net.sourceforge.pmd.lang.java.ast.ASTPrimaryExpression; +import net.sourceforge.pmd.lang.java.ast.ASTResource; import net.sourceforge.pmd.lang.java.ast.JavaParserVisitorAdapter; import net.sourceforge.pmd.lang.symboltable.NameDeclaration; import net.sourceforge.pmd.lang.symboltable.Scope; @@ -21,6 +24,20 @@ public class OccurrenceFinder extends JavaParserVisitorAdapter { private final Set additionalDeclarations = new HashSet<>(); + @Override + public Object visit(ASTResource node, Object data) { + // is this a concise resource reference? + if (node.jjtGetNumChildren() == 1) { + ASTName nameNode = (ASTName) node.jjtGetChild(0); + for (StringTokenizer st = new StringTokenizer(nameNode.getImage(), "."); st.hasMoreTokens();) { + JavaNameOccurrence occ = new JavaNameOccurrence(nameNode, st.nextToken()); + new Search(occ).execute(); + } + } + + return super.visit(node, data); + } + @Override public Object visit(ASTPrimaryExpression node, Object data) { NameFinder nameFinder = new NameFinder(node); diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UnusedLocalVariable.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UnusedLocalVariable.xml index 4883375d4e..32c62ecf2d 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UnusedLocalVariable.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UnusedLocalVariable.xml @@ -386,6 +386,25 @@ public class Test { double result = Math.sqrt((a) - b); System.out.println(result); } +} + ]]> + + + + #1190 [java] UnusedLocalVariable/UnusedPrivateField false-positive + 0 + diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UnusedPrivateField.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UnusedPrivateField.xml index 3124ab44b1..f5ff621713 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UnusedPrivateField.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UnusedPrivateField.xml @@ -597,6 +597,29 @@ public class IssueUnusedPrivateField { String helper = "some new string"; // hidden here System.out.println("helper = " + helper); } +} + ]]> + + + + #1190 [java] UnusedLocalVariable/UnusedPrivateField false-positive + 0 + From 407b2d1efe825659d455a2165329eaee671712ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Tue, 16 Apr 2019 16:55:05 -0300 Subject: [PATCH 18/34] Update changelog, refs #1190 --- docs/pages/release_notes.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index dcec3b34b5..4b2cd945c6 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -31,6 +31,7 @@ Being based on a proper Antlr grammar, CPD can: * java * [#1729](https://github.com/pmd/pmd/issues/1729): \[java] JavaRuleViolation loses information in `className` field when class has package-private access level * java-bestpractices + * [#1190](https://github.com/pmd/pmd/issues/1190): \[java] UnusedLocalVariable/UnusedPrivateField false-positive * [#1720](https://github.com/pmd/pmd/issues/1720): \[java] UnusedImports false positive for Javadoc link with array type * java-design * [#1760](https://github.com/pmd/pmd/issues/1760): \[java] UseObjectForClearerAPI flags private methods From 17b7837dcc599fe806ae18e36362637ba20eb74b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Tue, 16 Apr 2019 16:59:40 -0300 Subject: [PATCH 19/34] Bring back the TODO --- .../pmd/lang/java/typeresolution/ClassTypeResolver.java | 1 + 1 file changed, 1 insertion(+) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java index 76668f7822..ebb3aec9b9 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java @@ -508,6 +508,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { + typeToSearch.getType() + " due to: " + e; LOG.log(Level.WARNING, message); } + // TODO : report a missing class once we start doing that... return null; } From a4a7aa48b48dc6c09523f0584e1664ad58eec859 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Tue, 16 Apr 2019 17:00:38 -0300 Subject: [PATCH 20/34] Update changelog, refs #1776 --- docs/pages/release_notes.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index dcec3b34b5..bf631fd572 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -43,6 +43,7 @@ Being based on a proper Antlr grammar, CPD can: * [#1746](https://github.com/pmd/pmd/pull/1746): \[java] Update rule to prevent UnusedImport when using JavaDoc with array type - [itaigilo](https://github.com/itaigilo) * [#1752](https://github.com/pmd/pmd/pull/1752): \[java] UseObjectForClearerAPI Only For Public - [Björn Kautler](https://github.com/Vampire) * [#1761](https://github.com/pmd/pmd/pull/1761): \[dart] \[cpd] Added CPD support for Dart - [Maikel Steneker](https://github.com/maikelsteneker) +* [#1776](https://github.com/pmd/pmd/pull/1776): \[java] Show more detailed message when can't resolve field type - [Andrey Fomin](https://github.com/andrey-fomin) {% endtocmaker %} From cd465918b5fbb77a350c0476ae875ca7c431c7db Mon Sep 17 00:00:00 2001 From: Maikel Steneker Date: Wed, 17 Apr 2019 15:05:20 +0200 Subject: [PATCH 21/34] Changed location for AssignmentToNonFinalStatic violations from field declaration at to assignment in constructor. This makes it easier to identify the code that needs to be adjusted in order to resolve the violation. --- .../errorprone/AssignmentToNonFinalStaticRule.java | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/AssignmentToNonFinalStaticRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/AssignmentToNonFinalStaticRule.java index 4870e31778..0fdb1a95e1 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/AssignmentToNonFinalStaticRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/AssignmentToNonFinalStaticRule.java @@ -33,16 +33,15 @@ public class AssignmentToNonFinalStaticRule extends AbstractJavaRule { continue; } - if (initializedInConstructor(entry.getValue())) { - addViolation(data, decl.getNode(), decl.getImage()); + final Node location = initializedInConstructor(entry.getValue()); + if (location != null) { + addViolation(data, location, decl.getImage()); } } return super.visit(node, data); } - private boolean initializedInConstructor(List usages) { - boolean initInConstructor = false; - + private Node initializedInConstructor(List usages) { for (NameOccurrence occ : usages) { // specifically omitting prefix and postfix operators as there are // legitimate usages of these with static fields, e.g. typesafe enum pattern. @@ -50,12 +49,12 @@ public class AssignmentToNonFinalStaticRule extends AbstractJavaRule { Node node = occ.getLocation(); Node constructor = node.getFirstParentOfType(ASTConstructorDeclaration.class); if (constructor != null) { - initInConstructor = true; + return node; } } } - return initInConstructor; + return null; } } From 4a8c23472c0e3eb9df0e161890abccc89a0e8524 Mon Sep 17 00:00:00 2001 From: Maikel Steneker Date: Wed, 17 Apr 2019 15:17:03 +0200 Subject: [PATCH 22/34] Added test case for multiple violations of AssignmentToNonFinalStatic on the same variable. Rationale: whenever this rule produces a violation, all of the unsafe assignments need to be corrected. It's annoying to fix one of these, rerun PMD and then realize there's another unsafe assignment left. Therefore, all of these violations should be reported at once. --- .../errorprone/xml/AssignmentToNonFinalStatic.xml | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/AssignmentToNonFinalStatic.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/AssignmentToNonFinalStatic.xml index 7f4c7844d1..249aeb6839 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/AssignmentToNonFinalStatic.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/AssignmentToNonFinalStatic.xml @@ -28,6 +28,21 @@ public class Foo { Foo(int y) { x = y; } +} + ]]> + + + + 2 + From d4ca21bfd3e8d8a284a0bbfaa00cd847caae9b93 Mon Sep 17 00:00:00 2001 From: Maikel Steneker Date: Wed, 17 Apr 2019 15:20:15 +0200 Subject: [PATCH 23/34] AssignmentToNonFinalStatic now reports multiple violations in case there are multiple unsafe assignments on the same variable. --- .../errorprone/AssignmentToNonFinalStaticRule.java | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/AssignmentToNonFinalStaticRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/AssignmentToNonFinalStaticRule.java index 0fdb1a95e1..9c3f01631e 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/AssignmentToNonFinalStaticRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/AssignmentToNonFinalStaticRule.java @@ -4,6 +4,7 @@ package net.sourceforge.pmd.lang.java.rule.errorprone; +import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -33,15 +34,16 @@ public class AssignmentToNonFinalStaticRule extends AbstractJavaRule { continue; } - final Node location = initializedInConstructor(entry.getValue()); - if (location != null) { + final List locations = initializedInConstructor(entry.getValue()); + for (final Node location : locations) { addViolation(data, location, decl.getImage()); } } return super.visit(node, data); } - private Node initializedInConstructor(List usages) { + private List initializedInConstructor(List usages) { + final List unsafeAssignments = new ArrayList<>(); for (NameOccurrence occ : usages) { // specifically omitting prefix and postfix operators as there are // legitimate usages of these with static fields, e.g. typesafe enum pattern. @@ -49,12 +51,12 @@ public class AssignmentToNonFinalStaticRule extends AbstractJavaRule { Node node = occ.getLocation(); Node constructor = node.getFirstParentOfType(ASTConstructorDeclaration.class); if (constructor != null) { - return node; + unsafeAssignments.add(node); } } } - return null; + return unsafeAssignments; } } From fd13a4bf21a94e8cc7ec471400a77b5d2347bd48 Mon Sep 17 00:00:00 2001 From: Maikel Steneker Date: Wed, 17 Apr 2019 16:00:38 +0200 Subject: [PATCH 24/34] Improved test cases for AssignmentToNonFinalStatic by including the expected line numbers for violations in the test cases. --- .../java/rule/errorprone/xml/AssignmentToNonFinalStatic.xml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/AssignmentToNonFinalStatic.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/AssignmentToNonFinalStatic.xml index 249aeb6839..a0ccf0047b 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/AssignmentToNonFinalStatic.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/AssignmentToNonFinalStatic.xml @@ -8,6 +8,7 @@ clear rule violation ]]> 1 + 4 2 + 4,5 Date: Wed, 17 Apr 2019 11:32:09 -0300 Subject: [PATCH 25/34] Update changelog, refs #1781 --- docs/pages/release_notes.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index ec8d27e00e..e1b3170486 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -101,6 +101,12 @@ The designer will still be shipped with PMD's binaries. * The new PLSQL rule {% rule "plsql/codestyle/LineLength" %} (`plsql-codestyle`) helps to enforce a maximum line length. +### Modified Rules + +* The Java rule {% rule "java/errorprone/AssignmentToNonFinalStatic" %} (`java-errorprone`) will now report on each + assignment made within a constructor rather than on the field declaration. This makes it easier for developers to + find the offending statements. + ### Fixed Issues * doc @@ -152,6 +158,7 @@ The previously available variables such as `OPTS` or `HEAPSIZE` are deprecated a * [#1717](https://github.com/pmd/pmd/pull/1717): \[java] Fix false positive in useTryWithResources when using a custom close method with multiple arguments - [Rishabh Jain](https://github.com/jainrish) * [#1724](https://github.com/pmd/pmd/pull/1724): \[doc] Correct property override example - [Felix W. Dekker](https://github.com/FWDekker) * [#1737](https://github.com/pmd/pmd/pull/1737): \[java] fix escaping of CommentDefaultAccessModifier documentation - [itaigilo](https://github.com/itaigilo) +* [#1781](https://github.com/pmd/pmd/pull/1781): \[java] Location change in AssignmentToNonFinalStatic - [Maikel Steneker](https://github.com/maikelsteneker) {% endtocmaker %} From 7542306e05a93208fa1759a16ad171924888e45c Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 18 Apr 2019 19:43:16 +0200 Subject: [PATCH 26/34] Fix header in release notes --- docs/pages/release_notes.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index a1e5df6836..0494e03a2e 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -14,7 +14,7 @@ This is a {{ site.pmd.release_type }} release. ### New and noteworthy -#### Dart support +#### Dart support Thanks to the contribution from [Maikel Steneker](https://github.com/maikelsteneker), and built on top of the ongoing efforts to fully support Antlr-based languages, PMD now has CPD support for [Dart](https://www.dartlang.org/). From b55676e4d0956ae5da5ed1b4eecb21fba6e02dfa Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 18 Apr 2019 19:53:14 +0200 Subject: [PATCH 27/34] Add test case for NoPackage and Annotation Update release notes, fixes #1782, refs #1771 --- docs/pages/release_notes.md | 5 ++++ .../resources/category/java/codestyle.xml | 4 +-- .../java/rule/codestyle/xml/NoPackage.xml | 25 +++++++++---------- 3 files changed, 19 insertions(+), 15 deletions(-) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 0494e03a2e..edf8ae5b63 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -30,6 +30,9 @@ Being based on a proper Antlr grammar, CPD can: assignment made within a constructor rather than on the field declaration. This makes it easier for developers to find the offending statements. +* The Java rule {% rule "java/codestyle/NoPackage" %} (`java-codestyle`) will now report additionally enums + and annotations that do not have a package declaration. + ### Fixed Issues * go @@ -38,6 +41,8 @@ Being based on a proper Antlr grammar, CPD can: * [#1729](https://github.com/pmd/pmd/issues/1729): \[java] JavaRuleViolation loses information in `className` field when class has package-private access level * java-bestpractices * [#1720](https://github.com/pmd/pmd/issues/1720): \[java] UnusedImports false positive for Javadoc link with array type +* java-codestyle + * [#1782](https://github.com/pmd/pmd/issues/1782): \[java] NoPackage: False Negative for enums * java-design * [#1760](https://github.com/pmd/pmd/issues/1760): \[java] UseObjectForClearerAPI flags private methods diff --git a/pmd-java/src/main/resources/category/java/codestyle.xml b/pmd-java/src/main/resources/category/java/codestyle.xml index 9643356624..05bd9f803b 100644 --- a/pmd-java/src/main/resources/category/java/codestyle.xml +++ b/pmd-java/src/main/resources/category/java/codestyle.xml @@ -1350,11 +1350,11 @@ public class Foo { -Detects when a class or interface does not have a package definition. +Detects when a class, interface, enum or annotation does not have a package definition. 3 diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/NoPackage.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/NoPackage.xml index 95b3f865b7..a851baee5f 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/NoPackage.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/NoPackage.xml @@ -4,9 +4,7 @@ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://pmd.sourceforge.net/rule-tests http://pmd.sourceforge.net/rule-tests_1_0_0.xsd"> - + bad 1 - + good 0 - + nested package 0 - - + + #1782 no package in top-level enum 1 + + #1782 no package with annotation + 1 + + From c4fafeeb62ce013aeadae6167d72597401f8e3a0 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 18 Apr 2019 20:13:40 +0200 Subject: [PATCH 28/34] [ci] Use jdk12 for building --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index f814e1b5c7..8a289d5c6b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -46,7 +46,7 @@ before_install: - rvm use 2.4.1 # Install OpenJDK 11 - see https://sormuras.github.io/blog/2018-03-20-jdk-matrix.html install: - - . ./install-jdk.sh -F 11 -L GPL -W $HOME/jdk + - . ./install-jdk.sh -F 12 -L GPL -W $HOME/jdk - gem install bundler - bundle install --with=release_notes_preprocessing --path=vendor/bundle before_script: true From 42f40ddd0e5a8564a0b810afd904629b0d8b5f21 Mon Sep 17 00:00:00 2001 From: "Travis CI (pmd-bot)" Date: Thu, 18 Apr 2019 18:32:24 +0000 Subject: [PATCH 29/34] Update documentation TRAVIS_JOB_NUMBER=3667.1 TRAVIS_COMMIT_RANGE=b55676e4d095...c4fafeeb62ce --- docs/pages/pmd/rules/java.md | 2 +- docs/pages/pmd/rules/java/codestyle.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/pages/pmd/rules/java.md b/docs/pages/pmd/rules/java.md index 29d0669364..80ca10856f 100644 --- a/docs/pages/pmd/rules/java.md +++ b/docs/pages/pmd/rules/java.md @@ -102,7 +102,7 @@ folder: pmd/rules * [MethodArgumentCouldBeFinal](pmd_rules_java_codestyle.html#methodargumentcouldbefinal): A method argument that is never re-assigned within the method can be declared final. * [MethodNamingConventions](pmd_rules_java_codestyle.html#methodnamingconventions): Configurable naming conventions for method declarations. This rule reports method decl... * [MIsLeadingVariableName](pmd_rules_java_codestyle.html#misleadingvariablename): Deprecated Detects when a non-field has a name starting with 'm_'. This usually denotes a field and could b... -* [NoPackage](pmd_rules_java_codestyle.html#nopackage): Detects when a class or interface does not have a package definition. +* [NoPackage](pmd_rules_java_codestyle.html#nopackage): Detects when a class, interface, enum or annotation does not have a package definition. * [OnlyOneReturn](pmd_rules_java_codestyle.html#onlyonereturn): A method should have only one exit point, and that should be the last statement in the method. * [PackageCase](pmd_rules_java_codestyle.html#packagecase): Detects when a package definition contains uppercase characters. * [PrematureDeclaration](pmd_rules_java_codestyle.html#prematuredeclaration): Checks for variables that are defined before they might be used. A reference is deemed to be prem... diff --git a/docs/pages/pmd/rules/java/codestyle.md b/docs/pages/pmd/rules/java/codestyle.md index 88aab345da..d2ff5b66c1 100644 --- a/docs/pages/pmd/rules/java/codestyle.md +++ b/docs/pages/pmd/rules/java/codestyle.md @@ -1689,7 +1689,7 @@ public class Foo { **Priority:** Medium (3) -Detects when a class or interface does not have a package definition. +Detects when a class, interface, enum or annotation does not have a package definition. **This rule is defined by the following XPath expression:** ``` xpath From 84daa1df4f807d3bb665009ab0e0384141b91657 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Fri, 19 Apr 2019 18:51:20 +0200 Subject: [PATCH 30/34] [java] Add unit test for JavaTypeDefinitionSimple --- .../JavaTypeDefinitionSimpleTest.java | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/typedefinition/JavaTypeDefinitionSimpleTest.java diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/typedefinition/JavaTypeDefinitionSimpleTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/typedefinition/JavaTypeDefinitionSimpleTest.java new file mode 100644 index 0000000000..f990c12b6b --- /dev/null +++ b/pmd-java/src/test/java/net/sourceforge/pmd/typeresolution/typedefinition/JavaTypeDefinitionSimpleTest.java @@ -0,0 +1,66 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.typeresolution.typedefinition; + +import java.util.ArrayList; + +import org.junit.Assert; +import org.junit.Test; + +import net.sourceforge.pmd.lang.java.typeresolution.typedefinition.JavaTypeDefinition; + +public class JavaTypeDefinitionSimpleTest { + + /** + * Tests the raw type {@code ArrayList}. + */ + @Test + public void arrayListWithoutBoundGenerics() { + JavaTypeDefinition typeDef = JavaTypeDefinition.forClass(ArrayList.class); + Assert.assertTrue(typeDef.isGeneric()); + Assert.assertTrue(typeDef.isRawType()); + Assert.assertEquals(1, typeDef.getTypeParameterCount()); + + JavaTypeDefinition genericType = typeDef.getGenericType(0); + Assert.assertFalse(genericType.isGeneric()); + Assert.assertEquals(Object.class, genericType.getType()); + } + + /** + * Tests the type {@code ArrayList}. + */ + @Test + public void arrayListOfString() { + JavaTypeDefinition typeDef = JavaTypeDefinition.forClass(ArrayList.class, JavaTypeDefinition.forClass(String.class)); + Assert.assertTrue(typeDef.isGeneric()); + Assert.assertEquals(1, typeDef.getTypeParameterCount()); + Assert.assertTrue(typeDef.isClassOrInterface()); + Assert.assertFalse(typeDef.isArrayType()); + + JavaTypeDefinition genericType = typeDef.getGenericType(0); + Assert.assertFalse(genericType.isGeneric()); + Assert.assertEquals(String.class, genericType.getType()); + + JavaTypeDefinition genericTypeByName = typeDef.getGenericType("E"); + Assert.assertEquals(String.class, genericTypeByName.getType()); + } + + @Test + public void array() { + JavaTypeDefinition typeDef = JavaTypeDefinition.forClass(String[].class); + Assert.assertFalse(typeDef.isGeneric()); + Assert.assertTrue(typeDef.isArrayType()); + Assert.assertFalse(typeDef.isClassOrInterface()); + Assert.assertEquals(String.class, typeDef.getElementType().getType()); + Assert.assertFalse(typeDef.isPrimitive()); + } + + @Test + public void primitive() { + JavaTypeDefinition typeDef = JavaTypeDefinition.forClass(int.class); + Assert.assertTrue(typeDef.isPrimitive()); + Assert.assertFalse(typeDef.isClassOrInterface()); + } +} From f52b866798510f352ea04b6634e943a3ede236cc Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Mon, 22 Apr 2019 11:23:22 +0200 Subject: [PATCH 31/34] Don't use String.resolveConstantDesc in unit tests to avoid random test failures with java12 --- .../pmd/properties/MethodPropertyTest.java | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/properties/MethodPropertyTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/properties/MethodPropertyTest.java index 3ed12aa0fa..8e39e7da90 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/properties/MethodPropertyTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/properties/MethodPropertyTest.java @@ -8,6 +8,7 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import java.lang.reflect.Method; +import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -32,12 +33,23 @@ import net.sourceforge.pmd.util.ClassUtil; */ public class MethodPropertyTest extends AbstractPackagedPropertyDescriptorTester { - private static final Method[] ALL_METHODS = String.class.getDeclaredMethods(); + private static final Method[] ALL_METHODS; private static final String[] METHOD_SIGNATURES = {"String#indexOf(int)", "String#substring(int,int)", "java.lang.String#substring(int,int)", "Integer#parseInt(String)", "java.util.HashMap#put(Object,Object)", "HashMap#containsKey(Object)", }; + static { + List allMethods = new ArrayList<>(); + for (Method m : String.class.getDeclaredMethods()) { + // exclude String.resolveConstantDesc to avoid random test failure with java12 + // there are two methods with the same signature available, but different return types... + if (!m.getName().equals("resolveConstantDesc")) { + allMethods.add(m); + } + } + ALL_METHODS = allMethods.toArray(new Method[0]); + } public MethodPropertyTest() { super("Method"); From 829e9d4a9bafd97b54c1685da0cc024e4c3423a3 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Mon, 22 Apr 2019 11:24:42 +0200 Subject: [PATCH 32/34] Upgrade maven-pmd-plugin to 3.12.0 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 8ef49eaf73..01188c6079 100644 --- a/pom.xml +++ b/pom.xml @@ -271,7 +271,7 @@ Additionally it includes CPD, the copy-paste-detector. CPD finds duplicated code 5.0 2.22.1 3.0.0 - 3.11.0 + 3.12.0 1.10.1 3.0.1 4.7 From 8c7f19bdfe4d4fcd37fd7149539ba9746824ac43 Mon Sep 17 00:00:00 2001 From: Andreas Schmid Date: Mon, 22 Apr 2019 16:21:13 +0200 Subject: [PATCH 33/34] Use current classloader instead of Thread's classloader This fixes #1788 by using the current's classloader instead of the current Thread's ones. This is required if executing CPD in a separate worker. Also adjusts the release notes. --- docs/pages/release_notes.md | 2 ++ .../src/main/java/net/sourceforge/pmd/cpd/LanguageFactory.java | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 39b90fd7b6..6417af5052 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -49,6 +49,8 @@ Being based on a proper Antlr grammar, CPD can: * [#1782](https://github.com/pmd/pmd/issues/1782): \[java] NoPackage: False Negative for enums * java-design * [#1760](https://github.com/pmd/pmd/issues/1760): \[java] UseObjectForClearerAPI flags private methods +* cpd-core + * [#1788](https://github.com/pmd/pmd/issues/1788): \[cpd] \[core] Use better `ClassLoader` for `ServiceLoader` in `LanguageFactory` ### API Changes diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/cpd/LanguageFactory.java b/pmd-core/src/main/java/net/sourceforge/pmd/cpd/LanguageFactory.java index 336a49a00c..a032ae7663 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/cpd/LanguageFactory.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/cpd/LanguageFactory.java @@ -33,7 +33,8 @@ public final class LanguageFactory { private LanguageFactory() { List languagesList = new ArrayList<>(); - ServiceLoader languageLoader = ServiceLoader.load(Language.class); + // Use current class' classloader instead of the threads context classloader, see https://github.com/pmd/pmd/issues/1788 + ServiceLoader languageLoader = ServiceLoader.load(Language.class, getClass().getClassLoader()); Iterator iterator = languageLoader.iterator(); while (iterator.hasNext()) { try { From 594ba4da230a3716163775db9c181a78bb069994 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Mon, 22 Apr 2019 13:33:24 -0300 Subject: [PATCH 34/34] Update changelog, refs #1789 --- docs/pages/release_notes.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 6417af5052..d892d62d6b 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -35,6 +35,8 @@ Being based on a proper Antlr grammar, CPD can: ### Fixed Issues +* all + * [#1788](https://github.com/pmd/pmd/issues/1788): \[cpd] \[core] Use better `ClassLoader` for `ServiceLoader` in `LanguageFactory` * go * [#1751](https://github.com/pmd/pmd/issues/1751): \[go] Parsing errors encountered with escaped backslash * java @@ -49,8 +51,6 @@ Being based on a proper Antlr grammar, CPD can: * [#1782](https://github.com/pmd/pmd/issues/1782): \[java] NoPackage: False Negative for enums * java-design * [#1760](https://github.com/pmd/pmd/issues/1760): \[java] UseObjectForClearerAPI flags private methods -* cpd-core - * [#1788](https://github.com/pmd/pmd/issues/1788): \[cpd] \[core] Use better `ClassLoader` for `ServiceLoader` in `LanguageFactory` ### API Changes @@ -62,6 +62,7 @@ Being based on a proper Antlr grammar, CPD can: * [#1761](https://github.com/pmd/pmd/pull/1761): \[dart] \[cpd] Added CPD support for Dart - [Maikel Steneker](https://github.com/maikelsteneker) * [#1776](https://github.com/pmd/pmd/pull/1776): \[java] Show more detailed message when can't resolve field type - [Andrey Fomin](https://github.com/andrey-fomin) * [#1781](https://github.com/pmd/pmd/pull/1781): \[java] Location change in AssignmentToNonFinalStatic - [Maikel Steneker](https://github.com/maikelsteneker) +* [#1789](https://github.com/pmd/pmd/pull/1789): \[cpd] \[core] Use current classloader instead of Thread's classloader - [Andreas Schmid](https://github.com/aaschmid) {% endtocmaker %}