KT-2729 Warn on 'T?' where 'T' has a nullable upper bound

compiler/frontend/src/org/jetbrains/jet/lang/diagnostics/Errors.java

@@ -284,6 +284,7 @@ public interface Errors {
     SimpleDiagnosticFactory<JetClassInitializer> ANONYMOUS_INITIALIZER_WITHOUT_CONSTRUCTOR = SimpleDiagnosticFactory.create(ERROR);
     SimpleDiagnosticFactory<JetNullableType> NULLABLE_SUPERTYPE = SimpleDiagnosticFactory.create(ERROR, NULLABLE_TYPE);
     SimpleDiagnosticFactory<JetNullableType> REDUNDANT_NULLABLE = SimpleDiagnosticFactory.create(WARNING, NULLABLE_TYPE);
+    DiagnosticFactory1<JetNullableType, JetType> BASE_WITH_NULLABLE_UPPER_BOUND = DiagnosticFactory1.create(WARNING, NULLABLE_TYPE);
     DiagnosticFactory1<PsiElement, JetType> UNSAFE_CALL = DiagnosticFactory1.create(ERROR);
     SimpleDiagnosticFactory<JetSimpleNameExpression> AMBIGUOUS_LABEL = SimpleDiagnosticFactory.create(ERROR);
     DiagnosticFactory1<PsiElement, String> UNSUPPORTED = DiagnosticFactory1.create(ERROR);

compiler/frontend/src/org/jetbrains/jet/lang/diagnostics/rendering/DefaultErrorMessages.java

@@ -307,6 +307,9 @@ public class DefaultErrorMessages {
         MAP.put(ANONYMOUS_INITIALIZER_WITHOUT_CONSTRUCTOR, "Anonymous initializers are only allowed in the presence of a primary constructor");
         MAP.put(NULLABLE_SUPERTYPE, "A supertype cannot be nullable");
         MAP.put(REDUNDANT_NULLABLE, "Redundant '?'");
+        MAP.put(BASE_WITH_NULLABLE_UPPER_BOUND, "''{0}'' has a nullable upper bound. " +
+                                                "This means that a value of this type may be null. " +
+                                                "Using ''{0}?'' is likely to mislead the reader", RENDER_TYPE);
         MAP.put(UNSAFE_CALL, "Only safe (?.) or non-null asserted (!!.) calls are allowed on a nullable receiver of type {0}", RENDER_TYPE);
         MAP.put(AMBIGUOUS_LABEL, "Ambiguous label");
         MAP.put(UNSUPPORTED, "Unsupported [{0}]", TO_STRING);

compiler/frontend/src/org/jetbrains/jet/lang/resolve/TypeResolver.java

@@ -178,6 +178,9 @@ public class TypeResolver {
                     if (baseType.isNullable()) {
                         trace.report(REDUNDANT_NULLABLE.on(nullableType));
                     }
+                    else if (TypeUtils.hasNullableSuperType(baseType)) {
+                        trace.report(BASE_WITH_NULLABLE_UPPER_BOUND.on(nullableType, baseType));
+                    }
                     result[0] = TypeUtils.makeNullable(baseType);
                 }
 

compiler/frontend/src/org/jetbrains/jet/lang/types/TypeUtils.java

@@ -385,6 +385,15 @@ public class TypeUtils {
         return false;
     }
 
+    public static boolean hasNullableSuperType(@NotNull JetType type) {
+        for (JetType supertype : getAllSupertypes(type)) {
+            if (supertype.isNullable()) {
+                return true;
+            }
+        }
+        return false;
+    }
+
     public static boolean equalClasses(@NotNull JetType type1, @NotNull JetType type2) {
         DeclarationDescriptor declarationDescriptor1 = type1.getConstructor().getDeclarationDescriptor();
         if (declarationDescriptor1 == null) return false; // No class, classes are not equal

compiler/testData/diagnostics/tests/checkArguments/SpreadVarargs.kt

@@ -86,6 +86,6 @@ fun <T> joinG(x : Int, vararg a : T) : String {
     return b.toString()
 }
 
-fun <T> joinT(<!UNUSED_PARAMETER!>x<!> : Int, vararg <!UNUSED_PARAMETER!>a<!> : T) : T? {
+fun <T: Any> joinT(<!UNUSED_PARAMETER!>x<!> : Int, vararg <!UNUSED_PARAMETER!>a<!> : T) : T? {
     return null
 }

compiler/testData/diagnostics/tests/incompleteCode/incompleteAssignment.kt

@@ -20,7 +20,7 @@ fun test(expectedSum : Int, vararg data : Int) {
     assertEquals(actualSum, expectedSum, "\ndata = ${Arrays.toString(data)}\n" +
     "sum(data) = ${actualSum}, but must be $expectedSum ")
 }
-fun assertEquals<T>(actual : T?, expected : T?, message : Any? = null) {
+fun assertEquals<T: Any>(actual : T?, expected : T?, message : Any? = null) {
     if (actual != expected) {
         if (message == null)
             throw AssertionError()

compiler/testData/diagnostics/tests/inference/NoInferenceFromDeclaredBounds.kt

@@ -1,4 +1,4 @@
-fun fooT22<T>() : T? {
+fun fooT22<T: Any>() : T? {
   return null
 }
 

compiler/testData/diagnostics/tests/inference/completeInferenceIfManyFailed.kt

@@ -1,10 +1,10 @@
 package d
 
-fun <T> joinT(<!UNUSED_PARAMETER!>x<!>: Int, vararg <!UNUSED_PARAMETER!>a<!>: T): T? {
+fun <T: Any> joinT(<!UNUSED_PARAMETER!>x<!>: Int, vararg <!UNUSED_PARAMETER!>a<!>: T): T? {
     return null
 }
 
-fun <T> joinT(<!UNUSED_PARAMETER!>x<!>: Any, <!UNUSED_PARAMETER!>y<!>: T): T? {
+fun <T: Any> joinT(<!UNUSED_PARAMETER!>x<!>: Any, <!UNUSED_PARAMETER!>y<!>: T): T? {
     return null
 }
 

compiler/testData/diagnostics/tests/inference/regressions/kt2286.kt

@@ -24,4 +24,4 @@ abstract class Buggy {
 }
 
 //from library
-fun <T> Iterable<T>.find(<!UNUSED_PARAMETER!>predicate<!>: (T) -> Boolean) : T? {<!NO_RETURN_IN_FUNCTION_WITH_BLOCK_BODY!>}<!>
+fun <T: Any> Iterable<T>.find(<!UNUSED_PARAMETER!>predicate<!>: (T) -> Boolean) : T? {<!NO_RETURN_IN_FUNCTION_WITH_BLOCK_BODY!>}<!>

compiler/testData/diagnostics/tests/inference/regressions/kt2505.kt

@@ -6,8 +6,8 @@ trait MyType {}
 class MyClass<T> : MyType {}
 
 public open class HttpResponse() {
-    public open fun parseAs<T>(dataClass : MyClass<T>) : T? {
+    public open fun parseAs<T>(dataClass : MyClass<T>) : T {
-        return null
+        throw Exception()
     }
     public open fun parseAs(dataType : MyType) : Any? {
         return null
@@ -16,5 +16,5 @@ public open class HttpResponse() {
 
 fun test<R> (httpResponse: HttpResponse, rtype: MyClass<R>) {
     val res = httpResponse.parseAs( rtype )
-    res : R? //type mismatch: required R?, found T?
+    res : R //type mismatch: required R, found T
 }

compiler/testData/diagnostics/tests/inference/regressions/kt702.kt

@@ -6,7 +6,7 @@ fun getJavaClass<T>() : java.lang.Class<T> { <!NO_RETURN_IN_FUNCTION_WITH_BLOCK_
 
 public class Throwables() {
     class object {
-        public fun propagateIfInstanceOf<X : Throwable?>(throwable : Throwable?, declaredType : Class<X?>?) : Unit {
+        public fun propagateIfInstanceOf<X : Throwable?>(throwable : Throwable?, declaredType : Class<X<!BASE_WITH_NULLABLE_UPPER_BOUND!>?<!>>?) : Unit {
             if (((throwable != null) && declaredType?.isInstance(throwable)!!))
             {
                 throw declaredType?.cast(throwable)!!

compiler/testData/diagnostics/tests/inference/varargs/varargsAndPair.kt

@@ -1,4 +1,4 @@
-fun foo<T>(vararg <!UNUSED_PARAMETER!>ts<!>: T): T? = null
+fun foo<T: Any>(vararg <!UNUSED_PARAMETER!>ts<!>: T): T? = null
 
 class Pair<A>(a: A)
 

compiler/testData/diagnostics/tests/kt53.kt

@@ -1,4 +1,4 @@
-val <T, E> T.foo : E?
+val <T, E> T.foo : E<!BASE_WITH_NULLABLE_UPPER_BOUND!>?<!>
     get() = null
 
 fun test(): Int? {

compiler/testData/diagnostics/tests/nullableTypes/baseWithNullableUpperBound.kt

@@ -0,0 +1,24 @@
+fun nonMisleadingNullable<NN: Any, NNN: NN>(
+        <!UNUSED_PARAMETER!>nn<!>: NN?,
+        <!UNUSED_PARAMETER!>nnn<!>: NNN?
+) {}
+
+fun twoBounds<NN: Any, TWO_BOUNDS: Any>(
+        <!UNUSED_PARAMETER!>tb<!>: TWO_BOUNDS?
+
+) where TWO_BOUNDS : NN {}
+
+fun misleadingNullableSimple<T, N: T, INDIRECT: N>(
+        <!UNUSED_PARAMETER!>t<!>: T<!BASE_WITH_NULLABLE_UPPER_BOUND!>?<!>,
+        <!UNUSED_PARAMETER!>t2<!>: T<!BASE_WITH_NULLABLE_UPPER_BOUND!>?<!>,
+        <!UNUSED_PARAMETER!>n<!>: N<!BASE_WITH_NULLABLE_UPPER_BOUND!>?<!>,
+        <!UNUSED_PARAMETER!>ind<!>: INDIRECT<!BASE_WITH_NULLABLE_UPPER_BOUND!>?<!>
+) {}
+
+fun misleadingNullableMultiBound<FIRST_BOUND: Any?, SECOND_BOUND: Any>(
+        <!UNUSED_PARAMETER!>fb<!>: FIRST_BOUND<!BASE_WITH_NULLABLE_UPPER_BOUND!>?<!>,
+        <!UNUSED_PARAMETER!>sb<!>: SECOND_BOUND<!BASE_WITH_NULLABLE_UPPER_BOUND!>?<!>
+) where FIRST_BOUND: Any, SECOND_BOUND: Any? {
+}
+
+fun interactionWithRedundant<T>(<!UNUSED_PARAMETER!>t<!>: T<!BASE_WITH_NULLABLE_UPPER_BOUND!>?<!><!REDUNDANT_NULLABLE!>?<!>) {}

compiler/testData/diagnostics/tests/regressions/kt26.kt

@@ -7,6 +7,6 @@ import html.* // Must not be an error
 
 package html
 
-abstract class Factory<T> {
+abstract class Factory<T: Any> {
 fun create() : T? = null
 }

compiler/testData/diagnostics/tests/regressions/kt312.kt

@@ -1,6 +1,6 @@
 // KT-312 Nullability problem when a nullable version of a generic type is returned
 
-fun <T> Array<out T>.safeGet(index : Int) : T? {
+fun <T> Array<out T>.safeGet(index : Int) : T<!BASE_WITH_NULLABLE_UPPER_BOUND!>?<!> {
     return if (index < size) this[index] else null
 }
 

compiler/testData/diagnostics/tests/regressions/kt701.kt

@@ -3,7 +3,7 @@ fun getJavaClass<T>() : java.lang.Class<T> { return "" <!CAST_NEVER_SUCCEEDS!>as
 
 public class Throwables() {
     class object {
-        public fun propagateIfInstanceOf<X : Throwable?>(throwable : Throwable?, declaredType : Class<X?>?) {
+        public fun propagateIfInstanceOf<X : Throwable?>(throwable : Throwable?, declaredType : Class<X<!BASE_WITH_NULLABLE_UPPER_BOUND!>?<!>>?) {
             if (((throwable != null) && declaredType?.isInstance(throwable)!!))
             {
                 throw declaredType?.cast(throwable)!!

compiler/tests/org/jetbrains/jet/checkers/JetDiagnosticsTestGenerated.java

@@ -15,22 +15,19 @@
  */
 package org.jetbrains.jet.checkers;
 
-import junit.framework.Assert;
 import junit.framework.Test;
 import junit.framework.TestSuite;
-
-import java.io.File;
 import org.jetbrains.jet.JetTestUtils;
 import org.jetbrains.jet.test.InnerTestClasses;
 import org.jetbrains.jet.test.TestMetadata;
 
-import org.jetbrains.jet.checkers.AbstractDiagnosticsTestWithEagerResolve;
+import java.io.File;
 
 /** This class is generated by {@link org.jetbrains.jet.checkers.AbstractDiagnosticsTestWithEagerResolve}. DO NOT MODIFY MANUALLY */
 @InnerTestClasses({JetDiagnosticsTestGenerated.Tests.class, JetDiagnosticsTestGenerated.Script.class})
 public class JetDiagnosticsTestGenerated extends AbstractDiagnosticsTestWithEagerResolve {
     @TestMetadata("compiler/testData/diagnostics/tests")
-    @InnerTestClasses({Tests.Annotations.class, Tests.BackingField.class, Tests.Cast.class, Tests.CheckArguments.class, Tests.ControlFlowAnalysis.class, Tests.ControlStructures.class, Tests.DataClasses.class, Tests.DataFlow.class, Tests.DataFlowInfoTraversal.class, Tests.DeclarationChecks.class, Tests.Enum.class, Tests.Extensions.class, Tests.FunctionLiterals.class, Tests.Generics.class, Tests.IncompleteCode.class, Tests.Inference.class, Tests.Infos.class, Tests.J_k.class, Tests.Jdk_annotations.class, Tests.Library.class, Tests.NullabilityAndAutoCasts.class, Tests.Objects.class, Tests.OperatorsOverloading.class, Tests.Overload.class, Tests.Override.class, Tests.Redeclarations.class, Tests.Regressions.class, Tests.Scopes.class, Tests.Shadowing.class, Tests.Substitutions.class, Tests.Subtyping.class, Tests.Tuples.class, Tests.Varargs.class})
+    @InnerTestClasses({Tests.Annotations.class, Tests.BackingField.class, Tests.Cast.class, Tests.CheckArguments.class, Tests.ControlFlowAnalysis.class, Tests.ControlStructures.class, Tests.DataClasses.class, Tests.DataFlow.class, Tests.DataFlowInfoTraversal.class, Tests.DeclarationChecks.class, Tests.Enum.class, Tests.Extensions.class, Tests.FunctionLiterals.class, Tests.Generics.class, Tests.IncompleteCode.class, Tests.Inference.class, Tests.Infos.class, Tests.J_k.class, Tests.Jdk_annotations.class, Tests.Library.class, Tests.NullabilityAndAutoCasts.class, Tests.NullableTypes.class, Tests.Objects.class, Tests.OperatorsOverloading.class, Tests.Overload.class, Tests.Override.class, Tests.Redeclarations.class, Tests.Regressions.class, Tests.Scopes.class, Tests.Shadowing.class, Tests.Substitutions.class, Tests.Subtyping.class, Tests.Tuples.class, Tests.Varargs.class})
     public static class Tests extends AbstractDiagnosticsTestWithEagerResolve {
         @TestMetadata("Abstract.kt")
         public void testAbstract() throws Exception {
@@ -2294,6 +2291,11 @@ public class JetDiagnosticsTestGenerated extends AbstractDiagnosticsTestWithEage
                 JetTestUtils.assertAllTestsPresentByMetadata(this.getClass(), "org.jetbrains.jet.checkers.AbstractDiagnosticsTestWithEagerResolve", new File("compiler/testData/diagnostics/tests/nullableTypes"), "kt", true);
             }
             
+            @TestMetadata("baseWithNullableUpperBound.kt")
+            public void testBaseWithNullableUpperBound() throws Exception {
+                doTest("compiler/testData/diagnostics/tests/nullableTypes/baseWithNullableUpperBound.kt");
+            }
+            
             @TestMetadata("redundantNullable.kt")
             public void testRedundantNullable() throws Exception {
                 doTest("compiler/testData/diagnostics/tests/nullableTypes/redundantNullable.kt");
@@ -3541,6 +3543,7 @@ public class JetDiagnosticsTestGenerated extends AbstractDiagnosticsTestWithEage
             suite.addTest(Jdk_annotations.innerSuite());
             suite.addTestSuite(Library.class);
             suite.addTestSuite(NullabilityAndAutoCasts.class);
+            suite.addTestSuite(NullableTypes.class);
             suite.addTestSuite(Objects.class);
             suite.addTestSuite(OperatorsOverloading.class);
             suite.addTestSuite(Overload.class);