[Code Analysis. Inspection] IDEA-234302 System.getProperty(str) inspection implementation. Review Refactor

GitOrigin-RevId: e793b2306586a3ea6e12691b5d887fe1cf0b2913

java/java-analysis-impl/resources/messages/InspectionGadgetsBundle.properties

@@ -1087,7 +1087,6 @@ class.loader.instantiation.problem.descriptor=Instantiation of <code>#ref</code>
 public.static.array.field.problem.descriptor='public static' array field <code>#ref</code>, compromising security #loc
 public.static.collection.field.problem.descriptor='public static' collection field <code>#ref</code>, compromising security #loc
 abstract.class.with.only.one.direct.inheritor.problem.descriptor=Abstract class <code>#ref</code> has only one direct inheritor #loc
-system.get.property.problem.descriptor=Call <code>#ref</code> can be simplified for ''{0}''
 
 #other
 abstract.method.overrides.abstract.method.remove.quickfix=Remove redundant abstract method declaration

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 <code>#ref</code> 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

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
     }
   }

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
     }
   }

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<UCallExpression>()
+    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<UReferenceExpression>()?.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<Method>) {
+  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<UExpression> = emptyList())

jvm/jvm-analysis-impl/src/com/intellij/codeInspection/fix/ReplaceMethodCallFix.kt

@@ -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<UCallExpression>() ?: 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)
-  }
-}

jvm/jvm-analysis-impl/src/com/intellij/codeInspection/fix/ReplaceReferenceQuickFix.kt

@@ -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<UCallExpression>()
-    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<UReferenceExpression>()?.shortenReference() })
-  }
-
-  data class QualifiedReference(val name: String, val callExpressions: List<CallExpression>) {
-    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"))))
-  }
-}

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
   }
 

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
   }
 }

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)
   }
 }

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)
   }
 }