Clément Fournier committed 2021-04-08 23:23:04 +02:00
1 parent 3996049cc3
commit c2e65a2ec1
2 files changed
+42 -7

No files matched your search

@@ -27,15 +27,15 @@ public class UnnecessaryCastRule extends AbstractJavaRulechainRule {
}
@Override
public Object visit(ASTCastExpression node, Object data) {
ASTExpression operand = node.getOperand();
public Object visit(ASTCastExpression castExpr, Object data) {
ASTExpression operand = castExpr.getOperand();
// eg in
// Object o = (Integer) 1;
@Nullable ExprContext context = node.getConversionContextType(); // Object
@Nullable ExprContext context = castExpr.getConversionContextType(); // Object
JTypeMirror operandType = operand.getTypeMirror(); // int
JTypeMirror coercionType = node.getCastType().getTypeMirror(); // Integer
JTypeMirror coercionType = castExpr.getCastType().getTypeMirror(); // Integer
if (TypeOps.isUnresolvedOrNull(operandType)
|| TypeOps.isUnresolvedOrNull(coercionType)
@@ -49,11 +49,19 @@ 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 (isInvocationContext(castExpr.getConversionContextType())) {
// Then the cast may be used to determine the overload.
// We need to treat the casted lambda as a whole unit.
// todo see below
return null;
}
// Since the code is assumed to compile we'll just assume that coercionType
// is a functional interface.
if (context.getTargetType().equals(coercionType)) {
// then we also know that the context is functional
addViolation(data, node);
addViolation(data, castExpr);
}
// otherwise the cast is narrowing, and removing it would
// change the runtime class of the produced lambda.
@@ -64,7 +72,7 @@ public class UnnecessaryCastRule extends AbstractJavaRulechainRule {
}
if (castIsUnnecessary(context, operandType, coercionType)) {
addViolation(data, node);
addViolation(data, castExpr);
}
return null;
}
@@ -72,7 +80,7 @@ public class UnnecessaryCastRule extends AbstractJavaRulechainRule {
private static boolean castIsUnnecessary(ExprContext context,
JTypeMirror operandType,
JTypeMirror coercionType) {
if (context.isInvocationContext()) {
if (isInvocationContext(context)) {
// todo unsupported for now, the cast may be disambiguating overloads
return false;
}
@@ -83,4 +91,8 @@ public class UnnecessaryCastRule extends AbstractJavaRulechainRule {
boolean canOperandSuitContext = TypeConversion.isConvertibleThroughBoxing(operandType, contextType);
return isNotNarrowing && canOperandSuitContext;
}
private static boolean isInvocationContext(ExprContext context) {
return context != null && context.isInvocationContext();
}
}
@@ -362,6 +362,29 @@ public class MapCasts {
int getOrder();
}
]]></code>
</test-code>
<test-code>
<description>Cast as target type for method reference/lambda, with enclosing invoc context</description>
<expected-problems>0</expected-problems>
<code><![CDATA[
import java.lang.reflect.Constructor;
import java.security.AccessController;
import java.security.PrivilegedAction;
class Test {
static {
Object beanInstance;
if (System.getSecurityManager() != null) {
// these method calls are ambiguous if the cast is omitted
// (there's a PrivilegedExceptionAction overload)
beanInstance = AccessController.doPrivileged((PrivilegedAction<Object>) () -> new Object());
beanInstance = AccessController.doPrivileged((PrivilegedAction<Object>) Object::new);
}
}
}
]]></code>
</test-code>
</test-data>