[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
This commit is contained in:
Marco Pennekamp
2023-11-07 16:58:47 +01:00
committed by Space Team
parent 88d307d52c
commit c9cb2062bd
3 changed files with 34 additions and 18 deletions
@@ -1,5 +1,3 @@
// IGNORE_FIR
class SomeClass
@Suppress("CONFLICTING_OVERLOADS")
@@ -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)
@@ -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)
}
}
}