From 4a7b4d655cbb43512a088cfe0d3c5a233b66a3c2 Mon Sep 17 00:00:00 2001 From: Pavel Kirpichenkov Date: Fri, 14 Feb 2020 13:37:12 +0300 Subject: [PATCH] [NI] Fix completion for ILT when Nothing constraint is present Full completion should not be done if lower `Nothing` is the only proper constraint when constraint with ILT type is present. ILT will be selected as a resulting type and transformed into `Int` without attention to possible restrictions from outer calls. --- ...irOldFrontendDiagnosticsTestGenerated.java | 5 +++ .../components/CompletionModeCalculator.kt | 39 +++++++++++----- .../partialForIltWithNothing.fir.kt | 44 +++++++++++++++++++ .../completion/partialForIltWithNothing.kt | 44 +++++++++++++++++++ .../completion/partialForIltWithNothing.txt | 3 ++ .../testData/diagnostics/tests/when/kt9929.kt | 2 +- .../checkers/DiagnosticsTestGenerated.java | 5 +++ .../DiagnosticsUsingJavacTestGenerated.java | 5 +++ .../checkers/AbstractFirPsiCheckerTest.kt | 2 +- 9 files changed, 136 insertions(+), 13 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/inference/completion/partialForIltWithNothing.fir.kt create mode 100644 compiler/testData/diagnostics/tests/inference/completion/partialForIltWithNothing.kt create mode 100644 compiler/testData/diagnostics/tests/inference/completion/partialForIltWithNothing.txt diff --git a/compiler/fir/resolve/tests/org/jetbrains/kotlin/fir/FirOldFrontendDiagnosticsTestGenerated.java b/compiler/fir/resolve/tests/org/jetbrains/kotlin/fir/FirOldFrontendDiagnosticsTestGenerated.java index 83128c28ef8..c9ea1000714 100644 --- a/compiler/fir/resolve/tests/org/jetbrains/kotlin/fir/FirOldFrontendDiagnosticsTestGenerated.java +++ b/compiler/fir/resolve/tests/org/jetbrains/kotlin/fir/FirOldFrontendDiagnosticsTestGenerated.java @@ -10662,6 +10662,11 @@ public class FirOldFrontendDiagnosticsTestGenerated extends AbstractFirOldFronte runTest("compiler/testData/diagnostics/tests/inference/completion/partialForIlt.kt"); } + @TestMetadata("partialForIltWithNothing.kt") + public void testPartialForIltWithNothing() throws Exception { + runTest("compiler/testData/diagnostics/tests/inference/completion/partialForIltWithNothing.kt"); + } + @TestMetadata("transitiveConstraint.kt") public void testTransitiveConstraint() throws Exception { runTest("compiler/testData/diagnostics/tests/inference/completion/transitiveConstraint.kt"); diff --git a/compiler/resolution/src/org/jetbrains/kotlin/resolve/calls/components/CompletionModeCalculator.kt b/compiler/resolution/src/org/jetbrains/kotlin/resolve/calls/components/CompletionModeCalculator.kt index f2fc75067e8..c3a3cc42e4f 100644 --- a/compiler/resolution/src/org/jetbrains/kotlin/resolve/calls/components/CompletionModeCalculator.kt +++ b/compiler/resolution/src/org/jetbrains/kotlin/resolve/calls/components/CompletionModeCalculator.kt @@ -52,7 +52,7 @@ class CompletionModeCalculator { private val candidate: KotlinResolutionCandidate, private val returnType: UnwrappedType?, private val csCompleterContext: CsCompleterContext, - private val trivialConstraintTypeInferenceOracle: TrivialConstraintTypeInferenceOracle + private val trivialConstraintTypeInferenceOracle: TrivialConstraintTypeInferenceOracle, ) { private enum class FixationDirection { TO_SUBTYPE, EQUALITY @@ -192,13 +192,32 @@ class CompletionModeCalculator { val constraints = variableWithConstraints.constraints val variable = variableWithConstraints.typeVariable - // todo check correctness for @Exact - return constraints.isNotEmpty() && constraints.any { constraint -> - constraint.hasRequiredKind(direction) - && isProperType(constraint.type) - && !constraint.type.typeConstructor().isIntegerLiteralTypeConstructor() - && !isNothingConstraintForPartiallyAnalyzedVariable(constraint, variable) + // ILT constraint tracking is necessary to prevent incorrect full completion from Nothing constraint + // Consider ILT <: T; Nothing <: T for T requiring lower constraint + // Nothing would trigger full completion, but resulting type would be Int + // Possible restrictions on integer constant from outer calls would be ignored + + var iltConstraintPresent = false + var properConstraintPresent = false + var nonNothingProperConstraintPresent = false + + for (constraint in constraints) { + if (!constraint.hasRequiredKind(direction) || !isProperType(constraint.type)) + continue + + if (constraint.type.typeConstructor().isIntegerLiteralTypeConstructor()) { + iltConstraintPresent = true + } else if (trivialConstraintTypeInferenceOracle.isSuitableResultedType(constraint.type)) { + properConstraintPresent = true + nonNothingProperConstraintPresent = true + } else if (!isLowerConstraintForPartiallyAnalyzedVariable(constraint, variable)) { + properConstraintPresent = true + } } + + if (!properConstraintPresent) return false + + return !iltConstraintPresent || nonNothingProperConstraintPresent } private fun Constraint.hasRequiredKind(direction: FixationDirection) = when (direction) { @@ -206,13 +225,11 @@ class CompletionModeCalculator { FixationDirection.EQUALITY -> kind.isEqual() } - private fun CsCompleterContext.isNothingConstraintForPartiallyAnalyzedVariable( + private fun CsCompleterContext.isLowerConstraintForPartiallyAnalyzedVariable( constraint: Constraint, variable: TypeVariableMarker ): Boolean { - if (trivialConstraintTypeInferenceOracle.isSuitableResultedType(constraint.type) || !constraint.kind.isLower()) - return false - return postponedAtoms.any { atom -> + return constraint.kind.isLower() && postponedAtoms.any { atom -> atom.expectedType?.contains { type -> variable.defaultType() == type } ?: false } } diff --git a/compiler/testData/diagnostics/tests/inference/completion/partialForIltWithNothing.fir.kt b/compiler/testData/diagnostics/tests/inference/completion/partialForIltWithNothing.fir.kt new file mode 100644 index 00000000000..4d2efdc6084 --- /dev/null +++ b/compiler/testData/diagnostics/tests/inference/completion/partialForIltWithNothing.fir.kt @@ -0,0 +1,44 @@ +// !LANGUAGE: +NewInference +// !DIAGNOSTICS: -UNUSED_VARIABLE + +fun test(boolean: Boolean) { + val expectedLong: Long = if (boolean) { + if (boolean) { + 42 + } else { + return + } + } else { + return + } + + val expectedInt: Int = if (boolean) { + if (boolean) { + 42 + } else { + return + } + } else { + return + } + + val expectedShort: Short = if (boolean) { + if (boolean) { + 42 + } else { + return + } + } else { + return + } + + val expectedByte: Byte = if (boolean) { + if (boolean) { + 42 + } else { + return + } + } else { + return + } +} diff --git a/compiler/testData/diagnostics/tests/inference/completion/partialForIltWithNothing.kt b/compiler/testData/diagnostics/tests/inference/completion/partialForIltWithNothing.kt new file mode 100644 index 00000000000..16cabca67dd --- /dev/null +++ b/compiler/testData/diagnostics/tests/inference/completion/partialForIltWithNothing.kt @@ -0,0 +1,44 @@ +// !LANGUAGE: +NewInference +// !DIAGNOSTICS: -UNUSED_VARIABLE + +fun test(boolean: Boolean) { + val expectedLong: Long = if (boolean) { + if (boolean) { + 42 + } else { + return + } + } else { + return + } + + val expectedInt: Int = if (boolean) { + if (boolean) { + 42 + } else { + return + } + } else { + return + } + + val expectedShort: Short = if (boolean) { + if (boolean) { + 42 + } else { + return + } + } else { + return + } + + val expectedByte: Byte = if (boolean) { + if (boolean) { + 42 + } else { + return + } + } else { + return + } +} diff --git a/compiler/testData/diagnostics/tests/inference/completion/partialForIltWithNothing.txt b/compiler/testData/diagnostics/tests/inference/completion/partialForIltWithNothing.txt new file mode 100644 index 00000000000..2e9c8581c69 --- /dev/null +++ b/compiler/testData/diagnostics/tests/inference/completion/partialForIltWithNothing.txt @@ -0,0 +1,3 @@ +package + +public fun test(/*0*/ boolean: kotlin.Boolean): kotlin.Unit diff --git a/compiler/testData/diagnostics/tests/when/kt9929.kt b/compiler/testData/diagnostics/tests/when/kt9929.kt index f1dc2682288..c94ac61f194 100644 --- a/compiler/testData/diagnostics/tests/when/kt9929.kt +++ b/compiler/testData/diagnostics/tests/when/kt9929.kt @@ -1,5 +1,5 @@ // !WITH_NEW_INFERENCE -val test: Int = if (true) { +val test: Int = if (true) { when (2) { 1 -> 1 else -> null diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java index cb68a01c195..73f8ec451c9 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java @@ -10669,6 +10669,11 @@ public class DiagnosticsTestGenerated extends AbstractDiagnosticsTest { runTest("compiler/testData/diagnostics/tests/inference/completion/partialForIlt.kt"); } + @TestMetadata("partialForIltWithNothing.kt") + public void testPartialForIltWithNothing() throws Exception { + runTest("compiler/testData/diagnostics/tests/inference/completion/partialForIltWithNothing.kt"); + } + @TestMetadata("transitiveConstraint.kt") public void testTransitiveConstraint() throws Exception { runTest("compiler/testData/diagnostics/tests/inference/completion/transitiveConstraint.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java index f82e64c9b03..9d13b211964 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java @@ -10664,6 +10664,11 @@ public class DiagnosticsUsingJavacTestGenerated extends AbstractDiagnosticsUsing runTest("compiler/testData/diagnostics/tests/inference/completion/partialForIlt.kt"); } + @TestMetadata("partialForIltWithNothing.kt") + public void testPartialForIltWithNothing() throws Exception { + runTest("compiler/testData/diagnostics/tests/inference/completion/partialForIltWithNothing.kt"); + } + @TestMetadata("transitiveConstraint.kt") public void testTransitiveConstraint() throws Exception { runTest("compiler/testData/diagnostics/tests/inference/completion/transitiveConstraint.kt"); diff --git a/idea/tests/org/jetbrains/kotlin/checkers/AbstractFirPsiCheckerTest.kt b/idea/tests/org/jetbrains/kotlin/checkers/AbstractFirPsiCheckerTest.kt index 036c2c9958f..dda0ad66591 100644 --- a/idea/tests/org/jetbrains/kotlin/checkers/AbstractFirPsiCheckerTest.kt +++ b/idea/tests/org/jetbrains/kotlin/checkers/AbstractFirPsiCheckerTest.kt @@ -39,7 +39,7 @@ abstract class AbstractFirPsiCheckerTest : AbstractPsiCheckerTest() { throw FileComparisonFailure(e.message, e.expected, e.actual, File(e.filePath).absolutePath) } else { // Here we just check that we haven't crashed due to exception - 0L + 0 } } finally { if (configured) {