From b236d7801e7d2cd37c17fa95119bc3b80913b765 Mon Sep 17 00:00:00 2001 From: Karol Lewandowski Date: Tue, 18 Jun 2024 14:46:10 +0200 Subject: [PATCH] IJPL-115558: Fix UInjectionHost false-positive GitOrigin-RevId: 9f8685ae87d7c64688dea2c59ddfa4af49824f45 --- ...UastHintedVisitorAdapterHintsInspection.kt | 34 ++++++++--- ...HintedVisitorAdapterHintsInspectionTest.kt | 58 ++++++++++++++----- ...HintedVisitorAdapterHintsInspectionTest.kt | 32 +++++++++- 3 files changed, 102 insertions(+), 22 deletions(-) diff --git a/plugins/devkit/devkit-core/src/inspections/UastHintedVisitorAdapterHintsInspection.kt b/plugins/devkit/devkit-core/src/inspections/UastHintedVisitorAdapterHintsInspection.kt index 0b9c21c89bb6..8b8a3139953a 100644 --- a/plugins/devkit/devkit-core/src/inspections/UastHintedVisitorAdapterHintsInspection.kt +++ b/plugins/devkit/devkit-core/src/inspections/UastHintedVisitorAdapterHintsInspection.kt @@ -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, - overriddenMethods: List, - holder: ProblemsHolder) { + private fun checkMissingHints( + classLiteralAndExpandedClassesList: List, + overriddenMethods: List, + 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): 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, - overriddenMethods: List, - holder: ProblemsHolder) { + private fun checkRedundantHints( + classLiteralAndExpandedClassesList: List, + overriddenMethods: List, + 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 + val coveredClasses: Set, ) } diff --git a/plugins/devkit/devkit-java-tests/testSrc/org/jetbrains/idea/devkit/inspections/UastHintedVisitorAdapterHintsInspectionTest.kt b/plugins/devkit/devkit-java-tests/testSrc/org/jetbrains/idea/devkit/inspections/UastHintedVisitorAdapterHintsInspectionTest.kt index 01e4ebeb3c6d..6e881e80e19a 100644 --- a/plugins/devkit/devkit-java-tests/testSrc/org/jetbrains/idea/devkit/inspections/UastHintedVisitorAdapterHintsInspectionTest.kt +++ b/plugins/devkit/devkit-java-tests/testSrc/org/jetbrains/idea/devkit/inspections/UastHintedVisitorAdapterHintsInspectionTest.kt @@ -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[] 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[] 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[] HINTS = new Class[]{UForExpression.class, UDoWhileExpression.class}; @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[] hints = new Class[]{UForExpression.class, UDoWhileExpression.class}; 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()) diff --git a/plugins/devkit/devkit-kotlin-tests/testSrc/org/jetbrains/idea/devkit/kotlin/inspections/KtUastHintedVisitorAdapterHintsInspectionTest.kt b/plugins/devkit/devkit-kotlin-tests/testSrc/org/jetbrains/idea/devkit/kotlin/inspections/KtUastHintedVisitorAdapterHintsInspectionTest.kt index a3dddd513a23..172e33cb25dd 100644 --- a/plugins/devkit/devkit-kotlin-tests/testSrc/org/jetbrains/idea/devkit/kotlin/inspections/KtUastHintedVisitorAdapterHintsInspectionTest.kt +++ b/plugins/devkit/devkit-kotlin-tests/testSrc/org/jetbrains/idea/devkit/kotlin/inspections/KtUastHintedVisitorAdapterHintsInspectionTest.kt @@ -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())