From 1ce4075112d29f5bfe8522d3f7279402bdea50b2 Mon Sep 17 00:00:00 2001 From: Ilya Chernikov Date: Mon, 4 Oct 2021 12:22:03 +0200 Subject: [PATCH] Prepare CLI reporting infrastructure for non-PSI diagnostics --- .../messages/CompilerMessageLocation.kt | 4 +- .../FirDiagnosticsCompilerResultsReporter.kt | 144 ++++++++++++++++-- .../cli/common/messages/MessageUtil.java | 14 +- .../FirKotlinToJvmBytecodeCompiler.kt | 11 +- .../jetbrains/kotlin/fir/pipeline/analyse.kt | 15 +- .../impl/BaseDiagnosticReporter.kt | 1 + .../impl/DeduplicatingDiagnosticReporter.kt | 4 +- .../impl/DiagnosticReporterWithSuppress.kt | 8 +- .../impl/SimpleDiagnosticReporter.kt | 8 +- .../diagnostics/PsiDiagnosticUtils.java | 1 + 10 files changed, 182 insertions(+), 28 deletions(-) diff --git a/compiler/cli/cli-common/src/org/jetbrains/kotlin/cli/common/messages/CompilerMessageLocation.kt b/compiler/cli/cli-common/src/org/jetbrains/kotlin/cli/common/messages/CompilerMessageLocation.kt index fd5c2ff3355..6763f1ed874 100644 --- a/compiler/cli/cli-common/src/org/jetbrains/kotlin/cli/common/messages/CompilerMessageLocation.kt +++ b/compiler/cli/cli-common/src/org/jetbrains/kotlin/cli/common/messages/CompilerMessageLocation.kt @@ -22,9 +22,11 @@ interface CompilerMessageSourceLocation : Serializable { val path: String val line: Int val column: Int + // NOTE: Seems that the end-of-location data do not belong here conceptually, and now causes confusion with other usages + // TODO: consider removing it and switching REPL/Scripting diagnostis to the higher-level entities (KtDiagnostics) val lineEnd: Int get() = -1 val columnEnd: Int get() = -1 - val lineContent: String? + val lineContent: String? // related to the (start) line/column only, used to show start position in the console output } data class CompilerMessageLocation private constructor( diff --git a/compiler/cli/src/org/jetbrains/kotlin/cli/common/fir/FirDiagnosticsCompilerResultsReporter.kt b/compiler/cli/src/org/jetbrains/kotlin/cli/common/fir/FirDiagnosticsCompilerResultsReporter.kt index d038e35eaa9..0c4d8a47c34 100644 --- a/compiler/cli/src/org/jetbrains/kotlin/cli/common/fir/FirDiagnosticsCompilerResultsReporter.kt +++ b/compiler/cli/src/org/jetbrains/kotlin/cli/common/fir/FirDiagnosticsCompilerResultsReporter.kt @@ -12,10 +12,12 @@ import org.jetbrains.kotlin.cli.common.messages.AnalyzerWithCompilerReport import org.jetbrains.kotlin.cli.common.messages.CompilerMessageSourceLocation import org.jetbrains.kotlin.cli.common.messages.MessageCollector import org.jetbrains.kotlin.cli.common.messages.MessageUtil -import org.jetbrains.kotlin.diagnostics.DiagnosticUtils -import org.jetbrains.kotlin.diagnostics.KtDiagnostic -import org.jetbrains.kotlin.diagnostics.Severity +import org.jetbrains.kotlin.diagnostics.* +import org.jetbrains.kotlin.diagnostics.impl.BaseDiagnosticReporter import org.jetbrains.kotlin.fir.analysis.diagnostics.FirDefaultErrorMessages +import java.io.Closeable +import java.io.File +import java.io.InputStreamReader object FirDiagnosticsCompilerResultsReporter { fun reportDiagnostics(diagnostics: Collection, reporter: MessageCollector): Boolean { @@ -27,6 +29,38 @@ object FirDiagnosticsCompilerResultsReporter { return hasErrors } + fun reportToCollector(diagnosticsReporter: BaseDiagnosticReporter, messageCollector: MessageCollector): Boolean { + var hasErrors = false + for (filePath in diagnosticsReporter.diagnosticsByFilePath.keys) { + for (diagnostic in diagnosticsReporter.diagnosticsByFilePath[filePath].orEmpty().sortedWith(SingleFileDiagnosticComparator)) { + var filePositionFinder: SequentialFilePositionFinder? = null + try { + when { + diagnostic is KtPsiDiagnostic -> diagnostic.element.location(diagnostic) + else -> { + if (filePositionFinder == null) { + filePositionFinder = SequentialFilePositionFinder(filePath) + } + // NOTE: SequentialPositionFinder relies on the ascending order of the input offsets, so the code relies + // on the the appropriate sorting above + // Also the end offset is ignored, as it is irrelevant for the CLI reporting + val position = + filePositionFinder.findNextPosition(DiagnosticUtils.firstRange(diagnostic.textRanges).startOffset) + MessageUtil.createMessageLocation(filePath, position.lineContent, position.line, position.column, -1, -1) + } + }?.let { location -> + reportDiagnostic(diagnostic, location, messageCollector) + hasErrors = hasErrors || diagnostic.severity == Severity.ERROR + } + } finally { + filePositionFinder?.close() + } + } + } +// reportSpecialErrors(diagnostics) + return hasErrors + } + @Suppress("UNUSED_PARAMETER") private fun reportSpecialErrors(diagnostics: Collection) { /* @@ -40,15 +74,23 @@ object FirDiagnosticsCompilerResultsReporter { private fun reportDiagnostic(diagnostic: KtDiagnostic, reporter: MessageCollector): Boolean { if (!diagnostic.isValid) return false - diagnostic.location()?.let { location -> - val severity = AnalyzerWithCompilerReport.convertSeverity(diagnostic.severity) - // TODO: support multiple maps with messages - val renderer = FirDefaultErrorMessages.getRendererForDiagnostic(diagnostic) - reporter.report(severity, renderer.render(diagnostic), location) + diagnostic.location()?.let { + reportDiagnostic(diagnostic, it, reporter) } return diagnostic.severity == Severity.ERROR } + private fun reportDiagnostic( + diagnostic: KtDiagnostic, + location: CompilerMessageSourceLocation, + reporter: MessageCollector + ) { + val severity = AnalyzerWithCompilerReport.convertSeverity(diagnostic.severity) + // TODO: support multiple maps with messages + val renderer = FirDefaultErrorMessages.getRendererForDiagnostic(diagnostic) + reporter.report(severity, renderer.render(diagnostic), location) + } + private fun KtDiagnostic.location(): CompilerMessageSourceLocation? = when (val element = element) { is KtPsiSourceElement -> element.location(this) is KtLightSourceElement -> element.location(this) @@ -76,7 +118,7 @@ object FirDiagnosticsCompilerResultsReporter { override fun compare(o1: KtDiagnostic, o2: KtDiagnostic): Int { val element1 = o1.element val element2 = o1.element - // TODO: support light tree + // TODO: support light tree (likely obsolete, processing LT files in other code path) if (element1 !is KtPsiSourceElement || element2 !is KtPsiSourceElement) return 0 val file1 = element1.psi.containingFile @@ -93,4 +135,88 @@ object FirDiagnosticsCompilerResultsReporter { } else o1.factory.name.compareTo(o2.factory.name) } } + + private object SingleFileDiagnosticComparator : Comparator { + override fun compare(o1: KtDiagnostic, o2: KtDiagnostic): Int { + val range1 = DiagnosticUtils.firstRange(o1.textRanges) + val range2 = DiagnosticUtils.firstRange(o2.textRanges) + + return if (range1 != range2) { + DiagnosticUtils.TEXT_RANGE_COMPARATOR.compare(range1, range2) + } else o1.factory.name.compareTo(o2.factory.name) + } + } + + class SequentialFilePositionFinder(private val filePath: String?) : Closeable { + + // TODO: verify if returning NONE on invalid files is the desired behavior (instead of throwing IOError) + private var reader: InputStreamReader? = filePath?.let(::File)?.takeIf { it.isFile }?.reader(/* TODO: select proper charset */) + + private var currentLineContent: String? = null + private val buffer = CharArray(255) + private var bufLength = -1 + private var bufPos = 0 + private var endOfStream = false + private var skipNextLf = false + + private var charsRead = 0 + private var currentLine = 0 + + // assuming that if called multiple times, calls should be sorted by ascending offset + @Synchronized + fun findNextPosition(offset: Int, withLineContents: Boolean = true): PsiDiagnosticUtils.LineAndColumn { + if (offset < 0 || reader == null) return PsiDiagnosticUtils.LineAndColumn.NONE + + assert(offset >= charsRead) + + while (true) { + if (currentLineContent == null) { + currentLineContent = readNextLine() + } + + val col = offset - (charsRead - currentLineContent!!.length - 1)/* beginning of line offset */ + 1 /* col is 1-based */ + if (col <= currentLineContent!!.length) { + return PsiDiagnosticUtils.LineAndColumn(currentLine, col, if (withLineContents) currentLineContent else null) + } + + if (endOfStream) return PsiDiagnosticUtils.LineAndColumn.NONE + + currentLineContent = null + } + } + + private fun readNextLine() = buildString { + while (true) { + if (bufPos >= bufLength) { + bufLength = reader!!.read(buffer) + bufPos = 0 + if (bufLength < 0) { + endOfStream = true + break + } + } else { + val c = buffer[bufPos++] + charsRead++ + when { + c == '\n' && skipNextLf -> { + skipNextLf = false + } + c == '\n' || c == '\r' -> { + currentLine++ + skipNextLf = c == '\r' + break + } + else -> { + append(c) + skipNextLf = false + } + } + } + } + } + + override fun close() { + reader?.close() + } + } } diff --git a/compiler/cli/src/org/jetbrains/kotlin/cli/common/messages/MessageUtil.java b/compiler/cli/src/org/jetbrains/kotlin/cli/common/messages/MessageUtil.java index 7fc813fab34..db63eed450d 100644 --- a/compiler/cli/src/org/jetbrains/kotlin/cli/common/messages/MessageUtil.java +++ b/compiler/cli/src/org/jetbrains/kotlin/cli/common/messages/MessageUtil.java @@ -46,7 +46,19 @@ public class MessageUtil { String path = virtualFile != null ? virtualFileToPath(virtualFile) : defaultValue; PsiDiagnosticUtils.LineAndColumn start = range.getStart(); PsiDiagnosticUtils.LineAndColumn end = range.getEnd(); - return CompilerMessageLocationWithRange.create(path, start.getLine(), start.getColumn(), end.getLine(), end.getColumn(), start.getLineContent()); + return createMessageLocation(path, start.getLineContent(), start.getLine(), start.getColumn(), end.getLine(), end.getColumn()); + } + + @Nullable + public static CompilerMessageLocationWithRange createMessageLocation( + @Nullable String path, + @Nullable String lineContent, + int line, + int column, + int endLine, + int endColumn + ) { + return CompilerMessageLocationWithRange.create(path, line, column, endLine, endColumn, lineContent); } @NotNull diff --git a/compiler/cli/src/org/jetbrains/kotlin/cli/jvm/compiler/FirKotlinToJvmBytecodeCompiler.kt b/compiler/cli/src/org/jetbrains/kotlin/cli/jvm/compiler/FirKotlinToJvmBytecodeCompiler.kt index 81cad4bda92..abef519fc33 100644 --- a/compiler/cli/src/org/jetbrains/kotlin/cli/jvm/compiler/FirKotlinToJvmBytecodeCompiler.kt +++ b/compiler/cli/src/org/jetbrains/kotlin/cli/jvm/compiler/FirKotlinToJvmBytecodeCompiler.kt @@ -23,8 +23,8 @@ import org.jetbrains.kotlin.codegen.ClassBuilderFactories import org.jetbrains.kotlin.codegen.CodegenFactory import org.jetbrains.kotlin.codegen.state.GenerationState import org.jetbrains.kotlin.config.* +import org.jetbrains.kotlin.diagnostics.DiagnosticReporterFactory import org.jetbrains.kotlin.fir.* -import org.jetbrains.kotlin.diagnostics.KtDiagnostic import org.jetbrains.kotlin.fir.backend.Fir2IrResult import org.jetbrains.kotlin.fir.backend.jvm.FirJvmBackendClassResolver import org.jetbrains.kotlin.fir.backend.jvm.FirJvmBackendExtension @@ -60,7 +60,6 @@ import org.jetbrains.kotlin.psi.KtFile import org.jetbrains.kotlin.resolve.PlatformDependentAnalyzerServices import org.jetbrains.kotlin.resolve.jvm.platform.JvmPlatformAnalyzerServices import org.jetbrains.kotlin.resolve.multiplatform.isCommonSource -import org.jetbrains.kotlin.utils.addToStdlib.flattenTo import org.jetbrains.kotlin.utils.addToStdlib.runIf import org.jetbrains.kotlin.utils.newLinkedHashMapWithExpectedSize import java.io.File @@ -244,16 +243,16 @@ object FirKotlinToJvmBytecodeCompiler { val commonRawFir = commonSession?.buildFirFromKtFiles(commonKtFiles) val rawFir = session.buildFirFromKtFiles(ktFiles) - val allKtDiagnostics = mutableListOf() + val diagnosticsReporter = DiagnosticReporterFactory.createReporter() commonSession?.apply { val (commonScopeSession, commonFir) = runResolution(commonRawFir!!) - runCheckers(commonScopeSession, commonFir).values.flattenTo(allKtDiagnostics) + runCheckers(commonScopeSession, commonFir, diagnosticsReporter) } val (scopeSession, fir) = session.runResolution(rawFir) - session.runCheckers(scopeSession, fir).values.flattenTo(allKtDiagnostics) + session.runCheckers(scopeSession, fir, diagnosticsReporter) - val hasErrors = FirDiagnosticsCompilerResultsReporter.reportDiagnostics(allKtDiagnostics, messageCollector) + val hasErrors = FirDiagnosticsCompilerResultsReporter.reportToCollector(diagnosticsReporter, messageCollector) return if (syntaxErrors || hasErrors) null else FirResult(session, scopeSession, fir) } diff --git a/compiler/fir/entrypoint/src/org/jetbrains/kotlin/fir/pipeline/analyse.kt b/compiler/fir/entrypoint/src/org/jetbrains/kotlin/fir/pipeline/analyse.kt index 577f8d4b538..6368e561513 100644 --- a/compiler/fir/entrypoint/src/org/jetbrains/kotlin/fir/pipeline/analyse.kt +++ b/compiler/fir/entrypoint/src/org/jetbrains/kotlin/fir/pipeline/analyse.kt @@ -5,10 +5,11 @@ package org.jetbrains.kotlin.fir.pipeline -import org.jetbrains.kotlin.fir.FirSession -import org.jetbrains.kotlin.fir.analysis.collectors.FirDiagnosticsCollector +import org.jetbrains.kotlin.diagnostics.DiagnosticReporter import org.jetbrains.kotlin.diagnostics.DiagnosticReporterFactory import org.jetbrains.kotlin.diagnostics.KtDiagnostic +import org.jetbrains.kotlin.fir.FirSession +import org.jetbrains.kotlin.fir.analysis.collectors.FirDiagnosticsCollector import org.jetbrains.kotlin.fir.declarations.FirFile import org.jetbrains.kotlin.fir.resolve.ScopeSession import org.jetbrains.kotlin.fir.resolve.transformers.FirTotalResolveProcessor @@ -29,4 +30,12 @@ fun FirSession.runCheckers(scopeSession: ScopeSession, firFiles: List): put(file, reporter.diagnostics) } } -} \ No newline at end of file +} + +@OptIn(ExperimentalStdlibApi::class) +fun FirSession.runCheckers(scopeSession: ScopeSession, firFiles: List, reporter: DiagnosticReporter) { + val collector = FirDiagnosticsCollector.create(this, scopeSession) + for (file in firFiles) { + collector.collectDiagnostics(file, reporter) + } +} diff --git a/compiler/frontend.common/src/org/jetbrains/kotlin/diagnostics/impl/BaseDiagnosticReporter.kt b/compiler/frontend.common/src/org/jetbrains/kotlin/diagnostics/impl/BaseDiagnosticReporter.kt index add20d046d4..b64d70a5a5a 100644 --- a/compiler/frontend.common/src/org/jetbrains/kotlin/diagnostics/impl/BaseDiagnosticReporter.kt +++ b/compiler/frontend.common/src/org/jetbrains/kotlin/diagnostics/impl/BaseDiagnosticReporter.kt @@ -10,4 +10,5 @@ import org.jetbrains.kotlin.diagnostics.KtDiagnostic abstract class BaseDiagnosticReporter : DiagnosticReporter() { abstract val diagnostics: List + abstract val diagnosticsByFilePath: Map> } diff --git a/compiler/frontend.common/src/org/jetbrains/kotlin/diagnostics/impl/DeduplicatingDiagnosticReporter.kt b/compiler/frontend.common/src/org/jetbrains/kotlin/diagnostics/impl/DeduplicatingDiagnosticReporter.kt index 2a679d61ff7..ce00d4ca8b3 100644 --- a/compiler/frontend.common/src/org/jetbrains/kotlin/diagnostics/impl/DeduplicatingDiagnosticReporter.kt +++ b/compiler/frontend.common/src/org/jetbrains/kotlin/diagnostics/impl/DeduplicatingDiagnosticReporter.kt @@ -13,10 +13,10 @@ import org.jetbrains.kotlin.diagnostics.KtDiagnostic class DeduplicatingDiagnosticReporter(private val inner: DiagnosticReporter) : DiagnosticReporter() { - private val reported = mutableSetOf>() + private val reported = mutableSetOf>() override fun report(diagnostic: KtDiagnostic?, context: DiagnosticContext) { - if (diagnostic != null && reported.add(Pair(diagnostic.element, diagnostic.factory))) { + if (diagnostic != null && reported.add(Triple(context.containingFilePath, diagnostic.element, diagnostic.factory))) { inner.report(diagnostic, context) } } diff --git a/compiler/frontend.common/src/org/jetbrains/kotlin/diagnostics/impl/DiagnosticReporterWithSuppress.kt b/compiler/frontend.common/src/org/jetbrains/kotlin/diagnostics/impl/DiagnosticReporterWithSuppress.kt index fa0da3cb108..25b3e1f214f 100644 --- a/compiler/frontend.common/src/org/jetbrains/kotlin/diagnostics/impl/DiagnosticReporterWithSuppress.kt +++ b/compiler/frontend.common/src/org/jetbrains/kotlin/diagnostics/impl/DiagnosticReporterWithSuppress.kt @@ -9,14 +9,16 @@ import org.jetbrains.kotlin.diagnostics.DiagnosticContext import org.jetbrains.kotlin.diagnostics.KtDiagnostic class DiagnosticReporterWithSuppress : BaseDiagnosticReporter() { - private val _diagnostics: MutableList = mutableListOf() + private val _diagnosticsByFilePath: MutableMap> = mutableMapOf() override val diagnostics: List - get() = _diagnostics + get() = _diagnosticsByFilePath.flatMap { it.value } + override val diagnosticsByFilePath: Map> + get() = _diagnosticsByFilePath override fun report(diagnostic: KtDiagnostic?, context: DiagnosticContext) { if (diagnostic == null) return if (!context.isDiagnosticSuppressed(diagnostic)) { - _diagnostics += diagnostic + _diagnosticsByFilePath.getOrPut(context.containingFilePath) { mutableListOf() }.add(diagnostic) } } } diff --git a/compiler/frontend.common/src/org/jetbrains/kotlin/diagnostics/impl/SimpleDiagnosticReporter.kt b/compiler/frontend.common/src/org/jetbrains/kotlin/diagnostics/impl/SimpleDiagnosticReporter.kt index 2495510bda7..55b3a25fe8b 100644 --- a/compiler/frontend.common/src/org/jetbrains/kotlin/diagnostics/impl/SimpleDiagnosticReporter.kt +++ b/compiler/frontend.common/src/org/jetbrains/kotlin/diagnostics/impl/SimpleDiagnosticReporter.kt @@ -9,12 +9,14 @@ import org.jetbrains.kotlin.diagnostics.DiagnosticContext import org.jetbrains.kotlin.diagnostics.KtDiagnostic class SimpleDiagnosticReporter : BaseDiagnosticReporter() { - private val _diagnostics: MutableList = mutableListOf() + private val _diagnosticsByFilePath: MutableMap> = mutableMapOf() override val diagnostics: List - get() = _diagnostics + get() = _diagnosticsByFilePath.flatMap { it.value } + override val diagnosticsByFilePath: Map> + get() = _diagnosticsByFilePath override fun report(diagnostic: KtDiagnostic?, context: DiagnosticContext) { if (diagnostic == null) return - _diagnostics += diagnostic + _diagnosticsByFilePath.getOrPut(context.containingFilePath) { mutableListOf() }.add(diagnostic) } } diff --git a/compiler/psi/src/org/jetbrains/kotlin/diagnostics/PsiDiagnosticUtils.java b/compiler/psi/src/org/jetbrains/kotlin/diagnostics/PsiDiagnosticUtils.java index 683c7474fba..2285e3bfe15 100644 --- a/compiler/psi/src/org/jetbrains/kotlin/diagnostics/PsiDiagnosticUtils.java +++ b/compiler/psi/src/org/jetbrains/kotlin/diagnostics/PsiDiagnosticUtils.java @@ -17,6 +17,7 @@ import org.jetbrains.annotations.Nullable; import org.jetbrains.kotlin.psi.KtExpression; import org.jetbrains.kotlin.psi.psiUtil.PsiUtilsKt; +// TODO: extract PSI-independent parts, specifically coordinate classes public class PsiDiagnosticUtils { public static String atLocation(@NotNull PsiElement element) { if (element.isValid()) {