From c9cb2062bd9a4ad383136fb34362be7870f8284e Mon Sep 17 00:00:00 2001 From: Marco Pennekamp Date: Tue, 7 Nov 2023 16:58:47 +0100 Subject: [PATCH] [LL] Commit pending diagnostics from file checkers on top-level declarations - In KT-62899, diagnostics produced by file checkers weren't suppressed because the top-level declarations on which the diagnostics are reported were never visited by the same reporter. `LLFirDiagnosticVisitor` skips nested declarations by design, as diagnostics are collected for each structure element separately. However, suppression takes effect when diagnostics are committed on some element. If the element is not visited, suppression is skipped and the diagnostic is committed at the end without a check by `FileStructureElementDiagnosticsCollector.collectForStructureElement`. - The fix ensures that diagnostics are committed on top-level declarations, similar to how this was already done for class members. ^KT-62899 fixed --- .../conflictingOverloadsAtTopLevel.kt | 2 - .../conflictingOverloadsAtTopLevel.txt | 12 +++--- .../fir/diagnostics/LLFirDiagnosticVisitor.kt | 38 ++++++++++++++----- 3 files changed, 34 insertions(+), 18 deletions(-) diff --git a/analysis/analysis-api/testData/components/diagnosticsProvider/diagnostics/suppression/conflictingOverloadsAtTopLevel.kt b/analysis/analysis-api/testData/components/diagnosticsProvider/diagnostics/suppression/conflictingOverloadsAtTopLevel.kt index d1d78c59cde..83156e19465 100644 --- a/analysis/analysis-api/testData/components/diagnosticsProvider/diagnostics/suppression/conflictingOverloadsAtTopLevel.kt +++ b/analysis/analysis-api/testData/components/diagnosticsProvider/diagnostics/suppression/conflictingOverloadsAtTopLevel.kt @@ -1,5 +1,3 @@ -// IGNORE_FIR - class SomeClass @Suppress("CONFLICTING_OVERLOADS") diff --git a/analysis/analysis-api/testData/components/diagnosticsProvider/diagnostics/suppression/conflictingOverloadsAtTopLevel.txt b/analysis/analysis-api/testData/components/diagnosticsProvider/diagnostics/suppression/conflictingOverloadsAtTopLevel.txt index e86e623ada7..2cde519ca7b 100644 --- a/analysis/analysis-api/testData/components/diagnosticsProvider/diagnostics/suppression/conflictingOverloadsAtTopLevel.txt +++ b/analysis/analysis-api/testData/components/diagnosticsProvider/diagnostics/suppression/conflictingOverloadsAtTopLevel.txt @@ -1,7 +1,7 @@ Diagnostics from elements: - for PSI element of type KtStringTemplateExpression at (10,11-34) - ERROR_SUPPRESSION text ranges: [(130,153)] - PSI: KtStringTemplateExpression at (10,11-34) - for PSI element of type KtStringTemplateExpression at (5,11-34) - ERROR_SUPPRESSION text ranges: [(42,65)] - PSI: KtStringTemplateExpression at (5,11-34) + for PSI element of type KtStringTemplateExpression at (3,11-34) + ERROR_SUPPRESSION text ranges: [(27,50)] + PSI: KtStringTemplateExpression at (3,11-34) + for PSI element of type KtStringTemplateExpression at (8,11-34) + ERROR_SUPPRESSION text ranges: [(115,138)] + PSI: KtStringTemplateExpression at (8,11-34) diff --git a/analysis/low-level-api-fir/src/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostics/LLFirDiagnosticVisitor.kt b/analysis/low-level-api-fir/src/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostics/LLFirDiagnosticVisitor.kt index d516480110b..12d8fee545b 100644 --- a/analysis/low-level-api-fir/src/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostics/LLFirDiagnosticVisitor.kt +++ b/analysis/low-level-api-fir/src/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostics/LLFirDiagnosticVisitor.kt @@ -16,9 +16,9 @@ import org.jetbrains.kotlin.fir.analysis.collectors.CheckerRunningDiagnosticColl import org.jetbrains.kotlin.fir.analysis.collectors.DiagnosticCollectorComponents import org.jetbrains.kotlin.fir.declarations.FirCodeFragment import org.jetbrains.kotlin.fir.declarations.FirDeclaration +import org.jetbrains.kotlin.fir.declarations.FirFile import org.jetbrains.kotlin.fir.declarations.FirRegularClass import org.jetbrains.kotlin.fir.psi -import org.jetbrains.kotlin.psi import org.jetbrains.kotlin.psi.KtCodeFragment import org.jetbrains.kotlin.psi.KtDeclaration @@ -48,9 +48,7 @@ internal open class LLFirDiagnosticVisitor( checkCanceled() element.accept(components.reportCommitter, context) - if (element is FirRegularClass) { - suppressReportedDiagnosticsOnClassMembers(element) - } + commitPendingDiagnosticsOnNestedDeclarations(element) } override fun visitCodeFragment(codeFragment: FirCodeFragment, data: Nothing?) { @@ -84,13 +82,33 @@ internal open class LLFirDiagnosticVisitor( } /** - * Some FirClassChecker may report diagnostics on class member headers. - * That diagnostics should be suppressed if we have a `@Suppress` annotation on class member. + * File and class checkers may report diagnostics on top-level declarations and class members, such as conflicting overload errors. + * Because we are collecting diagnostics for each structure element separately, this visitor will not visit these nested declarations by + * default, as the file/class and its nested declarations are different structure elements. Instead, all diagnostics produced during the + * visitor run will be committed at the end (see [FileStructureElementDiagnosticsCollector.collectForStructureElement]). + * + * Skipping nested declarations circumvents error suppression with `@Suppress` on top-level declarations and class members. This is + * because suppression usually works as such: When a diagnostic is first reported on an element `E`, it is "pending". Once element `E` + * is visited by the diagnostic visitor, it commits all pending diagnostics for `E`, including those reported by a file/class checker. + * Diagnostics which are suppressed in the current context are instead removed. Without committing pending diagnostics on each element + * `E`, suppression cannot take effect. + * + * [commitPendingDiagnosticsOnNestedDeclarations] commits pending diagnostics for directly nested elements, allowing the report + * committer to take suppression into account. + * + * It suffices to commit pending diagnostics for directly nested declarations, because checkers can only report diagnostics on directly + * accessible children. For example, a file checker can report a diagnostic on a top-level class, but not its member function. */ - private fun suppressReportedDiagnosticsOnClassMembers(element: FirRegularClass) { - for (member in element.declarations) { - withAnnotationContainer(member) { - member.accept(components.reportCommitter, context) + private fun commitPendingDiagnosticsOnNestedDeclarations(element: FirElement) { + val declarations = when (element) { + is FirFile -> element.declarations + is FirRegularClass -> element.declarations + else -> return + } + + for (declaration in declarations) { + withAnnotationContainer(declaration) { + declaration.accept(components.reportCommitter, context) } } }