[java] New rule: UnusedAssignment

[oowekyala]

Describe the PR

Add the rule UnusedAssignment. This reports the same thing as DataflowAnomalyAnalysisRule, but doesn't use the current data flow framework. Improvements over DFAAR:

• It handles assignments in field initializers, including static initializers

  • The only thing it doesn't do is explicit this(...) constructor call, because our overload resolution doesn't resolve them afaik. This can be solved easily in the 7.0 framework.

• It handles control flow caused by shortcut boolean expressions

• It supports properly all features of Java 5+ (lambdas, foreach, try-with-resources, etc)

• The code is manageable and can be updated for new language features. The current DFA framework is impossible to navigate and to change.

• It never hangs or fail with obscure messages ([core] DFA hits a hard limit of 6 iterations without results and logs #873)

• It's nearly 4 times faster, if you factor in that DFAAR needs a preprocessing step of the tree that builds "DataflowNodes"

• Messages are readable (ref [java][doc] DataflowAnomalyAnalysis rule needs to be more descriptive and explanatory #2129). Compare:

  Found 'DU'-anomaly for variable 'z' (lines '4'-'9').

  to

  The initializer for variable 'z' is never used (goes out of scope)

• It's very precise, in principle it makes exactly the same assumptions as the ones a java compiler takes to determine definite assigned-ness. This means it reports less things than DFAAR, but every violation is actually a violation. We can let the regression tester be the judge of that though

The rule supports @SuppressWarnings("unused")

---

This rule is meant to replace DFAAR. That way, we remove the last dependency we have on our horrible data flow codebase, and can remove it in PMD 7. I think it would be more interesting to integrate a state-of-the-art data flow solver into pmd in the future, than try to reinvent the wheel (I don't plan to work on this for 7.0).

By necessity, the rule subsumes UnusedLocalVariable and UnusedFormalParameter. There's a switch to disable violations that would overlap with those rules (they're filtered out by default). But maybe this should actually replace those rules?

Related issues

• in:data-flow Affects the data flow analysis code
  • All the false positives were added as test cases of the new rule
  • We could close all those as wont-fix, if we deprecate DFAAR

Ready?

• Move to bestpractices.xml maybe? It's not really "error prone"

• Allow ignoring variables whose name starts with ignore. This is useful for exceptions, loop variables, or most importantly resources (which even if they're not explicitly used may serve a purpose)

• Added unit tests for fixed bug/feature

• Passing all unit tests

• Complete build ./mvnw clean verify passes (checked automatically by travis)

• Added (in-code) documentation (if needed)

[pmd-test]

	1 Message
📖	This changeset introduces 1140 new violations, 0 new errors and 0 new configuration errors, removes 0 violations, 0 errors and 0 configuration errors. Full report
✅	This changeset introduces 1221 new violations, 0 new errors and 0 new configuration errors, removes 51 violations, 0 errors and 0 configuration errors. Full report
✅	This changeset introduces 1321 new violations, 0 new errors and 0 new configuration errors, removes 51 violations, 0 errors and 0 configuration errors. Full report
✅	This changeset introduces 1377 new violations, 0 new errors and 0 new configuration errors, removes 51 violations, 0 errors and 0 configuration errors. Full report
✅	This changeset introduces 2531 new violations, 2 new errors and 0 new configuration errors, removes 51 violations, 0 errors and 0 configuration errors. Full report

Generated by 🚫 Danger

[adangel]

Awesome! 🎉

I'll create a issue to deprecate the DFA rule for 6.27.0.