Skip to content

Commit 7d6cb78

Browse files
t-kameyamamglukhikh
t-kameyama
authored and
mglukhikh
committed
Add inspection: Java mutator method used on immutable Kotlin Collections
In particular, fill, reverse, shuffle, sort calls are reported So #KT-22011 Fixed
1 parent bec28c8 commit 7d6cb78

File tree

8 files changed

+111
-25
lines changed

8 files changed

+111
-25
lines changed

‎idea/resources/inspectionDescriptions/JavaCollectionsStaticMethodOnImmutableList.html

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
<html>
2+
<body>
3+
This inspection reports immutable Kotlin collection may be changed with Java Collections method.
4+
</body>
5+
</html>

‎idea/src/META-INF/plugin.xml

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2623,6 +2623,15 @@
26232623
language="kotlin"
26242624
/>
26252625

2626+
<localInspection implementationClass="org.jetbrains.kotlin.idea.inspections.JavaCollectionsStaticMethodOnImmutableListInspection"
2627+
displayName="Immutable Kotlin collection may be changed with Java Collections method"
2628+
groupPath="Kotlin"
2629+
groupName="Probable bugs"
2630+
enabledByDefault="true"
2631+
level="WEAK WARNING"
2632+
language="kotlin"
2633+
/>
2634+
26262635
<localInspection implementationClass="org.jetbrains.kotlin.idea.inspections.RecursiveEqualsCallInspection"
26272636
displayName="Recursive equals call"
26282637
groupPath="Kotlin"

‎idea/src/org/jetbrains/kotlin/idea/inspections/JavaCollectionsStaticMethodCallInspection.kt renamed to ‎idea/src/org/jetbrains/kotlin/idea/inspections/JavaCollectionsStaticMethodInspection.kt

