New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[apex] Add OneDeclarationPerStatement rule #1315

Merged
merged 3 commits into from Aug 27, 2018

Conversation

Projects
None yet
4 participants
@jeffhube
Contributor

jeffhube commented Aug 21, 2018

Please, prefix the PR title with the language it applies to within brackets, such as [java] or [apex]. If not specific to a language, you can use [core]

Before submitting a PR, please check that:

  • The PR is submitted against master. The PMD team will merge back to support branches as needed.
  • ./mvnw test passes.
  • ./mvnw pmd:check passes.
  • ./mvnw checkstyle:check passes. Check this for more info

PR Description:

This PR adds the OneDeclarationPerStatement rule to apex. It applies to field declarations (inside classes) and variable declarations (inside methods).

Integer a, b;   // not recommended

Integer a;      // preferred approach
Integer b;
@pmd-test

This comment has been minimized.

pmd-test commented Aug 21, 2018

1 Message
📖 No java rules are changed!

Generated by 🚫 Danger

@jeffhube jeffhube force-pushed the jeffhube:onedeclaration branch from 82e7131 to a417612 Aug 21, 2018

<property name="xpath">
<value>
<![CDATA[
//VariableDeclarationStatements[VariableDeclaration[2]]|//FieldDeclarationStatements[FieldDeclaration[2]]

This comment has been minimized.

@oowekyala

oowekyala Aug 21, 2018

Member

I'd find it more readable to use the count function, e.g. FieldDeclarationStatements[count(FieldDeclaration) > 1], and to put the two alternatives on separate lines

@@ -149,6 +149,35 @@ public class Foo {
</example>
</rule>

<rule name="OneDeclarationPerStatement"
since="6.7.0"
message="Use one line for each declaration, it enhances code readability."

This comment has been minimized.

@oowekyala

oowekyala Aug 21, 2018

Member

Maybe "use one statement for each declaration"?

@jeffhube

This comment has been minimized.

Contributor

jeffhube commented Aug 22, 2018

Fixed, should I include the changes to the autogenerated docs in this PR?

@jsotuyod

This comment has been minimized.

Member

jsotuyod commented Aug 23, 2018

@jeffhube no need to! The CI build will do so and push back to the repo. Thanks for the PR!

@jsotuyod jsotuyod added this to the 6.7.0 milestone Aug 23, 2018

@jsotuyod jsotuyod added the a:new-rule label Aug 23, 2018

@jsotuyod

This comment has been minimized.

Member

jsotuyod commented Aug 27, 2018

For consistency, we may want to rename this and tune the rule to be equivalent to Java's OneDeclarationPerLine. I'll do it before merging.

@jsotuyod jsotuyod self-assigned this Aug 27, 2018

@jsotuyod jsotuyod merged commit 780a9e5 into pmd:master Aug 27, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jeffhube jeffhube deleted the jeffhube:onedeclaration branch Aug 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment