GROOVY-11490: STC: check null return of lambda

apache · Oct 18, 2024 · a818836 · a818836
1 parent ab5a811
commit a818836
Show file tree

Hide file tree

Showing 4 changed files with 20 additions and 20 deletions.
diff --git a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -376,8 +376,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
 
     public static final Statement GENERATED_EMPTY_STATEMENT = EmptyStatement.INSTANCE;
 
-    protected final ReturnAdder.ReturnStatementListener returnListener = returnStatement -> {
-        if (returnStatement.isReturningNullOrVoid()) return;
+    protected final ReturnAdder.ReturnStatementListener returnListener = (returnStatement) -> {
         ClassNode returnType = checkReturnType(returnStatement);
         if (this.typeCheckingContext.getEnclosingClosure() != null) {
             addClosureReturnType(returnType);
@@ -2392,10 +2391,13 @@ protected ClassNode checkReturnType(final ReturnStatement statement) {
                     && !GenericsUtils.hasUnresolvedGenerics(inferredReturnType)) {
                 if (isStringType(inferredReturnType) && isGStringOrGStringStringLUB(type)) {
                     type = STRING_TYPE; // GROOVY-9971: convert GString to String at point of return
-                } else if (GenericsUtils.buildWildcardType(wrapTypeIfNecessary(inferredReturnType)).isCompatibleWith(wrapTypeIfNecessary(type))) {
+                } else if (statement.isReturningNullOrVoid() ? !isPrimitiveType(inferredReturnType) // GROOVY-7713, GROOVY-8202
+                     : GenericsUtils.buildWildcardType(wrapTypeIfNecessary(inferredReturnType)).isCompatibleWith(wrapTypeIfNecessary(type))) {
                     type = inferredReturnType; // GROOVY-8310, GROOVY-10082, GROOVY-10091, GROOVY-10128, GROOVY-10306: allow simple covariance
-                } else if (!isPrimitiveVoid(type) && !extension.handleIncompatibleReturnType(statement, type)) { // GROOVY-10277: incompatible return value
-                    addStaticTypeError("Cannot return value of type " + prettyPrintType(type) + " for " + (enclosingClosure.getClosureExpression() instanceof LambdaExpression ? "lambda" : "closure") + " expecting " + prettyPrintType(inferredReturnType), expression);
+                } else if (!isPrimitiveVoid(type) && !extension.handleIncompatibleReturnType(statement, type)) { // GROOVY-10277: incompatible
+                    String value = (statement.isReturningNullOrVoid() ? "null" : "value of type " + prettyPrintType(type));
+                    String which = (enclosingClosure.getClosureExpression() instanceof LambdaExpression ? "lambda" : "closure");
+                    addStaticTypeError("Cannot return " + value + " for " + which + " expecting " + prettyPrintType(inferredReturnType), expression);
                 }
             }
             return type;
@@ -2404,7 +2406,7 @@ protected ClassNode checkReturnType(final ReturnStatement statement) {
         MethodNode enclosingMethod = typeCheckingContext.getEnclosingMethod();
         if (enclosingMethod != null && !enclosingMethod.isVoidMethod() && !enclosingMethod.isDynamicReturnType()) {
             ClassNode returnType = enclosingMethod.getReturnType();
-            if (!isPrimitiveVoid(getUnwrapper(type)) && !checkCompatibleAssignmentTypes(returnType, type, null, false)) {
+            if (!isPrimitiveVoid(getUnwrapper(type)) && !checkCompatibleAssignmentTypes(returnType, type, expression, false)) {
                 if (!extension.handleIncompatibleReturnType(statement, type)) {
                     addStaticTypeError("Cannot return value of type " + prettyPrintType(type) + " for method returning " + prettyPrintType(returnType), expression);
                 }

diff --git a/src/test/groovy/transform/stc/ClosuresSTCTest.groovy b/src/test/groovy/transform/stc/ClosuresSTCTest.groovy
@@ -358,7 +358,7 @@ class ClosuresSTCTest extends StaticTypeCheckingTestCase {
         'Cannot return value of type java.util.ArrayList<java.lang.Object> for closure expecting java.lang.Boolean'
     }
 
-    // GROOVY-8202
+    // GROOVY-7713, GROOVY-8202
     void testClosureReturnTypeInference9() {
         assertScript '''
             void proc() {
@@ -382,7 +382,7 @@ class ClosuresSTCTest extends StaticTypeCheckingTestCase {
                 c.call()
             }
             String test2(flag) {
-                Closure<String> c = { -> // Cannot assign Closure<Object> to Closure<String>
+                Closure<String> c = { ->
                     if (flag) {
                         'baz'
                     } else {

diff --git a/src/test/groovy/transform/stc/GenericsSTCTest.groovy b/src/test/groovy/transform/stc/GenericsSTCTest.groovy
@@ -5402,18 +5402,6 @@ class GenericsSTCTest extends StaticTypeCheckingTestCase {
         '''
     }
 
-    // GROOVY-7713
-    void testClosureReturnNull() {
-        assertScript '''
-            Closure<String> cl = {
-                if (hashCode() > 0) {
-                    return null
-                }
-                'foo'
-            }
-        '''
-    }
-
     // GROOVY-10528
     void testRawTypeGuard() {
         assertScript '''

diff --git a/src/test/groovy/transform/stc/LambdaTest.groovy b/src/test/groovy/transform/stc/LambdaTest.groovy
@@ -198,6 +198,16 @@ final class LambdaTest {
         '''
     }
 
+    // GROOVY-11490
+    @Test
+    void testUnaryOperator2() {
+        def err = shouldFail shell, '''
+            IntUnaryOperator f = (i) -> null
+            int i = f.applyAsInt(42)
+        '''
+        assert err =~ /Cannot return null for lambda expecting int/
+    }
+
     @Test
     void testBiConsumer() {
         assertScript shell, '''