phoedos/pmd
1
1
Fork 1
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge branch 'pr-2396'

Browse Source 
[apex] New rule: field declarations should be at start #2396
This commit is contained in:
Andreas Dangel
2020-04-04 17:44:07 +02:00
parent 130fee68de 92a518c991
commit 6196da44b0
5 changed files with 320 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
docs/pages/release_notes.md
View File

@ -30,7 +30,10 @@ 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.
### Fixed Issues
@ -120,6 +123,7 @@ 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)
* [#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 %}

74
pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/codestyle/FieldDeclarationsShouldBeAtStartRule.java Normal file
View File

@ -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());
}
}

25
pmd-apex/src/main/resources/category/apex/codestyle.xml
View File

@ -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}''"
@ -364,5 +388,4 @@ while (true) { // preferred approach
]]>
</example>
</rule>
</ruleset>

11
pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/codestyle/FieldDeclarationsShouldBeAtStartTest.java Normal file
View File

@ -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
}

207
pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/codestyle/xml/FieldDeclarationsShouldBeAtStart.xml Normal file
View File

@ -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>