From 2076a3109470bc461be08caa4549fbf4293ac69b Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Fri, 21 Oct 2016 16:04:49 +0300 Subject: [PATCH] Improve Collection/Array/String detection logic in intentions Use static methods from KotlinBuiltIns and check all supertypes, support cases of Collection, Map and CharSequence --- .../kotlin/builtins/KotlinBuiltIns.java | 29 ++++++------ .../jetbrains/kotlin/idea/intentions/Utils.kt | 47 +++++++------------ .../primitiveArrayEQEQ.kt | 8 ++++ .../primitiveArrayEQEQ.kt.after | 10 ++++ .../collection.kt | 6 +++ .../collection.kt.after | 6 +++ .../replaceSizeCheckWithIsNotEmpty/map.kt | 6 +++ .../map.kt.after | 6 +++ .../primitiveArray.kt | 6 +++ .../primitiveArray.kt.after | 6 +++ .../intentions/IntentionTestGenerated.java | 24 ++++++++++ 11 files changed, 109 insertions(+), 45 deletions(-) create mode 100644 idea/testData/intentions/replaceArrayEqualityOpWithArraysEquals/primitiveArrayEQEQ.kt create mode 100644 idea/testData/intentions/replaceArrayEqualityOpWithArraysEquals/primitiveArrayEQEQ.kt.after create mode 100644 idea/testData/intentions/replaceSizeCheckWithIsNotEmpty/collection.kt create mode 100644 idea/testData/intentions/replaceSizeCheckWithIsNotEmpty/collection.kt.after create mode 100644 idea/testData/intentions/replaceSizeCheckWithIsNotEmpty/map.kt create mode 100644 idea/testData/intentions/replaceSizeCheckWithIsNotEmpty/map.kt.after create mode 100644 idea/testData/intentions/replaceSizeCheckWithIsNotEmpty/primitiveArray.kt create mode 100644 idea/testData/intentions/replaceSizeCheckWithIsNotEmpty/primitiveArray.kt.after diff --git a/core/descriptors/src/org/jetbrains/kotlin/builtins/KotlinBuiltIns.java b/core/descriptors/src/org/jetbrains/kotlin/builtins/KotlinBuiltIns.java index 6eb203f1704..5a95e9e8353 100644 --- a/core/descriptors/src/org/jetbrains/kotlin/builtins/KotlinBuiltIns.java +++ b/core/descriptors/src/org/jetbrains/kotlin/builtins/KotlinBuiltIns.java @@ -213,11 +213,6 @@ public abstract class KotlinBuiltIns { public final FqName mutableMap = collectionsFqName("MutableMap"); public final FqName mutableMapEntry = mutableMap.child(Name.identifier("MutableEntry")); - private final FqNameUnsafe _collection = collection.toUnsafe(); - private final FqNameUnsafe _list = list.toUnsafe(); - private final FqNameUnsafe _set = set.toUnsafe(); - private final FqNameUnsafe _iterable = iterable.toUnsafe(); - public final FqNameUnsafe kClass = reflect("KClass"); public final FqNameUnsafe kCallable = reflect("KCallable"); public final FqNameUnsafe kProperty0 = reflect("KProperty0"); @@ -355,7 +350,7 @@ public abstract class KotlinBuiltIns { } @NotNull - public ClassDescriptor getPrimitiveClassDescriptor(@NotNull PrimitiveType type) { + private ClassDescriptor getPrimitiveClassDescriptor(@NotNull PrimitiveType type) { return getBuiltInClassByName(type.getTypeName().asString()); } @@ -506,11 +501,6 @@ public abstract class KotlinBuiltIns { return getBuiltInClassByName("String"); } - @NotNull - public ClassDescriptor getCharSequence() { - return getBuiltInClassByName("CharSequence"); - } - @NotNull public ClassDescriptor getComparable() { return getBuiltInClassByName("Comparable"); @@ -708,6 +698,7 @@ public abstract class KotlinBuiltIns { } return arrayType.getArguments().get(0).getType(); } + //noinspection SuspiciousMethodCalls KotlinType primitiveType = kotlinArrayTypeToPrimitiveKotlinType.get(TypeUtils.makeNotNullable(arrayType)); if (primitiveType == null) { throw new IllegalStateException("not array: " + arrayType); @@ -787,6 +778,10 @@ public abstract class KotlinBuiltIns { return descriptor instanceof ClassDescriptor && classFqNameEquals(descriptor, fqName); } + private static boolean isConstructedFromGivenClass(@NotNull KotlinType type, @NotNull FqName fqName) { + return isConstructedFromGivenClass(type, fqName.toUnsafe()); + } + private static boolean classFqNameEquals(@NotNull ClassifierDescriptor descriptor, @NotNull FqNameUnsafe fqName) { // Quick check to avoid creation of full FqName instance return descriptor.getName().equals(fqName.shortName()) && @@ -908,19 +903,23 @@ public abstract class KotlinBuiltIns { } public static boolean isCollectionOrNullableCollection(@NotNull KotlinType type) { - return isConstructedFromGivenClass(type, FQ_NAMES._collection); + return isConstructedFromGivenClass(type, FQ_NAMES.collection); } public static boolean isListOrNullableList(@NotNull KotlinType type) { - return isConstructedFromGivenClass(type, FQ_NAMES._list); + return isConstructedFromGivenClass(type, FQ_NAMES.list); } public static boolean isSetOrNullableSet(@NotNull KotlinType type) { - return isConstructedFromGivenClass(type, FQ_NAMES._set); + return isConstructedFromGivenClass(type, FQ_NAMES.set); + } + + public static boolean isMapOrNullableMap(@NotNull KotlinType type) { + return isConstructedFromGivenClass(type, FQ_NAMES.map); } public static boolean isIterableOrNullableIterable(@NotNull KotlinType type) { - return isConstructedFromGivenClass(type, FQ_NAMES._iterable); + return isConstructedFromGivenClass(type, FQ_NAMES.iterable); } public static boolean isKClass(@NotNull ClassDescriptor descriptor) { diff --git a/idea/src/org/jetbrains/kotlin/idea/intentions/Utils.kt b/idea/src/org/jetbrains/kotlin/idea/intentions/Utils.kt index ccb3cf06402..d07adea8f5e 100644 --- a/idea/src/org/jetbrains/kotlin/idea/intentions/Utils.kt +++ b/idea/src/org/jetbrains/kotlin/idea/intentions/Utils.kt @@ -26,9 +26,7 @@ import org.jetbrains.kotlin.idea.caches.resolve.analyzeAndGetResult import org.jetbrains.kotlin.idea.core.ShortenReferences import org.jetbrains.kotlin.idea.references.mainReference import org.jetbrains.kotlin.idea.util.IdeDescriptorRenderers -import org.jetbrains.kotlin.js.descriptorUtils.nameIfStandardType import org.jetbrains.kotlin.lexer.KtTokens -import org.jetbrains.kotlin.name.Name import org.jetbrains.kotlin.psi.* import org.jetbrains.kotlin.psi.typeRefHelpers.setReceiverTypeReference import org.jetbrains.kotlin.resolve.BindingContext @@ -235,42 +233,31 @@ private fun KtExpression.ifBranchesOrThis(): List { return listOf(then) + `else`?.ifBranchesOrThis().orEmpty() } -private val arrayName = Name.identifier("Array") - -private val collectionName = Name.identifier("Collection") - -private val stringName = Name.identifier("String") - -fun ResolvedCall.resolvedToArrayType() = - resultingDescriptor.returnType?.nameIfStandardType == arrayName - -fun ResolvedCall.resolvedToCollectionType() = - resultingDescriptor.returnType?.constructor?.supertypes?.firstOrNull { - it.nameIfStandardType == collectionName - } != null - -fun ResolvedCall.resolvedToStringType() = - resultingDescriptor.returnType?.nameIfStandardType == stringName +fun ResolvedCall.resolvedToArrayType(): Boolean = + resultingDescriptor.returnType.let { type -> + type != null && (KotlinBuiltIns.isArray(type) || KotlinBuiltIns.isPrimitiveArray(type)) + } fun KtElement?.isZero() = this?.text == "0" fun KtElement?.isOne() = this?.text == "1" +private fun KtExpression.isExpressionOfTypeOrSubtype(predicate: (KotlinType) -> Boolean): Boolean { + val returnType = getResolvedCall(analyze())?.resultingDescriptor?.returnType + return returnType != null && (returnType.constructor.supertypes + returnType).any(predicate) +} + fun KtElement?.isSizeOrLength(): Boolean { if (this !is KtDotQualifiedExpression) return false + return when (selectorExpression?.text) { - "size" -> receiverExpression.isArrayOrCollection() - "length" -> receiverExpression.isString() + "size" -> receiverExpression.isExpressionOfTypeOrSubtype { type -> + KotlinBuiltIns.isArray(type) || + KotlinBuiltIns.isPrimitiveArray(type) || + KotlinBuiltIns.isCollectionOrNullableCollection(type) || + KotlinBuiltIns.isMapOrNullableMap(type) + } + "length" -> receiverExpression.isExpressionOfTypeOrSubtype(KotlinBuiltIns::isCharSequenceOrNullableCharSequence) else -> false } } - -private fun KtExpression.isArrayOrCollection(): Boolean { - val resolvedCall = getResolvedCall(analyze()) ?: return false - return resolvedCall.resolvedToArrayType() || resolvedCall.resolvedToCollectionType() -} - -private fun KtExpression.isString(): Boolean { - val resolvedCall = getResolvedCall(analyze()) ?: return false - return resolvedCall.resolvedToStringType() -} diff --git a/idea/testData/intentions/replaceArrayEqualityOpWithArraysEquals/primitiveArrayEQEQ.kt b/idea/testData/intentions/replaceArrayEqualityOpWithArraysEquals/primitiveArrayEQEQ.kt new file mode 100644 index 00000000000..ca7b072ad07 --- /dev/null +++ b/idea/testData/intentions/replaceArrayEqualityOpWithArraysEquals/primitiveArrayEQEQ.kt @@ -0,0 +1,8 @@ +// WITH_RUNTIME +// INTENTION_TEXT: Replace '==' with 'Arrays.equals' +fun foo() { + val a = charArrayOf('a', 'b', 'c') + val b = charArrayOf('a', 'b', 'c') + if (a == b) { + } +} diff --git a/idea/testData/intentions/replaceArrayEqualityOpWithArraysEquals/primitiveArrayEQEQ.kt.after b/idea/testData/intentions/replaceArrayEqualityOpWithArraysEquals/primitiveArrayEQEQ.kt.after new file mode 100644 index 00000000000..920bfb782a9 --- /dev/null +++ b/idea/testData/intentions/replaceArrayEqualityOpWithArraysEquals/primitiveArrayEQEQ.kt.after @@ -0,0 +1,10 @@ +import java.util.Arrays + +// WITH_RUNTIME +// INTENTION_TEXT: Replace '==' with 'Arrays.equals' +fun foo() { + val a = charArrayOf('a', 'b', 'c') + val b = charArrayOf('a', 'b', 'c') + if (Arrays.equals(a, b)) { + } +} diff --git a/idea/testData/intentions/replaceSizeCheckWithIsNotEmpty/collection.kt b/idea/testData/intentions/replaceSizeCheckWithIsNotEmpty/collection.kt new file mode 100644 index 00000000000..34540760119 --- /dev/null +++ b/idea/testData/intentions/replaceSizeCheckWithIsNotEmpty/collection.kt @@ -0,0 +1,6 @@ +// WITH_RUNTIME + +fun foo() { + val c: Collection = listOf("") + c.size > 0 +} diff --git a/idea/testData/intentions/replaceSizeCheckWithIsNotEmpty/collection.kt.after b/idea/testData/intentions/replaceSizeCheckWithIsNotEmpty/collection.kt.after new file mode 100644 index 00000000000..a38fb194500 --- /dev/null +++ b/idea/testData/intentions/replaceSizeCheckWithIsNotEmpty/collection.kt.after @@ -0,0 +1,6 @@ +// WITH_RUNTIME + +fun foo() { + val c: Collection = listOf("") + c.isNotEmpty() +} diff --git a/idea/testData/intentions/replaceSizeCheckWithIsNotEmpty/map.kt b/idea/testData/intentions/replaceSizeCheckWithIsNotEmpty/map.kt new file mode 100644 index 00000000000..ce8028c9f8c --- /dev/null +++ b/idea/testData/intentions/replaceSizeCheckWithIsNotEmpty/map.kt @@ -0,0 +1,6 @@ +// WITH_RUNTIME + +fun foo() { + val m = mapOf("" to 1) + m.size > 0 +} diff --git a/idea/testData/intentions/replaceSizeCheckWithIsNotEmpty/map.kt.after b/idea/testData/intentions/replaceSizeCheckWithIsNotEmpty/map.kt.after new file mode 100644 index 00000000000..a6f7522ce9a --- /dev/null +++ b/idea/testData/intentions/replaceSizeCheckWithIsNotEmpty/map.kt.after @@ -0,0 +1,6 @@ +// WITH_RUNTIME + +fun foo() { + val m = mapOf("" to 1) + m.isNotEmpty() +} diff --git a/idea/testData/intentions/replaceSizeCheckWithIsNotEmpty/primitiveArray.kt b/idea/testData/intentions/replaceSizeCheckWithIsNotEmpty/primitiveArray.kt new file mode 100644 index 00000000000..35fa64f1950 --- /dev/null +++ b/idea/testData/intentions/replaceSizeCheckWithIsNotEmpty/primitiveArray.kt @@ -0,0 +1,6 @@ +// WITH_RUNTIME + +fun foo() { + val a = intArrayOf(1, 2, 3) + a.size > 0 +} diff --git a/idea/testData/intentions/replaceSizeCheckWithIsNotEmpty/primitiveArray.kt.after b/idea/testData/intentions/replaceSizeCheckWithIsNotEmpty/primitiveArray.kt.after new file mode 100644 index 00000000000..c39a781f8fe --- /dev/null +++ b/idea/testData/intentions/replaceSizeCheckWithIsNotEmpty/primitiveArray.kt.after @@ -0,0 +1,6 @@ +// WITH_RUNTIME + +fun foo() { + val a = intArrayOf(1, 2, 3) + a.isNotEmpty() +} diff --git a/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java index 5e876e85576..9a6d91c93de 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/intentions/IntentionTestGenerated.java @@ -11173,6 +11173,12 @@ public class IntentionTestGenerated extends AbstractIntentionTest { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/replaceArrayEqualityOpWithArraysEquals/arrayEQEQ.kt"); doTest(fileName); } + + @TestMetadata("primitiveArrayEQEQ.kt") + public void testPrimitiveArrayEQEQ() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/replaceArrayEqualityOpWithArraysEquals/primitiveArrayEQEQ.kt"); + doTest(fileName); + } } @TestMetadata("idea/testData/intentions/replaceExplicitFunctionLiteralParamWithIt") @@ -11567,6 +11573,12 @@ public class IntentionTestGenerated extends AbstractIntentionTest { doTest(fileName); } + @TestMetadata("collection.kt") + public void testCollection() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/replaceSizeCheckWithIsNotEmpty/collection.kt"); + doTest(fileName); + } + @TestMetadata("gt.kt") public void testGt() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/replaceSizeCheckWithIsNotEmpty/gt.kt"); @@ -11603,6 +11615,12 @@ public class IntentionTestGenerated extends AbstractIntentionTest { doTest(fileName); } + @TestMetadata("map.kt") + public void testMap() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/replaceSizeCheckWithIsNotEmpty/map.kt"); + doTest(fileName); + } + @TestMetadata("oppositeSign.kt") public void testOppositeSign() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/replaceSizeCheckWithIsNotEmpty/oppositeSign.kt"); @@ -11615,6 +11633,12 @@ public class IntentionTestGenerated extends AbstractIntentionTest { doTest(fileName); } + @TestMetadata("primitiveArray.kt") + public void testPrimitiveArray() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/replaceSizeCheckWithIsNotEmpty/primitiveArray.kt"); + doTest(fileName); + } + @TestMetadata("set.kt") public void testSet() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/intentions/replaceSizeCheckWithIsNotEmpty/set.kt");