Perform NullAway build-time checks in spring-expression

See gh-32475
This commit is contained in:
Sébastien Deleuze
2024-03-20 10:09:04 +01:00
parent 1ccd5512c5
commit f648fd7c3b
14 changed files with 41 additions and 12 deletions

View File

@@ -28,6 +28,7 @@ import java.util.Map;
import org.springframework.asm.ClassWriter;
import org.springframework.asm.MethodVisitor;
import org.springframework.asm.Opcodes;
import org.springframework.lang.Contract;
import org.springframework.lang.Nullable;
import org.springframework.util.ClassUtils;
import org.springframework.util.CollectionUtils;
@@ -583,6 +584,7 @@ public class CodeFlow implements Opcodes {
* @param descriptor type descriptor
* @return {@code true} if the descriptor is boolean compatible
*/
@Contract("null -> false")
public static boolean isBooleanCompatible(@Nullable String descriptor) {
return (descriptor != null && (descriptor.equals("Z") || descriptor.equals("Ljava/lang/Boolean")));
}
@@ -592,6 +594,7 @@ public class CodeFlow implements Opcodes {
* @param descriptor type descriptor
* @return {@code true} if a primitive type or {@code void}
*/
@Contract("null -> false")
public static boolean isPrimitive(@Nullable String descriptor) {
return (descriptor != null && descriptor.length() == 1);
}
@@ -601,6 +604,7 @@ public class CodeFlow implements Opcodes {
* @param descriptor the descriptor for a possible primitive array
* @return {@code true} if the descriptor a primitive array
*/
@Contract("null -> false")
public static boolean isPrimitiveArray(@Nullable String descriptor) {
if (descriptor == null) {
return false;
@@ -653,6 +657,7 @@ public class CodeFlow implements Opcodes {
* @param descriptor the descriptor for a type
* @return {@code true} if the descriptor is for a supported numeric type or boolean
*/
@Contract("null -> false")
public static boolean isPrimitiveOrUnboxableSupportedNumberOrBoolean(@Nullable String descriptor) {
if (descriptor == null) {
return false;
@@ -670,6 +675,7 @@ public class CodeFlow implements Opcodes {
* @param descriptor the descriptor for a type
* @return {@code true} if the descriptor is for a supported numeric type
*/
@Contract("null -> false")
public static boolean isPrimitiveOrUnboxableSupportedNumber(@Nullable String descriptor) {
if (descriptor == null) {
return false;
@@ -690,6 +696,7 @@ public class CodeFlow implements Opcodes {
* @param number the number to check
* @return {@code true} if it is an {@link Integer}, {@link Short} or {@link Byte}
*/
@Contract("null -> false")
public static boolean isIntegerForNumericOp(Number number) {
return (number instanceof Integer || number instanceof Short || number instanceof Byte);
}

View File

@@ -325,6 +325,7 @@ public class Indexer extends SpelNodeImpl {
CompilablePropertyAccessor compilablePropertyAccessor = (CompilablePropertyAccessor) this.cachedReadAccessor;
Assert.state(compilablePropertyAccessor != null, "No cached read accessor");
String propertyName = (String) stringLiteral.getLiteralValue().getValue();
Assert.state(propertyName != null, "No property name");
compilablePropertyAccessor.generateCode(propertyName, mv, cf);
}
@@ -565,6 +566,7 @@ public class Indexer extends SpelNodeImpl {
}
@Override
@SuppressWarnings("NullAway")
public TypedValue getValue() {
Class<?> targetObjectRuntimeClass = getObjectClass(this.targetObject);
try {
@@ -603,6 +605,7 @@ public class Indexer extends SpelNodeImpl {
}
@Override
@SuppressWarnings("NullAway")
public void setValue(@Nullable Object newValue) {
Class<?> contextObjectClass = getObjectClass(this.targetObject);
try {

View File

@@ -25,6 +25,7 @@ import org.springframework.expression.spel.ExpressionState;
import org.springframework.expression.spel.SpelEvaluationException;
import org.springframework.expression.spel.SpelMessage;
import org.springframework.expression.spel.support.BooleanTypedValue;
import org.springframework.lang.Contract;
import org.springframework.lang.Nullable;
/**
@@ -64,6 +65,7 @@ public class OpAnd extends Operator {
}
}
@Contract("null -> fail")
private void assertValueNotNull(@Nullable Boolean value) {
if (value == null) {
throw new SpelEvaluationException(SpelMessage.TYPE_CONVERSION_ERROR, "null", "boolean");

View File

@@ -68,19 +68,17 @@ public class OpEQ extends Operator {
cf.loadEvaluationContext(mv);
String leftDesc = getLeftOperand().exitTypeDescriptor;
String rightDesc = getRightOperand().exitTypeDescriptor;
boolean leftPrim = CodeFlow.isPrimitive(leftDesc);
boolean rightPrim = CodeFlow.isPrimitive(rightDesc);
cf.enterCompilationScope();
getLeftOperand().generateCode(mv, cf);
cf.exitCompilationScope();
if (leftPrim) {
if (CodeFlow.isPrimitive(leftDesc)) {
CodeFlow.insertBoxIfNecessary(mv, leftDesc.charAt(0));
}
cf.enterCompilationScope();
getRightOperand().generateCode(mv, cf);
cf.exitCompilationScope();
if (rightPrim) {
if (CodeFlow.isPrimitive(rightDesc)) {
CodeFlow.insertBoxIfNecessary(mv, rightDesc.charAt(0));
}

View File

@@ -69,19 +69,17 @@ public class OpNE extends Operator {
cf.loadEvaluationContext(mv);
String leftDesc = getLeftOperand().exitTypeDescriptor;
String rightDesc = getRightOperand().exitTypeDescriptor;
boolean leftPrim = CodeFlow.isPrimitive(leftDesc);
boolean rightPrim = CodeFlow.isPrimitive(rightDesc);
cf.enterCompilationScope();
getLeftOperand().generateCode(mv, cf);
cf.exitCompilationScope();
if (leftPrim) {
if (CodeFlow.isPrimitive(leftDesc)) {
CodeFlow.insertBoxIfNecessary(mv, leftDesc.charAt(0));
}
cf.enterCompilationScope();
getRightOperand().generateCode(mv, cf);
cf.exitCompilationScope();
if (rightPrim) {
if (CodeFlow.isPrimitive(rightDesc)) {
CodeFlow.insertBoxIfNecessary(mv, rightDesc.charAt(0));
}

View File

@@ -24,6 +24,7 @@ import org.springframework.expression.spel.ExpressionState;
import org.springframework.expression.spel.SpelEvaluationException;
import org.springframework.expression.spel.SpelMessage;
import org.springframework.expression.spel.support.BooleanTypedValue;
import org.springframework.lang.Contract;
import org.springframework.lang.Nullable;
/**
@@ -63,6 +64,7 @@ public class OpOr extends Operator {
}
}
@Contract("null -> fail")
private void assertValueNotNull(@Nullable Boolean value) {
if (value == null) {
throw new SpelEvaluationException(SpelMessage.TYPE_CONVERSION_ERROR, "null", "boolean");

View File

@@ -352,6 +352,7 @@ public abstract class Operator extends SpelNodeImpl {
* @param rightActualDescriptor the dynamic/runtime right object descriptor
* @return a DescriptorComparison object indicating the type of compatibility, if any
*/
@SuppressWarnings("NullAway")
public static DescriptorComparison checkNumericCompatibility(
@Nullable String leftDeclaredDescriptor, @Nullable String rightDeclaredDescriptor,
@Nullable String leftActualDescriptor, @Nullable String rightActualDescriptor) {

View File

@@ -134,7 +134,7 @@ public class PropertyOrFieldReference extends SpelNodeImpl {
// 'simple' object
try {
if (isWritableProperty(this.name,contextObject, evalContext)) {
Class<?> clazz = result.getTypeDescriptor().getType();
Class<?> clazz = resultDescriptor.getType();
Object newObject = ReflectionUtils.accessibleConstructor(clazz).newInstance();
writeProperty(contextObject, evalContext, this.name, newObject);
result = readProperty(contextObject, evalContext, this.name);
@@ -142,11 +142,11 @@ public class PropertyOrFieldReference extends SpelNodeImpl {
}
catch (InvocationTargetException ex) {
throw new SpelEvaluationException(getStartPosition(), ex.getTargetException(),
SpelMessage.UNABLE_TO_DYNAMICALLY_CREATE_OBJECT, result.getTypeDescriptor().getType());
SpelMessage.UNABLE_TO_DYNAMICALLY_CREATE_OBJECT, resultDescriptor.getType());
}
catch (Throwable ex) {
throw new SpelEvaluationException(getStartPosition(), ex,
SpelMessage.UNABLE_TO_DYNAMICALLY_CREATE_OBJECT, result.getTypeDescriptor().getType());
SpelMessage.UNABLE_TO_DYNAMICALLY_CREATE_OBJECT, resultDescriptor.getType());
}
}
}

View File

@@ -76,6 +76,7 @@ import org.springframework.expression.spel.ast.StringLiteral;
import org.springframework.expression.spel.ast.Ternary;
import org.springframework.expression.spel.ast.TypeReference;
import org.springframework.expression.spel.ast.VariableReference;
import org.springframework.lang.Contract;
import org.springframework.lang.Nullable;
import org.springframework.util.StringUtils;
@@ -164,6 +165,7 @@ class InternalSpelExpressionParser extends TemplateAwareExpressionParser {
// | (QMARK^ expression COLON! expression)
// | (ELVIS^ expression))?;
@Nullable
@SuppressWarnings("NullAway")
private SpelNodeImpl eatExpression() {
SpelNodeImpl expr = eatLogicalOrExpression();
Token t = peekToken();
@@ -274,6 +276,7 @@ class InternalSpelExpressionParser extends TemplateAwareExpressionParser {
//sumExpression: productExpression ( (PLUS^ | MINUS^) productExpression)*;
@Nullable
@SuppressWarnings("NullAway")
private SpelNodeImpl eatSumExpression() {
SpelNodeImpl expr = eatProductExpression();
while (peekToken(TokenKind.PLUS, TokenKind.MINUS, TokenKind.INC)) {
@@ -313,6 +316,7 @@ class InternalSpelExpressionParser extends TemplateAwareExpressionParser {
// powerExpr : unaryExpression (POWER^ unaryExpression)? (INC || DEC) ;
@Nullable
@SuppressWarnings("NullAway")
private SpelNodeImpl eatPowerIncDecExpression() {
SpelNodeImpl expr = eatUnaryExpression();
if (peekToken(TokenKind.POWER)) {
@@ -333,6 +337,7 @@ class InternalSpelExpressionParser extends TemplateAwareExpressionParser {
// unaryExpression: (PLUS^ | MINUS^ | BANG^ | INC^ | DEC^) unaryExpression | primaryExpression ;
@Nullable
@SuppressWarnings("NullAway")
private SpelNodeImpl eatUnaryExpression() {
if (peekToken(TokenKind.NOT, TokenKind.PLUS, TokenKind.MINUS)) {
Token t = takeToken();
@@ -755,6 +760,7 @@ class InternalSpelExpressionParser extends TemplateAwareExpressionParser {
qualifiedIdPieces.getLast().getEndPosition(), qualifiedIdPieces.toArray(new SpelNodeImpl[0]));
}
@Contract("null -> false")
private boolean isValidQualifiedId(@Nullable Token node) {
if (node == null || node.kind == TokenKind.LITERAL_STRING) {
return false;
@@ -1040,17 +1046,20 @@ class InternalSpelExpressionParser extends TemplateAwareExpressionParser {
return t.kind.toString().toLowerCase();
}
@Contract("_, null, _ -> fail; _, _, null -> fail")
private void checkOperands(Token token, @Nullable SpelNodeImpl left, @Nullable SpelNodeImpl right) {
checkLeftOperand(token, left);
checkRightOperand(token, right);
}
@Contract("_, null -> fail")
private void checkLeftOperand(Token token, @Nullable SpelNodeImpl operandExpression) {
if (operandExpression == null) {
throw internalException(token.startPos, SpelMessage.LEFT_OPERAND_PROBLEM);
}
}
@Contract("_, null -> fail")
private void checkRightOperand(Token token, @Nullable SpelNodeImpl operandExpression) {
if (operandExpression == null) {
throw internalException(token.startPos, SpelMessage.RIGHT_OPERAND_PROBLEM);

View File

@@ -155,6 +155,7 @@ public class ReflectivePropertyAccessor implements PropertyAccessor {
}
@Override
@SuppressWarnings("NullAway")
public TypedValue read(EvaluationContext context, @Nullable Object target, String name) throws AccessException {
Assert.state(target != null, "Target must not be null");
Class<?> type = (target instanceof Class<?> clazz ? clazz : target.getClass());
@@ -515,6 +516,7 @@ public class ReflectivePropertyAccessor implements PropertyAccessor {
* <p>Note: An optimized accessor is currently only usable for read attempts.
* Do not call this method if you need a read-write accessor.
*/
@SuppressWarnings("NullAway")
public PropertyAccessor createOptimalAccessor(EvaluationContext context, @Nullable Object target, String name) {
// Don't be clever for arrays or a null target...
if (target == null) {