Lines changed: 48 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -29,17 +29,7 @@ import org.jetbrains.kotlin.types.KotlinType
2929
class JavaCollectionsStaticMethodInspection : AbstractKotlinInspection() {
3030
override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean): PsiElementVisitor {
3131
return dotQualifiedExpressionVisitor(fun(expression) {
32-
val callExpression = expression.callExpression ?: return
33-
val args = callExpression.valueArguments
34-
val firstArg = args.firstOrNull() ?: return
35-
val context = expression.analyze(BodyResolveMode.PARTIAL)
36-
if (firstArg.getArgumentExpression()?.getType(context)?.isMutableListOrSubtype() != true) return
37-
38-
val descriptor = expression.getResolvedCall(context)?.resultingDescriptor as? JavaMethodDescriptor ?: return
39-
val fqName = descriptor.importableFqName?.asString() ?: return
40-
if (!canReplaceWithStdLib(expression, fqName, args)) return
41-
42-
val methodName = fqName.split(".").last()
32+
val (methodName, firstArg) = getTargetMethodOnMutableList(expression) ?: return
4333
holder.registerProblem(
4434
expression,
4535
"Java Collections static method call should be replaced with Kotlin stdlib",
@@ -49,23 +39,49 @@ class JavaCollectionsStaticMethodInspection : AbstractKotlinInspection() {
4939
})
5040
}
5141

52-
private fun canReplaceWithStdLib(expression: KtDotQualifiedExpression, fqName: String, args: List<KtValueArgument>): Boolean {
53-
if (!fqName.startsWith("java.util.Collections.")) return false
54-
val size = args.size
55-
return when (fqName) {
56-
"java.util.Collections.fill" -> checkApiVersion(ApiVersion.KOTLIN_1_2, expression) && size == 2
57-
"java.util.Collections.reverse" -> size == 1
58-
"java.util.Collections.shuffle" -> checkApiVersion(ApiVersion.KOTLIN_1_2, expression) && (size == 1 || size == 2)
59-
"java.util.Collections.sort" -> {
60-
size == 1 || (size == 2 && args.getOrNull(1)?.getArgumentExpression() is KtLambdaExpression)
42+
companion object {
43+
fun getTargetMethodOnImmutableList(expression: KtDotQualifiedExpression) =
44+
getTargetMethod(expression) { isListOrSubtype() && !isMutableListOrSubtype() }
45+
46+
private fun getTargetMethodOnMutableList(expression: KtDotQualifiedExpression) =
47+
getTargetMethod(expression) { isMutableListOrSubtype() }
48+
49+
private fun getTargetMethod(
50+
expression: KtDotQualifiedExpression,
51+
isValidFirstArgument: KotlinType.() -> Boolean
52+
): Pair<String, KtValueArgument>? {
53+
val callExpression = expression.callExpression ?: return null
54+
val args = callExpression.valueArguments
55+
val firstArg = args.firstOrNull() ?: return null
56+
val context = expression.analyze(BodyResolveMode.PARTIAL)
57+
if (firstArg.getArgumentExpression()?.getType(context)?.isValidFirstArgument() != true) return null
58+
59+
val descriptor = expression.getResolvedCall(context)?.resultingDescriptor as? JavaMethodDescriptor ?: return null
60+
val fqName = descriptor.importableFqName?.asString() ?: return null
61+
if (!canReplaceWithStdLib(expression, fqName, args)) return null
62+
63+
val methodName = fqName.split(".").last()
64+
return methodName to firstArg
65+
}
66+
67+
private fun canReplaceWithStdLib(expression: KtDotQualifiedExpression, fqName: String, args: List<KtValueArgument>): Boolean {
68+
if (!fqName.startsWith("java.util.Collections.")) return false
69+
val size = args.size
70+
return when (fqName) {
71+
"java.util.Collections.fill" -> checkApiVersion(ApiVersion.KOTLIN_1_2, expression) && size == 2
72+
"java.util.Collections.reverse" -> size == 1
73+
"java.util.Collections.shuffle" -> checkApiVersion(ApiVersion.KOTLIN_1_2, expression) && (size == 1 || size == 2)
74+
"java.util.Collections.sort" -> {
75+
size == 1 || (size == 2 && args.getOrNull(1)?.getArgumentExpression() is KtLambdaExpression)
76+
}
77+
else -> false
6178
}
62-
else -> false
6379
}
64-
}
6580

66-
private fun checkApiVersion(requiredVersion: ApiVersion, expression: KtDotQualifiedExpression): Boolean {
67-
val module = ModuleUtilCore.findModuleForPsiElement(expression) ?: return true
68-
return module.languageVersionSettings.apiVersion >= requiredVersion
81+
private fun checkApiVersion(requiredVersion: ApiVersion, expression: KtDotQualifiedExpression): Boolean {
82+
val module = ModuleUtilCore.findModuleForPsiElement(expression) ?: return true
83+
return module.languageVersionSettings.apiVersion >= requiredVersion
84+
}
6985
}
7086

7187
}
@@ -77,6 +93,13 @@ private fun KotlinType.isMutableListOrSubtype(): Boolean {
7793
return isMutableList() || constructor.supertypes.reversed().any { it.isMutableList() }
7894
}
7995

96+
private fun KotlinType.isList() =
97+
constructor.declarationDescriptor?.fqNameSafe == KotlinBuiltIns.FQ_NAMES.list
98+
99+
private fun KotlinType.isListOrSubtype(): Boolean {
100+
return isList() || constructor.supertypes.reversed().any { it.isList() }
101+
}
102+
80103
private class ReplaceWithStdLibFix(private val methodName: String, private val receiver: String) : LocalQuickFix {
81104
override fun getName() = "Replace with $receiver.$methodName"
82105

‎idea/src/org/jetbrains/kotlin/idea/inspections/JavaCollectionsStaticMethodOnImmutableListInspection.kt

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
/*
2+
* Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license
3+
* that can be found in the license/LICENSE.txt file.
4+
*/
5+
6+
package org.jetbrains.kotlin.idea.inspections
7+
8+
import com.intellij.codeInspection.ProblemsHolder
9+
import com.intellij.psi.PsiElementVisitor
10+
import org.jetbrains.kotlin.psi.dotQualifiedExpressionVisitor
11+
12+
class JavaCollectionsStaticMethodOnImmutableListInspection : AbstractKotlinInspection() {
13+
override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean): PsiElementVisitor {
14+
return dotQualifiedExpressionVisitor(fun(expression) {
15+
val (methodName, firstArg) = JavaCollectionsStaticMethodInspection.getTargetMethodOnImmutableList(expression) ?: return
16+
holder.registerProblem(
17+
expression,
18+
"The '${firstArg.text}' may be changed with Java Collections method '$methodName'"
19+
)
20+
})
21+
}
22+
}

‎idea/testData/inspections/javaCollectionsStaticMethodOnImmutableList/inspectionData/expected.xml

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<problems>
2+
<problem>
3+
<file>test.kt</file>
4+
<line>5</line>
5+
<module>light_idea_test_case</module>
6+
<entry_point TYPE="file" FQNAME="test.kt" />
7+
<problem_class severity="WARNING" attribute_key="WARNING_ATTRIBUTES">Immutable Kotlin collection may be changed with Java Collections method</problem_class>
8+
<description>The 'immutableList' may be changed with Java Collections method 'reverse'</description>
9+
</problem>
10+
</problems>

‎idea/testData/inspections/javaCollectionsStaticMethodOnImmutableList/inspectionData/inspections.test

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
// INSPECTION_CLASS: org.jetbrains.kotlin.idea.inspections.JavaCollectionsStaticMethodOnImmutableListInspection
2+
// RUNTIME_WITH_FULL_JDK

‎idea/testData/inspections/javaCollectionsStaticMethodOnImmutableList/test.kt

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
import java.util.Collections
2+
3+
fun test() {
4+
val immutableList = listOf(1, 2)
5+
Collections.reverse(immutableList)
6+
7+
val mutableList = mutableListOf(1)
8+
Collections.reverse(mutableList)
9+
}

‎idea/tests/org/jetbrains/kotlin/idea/codeInsight/InspectionTestGenerated.java

Lines changed: 6 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)