IJPL-115558: Fix UInjectionHost false-positive

GitOrigin-RevId: 9f8685ae87d7c64688dea2c59ddfa4af49824f45
This commit is contained in:
Karol Lewandowski
2024-06-18 14:46:10 +02:00
committed by intellij-monorepo-bot
parent 2777ad9723
commit b236d7801e
3 changed files with 102 additions and 22 deletions

View File

@@ -39,6 +39,16 @@ private val UAST_VISITOR_BASE_CLASS_NAMES = setOf(
"org.jetbrains.uast.visitor.AbstractUastNonRecursiveVisitor"
)
/**
* Stores the hint classes and their allowed visited elements.
* The reason for exceptions is that `UInjectionHost` is an interface that is not
* a part of `UElement` interfaces hierarchy but is implemented by concrete elements.
*/
private val HINT_EXCEPTIONS = mapOf(
"org.jetbrains.uast.expressions.UInjectionHost" to setOf("org.jetbrains.uast.ULiteralExpression",
"org.jetbrains.uast.UPolyadicExpression")
)
internal class UastHintedVisitorAdapterHintsInspection : DevKitUastInspectionBase() {
override fun isAllowed(holder: ProblemsHolder): Boolean {
@@ -91,9 +101,11 @@ internal class UastHintedVisitorAdapterHintsInspection : DevKitUastInspectionBas
checkRedundantHints(classLiteralAndExpandedClassesList, overriddenMethods, holder)
}
private fun checkMissingHints(classLiteralAndExpandedClassesList: List<ClassLiteralAndExpandedClasses>,
overriddenMethods: List<UMethod>,
holder: ProblemsHolder) {
private fun checkMissingHints(
classLiteralAndExpandedClassesList: List<ClassLiteralAndExpandedClasses>,
overriddenMethods: List<UMethod>,
holder: ProblemsHolder,
) {
val hintClasses = classLiteralAndExpandedClassesList.flatMap { it.coveredClasses }.toSet()
val methodsNotInHintClasses = overriddenMethods.filter { !it.isReachedByHints(hintClasses) }
for (redundantMethod in methodsNotInHintClasses) {
@@ -114,13 +126,19 @@ internal class UastHintedVisitorAdapterHintsInspection : DevKitUastInspectionBas
private fun UMethod.isReachedByHints(hintClasses: Set<PsiClass>): Boolean {
val visitedElementClass = this.resolveTheOnlyParameterClass() ?: return true // shouldn't happen
val visitedElementClassQualifiedName = visitedElementClass.qualifiedName
return visitedElementClass in hintClasses ||
hintClasses.any { it.isInheritor(visitedElementClass, true) } // visitElement(UElement) is covered by any hint class
hintClasses.any {
HINT_EXCEPTIONS[it.qualifiedName]?.contains(visitedElementClassQualifiedName) == true ||
it.isInheritor(visitedElementClass, true) // visitElement(UElement) is covered by any hint class
}
}
private fun checkRedundantHints(classLiteralAndExpandedClassesList: List<ClassLiteralAndExpandedClasses>,
overriddenMethods: List<UMethod>,
holder: ProblemsHolder) {
private fun checkRedundantHints(
classLiteralAndExpandedClassesList: List<ClassLiteralAndExpandedClasses>,
overriddenMethods: List<UMethod>,
holder: ProblemsHolder,
) {
val hintClassesNotInMethods = classLiteralAndExpandedClassesList
.filter { literalAndClasses ->
!overriddenMethods.any { it.isReachedByHints(literalAndClasses.coveredClasses) }
@@ -199,7 +217,7 @@ internal class UastHintedVisitorAdapterHintsInspection : DevKitUastInspectionBas
*/
private class ClassLiteralAndExpandedClasses(
val classLiteral: UClassLiteralExpression,
val coveredClasses: Set<PsiClass>
val coveredClasses: Set<PsiClass>,
)
}

View File

@@ -10,7 +10,7 @@ class UastHintedVisitorAdapterHintsInspectionTest : UastHintedVisitorAdapterHint
"""
class TestInspection extends LocalInspectionTool {
@Override
public PsiElementVisitor buildVisitor(ProblemsHolder holder, boolean isOnTheFly) {
public PsiElementVisitor buildVisitor(ProblemsHolder holder, boolean isOnTheFly) {
return UastHintedVisitorAdapter.create(
Language.ANY,
new AbstractUastNonRecursiveVisitor() {
@@ -40,7 +40,7 @@ class UastHintedVisitorAdapterHintsInspectionTest : UastHintedVisitorAdapterHint
@SuppressWarnings("unchecked")
private static final Class<? extends UElement>[] HINTS = new Class[]{UForExpression.class};
@Override
public PsiElementVisitor buildVisitor(ProblemsHolder holder, boolean isOnTheFly) {
public PsiElementVisitor buildVisitor(ProblemsHolder holder, boolean isOnTheFly) {
return UastHintedVisitorAdapter.create(
Language.ANY,
new AbstractUastNonRecursiveVisitor() {
@@ -69,7 +69,7 @@ class UastHintedVisitorAdapterHintsInspectionTest : UastHintedVisitorAdapterHint
class TestInspection extends LocalInspectionTool {
@SuppressWarnings("unchecked")
@Override
public PsiElementVisitor buildVisitor(ProblemsHolder holder, boolean isOnTheFly) {
public PsiElementVisitor buildVisitor(ProblemsHolder holder, boolean isOnTheFly) {
Class<? extends UElement>[] hints = new Class[]{UForExpression.class};
return UastHintedVisitorAdapter.create(
Language.ANY,
@@ -99,7 +99,7 @@ class UastHintedVisitorAdapterHintsInspectionTest : UastHintedVisitorAdapterHint
class TestInspection extends LocalInspectionTool {
@SuppressWarnings("unchecked")
@Override
public PsiElementVisitor buildVisitor(ProblemsHolder holder, boolean isOnTheFly) {
public PsiElementVisitor buildVisitor(ProblemsHolder holder, boolean isOnTheFly) {
return UastHintedVisitorAdapter.create(Language.ANY, new MyVisitor(), new Class[]{UForExpression.class});
}
private static class MyVisitor extends AbstractUastNonRecursiveVisitor {
@@ -137,7 +137,7 @@ class UastHintedVisitorAdapterHintsInspectionTest : UastHintedVisitorAdapterHint
private void inspectLoopExpression() {}
};
@Override
public PsiElementVisitor buildVisitor(ProblemsHolder holder, boolean isOnTheFly) {
public PsiElementVisitor buildVisitor(ProblemsHolder holder, boolean isOnTheFly) {
return UastHintedVisitorAdapter.create(
Language.ANY,
VISITOR,
@@ -153,7 +153,7 @@ class UastHintedVisitorAdapterHintsInspectionTest : UastHintedVisitorAdapterHint
"""
class TestInspection extends LocalInspectionTool {
@Override
public PsiElementVisitor buildVisitor(ProblemsHolder holder, boolean isOnTheFly) {
public PsiElementVisitor buildVisitor(ProblemsHolder holder, boolean isOnTheFly) {
return UastHintedVisitorAdapter.create(
Language.ANY,
new AbstractUastNonRecursiveVisitor() {
@@ -178,7 +178,7 @@ class UastHintedVisitorAdapterHintsInspectionTest : UastHintedVisitorAdapterHint
@SuppressWarnings("unchecked")
private static final Class<? extends UElement>[] HINTS = new Class[]{UForExpression.class, <warning descr="'UDoWhileExpression' is provided in hints, but the element is not visited in the visitor">UDoWhileExpression.class</warning>};
@Override
public PsiElementVisitor buildVisitor(ProblemsHolder holder, boolean isOnTheFly) {
public PsiElementVisitor buildVisitor(ProblemsHolder holder, boolean isOnTheFly) {
return UastHintedVisitorAdapter.create(
Language.ANY,
new AbstractUastNonRecursiveVisitor() {
@@ -202,7 +202,7 @@ class UastHintedVisitorAdapterHintsInspectionTest : UastHintedVisitorAdapterHint
class TestInspection extends LocalInspectionTool {
@SuppressWarnings("unchecked")
@Override
public PsiElementVisitor buildVisitor(ProblemsHolder holder, boolean isOnTheFly) {
public PsiElementVisitor buildVisitor(ProblemsHolder holder, boolean isOnTheFly) {
Class<? extends UElement>[] hints = new Class[]{UForExpression.class, <warning descr="'UDoWhileExpression' is provided in hints, but the element is not visited in the visitor">UDoWhileExpression.class</warning>};
return UastHintedVisitorAdapter.create(
Language.ANY,
@@ -226,7 +226,7 @@ class UastHintedVisitorAdapterHintsInspectionTest : UastHintedVisitorAdapterHint
"""
class TestInspection extends LocalInspectionTool {
@Override
public PsiElementVisitor buildVisitor(ProblemsHolder holder, boolean isOnTheFly) {
public PsiElementVisitor buildVisitor(ProblemsHolder holder, boolean isOnTheFly) {
return UastHintedVisitorAdapter.create(
Language.ANY,
new MyVisitor(),
@@ -250,7 +250,7 @@ class UastHintedVisitorAdapterHintsInspectionTest : UastHintedVisitorAdapterHint
"""
class TestInspection extends LocalInspectionTool {
@Override
public PsiElementVisitor buildVisitor(ProblemsHolder holder, boolean isOnTheFly) {
public PsiElementVisitor buildVisitor(ProblemsHolder holder, boolean isOnTheFly) {
AbstractUastNonRecursiveVisitor visitor = new AbstractUastNonRecursiveVisitor() {
@Override
public boolean visitForExpression(UForExpression node) {
@@ -274,7 +274,7 @@ class UastHintedVisitorAdapterHintsInspectionTest : UastHintedVisitorAdapterHint
"""
class TestInspection extends LocalInspectionTool {
@Override
public PsiElementVisitor buildVisitor(ProblemsHolder holder, boolean isOnTheFly) {
public PsiElementVisitor buildVisitor(ProblemsHolder holder, boolean isOnTheFly) {
return UastHintedVisitorAdapter.create(
Language.ANY,
new AbstractUastNonRecursiveVisitor() {
@@ -302,7 +302,7 @@ class UastHintedVisitorAdapterHintsInspectionTest : UastHintedVisitorAdapterHint
"""
class TestInspection extends LocalInspectionTool {
@Override
public PsiElementVisitor buildVisitor(ProblemsHolder holder, boolean isOnTheFly) {
public PsiElementVisitor buildVisitor(ProblemsHolder holder, boolean isOnTheFly) {
return UastHintedVisitorAdapter.create(
Language.ANY,
new AbstractUastNonRecursiveVisitor() {
@@ -328,7 +328,7 @@ class UastHintedVisitorAdapterHintsInspectionTest : UastHintedVisitorAdapterHint
"""
class TestInspection extends LocalInspectionTool {
@Override
public PsiElementVisitor buildVisitor(ProblemsHolder holder, boolean isOnTheFly) {
public PsiElementVisitor buildVisitor(ProblemsHolder holder, boolean isOnTheFly) {
return UastHintedVisitorAdapter.create(
Language.ANY,
new AbstractUastNonRecursiveVisitor() {
@@ -346,6 +346,37 @@ class UastHintedVisitorAdapterHintsInspectionTest : UastHintedVisitorAdapterHint
""".trimIndent())
}
fun `test should not report when ULiteralExpression or UPolyadicExpression visited and UInjectionHost element hinted`() {
doTest(
"""
class TestInspection extends LocalInspectionTool {
@Override
public PsiElementVisitor buildVisitor(ProblemsHolder holder, boolean isOnTheFly) {
return UastHintedVisitorAdapter.create(
Language.ANY,
new AbstractUastNonRecursiveVisitor() {
@Override
public boolean visitPolyadicExpression(UPolyadicExpression node) {
if (!(node instanceof UInjectionHost)) return true;
processInjectionHost((UInjectionHost)node);
return super.visitPolyadicExpression(node);
}
@Override
public boolean visitLiteralExpression(ULiteralExpression node) {
if (!(node instanceof UInjectionHost)) return true;
processInjectionHost((UInjectionHost)node);
return super.visitExpression(node);
}
private void processInjectionHost(UInjectionHost node) {}
},
new Class[]{UInjectionHost.class}
);
}
}
""".trimIndent())
}
private fun doTest(@Language("JAVA") code: String) {
myFixture.configureByText(
"TestInspection.java",
@@ -357,6 +388,7 @@ class UastHintedVisitorAdapterHintsInspectionTest : UastHintedVisitorAdapterHint
import com.intellij.psi.PsiElementVisitor;
import com.intellij.uast.UastHintedVisitorAdapter;
import org.jetbrains.uast.*;
import org.jetbrains.uast.expressions.*;
import org.jetbrains.uast.visitor.AbstractUastNonRecursiveVisitor;
$code
""".trimIndent())

View File

@@ -1,5 +1,5 @@
// Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license.
package org.jetbrains.idea.devkit.kotlin.inspections;
package org.jetbrains.idea.devkit.kotlin.inspections
import org.intellij.lang.annotations.Language
import org.jetbrains.idea.devkit.inspections.UastHintedVisitorAdapterHintsInspectionTestBase
@@ -331,6 +331,35 @@ class KtUastHintedVisitorAdapterHintsInspectionTest : UastHintedVisitorAdapterHi
""".trimIndent())
}
fun `test should not report when ULiteralExpression or UPolyadicExpression visited and UInjectionHost element hinted`() {
doTest(
"""
class TestInspection : LocalInspectionTool() {
@Override
override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean): PsiElementVisitor {
return UastHintedVisitorAdapter.create(
Language.ANY,
object : AbstractUastNonRecursiveVisitor() {
override fun visitPolyadicExpression(node: UPolyadicExpression): Boolean {
if (node !is UInjectionHost) return true
processInjectionHost(node)
return super.visitPolyadicExpression(node)
}
override fun visitLiteralExpression(node: ULiteralExpression): Boolean {
if (node !is UInjectionHost) return true
processInjectionHost(node)
return super.visitExpression(node)
}
private fun processInjectionHost(@Suppress("UNUSED_PARAMETER") node: UInjectionHost) {}
},
arrayOf(UInjectionHost::class.java)
)
}
}
""".trimIndent())
}
private fun doTest(@Language("kotlin") code: String) {
myFixture.configureByText(
"TestInspection.kt",
@@ -342,6 +371,7 @@ class KtUastHintedVisitorAdapterHintsInspectionTest : UastHintedVisitorAdapterHi
import com.intellij.psi.PsiElementVisitor
import com.intellij.uast.UastHintedVisitorAdapter
import org.jetbrains.uast.*
import org.jetbrains.uast.expressions.*
import org.jetbrains.uast.visitor.AbstractUastNonRecursiveVisitor
$code
""".trimIndent())