Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve compiler's diagnostic messages #6931

Merged
merged 21 commits into from
Jun 8, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 65 additions & 25 deletions engine/runtime/src/main/scala/org/enso/compiler/Compiler.scala
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package org.enso.compiler

import com.oracle.truffle.api.TruffleLogger
import com.oracle.truffle.api.source.Source
import com.oracle.truffle.api.source.{Source}
Akirathan marked this conversation as resolved.
Show resolved Hide resolved
import org.enso.compiler.codegen.{IrToTruffle, RuntimeStubsGenerator}
import org.enso.compiler.context.{FreshNameSupply, InlineContext, ModuleContext}
import org.enso.compiler.core.IR
Expand Down Expand Up @@ -34,7 +34,6 @@ import java.util.concurrent.{
TimeUnit
}
import java.util.logging.Level

import scala.jdk.OptionConverters._
import java.util.concurrent.Future

Expand Down Expand Up @@ -985,25 +984,8 @@ class Compiler(
diagnostics: List[IR.Diagnostic],
source: Source
): Boolean = {
val errors = diagnostics.collect { case e: IR.Error => e }
val warnings = diagnostics.collect { case w: IR.Warning => w }

if (warnings.nonEmpty) {
output.println("Compiler encountered warnings:")
warnings.foreach { warning =>
output.println(formatDiagnostic(warning, source))
}
}

if (errors.nonEmpty) {
output.println("Compiler encountered errors:")
errors.foreach { error =>
output.println(formatDiagnostic(error, source))
}
true
} else {
false
}
diagnostics.foreach(diag => output.println(formatDiagnostic(diag, source)))
diagnostics.exists(_.isInstanceOf[IR.Error])
}

/** Pretty prints compiler diagnostics.
Expand All @@ -1016,10 +998,68 @@ class Compiler(
diagnostic: IR.Diagnostic,
source: Source
): String = {
fileLocationFromSection(
diagnostic.location,
source
) + ": " + diagnostic.formattedMessage
val outSupportsAnsiColors = System.console() != null
val ansiReset = "\u001B[0m"
Akirathan marked this conversation as resolved.
Show resolved Hide resolved
val ansiRed = "\u001B[31m"
val ansiYellow = "\u001B[33m"
val ansiBold = "\u001B[1m"

def yellowBold(text: String) =
if (outSupportsAnsiColors) ansiYellow + ansiBold + text + ansiReset
else text
def redBold(text: String) =
if (outSupportsAnsiColors) ansiRed + ansiBold + text + ansiReset else text
def bold(text: String) =
if (outSupportsAnsiColors) ansiBold + text + ansiReset else text

val (coloredFunc, subject) = diagnostic match {
case _: IR.Error => (redBold _, "error: ")
case _: IR.Warning => (yellowBold _, "warning: ")
case _ => throw new IllegalStateException("Unexpected diagnostic type")
}

diagnostic.location match {
case Some(location) =>
val section = source.createSection(location.start, location.length)
val isOneLine = section.getStartLine == section.getEndLine
if (isOneLine) {
val lineNumber = section.getStartLine
val startColumn = section.getStartColumn
val endColumn = section.getEndColumn
val sectionLen = endColumn - startColumn
val line = source.createSection(lineNumber).getCharacters.toString
val sb = new StringBuilder()
sb
.append(
bold(
source.getName + ":" + lineNumber + ":" + startColumn + "-" + endColumn + ": "
)
)
.append(coloredFunc(subject))
.append(diagnostic.formattedMessage)
.append("\n")
sb
.append(line.substring(0, startColumn - 1))
.append(coloredFunc(line.substring(startColumn - 1, endColumn)))
.append(line.substring(endColumn))
.append("\n")
sb
.append(" " * (startColumn - 1))
.append(coloredFunc("^" + "~" * sectionLen))
sb.toString
} else {
bold(fileLocationFromSection(diagnostic.location, source)) +
": " +
coloredFunc(subject) +
diagnostic.formattedMessage
}
case None =>
// We dont have location information, so we just print the message
bold(fileLocationFromSection(diagnostic.location, source)) +
Akirathan marked this conversation as resolved.
Show resolved Hide resolved
": " +
coloredFunc(subject)
diagnostic.formattedMessage
}
}

private def fileLocationFromSection(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class ImportsTest extends PackageTest {
"Import statements" should "report errors when they cannot be resolved" in {
the[InterpreterException] thrownBy evalTestProject(
"Test_Bad_Imports"
) should have message "Compilation aborted due to errors."
) should have message "Aborting due to"
val outLines = consumeOut
outLines(2) should include(
"Package containing the module Surely_This.Does_Not_Exist.My_Module " +
Expand Down Expand Up @@ -124,7 +124,7 @@ class ImportsTest extends PackageTest {
"TestSubmodulesNameConflict"
) should have message "Method `c_mod_method` of type C.type could not be found."
val outLines = consumeOut
outLines(2) should include
outLines(1) should include
"Declaration of type C shadows module local.TestSubmodulesNameConflict.A.B.C making it inaccessible via a qualified name."
}

Expand Down Expand Up @@ -160,10 +160,10 @@ class ImportsTest extends PackageTest {
"Test_Polyglot_Exports"
) should have message "Compilation aborted due to errors."
val outLines = consumeOut
outLines should have length 3
outLines should have length 4
outLines(
2
) shouldEqual "Main.enso[5:16-5:19]: The name `Long` could not be found."
1
) shouldEqual "Main.enso:5:16-19: error: The name `Long` could not be found."
}

"Constructors" should "be importable" in {
Expand All @@ -180,10 +180,10 @@ class ImportsTest extends PackageTest {
) should have message "Compilation aborted due to errors."

val outLines = consumeOut
outLines should have length 3
outLines should have length 4
outLines(
2
) shouldEqual "Main.enso[2:14-2:17]: Fully qualified name references a library Standard.Base but an import statement for it is missing."
) shouldEqual "Main.enso:2:14-17: error: Fully qualified name references a library Standard.Base but an import statement for it is missing."
}

"Fully qualified names" should "be resolved when library has already been loaded" in {
Expand All @@ -200,10 +200,10 @@ class ImportsTest extends PackageTest {
"Test_Fully_Qualified_Name_Conflict"
) should have message "Method `Foo` of type Atom.type could not be found."
val outLines = consumeOut
outLines should have length 3
outLines should have length 4
outLines(
2
) shouldEqual "Main.enso[2:1-2:57]: The exported type `Atom` in `local.Test_Fully_Qualified_Name_Conflict.Atom` module will cause name conflict when attempting to use a fully qualified name of the `local.Test_Fully_Qualified_Name_Conflict.Atom.Foo` module."
1
) shouldEqual "Main.enso:2:1-57: error: The exported type `Atom` in `local.Test_Fully_Qualified_Name_Conflict.Atom` module will cause name conflict when attempting to use a fully qualified name of the `local.Test_Fully_Qualified_Name_Conflict.Atom.Foo` module."
}

"Deeply nested modules" should "infer correct synthetic modules" in {
Expand Down