diff --git a/idea/resources/inspectionDescriptions/JavaCollectionsStaticMethodOnImmutableList.html b/idea/resources/inspectionDescriptions/JavaCollectionsStaticMethodOnImmutableList.html new file mode 100644 index 00000000000..dbd9c3bcf4d --- /dev/null +++ b/idea/resources/inspectionDescriptions/JavaCollectionsStaticMethodOnImmutableList.html @@ -0,0 +1,5 @@ + + +This inspection reports immutable Kotlin collection may be changed with Java Collections method. + + diff --git a/idea/src/META-INF/plugin.xml b/idea/src/META-INF/plugin.xml index 702761dfc81..eab227abff1 100644 --- a/idea/src/META-INF/plugin.xml +++ b/idea/src/META-INF/plugin.xml @@ -2623,6 +2623,15 @@ language="kotlin" /> + + ): Boolean { - if (!fqName.startsWith("java.util.Collections.")) return false - val size = args.size - return when (fqName) { - "java.util.Collections.fill" -> checkApiVersion(ApiVersion.KOTLIN_1_2, expression) && size == 2 - "java.util.Collections.reverse" -> size == 1 - "java.util.Collections.shuffle" -> checkApiVersion(ApiVersion.KOTLIN_1_2, expression) && (size == 1 || size == 2) - "java.util.Collections.sort" -> { - size == 1 || (size == 2 && args.getOrNull(1)?.getArgumentExpression() is KtLambdaExpression) - } - else -> false - } - } + companion object { + fun getTargetMethodOnImmutableList(expression: KtDotQualifiedExpression) = + getTargetMethod(expression) { isListOrSubtype() && !isMutableListOrSubtype() } - private fun checkApiVersion(requiredVersion: ApiVersion, expression: KtDotQualifiedExpression): Boolean { - val module = ModuleUtilCore.findModuleForPsiElement(expression) ?: return true - return module.languageVersionSettings.apiVersion >= requiredVersion + private fun getTargetMethodOnMutableList(expression: KtDotQualifiedExpression) = + getTargetMethod(expression) { isMutableListOrSubtype() } + + private fun getTargetMethod( + expression: KtDotQualifiedExpression, + isValidFirstArgument: KotlinType.() -> Boolean + ): Pair? { + val callExpression = expression.callExpression ?: return null + val args = callExpression.valueArguments + val firstArg = args.firstOrNull() ?: return null + val context = expression.analyze(BodyResolveMode.PARTIAL) + if (firstArg.getArgumentExpression()?.getType(context)?.isValidFirstArgument() != true) return null + + val descriptor = expression.getResolvedCall(context)?.resultingDescriptor as? JavaMethodDescriptor ?: return null + val fqName = descriptor.importableFqName?.asString() ?: return null + if (!canReplaceWithStdLib(expression, fqName, args)) return null + + val methodName = fqName.split(".").last() + return methodName to firstArg + } + + private fun canReplaceWithStdLib(expression: KtDotQualifiedExpression, fqName: String, args: List): Boolean { + if (!fqName.startsWith("java.util.Collections.")) return false + val size = args.size + return when (fqName) { + "java.util.Collections.fill" -> checkApiVersion(ApiVersion.KOTLIN_1_2, expression) && size == 2 + "java.util.Collections.reverse" -> size == 1 + "java.util.Collections.shuffle" -> checkApiVersion(ApiVersion.KOTLIN_1_2, expression) && (size == 1 || size == 2) + "java.util.Collections.sort" -> { + size == 1 || (size == 2 && args.getOrNull(1)?.getArgumentExpression() is KtLambdaExpression) + } + else -> false + } + } + + private fun checkApiVersion(requiredVersion: ApiVersion, expression: KtDotQualifiedExpression): Boolean { + val module = ModuleUtilCore.findModuleForPsiElement(expression) ?: return true + return module.languageVersionSettings.apiVersion >= requiredVersion + } } } @@ -77,6 +93,13 @@ private fun KotlinType.isMutableListOrSubtype(): Boolean { return isMutableList() || constructor.supertypes.reversed().any { it.isMutableList() } } +private fun KotlinType.isList() = + constructor.declarationDescriptor?.fqNameSafe == KotlinBuiltIns.FQ_NAMES.list + +private fun KotlinType.isListOrSubtype(): Boolean { + return isList() || constructor.supertypes.reversed().any { it.isList() } +} + private class ReplaceWithStdLibFix(private val methodName: String, private val receiver: String) : LocalQuickFix { override fun getName() = "Replace with $receiver.$methodName" diff --git a/idea/src/org/jetbrains/kotlin/idea/inspections/JavaCollectionsStaticMethodOnImmutableListInspection.kt b/idea/src/org/jetbrains/kotlin/idea/inspections/JavaCollectionsStaticMethodOnImmutableListInspection.kt new file mode 100644 index 00000000000..0a88d315b68 --- /dev/null +++ b/idea/src/org/jetbrains/kotlin/idea/inspections/JavaCollectionsStaticMethodOnImmutableListInspection.kt @@ -0,0 +1,22 @@ +/* + * Copyright 2000-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license + * that can be found in the license/LICENSE.txt file. + */ + +package org.jetbrains.kotlin.idea.inspections + +import com.intellij.codeInspection.ProblemsHolder +import com.intellij.psi.PsiElementVisitor +import org.jetbrains.kotlin.psi.dotQualifiedExpressionVisitor + +class JavaCollectionsStaticMethodOnImmutableListInspection : AbstractKotlinInspection() { + override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean): PsiElementVisitor { + return dotQualifiedExpressionVisitor(fun(expression) { + val (methodName, firstArg) = JavaCollectionsStaticMethodInspection.getTargetMethodOnImmutableList(expression) ?: return + holder.registerProblem( + expression, + "The '${firstArg.text}' may be changed with Java Collections method '$methodName'" + ) + }) + } +} diff --git a/idea/testData/inspections/javaCollectionsStaticMethodOnImmutableList/inspectionData/expected.xml b/idea/testData/inspections/javaCollectionsStaticMethodOnImmutableList/inspectionData/expected.xml new file mode 100644 index 00000000000..aa1f0b7975e --- /dev/null +++ b/idea/testData/inspections/javaCollectionsStaticMethodOnImmutableList/inspectionData/expected.xml @@ -0,0 +1,10 @@ + + + test.kt + 5 + light_idea_test_case + + Immutable Kotlin collection may be changed with Java Collections method + The 'immutableList' may be changed with Java Collections method 'reverse' + + diff --git a/idea/testData/inspections/javaCollectionsStaticMethodOnImmutableList/inspectionData/inspections.test b/idea/testData/inspections/javaCollectionsStaticMethodOnImmutableList/inspectionData/inspections.test new file mode 100644 index 00000000000..30bef778014 --- /dev/null +++ b/idea/testData/inspections/javaCollectionsStaticMethodOnImmutableList/inspectionData/inspections.test @@ -0,0 +1,2 @@ +// INSPECTION_CLASS: org.jetbrains.kotlin.idea.inspections.JavaCollectionsStaticMethodOnImmutableListInspection +// RUNTIME_WITH_FULL_JDK \ No newline at end of file diff --git a/idea/testData/inspections/javaCollectionsStaticMethodOnImmutableList/test.kt b/idea/testData/inspections/javaCollectionsStaticMethodOnImmutableList/test.kt new file mode 100644 index 00000000000..3fe3d0f3249 --- /dev/null +++ b/idea/testData/inspections/javaCollectionsStaticMethodOnImmutableList/test.kt @@ -0,0 +1,9 @@ +import java.util.Collections + +fun test() { + val immutableList = listOf(1, 2) + Collections.reverse(immutableList) + + val mutableList = mutableListOf(1) + Collections.reverse(mutableList) +} diff --git a/idea/tests/org/jetbrains/kotlin/idea/codeInsight/InspectionTestGenerated.java b/idea/tests/org/jetbrains/kotlin/idea/codeInsight/InspectionTestGenerated.java index cc461ca36ad..9179208e505 100644 --- a/idea/tests/org/jetbrains/kotlin/idea/codeInsight/InspectionTestGenerated.java +++ b/idea/tests/org/jetbrains/kotlin/idea/codeInsight/InspectionTestGenerated.java @@ -198,6 +198,12 @@ public class InspectionTestGenerated extends AbstractInspectionTest { doTest(fileName); } + @TestMetadata("javaCollectionsStaticMethodOnImmutableList/inspectionData/inspections.test") + public void testJavaCollectionsStaticMethodOnImmutableList_inspectionData_Inspections_test() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspections/javaCollectionsStaticMethodOnImmutableList/inspectionData/inspections.test"); + doTest(fileName); + } + @TestMetadata("kt18195/inspectionData/inspections.test") public void testKt18195_inspectionData_Inspections_test() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("idea/testData/inspections/kt18195/inspectionData/inspections.test");