KT-34644 Give return keyword more priority in completion where appropriate

- See `returnExpressionItems::returnIsProbableInPosition` to get an idea where return is supposed to be more appropriate
  - End of the block (except for top level block of unit function or cycle) or as single expression in if or when
  - Right side of the elvis operator (which is not already in return)
- Add additional `probable_keyword` weigher to signalize about probable keyword (for now it is only for `return`)
- ^KT-34644 Fixed
This commit is contained in:
Roman Golyshev
2019-11-08 18:23:58 +03:00
committed by Roman Golyshev
parent de11b4cb64
commit faf4c05fc8
32 changed files with 546 additions and 4 deletions
@@ -14,22 +14,23 @@ import com.intellij.openapi.util.Key
import com.intellij.patterns.ElementPattern import com.intellij.patterns.ElementPattern
import com.intellij.patterns.StandardPatterns import com.intellij.patterns.StandardPatterns
import com.intellij.psi.PsiDocumentManager import com.intellij.psi.PsiDocumentManager
import com.intellij.psi.PsiElement
import org.jetbrains.kotlin.builtins.KotlinBuiltIns import org.jetbrains.kotlin.builtins.KotlinBuiltIns
import org.jetbrains.kotlin.builtins.isFunctionType import org.jetbrains.kotlin.builtins.isFunctionType
import org.jetbrains.kotlin.descriptors.* import org.jetbrains.kotlin.descriptors.*
import org.jetbrains.kotlin.idea.KotlinIcons import org.jetbrains.kotlin.idea.KotlinIcons
import org.jetbrains.kotlin.idea.completion.handlers.CastReceiverInsertHandler import org.jetbrains.kotlin.idea.completion.handlers.CastReceiverInsertHandler
import org.jetbrains.kotlin.idea.completion.handlers.WithTailInsertHandler import org.jetbrains.kotlin.idea.completion.handlers.WithTailInsertHandler
import org.jetbrains.kotlin.idea.completion.smart.isProbableKeyword
import org.jetbrains.kotlin.idea.core.ImportableFqNameClassifier import org.jetbrains.kotlin.idea.core.ImportableFqNameClassifier
import org.jetbrains.kotlin.idea.core.ShortenReferences import org.jetbrains.kotlin.idea.core.ShortenReferences
import org.jetbrains.kotlin.idea.imports.importableFqName import org.jetbrains.kotlin.idea.imports.importableFqName
import org.jetbrains.kotlin.idea.resolve.ResolutionFacade import org.jetbrains.kotlin.idea.resolve.ResolutionFacade
import org.jetbrains.kotlin.idea.util.* import org.jetbrains.kotlin.idea.util.*
import org.jetbrains.kotlin.lexer.KtTokens
import org.jetbrains.kotlin.name.Name import org.jetbrains.kotlin.name.Name
import org.jetbrains.kotlin.psi.* import org.jetbrains.kotlin.psi.*
import org.jetbrains.kotlin.psi.psiUtil.endOffset import org.jetbrains.kotlin.psi.psiUtil.*
import org.jetbrains.kotlin.psi.psiUtil.parentsWithSelf
import org.jetbrains.kotlin.psi.psiUtil.startOffset
import org.jetbrains.kotlin.renderer.render import org.jetbrains.kotlin.renderer.render
import org.jetbrains.kotlin.resolve.BindingContext import org.jetbrains.kotlin.resolve.BindingContext
import org.jetbrains.kotlin.resolve.DescriptorUtils import org.jetbrains.kotlin.resolve.DescriptorUtils
@@ -223,6 +224,22 @@ fun returnExpressionItems(bindingContext: BindingContext, position: KtElement):
} }
} }
val isOnTopLevelInUnitFunction = isUnit && position.parent?.parent === parent
val isInsideLambda = position.getNonStrictParentOfType<KtFunctionLiteral>()?.let { parent.isAncestor(it) } == true
fun returnIsProbableInPosition(): Boolean = when {
isInsideLambda -> false // for now we do not want to alter completion inside lambda bodies
position.inReturnExpression() -> false
position.isRightOperandInElvis() -> true
position.isLastOrSingleStatement() && !position.isDirectlyInLoopBody() && !isOnTopLevelInUnitFunction -> true
else -> false
}
if (returnIsProbableInPosition()) {
blockBodyReturns.forEach { it.isProbableKeyword = true }
}
result.addAll(blockBodyReturns) result.addAll(blockBodyReturns)
} }
break break
@@ -232,6 +249,56 @@ fun returnExpressionItems(bindingContext: BindingContext, position: KtElement):
return result return result
} }
private fun KtElement.isDirectlyInLoopBody(): Boolean {
val possibleLoop = when (parent) {
is KtBlockExpression -> parent.parent?.parent
is KtContainerNodeForControlStructureBody -> parent.parent
else -> null
}
return possibleLoop is KtLoopExpression
}
fun KtElement.isRightOperandInElvis(): Boolean {
val elvisParent = parent as? KtBinaryExpression ?: return false
return elvisParent.operationToken == KtTokens.ELVIS && elvisParent.right === this
}
/**
* Checks if expression is either last expression in a block, or a single expression in position where single
* expressions are allowed (`when` entries, `for` and `while` loops, and `if`s).
*/
private fun PsiElement.isLastOrSingleStatement(): Boolean =
when (val containingExpression = parent) {
is KtBlockExpression -> containingExpression.statements.lastOrNull() === this
is KtWhenEntry, is KtContainerNodeForControlStructureBody -> true
else -> false
}
private fun KtElement.inReturnExpression(): Boolean = findReturnExpression(this) != null
/**
* If [expression] is directly relates to the return expression already, this return expression will be found.
*
* Examples:
*
* ```kotlin
* return 10 // 10 is in return
* return if (true) 10 else 20 // 10 and 20 are in return
* return 10 ?: 20 // 10 and 20 are in return
* return when { true -> 10 ; else -> { 20; 30 } } // 10 and 30 are in return, but 20 is not
* ```
*/
private tailrec fun findReturnExpression(expression: PsiElement?): KtReturnExpression? =
when (val parent = expression?.parent) {
is KtReturnExpression -> parent
is KtBinaryExpression -> findReturnExpression(parent.takeIf { it.operationToken == KtTokens.ELVIS })
is KtContainerNodeForControlStructureBody, is KtIfExpression -> findReturnExpression(parent)
is KtBlockExpression -> findReturnExpression(parent.takeIf { expression.isLastOrSingleStatement() })
is KtWhenEntry -> findReturnExpression(parent.parent)
else -> null
}
private fun KtDeclarationWithBody.returnType(bindingContext: BindingContext): KotlinType? { private fun KtDeclarationWithBody.returnType(bindingContext: BindingContext): KotlinType? {
val callable = bindingContext[BindingContext.DECLARATION_TO_DESCRIPTOR, this] as? CallableDescriptor ?: return null val callable = bindingContext[BindingContext.DECLARATION_TO_DESCRIPTOR, this] as? CallableDescriptor ?: return null
return callable.returnType return callable.returnType
@@ -115,6 +115,7 @@ object SmartCompletionPriorityWeigher : LookupElementWeigher("kotlin.smartComple
object KindWeigher : LookupElementWeigher("kotlin.kind") { object KindWeigher : LookupElementWeigher("kotlin.kind") {
private enum class Weight { private enum class Weight {
probableKeyword,
enumMember, enumMember,
callable, callable,
keyword, keyword,
@@ -137,7 +138,7 @@ object KindWeigher : LookupElementWeigher("kotlin.kind") {
} }
} }
is KeywordLookupObject -> Weight.keyword is KeywordLookupObject -> if (element.isProbableKeyword) Weight.probableKeyword else Weight.keyword
else -> Weight.default else -> Weight.default
} }
@@ -33,6 +33,7 @@ import org.jetbrains.kotlin.idea.completion.suppressAutoInsertion
import org.jetbrains.kotlin.idea.core.* import org.jetbrains.kotlin.idea.core.*
import org.jetbrains.kotlin.idea.resolve.ResolutionFacade import org.jetbrains.kotlin.idea.resolve.ResolutionFacade
import org.jetbrains.kotlin.idea.util.* import org.jetbrains.kotlin.idea.util.*
import org.jetbrains.kotlin.psi.NotNullableUserDataProperty
import org.jetbrains.kotlin.resolve.BindingContext import org.jetbrains.kotlin.resolve.BindingContext
import org.jetbrains.kotlin.types.TypeSubstitutor import org.jetbrains.kotlin.types.TypeSubstitutor
import org.jetbrains.kotlin.types.expressions.DoubleColonExpressionResolver import org.jetbrains.kotlin.types.expressions.DoubleColonExpressionResolver
@@ -298,6 +299,8 @@ fun LookupElement.assignSmartCompletionPriority(priority: SmartCompletionItemPri
return this return this
} }
var LookupElement.isProbableKeyword: Boolean by NotNullableUserDataProperty(Key.create("PROBABLE_KEYWORD_KEY"), false)
fun DeclarationDescriptor.fuzzyTypesForSmartCompletion( fun DeclarationDescriptor.fuzzyTypesForSmartCompletion(
smartCastCalculator: SmartCastCalculator, smartCastCalculator: SmartCastCalculator,
callTypeAndReceiver: CallTypeAndReceiver<*, *>, callTypeAndReceiver: CallTypeAndReceiver<*, *>,
@@ -0,0 +1,11 @@
fun returnFun() {}
fun usage() {
if (true) {
re<caret>
return
}
}
// ORDER: returnFun
// ORDER: return
@@ -0,0 +1,9 @@
fun returnFun() {}
fun usage() {
re<caret>
return
}
// ORDER: returnFun
// ORDER: return
@@ -0,0 +1,10 @@
fun returnFun() {}
fun usage() {
if (true) {
re<caret>
}
}
// ORDER: return
// ORDER: returnFun
@@ -0,0 +1,8 @@
fun returnFun() {}
fun usage() {
re<caret>
}
// ORDER: returnFun
// ORDER: return
@@ -0,0 +1,10 @@
fun returnFun() {}
fun usage() {
for (i in 1..10) {
re<caret>
}
}
// ORDER: returnFun
// ORDER: return
@@ -0,0 +1,8 @@
fun returnFun() {}
fun usage() {
for (i in 1..10) re<caret>
}
// ORDER: returnFun
// ORDER: return
@@ -0,0 +1,8 @@
fun returnFun() {}
fun usage() {
if (true) re<caret>
}
// ORDER: return
// ORDER: returnFun
@@ -0,0 +1,9 @@
fun returnFun() {}
fun usage(a: Int?) {
a ?: re<caret>
return
}
// ORDER: return
// ORDER: returnFun
@@ -0,0 +1,12 @@
fun returnFun(): Int = 10
fun <T> returnAnything(): T = null!!
fun usage(a: Int?) {
a ?: re<caret>
}
// function of the same type as `a` is preferred to return in this case
// ORDER: returnFun
// ORDER: return
// ORDER: returnAnything
@@ -0,0 +1,11 @@
fun returnFun() {}
fun usage(a: Int) {
when (a) {
10 -> re<caret>
}
return
}
// ORDER: return
// ORDER: returnFun
@@ -0,0 +1,13 @@
fun returnFun() {}
fun usage(a: Int) {
when (a) {
10 -> {
re<caret>
}
}
return
}
// ORDER: return
// ORDER: returnFun
@@ -0,0 +1,13 @@
fun returnFun(): Int = 10
fun usage(): Int {
if (true) {
re<caret>
return 20
}
return 10
}
// ORDER: returnFun
// ORDER: return
@@ -0,0 +1,9 @@
fun returnFun(): Int = 10
fun usage(): Int {
re<caret>
return 10
}
// ORDER: returnFun
// ORDER: return
@@ -0,0 +1,12 @@
fun returnFun(): Int = 10
fun usage(): Int {
if (true) {
re<caret>
}
return 10
}
// ORDER: return
// ORDER: returnFun
@@ -0,0 +1,8 @@
fun returnFun(): Int = 10
fun usage(): Int {
re<caret>
}
// ORDER: return
// ORDER: returnFun
@@ -0,0 +1,12 @@
fun returnFun(): Int = 10
fun usage(): Int {
for (i in 1..10) {
re<caret>
}
return 10
}
// ORDER: returnFun
// ORDER: return
@@ -0,0 +1,10 @@
fun returnFun(): Int = 10
fun usage(): Int {
for (i in 1..10) re<caret>
return 10
}
// ORDER: returnFun
// ORDER: return
@@ -0,0 +1,10 @@
fun returnFun(): Int = 10
fun usage(): Int {
if (true) re<caret>
return 10
}
// ORDER: return
// ORDER: returnFun
@@ -0,0 +1,9 @@
fun returnFun() {}
fun usage(a: Int?): Int {
a ?: re<caret>
return 10
}
// ORDER: return
// ORDER: returnFun
@@ -0,0 +1,8 @@
fun reportError(): Nothing
fun usage(a: Int?): Int {
return a ?: a ?: a ?: re<caret>
}
// ORDER: reportError
// ORDER: return
@@ -0,0 +1,13 @@
fun returnFun(): Int = 10
fun <T> returnAnything(): T = null!!
fun usage(a: Int?): Int {
a ?: re<caret>
return 10
}
// function of the same type as `a` is preferred to return in this case
// ORDER: returnFun
// ORDER: return
// ORDER: returnAnything
@@ -0,0 +1,8 @@
fun reportError(): Nothing
fun usage(a: Int?): Int {
return if (a == null) re<caret> else a
}
// ORDER: reportError
// ORDER: return
@@ -0,0 +1,16 @@
fun reportError(): Nothing
fun usage(a: Int?): Int {
return when {
a == null -> {
if (true) { re<caret> }
10
}
else -> a
}
}
// ORDER: return
// ORDER: reportError
@@ -0,0 +1,9 @@
fun returnFun(): String = ""
fun usage(a: Int?): Int {
a to re<caret>
return 10
}
// ORDER: returnFun
// ORDER: return
@@ -0,0 +1,11 @@
fun reportError(): Nothing
fun usage(a: Int?): Int {
return when {
a == null -> re<caret>
else -> a
}
}
// ORDER: reportError
// ORDER: return
@@ -0,0 +1,11 @@
fun returnFun(): Int = 10
fun usage(a: Int): Int {
when (a) {
10 -> re<caret>
}
return 10
}
// ORDER: return
// ORDER: returnFun
@@ -0,0 +1,13 @@
fun returnFun(): Int = 10
fun usage(a: Int): Int {
when (a) {
10 -> {
re<caret>
}
}
return 10
}
// ORDER: return
// ORDER: returnFun
@@ -0,0 +1,11 @@
fun reportError(): Nothing
fun usage(a: Int?): Int {
return when {
a == null -> re<caret>
else -> a
}
}
// ORDER: reportError
// ORDER: return
@@ -213,6 +213,185 @@ public class BasicCompletionWeigherTestGenerated extends AbstractBasicCompletion
runTest("idea/idea-completion/testData/weighers/basic/UnavailableDslReceiver.kt"); runTest("idea/idea-completion/testData/weighers/basic/UnavailableDslReceiver.kt");
} }
@TestMetadata("idea/idea-completion/testData/weighers/basic/contextualReturn")
@TestDataPath("$PROJECT_ROOT")
@RunWith(JUnit3RunnerWithInners.class)
public static class ContextualReturn extends AbstractBasicCompletionWeigherTest {
private void runTest(String testDataFilePath) throws Exception {
KotlinTestUtils.runTest(this::doTest, this, testDataFilePath);
}
public void testAllFilesPresentInContextualReturn() throws Exception {
KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/idea-completion/testData/weighers/basic/contextualReturn"), Pattern.compile("^([^.]+)\\.(kt|kts)$"), true);
}
@TestMetadata("idea/idea-completion/testData/weighers/basic/contextualReturn/noReturnType")
@TestDataPath("$PROJECT_ROOT")
@RunWith(JUnit3RunnerWithInners.class)
public static class NoReturnType extends AbstractBasicCompletionWeigherTest {
private void runTest(String testDataFilePath) throws Exception {
KotlinTestUtils.runTest(this::doTest, this, testDataFilePath);
}
public void testAllFilesPresentInNoReturnType() throws Exception {
KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/idea-completion/testData/weighers/basic/contextualReturn/noReturnType"), Pattern.compile("^([^.]+)\\.(kt|kts)$"), true);
}
@TestMetadata("BeginOfNestedBlock.kt")
public void testBeginOfNestedBlock() throws Exception {
runTest("idea/idea-completion/testData/weighers/basic/contextualReturn/noReturnType/BeginOfNestedBlock.kt");
}
@TestMetadata("BeginOfTopLevelBlock.kt")
public void testBeginOfTopLevelBlock() throws Exception {
runTest("idea/idea-completion/testData/weighers/basic/contextualReturn/noReturnType/BeginOfTopLevelBlock.kt");
}
@TestMetadata("EndOfNestedBlock.kt")
public void testEndOfNestedBlock() throws Exception {
runTest("idea/idea-completion/testData/weighers/basic/contextualReturn/noReturnType/EndOfNestedBlock.kt");
}
@TestMetadata("EndOfTopLevelBlock.kt")
public void testEndOfTopLevelBlock() throws Exception {
runTest("idea/idea-completion/testData/weighers/basic/contextualReturn/noReturnType/EndOfTopLevelBlock.kt");
}
@TestMetadata("ForWithBody.kt")
public void testForWithBody() throws Exception {
runTest("idea/idea-completion/testData/weighers/basic/contextualReturn/noReturnType/ForWithBody.kt");
}
@TestMetadata("ForWithoutBody.kt")
public void testForWithoutBody() throws Exception {
runTest("idea/idea-completion/testData/weighers/basic/contextualReturn/noReturnType/ForWithoutBody.kt");
}
@TestMetadata("IfWithoutBody.kt")
public void testIfWithoutBody() throws Exception {
runTest("idea/idea-completion/testData/weighers/basic/contextualReturn/noReturnType/IfWithoutBody.kt");
}
@TestMetadata("InElvis.kt")
public void testInElvis() throws Exception {
runTest("idea/idea-completion/testData/weighers/basic/contextualReturn/noReturnType/InElvis.kt");
}
@TestMetadata("InElvisWhenSmartCompletionWins.kt")
public void testInElvisWhenSmartCompletionWins() throws Exception {
runTest("idea/idea-completion/testData/weighers/basic/contextualReturn/noReturnType/InElvisWhenSmartCompletionWins.kt");
}
@TestMetadata("InWhenSingleExpression.kt")
public void testInWhenSingleExpression() throws Exception {
runTest("idea/idea-completion/testData/weighers/basic/contextualReturn/noReturnType/InWhenSingleExpression.kt");
}
@TestMetadata("InWhenWithBody.kt")
public void testInWhenWithBody() throws Exception {
runTest("idea/idea-completion/testData/weighers/basic/contextualReturn/noReturnType/InWhenWithBody.kt");
}
}
@TestMetadata("idea/idea-completion/testData/weighers/basic/contextualReturn/withReturnType")
@TestDataPath("$PROJECT_ROOT")
@RunWith(JUnit3RunnerWithInners.class)
public static class WithReturnType extends AbstractBasicCompletionWeigherTest {
private void runTest(String testDataFilePath) throws Exception {
KotlinTestUtils.runTest(this::doTest, this, testDataFilePath);
}
public void testAllFilesPresentInWithReturnType() throws Exception {
KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("idea/idea-completion/testData/weighers/basic/contextualReturn/withReturnType"), Pattern.compile("^([^.]+)\\.(kt|kts)$"), true);
}
@TestMetadata("BeginOfNestedBlock.kt")
public void testBeginOfNestedBlock() throws Exception {
runTest("idea/idea-completion/testData/weighers/basic/contextualReturn/withReturnType/BeginOfNestedBlock.kt");
}
@TestMetadata("BeginOfTopLevelBlock.kt")
public void testBeginOfTopLevelBlock() throws Exception {
runTest("idea/idea-completion/testData/weighers/basic/contextualReturn/withReturnType/BeginOfTopLevelBlock.kt");
}
@TestMetadata("EndOfNestedBlock.kt")
public void testEndOfNestedBlock() throws Exception {
runTest("idea/idea-completion/testData/weighers/basic/contextualReturn/withReturnType/EndOfNestedBlock.kt");
}
@TestMetadata("EndOfTopLevelBlock.kt")
public void testEndOfTopLevelBlock() throws Exception {
runTest("idea/idea-completion/testData/weighers/basic/contextualReturn/withReturnType/EndOfTopLevelBlock.kt");
}
@TestMetadata("ForWithBody.kt")
public void testForWithBody() throws Exception {
runTest("idea/idea-completion/testData/weighers/basic/contextualReturn/withReturnType/ForWithBody.kt");
}
@TestMetadata("ForWithoutBody.kt")
public void testForWithoutBody() throws Exception {
runTest("idea/idea-completion/testData/weighers/basic/contextualReturn/withReturnType/ForWithoutBody.kt");
}
@TestMetadata("IfWithoutBody.kt")
public void testIfWithoutBody() throws Exception {
runTest("idea/idea-completion/testData/weighers/basic/contextualReturn/withReturnType/IfWithoutBody.kt");
}
@TestMetadata("InElvis.kt")
public void testInElvis() throws Exception {
runTest("idea/idea-completion/testData/weighers/basic/contextualReturn/withReturnType/InElvis.kt");
}
@TestMetadata("InElvisInReturn.kt")
public void testInElvisInReturn() throws Exception {
runTest("idea/idea-completion/testData/weighers/basic/contextualReturn/withReturnType/InElvisInReturn.kt");
}
@TestMetadata("InElvisWhenSmartCompletionWins.kt")
public void testInElvisWhenSmartCompletionWins() throws Exception {
runTest("idea/idea-completion/testData/weighers/basic/contextualReturn/withReturnType/InElvisWhenSmartCompletionWins.kt");
}
@TestMetadata("InIfAsReturnedExpression.kt")
public void testInIfAsReturnedExpression() throws Exception {
runTest("idea/idea-completion/testData/weighers/basic/contextualReturn/withReturnType/InIfAsReturnedExpression.kt");
}
@TestMetadata("InIfInWhenWithBodyAsReturnedExpression.kt")
public void testInIfInWhenWithBodyAsReturnedExpression() throws Exception {
runTest("idea/idea-completion/testData/weighers/basic/contextualReturn/withReturnType/InIfInWhenWithBodyAsReturnedExpression.kt");
}
@TestMetadata("InNotElvisBinaryOperator.kt")
public void testInNotElvisBinaryOperator() throws Exception {
runTest("idea/idea-completion/testData/weighers/basic/contextualReturn/withReturnType/InNotElvisBinaryOperator.kt");
}
@TestMetadata("InWhenAsReturnedExpression.kt")
public void testInWhenAsReturnedExpression() throws Exception {
runTest("idea/idea-completion/testData/weighers/basic/contextualReturn/withReturnType/InWhenAsReturnedExpression.kt");
}
@TestMetadata("InWhenSingleExpression.kt")
public void testInWhenSingleExpression() throws Exception {
runTest("idea/idea-completion/testData/weighers/basic/contextualReturn/withReturnType/InWhenSingleExpression.kt");
}
@TestMetadata("InWhenWithBody.kt")
public void testInWhenWithBody() throws Exception {
runTest("idea/idea-completion/testData/weighers/basic/contextualReturn/withReturnType/InWhenWithBody.kt");
}
@TestMetadata("InWhenWithBodyAsReturnedExpression.kt")
public void testInWhenWithBodyAsReturnedExpression() throws Exception {
runTest("idea/idea-completion/testData/weighers/basic/contextualReturn/withReturnType/InWhenWithBodyAsReturnedExpression.kt");
}
}
}
@TestMetadata("idea/idea-completion/testData/weighers/basic/expectedInfo") @TestMetadata("idea/idea-completion/testData/weighers/basic/expectedInfo")
@TestDataPath("$PROJECT_ROOT") @TestDataPath("$PROJECT_ROOT")
@RunWith(JUnit3RunnerWithInners.class) @RunWith(JUnit3RunnerWithInners.class)