Make useless elvis diagnostic more consistent for new and old inference

Also, remove diagnostics that can be covered by usual USELESS_ELVIS diagnostic
2018-04-05 16:43:52 +03:00
parent 4e11555c46
commit 612baacc25
13 changed files with 88 additions and 27 deletions
@@ -856,8 +856,6 @@ public interface Errors {
    DiagnosticFactory0<PsiElement> NOT_NULL_ASSERTION_ON_CALLABLE_REFERENCE = DiagnosticFactory0.create(WARNING);

    DiagnosticFactory1<KtBinaryExpression, KotlinType> USELESS_ELVIS = DiagnosticFactory1.create(WARNING, PositioningStrategies.USELESS_ELVIS);
-    DiagnosticFactory0<PsiElement> USELESS_ELVIS_ON_LAMBDA_EXPRESSION = DiagnosticFactory0.create(WARNING);
-    DiagnosticFactory0<PsiElement> USELESS_ELVIS_ON_CALLABLE_REFERENCE = DiagnosticFactory0.create(WARNING);
    DiagnosticFactory0<KtBinaryExpression> USELESS_ELVIS_RIGHT_IS_NULL =
            DiagnosticFactory0.create(WARNING, PositioningStrategies.USELESS_ELVIS);

@@ -509,8 +509,6 @@ public class DefaultErrorMessages {
        MAP.put(REPEATED_BOUND, "Type parameter already has this bound");
        MAP.put(DYNAMIC_UPPER_BOUND, "Dynamic type can not be used as an upper bound");
        MAP.put(USELESS_ELVIS, "Elvis operator (?:) always returns the left operand of non-nullable type {0}", RENDER_TYPE);
-        MAP.put(USELESS_ELVIS_ON_LAMBDA_EXPRESSION, "Left operand of elvis operator (?:) is a lambda expression");
-        MAP.put(USELESS_ELVIS_ON_CALLABLE_REFERENCE, "Left operand of elvis operator (?:) is a callable reference expression");
        MAP.put(USELESS_ELVIS_RIGHT_IS_NULL, "Right operand of elvis operator (?:) is useless if it is null");
        MAP.put(CONFLICTING_UPPER_BOUNDS, "Upper bounds of {0} have empty intersection", NAME);

@@ -94,7 +94,8 @@ private val DEFAULT_CALL_CHECKERS = listOf(
    CoroutineSuspendCallChecker, BuilderFunctionsCallChecker, DslScopeViolationCallChecker, MissingDependencyClassChecker,
    CallableReferenceCompatibilityChecker(), LateinitIntrinsicApplicabilityChecker,
    UnderscoreUsageChecker, AssigningNamedArgumentToVarargChecker(),
-    PrimitiveNumericComparisonCallChecker, LambdaWithSuspendModifierCallChecker
+    PrimitiveNumericComparisonCallChecker, LambdaWithSuspendModifierCallChecker,
+    UselessElvisCallChecker()
 )
 private val DEFAULT_TYPE_CHECKERS = emptyList<AdditionalTypeChecker>()
 private val DEFAULT_CLASSIFIER_USAGE_CHECKERS = listOf(
@@ -0,0 +1,50 @@
+/*
+ * Copyright 2010-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license
+ * that can be found in the license/LICENSE.txt file.
+ */
+
+package org.jetbrains.kotlin.resolve.calls.checkers
+
+import com.intellij.psi.PsiElement
+import org.jetbrains.kotlin.diagnostics.Errors
+import org.jetbrains.kotlin.diagnostics.reportDiagnosticOnce
+import org.jetbrains.kotlin.psi.KtBinaryExpression
+import org.jetbrains.kotlin.psi.KtPsiUtil
+import org.jetbrains.kotlin.resolve.calls.inference.model.TypeVariableTypeConstructor
+import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall
+import org.jetbrains.kotlin.resolve.calls.smartcasts.Nullability
+import org.jetbrains.kotlin.types.TypeUtils
+import org.jetbrains.kotlin.types.expressions.ControlStructureTypingUtils
+import org.jetbrains.kotlin.types.isError
+import org.jetbrains.kotlin.types.isNullabilityFlexible
+import org.jetbrains.kotlin.types.typeUtil.contains
+
+class UselessElvisCallChecker : CallChecker {
+    override fun check(resolvedCall: ResolvedCall<*>, reportOn: PsiElement, context: CallCheckerContext) {
+        if (resolvedCall.resultingDescriptor.name != ControlStructureTypingUtils.ResolveConstruct.ELVIS.specialFunctionName) return
+
+        val elvisBinaryExpression = resolvedCall.call.callElement as? KtBinaryExpression ?: return
+        val left = elvisBinaryExpression.left ?: return
+        val right = elvisBinaryExpression.right ?: return
+
+        val leftType = context.trace.getType(left) ?: return
+
+        // if type contains not fixed `TypeVariable` it means that call wasn't completed, we should wait for its completion first
+        if (leftType.isError || leftType.contains { it.constructor is TypeVariableTypeConstructor }) return
+
+        if (!TypeUtils.isNullableType(leftType)) {
+            context.trace.reportDiagnosticOnce(Errors.USELESS_ELVIS.on(elvisBinaryExpression, leftType))
+            return
+        }
+
+        val dataFlowValue = context.dataFlowValueFactory.createDataFlowValue(left, leftType, context.resolutionContext)
+        if (context.dataFlowInfo.getStableNullability(dataFlowValue) == Nullability.NOT_NULL) {
+            context.trace.reportDiagnosticOnce(Errors.USELESS_ELVIS.on(elvisBinaryExpression, leftType))
+            return
+        }
+
+        if (KtPsiUtil.isNullConstant(right) && !leftType.isNullabilityFlexible()) {
+            context.trace.reportDiagnosticOnce(Errors.USELESS_ELVIS_RIGHT_IS_NULL.on(elvisBinaryExpression))
+        }
+    }
+}
@@ -1254,19 +1254,10 @@ public class BasicExpressionTypingVisitor extends ExpressionTypingVisitor {
        boolean isLeftFunctionLiteral = ArgumentTypeResolver.isFunctionLiteralArgument(left, context);
        boolean isLeftCallableReference = ArgumentTypeResolver.isCallableReferenceArgument(left, context);
        if (leftTypeInfo == null && (isLeftFunctionLiteral || isLeftCallableReference)) {
-            DiagnosticFactory0<PsiElement> diagnosticFactory =
-                    isLeftFunctionLiteral ? USELESS_ELVIS_ON_LAMBDA_EXPRESSION : USELESS_ELVIS_ON_CALLABLE_REFERENCE;
-            context.trace.report(diagnosticFactory.on(expression.getOperationReference()));
            return TypeInfoFactoryKt.noTypeInfo(context);
        }
        assert leftTypeInfo != null : "Left expression was not processed: " + expression;
        KotlinType leftType = leftTypeInfo.getType();
-        if (isKnownToBeNotNull(left, leftType, context)) {
-            context.trace.report(USELESS_ELVIS.on(expression, leftType));
-        }
-        else if (KtPsiUtil.isNullConstant(right) && leftType != null && !FlexibleTypesKt.isNullabilityFlexible(leftType)) {
-            context.trace.report(USELESS_ELVIS_RIGHT_IS_NULL.on(expression));
-        }
        KotlinTypeInfo rightTypeInfo = BindingContextUtils.getRecordedTypeInfo(right, context.trace.getBindingContext());
        if (rightTypeInfo == null && ArgumentTypeResolver.isFunctionLiteralOrCallableReference(right, context)) {
            // the type is computed later in call completer according to the '?:' semantics as a function
@@ -1,4 +1,3 @@
-// !WITH_NEW_INFERENCE
 // !DIAGNOSTICS: -UNUSED_PARAMETER, -UNUSED_VARIABLE

 fun baz(i: Int) = i
@@ -9,7 +8,7 @@ fun nullableFun(): ((Int) -> Int)? = null
 fun test() {
    val x1: (Int) -> Int = bar(if (true) ::baz else ::baz)
    val x2: (Int) -> Int = bar(nullableFun() ?: ::baz)
-    val x3: (Int) -> Int = bar(::baz <!OI;USELESS_ELVIS!><!NI;USELESS_ELVIS_ON_CALLABLE_REFERENCE!>?:<!> ::baz<!>)
+    val x3: (Int) -> Int = bar(::baz <!USELESS_ELVIS!>?: ::baz<!>)

    val i = 0
    val x4: (Int) -> Int = bar(when (i) {
@@ -2,20 +2,20 @@
 // See KT-8277
 // NI_EXPECTED_FILE

-val v = { true } <!USELESS_ELVIS!>?: ( { true } <!USELESS_ELVIS_ON_LAMBDA_EXPRESSION!>?:<!> null!! )<!>
+val v = { true } <!USELESS_ELVIS!>?: ( { true } <!USELESS_ELVIS!>?:null!!<!> )<!>

 val w = if (true) {
    { true }
 }
 else {
-    { true } <!USELESS_ELVIS_ON_LAMBDA_EXPRESSION!>?:<!> null!!
+    { true } <!USELESS_ELVIS!>?: null!!<!>
 }

 val ww = if (true) {
-    <!OI;TYPE_MISMATCH!>{ true }<!> <!USELESS_ELVIS_ON_LAMBDA_EXPRESSION!>?:<!> null!!
+    <!OI;TYPE_MISMATCH!>{ true }<!> <!USELESS_ELVIS!>?: null!!<!>
 }
 else if (true) {
-    <!OI;TYPE_MISMATCH!>{ true }<!> <!USELESS_ELVIS_ON_LAMBDA_EXPRESSION!>?:<!> null!!
+    <!OI;TYPE_MISMATCH!>{ true }<!> <!USELESS_ELVIS!>?: null!!<!>
 }
 else {
    null!!
@@ -34,6 +34,6 @@ val bbb = null ?: ( l() <!USELESS_ELVIS_RIGHT_IS_NULL!>?: null<!>)
 val bbbb = ( l() <!USELESS_ELVIS_RIGHT_IS_NULL!>?: null<!>) ?: ( l() <!USELESS_ELVIS_RIGHT_IS_NULL!>?: null<!>)

 fun f(x : Long?): Long {
-    var a = x ?: (<!NI;TYPE_MISMATCH, NI;TYPE_MISMATCH, TYPE_MISMATCH!>fun() {}<!> <!OI;USELESS_ELVIS!><!NI;USELESS_ELVIS_ON_LAMBDA_EXPRESSION!>?:<!> <!NI;TYPE_MISMATCH, NI;TYPE_MISMATCH, TYPE_MISMATCH!>fun() {}<!><!>)
+    var a = x ?: (<!NI;TYPE_MISMATCH, NI;TYPE_MISMATCH, TYPE_MISMATCH!>fun() {}<!> <!USELESS_ELVIS!>?: <!NI;TYPE_MISMATCH, NI;TYPE_MISMATCH, TYPE_MISMATCH!>fun() {}<!><!>)
    return <!OI;DEBUG_INFO_ELEMENT_WITH_ERROR_TYPE!>a<!>
 }
@@ -9,7 +9,7 @@ fun test() {

    // KT-KT-9070
    <!TYPE_MISMATCH!>{ }<!> <!USELESS_ELVIS!>?: 1<!>
-    use(<!NI;TYPE_MISMATCH, NI;TYPE_MISMATCH, NI;TYPE_MISMATCH!>{ 2 }<!> <!USELESS_ELVIS_ON_LAMBDA_EXPRESSION!>?:<!> 1);
+    use(<!NI;TYPE_MISMATCH, NI;TYPE_MISMATCH, NI;TYPE_MISMATCH!>{ 2 }<!> <!USELESS_ELVIS!>?: 1<!>);

    1 <!USELESS_ELVIS!>?: <!TYPE_MISMATCH, UNUSED_LAMBDA_EXPRESSION!>{ }<!><!>
    use(1 <!USELESS_ELVIS!>?: <!NI;TYPE_MISMATCH, NI;TYPE_MISMATCH, NI;TYPE_MISMATCH!>{ }<!><!>)
@@ -1,4 +1,3 @@
-// !WITH_NEW_INFERENCE
 // !DIAGNOSTICS: -UNUSED_PARAMETER, -SENSELESS_COMPARISON, -DEBUG_INFO_SMARTCAST

 fun <T: Any?> test1(t: Any?): Any {
@@ -23,18 +22,18 @@ fun <T> nullable(): T? = null
 fun <T> dependOn(x: T) = x

 fun test() {
-    takeNotNull(notNull() <!USELESS_ELVIS!>?: ""<!>)
+    takeNotNull(notNull() ?: "")
    takeNotNull(nullable() ?: "")

    val x: String? = null
-    takeNotNull(dependOn(x) <!NI;USELESS_ELVIS!>?: ""<!>)
-    takeNotNull(dependOn(dependOn(x)) <!NI;USELESS_ELVIS!>?: ""<!>)
+    takeNotNull(dependOn(x) ?: "")
+    takeNotNull(dependOn(dependOn(x)) ?: "")
    takeNotNull(dependOn(dependOn(x as String)) <!USELESS_ELVIS!>?: ""<!>)

    if (x != null) {
        takeNotNull(dependOn(x) <!USELESS_ELVIS!>?: ""<!>)
        takeNotNull(dependOn(dependOn(x)) <!USELESS_ELVIS!>?: ""<!>)
-        takeNotNull(dependOn(dependOn(x) as? String) <!NI;USELESS_ELVIS!>?: ""<!>)
+        takeNotNull(dependOn(dependOn(x) as? String) ?: "")
    }

    takeNotNull(bar()!!)
@@ -0,0 +1,8 @@
+// !DIAGNOSTICS: -UNUSED_PARAMETER
+
+fun test() {
+    take(nullable() ?: nullable() ?: "foo")
+}
+
+fun <T> nullable(): T? = TODO()
+fun take(x: Any) {}
@@ -0,0 +1,5 @@
+package
+
+public fun </*0*/ T> nullable(): T?
+public fun take(/*0*/ x: kotlin.Any): kotlin.Unit
+public fun test(): kotlin.Unit
@@ -16903,6 +16903,12 @@ public class DiagnosticsTestGenerated extends AbstractDiagnosticsTest {
                    doTest(fileName);
                }

+                @TestMetadata("noWarningOnDoubleElvis.kt")
+                public void testNoWarningOnDoubleElvis() throws Exception {
+                    String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/platformTypes/nullabilityWarnings/noWarningOnDoubleElvis.kt");
+                    doTest(fileName);
+                }
+
                @TestMetadata("notNullAfterSafeCall.kt")
                public void testNotNullAfterSafeCall() throws Exception {
                    String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/platformTypes/nullabilityWarnings/notNullAfterSafeCall.kt");
@@ -16903,6 +16903,12 @@ public class DiagnosticsUsingJavacTestGenerated extends AbstractDiagnosticsUsing
                    doTest(fileName);
                }

+                @TestMetadata("noWarningOnDoubleElvis.kt")
+                public void testNoWarningOnDoubleElvis() throws Exception {
+                    String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/platformTypes/nullabilityWarnings/noWarningOnDoubleElvis.kt");
+                    doTest(fileName);
+                }
+
                @TestMetadata("notNullAfterSafeCall.kt")
                public void testNotNullAfterSafeCall() throws Exception {
                    String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/platformTypes/nullabilityWarnings/notNullAfterSafeCall.kt");