RetroSearch Browse

Home - News ( United States | United Kingdom | Italy | Germany ) - Football scores

Showing content from https://github.com/pmd/pmd/issues/904 below:

[core] Tree traversal revision · Issue #904 · pmd/pmd · GitHub

I was recently digging into #370, which is false reporting on lambdas when finding code such as:

LOGGER.info(() -> "Bla " + " bla");

when I realized the problem laid on this line

&& node.jjtGetChild(1).hasDescendantOfType(ASTAdditiveExpression.class)) {

hasDescendantOfType completely ignores find boundaries. getFirstDescendantOfType also completely ignores find boundaries. Moreover, all tree transversal operations ignore find boundaries, except for findDescendantsOfType which doesn't (but has a convoluted overload which allows to).

So, node.hasDescendantOfType(XXX.class) may return true, node.getFirstDescendantOfType(XXX.class) won't return null, but node.findDescendantsOfType(XXX.class) may return empty. Rejoyce!

Moreover, most often than not, we don't really want to transverse along find boundaries. Actually, pretty much all current rules are broken on this respect. Some brief examples I was able to find in under 15'

Apparently when finding

public void foo() {
  class Bar {
    private Object o = new Object();
  }
}

The whole class Bar { … } block is a single ASTBlockStatement which is convinced this is an allocation statement.

public final boolean isAllocation() { return hasDescendantOfType(ASTAllocationExpression.class); }

But lambdas in particular are easier to find anywhere and completely break logic:

The complexity of the statement return () -> { someValue && doStuff() }; is 1, even though we are simply creating a lambda and in doing so performing exactly 0 boolean operations.

public static int booleanExpressionComplexity(ASTExpression expr) { if (expr == null) { return 0; } List<ASTConditionalAndExpression> andNodes = expr.findDescendantsOfType(ASTConditionalAndExpression.class); List<ASTConditionalOrExpression> orNodes = expr.findDescendantsOfType(ASTConditionalOrExpression.class);

Returning a lambda can also wreck havoc on sun secure rules

private boolean hasTernaryNullCheck(ASTReturnStatement ret) { ASTConditionalExpression condition = ret.getFirstDescendantOfType(ASTConditionalExpression.class); return condition.jjtGetChild(0) instanceof ASTEqualityExpression && condition.jjtGetChild(0).hasImageEqualTo("==") && condition.jjtGetChild(0).jjtGetChild(0).hasDescendantOfType(ASTName.class) && condition.jjtGetChild(0).jjtGetChild(1).hasDescendantOfType(ASTNullLiteral.class); } private boolean hasTernaryCondition(ASTReturnStatement ret) { ASTConditionalExpression condition = ret.getFirstDescendantOfType(ASTConditionalExpression.class); return condition != null && condition.isTernary(); }

I have so far been unable to find a single case where:

we don't intend to look ONLY within find boundaries
it's irrelevant due to the AST format (ie: looking for imports, which can only be direct children of the ACU)

On the first scenario, the current transversing is broken and may produce FPs / FNs. We may actually be reporting duplicate violations as lambdas / nested local classes statements may be analyzed more than once.
On the second scenario, we are simply inefficient by digging too deep

I did a quick test, and changing getFirstDescendantOfType (and therefore hasDescendantOfType) to respect find boundaries resulted in 0 errors, except for for this one statement where on a test we are being lazy:

ASTAllocationExpression allocationExpression = acu.getFirstDescendantOfType(ASTAllocationExpression.class); ASTAllocationExpression nestedAllocation = allocationExpression.getFirstDescendantOfType(ASTAllocationExpression.class);

On the other hand, I'm sure I can come up with tests for all these rules where lambdas an local classes produce FPs / FNs.

@adangel @oowekyala what do you think? Shall we change the default behavior on PMD 6.1.0? Shall we deprecate those methods and add new ones allowing to cross boundaries on demand? Is there really a case for crossing boundaries?

RetroSearch is an open source project built by @garambo | Open a GitHub Issue

Search and Browse the WWW like it's 1997 | Search results from DuckDuckGo

HTML: 3.2 | Encoding: UTF-8 | Version: 0.7.4