[jvm] Clean up SuppressionAnnotationInspection

Improves inspection message and code structure. #IDEA-337709

GitOrigin-RevId: cc1fd2482724ea235a301b770abbcc02d1fcf6cc
This commit is contained in:
Bart van Helvert
2023-12-12 23:52:06 +01:00
committed by intellij-monorepo-bot
parent a1720d6cb9
commit e43e1dd876
4 changed files with 177 additions and 195 deletions

View File

@@ -206,7 +206,8 @@ dialog.title.choose.annotation=Choose {0}
jvm.inspections.dependency.intention.description=Opens a dialog to configure dependency rules between scopes.
inspection.suppression.annotation.display.name=Inspection suppression annotation
inspection.suppression.annotation.problem.descriptor=Inspection suppression annotation <code>#ref</code> #loc
inspection.suppression.annotation.problem.descriptor=Annotation suppresses {0} #loc
inspection.suppression.comment.problem.descriptor=Comment suppresses {0} #loc
ignored.suppressions=Ignored suppressions:
remove.suppress.comment.fix.family.name=Remove //{0}
allow.suppressions.fix.family.name=Allow suppressions

View File

@@ -3,6 +3,7 @@ package com.intellij.codeInspection
import com.intellij.analysis.JvmAnalysisBundle
import com.intellij.codeInspection.options.OptPane
import com.intellij.ide.nls.NlsMessages
import com.intellij.lang.LanguageCommenters
import com.intellij.lang.jvm.JvmModifiersOwner
import com.intellij.lang.jvm.actions.annotationRequest
@@ -21,6 +22,28 @@ import org.jetbrains.uast.*
import org.jetbrains.uast.expressions.UInjectionHost
import org.jetbrains.uast.visitor.AbstractUastNonRecursiveVisitor
@VisibleForTesting
class SuppressionAnnotationInspection : AbstractBaseUastLocalInspectionTool() {
var myAllowedSuppressions: MutableList<String> = mutableListOf()
override fun getOptionsPane(): OptPane = OptPane.pane(
OptPane.stringList("myAllowedSuppressions", JvmAnalysisBundle.message("ignored.suppressions"))
)
override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean): PsiElementVisitor {
return UastHintedVisitorAdapter.create(
holder.file.language,
SuppressionAnnotationVisitor(holder, myAllowedSuppressions, this),
arrayOf(UAnnotation::class.java, UComment::class.java),
true
)
}
override fun isSuppressedFor(element: PsiElement): Boolean = false
override fun getBatchSuppressActions(element: PsiElement?): Array<SuppressQuickFix> = SuppressQuickFix.EMPTY_ARRAY
}
private class SuppressAnnotationDescriptor(val pkg: String, val shortName: String, val attributeValue: String)
private val suppressAnnotations = listOf(
@@ -28,160 +51,118 @@ private val suppressAnnotations = listOf(
SuppressAnnotationDescriptor("java.lang", "SuppressWarnings", "value")
)
@VisibleForTesting
class SuppressionAnnotationInspection : AbstractBaseUastLocalInspectionTool() {
var myAllowedSuppressions: MutableList<String> = ArrayList()
override fun getOptionsPane(): OptPane {
return OptPane.pane(OptPane.stringList("myAllowedSuppressions", JvmAnalysisBundle.message("ignored.suppressions")))
private fun UAnnotation.suppressIds(): List<String>? {
fun UAnnotation.suppressDescriptor(): SuppressAnnotationDescriptor? {
val shortName = uastAnchor?.name ?: return null
if (suppressAnnotations.none { descr -> descr.shortName == shortName }) return null
return suppressAnnotations.firstOrNull { descr -> "${descr.pkg}.${descr.shortName}" == qualifiedName }
}
override fun buildVisitor(holder: ProblemsHolder, isOnTheFly: Boolean): PsiElementVisitor {
return UastHintedVisitorAdapter.create(
holder.file.language,
SuppressionAnnotationVisitor(holder),
arrayOf(UAnnotation::class.java, UComment::class.java),
true
val suppressDescriptor = suppressDescriptor()
if (suppressDescriptor == null) return null
return flattenedAttributeValues(suppressDescriptor.attributeValue).mapNotNull {
when (it) {
is UInjectionHost, is UReferenceExpression -> it.evaluateString()
else -> null
}
}
}
private fun UComment.suppressIds(): List<String>? {
fun UComment.isSuppressComment(): Boolean {
val commentPrefix = LanguageCommenters.INSTANCE.forLanguage(lang)?.lineCommentPrefix ?: return false
if (text.length <= commentPrefix.length && !text.startsWith(commentPrefix)) return false
val strippedComment = text.substring(commentPrefix.length).trim()
return strippedComment.startsWith(SuppressionUtilCore.SUPPRESS_INSPECTIONS_TAG_NAME)
}
if (!isSuppressComment()) return null
val matcher = SuppressionUtil.SUPPRESS_IN_LINE_COMMENT_PATTERN.matcher(text)
if (!matcher.matches()) return emptyList()
val suppressedIds = matcher.group(1).trim()
return StringUtil.tokenize(suppressedIds, ",").toList().map { it.trim() }
}
private class SuppressionAnnotationVisitor(
private val holder: ProblemsHolder,
private val allowedSuppressions: List<String>,
private val inspection: SuppressionAnnotationInspection // quick fixes might change inspection settings
) : AbstractUastNonRecursiveVisitor() {
override fun visitComment(node: UComment): Boolean {
val suppressIds = node.suppressIds() ?: return true
val fixes = when {
suppressIds.isEmpty() -> arrayOf(RemoveSuppressCommentFix())
suppressIds.any { !allowedSuppressions.contains(it) } -> arrayOf(RemoveSuppressCommentFix(), AllowSuppressionsFix(inspection))
else -> return true
}
val message = JvmAnalysisBundle.message(
"inspection.suppression.comment.problem.descriptor",
NlsMessages.formatAndList(suppressIds.map { "'$it'" })
)
holder.registerProblem(node.sourcePsi, message, *fixes)
return true
}
override fun isSuppressedFor(element: PsiElement): Boolean {
return false
override fun visitAnnotation(node: UAnnotation): Boolean {
val suppressIds = node.suppressIds() ?: return true
val fixes = when {
suppressIds.isEmpty() -> node.removeAnnotationFixes()
suppressIds.any { !allowedSuppressions.contains(it) } -> listOf(AllowSuppressionsFix(inspection)) + node.removeAnnotationFixes()
else -> return true
}
val message = JvmAnalysisBundle.message(
"inspection.suppression.annotation.problem.descriptor",
NlsMessages.formatAndList(suppressIds.map { "'$it'" })
)
holder.registerUProblem(node, message, *fixes.toTypedArray())
return true
}
override fun getBatchSuppressActions(element: PsiElement?): Array<SuppressQuickFix> {
return SuppressQuickFix.EMPTY_ARRAY
}
private inner class SuppressionAnnotationVisitor(private val holder: ProblemsHolder) : AbstractUastNonRecursiveVisitor() {
override fun visitComment(node: UComment): Boolean {
if (isSuppressComment(node)) {
val ids = getIdsFromComment(node.text)
if (ids != null) {
for (id in ids) {
if (!myAllowedSuppressions.contains(id)) {
registerProblem(node, true)
break
}
}
}
else {
registerProblem(node, false)
}
}
return true
}
private fun isSuppressComment(comment: UComment): Boolean {
val text = comment.text
val commentPrefix = LanguageCommenters.INSTANCE.forLanguage(comment.lang)?.lineCommentPrefix ?: return false
if (text.length <= commentPrefix.length && !text.startsWith(commentPrefix)) {
return false
}
val strippedComment = text.substring(commentPrefix.length).trim()
return strippedComment.startsWith(SuppressionUtilCore.SUPPRESS_INSPECTIONS_TAG_NAME)
}
private fun registerProblem(comment: UComment, addAllowSuppressionsFix: Boolean) {
val fixes = if (addAllowSuppressionsFix) arrayOf(RemoveSuppressCommentFix(), AllowSuppressionsFix())
else arrayOf(RemoveSuppressCommentFix())
holder.registerProblem(comment.sourcePsi, JvmAnalysisBundle.message("inspection.suppression.annotation.problem.descriptor"), *fixes)
}
override fun visitAnnotation(node: UAnnotation): Boolean {
val suppressDescriptor = node.suppressDescriptor()
if (suppressDescriptor == null) return true
val suppressIds = node.suppressIds(suppressDescriptor.attributeValue)
when {
suppressIds.isNotEmpty() && !myAllowedSuppressions.containsAll(suppressIds) -> registerProblem(node, true)
suppressIds.isEmpty() -> registerProblem(node, false)
}
return true
}
private fun registerProblem(annotation: UAnnotation, addAllowSuppressionsFix: Boolean) {
val sourcePsi = annotation.sourcePsi ?: return
var fixes: Array<LocalQuickFix> = if (addAllowSuppressionsFix) arrayOf(AllowSuppressionsFix()) else arrayOf()
val removeAnnotationFix = getRemoveAnnotationFix(annotation, sourcePsi)
if (removeAnnotationFix != null) {
fixes += removeAnnotationFix
}
holder.registerProblem(sourcePsi, JvmAnalysisBundle.message("inspection.suppression.annotation.problem.descriptor"), *fixes)
}
private fun getRemoveAnnotationFix(annotationElement: UAnnotation, sourcePsi: PsiElement): LocalQuickFix? {
val owner = annotationElement.getParentOfType<UDeclaration>()?.javaPsi as? JvmModifiersOwner ?: return null
val annotationQualifiedName = annotationElement.qualifiedName ?: return null
val actions = createRemoveAnnotationActions(owner, annotationRequest(annotationQualifiedName))
return IntentionWrapper.wrapToQuickFixes(actions, sourcePsi.containingFile).singleOrNull()
}
}
private class RemoveSuppressCommentFix : PsiUpdateModCommandQuickFix() {
override fun applyFix(project: Project, startElement: PsiElement, updater: ModPsiUpdater) {
startElement.delete()
}
override fun getFamilyName(): String {
return JvmAnalysisBundle.message("remove.suppress.comment.fix.family.name", SuppressionUtilCore.SUPPRESS_INSPECTIONS_TAG_NAME)
}
}
private inner class AllowSuppressionsFix : ModCommandQuickFix() {
override fun perform(project: Project, descriptor: ProblemDescriptor): ModCommand {
val psiElement = descriptor.psiElement
val ids = getIds(psiElement) ?: return ModCommand.nop()
return ModCommand.updateInspectionOption(psiElement, this@SuppressionAnnotationInspection) { inspection ->
for (id in ids) {
if (!inspection.myAllowedSuppressions.contains(id)) {
inspection.myAllowedSuppressions.add(id)
}
}
}
}
private fun getIds(psiElement: PsiElement): Collection<String>? {
val annotation = psiElement.toUElement()?.getParentOfType<UAnnotation>(strict = false)
if (annotation != null) {
val attributeValue = annotation.suppressDescriptor()?.attributeValue ?: return null
return annotation.suppressIds(attributeValue)
}
else {
val comment = psiElement.toUElement(UComment::class.java) ?: return null
return getIdsFromComment(comment.text)
}
}
override fun getName(): String {
return JvmAnalysisBundle.message("allow.suppressions.fix.text")
}
override fun getFamilyName(): String {
return JvmAnalysisBundle.message("allow.suppressions.fix.family.name")
}
private fun UAnnotation.removeAnnotationFixes(): List<LocalQuickFix> {
val owner = getParentOfType<UDeclaration>()?.javaPsi as? JvmModifiersOwner ?: return emptyList()
val file = sourcePsi?.containingFile ?: return emptyList()
val fqn = qualifiedName ?: return emptyList()
return IntentionWrapper.wrapToQuickFixes(createRemoveAnnotationActions(owner, annotationRequest(fqn)), file)
}
}
private fun UAnnotation.suppressDescriptor(): SuppressAnnotationDescriptor? {
val shortName = uastAnchor?.name ?: return null
if (suppressAnnotations.none { descr -> descr.shortName == shortName }) return null
return suppressAnnotations.firstOrNull { descr -> "${descr.pkg}.${descr.shortName}" == qualifiedName }
}
private class RemoveSuppressCommentFix : PsiUpdateModCommandQuickFix() {
override fun applyFix(project: Project, startElement: PsiElement, updater: ModPsiUpdater) {
startElement.delete()
}
private fun UAnnotation.suppressIds(attributeName: String): List<String> = flattenedAttributeValues(attributeName).mapNotNull {
when (it) {
is UInjectionHost, is UReferenceExpression -> it.evaluateString()
else -> null
override fun getFamilyName(): String {
return JvmAnalysisBundle.message("remove.suppress.comment.fix.family.name", SuppressionUtilCore.SUPPRESS_INSPECTIONS_TAG_NAME)
}
}
private fun getIdsFromComment(commentText: String): List<String>? {
val matcher = SuppressionUtil.SUPPRESS_IN_LINE_COMMENT_PATTERN.matcher(commentText)
if (matcher.matches()) {
val suppressedIds = matcher.group(1).trim()
return StringUtil.tokenize(suppressedIds, ",").toList().map { it.trim() }
private class AllowSuppressionsFix(private val inspection: SuppressionAnnotationInspection) : ModCommandQuickFix() {
override fun perform(project: Project, descriptor: ProblemDescriptor): ModCommand {
val anchor = descriptor.psiElement
val suppressIds = anchor.suppressIdsFromAnchor() ?: return ModCommand.nop()
if (suppressIds.isEmpty()) return ModCommand.nop()
return ModCommand.updateInspectionOption(anchor, inspection) { inspection ->
for (suppressId in suppressIds) {
if (!inspection.myAllowedSuppressions.contains(suppressId)) {
inspection.myAllowedSuppressions.add(suppressId)
}
}
}
}
else {
return null
private fun PsiElement.suppressIdsFromAnchor(): List<String>? {
return when (val suppressIdContainer = getUastParentOfTypes(arrayOf(UComment::class.java, UAnnotation::class.java), strict = false)) {
is UComment -> suppressIdContainer.suppressIds()
is UAnnotation -> suppressIdContainer.suppressIds()
else -> emptyList()
}
}
override fun getName(): String {
return JvmAnalysisBundle.message("allow.suppressions.fix.text")
}
override fun getFamilyName(): String {
return JvmAnalysisBundle.message("allow.suppressions.fix.family.name")
}
}

View File

@@ -10,27 +10,27 @@ class JavaSuppressionAnnotationInspectionTest : SuppressionAnnotationInspectionT
myFixture.testHighlighting(
JvmLanguage.JAVA,
"""
<warning descr="Inspection suppression annotation '@SuppressWarnings({\"ALL\", \"SuppressionAnnotation\"})'">@SuppressWarnings({"ALL", "SuppressionAnnotation"})</warning>
public class A {
<warning descr="Inspection suppression annotation '@SuppressWarnings(\"PublicField\")'">@SuppressWarnings("PublicField")</warning>
public String s;
<warning descr="Inspection suppression annotation '@SuppressWarnings({})'">@SuppressWarnings({})</warning>
public String t;
void foo() {
<warning descr="Inspection suppression annotation '//noinspection HardCodedStringLiteral'">//noinspection HardCodedStringLiteral</warning>
System.out.println("hello");
<warning descr="Inspection suppression annotation '// noinspection'">// noinspection</warning>
System.out.println();
}
@SuppressWarnings("FreeSpeech")
void bar() {
//noinspection FreeSpeech
System.out.println();
}
@<warning descr="Annotation suppresses 'ALL' and 'SuppressionAnnotation'">SuppressWarnings</warning>({"ALL", "SuppressionAnnotation"})
public class A {
@<warning descr="Annotation suppresses 'PublicField'">SuppressWarnings</warning>("PublicField")
public String s;
@<warning descr="Annotation suppresses">SuppressWarnings</warning>({})
public String t;
void foo() {
<warning descr="Comment suppresses 'HardCodedStringLiteral'">//noinspection HardCodedStringLiteral</warning>
System.out.println("hello");
<warning descr="Comment suppresses">// noinspection</warning>
System.out.println();
}
""".trimIndent(),
@SuppressWarnings("FreeSpeech")
void bar() {
//noinspection FreeSpeech
System.out.println();
}
}
""".trimIndent(),
fileName = "A"
)
}
@@ -38,7 +38,7 @@ class JavaSuppressionAnnotationInspectionTest : SuppressionAnnotationInspectionT
fun `test quickfix - remove annotation`() {
myFixture.testQuickFix(JvmLanguage.JAVA, """
public class A {
@SuppressWarnings("PublicField", "Hard<caret>CodedStringLiteral")
@Suppress<caret>Warnings("PublicField", "HardCodedStringLiteral")
public String s = "test";
}
""".trimIndent(), """
@@ -64,7 +64,7 @@ class JavaSuppressionAnnotationInspectionTest : SuppressionAnnotationInspectionT
fun `test quickfix - allow a single suppression from annotation`() {
testAllowSuppressionQuickFix(JvmLanguage.JAVA, """
public class A {
@SuppressWarnings("Public<caret>Field")
@Suppress<caret>Warnings("PublicField")
public String s = "test";
}
""".trimIndent(), "PublicField")
@@ -73,7 +73,7 @@ class JavaSuppressionAnnotationInspectionTest : SuppressionAnnotationInspectionT
fun `test quickfix - allow a single suppression from annotation when array form used`() {
testAllowSuppressionQuickFix(JvmLanguage.JAVA, """
public class A {
@SuppressWarnings({"Public<caret>Field"})
@Suppress<caret>Warnings({"PublicField"})
public String s = "test";
}
""".trimIndent(), "PublicField")
@@ -82,7 +82,7 @@ class JavaSuppressionAnnotationInspectionTest : SuppressionAnnotationInspectionT
fun `test quickfix - allow a single suppression from annotation when explicit attribute name exists`() {
testAllowSuppressionQuickFix(JvmLanguage.JAVA, """
public class A {
@SuppressWarnings(value = "Public<caret>Field")
@Suppress<caret>Warnings(value = "PublicField")
public String s = "test";
}
""".trimIndent(), "PublicField")
@@ -91,7 +91,7 @@ class JavaSuppressionAnnotationInspectionTest : SuppressionAnnotationInspectionT
fun `test quickfix - allow multiple suppressions from annotation when array form used`() {
testAllowSuppressionQuickFix(JvmLanguage.JAVA, """
public class A {
@SuppressWarnings({"Public<caret>Field", "HardCodedStringLiteral"})
@Suppress<caret>Warnings({"PublicField", "HardCodedStringLiteral"})
public String s = "test";
}
""".trimIndent(), "PublicField", "HardCodedStringLiteral")
@@ -100,7 +100,7 @@ class JavaSuppressionAnnotationInspectionTest : SuppressionAnnotationInspectionT
fun `test quickfix - allow multiple suppressions from annotation when explicit attribute name exists`() {
testAllowSuppressionQuickFix(JvmLanguage.JAVA, """
public class A {
@SuppressWarnings(value = {"Public<caret>Field", "HardCodedStringLiteral"})
@Suppress<caret>Warnings(value = {"PublicField", "HardCodedStringLiteral"})
public String s = "test";
}
""".trimIndent(), "PublicField", "HardCodedStringLiteral")

View File

@@ -10,28 +10,28 @@ class KotlinSuppressionAnnotationInspectionTest : SuppressionAnnotationInspectio
myFixture.testHighlighting(
JvmLanguage.KOTLIN,
"""
<warning descr="Inspection suppression annotation '@Suppress(\"ALL\", \"SuppressionAnnotation\")'">@Suppress("ALL", "SuppressionAnnotation")</warning>
class A {
<warning descr="Inspection suppression annotation '@Suppress(\"PublicField\")'">@Suppress("PublicField")</warning>
var s: String? = null
<warning descr="Inspection suppression annotation '@Suppress'">@Suppress</warning>
var t: String? = null
fun foo() {
<warning descr="Inspection suppression annotation '//noinspection HardCodedStringLiteral'">//noinspection HardCodedStringLiteral</warning>
any("hello")
<warning descr="Inspection suppression annotation '// noinspection'">// noinspection</warning>
any()
}
@Suppress("FreeSpeech")
fun bar() {
// noinspection FreeSpeech
any()
}
@<warning descr="Annotation suppresses 'ALL' and 'SuppressionAnnotation'">Suppress</warning>("ALL", "SuppressionAnnotation")
class A {
@<warning descr="Annotation suppresses 'PublicField'">Suppress</warning>("PublicField")
var s: String? = null
@<warning descr="Annotation suppresses">Suppress</warning>
var t: String? = null
fun foo() {
<warning descr="Comment suppresses 'HardCodedStringLiteral'">//noinspection HardCodedStringLiteral</warning>
any("hello")
<warning descr="Comment suppresses">// noinspection</warning>
any()
}
private fun any(s: String? = null): String? = s
@Suppress("FreeSpeech")
fun bar() {
// noinspection FreeSpeech
any()
}
}
private fun any(s: String? = null): String? = s
""".trimIndent()
)
}
@@ -39,7 +39,7 @@ class KotlinSuppressionAnnotationInspectionTest : SuppressionAnnotationInspectio
fun `test quickfix - remove annotation`() {
myFixture.testQuickFix(JvmLanguage.KOTLIN, """
class A {
@Suppress("PublicField", "Hard<caret>CodedStringLiteral")
@Supp<caret>ress("PublicField", "HardCodedStringLiteral")
var s: String = "test"
}
""".trimIndent(), """
@@ -65,7 +65,7 @@ class KotlinSuppressionAnnotationInspectionTest : SuppressionAnnotationInspectio
fun `test quickfix - allow a single suppression from annotation`() {
testAllowSuppressionQuickFix(JvmLanguage.KOTLIN, """
class A {
@Suppress("Public<caret>Field")
@Supp<caret>ress("PublicField")
var s: String = "test"
}
""".trimIndent(), "PublicField")
@@ -74,7 +74,7 @@ class KotlinSuppressionAnnotationInspectionTest : SuppressionAnnotationInspectio
fun `test quickfix - allow a single suppression from annotation when array form used`() {
testAllowSuppressionQuickFix(JvmLanguage.KOTLIN, """
class A {
@Suppress(["Public<caret>Field"])
@Supp<caret>ress(["PublicField"])
var s: String = "test"
}
""".trimIndent(), "PublicField")
@@ -83,7 +83,7 @@ class KotlinSuppressionAnnotationInspectionTest : SuppressionAnnotationInspectio
fun `test quickfix - allow a single suppression from annotation when explicit attribute name exists`() {
testAllowSuppressionQuickFix(JvmLanguage.KOTLIN, """
class A {
@Suppress(names = "Public<caret>Field")
@Supp<caret>ress(names = "PublicField")
var s: String = "test"
}
""".trimIndent(), "PublicField")
@@ -92,7 +92,7 @@ class KotlinSuppressionAnnotationInspectionTest : SuppressionAnnotationInspectio
fun `test quickfix - allow multiple suppressions from annotation`() {
testAllowSuppressionQuickFix(JvmLanguage.KOTLIN, """
class A {
@Suppress("Public<caret>Field", "HardCodedStringLiteral")
@Supp<caret>ress("PublicField", "HardCodedStringLiteral")
var s: String = "test"
}
""".trimIndent(), "PublicField", "HardCodedStringLiteral")
@@ -101,7 +101,7 @@ class KotlinSuppressionAnnotationInspectionTest : SuppressionAnnotationInspectio
fun `test quickfix - allow multiple suppressions from annotation when array form used`() {
testAllowSuppressionQuickFix(JvmLanguage.KOTLIN, """
class A {
@Suppress(["Public<caret>Field", "HardCodedStringLiteral"])
@Supp<caret>ress(["PublicField", "HardCodedStringLiteral"])
var s: String = "test"
}
""".trimIndent(), "PublicField", "HardCodedStringLiteral")
@@ -110,7 +110,7 @@ class KotlinSuppressionAnnotationInspectionTest : SuppressionAnnotationInspectio
fun `test quickfix - allow multiple suppressions from annotation when explicit attribute name exists`() {
testAllowSuppressionQuickFix(JvmLanguage.KOTLIN, """
class A {
@Suppress(names = ["Public<caret>Field", "HardCodedStringLiteral"])
@Supp<caret>ress(names = ["PublicField", "HardCodedStringLiteral"])
var s: String = "test"
}
""".trimIndent(), "PublicField", "HardCodedStringLiteral")
@@ -124,7 +124,7 @@ class KotlinSuppressionAnnotationInspectionTest : SuppressionAnnotationInspectio
}
class A {
@Suppress([Constants.PUBLIC_<caret>FIELD, Constants.HARD_CODED_STRING_LITERAL])
@Supp<caret>ress([Constants.PUBLIC_FIELD, Constants.HARD_CODED_STRING_LITERAL])
var s: String = "test"
}
""".trimIndent(), "PublicField", "HardCodedStringLiteral")