Merge branch '7.0.x' into java-grammar

This commit is contained in:
Clément Fournier
2020-03-28 15:28:34 +01:00
32 changed files with 358 additions and 85 deletions

View File

@ -1,20 +1,41 @@
---
name: Bug report
about: Create a report to help us improve
title: ''
labels: bug
assignees: ''
---
<!-- Please, prefix the report title with the language it applies to within brackets, such as [java] or [apex].
If not specific to a language, you can use [core]. -->
<!-- NB: issues about the rule designer should be opened at https://github.com/pmd/pmd-designer/issues -->
**Affects PMD Version:**
**Rule:**
Make sure, to test with the latest PMD version.
**Description:**
A clear and concise description of what the bug is.
**Exception Stacktrace:**
```
# Copy-paste the stack trace here
```
**Code Sample demonstrating the issue:**
```
```
**Steps to reproduce:**
Please provide detailed steps for how we can reproduce the bug.
1. ... (e.g. if you're using maven: `mvn clean verify`)
2. ...
**Running PMD through:** *[CLI | Ant | Maven | Gradle | Designer | Other]*
<!-- If relevant, also include your JDK and OS information, e.g. for ClassNotFoundException, LinkageError, reflection failures, etc. -->

8
.github/ISSUE_TEMPLATE/config.yml vendored Normal file
View File

@ -0,0 +1,8 @@
blank_issues_enabled: false
contact_links:
- name: PMD Designer Issues
url: https://github.com/pmd/pmd-designer/issues
about: Issues about the rule designer
- name: PMD Eclipse Plugin Issues
url: https://github.com/pmd/pmd-eclipse-plugin/issues
about: Issues about the PMD Eclipse Plugin

View File

@ -0,0 +1,20 @@
---
name: Feature request
about: Suggest an idea for this project
title: ''
labels: enhancement
assignees: ''
---
**Is your feature request related to a problem? Please describe.**
A clear and concise description of what the problem is. Ex. I'm always frustrated when [...]
**Describe the solution you'd like**
A clear and concise description of what you want to happen.
**Describe alternatives you've considered**
A clear and concise description of any alternative solutions or features you've considered.
**Additional context**
Add any other context about the feature request here.

27
.github/ISSUE_TEMPLATE/new_rule.md vendored Normal file
View File

@ -0,0 +1,27 @@
---
name: New Rule
about: You have an idea for a new rule? Great!
title: ''
labels: new-rule
assignees: ''
---
<!-- Please, prefix the report title with the language it applies to within brackets, such as [java] or [apex] -->
**Proposed Rule Name:**
**Proposed Category:** One of [Best Practices | Code Style | Design | Documentation | Error Prone | Multithreading | Performance | Security]
**Description:**
**Code Sample:** This should include code, that should be flagged by the rule. If possible, the "correct" code
according to this new rule should also be demonstrated.
```
```
**Possible Properties:**
* Should this rule be customizable via properties?

14
.github/ISSUE_TEMPLATE/question.md vendored Normal file
View File

@ -0,0 +1,14 @@
---
name: Question
about: Feel free to ask any question about PMD and its usage
title: ''
labels: question
assignees: ''
---
<!-- Have a look at https://github.com/pmd/pmd/issues?utf8=%E2%9C%93&q=label%3Aa%3Aquestion if there is already
a similar question -->
**Description:**

View File

@ -0,0 +1,32 @@
---
name: Rule violation
about: Let us know about a false positive/false negative
title: ''
labels: bug
assignees: ''
---
<!-- Please, prefix the report title with the language it applies to within brackets, such as [java] or [apex] -->
**Affects PMD Version:**
**Rule:**
Please provide the rule name and a link to the rule documentation:
<https://pmd.github.io/latest/pmd_rules_XXX_XXX.html#XXX>
**Description:**
**Code Sample demonstrating the issue:**
```
```
**Expected outcome:**
* Does PMD report a violation, where there shouldn't be one? -> false-positive
* Is PMD missing to report a violation, where there should be one? -> false-negative
**Running PMD through:** *[CLI | Ant | Maven | Gradle | Designer | Other]*

View File

@ -11,7 +11,7 @@ author: Tom Copeland <tom@infoether.org>
* March 2020 - [Helping Salesforce developers create readable and maintainable Apex code](https://gearset.com/blog/helping-sf-developers-create-readable-and-maintainable-apex-code)
* July 2019 - [Apex PMD | Static code analysis - Apex Hours](https://youtu.be/34PxAHtAavU)
* July 2019 - [Apex PMD \| Static code analysis - Apex Hours](https://youtu.be/34PxAHtAavU)
* June 2019 - [Pluralsight](https://www.pluralsight.com/authors/don-robins) Course about leveraging PMD usage for Salesforce by [Robert Sösemann](https://github.com/rsoesemann) (Apex Language Module Contributor) [Play by Play: Automated Code Analysis in Salesforce - a Tools Deep-Dive](https://www.pluralsight.com/courses/play-by-play-automated-code-analysis-in-salesforce)

View File

@ -2,7 +2,8 @@
title: Maven PMD Plugin
tags: [userdocs, tools]
permalink: pmd_userdocs_tools_maven.html
last_updated: August 2017
last_updated: March 2020
mpmd_version: 3.13.0
author: >
Miguel Griffa <mikkey@users.sourceforge.net>,
Romain PELISSE <belaran@gmail.com>,
@ -13,6 +14,36 @@ author: >
### Running the pmd plugin
#### Choosing the plugin version
When adding the maven-pmd-plugin to your pom.xml, you need to select a version. To figure out the
latest available version, have a look at the official [maven-pmd-plugin documentation](https://maven.apache.org/plugins/maven-pmd-plugin/).
As of {{ page.last_updated }}, the current plugin version is **{{ page.mpmd_version }}**.
The version of the plugin should be specified in `<build><pluginManagement/></build>` and if using the project
report additionally in `<reporting><plugins/></reporting>` elements. Here's an example for the pluginManagement
section:
```xml
<build>
<pluginManagement>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-pmd-plugin</artifactId>
<version>{{ page.mpmd_version }}</version>
</plugin>
</plugins>
</pluginManagement>
</build>
```
When defining the version in the pluginManagment section, then it doesn't need to be specified in the normal plugins
section. However, it should additionally be specified in the reporting section.
More information, see [Guide to Configuring Plugin-ins](https://maven.apache.org/guides/mini/guide-configuring-plugins.html).
#### Generating a project report
To include the PMD report in the project reports section add the following lines under
@ -26,6 +57,7 @@ the reports element in your pom.xml:
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-pmd-plugin</artifactId>
<version>{{ page.mpmd_version }}</version>
</plugin>
</plugins>
</reporting>
@ -58,8 +90,11 @@ PMD finds some violations. Therefore the `check` goal is used:
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-pmd-plugin</artifactId>
<version>{{ page.mpmd_version }}</version> <!-- or use version from pluginManagement -->
<configuration>
<failOnViolation>true</failOnViolation> <!-- this is actually true by default, but can be disabled -->
<!-- failOnViolation is actually true by default, but can be disabled -->
<failOnViolation>true</failOnViolation>
<!-- printFailingErrors is pretty useful -->
<printFailingErrors>true</printFailingErrors>
</configuration>
<executions>
@ -87,11 +122,13 @@ you add `cpd-check` as a goal.
To specify a ruleset, simply edit the previous configuration:
``` xml
<reporting>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-pmd-plugin</artifactId>
<version>{{ page.mpmd_version }}</version>
<configuration>
<rulesets>
<ruleset>/rulesets/java/quickstart.xml</ruleset>
@ -102,6 +139,7 @@ To specify a ruleset, simply edit the previous configuration:
</plugin>
</plugins>
</reporting>
```
The value of the 'ruleset' element can either be a relative address, an absolute address or even an url.
@ -117,23 +155,30 @@ will be able to resolve those other ruleset references.
When using the Maven PMD plugin 3.8 or later along with PMD 5.6.0 or later, you can enable incremental analysis to
speed up PMD's execution while retaining the quality of the analysis. You can additionally customize where the cache is stored::
```xml
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-pmd-plugin</artifactId>
<version>{{ page.mpmd_version }}</version> <!-- or use version from pluginManagement -->
<configuration>
<analysisCache>true</analysisCache> <!-- enable incremental analysis -->
<analysisCacheLocation>${project.build.directory}/pmd/pmd.cache</analysisCacheLocation> <!-- Optional: points to this location by default -->
<!-- enable incremental analysis -->
<analysisCache>true</analysisCache>
<!-- analysisCacheLocation: optional - points to the following location by default -->
<analysisCacheLocation>${project.build.directory}/pmd/pmd.cache</analysisCacheLocation>
</configuration>
</plugin>
```
#### Other configurations
The Maven PMD plugin allows you to configure CPD, targetJDK, and the use of XRef to link
the report to html source files, and the file encoding:
```xml
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-pmd-plugin</artifactId>
<version>{{ page.mpmd_version }}</version> <!-- or use version from pluginManagement -->
<configuration>
<linkXRef>true</linkXRef>
<sourceEncoding>ISO-8859-1</sourceEncoding>
@ -141,6 +186,7 @@ the report to html source files, and the file encoding:
<targetJdk>1.4</targetJdk>
</configuration>
</plugin>
```
#### Upgrading the PMD version at runtime
@ -162,7 +208,7 @@ Maven plugin will use and benefit from the latest bugfixes and enhancements:
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-pmd-plugin</artifactId>
<version>3.8</version>
<version>{{ page.mpmd_version }}</version>
<dependencies>
<dependency>
<groupId>net.sourceforge.pmd</groupId>

View File

@ -41,6 +41,7 @@ Note that XPath 1.0 support, the default XPath version, is deprecated since PMD
* apex
* [#2210](https://github.com/pmd/pmd/issues/2210): \[apex] ApexCRUDViolation: Support WITH SECURITY_ENFORCED
* [#2358](https://github.com/pmd/pmd/issues/2358): \[apex] Invalid Apex in Cognitive Complexity tests
### API Changes
@ -104,7 +105,9 @@ implementations, and their corresponding Parser if it exists (in the same packag
### External Contributions
* [#2312](https://github.com/pmd/pmd/pull/2312): \[apex] Update ApexCRUDViolation Rule - [Joshua S Arquilevich](https://github.com/jarquile)
* [#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)
{% endtocmaker %}

View File

@ -61,6 +61,7 @@ public abstract class AbstractApexNode<T extends AstNode> extends AbstractApexNo
return node;
}
@Override
public boolean hasRealLoc() {
try {
Location loc = node.getLoc();

View File

@ -50,4 +50,6 @@ public interface ApexNode<T extends AstNode> extends Node {
@Override
ApexNode<?> getParent();
boolean hasRealLoc();
}

View File

@ -18,7 +18,6 @@ import net.sourceforge.pmd.lang.apex.ast.ASTParameter;
import net.sourceforge.pmd.lang.apex.ast.ASTProperty;
import net.sourceforge.pmd.lang.apex.ast.ASTUserClass;
import net.sourceforge.pmd.lang.apex.ast.ASTUserInterface;
import net.sourceforge.pmd.lang.apex.ast.AbstractApexNode;
import net.sourceforge.pmd.lang.apex.ast.ApexNode;
import net.sourceforge.pmd.lang.apex.rule.AbstractApexRule;
@ -107,7 +106,7 @@ public class ApexDocRule extends AbstractApexRule {
return data;
}
private void handleClassOrInterface(AbstractApexNode<?> node, Object data) {
private void handleClassOrInterface(ApexNode<?> node, Object data) {
ApexDocComment comment = getApexDocComment(node);
if (comment == null) {
if (shouldHaveApexDocs(node)) {
@ -120,7 +119,7 @@ public class ApexDocRule extends AbstractApexRule {
}
}
private boolean shouldHaveApexDocs(AbstractApexNode<?> node) {
private boolean shouldHaveApexDocs(ApexNode<?> node) {
if (!node.hasRealLoc()) {
return false;
}

View File

@ -12,7 +12,7 @@ import net.sourceforge.pmd.lang.apex.ast.ASTProperty;
import net.sourceforge.pmd.lang.apex.ast.ASTUserClass;
import net.sourceforge.pmd.lang.apex.ast.ASTUserEnum;
import net.sourceforge.pmd.lang.apex.ast.ASTUserInterface;
import net.sourceforge.pmd.lang.apex.ast.AbstractApexNode;
import net.sourceforge.pmd.lang.apex.ast.ApexNode;
import net.sourceforge.pmd.lang.apex.rule.AbstractApexRule;
/**
@ -58,7 +58,7 @@ public class AvoidNonExistentAnnotationsRule extends AbstractApexRule {
return checkForNonExistentAnnotation(node, node.getModifiers(), data);
}
private Object checkForNonExistentAnnotation(final AbstractApexNode<?> node, final ASTModifierNode modifierNode, final Object data) {
private Object checkForNonExistentAnnotation(final ApexNode<?> node, final ASTModifierNode modifierNode, final Object data) {
for (ASTAnnotation annotation : modifierNode.findChildrenOfType(ASTAnnotation.class)) {
if (!annotation.isResolved()) {
addViolationWithMessage(data, node, "Use of non existent annotations will lead to broken Apex code which will not compile in the future.");

View File

@ -13,7 +13,7 @@ import net.sourceforge.pmd.lang.apex.ast.ASTMethodCallExpression;
import net.sourceforge.pmd.lang.apex.ast.ASTUserClass;
import net.sourceforge.pmd.lang.apex.ast.ASTVariableDeclaration;
import net.sourceforge.pmd.lang.apex.ast.ASTVariableExpression;
import net.sourceforge.pmd.lang.apex.ast.AbstractApexNode;
import net.sourceforge.pmd.lang.apex.ast.ApexNode;
import net.sourceforge.pmd.lang.apex.rule.AbstractApexRule;
import net.sourceforge.pmd.lang.apex.rule.internal.Helper;
@ -65,7 +65,7 @@ public class ApexBadCryptoRule extends AbstractApexRule {
return data;
}
private void findSafeVariables(AbstractApexNode<?> var) {
private void findSafeVariables(ApexNode<?> var) {
ASTMethodCallExpression methodCall = var.getFirstChildOfType(ASTMethodCallExpression.class);
if (methodCall != null && Helper.isMethodName(methodCall, BLOB, VALUE_OF)) {
ASTVariableExpression variable = var.getFirstChildOfType(ASTVariableExpression.class);

View File

@ -15,7 +15,7 @@ import net.sourceforge.pmd.lang.apex.ast.ASTLiteralExpression;
import net.sourceforge.pmd.lang.apex.ast.ASTMethodCallExpression;
import net.sourceforge.pmd.lang.apex.ast.ASTVariableDeclaration;
import net.sourceforge.pmd.lang.apex.ast.ASTVariableExpression;
import net.sourceforge.pmd.lang.apex.ast.AbstractApexNode;
import net.sourceforge.pmd.lang.apex.ast.ApexNode;
import net.sourceforge.pmd.lang.apex.rule.AbstractApexRule;
import net.sourceforge.pmd.lang.apex.rule.internal.Helper;
@ -50,7 +50,7 @@ public class ApexInsecureEndpointRule extends AbstractApexRule {
return data;
}
private void findInsecureEndpoints(AbstractApexNode<?> node) {
private void findInsecureEndpoints(ApexNode<?> node) {
ASTVariableExpression variableNode = node.getFirstChildOfType(ASTVariableExpression.class);
findInnerInsecureEndpoints(node, variableNode);
@ -61,7 +61,7 @@ public class ApexInsecureEndpointRule extends AbstractApexRule {
}
private void findInnerInsecureEndpoints(AbstractApexNode<?> node, ASTVariableExpression variableNode) {
private void findInnerInsecureEndpoints(ApexNode<?> node, ASTVariableExpression variableNode) {
ASTLiteralExpression literalNode = node.getFirstChildOfType(ASTLiteralExpression.class);
if (literalNode != null && variableNode != null) {
@ -94,7 +94,7 @@ public class ApexInsecureEndpointRule extends AbstractApexRule {
}
private void runChecks(AbstractApexNode<?> node, Object data) {
private void runChecks(ApexNode<?> node, Object data) {
ASTLiteralExpression literalNode = node.getFirstChildOfType(ASTLiteralExpression.class);
if (literalNode != null && literalNode.isString()) {
String literal = literalNode.getImage();

View File

@ -17,7 +17,6 @@ import net.sourceforge.pmd.lang.apex.ast.ASTNewObjectExpression;
import net.sourceforge.pmd.lang.apex.ast.ASTUserClass;
import net.sourceforge.pmd.lang.apex.ast.ASTVariableDeclaration;
import net.sourceforge.pmd.lang.apex.ast.ASTVariableExpression;
import net.sourceforge.pmd.lang.apex.ast.AbstractApexNode;
import net.sourceforge.pmd.lang.apex.ast.ApexNode;
import net.sourceforge.pmd.lang.apex.rule.AbstractApexRule;
import net.sourceforge.pmd.lang.apex.rule.internal.Helper;
@ -66,7 +65,7 @@ public class ApexOpenRedirectRule extends AbstractApexRule {
return data;
}
private void findSafeLiterals(AbstractApexNode<?> node) {
private void findSafeLiterals(ApexNode<?> node) {
ASTBinaryExpression binaryExp = node.getFirstChildOfType(ASTBinaryExpression.class);
if (binaryExp != null) {
findSafeLiterals(binaryExp);

View File

@ -23,7 +23,7 @@ import net.sourceforge.pmd.lang.apex.ast.ASTStandardCondition;
import net.sourceforge.pmd.lang.apex.ast.ASTUserClass;
import net.sourceforge.pmd.lang.apex.ast.ASTVariableDeclaration;
import net.sourceforge.pmd.lang.apex.ast.ASTVariableExpression;
import net.sourceforge.pmd.lang.apex.ast.AbstractApexNode;
import net.sourceforge.pmd.lang.apex.ast.ApexNode;
import net.sourceforge.pmd.lang.apex.rule.AbstractApexRule;
import net.sourceforge.pmd.lang.apex.rule.internal.Helper;
@ -118,7 +118,7 @@ public class ApexSOQLInjectionRule extends AbstractApexRule {
}
private void findSanitizedVariables(AbstractApexNode<?> node) {
private void findSanitizedVariables(ApexNode<?> node) {
final ASTVariableExpression left = node.getFirstChildOfType(ASTVariableExpression.class);
final ASTLiteralExpression literal = node.getFirstChildOfType(ASTLiteralExpression.class);
final ASTMethodCallExpression right = node.getFirstChildOfType(ASTMethodCallExpression.class);
@ -165,7 +165,7 @@ public class ApexSOQLInjectionRule extends AbstractApexRule {
}
}
private void findSelectContainingVariables(AbstractApexNode<?> node) {
private void findSelectContainingVariables(ApexNode<?> node) {
final ASTVariableExpression left = node.getFirstChildOfType(ASTVariableExpression.class);
final ASTBinaryExpression right = node.getFirstChildOfType(ASTBinaryExpression.class);
if (left != null && right != null) {

View File

@ -15,7 +15,7 @@ import net.sourceforge.pmd.lang.apex.ast.ASTMethodCallExpression;
import net.sourceforge.pmd.lang.apex.ast.ASTUserClass;
import net.sourceforge.pmd.lang.apex.ast.ASTVariableDeclaration;
import net.sourceforge.pmd.lang.apex.ast.ASTVariableExpression;
import net.sourceforge.pmd.lang.apex.ast.AbstractApexNode;
import net.sourceforge.pmd.lang.apex.ast.ApexNode;
import net.sourceforge.pmd.lang.apex.rule.AbstractApexRule;
import net.sourceforge.pmd.lang.apex.rule.internal.Helper;
@ -83,7 +83,7 @@ public class ApexSuggestUsingNamedCredRule extends AbstractApexRule {
}
private void findAuthLiterals(final AbstractApexNode<?> node) {
private void findAuthLiterals(final ApexNode<?> node) {
ASTLiteralExpression literal = node.getFirstChildOfType(ASTLiteralExpression.class);
if (literal != null) {
ASTVariableExpression variable = node.getFirstChildOfType(ASTVariableExpression.class);
@ -95,7 +95,7 @@ public class ApexSuggestUsingNamedCredRule extends AbstractApexRule {
}
}
private void runChecks(final AbstractApexNode<?> node, Object data) {
private void runChecks(final ApexNode<?> node, Object data) {
ASTLiteralExpression literalNode = node.getFirstChildOfType(ASTLiteralExpression.class);
if (literalNode != null) {
if (isAuthorizationLiteral(literalNode)) {

View File

@ -17,7 +17,7 @@ import net.sourceforge.pmd.lang.apex.ast.ASTReturnStatement;
import net.sourceforge.pmd.lang.apex.ast.ASTUserClass;
import net.sourceforge.pmd.lang.apex.ast.ASTVariableDeclaration;
import net.sourceforge.pmd.lang.apex.ast.ASTVariableExpression;
import net.sourceforge.pmd.lang.apex.ast.AbstractApexNode;
import net.sourceforge.pmd.lang.apex.ast.ApexNode;
import net.sourceforge.pmd.lang.apex.rule.AbstractApexRule;
import net.sourceforge.pmd.lang.apex.rule.internal.Helper;
@ -158,7 +158,7 @@ public class ApexXSSFromURLParamRule extends AbstractApexRule {
}
private void findTaintedVariables(AbstractApexNode<?> node, Object data) {
private void findTaintedVariables(ApexNode<?> node, Object data) {
final ASTMethodCallExpression right = node.getFirstChildOfType(ASTMethodCallExpression.class);
// Looks for: (String) foo =
// ApexPages.currentPage().getParameters().get(..)
@ -203,7 +203,7 @@ public class ApexXSSFromURLParamRule extends AbstractApexRule {
}
private void processVariableAssignments(AbstractApexNode<?> node, Object data, final boolean reverseOrder) {
private void processVariableAssignments(ApexNode<?> node, Object data, final boolean reverseOrder) {
ASTMethodCallExpression methodCallAssignment = node.getFirstChildOfType(ASTMethodCallExpression.class);
if (methodCallAssignment != null) {
@ -246,7 +246,7 @@ public class ApexXSSFromURLParamRule extends AbstractApexRule {
}
private void processBinaryExpression(AbstractApexNode<?> node, Object data) {
private void processBinaryExpression(ApexNode<?> node, Object data) {
ASTBinaryExpression nestedBinaryExpression = node.getFirstChildOfType(ASTBinaryExpression.class);
if (nestedBinaryExpression != null) {
processBinaryExpression(nestedBinaryExpression, data);

View File

@ -24,7 +24,7 @@ improves the readability of test output.
<![CDATA[
@isTest
public class Foo {
@isTest
@isTest
static void methodATest() {
System.assertNotEquals('123', o.StageName); // not good
System.assertEquals('123', o.StageName, 'Opportunity stageName is wrong.'); // good
@ -50,12 +50,12 @@ with messages provide the developer a clearer idea of what the test does.
<![CDATA[
@isTest
public class Foo {
public static testMethod void testSomething() {
Account a = null;
// This is better than having a NullPointerException
// System.assertNotEquals(a, null, 'account not found');
a.toString();
}
public static testMethod void testSomething() {
Account a = null;
// This is better than having a NullPointerException
// System.assertNotEquals(a, null, 'account not found');
a.toString();
}
}
]]>
</example>
@ -106,12 +106,12 @@ Apex unit tests should not use @isTest(seeAllData=true) because it opens up the
<![CDATA[
@isTest(seeAllData = true)
public class Foo {
public static testMethod void testSomething() {
Account a = null;
// This is better than having a NullPointerException
// System.assertNotEquals(a, null, 'account not found');
a.toString();
}
public static testMethod void testSomething() {
Account a = null;
// This is better than having a NullPointerException
// System.assertNotEquals(a, null, 'account not found');
a.toString();
}
}
]]>
</example>

View File

@ -121,7 +121,10 @@ public class Foo {
if (a.Phone == null) { // +1
a.Phone = phone;
update a;
return true;
}
return false;
}
// Has a cognitive complexity of 5
@ -189,7 +192,7 @@ same datatype. These situations usually denote the need for new objects to wrap
<example>
<![CDATA[
// too many arguments liable to be mixed up
public void addPerson(int birthYear, int birthMonth, int birthDate, int height, int weight, int ssn) {
public void addPerson(Integer birthYear, Integer birthMonth, Integer birthDate, Integer height, Integer weight, Integer ssn) {
// ...
}
// preferred approach
@ -272,8 +275,8 @@ lines of code that are split are counted as one.
<![CDATA[
public class Foo extends Bar {
//this method only has 1 NCSS lines
public Integer methd() {
super.methd();
public Integer method() {
super.method();
@ -382,11 +385,11 @@ city/state/zip fields could park them within a single Address field.
<![CDATA[
public class Person {
// too many separate fields
int birthYear;
int birthMonth;
int birthDate;
float height;
float weight;
Integer birthYear;
Integer birthMonth;
Integer birthDate;
Double height;
Double weight;
}
public class Person {

View File

@ -72,7 +72,7 @@ Avoid directly accessing Trigger.old and Trigger.new as it can lead to a bug. Tr
trigger AccountTrigger on Account (before insert, before update) {
Account a = Trigger.new[0]; //Bad: Accessing the trigger array directly is not recommended.
for ( Account a : Trigger.new ){
for ( Account a : Trigger.new ) {
//Good: Iterate through the trigger.new array instead.
}
}
@ -96,9 +96,9 @@ the logic can dynamically identify the proper data to operate against and not fa
public without sharing class Foo {
void foo() {
//Error - hardcoded the record type id
if(a.RecordTypeId == '012500000009WAr'){
if (a.RecordTypeId == '012500000009WAr') {
//do some logic here.....
} else if(a.RecordTypeId == '0123000000095Km'){
} else if (a.RecordTypeId == '0123000000095Km') {
//do some logic here for a different record type...
}
}
@ -131,12 +131,12 @@ or reported.
<example>
<![CDATA[
public void doSomething() {
...
try {
insert accounts;
} catch (DmlException dmle) {
// not good
}
...
try {
insert accounts;
} catch (DmlException dmle) {
// not good
}
}
]]>
</example>
@ -165,11 +165,11 @@ Empty If Statement finds instances where a condition is checked but nothing is d
<example>
<![CDATA[
public class Foo {
public void bar(Integer x) {
if (x == 0) {
// empty!
public void bar(Integer x) {
if (x == 0) {
// empty!
}
}
}
}
]]>
</example>
@ -199,9 +199,9 @@ Empty block statements serve no purpose and should be removed.
<![CDATA[
public class Foo {
private int _bar;
private Integer _bar;
public void setBar(int bar) {
public void setBar(Integer bar) {
// empty
}
@ -244,7 +244,7 @@ public class Foo {
public class Foo {
public void bar() {
try {
int x=2;
Integer x=2;
} finally {
// empty!
}

View File

@ -49,7 +49,10 @@ abstract class IteratorBasedNStream<T extends Node> implements NodeStream<T> {
@Override
public <R extends Node> NodeStream<R> flatMap(Function<? super T, ? extends @Nullable NodeStream<? extends R>> mapper) {
return mapIter(iter -> IteratorUtil.flatMap(iter, mapper.andThen(IteratorBasedNStream::safeMap)));
// Note temporary function is complete typing is needed so that it compiles with ejc
// see https://bugs.eclipse.org/bugs/show_bug.cgi?id=561482
Function<? super T, Iterator<? extends R>> mapped = mapper.andThen(IteratorBasedNStream::safeMap);
return mapIter(iter -> IteratorUtil.flatMap(iter, mapped));
}
private static <R extends Node> @NonNull Iterator<? extends R> safeMap(@Nullable NodeStream<? extends R> ns) {

View File

@ -77,7 +77,7 @@ public class SaxonXPathRuleQuery extends AbstractXPathRuleQuery {
/**
* Representation of an XPath query, created at {@link #initializeXPathExpression()} using {@link #xpath}.
*/
private XPathExpression xpathExpression;
XPathExpression xpathExpression;
/**
* Holds the static context later used to match the variables in the dynamic context in

View File

@ -13,8 +13,14 @@ import net.sf.saxon.om.Axis;
/**
* Simple printer for saxon expressions. Might be useful for debugging / during development.
*
* <p>Example:
* <pre>
* ExpressionPrinter printer = new ExpressionPrinter();
* printer.visit(query.xpathExpression.getInternalExpression());
* </pre>
*/
public class ExpressionPrinter extends Visitor {
public class ExpressionPrinter extends SaxonExprVisitor {
private int depth = 0;
private void print(String s) {

View File

@ -13,6 +13,7 @@ import net.sf.saxon.Configuration;
import net.sf.saxon.expr.AxisExpression;
import net.sf.saxon.expr.Expression;
import net.sf.saxon.expr.FilterExpression;
import net.sf.saxon.expr.LazyExpression;
import net.sf.saxon.expr.PathExpression;
import net.sf.saxon.expr.RootExpression;
import net.sf.saxon.om.Axis;
@ -33,17 +34,22 @@ import net.sf.saxon.type.Type;
* <p>DocumentSorter expression is removed. The sorting of the resulting nodes needs to be done
* after all (sub)expressions have been executed.
*/
public class RuleChainAnalyzer extends Visitor {
public class RuleChainAnalyzer extends SaxonExprVisitor {
private final Configuration configuration;
private String rootElement;
private boolean rootElementReplaced;
private boolean insideLazyExpression;
private boolean foundPathInsideLazy;
public RuleChainAnalyzer(Configuration currentConfiguration) {
this.configuration = currentConfiguration;
}
public String getRootElement() {
return rootElement;
if (!foundPathInsideLazy && rootElementReplaced) {
return rootElement;
}
return null;
}
@Override
@ -55,7 +61,7 @@ public class RuleChainAnalyzer extends Visitor {
@Override
public Expression visit(PathExpression e) {
if (rootElement == null) {
if (!insideLazyExpression && rootElement == null) {
Expression result = super.visit(e);
if (rootElement != null && !rootElementReplaced) {
if (result instanceof PathExpression) {
@ -79,6 +85,9 @@ public class RuleChainAnalyzer extends Visitor {
}
return result;
} else {
if (insideLazyExpression) {
foundPathInsideLazy = true;
}
return super.visit(e);
}
}
@ -96,6 +105,15 @@ public class RuleChainAnalyzer extends Visitor {
return super.visit(e);
}
@Override
public Expression visit(LazyExpression e) {
boolean prevCtx = insideLazyExpression;
insideLazyExpression = true;
Expression result = super.visit(e);
insideLazyExpression = prevCtx;
return result;
}
public static Comparator<Node> documentOrderComparator() {
return net.sourceforge.pmd.lang.rule.xpath.internal.DocumentSorter.INSTANCE;
}

View File

@ -5,8 +5,10 @@
package net.sourceforge.pmd.lang.rule.xpath.internal;
import net.sf.saxon.expr.AxisExpression;
import net.sf.saxon.expr.BooleanExpression;
import net.sf.saxon.expr.Expression;
import net.sf.saxon.expr.FilterExpression;
import net.sf.saxon.expr.LazyExpression;
import net.sf.saxon.expr.LetExpression;
import net.sf.saxon.expr.PathExpression;
import net.sf.saxon.expr.QuantifiedExpression;
@ -14,7 +16,7 @@ import net.sf.saxon.expr.RootExpression;
import net.sf.saxon.expr.VennExpression;
import net.sf.saxon.sort.DocumentSorter;
abstract class Visitor {
abstract class SaxonExprVisitor {
public Expression visit(DocumentSorter e) {
Expression base = visit(e.getBaseExpression());
return new DocumentSorter(base);
@ -63,6 +65,18 @@ abstract class Visitor {
return result;
}
public Expression visit(LazyExpression e) {
Expression base = visit(e.getBaseExpression());
return LazyExpression.makeLazyExpression(base);
}
public Expression visit(BooleanExpression e) {
final Expression[] operands = e.getOperands();
Expression operand0 = visit(operands[0]);
Expression operand1 = visit(operands[1]);
return new BooleanExpression(operand0, e.getOperator(), operand1);
}
public Expression visit(Expression expr) {
Expression result;
if (expr instanceof DocumentSorter) {
@ -81,6 +95,10 @@ abstract class Visitor {
result = visit((QuantifiedExpression) expr);
} else if (expr instanceof LetExpression) {
result = visit((LetExpression) expr);
} else if (expr instanceof LazyExpression) {
result = visit((LazyExpression) expr);
} else if (expr instanceof BooleanExpression) {
result = visit((BooleanExpression) expr);
} else {
result = expr;
}

View File

@ -17,7 +17,7 @@ import net.sf.saxon.expr.VennExpression;
*
* <p>E.g. "//A | //B | //C" will result in 3 expressions "//A", "//B", and "//C".
*/
class SplitUnions extends Visitor {
class SplitUnions extends SaxonExprVisitor {
private List<Expression> expressions = new ArrayList<>();
@Override

View File

@ -17,10 +17,14 @@ import net.sourceforge.pmd.lang.ast.DummyNode;
import net.sourceforge.pmd.lang.ast.DummyRoot;
import net.sourceforge.pmd.lang.ast.Node;
import net.sourceforge.pmd.lang.ast.ParseException;
import net.sourceforge.pmd.lang.ast.xpath.DefaultASTXPathHandler;
import net.sourceforge.pmd.lang.rule.AbstractRuleChainVisitor;
import net.sourceforge.pmd.lang.rule.ParametricRuleViolation;
import net.sourceforge.pmd.lang.rule.impl.DefaultRuleViolationFactory;
import net.sf.saxon.expr.XPathContext;
import net.sf.saxon.sxpath.IndependentContext;
/**
* Dummy language used for testing PMD.
*/
@ -67,6 +71,26 @@ public class DummyLanguageModule extends BaseLanguageModule {
super(DummyAstStages.class);
}
public static class TestFunctions {
public static boolean typeIs(final XPathContext context, final String fullTypeName) {
return false;
}
}
@Override
public XPathHandler getXPathHandler() {
return new DefaultASTXPathHandler() {
@Override
public void initialize(IndependentContext context) {
super.initialize(context, LanguageRegistry.getLanguage(DummyLanguageModule.NAME), TestFunctions.class);
}
@Override
public void initialize() {
}
};
}
@Override
public RuleViolationFactory getRuleViolationFactory() {
return new RuleViolationFactory();

View File

@ -18,7 +18,6 @@ import net.sourceforge.pmd.properties.PropertyDescriptor;
import net.sourceforge.pmd.properties.PropertyFactory;
import net.sf.saxon.expr.Expression;
import net.sf.saxon.expr.XPathContext;
public class SaxonXPathRuleQueryTest {
@ -64,10 +63,42 @@ public class SaxonXPathRuleQueryTest {
assertExpression("((((/)/descendant::element(dummyNode, xs:anyType))[QuantifiedExpression(Atomizer(attribute::attribute(Test2, xs:anyAtomicType)), ($qq:qq1741979653 singleton eq true()))])[QuantifiedExpression(Atomizer(attribute::attribute(Test1, xs:anyAtomicType)), ($qq:qq1529060733 singleton eq false()))])", query.nodeNameToXPaths.get(SaxonXPathRuleQuery.AST_ROOT).get(0));
}
public static class TestFunctions {
public static boolean typeIs(final XPathContext context, final String fullTypeName) {
return false;
}
@Test
public void ruleChainVisitsCustomFunctions() {
SaxonXPathRuleQuery query = createQuery("//dummyNode[pmd-dummy:typeIs(@Image)]");
List<String> ruleChainVisits = query.getRuleChainVisits();
Assert.assertEquals(1, ruleChainVisits.size());
Assert.assertTrue(ruleChainVisits.contains("dummyNode"));
Assert.assertEquals(2, query.nodeNameToXPaths.size());
assertExpression("(self::node()[pmd-dummy:typeIs(CardinalityChecker(ItemChecker(UntypedAtomicConverter(Atomizer(attribute::attribute(Image, xs:anyAtomicType))))))])", query.nodeNameToXPaths.get("dummyNode").get(0));
assertExpression("DocumentSorter((((/)/descendant-or-self::node())/(child::element(dummyNode, xs:anyType)[pmd-dummy:typeIs(CardinalityChecker(ItemChecker(UntypedAtomicConverter(Atomizer(attribute::attribute(Image, xs:anyAtomicType))))))])))", query.nodeNameToXPaths.get(SaxonXPathRuleQuery.AST_ROOT).get(0));
}
/**
* If a query contains another unbounded path expression other than the first one, it must be
* excluded from rule chain execution. Saxon itself optimizes this quite good already.
*/
@Test
public void ruleChainVisitsUnboundedPathExpressions() {
SaxonXPathRuleQuery query = createQuery("//dummyNode[//ClassOrInterfaceType]");
List<String> ruleChainVisits = query.getRuleChainVisits();
Assert.assertEquals(0, ruleChainVisits.size());
Assert.assertEquals(1, query.nodeNameToXPaths.size());
assertExpression("LetExpression(LazyExpression(((/)/descendant::element(ClassOrInterfaceType, xs:anyType))), (((/)/descendant::element(dummyNode, xs:anyType))[$zz:zz771775563]))", query.nodeNameToXPaths.get(SaxonXPathRuleQuery.AST_ROOT).get(0));
// second sample, more complex
query = createQuery("//dummyNode[ancestor::ClassOrInterfaceDeclaration[//ClassOrInterfaceType]]");
ruleChainVisits = query.getRuleChainVisits();
Assert.assertEquals(0, ruleChainVisits.size());
Assert.assertEquals(1, query.nodeNameToXPaths.size());
assertExpression("LetExpression(LazyExpression(((/)/descendant::element(ClassOrInterfaceType, xs:anyType))), (((/)/descendant::element(dummyNode, xs:anyType))[(ancestor::element(ClassOrInterfaceDeclaration, xs:anyType)[$zz:zz106374177])]))", query.nodeNameToXPaths.get(SaxonXPathRuleQuery.AST_ROOT).get(0));
// third example, with boolean expr
query = createQuery("//dummyNode[//ClassOrInterfaceType or //OtherNode]");
ruleChainVisits = query.getRuleChainVisits();
Assert.assertEquals(0, ruleChainVisits.size());
Assert.assertEquals(1, query.nodeNameToXPaths.size());
assertExpression("LetExpression(LazyExpression((((/)/descendant::element(ClassOrInterfaceType, xs:anyType)) or ((/)/descendant::element(OtherNode, xs:anyType)))), (((/)/descendant::element(dummyNode, xs:anyType))[$zz:zz1364913072]))", query.nodeNameToXPaths.get(SaxonXPathRuleQuery.AST_ROOT).get(0));
}
@Test

Some files were not shown because too many files have changed in this diff Show More