diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md index 0bd4d2f99f..5c752659ea 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.md +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -2,7 +2,7 @@ name: Bug report about: Create a report to help us improve title: '' -labels: bug +labels: 'a:bug' assignees: '' --- diff --git a/.github/ISSUE_TEMPLATE/feature_request.md b/.github/ISSUE_TEMPLATE/feature_request.md index 5efb987e38..ecbd0ebc84 100644 --- a/.github/ISSUE_TEMPLATE/feature_request.md +++ b/.github/ISSUE_TEMPLATE/feature_request.md @@ -2,7 +2,7 @@ name: Feature request about: Suggest an idea for this project title: '' -labels: enhancement +labels: 'an:enhancement' assignees: '' --- diff --git a/.github/ISSUE_TEMPLATE/new_rule.md b/.github/ISSUE_TEMPLATE/new_rule.md index ec48bd982c..4e79d37f11 100644 --- a/.github/ISSUE_TEMPLATE/new_rule.md +++ b/.github/ISSUE_TEMPLATE/new_rule.md @@ -2,7 +2,7 @@ name: New Rule about: You have an idea for a new rule? Great! title: '' -labels: new-rule +labels: 'a:new-rule' assignees: '' --- diff --git a/.github/ISSUE_TEMPLATE/question.md b/.github/ISSUE_TEMPLATE/question.md index 9389f8c3ba..5af12acfa1 100644 --- a/.github/ISSUE_TEMPLATE/question.md +++ b/.github/ISSUE_TEMPLATE/question.md @@ -2,7 +2,7 @@ name: Question about: Feel free to ask any question about PMD and its usage title: '' -labels: question +labels: 'a:question' assignees: '' --- diff --git a/.github/ISSUE_TEMPLATE/rule_violation.md b/.github/ISSUE_TEMPLATE/rule_violation.md index 92404bd43b..dd2c61ab0d 100644 --- a/.github/ISSUE_TEMPLATE/rule_violation.md +++ b/.github/ISSUE_TEMPLATE/rule_violation.md @@ -2,7 +2,7 @@ name: Rule violation about: Let us know about a false positive/false negative title: '' -labels: bug +labels: 'a:bug' assignees: '' --- diff --git a/docs/_data/sidebars/pmd_sidebar.yml b/docs/_data/sidebars/pmd_sidebar.yml index 3f8b4b3290..0064c6adf8 100644 --- a/docs/_data/sidebars/pmd_sidebar.yml +++ b/docs/_data/sidebars/pmd_sidebar.yml @@ -445,4 +445,7 @@ entries: - title: Merging pull requests url: /pmd_projectdocs_committers_merging_pull_requests.html output: web, pdf + - title: Main Landing page + url: /pmd_projectdocs_committers_main_landing_page.html + output: web, pdf diff --git a/docs/pages/pmd/projectdocs/committers/main_landing_page.md b/docs/pages/pmd/projectdocs/committers/main_landing_page.md new file mode 100644 index 0000000000..a0f2515068 --- /dev/null +++ b/docs/pages/pmd/projectdocs/committers/main_landing_page.md @@ -0,0 +1,85 @@ +--- +title: Main Landing Page +permalink: pmd_projectdocs_committers_main_landing_page.html +last_updated: March 2020 +author: Andreas Dangel +--- + +The main homepage of PMD is hosted by Github Pages. + +The repository is . + +It uses [Jekyll](https://jekyllrb.com/) to generate the static html pages. Jekyll is +executed by github for every push to the repository. Please note, that it takes some time +until Jekyll has been executed and due to caching, the homepage is not updated immediately. +It usually takes 15 minutes. + + +## Contents + +* Main page - aka "Landing page": + * Layout: [_layouts/default.html](https://github.com/pmd/pmd.github.io/blob/master/_layouts/default.html). + It includes all the sub section, which can be found in the includes directory [_includes/](https://github.com/pmd/pmd.github.io/tree/master/_includes) + * The latest PMD version is configured in `_config.yml` and the variables `site.pmd.latestVersion` are used + e.g. in [_includes/home.html](https://github.com/pmd/pmd.github.io/blob/master/_includes/home.html). +* Blog - aka "News": + * This is a section on main page. It shows the 5 latest news. See [_includes/news.html](https://github.com/pmd/pmd.github.io/blob/master/_includes/news.html). + * There is also a sub page "news" which lists all news. + * Layout: [_layouts/news.html](https://github.com/pmd/pmd.github.io/blob/master/_layouts/news.html) + * Page (which is pretty empty): [news.html](https://github.com/pmd/pmd.github.io/blob/master/news.html) +* Documentation for the latest release: + * The PMD documentation of the latest release is simply copied as static html into the folder [latest/](https://github.com/pmd/pmd.github.io/tree/master/latest). + This makes the latest release documentation available under the stable URL + . This URL is also used for the [sitemap.xml](https://github.com/pmd/pmd.github.io/blob/master/sitemap.xml). +* Documentation for previous releases are still being kept under the folders `pmd-/`. + + +## Building the page locally + +Since the repository contains the documentation for many old PMD releases, it is quite big. When executing +Jekyll to generate the site, it copies all the files to the folder `_site/` - and this can take a while. + +In order to speed things up locally, consider to add `pmd-*` to the exclude patterns in `_config.yml`. See +also the comments in this file. + +Then it is a matter of simply executing `bundle exec jekyll serve`. This will generate the site and host +it on localhost, so you can test the page at . + + +## Updates during a release + +When creating a new PMD release, some content of the main page need to be updated as well. +This done as part of the [Release process](pmd_projectdocs_committers_releasing.html), but is +summarized here as well: + +* The versions (e.g. `pmd.latestVersion`) needs to be updated in `_config.yml` + * This is needed to generate the correct links and texts for the latest version on landing page +* The new PMD documentation needs to be copied to `/pmd-/` +* Then this folder needs to copied to `/latest/`, actually replacing the old version. +* A new blog post with release notes is added: `/_posts/YYYY-mm-dd-PMD-.md` +* The sitemap `sitemap.xml` is regenerated + +Some of these steps are automated through `do-release.sh` (like blog post), some are manual steps +(updating the version in _config.yml) and other steps are done on the travis-ci-build (like +copying the new documentation). + +## Adding a new blog post + +Adding a new blog post is as easy as: + +* Creating a new file in the folder "_posts": `/_posts/YYYY-mm-dd-.md` +* The file name needs to fit this pattern. The date of the blog post is taken from the file name. The "<title>" + is used for the url. +* The file is a markdown file starting with a frontmatter for jekyll. Just use this template for the new file: + +``` +--- +layout: post +title: Title +--- + +Here comes the text +``` + +Once you commit and push it, Github will run Jekyll and update the page. The Jekyll templates take care that +the new post is recognized and added to the news section and also on the news subpage. diff --git a/docs/pages/pmd/projectdocs/committers/releasing.md b/docs/pages/pmd/projectdocs/committers/releasing.md index 16a64bc112..0951ac7e16 100644 --- a/docs/pages/pmd/projectdocs/committers/releasing.md +++ b/docs/pages/pmd/projectdocs/committers/releasing.md @@ -1,5 +1,5 @@ --- -title: Releasing +title: Release process permalink: pmd_projectdocs_committers_releasing.html author: Romain Pelisse <rpelisse@users.sourceforge.net>, Andreas Dangel <adangel@users.sourceforge.net> --- diff --git a/docs/pages/pmd/userdocs/incremental_analysis.md b/docs/pages/pmd/userdocs/incremental_analysis.md index 701fa19da3..5ba251e901 100644 --- a/docs/pages/pmd/userdocs/incremental_analysis.md +++ b/docs/pages/pmd/userdocs/incremental_analysis.md @@ -24,7 +24,7 @@ untouched, files with violations will be listed with full detail. Therefore, its Incremental analysis is enabled automatically once a location to store the cache has been defined. From command-line that is done through the [`-cache`](pmd_userdocs_cli_reference.html#cache) argument, but support for the feature is available for tools integrating PMD such as [Ant](pmd_userdocs_tools_ant.html), -[Maven](pmd_userdocs_tools_maven.html), and Gradle. +[Maven](pmd_userdocs_tools_maven.html), and [Gradle](pmd_userdocs_tools_gradle.html). ### Disabling incremental analysis @@ -32,3 +32,83 @@ available for tools integrating PMD such as [Ant](pmd_userdocs_tools_ant.html), By default, PMD will suggest to use an analysis cache by logging a warning. If you'd like to disable this warning, or ignore the analysis cache for a few runs, you can use the [`-no-cache`](pmd_userdocs_cli_reference.html#no-cache) switch. + + +### FAQ + +#### When is the cache invalidated? + +On the following reasons, the complete cache file is considered invalid: + +* The PMD version differs. Since each PMD version might have fixed some false-positives or false-negatives for rules, + a cache file created with a different version is considered invalid. The version comparison is exact. +* The used ruleset has been changed. If the ruleset is changed in any way (e.g. adding/removing rules, changing + rule properties, ...), the cache is considered invalid. +* The [`auxclasspath`](pmd_userdocs_cli_reference.html#auxclasspath) changed. The auxclasspath is used during + type resolution. A changed auxclasspath can result for rules, that use type resolution, in different + violations. Usually, if the auxclasspath is correct and type resolution works, the rules report less false-positives. + To make sure, the correct violations are reported, the cache is considered invalid, if the auxclasspath has changed. +* The execution classpath has been changed. On the execution classpath not only the PMD classes are located, but also + the implementation of e.g. custom rules. If any jar file/class file on the execution classpath is changed, then + the cache is considered invalid as well. + +#### What is stored in the cache file? + +The cache file consists of a header and a body. The header stores the information which is used to decided +whether the whole cache file is valid or not (see above). The following information is stored: + +* PMD Version +* Ruleset checksum +* Auxclasspath checksum +* Execution classpath checksum + +The body contains an entry for every file that has been analyzed. For every file, the following information +is stored: + +* The full (absolute) pathname of the file +* The checksum of the file itself +* 0 or more rule violations with all the info (line number, etc.) + +You can think of the cache as a Map where the filepath is used as the key +and the violations found in previous runs are the value. + +The cache is in the end just a file with serialized data (binary). The implementation is +{% jdoc core::cache.FileAnalysisCache %}. + +#### How does PMD detect whether a file has been changed? + +When analyzing a file, PMD records the checksum of the file content and stores this +together with the violations in the cache file. When running PMD with the cache file, +PMD looks up the file in the cache and compares the checksums. +If the checksums match, then the file is not even parsed, the rules +are not executed and the violations for this file are entirely used from the cache. +If the checksum doesn't match, then the cached violations are discarded (if there are any) +and the file is fully processed: the file is parsed and all the rules are run for it. +After we are done, the cache is updated with the new violations. + +#### Can I reuse a cache created on branch A for analyzing my project on branch B? + +This is possible. As long as the same PMD version and same ruleset is used on both branches. +Also note, that if the branch uses a different dependencies, the auxclasspath is different on both +classes, which invalidates the cache completely. If you project uses e.g. Maven for dependency +management and your branch uses different dependencies (either different version or completely different +artifacts), then the auxclasspath is changed. + +If files have been renamed on the branch, these files will be analyzed again since PMD uses +the file names to assign existing rule violations from the cache. Also, if the full path name +of the file changes, because the other branch is checked out at a different location, then all +the cached files don't match. + +Apart from these restrictions, PMD will only analyze files that changed between runs. +If your previous run was on branch A and then you run on branch B using the same cache file, +it will only look at files that are different between the 2 branches. + +#### Can I reuse a cache file across different machines? + +This is only possible, if the other machine uses the exact same path names. That means that +your project needs to be checked out into the same directory structure. + +Additionally, all the other restrictions apply (same PMD version, same ruleset, same auxclasspath, +same execution classpath). + +See also issue [#2063 [core] Support sharing incremental analysis cache file across different machines](https://github.com/pmd/pmd/issues/2063). diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 24ed6f31bd..ec3ff367b9 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -35,13 +35,30 @@ not change the result of your rules*, if it does, please report a bug at https:/ Note that XPath 1.0 support, the default XPath version, is deprecated since PMD 6.22.0. **We highly recommend that you upgrade your rules to XPath 2.0**. Please refer to the [migration guide](https://pmd.github.io/latest/pmd_userdocs_extending_writing_xpath_rules.html#migrating-from-10-to-20). +#### New Rules +* The new Apex rule {% rule "apex/codestyle/FieldDeclarationsShouldBeAtStart" %} (`apex-codestyle`) + helps to ensure that field declarations are always at the beginning of a class. + +* The new Apex rule {% rule "apex/bestpractices/UnusedLocalVariable" %} (`apex-bestpractices`) detects unused + local variables. ### Fixed Issues -* apex - * [#2210](https://github.com/pmd/pmd/issues/2210): \[apex] ApexCRUDViolation: Support WITH SECURITY_ENFORCED +* apex-design * [#2358](https://github.com/pmd/pmd/issues/2358): \[apex] Invalid Apex in Cognitive Complexity tests +* apex-security + * [#2210](https://github.com/pmd/pmd/issues/2210): \[apex] ApexCRUDViolation: Support WITH SECURITY_ENFORCED + * [#2399](https://github.com/pmd/pmd/issues/2399): \[apex] ApexCRUDViolation: false positive with security enforced with line break +* core + * [#2355](https://github.com/pmd/pmd/issues/2355): \[doc] Improve documentation about incremental analysis + * [#2356](https://github.com/pmd/pmd/issues/2356): \[doc] Add missing doc about pmd.github.io +* java + * [#2378](https://github.com/pmd/pmd/issues/2378): \[java] AbstractJUnitRule has bad performance on large code bases +* java-codestyle + * [#1723](https://github.com/pmd/pmd/issues/1723): \[java] UseDiamondOperator false-positive inside lambda +* java-design + * [#2390](https://github.com/pmd/pmd/issues/2390): \[java] AbstractClassWithoutAnyMethod: missing violation for nested classes ### API Changes @@ -101,6 +118,12 @@ implementations, and their corresponding Parser if it exists (in the same packag * {% jdoc matlab::lang.matlab.MatlabTokenManager %} * {% jdoc objectivec::lang.objectivec.ObjectiveCTokenManager %} +##### For removal + +* {% jdoc !!core::lang.Parser#getTokenManager(java.lang.String,java.io.Reader) %} +* {% jdoc !!core::lang.TokenManager#setFileName(java.lang.String) %} +* {% jdoc !!core::lang.ast.AbstractTokenManager#setFileName(java.lang.String) %} +* {% jdoc !!core::lang.ast.AbstractTokenManager#getFileName(java.lang.String) %} ### External Contributions @@ -108,6 +131,9 @@ implementations, and their corresponding Parser if it exists (in the same packag * [#2314](https://github.com/pmd/pmd/pull/2314): \[doc] maven integration - Add version to plugin - [Pham Hai Trung](https://github.com/gpbp) * [#2353](https://github.com/pmd/pmd/pull/2353): \[plsql] xmlforest with optional AS - [Piotr Szymanski](https://github.com/szyman23) * [#2383](https://github.com/pmd/pmd/pull/2383): \[apex] Fix invalid apex in documentation - [Gwilym Kuiper](https://github.com/gwilymatgearset) +* [#2395](https://github.com/pmd/pmd/pull/2395): \[apex] New Rule: Unused local variables - [Gwilym Kuiper](https://github.com/gwilymatgearset) +* [#2396](https://github.com/pmd/pmd/pull/2396): \[apex] New rule: field declarations should be at start - [Gwilym Kuiper](https://github.com/gwilymatgearset) +* [#2397](https://github.com/pmd/pmd/pull/2397): \[apex] fixed WITH SECURITY_ENFORCED regex to recognise line break characters - [Kieran Black](https://github.com/kieranlblack) {% endtocmaker %} diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/UnusedLocalVariableRule.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/UnusedLocalVariableRule.java new file mode 100644 index 0000000000..b898dd6999 --- /dev/null +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/UnusedLocalVariableRule.java @@ -0,0 +1,38 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.apex.rule.bestpractices; + +import java.util.List; + +import net.sourceforge.pmd.lang.apex.ast.ASTBlockStatement; +import net.sourceforge.pmd.lang.apex.ast.ASTVariableDeclaration; +import net.sourceforge.pmd.lang.apex.ast.ASTVariableExpression; +import net.sourceforge.pmd.lang.apex.rule.AbstractApexRule; + +public class UnusedLocalVariableRule extends AbstractApexRule { + public UnusedLocalVariableRule() { + addRuleChainVisit(ASTVariableDeclaration.class); + } + + @Override + public Object visit(ASTVariableDeclaration node, Object data) { + String variableName = node.getImage(); + + ASTBlockStatement variableContext = node.getFirstParentOfType(ASTBlockStatement.class); + List<ASTVariableExpression> potentialUsages = variableContext.findDescendantsOfType(ASTVariableExpression.class); + + for (ASTVariableExpression usage : potentialUsages) { + if (usage.getParent() == node) { + continue; + } + if (usage.getImage().equals(variableName)) { + return data; + } + } + + addViolation(data, node, variableName); + return data; + } +} diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/codestyle/FieldDeclarationsShouldBeAtStartRule.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/codestyle/FieldDeclarationsShouldBeAtStartRule.java new file mode 100644 index 0000000000..0c3c056306 --- /dev/null +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/codestyle/FieldDeclarationsShouldBeAtStartRule.java @@ -0,0 +1,74 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.apex.rule.codestyle; + +import java.util.ArrayList; +import java.util.Comparator; +import java.util.List; +import java.util.Optional; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +import net.sourceforge.pmd.lang.apex.ast.ASTBlockStatement; +import net.sourceforge.pmd.lang.apex.ast.ASTField; +import net.sourceforge.pmd.lang.apex.ast.ASTMethod; +import net.sourceforge.pmd.lang.apex.ast.ASTProperty; +import net.sourceforge.pmd.lang.apex.ast.ASTUserClass; +import net.sourceforge.pmd.lang.apex.ast.ApexNode; +import net.sourceforge.pmd.lang.apex.rule.AbstractApexRule; + +public class FieldDeclarationsShouldBeAtStartRule extends AbstractApexRule { + private static final Comparator<ApexNode<?>> NODE_BY_SOURCE_LOCATION_COMPARATOR = + Comparator + .<ApexNode<?>>comparingInt(ApexNode::getBeginLine) + .thenComparing(ApexNode::getBeginColumn); + public static final String STATIC_INITIALIZER_METHOD_NAME = "<clinit>"; + + public FieldDeclarationsShouldBeAtStartRule() { + addRuleChainVisit(ASTUserClass.class); + } + + @Override + public Object visit(ASTUserClass node, Object data) { + // Unfortunately the parser re-orders the AST to put field declarations before method declarations + // so we have to rely on line numbers / positions to work out where the first non-field declaration starts + // so we can check if the fields are in acceptable places. + List<ASTField> fields = node.findChildrenOfType(ASTField.class); + + List<ApexNode<?>> nonFieldDeclarations = new ArrayList<>(); + + nonFieldDeclarations.addAll(getMethodNodes(node)); + nonFieldDeclarations.addAll(node.findChildrenOfType(ASTUserClass.class)); + nonFieldDeclarations.addAll(node.findChildrenOfType(ASTProperty.class)); + nonFieldDeclarations.addAll(node.findChildrenOfType(ASTBlockStatement.class)); + + Optional<ApexNode<?>> firstNonFieldDeclaration = nonFieldDeclarations.stream() + .filter(ApexNode::hasRealLoc) + .min(NODE_BY_SOURCE_LOCATION_COMPARATOR); + + if (!firstNonFieldDeclaration.isPresent()) { + // there is nothing except field declaration, so that has to come first + return data; + } + + for (ASTField field : fields) { + if (NODE_BY_SOURCE_LOCATION_COMPARATOR.compare(field, firstNonFieldDeclaration.get()) > 0) { + addViolation(data, field, field.getName()); + } + } + + return data; + } + + private List<ApexNode<?>> getMethodNodes(ASTUserClass node) { + // The method <clinit> represents static initializer blocks, of which there can be many. The + // <clinit> method doesn't contain location information, however the containing ASTBlockStatements do, + // so we fetch them for that method only. + return node.findChildrenOfType(ASTMethod.class).stream() + .flatMap(method -> method.getImage().equals(STATIC_INITIALIZER_METHOD_NAME) + ? method.findChildrenOfType(ASTBlockStatement.class).stream() : Stream.of(method)) + .collect(Collectors.toList()); + } +} diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexCRUDViolationRule.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexCRUDViolationRule.java index 370ca7d0dd..c2dc6bbce0 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexCRUDViolationRule.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexCRUDViolationRule.java @@ -85,7 +85,7 @@ public class ApexCRUDViolationRule extends AbstractApexRule { private static final String[] RESERVED_KEYS_FLS = new String[] { "Schema", S_OBJECT_TYPE, }; - private static final Pattern WITH_SECURITY_ENFORCED = Pattern.compile("(?i).*[^']\\s*WITH\\s+SECURITY_ENFORCED\\s*[^']*"); + private static final Pattern WITH_SECURITY_ENFORCED = Pattern.compile("(?is).*[^']\\s*WITH\\s+SECURITY_ENFORCED\\s*[^']*"); private final Map<String, String> varToTypeMapping = new HashMap<>(); private final ListMultimap<String, String> typeToDMLOperationMapping = ArrayListMultimap.create(); diff --git a/pmd-apex/src/main/resources/category/apex/bestpractices.xml b/pmd-apex/src/main/resources/category/apex/bestpractices.xml index 9d2913a771..f91fa5095a 100644 --- a/pmd-apex/src/main/resources/category/apex/bestpractices.xml +++ b/pmd-apex/src/main/resources/category/apex/bestpractices.xml @@ -208,4 +208,25 @@ public class Foo { </example> </rule> + <rule name="UnusedLocalVariable" + since="6.23.0" + language="apex" + message="Variable ''{0}'' defined but not used" + class="net.sourceforge.pmd.lang.apex.rule.bestpractices.UnusedLocalVariableRule" + externalInfoUrl="${pmd.website.baseurl}/pmd_rules_apex_bestpractices.html#unusedlocalvariable"> + <description> +Detects when a local variable is declared and/or assigned but not used. + </description> + <example> +<![CDATA[ + public Boolean bar(String z) { + String x = 'some string'; // not used + + String y = 'some other string'; // used in the next line + return z.equals(y); + } +]]> + </example> + </rule> + </ruleset> diff --git a/pmd-apex/src/main/resources/category/apex/codestyle.xml b/pmd-apex/src/main/resources/category/apex/codestyle.xml index fea01c375b..3efffb249a 100644 --- a/pmd-apex/src/main/resources/category/apex/codestyle.xml +++ b/pmd-apex/src/main/resources/category/apex/codestyle.xml @@ -102,6 +102,30 @@ if (foo) { // preferred approach </example> </rule> + <rule name="FieldDeclarationsShouldBeAtStart" + language="apex" + since="6.23.0" + message="Field declaration for ''{0}'' should be before method declarations in its class" + class="net.sourceforge.pmd.lang.apex.rule.codestyle.FieldDeclarationsShouldBeAtStartRule" + externalInfoUrl="${pmd.website.baseurl}/pmd_rules_apex_codestyle.html#fielddeclarationsshouldbeatstart"> + <description> + Field declarations should appear before method declarations within a class. + </description> + <priority>3</priority> + <example> +<![CDATA[ +class Foo { + public Integer someField; // good + + public void someMethod() { + } + + public Integer anotherField; // bad +} +]]> + </example> + </rule> + <rule name="FieldNamingConventions" since="6.15.0" message="The {0} name ''{1}'' doesn''t match ''{2}''" @@ -336,5 +360,4 @@ while (true) { // preferred approach ]]> </example> </rule> - </ruleset> diff --git a/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/UnusedLocalVariableTest.java b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/UnusedLocalVariableTest.java new file mode 100644 index 0000000000..cef1960afc --- /dev/null +++ b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/UnusedLocalVariableTest.java @@ -0,0 +1,11 @@ +/* + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.apex.rule.bestpractices; + +import net.sourceforge.pmd.testframework.PmdRuleTst; + +public class UnusedLocalVariableTest extends PmdRuleTst { + // no additional unit tests +} diff --git a/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/codestyle/FieldDeclarationsShouldBeAtStartTest.java b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/codestyle/FieldDeclarationsShouldBeAtStartTest.java new file mode 100644 index 0000000000..c22140d52d --- /dev/null +++ b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/codestyle/FieldDeclarationsShouldBeAtStartTest.java @@ -0,0 +1,11 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.apex.rule.codestyle; + +import net.sourceforge.pmd.testframework.PmdRuleTst; + +public class FieldDeclarationsShouldBeAtStartTest extends PmdRuleTst { + // no additional unit tests +} diff --git a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/bestpractices/xml/UnusedLocalVariable.xml b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/bestpractices/xml/UnusedLocalVariable.xml new file mode 100644 index 0000000000..5e5e7ed803 --- /dev/null +++ b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/bestpractices/xml/UnusedLocalVariable.xml @@ -0,0 +1,82 @@ +<?xml version="1.0" encoding="utf-8" ?> +<test-data + xmlns="http://pmd.sourceforge.net/rule-tests" + 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"> + <test-code> + <description>Unused variables should result in errors</description> + <expected-problems>2</expected-problems> + <expected-linenumbers>3,7</expected-linenumbers> + <expected-messages> + <message>Variable 'foo' defined but not used</message> + <message>Variable 'foo' defined but not used</message> + </expected-messages> + <code> +<![CDATA[ +public class Foo { + public void assignedVariable() { + String foo = 'unused string'; + } + + public void justADeclaration() { + String foo; + } +} +]]> + </code> + </test-code> + + <test-code> + <description>Used variables should not result in errors</description> + <expected-problems>0</expected-problems> + <code> +<![CDATA[ +public class Foo { + public String basicUsage() { + String x = 'used variable'; + return x; + } + + public Account moreComplexUsage() { + String x = 'blah'; + return [SELECT Id FROM Account WHERE Name = :x]; + } + + public String usageInBlocks(Boolean y) { + String x = 'used variable'; + + if (y) { + return x; + } else { + return 'some other string'; + } + } +} +]]> + </code> + </test-code> + + <test-code> + <description>Shadowing a field</description> + <expected-problems>1</expected-problems> + <expected-linenumbers>5</expected-linenumbers> + <code><![CDATA[ +public class Foo { + private String myfield; + + public void unused() { + String myfield = 'unused string'; + } + + public String usedDifferentMethod() { + String myfield = 'used'; + return myfield; + } + + public String fieldUsage() { + return myfield; + } +} + ]]></code> + </test-code> +</test-data> \ No newline at end of file diff --git a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/codestyle/xml/FieldDeclarationsShouldBeAtStart.xml b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/codestyle/xml/FieldDeclarationsShouldBeAtStart.xml new file mode 100644 index 0000000000..f8f1b95503 --- /dev/null +++ b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/codestyle/xml/FieldDeclarationsShouldBeAtStart.xml @@ -0,0 +1,207 @@ +<?xml version="1.0" encoding="UTF-8"?> + +<test-data + xmlns="http://pmd.sourceforge.net/rule-tests" + 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"> + <test-code> + <description>Does not warn if there are no methods</description> + <expected-problems>0</expected-problems> + <code> +<![CDATA[ +class Foo { + public Integer thisIsOkay; +} +]]> + </code> + </test-code> + + <test-code> + <description>Does warn if a field is after a method</description> + <expected-problems>1</expected-problems> + <expected-linenumbers>4</expected-linenumbers> + <expected-messages> + <message>Field declaration for 'thisIsNotOkay' should be before method declarations in its class</message> + </expected-messages> + <code> +<![CDATA[ +class Foo { + public void someMethod() {} + + public Integer thisIsNotOkay; +} +]]> + </code> + </test-code> + + <test-code> + <description>Warns if field is after constructor</description> + <expected-problems>1</expected-problems> + <expected-linenumbers>6</expected-linenumbers> + <expected-messages> + <message>Field declaration for 'someField' should be before method declarations in its class</message> + </expected-messages> + <code> +<![CDATA[ +class Foo { + public Foo(Integer someValue) { + someField = someValue; + } + + private Integer someField; +} +]]> + </code> + </test-code> + + <test-code> + <description>Warns only for fields after the first method declaration</description> + <expected-problems>1</expected-problems> + <expected-linenumbers>8</expected-linenumbers> + <expected-messages> + <message>Field declaration for 'thisFieldIsNotOkay' should be before method declarations in its class</message> + </expected-messages> + <code> +<![CDATA[ +class Foo { + private Integer thisFieldIsOkay; + + public Foo(Integer someValue) { + someField = someValue; + } + + private Integer thisFieldIsNotOkay; +} +]]> + </code> + </test-code> + + <test-code> + <description>Warns for fields defined on the same line after a method</description> + <expected-problems>1</expected-problems> + <expected-linenumbers>2</expected-linenumbers> + <expected-messages> + <message>Field declaration for 'thisFieldIsNotOkay' should be before method declarations in its class</message> + </expected-messages> + <code> +<![CDATA[ +class Foo { + public Foo(Integer someValue) { someField = someValue; } private Integer thisFieldIsNotOkay; +} +]]> + </code> + </test-code> + + <test-code> + <description>Does not warn for fields defined on the same line before a method</description> + <expected-problems>0</expected-problems> + <code> + <![CDATA[ +class Foo { + private Integer thisFieldIsOkay; public Foo(Integer someValue) { someField = someValue; } +} +]]> + </code> + </test-code> + + <test-code> + <description>Allows nested classes to have fields</description> + <expected-problems>0</expected-problems> + <code> + <![CDATA[ +class Foo { + void bar() { } + + private class InnerFoo { + public Integer thisIsOkay; + } +} +]]> + </code> + </test-code> + + <test-code> + <description>Allows nested classes to have fields</description> + <expected-problems>1</expected-problems> + <expected-linenumbers>9</expected-linenumbers> + <code> + <![CDATA[ +class Foo { + void bar() { } + + private class InnerFoo { + public Integer thisIsOkay; + + public void bar() {} + + public Integer thisIsNotOkay; + } +} +]]> + </code> + </test-code> + + <test-code> + <description>Fields should go before inner classes too</description> + <expected-problems>1</expected-problems> + <expected-linenumbers>4</expected-linenumbers> + <code> +<![CDATA[ +class Foo { + private class InnerFoo {} + + public Integer thisIsNotOkay; +} +]]> + </code> + </test-code> + + <test-code> + <description>Fields should go before properties too</description> + <expected-problems>1</expected-problems> + <expected-linenumbers>4</expected-linenumbers> + <code> + <![CDATA[ +class Foo { + public Integer someProperty { get; } + + public Integer thisIsNotOkay; +} +]]> + </code> + </test-code> + + <test-code> + <description>Fields should go before block statements</description> + <expected-problems>1</expected-problems> + <expected-linenumbers>6</expected-linenumbers> + <code> + <![CDATA[ +class Foo { + { + System.debug('Hello'); + } + + public Integer thisIsNotOkay; +} +]]> + </code> + </test-code> + + <test-code> + <description>Fields should go before static block statements</description> + <expected-problems>1</expected-problems> + <expected-linenumbers>6</expected-linenumbers> + <code> + <![CDATA[ +class Foo { + static { + System.debug('Hello'); + } + + public Integer thisIsNotOkay; +} +]]> + </code> + </test-code> +</test-data> \ No newline at end of file diff --git a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexCRUDViolation.xml b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexCRUDViolation.xml index 6df26b5834..3d0293ebcb 100644 --- a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexCRUDViolation.xml +++ b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexCRUDViolation.xml @@ -275,6 +275,19 @@ public class Foo { } ]]></code> </test-code> + <test-code> + <description>Accepts Closure SECURITY ENFORCED Line Break </description> + <expected-problems>0</expected-problems> + <code><![CDATA[ +public class Foo { + public Contact foo(String tempID) { + Contact c = [SELECT Name FROM Contact WHERE Id=: tempID + WITH SECURITY_ENFORCED]; + return c; + } +} ]]></code> + </test-code> + <test-code> <description>Accepts Closure SECURITY ENFORCED in a List </description> <expected-problems>0</expected-problems> @@ -287,6 +300,19 @@ public class Foo { } ]]></code> </test-code> + <test-code> + <description>Accepts Closure SECURITY ENFORCED in a List Line Break</description> + <expected-problems>0</expected-problems> + <code><![CDATA[ +public class Foo { + public List<Contact> m() { + List<Contact> c = [SELECT Name FROM Contact + WITH SECURITY_ENFORCED LIMIT 1]; + return c; + } +} ]]></code> + </test-code> + <test-code> <description>Accepts Closure SECURITY ENFORCED with Case Insensitivity </description> <expected-problems>0</expected-problems> @@ -299,6 +325,19 @@ public class Foo { } ]]></code> </test-code> + <test-code> + <description>Accepts Closure SECURITY ENFORCED with Case Insensitivity Line Break </description> + <expected-problems>0</expected-problems> + <code><![CDATA[ +public class Foo { + public Contact foo(String tempID) { + Contact c = [SELECT Name FROM Contact WHERE Id=: tempID + WItH SECURITY_ENFORCED]; + return c; + } +} ]]></code> + </test-code> + <test-code> <description>Accepts Closure SECURITY ENFORCED Not Secured </description> <expected-problems>1</expected-problems> @@ -323,6 +362,19 @@ public class Foo { } ]]></code> </test-code> + <test-code> + <description>Accepts Closure SECURITY ENFORCED Secured Line Break </description> + <expected-problems>0</expected-problems> + <code><![CDATA[ +public class Foo { + public Contact foo() { + Contact c = [SELECT Name FROM Contact WHERE Name = 'WITH SECURITY_ENFORCED' + WITH SECURITY_ENFORCED]; + return c; + } +} ]]></code> + </test-code> + <test-code> <description>Proper accessibility CRUD,FLS </description> <expected-problems>0</expected-problems> diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/Parser.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/Parser.java index 3546ce01b5..a9e1581f54 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/Parser.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/Parser.java @@ -36,7 +36,9 @@ public interface Parser { * @param source * Reader that provides the source code to tokenize. * @return A TokenManager for reading token. + * @deprecated For removal in 7.0.0 */ + @Deprecated TokenManager getTokenManager(String fileName, Reader source); diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/TokenManager.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/TokenManager.java index e0e67c78cc..edb26311d1 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/TokenManager.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/TokenManager.java @@ -11,5 +11,10 @@ public interface TokenManager { // TODO : Change the return to GenericToken in 7.0.0 - maybe even use generics TokenManager<T extends GenericToken> Object getNextToken(); + + /** + * @deprecated For removal in 7.0.0 + */ + @Deprecated void setFileName(String fileName); } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/AbstractTokenManager.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/AbstractTokenManager.java index 0e2e9408ac..a1f2e2801e 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/AbstractTokenManager.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/AbstractTokenManager.java @@ -19,10 +19,18 @@ public abstract class AbstractTokenManager { protected Map<Integer, String> suppressMap = new HashMap<>(); protected String suppressMarker = PMD.SUPPRESS_MARKER; + /** + * @deprecated For removal in 7.0.0 + */ + @Deprecated public static void setFileName(String fileName) { AbstractTokenManager.fileName.set(fileName); } + /** + * @deprecated For removal in 7.0.0 + */ + @Deprecated public static String getFileName() { String fileName = AbstractTokenManager.fileName.get(); return fileName == null ? "(no file name provided)" : fileName; diff --git a/pmd-core/src/main/resources/rulesets/releases/6230.xml b/pmd-core/src/main/resources/rulesets/releases/6230.xml new file mode 100644 index 0000000000..e2d8620ff2 --- /dev/null +++ b/pmd-core/src/main/resources/rulesets/releases/6230.xml @@ -0,0 +1,14 @@ +<?xml version="1.0"?> + +<ruleset name="6230" + xmlns="http://pmd.sourceforge.net/ruleset/2.0.0" + xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" + xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 https://pmd.sourceforge.io/ruleset_2_0_0.xsd"> + <description> +This ruleset contains links to rules that are new in PMD v6.23.0 + </description> + + <rule ref="category/apex/bestpractices.xml/UnusedLocalVariable"/> + <rule ref="category/apex/codestyle.xml/FieldDeclarationsShouldBeAtStart"/> + +</ruleset> diff --git a/pmd-java/etc/grammar/Java.jjt b/pmd-java/etc/grammar/Java.jjt index e5055dbd50..6b2e7ccbb4 100644 --- a/pmd-java/etc/grammar/Java.jjt +++ b/pmd-java/etc/grammar/Java.jjt @@ -290,6 +290,53 @@ class JavaParserImpl { return getToken(1).kind == IDENTIFIER && getToken(1).getImage().equals(keyword); } + /** + * True if we're in a switch block, one precondition for parsing a yield + * statement. + */ + private boolean inSwitchExprBlock = false; + + private boolean isYieldStart() { + return inSwitchExprBlock + && isKeyword("yield") + && mayStartExprAfterYield(2); + } + + private boolean mayStartExprAfterYield(final int offset) { + // based off of https://hg.openjdk.java.net/jdk/jdk/file/bc3da0226ffa/src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavacParser.java#l2580 + // please don't sue me + Token token = getToken(offset); + if (token == null) return false; // eof + switch (token.kind) { + case PLUS: case MINUS: case STRING_LITERAL: case CHARACTER_LITERAL: + case INTEGER_LITERAL: case FLOATING_POINT_LITERAL: case HEX_FLOATING_POINT_LITERAL: + case NULL: case IDENTIFIER: case TRUE: case FALSE: + case NEW: case SWITCH: case THIS: case SUPER: + return true; + case INCR: case DECR: + return getToken(offset + 1).kind != SEMICOLON; // eg yield++; + case LPAREN: + int lookahead = offset + 1; + int balance = 1; + Token t; + while ((t = getToken(lookahead)) != null && balance > 0) { + switch (t.kind) { + case LPAREN: balance++; break; + case RPAREN: balance--; break; + case COMMA: if (balance == 1) return false; // a method call, eg yield(1, 2); + } + lookahead++; + } + // lambda: yield () -> {}; + // method call: yield (); + return t != null + && (lookahead != offset + 2 // ie () + || t.kind == LAMBDA); + default: + return false; + } + } + private boolean shouldStartStatementInSwitch() { switch (getToken(1).kind) { case _DEFAULT: @@ -1650,9 +1697,12 @@ void PostfixExpression() #void: void SwitchExpression() : -{} +{boolean prevInSwitchBlock = inSwitchExprBlock;} { - "switch" "(" Expression() ")" SwitchBlock() + "switch" "(" Expression() ")" + {inSwitchExprBlock = true;} + SwitchBlock() + {inSwitchExprBlock = prevInSwitchBlock;} } /** @@ -1985,6 +2035,7 @@ void Statement() #void: {} { Block() +| LOOKAHEAD( { isYieldStart() } ) YieldStatement() | EmptyStatement() | SwitchStatement() | IfStatement() @@ -2021,7 +2072,7 @@ void BlockStatement() #void: {} { LOOKAHEAD( { isNextTokenAnAssert() } ) AssertStatement() -| LOOKAHEAD({ jdkVersion >= 13 && isKeyword("yield") }) YieldStatement() +| LOOKAHEAD( { isYieldStart() } ) YieldStatement() | LOOKAHEAD(( "final" | Annotation() )* Type() <IDENTIFIER>) LocalVariableDeclaration() ";" { diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTCompilationUnit.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTCompilationUnit.java index 7207014e79..7cd700e683 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTCompilationUnit.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTCompilationUnit.java @@ -54,6 +54,11 @@ public final class ASTCompilationUnit extends AbstractJavaTypeNode implements Ro return AstImplUtil.getChildAs(this, 0, ASTPackageDeclaration.class); } + @Override + public ASTCompilationUnit getRoot() { + return this; + } + /** * Returns the package name of this compilation unit. If there is no * package declaration, then returns the empty string. @@ -80,18 +85,12 @@ public final class ASTCompilationUnit extends AbstractJavaTypeNode implements Ro return classTypeResolver; } - - @Override - public ASTCompilationUnit getRoot() { - return this; } @Override public @NonNull JSymbolTable getSymbolTable() { assert symbolTable != null : "Symbol table wasn't set"; return symbolTable; - } - @InternalApi @Deprecated public void setClassTypeResolver(ClassTypeResolver classTypeResolver) { diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/DuplicateImportsRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/DuplicateImportsRule.java index 655cd8c600..ee26f49b20 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/DuplicateImportsRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/DuplicateImportsRule.java @@ -66,7 +66,7 @@ public class DuplicateImportsRule extends AbstractJavaRule { return true; } } else { - Class<?> importClass = node.getClassTypeResolver().loadClass(thisImportOnDemand.getName()); + Class<?> importClass = node.getClassTypeResolver().loadClassOrNull(thisImportOnDemand.getName()); if (importClass != null) { for (Method m : importClass.getMethods()) { if (Modifier.isStatic(m.getModifiers()) && m.getName().equals(singleTypeName)) { 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 9221240f3d..a069c2ae02 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 @@ -82,6 +82,7 @@ import net.sourceforge.pmd.lang.java.ast.TypeNode; import net.sourceforge.pmd.lang.java.ast.UnaryOp; import net.sourceforge.pmd.lang.java.symboltable.ClassScope; import net.sourceforge.pmd.lang.java.symboltable.VariableNameDeclaration; +import net.sourceforge.pmd.lang.java.typeresolution.internal.NullableClassLoader; import net.sourceforge.pmd.lang.java.typeresolution.typedefinition.JavaTypeDefinition; import net.sourceforge.pmd.lang.symboltable.NameOccurrence; import net.sourceforge.pmd.lang.symboltable.Scope; @@ -95,7 +96,7 @@ import net.sourceforge.pmd.lang.symboltable.Scope; @Deprecated @InternalApi -public class ClassTypeResolver extends JavaParserVisitorAdapter { +public class ClassTypeResolver extends JavaParserVisitorAdapter implements NullableClassLoader { private static final Logger LOG = Logger.getLogger(ClassTypeResolver.class.getName()); @@ -1346,10 +1347,15 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { return pmdClassLoader.loadClassOrNull(fullyQualifiedClassName) != null; } - public Class<?> loadClass(String fullyQualifiedClassName) { + @Override + public Class<?> loadClassOrNull(String fullyQualifiedClassName) { return pmdClassLoader.loadClassOrNull(fullyQualifiedClassName); } + public Class<?> loadClass(String fullyQualifiedClassName) { + return loadClassOrNull(fullyQualifiedClassName); + } + private Class<?> processOnDemand(String qualifiedName) { for (String entry : importedOnDemand) { String fullClassName = entry + "." + qualifiedName; @@ -1392,12 +1398,12 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { String strPackage = anImportDeclaration.getPackageName(); if (anImportDeclaration.isStatic()) { if (anImportDeclaration.isImportOnDemand()) { - importOnDemandStaticClasses.add(JavaTypeDefinition.forClass(loadClass(strPackage))); + importOnDemandStaticClasses.add(JavaTypeDefinition.forClass(loadClassOrNull(strPackage))); } else { // not import on-demand String strName = anImportDeclaration.getImportedName(); String fieldName = strName.substring(strName.lastIndexOf('.') + 1); - Class<?> staticClassWithField = loadClass(strPackage); + Class<?> staticClassWithField = loadClassOrNull(strPackage); if (staticClassWithField != null) { JavaTypeDefinition typeDef = getFieldType(JavaTypeDefinition.forClass(staticClassWithField), fieldName, currentAcu.getType()); diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/PMDASMClassLoader.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/PMDASMClassLoader.java index 24156d8093..3446631436 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/PMDASMClassLoader.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/PMDASMClassLoader.java @@ -15,6 +15,7 @@ import java.util.concurrent.ConcurrentMap; import org.objectweb.asm.ClassReader; import net.sourceforge.pmd.annotation.InternalApi; +import net.sourceforge.pmd.lang.java.typeresolution.internal.NullableClassLoader; import net.sourceforge.pmd.lang.java.typeresolution.visitors.PMDASMVisitor; /* @@ -36,7 +37,7 @@ import net.sourceforge.pmd.lang.java.typeresolution.visitors.PMDASMVisitor; */ @InternalApi @Deprecated -public final class PMDASMClassLoader extends ClassLoader { +public final class PMDASMClassLoader extends ClassLoader implements NullableClassLoader { private static PMDASMClassLoader cachedPMDASMClassLoader; private static ClassLoader cachedClassLoader; @@ -88,6 +89,7 @@ public final class PMDASMClassLoader extends ClassLoader { * Not throwing CNFEs to represent failure makes a huge performance * difference. Typeres as a whole is 2x faster. */ + @Override public Class<?> loadClassOrNull(String name) { if (dontBother.containsKey(name)) { return null; diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/TypeHelper.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/TypeHelper.java index abdab69430..b871d925ab 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/TypeHelper.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/TypeHelper.java @@ -6,7 +6,10 @@ package net.sourceforge.pmd.lang.java.typeresolution; import static net.sourceforge.pmd.util.CollectionUtil.any; -import org.apache.commons.lang3.ClassUtils; +import java.util.Map; + +import org.apache.commons.lang3.StringUtils; +import org.apache.commons.lang3.Validate; import net.sourceforge.pmd.lang.java.ast.ASTAnnotationTypeDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration; @@ -14,9 +17,27 @@ import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceType; import net.sourceforge.pmd.lang.java.ast.ASTEnumDeclaration; import net.sourceforge.pmd.lang.java.ast.TypeNode; import net.sourceforge.pmd.lang.java.symboltable.TypedNameDeclaration; +import net.sourceforge.pmd.lang.java.typeresolution.internal.NullableClassLoader; +import net.sourceforge.pmd.lang.java.typeresolution.internal.NullableClassLoader.ClassLoaderWrapper; public final class TypeHelper { + /** Maps names of primitives to their corresponding primitive {@code Class}es. */ + private static final Map<String, Class<?>> PRIMITIVES_BY_NAME = new HashMap<>(); + + + static { + PRIMITIVES_BY_NAME.put("boolean", Boolean.TYPE); + PRIMITIVES_BY_NAME.put("byte", Byte.TYPE); + PRIMITIVES_BY_NAME.put("char", Character.TYPE); + PRIMITIVES_BY_NAME.put("short", Short.TYPE); + PRIMITIVES_BY_NAME.put("int", Integer.TYPE); + PRIMITIVES_BY_NAME.put("long", Long.TYPE); + PRIMITIVES_BY_NAME.put("double", Double.TYPE); + PRIMITIVES_BY_NAME.put("float", Float.TYPE); + PRIMITIVES_BY_NAME.put("void", Void.TYPE); + } + private TypeHelper() { // utility class } @@ -35,6 +56,10 @@ public final class TypeHelper { * @return <code>true</code> if type node n is of type clazzName or a subtype of clazzName */ public static boolean isA(final TypeNode n, final String clazzName) { + if (n.getType() != null && n.getType().isAnnotation()) { + return isAnnotationSubtype(n.getType(), clazzName); + } + final Class<?> clazz = loadClassWithNodeClassloader(n, clazzName); if (clazz != null || n.getType() != null) { @@ -48,6 +73,20 @@ public final class TypeHelper { return fallbackIsA(n, clazzName); } + /** + * Returns true if the class n is a subtype of clazzName, given n + * is an annotationt type. + */ + private static boolean isAnnotationSubtype(Class<?> n, String clazzName) { + assert n != null && n.isAnnotation() : "Not an annotation type"; + // then, the supertype may only be Object, j.l.Annotation, or the class name + // this avoids classloading altogether + // this is used e.g. by the typeIs function in XPath + return "java.lang.annotation.Annotation".equals(clazzName) + || "java.lang.Object".equals(clazzName) + || clazzName.equals(n.getName()); + } + private static boolean fallbackIsA(TypeNode n, String clazzName) { if (clazzName.equals(n.getImage()) || clazzName.endsWith("." + n.getImage())) { return true; @@ -70,9 +109,11 @@ public final class TypeHelper { return "java.lang.Enum".equals(clazzName) // supertypes of Enum || "java.lang.Comparable".equals(clazzName) - || "java.io.Serializable".equals(clazzName); + || "java.io.Serializable".equals(clazzName) + || "java.lang.Object".equals(clazzName); } else if (n instanceof ASTAnnotationTypeDeclaration) { - return "java.lang.annotation.Annotation".equals(clazzName); + return "java.lang.annotation.Annotation".equals(clazzName) + || "java.lang.Object".equals(clazzName); } return false; @@ -89,6 +130,11 @@ public final class TypeHelper { * @throws NullPointerException if n is null */ public static boolean isExactlyA(final TypeNode n, final String clazzName) { + if (n.getType() != null && n.getType().getName().equals(clazzName)) { + // fast path avoiding classloading + return true; + } + final Class<?> clazz = loadClassWithNodeClassloader(n, clazzName); if (clazz != null) { @@ -100,34 +146,87 @@ public final class TypeHelper { private static Class<?> loadClassWithNodeClassloader(final TypeNode n, final String clazzName) { if (n.getType() != null) { - return loadClass(n.getType().getClassLoader(), clazzName); + return loadClass(n.getRoot().getClassTypeResolver(), clazzName); } return null; } - private static Class<?> loadClass(final ClassLoader nullableClassLoader, final String clazzName) { - try { - ClassLoader classLoader = nullableClassLoader; - if (classLoader == null) { - // Using the system classloader then - classLoader = ClassLoader.getSystemClassLoader(); + + /** + * Load a class. Supports loading array types like 'java.lang.String[]' and + * converting a canonical name to a binary name (eg 'java.util.Map.Entry' -> + * 'java.util.Map$Entry'). + */ + // test only + static Class<?> loadClass(NullableClassLoader ctr, String className) { + return loadClassMaybeArray(ctr, StringUtils.deleteWhitespace(className)); + } + + private static Class<?> loadClassFromCanonicalName(NullableClassLoader ctr, String className) { + Class<?> clazz = PRIMITIVES_BY_NAME.get(className); + if (clazz == null) { + clazz = ctr.loadClassOrNull(className); + } + if (clazz != null) { + return clazz; + } + // allow path separators (.) as inner class name separators + final int lastDotIndex = className.lastIndexOf('.'); + + if (lastDotIndex >= 0) { + String asInner = className.substring(0, lastDotIndex) + + '$' + className.substring(lastDotIndex + 1); + return loadClassFromCanonicalName(ctr, asInner); + } + return null; + } + + + private static Class<?> loadClassMaybeArray(NullableClassLoader classLoader, + String className) { + Validate.notNull(className, "className must not be null."); + if (className.endsWith("[]")) { + int dimension = 0; + int i = className.length(); + while (i >= 2 && className.startsWith("[]", i - 2)) { + dimension++; + i -= 2; } - // If the requested type is in the classpath, using the same classloader should work - return ClassUtils.getClass(classLoader, clazzName); - } catch (ClassNotFoundException ignored) { - // The requested type is not on the auxclasspath. This might happen, if the type node - // is probed for a specific type (e.g. is is a JUnit5 Test Annotation class). - // Failing to resolve clazzName does not necessarily indicate an incomplete auxclasspath. - } catch (final LinkageError expected) { - // We found the class but it's invalid / incomplete. This may be an incomplete auxclasspath - // if it was a NoClassDefFoundError. TODO : Report it? + checkJavaIdent(className, i); + String elementName = className.substring(0, i); + + Class<?> elementType = loadClassFromCanonicalName(classLoader, elementName); + if (elementType == null) { + return null; + } + + return Array.newInstance(elementType, (int[]) Array.newInstance(int.class, dimension)).getClass(); + } else { + checkJavaIdent(className, className.length()); + return loadClassFromCanonicalName(classLoader, className); + } + } + + private static IllegalArgumentException invalidClassName(String className) { + return new IllegalArgumentException("Not a valid class name \"" + className + "\""); + } + + private static void checkJavaIdent(String className, int endOffsetExclusive) { + if (endOffsetExclusive <= 0 || !Character.isJavaIdentifierStart(className.charAt(0))) { + throw invalidClassName(className); } - return null; + for (int i = 1; i < endOffsetExclusive; i++) { + char c = className.charAt(i); + if (!(Character.isJavaIdentifierPart(c) || c == '.')) { + throw invalidClassName(className); + } + } } + /** @see #isA(TypeNode, String) */ public static boolean isA(TypeNode n, Class<?> clazz) { return subclasses(n, clazz); @@ -141,7 +240,7 @@ public final class TypeHelper { Class<?> type = vnd.getType(); for (final Class<?> clazz : clazzes) { if (type != null && type.equals(clazz) || type == null - && (clazz.getSimpleName().equals(vnd.getTypeImage()) || clazz.getName().equals(vnd.getTypeImage()))) { + && (clazz.getSimpleName().equals(vnd.getTypeImage()) || clazz.getName().equals(vnd.getTypeImage()))) { return true; } } @@ -191,7 +290,7 @@ public final class TypeHelper { public static boolean isA(TypedNameDeclaration vnd, String className) { Class<?> type = vnd.getType(); if (type != null) { - Class<?> clazz = loadClass(type.getClassLoader(), className); + Class<?> clazz = loadClass(ClassLoaderWrapper.wrapNullable(type.getClassLoader()), className); if (clazz != null) { return clazz.isAssignableFrom(type); } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/internal/NullableClassLoader.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/internal/NullableClassLoader.java new file mode 100644 index 0000000000..b7ba7d8b89 --- /dev/null +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/internal/NullableClassLoader.java @@ -0,0 +1,49 @@ +/* + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.java.typeresolution.internal; + +import net.sourceforge.pmd.lang.java.typeresolution.PMDASMClassLoader; + +/** + * A classloader that doesn't throw a {@link ClassNotFoundException} + * to report unresolved classes. This is a big performance improvement + * for {@link PMDASMClassLoader}, which caches negative cases. + * + * <p>See https://github.com/pmd/pmd/pull/2236 + */ +public interface NullableClassLoader { + + /** + * Load a class from its binary name. Returns null if not found. + */ + Class<?> loadClassOrNull(String binaryName); + + + class ClassLoaderWrapper implements NullableClassLoader { + + private final ClassLoader classLoader; + + private ClassLoaderWrapper(ClassLoader classLoader) { + assert classLoader != null : "Null classloader"; + this.classLoader = classLoader; + } + + @Override + public Class<?> loadClassOrNull(String binaryName) { + try { + return classLoader.loadClass(binaryName); + } catch (ClassNotFoundException e) { + return null; + } + } + + public static ClassLoaderWrapper wrapNullable(ClassLoader classLoader) { + if (classLoader == null) { + classLoader = ClassLoader.getSystemClassLoader(); + } + return new ClassLoaderWrapper(classLoader); + } + } +} diff --git a/pmd-java/src/main/resources/category/java/bestpractices.xml b/pmd-java/src/main/resources/category/java/bestpractices.xml index 48d119a350..b77d84b560 100644 --- a/pmd-java/src/main/resources/category/java/bestpractices.xml +++ b/pmd-java/src/main/resources/category/java/bestpractices.xml @@ -1409,16 +1409,21 @@ This rule detects JUnit assertions in object equality. These assertions should b PrimaryPrefix/Name[@Image = 'assertTrue'] ][ PrimarySuffix/Arguments/ArgumentList/Expression/PrimaryExpression/PrimaryPrefix/Name - [ends-with(@Image, '.equals')] + [ends-with(@Image, '.equals')] ] -[ancestor::ClassOrInterfaceDeclaration[//ClassOrInterfaceType[pmd-java:typeIs('junit.framework.TestCase')] - or //MarkerAnnotation/Name[ +[ancestor::ClassOrInterfaceDeclaration[ + pmd-java:typeIs('junit.framework.TestCase') + or .//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') - or pmd-java:typeIs('org.junit.jupiter.api.TestFactory') or pmd-java:typeIs('org.junit.jupiter.api.TestTemplate') + or pmd-java:typeIs('org.junit.jupiter.api.Test') + or pmd-java:typeIs('org.junit.jupiter.api.RepeatedTest') + or pmd-java:typeIs('org.junit.jupiter.api.TestFactory') + or pmd-java:typeIs('org.junit.jupiter.api.TestTemplate') or pmd-java:typeIs('org.junit.jupiter.params.ParameterizedTest') + ] ] -]]]]> +] +]]> </value> </property> </properties> @@ -1451,20 +1456,25 @@ more specific methods, like assertNull, assertNotNull. <value> <![CDATA[ //PrimaryExpression[ - PrimaryPrefix/Name[@Image = 'assertTrue' or @Image = 'assertFalse'] + PrimaryPrefix/Name[@Image = 'assertTrue' or @Image = 'assertFalse'] ][ - PrimarySuffix/Arguments/ArgumentList[ - Expression/EqualityExpression/PrimaryExpression/PrimaryPrefix/Literal/NullLiteral - ] -] -[ancestor::ClassOrInterfaceDeclaration[//ClassOrInterfaceType[pmd-java:typeIs('junit.framework.TestCase')] - or //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') - or pmd-java:typeIs('org.junit.jupiter.api.TestFactory') or pmd-java:typeIs('org.junit.jupiter.api.TestTemplate') - or pmd-java:typeIs('org.junit.jupiter.params.ParameterizedTest') + PrimarySuffix/Arguments/ArgumentList[ + Expression/EqualityExpression/PrimaryExpression/PrimaryPrefix/Literal/NullLiteral ] -]]]]> +] +[ancestor::ClassOrInterfaceDeclaration[ + pmd-java:typeIs('junit.framework.TestCase') + or .//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') + or pmd-java:typeIs('org.junit.jupiter.api.TestFactory') + or pmd-java:typeIs('org.junit.jupiter.api.TestTemplate') + or pmd-java:typeIs('org.junit.jupiter.params.ParameterizedTest') + ] + ] +] +]]> </value> </property> </properties> @@ -1499,20 +1509,25 @@ by more specific methods, like assertSame, assertNotSame. <value> <![CDATA[ //PrimaryExpression[ - PrimaryPrefix/Name - [@Image = 'assertTrue' or @Image = 'assertFalse'] + PrimaryPrefix/Name[@Image = 'assertTrue' or @Image = 'assertFalse'] ] -[PrimarySuffix/Arguments - /ArgumentList/Expression - /EqualityExpression[count(.//NullLiteral) = 0]] -[ancestor::ClassOrInterfaceDeclaration[//ClassOrInterfaceType[pmd-java:typeIs('junit.framework.TestCase')] - or //MarkerAnnotation/Name[ +[ + PrimarySuffix/Arguments/ArgumentList/Expression/EqualityExpression + [count(.//NullLiteral) = 0] +] +[ancestor::ClassOrInterfaceDeclaration[ + pmd-java:typeIs('junit.framework.TestCase') + or .//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') - or pmd-java:typeIs('org.junit.jupiter.api.TestFactory') or pmd-java:typeIs('org.junit.jupiter.api.TestTemplate') + or pmd-java:typeIs('org.junit.jupiter.api.Test') + or pmd-java:typeIs('org.junit.jupiter.api.RepeatedTest') + or pmd-java:typeIs('org.junit.jupiter.api.TestFactory') + or pmd-java:typeIs('org.junit.jupiter.api.TestTemplate') or pmd-java:typeIs('org.junit.jupiter.params.ParameterizedTest') + ] ] -]]]]> +] +]]> </value> </property> </properties> diff --git a/pmd-java/src/main/resources/category/java/codestyle.xml b/pmd-java/src/main/resources/category/java/codestyle.xml index 946b0268e9..c8e5db43f8 100644 --- a/pmd-java/src/main/resources/category/java/codestyle.xml +++ b/pmd-java/src/main/resources/category/java/codestyle.xml @@ -1632,18 +1632,21 @@ which makes the code also more readable. </description> <priority>3</priority> <properties> + <property name="version" value="2.0" /> <property name="xpath"> <value> <![CDATA[ -//VariableInitializer[preceding-sibling::VariableDeclaratorId[1]/@TypeInferred="false"] -//PrimaryExpression[not(PrimarySuffix)] -[not(ancestor::ArgumentList)] -/PrimaryPrefix/AllocationExpression[ClassOrInterfaceType[@AnonymousClass='false']/TypeArguments//ReferenceType[not(.//TypeArguments)]] +( +//VariableInitializer[preceding-sibling::VariableDeclaratorId[1]/@TypeInferred=false()] | -//StatementExpression[AssignmentOperator][PrimaryExpression/PrimaryPrefix[not(Expression)]] -//PrimaryExpression[not(PrimarySuffix)] -[not(ancestor::ArgumentList)] -/PrimaryPrefix/AllocationExpression[ClassOrInterfaceType[@AnonymousClass='false']/TypeArguments//ReferenceType[not(.//TypeArguments)]] +//StatementExpression[AssignmentOperator and PrimaryExpression/PrimaryPrefix[not(Expression)]] +) +/Expression/PrimaryExpression[not(PrimarySuffix) and not(ancestor::ArgumentList)] +/PrimaryPrefix +/AllocationExpression + [@AnonymousClass=false()] + [ClassOrInterfaceType/TypeArguments[@Diamond=false() and not(TypeArgument//TypeArguments)]] + [not(ArrayDimsAndInits)] ]]> </value> </property> diff --git a/pmd-java/src/main/resources/category/java/design.xml b/pmd-java/src/main/resources/category/java/design.xml index 84e6c2a9ba..60ef41f8b9 100644 --- a/pmd-java/src/main/resources/category/java/design.xml +++ b/pmd-java/src/main/resources/category/java/design.xml @@ -27,7 +27,7 @@ protected constructor in order to prevent instantiation than make the class misl <![CDATA[ //ClassOrInterfaceDeclaration [@Abstract = 'true'] - [count(//MethodDeclaration) + count(//ConstructorDeclaration) = 0] + [count(./ClassOrInterfaceBody/*/MethodDeclaration) + count(./ClassOrInterfaceBody/*/ConstructorDeclaration) = 0] [not(../Annotation/MarkerAnnotation/Name[pmd-java:typeIs('com.google.auto.value.AutoValue')])] ]]> </value> @@ -1088,20 +1088,24 @@ as: <![CDATA[ //StatementExpression [ -.//Name[@Image='assertTrue' or @Image='assertFalse'] -and -PrimaryExpression/PrimarySuffix/Arguments/ArgumentList - /Expression/UnaryExpressionNotPlusMinus[@Image='!'] -/PrimaryExpression/PrimaryPrefix + .//Name[@Image='assertTrue' or @Image='assertFalse'] + and + PrimaryExpression/PrimarySuffix/Arguments/ArgumentList/Expression/UnaryExpressionNotPlusMinus[@Image='!'] + /PrimaryExpression/PrimaryPrefix ] -[ancestor::ClassOrInterfaceDeclaration[//ClassOrInterfaceType[pmd-java:typeIs('junit.framework.TestCase')] - or //MarkerAnnotation/Name[ +[ancestor::ClassOrInterfaceDeclaration[ + pmd-java:typeIs('junit.framework.TestCase') + or .//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') - or pmd-java:typeIs('org.junit.jupiter.api.TestFactory') or pmd-java:typeIs('org.junit.jupiter.api.TestTemplate') + or pmd-java:typeIs('org.junit.jupiter.api.Test') + or pmd-java:typeIs('org.junit.jupiter.api.RepeatedTest') + or pmd-java:typeIs('org.junit.jupiter.api.TestFactory') + or pmd-java:typeIs('org.junit.jupiter.api.TestTemplate') or pmd-java:typeIs('org.junit.jupiter.params.ParameterizedTest') + ] ] -]]]]> +] +]]> </value> </property> </properties> diff --git a/pmd-java/src/main/resources/category/java/errorprone.xml b/pmd-java/src/main/resources/category/java/errorprone.xml index edc4995f84..4d416a7785 100644 --- a/pmd-java/src/main/resources/category/java/errorprone.xml +++ b/pmd-java/src/main/resources/category/java/errorprone.xml @@ -2157,14 +2157,19 @@ Some JUnit framework methods are easy to misspell. or (not(@Name = 'tearDown') and translate(@Name, 'TEARdOWN', 'tearDown') = 'tearDown')] [@Arity = 0] -[ancestor::ClassOrInterfaceDeclaration[//ClassOrInterfaceType[pmd-java:typeIs('junit.framework.TestCase')] - or //MarkerAnnotation/Name[ +[ancestor::ClassOrInterfaceDeclaration[ + pmd-java:typeIs('junit.framework.TestCase') + or .//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') - or pmd-java:typeIs('org.junit.jupiter.api.TestFactory') or pmd-java:typeIs('org.junit.jupiter.api.TestTemplate') + or pmd-java:typeIs('org.junit.jupiter.api.Test') + or pmd-java:typeIs('org.junit.jupiter.api.RepeatedTest') + or pmd-java:typeIs('org.junit.jupiter.api.TestFactory') + or pmd-java:typeIs('org.junit.jupiter.api.TestTemplate') or pmd-java:typeIs('org.junit.jupiter.params.ParameterizedTest') + ] ] -]]]]> +] +]]> </value> </property> </properties> @@ -2197,14 +2202,19 @@ The suite() method in a JUnit test needs to be both public and static. //MethodDeclaration[not(@Static='true') or not(@Public='true')] [@Name='suite'] [@Arity = 0] -[ancestor::ClassOrInterfaceDeclaration[//ClassOrInterfaceType[pmd-java:typeIs('junit.framework.TestCase')] - or //MarkerAnnotation/Name[ +[ancestor::ClassOrInterfaceDeclaration[ + pmd-java:typeIs('junit.framework.TestCase') + or .//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') - or pmd-java:typeIs('org.junit.jupiter.api.TestFactory') or pmd-java:typeIs('org.junit.jupiter.api.TestTemplate') + or pmd-java:typeIs('org.junit.jupiter.api.Test') + or pmd-java:typeIs('org.junit.jupiter.api.RepeatedTest') + or pmd-java:typeIs('org.junit.jupiter.api.TestFactory') + or pmd-java:typeIs('org.junit.jupiter.api.TestTemplate') or pmd-java:typeIs('org.junit.jupiter.params.ParameterizedTest') + ] ] -]]]]> +] +]]> </value> </property> </properties> @@ -3176,22 +3186,28 @@ an error, use the fail() method and provide an indication message of why it did. <![CDATA[ //StatementExpression [ -PrimaryExpression/PrimaryPrefix/Name[@Image='assertTrue' or @Image='assertFalse'] -and -PrimaryExpression/PrimarySuffix/Arguments/ArgumentList/Expression -[PrimaryExpression/PrimaryPrefix/Literal/BooleanLiteral -or -UnaryExpressionNotPlusMinus[@Image='!'] -/PrimaryExpression/PrimaryPrefix[Literal/BooleanLiteral or Name[count(../../*)=1]]] -] -[ancestor::ClassOrInterfaceDeclaration[//ClassOrInterfaceType[pmd-java:typeIs('junit.framework.TestCase')] - or //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') - or pmd-java:typeIs('org.junit.jupiter.api.TestFactory') or pmd-java:typeIs('org.junit.jupiter.api.TestTemplate') - or pmd-java:typeIs('org.junit.jupiter.params.ParameterizedTest') + PrimaryExpression/PrimaryPrefix/Name[@Image='assertTrue' or @Image='assertFalse'] + and + PrimaryExpression/PrimarySuffix/Arguments/ArgumentList/Expression[ + PrimaryExpression/PrimaryPrefix/Literal/BooleanLiteral + or + UnaryExpressionNotPlusMinus[@Image='!'] + /PrimaryExpression/PrimaryPrefix[Literal/BooleanLiteral or Name[count(../../*)=1]] ] -]]]]> +] +[ancestor::ClassOrInterfaceDeclaration[ + pmd-java:typeIs('junit.framework.TestCase') + or .//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') + or pmd-java:typeIs('org.junit.jupiter.api.TestFactory') + or pmd-java:typeIs('org.junit.jupiter.api.TestTemplate') + or pmd-java:typeIs('org.junit.jupiter.params.ParameterizedTest') + ] + ] +] +]]> </value> </property> </properties> diff --git a/pmd-java/src/main/resources/category/java/multithreading.xml b/pmd-java/src/main/resources/category/java/multithreading.xml index 0d1790f7e9..5401725608 100644 --- a/pmd-java/src/main/resources/category/java/multithreading.xml +++ b/pmd-java/src/main/resources/category/java/multithreading.xml @@ -172,12 +172,12 @@ Explicitly calling Thread.run() method will execute in the caller's thread of co //StatementExpression/PrimaryExpression [ PrimaryPrefix + [pmd-java:typeIs('java.lang.Thread')] [ - ./Name[ends-with(@Image, '.run') or @Image = 'run'] - and substring-before(Name/@Image, '.') =//VariableDeclarator/VariableDeclaratorId/@Image - [../../../Type/ReferenceType/ClassOrInterfaceType[pmd-java:typeIs('java.lang.Thread')]] - or (./AllocationExpression/ClassOrInterfaceType[pmd-java:typeIs('java.lang.Thread')] - and ../PrimarySuffix[@Image = 'run']) + ./Name[ends-with(@Image, '.run') or @Image = 'run'] + or + ./AllocationExpression/ClassOrInterfaceType[pmd-java:typeIs('java.lang.Thread')] + and ../PrimarySuffix[@Image = 'run'] ] ] ]]> diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/Java14Test.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/Java14Test.java new file mode 100644 index 0000000000..c9c55642b5 --- /dev/null +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/Java14Test.java @@ -0,0 +1,204 @@ +/* + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + + +package net.sourceforge.pmd.lang.java.ast; + +import static org.hamcrest.CoreMatchers.instanceOf; +import static org.junit.Assert.assertThat; + +import java.util.ArrayList; +import java.util.List; + +import org.junit.Assert; +import org.junit.Test; + +import net.sourceforge.pmd.lang.ast.ParseException; +import net.sourceforge.pmd.lang.java.JavaParsingHelper; + +/** + * Tests new java14 standard features. + */ +public class Java14Test { + private final JavaParsingHelper java14 = + JavaParsingHelper.WITH_PROCESSING.withDefaultVersion("14") + .withResourceContext(Java14Test.class, "jdkversiontests/java14/"); + + private final JavaParsingHelper java14p = java14.withDefaultVersion("14-preview"); + private final JavaParsingHelper java13 = java14.withDefaultVersion("13"); + private final JavaParsingHelper java13p = java14.withDefaultVersion("13-preview"); + + /** + * Tests switch expressions with yield. + * The switch expressions have no changed between java 13-preview and 14, so behave exactly the same. + */ + @Test + public void switchExpressions() { + parseAndCheckSwitchExpression(java13p); + parseAndCheckSwitchExpression(java14); + parseAndCheckSwitchExpression(java14p); + } + + /** + * In java13, switch expressions are only available with preview. + */ + @Test(expected = ParseException.class) + public void switchExpressions13ShouldFail() { + parseAndCheckSwitchExpression(java13); + } + + private void parseAndCheckSwitchExpression(JavaParsingHelper parser) { + ASTCompilationUnit compilationUnit = parser.parseResource("SwitchExpressions.java"); + List<ASTSwitchStatement> switchStatements = compilationUnit.findDescendantsOfType(ASTSwitchStatement.class); + Assert.assertEquals(2, switchStatements.size()); + + Assert.assertTrue(switchStatements.get(0).getChild(0) instanceof ASTExpression); + Assert.assertTrue(switchStatements.get(0).getChild(1) instanceof ASTSwitchLabeledExpression); + Assert.assertTrue(switchStatements.get(0).getChild(1).getChild(0) instanceof ASTSwitchLabel); + Assert.assertEquals(3, switchStatements.get(0).getChild(1).getChild(0).getNumChildren()); + Assert.assertTrue(switchStatements.get(0).getChild(2).getChild(0) instanceof ASTSwitchLabel); + Assert.assertFalse(((ASTSwitchLabel) switchStatements.get(0).getChild(2).getChild(0)).isDefault()); + Assert.assertEquals(1, switchStatements.get(0).getChild(2).getChild(0).getNumChildren()); + + Assert.assertTrue(switchStatements.get(1).getChild(3) instanceof ASTSwitchLabeledExpression); + Assert.assertTrue(switchStatements.get(1).getChild(3).getChild(0) instanceof ASTSwitchLabel); + Assert.assertTrue(((ASTSwitchLabel) switchStatements.get(1).getChild(3).getChild(0)).isDefault()); + + List<ASTSwitchExpression> switchExpressions = compilationUnit.findDescendantsOfType(ASTSwitchExpression.class); + Assert.assertEquals(4, switchExpressions.size()); + + Assert.assertEquals(Integer.TYPE, switchExpressions.get(0).getType()); + Assert.assertEquals(4, switchExpressions.get(0).findChildrenOfType(ASTSwitchLabeledExpression.class).size()); + Assert.assertEquals(Integer.TYPE, switchExpressions.get(0).getFirstChildOfType(ASTSwitchLabeledExpression.class) + .getFirstChildOfType(ASTExpression.class).getType()); + + Assert.assertTrue(switchExpressions.get(1).getChild(3) instanceof ASTSwitchLabeledBlock); + + Assert.assertEquals(Integer.TYPE, switchExpressions.get(2).getType()); + List<ASTYieldStatement> yields = switchExpressions.get(2).findDescendantsOfType(ASTYieldStatement.class); + Assert.assertEquals(4, yields.size()); + Assert.assertEquals("SwitchExpressions.BAZ", yields.get(2).getImage()); + + Assert.assertEquals(String.class, switchExpressions.get(3).getType()); + } + + @Test + public void checkYieldConditionalBehaviour() { + checkYieldStatements(java13p); + } + + @Test + public void checkYieldConditionalBehaviourJ14() { + checkYieldStatements(java14); + } + + private void checkYieldStatements(JavaParsingHelper parser) { + ASTCompilationUnit compilationUnit = parser.parseResource("YieldStatements.java"); + List<ASTBlockStatement> blockStmts = compilationUnit.findDescendantsOfType(ASTBlockStatement.class); + List<JavaNode> stmts = new ArrayList<>(); + // fetch the interesting node, on the java-grammar branch this is not needed + for (int i = 0; i < blockStmts.size(); i++) { + JavaNode child = blockStmts.get(i).getChild(0); + + if (child instanceof ASTStatement) { + stmts.add(child.getChild(0)); + } else { + stmts.add(child); + } + } + + Assert.assertEquals(18, stmts.size()); + + int i = 0; + assertThat(stmts.get(i++), instanceOf(ASTLocalVariableDeclaration.class)); + assertThat(stmts.get(i++), instanceOf(ASTStatementExpression.class)); + assertThat(stmts.get(i++), instanceOf(ASTStatementExpression.class)); + assertThat(stmts.get(i++), instanceOf(ASTStatementExpression.class)); + + assertThat(stmts.get(i++), instanceOf(ASTStatementExpression.class)); + + assertThat(stmts.get(i++), instanceOf(ASTStatementExpression.class)); + assertThat(stmts.get(i++), instanceOf(ASTStatementExpression.class)); + assertThat(stmts.get(i++), instanceOf(ASTYieldStatement.class)); + assertThat(stmts.get(i++), instanceOf(ASTYieldStatement.class)); + assertThat(stmts.get(i++), instanceOf(ASTYieldStatement.class)); + assertThat(stmts.get(i++), instanceOf(ASTStatementExpression.class)); + assertThat(stmts.get(i++), instanceOf(ASTStatementExpression.class)); + + assertThat(stmts.get(i++), instanceOf(ASTIfStatement.class)); + + assertThat(stmts.get(i++), instanceOf(ASTStatementExpression.class)); + assertThat(stmts.get(i++), instanceOf(ASTYieldStatement.class)); + + assertThat(stmts.get(i++), instanceOf(ASTYieldStatement.class)); + assertThat(stmts.get(i++), instanceOf(ASTStatementExpression.class)); + assertThat(stmts.get(i++), instanceOf(ASTYieldStatement.class)); + + Assert.assertEquals(i, stmts.size()); + } + + @Test + public void multipleCaseLabels() { + multipleCaseLabels(java13p); + multipleCaseLabels(java14); + multipleCaseLabels(java14p); + } + + private void multipleCaseLabels(JavaParsingHelper parser) { + ASTCompilationUnit compilationUnit = parser.parseResource("MultipleCaseLabels.java"); + ASTSwitchStatement switchStatement = compilationUnit.getFirstDescendantOfType(ASTSwitchStatement.class); + Assert.assertTrue(switchStatement.getChild(0) instanceof ASTExpression); + Assert.assertTrue(switchStatement.getChild(1) instanceof ASTSwitchLabel); + ASTSwitchLabel switchLabel = switchStatement.getFirstChildOfType(ASTSwitchLabel.class); + Assert.assertEquals(3, switchLabel.findChildrenOfType(ASTExpression.class).size()); + } + + @Test + public void switchRules() { + switchRules(java13p); + switchRules(java14); + switchRules(java14p); + } + + private void switchRules(JavaParsingHelper parser) { + ASTCompilationUnit compilationUnit = parser.parseResource("SwitchRules.java"); + ASTSwitchStatement switchStatement = compilationUnit.getFirstDescendantOfType(ASTSwitchStatement.class); + Assert.assertTrue(switchStatement.getChild(0) instanceof ASTExpression); + Assert.assertTrue(switchStatement.getChild(1) instanceof ASTSwitchLabeledExpression); + ASTSwitchLabeledExpression switchLabeledExpression = (ASTSwitchLabeledExpression) switchStatement.getChild(1); + Assert.assertEquals(2, switchLabeledExpression.getNumChildren()); + Assert.assertTrue(switchLabeledExpression.getChild(0) instanceof ASTSwitchLabel); + Assert.assertTrue(switchLabeledExpression.getChild(1) instanceof ASTExpression); + + ASTSwitchLabeledBlock switchLabeledBlock = (ASTSwitchLabeledBlock) switchStatement.getChild(4); + Assert.assertEquals(2, switchLabeledBlock.getNumChildren()); + Assert.assertTrue(switchLabeledBlock.getChild(0) instanceof ASTSwitchLabel); + Assert.assertTrue(switchLabeledBlock.getChild(1) instanceof ASTBlock); + + ASTSwitchLabeledThrowStatement switchLabeledThrowStatement = (ASTSwitchLabeledThrowStatement) switchStatement.getChild(5); + Assert.assertEquals(2, switchLabeledThrowStatement.getNumChildren()); + Assert.assertTrue(switchLabeledThrowStatement.getChild(0) instanceof ASTSwitchLabel); + Assert.assertTrue(switchLabeledThrowStatement.getChild(1) instanceof ASTThrowStatement); + } + + @Test + public void simpleSwitchExpressions() { + simpleSwitchExpressions(java13p); + simpleSwitchExpressions(java14); + simpleSwitchExpressions(java14p); + } + + private void simpleSwitchExpressions(JavaParsingHelper parser) { + ASTCompilationUnit compilationUnit = parser.parseResource("SimpleSwitchExpressions.java"); + ASTSwitchExpression switchExpression = compilationUnit.getFirstDescendantOfType(ASTSwitchExpression.class); + Assert.assertEquals(6, switchExpression.getNumChildren()); + Assert.assertTrue(switchExpression.getChild(0) instanceof ASTExpression); + Assert.assertEquals(5, switchExpression.findChildrenOfType(ASTSwitchLabeledRule.class).size()); + + ASTLocalVariableDeclaration localVar = compilationUnit.findDescendantsOfType(ASTLocalVariableDeclaration.class).get(1); + ASTVariableDeclarator localVarDecl = localVar.getFirstChildOfType(ASTVariableDeclarator.class); + Assert.assertEquals(Integer.TYPE, localVarDecl.getType()); + Assert.assertEquals(Integer.TYPE, switchExpression.getType()); + } +} diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/typeresolution/TypeHelperTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/typeresolution/TypeHelperTest.java index c158e669dc..e0a744135d 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/typeresolution/TypeHelperTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/typeresolution/TypeHelperTest.java @@ -4,19 +4,30 @@ package net.sourceforge.pmd.lang.java.typeresolution; +import static net.sourceforge.pmd.lang.java.typeresolution.internal.NullableClassLoader.ClassLoaderWrapper.wrapNullable; + import java.io.Serializable; import java.lang.annotation.Annotation; +import java.util.Map; import org.junit.Assert; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import net.sourceforge.pmd.lang.java.ast.ASTAnnotationTypeDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTEnumDeclaration; +import net.sourceforge.pmd.lang.java.ast.TypeNode; import net.sourceforge.pmd.lang.java.symboltable.BaseNonParserTest; +import net.sourceforge.pmd.lang.java.typeresolution.internal.NullableClassLoader; public class TypeHelperTest extends BaseNonParserTest { + private static final NullableClassLoader LOADER = wrapNullable(TypeHelperTest.class.getClassLoader()); + + @Rule + public final ExpectedException expect = ExpectedException.none(); @Test public void testIsAFallback() { @@ -46,11 +57,10 @@ public class TypeHelperTest extends BaseNonParserTest { Assert.assertNull(klass.getType()); Assert.assertTrue(TypeHelper.isA(klass, "org.FooBar")); - Assert.assertTrue(TypeHelper.isA(klass, "java.lang.Iterable")); - Assert.assertTrue(TypeHelper.isA(klass, Iterable.class)); - Assert.assertTrue(TypeHelper.isA(klass, Enum.class)); - Assert.assertTrue(TypeHelper.isA(klass, Serializable.class)); - Assert.assertTrue(TypeHelper.isA(klass, Comparable.class)); + assertIsA(klass, Iterable.class); + assertIsA(klass, Enum.class); + assertIsA(klass, Serializable.class); + assertIsA(klass, Object.class); } @Test @@ -64,8 +74,58 @@ public class TypeHelperTest extends BaseNonParserTest { Assert.assertNull(klass.getType()); Assert.assertTrue(TypeHelper.isA(klass, "org.FooBar")); - Assert.assertTrue(TypeHelper.isA(klass, Annotation.class)); + assertIsA(klass, Annotation.class); + assertIsA(klass, Object.class); + } + + private void assertIsA(TypeNode node, Class<?> type) { + Assert.assertTrue("TypeHelper::isA with class arg: " + type.getCanonicalName(), + TypeHelper.isA(node, type)); + Assert.assertTrue("TypeHelper::isA with string arg: " + type.getCanonicalName(), + TypeHelper.isA(node, type.getCanonicalName())); + } + + private void assertIsExactlyA(TypeNode node, Class<?> type) { + Assert.assertTrue(TypeHelper.isExactlyA(node, type.getCanonicalName())); + assertIsA(node, type); } + @Test + public void testNestedClass() { + Class<?> c = TypeHelper.loadClass(LOADER, "java.util.Map.Entry"); + Assert.assertEquals(Map.Entry.class, c); + } + + + @Test + public void testPrimitiveArray() { + Class<?> c = TypeHelper.loadClass(LOADER, "int[ ]"); + Assert.assertEquals(int[].class, c); + } + + @Test + public void testNestedClassArray() { + Class<?> c = TypeHelper.loadClass(LOADER, "java.util.Map.Entry[ ]"); + Assert.assertEquals(Map.Entry[].class, c); + } + + @Test + public void testInvalidName() { + expect.expect(IllegalArgumentException.class); + TypeHelper.loadClass(LOADER, "java.util.Map ]"); + } + + @Test + public void testInvalidName2() { + expect.expect(IllegalArgumentException.class); + TypeHelper.loadClass(LOADER, "[]"); + } + + @Test + public void testNullName() { + expect.expect(NullPointerException.class); + TypeHelper.loadClass(LOADER, null); + } + } diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/ast/jdkversiontests/java14/YieldStatements.java b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/ast/jdkversiontests/java14/YieldStatements.java new file mode 100644 index 0000000000..f56e25646f --- /dev/null +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/ast/jdkversiontests/java14/YieldStatements.java @@ -0,0 +1,35 @@ +/** + * @see <a href="https://openjdk.java.net/jeps/361">JEP 361: Switch Expressions (Standard)</a> + */ +public class YieldStatements { + { + int yield = 0; + yield = 2; // should be an assignment + yield (2); // should be a method call + yield(a,b); // should be a method call + + + yield = switch (e) { // must be a switch expr + case 1 -> { + yield(a,b); // should be a method call + yield = 2; // should be an assignment + yield (2); // should be a yield statement + yield++bar; // should be a yield statement (++bar is an expression) + yield--bar; // should be a yield statement (--bar is an expression) + yield++; // should be an increment (not an error) + yield--; // should be a decrement (not an error) + + if (true) yield(2); + else yield 4; + + yield = switch (foo) { // putting a switch in the middles checks the reset behavior + case 4 -> {yield(5);} // should be a yield statement + }; + + yield () -> {}; // should be a yield statement + yield (); // should be a method call + yield (2); // should be a yield statement + } + }; + } +} diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/UseDiamondOperator.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/UseDiamondOperator.xml index 7d61c4a29a..b65723cb19 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/UseDiamondOperator.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/UseDiamondOperator.xml @@ -1,13 +1,12 @@ <?xml version="1.0" encoding="UTF-8"?> -<test-data - xmlns="http://pmd.sourceforge.net/rule-tests" - 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"> - <test-code> - <description>Use Diamond</description> - <expected-problems>2</expected-problems> - <expected-linenumbers>6,8</expected-linenumbers> - <code><![CDATA[ +<test-data xmlns="http://pmd.sourceforge.net/rule-tests" + 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"> + <test-code> + <description>Use Diamond</description> + <expected-problems>2</expected-problems> + <expected-linenumbers>6,11</expected-linenumbers> + <code><![CDATA[ import java.util.ArrayList; import java.util.List; public class Foo { @@ -15,17 +14,20 @@ public class Foo { public void foo() { List<String> strings = new ArrayList<String>(); List<String> strings2 = new ArrayList<>(); + List<List<String>> strings3 = new ArrayList<>(); + // this is a known false negative, see at the bottom + List<List<String>> strings4 = new ArrayList<List<List<String>>>(); this.field = new ArrayList<String>(); } } - ]]></code> - </test-code> - <test-code> - <description>False positive cases: anonymous classes, methods calls</description> - <expected-problems>0</expected-problems> - <code><![CDATA[ + ]]></code> + </test-code> + + <test-code> + <description>False positive cases: anonymous classes, methods calls</description> + <expected-problems>0</expected-problems> + <code><![CDATA[ public class Foo { - private WeakReference<Class<?>> typeReference; public void foo() { Collections.sort(files, new Comparator<DataSource>() { @Override @@ -49,8 +51,6 @@ public class Foo { } }; Iterator<Node> EMPTY_ITERATOR = new ArrayList<Node>().iterator(); - Class<?> type = null; - typeReference = new WeakReference<Class<?>>(type); ((ListNode<E>) rev).reverseCache = new SoftReference<ImmutableList<E>>(this); } public Map<PropertyDescriptor<?>, Object> getOverriddenPropertiesByPropertyDescriptor() { @@ -58,12 +58,13 @@ public class Foo { } } ]]></code> - </test-code> - <test-code> - <description>#1624[java] UseDiamondOperator doesn't work with var</description> - <expected-problems>1</expected-problems> - <expected-linenumbers>6</expected-linenumbers> - <code><![CDATA[ + </test-code> + + <test-code> + <description>#1624[java] UseDiamondOperator doesn't work with var</description> + <expected-problems>1</expected-problems> + <expected-linenumbers>6</expected-linenumbers> + <code><![CDATA[ import java.util.ArrayList; public class Buzz { public void buzz() { @@ -72,13 +73,14 @@ public class Buzz { f = new ArrayList<String>(); // flagged by rule } } - ]]></code> - </test-code> - <test-code> - <description>Multiple initializations in a single declaration</description> - <expected-problems>1</expected-problems> - <expected-linenumbers>6</expected-linenumbers> - <code><![CDATA[ + ]]></code> + </test-code> + + <test-code> + <description>Multiple initializations in a single declaration</description> + <expected-problems>1</expected-problems> + <expected-linenumbers>6</expected-linenumbers> + <code><![CDATA[ import java.util.ArrayList; import java.util.List; public class Buzz { @@ -88,6 +90,93 @@ public class Buzz { baz = new ArrayList<>(); // ok } } - ]]></code> - </test-code> -</test-data> + ]]></code> + </test-code> + + <test-code> + <description>#1723 FP with var inside lambda (declaration)</description> + <expected-problems>0</expected-problems> + <code><![CDATA[ +class Foo { + { + Runnable someAction = () -> { + var foo = new ArrayList<String>(5); // ok + System.err.println(foo); + }; + } +} + ]]></code> + </test-code> + + <test-code> + <description>#1723 FP with var inside lambda (assignment)</description> + <expected-problems>0</expected-problems> + <code><![CDATA[ +class Foo { + { + Runnable someAction; + someAction = () -> { + var foo = new ArrayList<String>(5); // ok + System.err.println(foo); + }; + } +} + ]]></code> + </test-code> + + <test-code> + <description>FP with array creation</description> + <expected-problems>0</expected-problems> + <code><![CDATA[ +class Foo { + { + Class<?> c = new Class<?>[0]; + } +} + ]]></code> + </test-code> + + <!-- These tests depend on the Java version used --> + <!-- For now we keep the old behaviour of ignoring type + arguments that have type arguments themselves, ie we have + false negatives. We can improve that with better type resolution + in PMD 7. --> + + <test-code regressionTest="false"> + <description>(J7) Version sensitive tests</description> + <expected-problems>1</expected-problems> + <expected-linenumbers>6</expected-linenumbers> + <code><![CDATA[ +public class Foo { + private WeakReference<Class<?>> typeReference; + public void foo() { + // this should be positive in Java 8, negative in Java 7 + // this is because in java 7, new WeakReference<>(String.class) types as WeakReference<Class<String>> + // which is incompatible with WeakReference<Class<?>>, whereas Java 8's type inference is better. + typeReference = new WeakReference<Class<?>>(String.class); + Class<?> type = null; + typeReference = new WeakReference<Class<?>>(type); // this should be positive on all versions + } +} + ]]></code> + <source-type>java 1.7</source-type> + </test-code> + + <test-code regressionTest="false"> + <description>(J8) Version sensitive tests</description> + <expected-problems>2</expected-problems> + <expected-linenumbers>4,6</expected-linenumbers> + <code><![CDATA[ +public class Foo { + private WeakReference<Class<?>> typeReference; + public void foo() { + typeReference = new WeakReference<Class<?>>(String.class); // pos + Class<?> type = null; + typeReference = new WeakReference<Class<?>>(type); // pos + } +} + ]]></code> + <source-type>java 1.8</source-type> + </test-code> + +</test-data> \ No newline at end of file diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/AbstractClassWithoutAnyMethod.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/AbstractClassWithoutAnyMethod.xml index 96a38ed646..a25b160013 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/AbstractClassWithoutAnyMethod.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/AbstractClassWithoutAnyMethod.xml @@ -76,4 +76,37 @@ import com.google.auto.value.AutoValue; } ]]></code> </test-code> + + + <test-code> + <description>FN with nested class</description> + <expected-problems>1</expected-problems> + <expected-linenumbers>1</expected-linenumbers> + <code><![CDATA[ +abstract class Foo { + + class Inner { + void ohio() {} + } + +} + ]]></code> + </test-code> + + <test-code> + <description>FN with sibling class</description> + <expected-problems>1</expected-problems> + <expected-linenumbers>3</expected-linenumbers> + <code><![CDATA[ +class Foo { + + abstract class Pos {} + + class Sibling { + void ohio() {} + } + +} + ]]></code> + </test-code> </test-data>