diff --git a/java/java-analysis-impl/resources/messages/InspectionGadgetsBundle.properties b/java/java-analysis-impl/resources/messages/InspectionGadgetsBundle.properties index a904c282cb8d..3907a17e9e22 100644 --- a/java/java-analysis-impl/resources/messages/InspectionGadgetsBundle.properties +++ b/java/java-analysis-impl/resources/messages/InspectionGadgetsBundle.properties @@ -1087,7 +1087,6 @@ class.loader.instantiation.problem.descriptor=Instantiation of #ref public.static.array.field.problem.descriptor='public static' array field #ref, compromising security #loc public.static.collection.field.problem.descriptor='public static' collection field #ref, compromising security #loc abstract.class.with.only.one.direct.inheritor.problem.descriptor=Abstract class #ref has only one direct inheritor #loc -system.get.property.problem.descriptor=Call #ref can be simplified for ''{0}'' #other abstract.method.overrides.abstract.method.remove.quickfix=Remove redundant abstract method declaration diff --git a/jvm/jvm-analysis-impl/resources/messages/JvmAnalysisBundle.properties b/jvm/jvm-analysis-impl/resources/messages/JvmAnalysisBundle.properties index 3ce63aa7ed82..8bf18f4bef6e 100644 --- a/jvm/jvm-analysis-impl/resources/messages/JvmAnalysisBundle.properties +++ b/jvm/jvm-analysis-impl/resources/messages/JvmAnalysisBundle.properties @@ -288,7 +288,9 @@ jvm.inspections.rename.quickfix.text="Rename to ''{0}'' jvm.inspections.remove.annotation.quickfix.name=Remove annotation jvm.inspections.remove.annotation.quickfix.text=Remove ''@{0}'' annotation -jvm.inspections.system.get.property.display.name=Call to 'System.getProperty(str)' could be simplified for certain predefined constants +jvm.inspections.system.get.property.display.name=Call to 'System.getProperty(str)' could be simplified +jvm.inspections.system.get.property.problem.descriptor=Call #ref can be simplified for ''{0}'' + can.t.build.uast.tree.for.file=Can't build UAST tree for file title.uast=UAST current.version=Current version diff --git a/jvm/jvm-analysis-impl/src/com/intellij/codeInspection/SystemGetPropertyInspection.kt b/jvm/jvm-analysis-impl/src/com/intellij/codeInspection/SystemGetPropertyInspection.kt index e266bc45fd11..be5afb2991d2 100644 --- a/jvm/jvm-analysis-impl/src/com/intellij/codeInspection/SystemGetPropertyInspection.kt +++ b/jvm/jvm-analysis-impl/src/com/intellij/codeInspection/SystemGetPropertyInspection.kt @@ -1,17 +1,36 @@ // Copyright 2000-2023 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. package com.intellij.codeInspection -import com.intellij.codeInspection.fix.ReplaceReferenceQuickFix -import com.intellij.codeInspection.fix.ReplaceReferenceQuickFix.Companion.PROPERTIES_TO_OPTIMIZE +import com.intellij.analysis.JvmAnalysisBundle +import com.intellij.codeInspection.fix.CallableExpression +import com.intellij.codeInspection.fix.Method +import com.intellij.codeInspection.fix.QualifiedReference +import com.intellij.codeInspection.fix.ReplaceCallableExpressionQuickFix import com.intellij.psi.PsiElementVisitor import com.intellij.uast.UastHintedVisitorAdapter -import com.siyeh.InspectionGadgetsBundle import com.siyeh.ig.callMatcher.CallMatcher import org.jetbrains.uast.UCallExpression import org.jetbrains.uast.visitor.AbstractUastNonRecursiveVisitor private val SYSTEM_GET_PROPERTY = CallMatcher.staticCall("java.lang.System", "getProperty") - .parameterCount(1) + .parameterTypes("java.lang.String") + +private val PROPERTIES_TO_OPTIMIZE = mapOf( + "file.separator" to CallableExpression(QualifiedReference("java.nio.file.FileSystems"), + listOf(Method("getDefault", + "java.nio.file.FileSystem"), + Method("getSeparator", + "java.lang.String"))), + "path.separator" to CallableExpression(QualifiedReference("java.io.File.pathSeparator"), + emptyList()), + "line.separator" to CallableExpression(QualifiedReference("java.lang.System"), + listOf(Method("lineSeparator", + "java.lang.String"))), + "file.encoding" to CallableExpression(QualifiedReference("java.nio.charset.Charset"), + listOf(Method("defaultCharset", + "java.nio.charset.Charset"), + Method("displayName", + "java.lang.String")))) class SystemGetPropertyInspection : AbstractBaseUastLocalInspectionTool() { override fun buildVisitor(holder: ProblemsHolder, @@ -24,8 +43,9 @@ class SystemGetPropertyInspection : AbstractBaseUastLocalInspectionTool() { if (!SYSTEM_GET_PROPERTY.uCallMatches(node)) return true val propertyValue = node.getArgumentForParameter(0)?.evaluate() as? String ?: return true if (propertyValue !in PROPERTIES_TO_OPTIMIZE.keys) return true - val message = InspectionGadgetsBundle.message("system.get.property.problem.descriptor", propertyValue) - holder.registerUProblem(node, message, ReplaceReferenceQuickFix(propertyValue)) + val message = JvmAnalysisBundle.message("jvm.inspections.system.get.property.problem.descriptor", propertyValue) + val qualifiedReference = PROPERTIES_TO_OPTIMIZE[propertyValue] ?: error("Unknown property!") + holder.registerUProblem(node, message, ReplaceCallableExpressionQuickFix(qualifiedReference)) return true } } diff --git a/jvm/jvm-analysis-impl/src/com/intellij/codeInspection/ThreadRunInspection.kt b/jvm/jvm-analysis-impl/src/com/intellij/codeInspection/ThreadRunInspection.kt index 84e72fe8a739..fae9fb1cbb49 100644 --- a/jvm/jvm-analysis-impl/src/com/intellij/codeInspection/ThreadRunInspection.kt +++ b/jvm/jvm-analysis-impl/src/com/intellij/codeInspection/ThreadRunInspection.kt @@ -1,7 +1,9 @@ // Copyright 2000-2021 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file. package com.intellij.codeInspection -import com.intellij.codeInspection.fix.ReplaceMethodCallFix +import com.intellij.codeInspection.fix.CallableExpression +import com.intellij.codeInspection.fix.Method +import com.intellij.codeInspection.fix.ReplaceCallableExpressionQuickFix import com.intellij.psi.PsiElementVisitor import com.intellij.uast.UastHintedVisitorAdapter import com.siyeh.InspectionGadgetsBundle @@ -21,7 +23,9 @@ class ThreadRunInspection : AbstractBaseUastLocalInspectionTool() { if (!THREAD_RUN.uCallMatches(node)) return true if (node.receiver is USuperExpression) return true val message = InspectionGadgetsBundle.message("thread.run.problem.descriptor") - holder.registerUProblem(node, message, ReplaceMethodCallFix("start")) + holder.registerUProblem(node, message, ReplaceCallableExpressionQuickFix( + CallableExpression(null, + listOf(Method("start"))))) return true } } diff --git a/jvm/jvm-analysis-impl/src/com/intellij/codeInspection/fix/ReplaceCallableExpressionQuickFix.kt b/jvm/jvm-analysis-impl/src/com/intellij/codeInspection/fix/ReplaceCallableExpressionQuickFix.kt new file mode 100644 index 000000000000..61888ab5b952 --- /dev/null +++ b/jvm/jvm-analysis-impl/src/com/intellij/codeInspection/fix/ReplaceCallableExpressionQuickFix.kt @@ -0,0 +1,60 @@ +// Copyright 2000-2023 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. +package com.intellij.codeInspection.fix + +import com.intellij.codeInsight.intention.FileModifier.SafeFieldForPreview +import com.intellij.codeInspection.CommonQuickFixBundle +import com.intellij.modcommand.ModPsiUpdater +import com.intellij.modcommand.PsiUpdateModCommandQuickFix +import com.intellij.openapi.project.Project +import com.intellij.psi.PsiElement +import com.intellij.psi.PsiType +import com.intellij.psi.search.GlobalSearchScope +import org.jetbrains.uast.* +import org.jetbrains.uast.generate.getUastElementFactory +import org.jetbrains.uast.generate.shortenReference + +class ReplaceCallableExpressionQuickFix(@SafeFieldForPreview private val callableExpression: CallableExpression) : PsiUpdateModCommandQuickFix() { + override fun getFamilyName(): String = CommonQuickFixBundle.message("fix.replace.with.x", callableExpression) + + override fun applyFix(project: Project, element: PsiElement, updater: ModPsiUpdater) { + val uCall = element.getUastParentOfType() + val uFactory = uCall?.getUastElementFactory(project) + val oldUParent = uCall?.getQualifiedParentOrThis() + val uQualifiedReference = callableExpression.qualifiedReference?.let { + uFactory?.createQualifiedReference(it.name, null) + } ?: uCall?.receiver + + val newUElement = callableExpression.methods.fold(uQualifiedReference) { receiver, method -> + uFactory?.createCallExpression(receiver?.getQualifiedParentOrThis(), + method.name, + method.parameters, + extractReturnType(project, method), + UastCallKind.METHOD_CALL) + } + + val oldPsi = oldUParent?.sourcePsi ?: return + val newPsi = newUElement?.getQualifiedParentOrThis()?.sourcePsi ?: return + oldPsi.replace(newPsi).toUElementOfType()?.shortenReference() + } + + private fun extractReturnType(project: Project, method: Method): PsiType? = method.returnType?.let { + PsiType.getTypeByName(method.returnType, project, GlobalSearchScope.allScope(project)) + } +} + +data class CallableExpression(val qualifiedReference: QualifiedReference?, val methods: List) { + override fun toString(): String = if (qualifiedReference == null) { + methods.joinToString(separator = "().", postfix = "()") { it.name } + } + else if (methods.isEmpty()) { + qualifiedReference.name + } + else { + "${qualifiedReference.name}.${methods.joinToString(separator = "().", postfix = "()") { it.name }}" + } +} + +@JvmInline +value class QualifiedReference(val name: String) + +data class Method(val name: String, val returnType: String? = null, val parameters: List = emptyList()) \ No newline at end of file diff --git a/jvm/jvm-analysis-impl/src/com/intellij/codeInspection/fix/ReplaceMethodCallFix.kt b/jvm/jvm-analysis-impl/src/com/intellij/codeInspection/fix/ReplaceMethodCallFix.kt deleted file mode 100644 index 866a9ac12e39..000000000000 --- a/jvm/jvm-analysis-impl/src/com/intellij/codeInspection/fix/ReplaceMethodCallFix.kt +++ /dev/null @@ -1,26 +0,0 @@ -// Copyright 2000-2023 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. -package com.intellij.codeInspection.fix - -import com.intellij.codeInspection.CommonQuickFixBundle -import com.intellij.codeInspection.LocalQuickFix -import com.intellij.codeInspection.ProblemDescriptor -import com.intellij.openapi.project.Project -import org.jetbrains.uast.UCallExpression -import org.jetbrains.uast.generate.getUastElementFactory -import org.jetbrains.uast.generate.replace -import org.jetbrains.uast.getQualifiedParentOrThis -import org.jetbrains.uast.getUastParentOfType - -class ReplaceMethodCallFix(val methodName: String) : LocalQuickFix { - override fun getFamilyName(): String = CommonQuickFixBundle.message("fix.replace.with.x", "$methodName()") - - override fun applyFix(project: Project, descriptor: ProblemDescriptor) { - val uCall = descriptor.psiElement.getUastParentOfType() ?: return - val uFactory = uCall.getUastElementFactory(project) ?: return - val newCall = uFactory.createCallExpression( - uCall.receiver, methodName, uCall.valueArguments, null, uCall.kind, null - ) ?: return - val oldCall = uCall.getQualifiedParentOrThis() - oldCall.replace(newCall) - } -} \ No newline at end of file diff --git a/jvm/jvm-analysis-impl/src/com/intellij/codeInspection/fix/ReplaceReferenceQuickFix.kt b/jvm/jvm-analysis-impl/src/com/intellij/codeInspection/fix/ReplaceReferenceQuickFix.kt deleted file mode 100644 index 2cb8bac74154..000000000000 --- a/jvm/jvm-analysis-impl/src/com/intellij/codeInspection/fix/ReplaceReferenceQuickFix.kt +++ /dev/null @@ -1,65 +0,0 @@ -// Copyright 2000-2023 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. -package com.intellij.codeInspection.fix - -import com.intellij.codeInsight.intention.FileModifier.SafeFieldForPreview -import com.intellij.codeInspection.CommonQuickFixBundle -import com.intellij.codeInspection.ProblemDescriptor -import com.intellij.modcommand.ModCommand -import com.intellij.modcommand.ModCommandQuickFix -import com.intellij.openapi.project.Project -import com.intellij.psi.PsiType -import com.intellij.psi.search.GlobalSearchScope -import org.jetbrains.uast.* -import org.jetbrains.uast.generate.getUastElementFactory -import org.jetbrains.uast.generate.shortenReference -import java.util.function.Consumer - -class ReplaceReferenceQuickFix(propertyName: String) : ModCommandQuickFix() { - @SafeFieldForPreview - private val qualifiedReference = PROPERTIES_TO_OPTIMIZE[propertyName] ?: error("Unknown property") - - override fun getFamilyName(): String = CommonQuickFixBundle.message("fix.replace.with.x", qualifiedReference) - - override fun perform(project: Project, descriptor: ProblemDescriptor): ModCommand { - val uCall = descriptor.psiElement.getUastParentOfType() - val uFactory = uCall?.getUastElementFactory(project) - val oldUParent = uCall?.getQualifiedParentOrThis() - val newUElement = uFactory?.createQualifiedReference(qualifiedReference.name, null)?.let { - qualifiedReference.callExpressions.fold(it as? UExpression) { receiver, callExpressionArgs -> - uFactory.createCallExpression(receiver?.getQualifiedParentOrThis(), - callExpressionArgs.name, - emptyList(), - PsiType.getTypeByName(callExpressionArgs.returnType, project, GlobalSearchScope.allScope(project)), - UastCallKind.METHOD_CALL) - } - } ?: return ModCommand.nop() - - val oldPsi = oldUParent?.sourcePsi ?: return ModCommand.nop() - val newPsi = newUElement.getQualifiedParentOrThis().sourcePsi ?: return ModCommand.nop() - return ModCommand.psiUpdate(oldPsi, Consumer { it.replace(newPsi).toUElementOfType()?.shortenReference() }) - } - - data class QualifiedReference(val name: String, val callExpressions: List) { - override fun toString(): String = if (callExpressions.isEmpty()) { - name - } - else { - "$name.${callExpressions.joinToString(separator = "().", postfix = "()") { it.name }}" - } - } - - data class CallExpression(val name: String, val returnType: String) - - companion object { - val PROPERTIES_TO_OPTIMIZE = mapOf( - "file.separator" to QualifiedReference("java.nio.file.FileSystems", - listOf(CallExpression("getDefault", "java.nio.file.FileSystem"), - CallExpression("getSeparator", "java.lang.String"))), - "path.separator" to QualifiedReference("java.io.File.pathSeparator", emptyList()), - "line.separator" to QualifiedReference("java.lang.System", - listOf(CallExpression("lineSeparator", "java.lang.String"))), - "file.encoding" to QualifiedReference("java.nio.charset.Charset", - listOf(CallExpression("defaultCharset", "java.nio.charset.Charset"), - CallExpression("displayName", "java.lang.String")))) - } -} \ No newline at end of file diff --git a/jvm/jvm-analysis-impl/src/com/intellij/codeInspection/test/junit/JUnitAssertEqualsMayBeAssertSameInspection.kt b/jvm/jvm-analysis-impl/src/com/intellij/codeInspection/test/junit/JUnitAssertEqualsMayBeAssertSameInspection.kt index 66e6960920a5..aa0771e671de 100644 --- a/jvm/jvm-analysis-impl/src/com/intellij/codeInspection/test/junit/JUnitAssertEqualsMayBeAssertSameInspection.kt +++ b/jvm/jvm-analysis-impl/src/com/intellij/codeInspection/test/junit/JUnitAssertEqualsMayBeAssertSameInspection.kt @@ -3,7 +3,9 @@ package com.intellij.codeInspection.test.junit import com.intellij.analysis.JvmAnalysisBundle import com.intellij.codeInspection.* -import com.intellij.codeInspection.fix.ReplaceMethodCallFix +import com.intellij.codeInspection.fix.CallableExpression +import com.intellij.codeInspection.fix.Method +import com.intellij.codeInspection.fix.ReplaceCallableExpressionQuickFix import com.intellij.psi.* import com.intellij.psi.util.PsiUtil import com.intellij.uast.UastHintedVisitorAdapter @@ -32,7 +34,9 @@ private class JUnitAssertEqualsMayBeAssertSameVisitor(private val holder: Proble if (!couldBeAssertSameArgument(assertHint.firstArgument)) return true if (!couldBeAssertSameArgument(assertHint.secondArgument)) return true val message = JvmAnalysisBundle.message("jvm.inspections.junit.assertequals.may.be.assertsame.problem.descriptor") - holder.registerUProblem(node, message, ReplaceMethodCallFix("assertSame")) + holder.registerUProblem(node, message, ReplaceCallableExpressionQuickFix(CallableExpression( + null, + listOf(Method("assertSame", null, node.valueArguments))))) return true } diff --git a/jvm/jvm-analysis-impl/src/com/intellij/codeInspection/test/junit/JUnitAssertEqualsOnArrayInspection.kt b/jvm/jvm-analysis-impl/src/com/intellij/codeInspection/test/junit/JUnitAssertEqualsOnArrayInspection.kt index 5b19e44943ee..4316b36ac4c0 100644 --- a/jvm/jvm-analysis-impl/src/com/intellij/codeInspection/test/junit/JUnitAssertEqualsOnArrayInspection.kt +++ b/jvm/jvm-analysis-impl/src/com/intellij/codeInspection/test/junit/JUnitAssertEqualsOnArrayInspection.kt @@ -3,7 +3,9 @@ package com.intellij.codeInspection.test.junit import com.intellij.analysis.JvmAnalysisBundle import com.intellij.codeInspection.* -import com.intellij.codeInspection.fix.ReplaceMethodCallFix +import com.intellij.codeInspection.fix.CallableExpression +import com.intellij.codeInspection.fix.Method +import com.intellij.codeInspection.fix.ReplaceCallableExpressionQuickFix import com.intellij.psi.PsiArrayType import com.intellij.psi.PsiElementVisitor import com.intellij.psi.PsiFile @@ -35,7 +37,9 @@ private class JUnitAssertEqualsOnArrayVisitor(private val holder: ProblemsHolder val sectArgType = assertHint.secondArgument.getExpressionType() ?: return true if (firstArgType !is PsiArrayType || sectArgType !is PsiArrayType) return true val message = JvmAnalysisBundle.message("jvm.inspections.junit.assertequals.on.array.problem.descriptor") - holder.registerUProblem(node, message, ReplaceMethodCallFix("assertArrayEquals")) + holder.registerUProblem(node, message, ReplaceCallableExpressionQuickFix(CallableExpression( + null, + listOf(Method("assertArrayEquals", null, node.valueArguments))))) return true } } \ No newline at end of file diff --git a/jvm/jvm-analysis-java-tests/testSrc/com/intellij/codeInspection/tests/java/JavaSystemGetPropertyInspectionTest.kt b/jvm/jvm-analysis-java-tests/testSrc/com/intellij/codeInspection/tests/java/JavaSystemGetPropertyInspectionTest.kt index def46895e233..24eb47579fc7 100644 --- a/jvm/jvm-analysis-java-tests/testSrc/com/intellij/codeInspection/tests/java/JavaSystemGetPropertyInspectionTest.kt +++ b/jvm/jvm-analysis-java-tests/testSrc/com/intellij/codeInspection/tests/java/JavaSystemGetPropertyInspectionTest.kt @@ -32,7 +32,7 @@ class JavaSystemGetPropertyInspectionTest : SystemGetPropertyInspectionTestBase( FileSystems.getDefault().getSeparator(); } } - """.trimIndent(), "Replace with 'java.nio.file.FileSystems.getDefault().getSeparator()'") + """.trimIndent(), "Replace with 'java.nio.file.FileSystems.getDefault().getSeparator()'", true) } fun `test quickfix path-separator`() { @@ -50,7 +50,7 @@ class JavaSystemGetPropertyInspectionTest : SystemGetPropertyInspectionTestBase( File.pathSeparator; } } - """.trimIndent(), "Replace with 'java.io.File.pathSeparator'") + """.trimIndent(), "Replace with 'java.io.File.pathSeparator'", true) } fun `test quickfix line-separator`() { @@ -66,7 +66,7 @@ class JavaSystemGetPropertyInspectionTest : SystemGetPropertyInspectionTestBase( System.lineSeparator(); } } - """.trimIndent(), "Replace with 'java.lang.System.lineSeparator()'") + """.trimIndent(), "Replace with 'java.lang.System.lineSeparator()'", true) } fun `test quickfix file-encoding`() { @@ -84,6 +84,6 @@ class JavaSystemGetPropertyInspectionTest : SystemGetPropertyInspectionTestBase( Charset.defaultCharset().displayName(); } } - """.trimIndent(), "Replace with 'java.nio.charset.Charset.defaultCharset().displayName()'") + """.trimIndent(), "Replace with 'java.nio.charset.Charset.defaultCharset().displayName()'", true) } } \ No newline at end of file diff --git a/jvm/jvm-analysis-kotlin-tests/testSrc/com/intellij/codeInspection/tests/kotlin/KotlinSystemGetPropertyInspectionTest.kt b/jvm/jvm-analysis-kotlin-tests/testSrc/com/intellij/codeInspection/tests/kotlin/KotlinSystemGetPropertyInspectionTest.kt index 02c3f65250bb..0dd6c07f6098 100644 --- a/jvm/jvm-analysis-kotlin-tests/testSrc/com/intellij/codeInspection/tests/kotlin/KotlinSystemGetPropertyInspectionTest.kt +++ b/jvm/jvm-analysis-kotlin-tests/testSrc/com/intellij/codeInspection/tests/kotlin/KotlinSystemGetPropertyInspectionTest.kt @@ -26,7 +26,7 @@ class KotlinSystemGetPropertyInspectionTest : SystemGetPropertyInspectionTestBas fun foo() { FileSystems.getDefault().getSeparator() } - """.trimIndent(), "Replace with 'java.nio.file.FileSystems.getDefault().getSeparator()'") + """.trimIndent(), "Replace with 'java.nio.file.FileSystems.getDefault().getSeparator()'", true) } fun `test quickfix path-separator`() { @@ -40,7 +40,7 @@ class KotlinSystemGetPropertyInspectionTest : SystemGetPropertyInspectionTestBas fun foo() { File.pathSeparator } - """.trimIndent(), "Replace with 'java.io.File.pathSeparator'") + """.trimIndent(), "Replace with 'java.io.File.pathSeparator'", true) } fun `test quickfix line-separator`() { @@ -52,7 +52,7 @@ class KotlinSystemGetPropertyInspectionTest : SystemGetPropertyInspectionTestBas fun foo() { System.lineSeparator() } - """.trimIndent(), "Replace with 'java.lang.System.lineSeparator()'") + """.trimIndent(), "Replace with 'java.lang.System.lineSeparator()'", true) } fun `test quickfix file-encoding`() { @@ -66,6 +66,6 @@ class KotlinSystemGetPropertyInspectionTest : SystemGetPropertyInspectionTestBas fun foo() { Charset.defaultCharset().displayName() } - """.trimIndent(), "Replace with 'java.nio.charset.Charset.defaultCharset().displayName()'") + """.trimIndent(), "Replace with 'java.nio.charset.Charset.defaultCharset().displayName()'", true) } } \ No newline at end of file