From fcb6055913461d60e5c85c4e98d22483b7f3e34a Mon Sep 17 00:00:00 2001 From: Andrey Breslav Date: Thu, 5 Feb 2015 19:18:10 +0300 Subject: [PATCH] SenselessComparisonChecker used for @NotNull values from Java --- .../load/kotlin/KotlinJvmCheckerProvider.kt | 32 ++++++++++--- .../BasicExpressionTypingVisitor.java | 5 +-- .../expressions/SenselessComparisonChecker.kt | 9 ++-- .../nullabilityWarnings/elvis.kt | 2 +- .../nullabilityWarnings/safeCall.kt | 2 + .../senselessComparisonEquals.kt | 45 +++++++++++++++++++ .../senselessComparisonEquals.txt | 3 ++ .../senselessComparisonIdentityEquals.kt | 43 ++++++++++++++++++ .../senselessComparisonIdentityEquals.txt | 3 ++ .../checkers/JetDiagnosticsTestGenerated.java | 12 +++++ 10 files changed, 141 insertions(+), 15 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/platformTypes/nullabilityWarnings/senselessComparisonEquals.kt create mode 100644 compiler/testData/diagnostics/tests/platformTypes/nullabilityWarnings/senselessComparisonEquals.txt create mode 100644 compiler/testData/diagnostics/tests/platformTypes/nullabilityWarnings/senselessComparisonIdentityEquals.kt create mode 100644 compiler/testData/diagnostics/tests/platformTypes/nullabilityWarnings/senselessComparisonIdentityEquals.txt diff --git a/compiler/frontend.java/src/org/jetbrains/kotlin/load/kotlin/KotlinJvmCheckerProvider.kt b/compiler/frontend.java/src/org/jetbrains/kotlin/load/kotlin/KotlinJvmCheckerProvider.kt index 4f8f746bbcb..69a62be9fce 100644 --- a/compiler/frontend.java/src/org/jetbrains/kotlin/load/kotlin/KotlinJvmCheckerProvider.kt +++ b/compiler/frontend.java/src/org/jetbrains/kotlin/load/kotlin/KotlinJvmCheckerProvider.kt @@ -62,6 +62,7 @@ import org.jetbrains.kotlin.resolve.calls.smartcasts.Nullability import org.jetbrains.kotlin.psi.JetPostfixExpression import org.jetbrains.kotlin.psi.JetBinaryExpression import org.jetbrains.kotlin.resolve.BindingContext +import org.jetbrains.kotlin.types.expressions.SenselessComparisonChecker public object KotlinJvmCheckerProvider : AdditionalCheckerProvider( annotationCheckers = listOf(PlatformStaticAnnotationChecker(), LocalFunInlineChecker(), ReifiedTypeParameterAnnotationChecker(), NativeFunChecker()), @@ -202,7 +203,7 @@ public class JavaNullabilityWarningsChecker : AdditionalTypeChecker { if (expression.getOperationToken() == JetTokens.EXCLEXCL) { val baseExpression = expression.getBaseExpression() val baseExpressionType = c.trace.get(BindingContext.EXPRESSION_TYPE, baseExpression) ?: return - warnIfNotNull( + doIfNotNull( DataFlowValueFactory.createDataFlowValue(baseExpression, baseExpressionType, c.trace.getBindingContext()), c ) { @@ -210,25 +211,42 @@ public class JavaNullabilityWarningsChecker : AdditionalTypeChecker { } } is JetBinaryExpression -> - if (expression.getOperationToken() == JetTokens.ELVIS) { + when (expression.getOperationToken()) { + JetTokens.ELVIS -> { val baseExpression = expression.getLeft() val baseExpressionType = c.trace.get(BindingContext.EXPRESSION_TYPE, baseExpression) ?: return - warnIfNotNull( + doIfNotNull( DataFlowValueFactory.createDataFlowValue(baseExpression, baseExpressionType, c.trace.getBindingContext()), c ) { c.trace.report(Errors.USELESS_ELVIS.on(expression.getOperationReference(), baseExpressionType)) } } + JetTokens.EQEQ, + JetTokens.EXCLEQ, + JetTokens.EQEQEQ, + JetTokens.EXCLEQEQEQ -> { + if (expression.getLeft() != null && expression.getRight() != null) { + SenselessComparisonChecker.checkSenselessComparisonWithNull( + expression, expression.getLeft(), expression.getRight(), c.trace, + { c.trace.get(BindingContext.EXPRESSION_TYPE, it) }, + { + value -> + doIfNotNull(value, c) { Nullability.NOT_NULL } ?: Nullability.UNKNOWN + } + ) + } + } + } } } - private fun warnIfNotNull(dataFlowValue: DataFlowValue, c: ResolutionContext<*>, reportWarning: () -> Unit) { + private fun doIfNotNull(dataFlowValue: DataFlowValue, c: ResolutionContext<*>, body: () -> T): T? { if (c.dataFlowInfo.getNullability(dataFlowValue).canBeNull() && dataFlowValue.getType().mustNotBeNull() == NullabilityInformationSource.JAVA) { - reportWarning() + return body() } - + return null } override fun checkReceiver( @@ -261,7 +279,7 @@ public class JavaNullabilityWarningsChecker : AdditionalTypeChecker { } else { // TODO: Compiler bug - warnIfNotNull(dataFlowValue, c as ResolutionContext<*>) { + doIfNotNull(dataFlowValue, c as ResolutionContext<*>) { c.trace.report(Errors.UNNECESSARY_SAFE_CALL.on(c.call.getCallOperationNode().getPsi(), receiverArgument.getType())) } } diff --git a/compiler/frontend/src/org/jetbrains/kotlin/types/expressions/BasicExpressionTypingVisitor.java b/compiler/frontend/src/org/jetbrains/kotlin/types/expressions/BasicExpressionTypingVisitor.java index e4282901ef8..e93ddc18136 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/types/expressions/BasicExpressionTypingVisitor.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/types/expressions/BasicExpressionTypingVisitor.java @@ -1152,7 +1152,7 @@ public class BasicExpressionTypingVisitor extends ExpressionTypingVisitor { context.trace.report(EQUALITY_NOT_APPLICABLE.on(expression, expression.getOperationReference(), leftType, rightType)); } SenselessComparisonChecker.checkSenselessComparisonWithNull( - expression, left, right, context, + expression, left, right, context.trace, new Function1() { @Override public JetType invoke(JetExpression expression) { @@ -1164,8 +1164,7 @@ public class BasicExpressionTypingVisitor extends ExpressionTypingVisitor { public Nullability invoke(DataFlowValue value) { return context.dataFlowInfo.getNullability(value); } - } - ); + }); } } } diff --git a/compiler/frontend/src/org/jetbrains/kotlin/types/expressions/SenselessComparisonChecker.kt b/compiler/frontend/src/org/jetbrains/kotlin/types/expressions/SenselessComparisonChecker.kt index 24eaa0064e4..bd801b79dd2 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/types/expressions/SenselessComparisonChecker.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/types/expressions/SenselessComparisonChecker.kt @@ -24,13 +24,14 @@ import org.jetbrains.kotlin.resolve.calls.smartcasts.Nullability import org.jetbrains.kotlin.diagnostics.Errors import kotlin.platform.platformStatic import org.jetbrains.kotlin.resolve.calls.smartcasts.DataFlowValue +import org.jetbrains.kotlin.resolve.BindingTrace object SenselessComparisonChecker { platformStatic fun checkSenselessComparisonWithNull( expression: JetBinaryExpression, left: JetExpression, right: JetExpression, - context: ExpressionTypingContext, + trace: BindingTrace, getType: (JetExpression) -> JetType?, getNullability: (DataFlowValue) -> Nullability ) { @@ -43,10 +44,10 @@ object SenselessComparisonChecker { if (type == null || type.isError()) return val operationSign = expression.getOperationReference() - val value = DataFlowValueFactory.createDataFlowValue(expr, type, context.trace.getBindingContext()) - val nullability = getNullability(value) + val value = DataFlowValueFactory.createDataFlowValue(expr, type, trace.getBindingContext()) val equality = operationSign.getReferencedNameElementType() == JetTokens.EQEQ || operationSign.getReferencedNameElementType() == JetTokens.EQEQEQ + val nullability = getNullability(value) val expressionIsAlways = if (nullability == Nullability.NULL) equality @@ -54,6 +55,6 @@ object SenselessComparisonChecker { else if (nullability == Nullability.IMPOSSIBLE) false else return - context.trace.report(Errors.SENSELESS_COMPARISON.on(expression, expression, expressionIsAlways)) + trace.report(Errors.SENSELESS_COMPARISON.on(expression, expression, expressionIsAlways)) } } \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/platformTypes/nullabilityWarnings/elvis.kt b/compiler/testData/diagnostics/tests/platformTypes/nullabilityWarnings/elvis.kt index 6223ccdd989..cd62522080d 100644 --- a/compiler/testData/diagnostics/tests/platformTypes/nullabilityWarnings/elvis.kt +++ b/compiler/testData/diagnostics/tests/platformTypes/nullabilityWarnings/elvis.kt @@ -1,4 +1,4 @@ -// !DIAGNOSTICS: -UNUSED_VARIABLE +// !DIAGNOSTICS: -UNUSED_VARIABLE -SENSELESS_COMPARISON // FILE: p/J.java package p; diff --git a/compiler/testData/diagnostics/tests/platformTypes/nullabilityWarnings/safeCall.kt b/compiler/testData/diagnostics/tests/platformTypes/nullabilityWarnings/safeCall.kt index bc6acd772f6..9c20b759ae9 100644 --- a/compiler/testData/diagnostics/tests/platformTypes/nullabilityWarnings/safeCall.kt +++ b/compiler/testData/diagnostics/tests/platformTypes/nullabilityWarnings/safeCall.kt @@ -1,3 +1,5 @@ +// !DIAGNOSTICS: -SENSELESS_COMPARISON + // FILE: p/J.java package p; diff --git a/compiler/testData/diagnostics/tests/platformTypes/nullabilityWarnings/senselessComparisonEquals.kt b/compiler/testData/diagnostics/tests/platformTypes/nullabilityWarnings/senselessComparisonEquals.kt new file mode 100644 index 00000000000..468ff5c4959 --- /dev/null +++ b/compiler/testData/diagnostics/tests/platformTypes/nullabilityWarnings/senselessComparisonEquals.kt @@ -0,0 +1,45 @@ +// !DIAGNOSTICS: -UNUSED_EXPRESSION + +// FILE: p/J.java +package p; + +import org.jetbrains.annotations.*; + +public class J { + @NotNull + public static J staticNN; + @Nullable + public static J staticN; + public static J staticJ; +} + +// FILE: k.kt + +import p.* + +fun test() { + // @NotNull platform type + val platformNN = J.staticNN + // @Nullable platform type + val platformN = J.staticN + // platform type with no annotation + val platformJ = J.staticJ + + val a: Any? = null + + if (platformNN != null) {} + if (null != platformNN) {} + if (platformNN == null) {} + if (null == platformNN) {} + + if (a != null && platformNN != a) {} + + if (platformN != null) {} + if (platformN == null) {} + if (a == null && platformN == a) {} + + if (platformJ != null) {} + if (platformJ == null) {} + if (a == null && platformJ == a) {} +} + diff --git a/compiler/testData/diagnostics/tests/platformTypes/nullabilityWarnings/senselessComparisonEquals.txt b/compiler/testData/diagnostics/tests/platformTypes/nullabilityWarnings/senselessComparisonEquals.txt new file mode 100644 index 00000000000..b8def46715c --- /dev/null +++ b/compiler/testData/diagnostics/tests/platformTypes/nullabilityWarnings/senselessComparisonEquals.txt @@ -0,0 +1,3 @@ +package + +internal fun test(): kotlin.Unit diff --git a/compiler/testData/diagnostics/tests/platformTypes/nullabilityWarnings/senselessComparisonIdentityEquals.kt b/compiler/testData/diagnostics/tests/platformTypes/nullabilityWarnings/senselessComparisonIdentityEquals.kt new file mode 100644 index 00000000000..6cab5d050bd --- /dev/null +++ b/compiler/testData/diagnostics/tests/platformTypes/nullabilityWarnings/senselessComparisonIdentityEquals.kt @@ -0,0 +1,43 @@ +// !DIAGNOSTICS: -UNUSED_EXPRESSION + +// FILE: p/J.java +package p; + +import org.jetbrains.annotations.*; + +public class J { + @NotNull + public static J staticNN; + @Nullable + public static J staticN; + public static J staticJ; +} + +// FILE: k.kt + +import p.* + +fun test() { + // @NotNull platform type + val platformNN = J.staticNN + // @Nullable platform type + val platformN = J.staticN + // platform type with no annotation + val platformJ = J.staticJ + + val a: Any? = null + + if (platformNN !== null) {} + if (null !== platformNN) {} + if (platformNN === null) {} + if (null === platformNN) {} + + if (platformN !== null) {} + if (platformN === null) {} + if (a === null && platformN === a) {} + + if (platformJ !== null) {} + if (platformJ === null) {} + if (a === null && platformJ === a) {} +} + diff --git a/compiler/testData/diagnostics/tests/platformTypes/nullabilityWarnings/senselessComparisonIdentityEquals.txt b/compiler/testData/diagnostics/tests/platformTypes/nullabilityWarnings/senselessComparisonIdentityEquals.txt new file mode 100644 index 00000000000..b8def46715c --- /dev/null +++ b/compiler/testData/diagnostics/tests/platformTypes/nullabilityWarnings/senselessComparisonIdentityEquals.txt @@ -0,0 +1,3 @@ +package + +internal fun test(): kotlin.Unit diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestGenerated.java index 457ec5ffe00..585e3109180 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/JetDiagnosticsTestGenerated.java @@ -8664,6 +8664,18 @@ public class JetDiagnosticsTestGenerated extends AbstractJetDiagnosticsTest { doTest(fileName); } + @TestMetadata("senselessComparisonEquals.kt") + public void testSenselessComparisonEquals() throws Exception { + String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/platformTypes/nullabilityWarnings/senselessComparisonEquals.kt"); + doTest(fileName); + } + + @TestMetadata("senselessComparisonIdentityEquals.kt") + public void testSenselessComparisonIdentityEquals() throws Exception { + String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/platformTypes/nullabilityWarnings/senselessComparisonIdentityEquals.kt"); + doTest(fileName); + } + @TestMetadata("throw.kt") public void testThrow() throws Exception { String fileName = JetTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/platformTypes/nullabilityWarnings/throw.kt");