Skip to content

[core] Implement violation decorators#4050

Merged
adangel merged 15 commits into
pmd:pmd/7.0.xfrom
oowekyala:violation-decorators
Jan 16, 2023
Merged

[core] Implement violation decorators#4050
adangel merged 15 commits into
pmd:pmd/7.0.xfrom
oowekyala:violation-decorators

Conversation

@oowekyala

@oowekyala oowekyala commented Jul 19, 2022

Copy link
Copy Markdown
Member

Describe the PR

Related issues

Ready?

  • Added unit tests for fixed bug/feature
  • Passing all unit tests
  • Complete build ./mvnw clean verify passes (checked automatically by github actions)
  • Added (in-code) documentation (if needed)

@oowekyala oowekyala added this to the 7.0.0 milestone Jul 19, 2022
@ghost

ghost commented Jul 19, 2022

Copy link
Copy Markdown
2 Messages
📖 Compared to pmd/7.0.x:
This changeset changes 110 violations,
introduces 1 new violations, 2578 new errors and 0 new configuration errors,
removes 1 violations, 0 errors and 0 configuration errors.
Full report
📖 Compared to master:
This changeset changes 49641 violations,
introduces 33909 new violations, 4024 new errors and 0 new configuration errors,
removes 195793 violations, 4 errors and 7 configuration errors.
Full report
Compared to pmd/7.0.x:
This changeset changes 123 violations,
introduces 20 new violations, 0 new errors and 0 new configuration errors,
removes 22 violations, 0 errors and 0 configuration errors.
Full report
Compared to master:
This changeset changes 50608 violations,
introduces 34659 new violations, 9 new errors and 0 new configuration errors,
removes 138300 violations, 4 errors and 7 configuration errors.
Full report
Compared to pmd/7.0.x:
This changeset changes 125 violations,
introduces 25 new violations, 0 new errors and 0 new configuration errors,
removes 28 violations, 0 errors and 0 configuration errors.
Full report
Compared to master:
This changeset changes 0 violations,
introduces 695509 new violations, 22 new errors and 0 new configuration errors,
removes 822667 violations, 17 errors and 7 configuration errors.
Full report
Compared to pmd/7.0.x:
This changeset changes 129 violations,
introduces 28 new violations, 0 new errors and 0 new configuration errors,
removes 23 violations, 0 errors and 0 configuration errors.
Full report
Compared to master:
This changeset changes 0 violations,
introduces 699661 new violations, 22 new errors and 0 new configuration errors,
removes 827036 violations, 39 errors and 6 configuration errors.
Full report

Generated by 🚫 Danger

@oowekyala oowekyala force-pushed the violation-decorators branch from 7b24358 to 099c46c Compare November 24, 2022 14:22
@oowekyala oowekyala force-pushed the violation-decorators branch from 099c46c to 4b5575e Compare November 24, 2022 14:30
@oowekyala oowekyala marked this pull request as ready for review November 24, 2022 16:43
@oowekyala

Copy link
Copy Markdown
Member Author

Regression: AvoidUsingHardCodedIP violation message is Do not hard code the IP address ${variableName}

@oowekyala oowekyala marked this pull request as draft November 24, 2022 16:46
@oowekyala oowekyala marked this pull request as ready for review November 26, 2022 17:40

@adangel adangel left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

I'll fix the issues and merge it afterwards.

Comment thread pmd-core/src/main/java/net/sourceforge/pmd/RuleContext.java
Comment thread pmd-core/src/main/java/net/sourceforge/pmd/RuleViolation.java
Comment thread pmd-core/src/main/java/net/sourceforge/pmd/renderers/CSVWriter.java
maybeAdd("class", rv.getAdditionalInfo().get(RuleViolation.CLASS_NAME));
maybeAdd("method", rv.getAdditionalInfo().get(RuleViolation.METHOD_NAME));
maybeAdd("variable", rv.getAdditionalInfo().get(RuleViolation.VARIABLE_NAME));
// todo other additional info keys are not rendered

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rendering additional info keys in XML renderer requires to open up the schema to allow arbitrary attributes. currently these attributes (package, class, method, variable) are declared explicitly. This could be achieved by adding anyAttribute (https://www.w3schools.com/xml/schema_complex_anyattribute.asp).

Comment on lines +69 to +71
<expected-messages>
<message>Do not hard code the IP address badIdea</message>
</expected-messages>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems this was never tested before... and it doesn't work on PMD6 either.

The undocumented feature of referencing ${variableName} is only used by 3 rules: AvoidUsingHardCodedIP, RedundantFieldInitializer, JUnitTestContainsTooManyAsserts

Ok, it turns out, that it doesn't work only for AvoidUsingHardCodedIP - the reported node is a ASTLiteral, which is not considered in PMD6. But here, we consider it as a ASTExpression and search the parent axis for variable names.

I'd keep it on PMD6 as is and consider this as a bugfix for PMD 7.

adangel added a commit that referenced this pull request Jan 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants