Add more conversion context rules, improve UnnecessaryCast

pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTStatementExpressionList.java

@@ -4,7 +4,7 @@
 
 package net.sourceforge.pmd.lang.java.ast;
 
-import java.util.Iterator;
+import net.sourceforge.pmd.lang.java.ast.ASTList.ASTNonEmptyList;
 
 /**
  * A list of statement expressions. Statement expressions are those
@@ -20,10 +20,10 @@ import java.util.Iterator;
  *
  * </pre>
  */
-public final class ASTStatementExpressionList extends AbstractStatement implements Iterable<ASTExpression> {
+public final class ASTStatementExpressionList extends ASTNonEmptyList<ASTExpression> implements ASTStatement {
 
     ASTStatementExpressionList(int id) {
-        super(id);
+        super(id, ASTExpression.class);
     }
 
 
@@ -31,9 +31,4 @@ public final class ASTStatementExpressionList extends AbstractStatement implemen
     protected <P, R> R acceptVisitor(JavaVisitor<? super P, ? extends R> visitor, P data) {
         return visitor.visit(this, data);
     }
-
-    @Override
-    public Iterator<ASTExpression> iterator() {
-        return children(ASTExpression.class).iterator();
-    }
 }

pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/PolyResolution.java

@@ -448,6 +448,10 @@ final class PolyResolution {
                 return node.getIndexInParent() == 0 ? booleanCtx // condition
                                                     : stringCtx; // message
 
+            } else if (papa instanceof ASTIfStatement || papa instanceof ASTLoopStatement && !(papa instanceof ASTForeachStatement)) {
+
+                return booleanCtx; // condition
+
             } else if (papa instanceof ASTConditionalExpression && node.getIndexInParent() != 0) {
                 assert ((ASTConditionalExpression) papa).isStandalone()
                     : "Expected standalone ternary, otherwise doesCascadeContext(..) would have returned true";

pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryCastRule.java

@@ -41,8 +41,7 @@ public class UnnecessaryCastRule extends AbstractJavaRulechainRule {
         JTypeMirror operandType = operand.getTypeMirror();                      // int
 
         if (TypeOps.isUnresolvedOrNull(operandType)
-            || TypeOps.isUnresolvedOrNull(coercionType)
-            || context.isMissing()) {
+            || TypeOps.isUnresolvedOrNull(coercionType)) {
             return null;
         }
 
@@ -52,7 +51,7 @@ public class UnnecessaryCastRule extends AbstractJavaRulechainRule {
         if (operand instanceof ASTLambdaExpression || operand instanceof ASTMethodReference) {
             // Then the cast provides a target type for the expression (always).
             // We need to check the enclosing context, as if it's invocation we give up for now
-            if (castExpr.getConversionContext().isInvocationContext()) {
+            if (context.isMissing() || context.isInvocationContext()) {
                 // Then the cast may be used to determine the overload.
                 // We need to treat the casted lambda as a whole unit.
                 // todo see below
@@ -73,6 +72,15 @@ public class UnnecessaryCastRule extends AbstractJavaRulechainRule {
             return null;
         }
 
+        if (context.isMissing()) {
+            // we have a more limited set of violation conditions here
+            if (!operandType.isBottom() && operandType.isSubtypeOf(coercionType)) {
+                reportCast(castExpr, data);
+            }
+            return null;
+        }
+        // context is not missing from here on
+
         boolean isInTernary = castExpr.getParent() instanceof ASTConditionalExpression;
 
         if (castIsUnnecessary(context, coercionType, operandType, isInTernary)) {

pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/types/internal/infer/ConversionContextTests.kt

@@ -5,10 +5,9 @@
 
 package net.sourceforge.pmd.lang.java.types.internal.infer
 
+import net.sourceforge.pmd.lang.ast.test.component6
 import net.sourceforge.pmd.lang.ast.test.shouldBe
-import net.sourceforge.pmd.lang.java.ast.ASTExpression
-import net.sourceforge.pmd.lang.java.ast.ASTVariableAccess
-import net.sourceforge.pmd.lang.java.ast.ProcessorTestSpec
+import net.sourceforge.pmd.lang.java.ast.*
 import net.sourceforge.pmd.lang.java.types.STRING
 import net.sourceforge.pmd.lang.java.types.parseWithTypeInferenceSpy
 import net.sourceforge.pmd.lang.java.types.shouldHaveType
@@ -103,19 +102,64 @@ class ConversionContextTests : ProcessorTestSpec({
             class Foo {
                 static void m(Boolean boxedBool, boolean bool, String str) {
                     assert boxedBool;
-                    assert bool;
                     assert bool : str;
                 }
             }
         """)
 
-        val (boxedBool, bool, bool2, str) = acu.descendants(ASTVariableAccess::class.java).toList()
+        val (boxedBool, bool, str) = acu.descendants(ASTVariableAccess::class.java).toList()
 
         spy.shouldBeOk {
             boxedBool.conversionContext::getTargetType shouldBe ts.BOOLEAN
             bool.conversionContext::getTargetType shouldBe ts.BOOLEAN
-            bool2.conversionContext::getTargetType shouldBe ts.BOOLEAN
             str.conversionContext::getTargetType shouldBe ts.STRING
         }
     }
+
+    parserTest("Test context of statements with conditions") {
+
+        val (acu, spy) = parser.parseWithTypeInferenceSpy("""
+            class Foo {
+                static void m(Boolean boxedBool, boolean bool, String str, int[] ints) {
+                    if (boxedBool);
+                    while (boxedBool);
+                    for (int i = 0; boxedBool; i++) {}
+                    do; while (boxedBool);
+                    for (int i : ints);
+                }
+            }
+        """)
+
+        val (ifstmt, whilestmt, forstmt, _, dostmt, foreachstmt) = acu.descendants(ASTVariableAccess::class.java).toList()
+        val forUpdate = acu.descendants(ASTForUpdate::class.java).firstOrThrow().exprList[0]
+
+        spy.shouldBeOk {
+
+            ifstmt.conversionContext::getTargetType shouldBe ts.BOOLEAN
+            whilestmt.conversionContext::getTargetType shouldBe ts.BOOLEAN
+            forstmt.conversionContext::getTargetType shouldBe ts.BOOLEAN
+            dostmt.conversionContext::getTargetType shouldBe ts.BOOLEAN
+
+            forUpdate.conversionContext::getTargetType shouldBe null
+            foreachstmt.conversionContext::getTargetType shouldBe null
+        }
+    }
+    parserTest("Test missing context in lhs of method") {
+
+        val (acu, spy) = parser.parseWithTypeInferenceSpy("""
+            class Scratch {
+                static void m(Boolean boxedBool) {
+                    ((Boolean) boxedBool).booleanValue(); 
+                    ((Object) boxedBool).toString();
+                }
+            }
+        """)
+
+        val (booleanCast, objectCast) = acu.descendants(ASTCastExpression::class.java).toList()
+
+        spy.shouldBeOk {
+            booleanCast.conversionContext::getTargetType shouldBe null
+            objectCast.conversionContext::getTargetType shouldBe null
+        }
+    }
 })

pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/UnnecessaryCast.xml

@@ -617,4 +617,31 @@ class Scratch {
             }
             ]]></code>
     </test-code>
+    <test-code>
+        <description>Loops</description>
+        <expected-problems>5</expected-problems>
+        <code><![CDATA[
+            class Scratch {
+                static void m(Boolean boxedBool, boolean bool, String str, int[] ints) {
+                    if ((boolean) boxedBool);
+                    while ((boolean) boxedBool);
+                    for (int i = 0; (boolean) boxedBool; i++) {}
+                    do; while ((boolean) boxedBool);
+                    for (int i : (int[]) ints);
+                }
+            }
+            ]]></code>
+    </test-code>
+    <test-code>
+        <description>Missing context identity cast</description>
+        <expected-problems>2</expected-problems>
+        <code><![CDATA[
+            class Scratch {
+                static void m(Boolean boxedBool) {
+                    ((Boolean) boxedBool).booleanValue(); // same type
+                    ((Object) boxedBool).toString(); // supertype
+                }
+            }
+            ]]></code>
+    </test-code>
 </test-data>