Improved diagnostic rendering #42236

RadCod3 · 2024-02-28T05:09:30Z

Purpose

The purpose of this PR is to introduce an enhanced diagnostic rendering feature to the Ballerina compiler, aligning it with the standards set by the Rustc compiler. We do this by providing users with more informative and user-friendly error messages.
This addresses task 1 of #42124.

Approach

I've created a separate class to use Diagnostic objects and output a better error message. This is to maintain compatibility with anything else that depends on the current diagnostic implementations. Furthermore, I've used a bumped up version of jline to access a library called jansi which was merged into jline, for cross platform supported ANSI rendering.
Information about what token is indeed missing was passed using the properties() function in SyntaxDiagnostic.java.

Samples

Missing tokens and keywords

These type of errors are rendered as below,

The + symbol represents that it should be newly added.

Invalid tokens

Invalid tokens are highlighted in red in addition to being pointed by a ^ symbol.

Other errors

Other errors in general are underlined by ^ symbols.

Multi-lined diagnostics are rendered as below. If it's more than 2 lines, the lines in between are hidden.

Remarks

For filtering certain syntax errors and semantic errors I've used BCE error codes given by the DiagnosticAPI. Changing those in the future will break the current implementation.
The @|...|@ used in the code are the syntax used by jansi to render the correct ANSI escape codes. See jansi/AnsiRenderer.
While working on this I found this improvement, [Improvement]: Combine redundant compiler diagnostic error codes #42202
Encountered this bug, [Bug]: Issue with running new tests in ballerina-cli #42339 while writing tests. Used a workaround for now.

Check List

CLAassistant · 2024-02-28T05:09:36Z

All committers have signed the CLA.

cli/ballerina-cli/src/main/java/io/ballerina/cli/task/CompileTask.java

cli/ballerina-cli/src/main/java/io/ballerina/cli/utils/AnnotateDiagnostics.java

...arser/src/main/java/io/ballerina/compiler/internal/diagnostics/StringDiagnosticProperty.java

cli/ballerina-cli/src/main/java/io/ballerina/cli/utils/AnnotateDiagnostics.java

cli/ballerina-cli/src/main/java/io/ballerina/cli/task/CompileTask.java

cli/ballerina-cli/src/main/java/io/ballerina/cli/utils/AnnotateDiagnostics.java

cli/ballerina-cli/src/main/java/io/ballerina/cli/utils/DiagnosticAnnotation.java

codecov · 2024-03-05T10:11:08Z

Codecov Report

Attention: Patch coverage is 92.42424% with 15 lines in your changes missing coverage. Please review.

Project coverage is 76.60%. Comparing base (148286d) to head (876501f).
Report is 459 commits behind head on master.

❗ Current head 876501f differs from pull request most recent head 12e70f6

Please upload reports for the commit 12e70f6 to get more accurate results.

Files	Patch %	Lines
...ballerina/cli/diagnostics/AnnotateDiagnostics.java	89.88%	3 Missing and 6 partials ⚠️
...internal/diagnostics/StringDiagnosticProperty.java	60.00%	4 Missing ⚠️
...c/main/java/io/ballerina/cli/task/CompileTask.java	77.77%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #42236      +/-   ##
============================================
- Coverage     76.80%   76.60%   -0.20%     
+ Complexity    53972    53574     -398     
============================================
  Files          2924     2908      -16     
  Lines        203952   202367    -1585     
  Branches      26597    26354     -243     
============================================
- Hits         156638   155028    -1610     
- Misses        38777    38846      +69     
+ Partials       8537     8493      -44

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

...st/resources/test-resources/diagnostics-test-files/bal-project-error/project1/Ballerina.toml

...src/test/resources/test-resources/diagnostics-test-files/bal-project-error/project2/main.bal

cli/ballerina-cli/src/main/java/io/ballerina/cli/diagnostics/AnnotateDiagnostics.java

cli/ballerina-cli/src/test/java/io/ballerina/cli/diagnostics/AnnotateDiagnosticsTest.java

Shifted from string concatenation to string builders. Combined some common login in DiagnosticAnnotation for when lines are hidden and not hidden in the multi-line diagnostic case. Also made the amount of lines to be shown before we start hiding lines configurable.

github-actions · 2024-06-08T19:33:09Z

This PR has been open for more than 15 days with no activity. This will be closed in 3 days unless the stale label is removed or commented.

github-actions · 2024-06-24T19:33:19Z

This PR has been open for more than 15 days with no activity. This will be closed in 3 days unless the stale label is removed or commented.

Shadow-Devil

Fix checkstyle

Shadow-Devil · 2024-06-25T14:03:27Z

cli/ballerina-cli/src/main/java/io/ballerina/cli/diagnostics/DiagnosticAnnotation.java

+
+    /**
+     * Represents a result of truncating a line.
+     *


Suggested change

*

* @param needsWrap If the result needs to be wrapped

Shadow-Devil · 2024-06-25T14:07:26Z

cli/ballerina-cli/src/main/java/io/ballerina/cli/diagnostics/DiagnosticAnnotation.java

+            }
+            else {
+


Suggested change

}

else {

} else {

github-actions · 2024-07-10T19:33:13Z

This PR has been open for more than 15 days with no activity. This will be closed in 3 days unless the stale label is removed or commented.

github-actions · 2024-07-13T19:33:37Z

Closed PR due to inactivity for more than 18 days.

SasinduDilshara · 2024-09-06T08:21:58Z

cli/ballerina-cli/src/test/resources/test-resources/diagnostics-test-files/bal-error/tabs.bal

+	if (a < 0) {
+		return 0;
+	} else (b < 0) {
+		return 0;
+	}


Suggested change

if (a < 0) {

return 0;

} else (b < 0) {

return 0;

}

if (a < 0 || b < 0) {

return 0;

}

SasinduDilshara · 2024-09-06T08:25:19Z

cli/ballerina-cli/src/main/java/io/ballerina/cli/diagnostics/DiagnosticAnnotation.java

+    private static final String INTERNAL_COLOR = "blue";
+    private static final String HINT_COLOR = "green";
+    private static final String INFO_COLOR = "blue";
+    private static final String WARNING_COLOR = "yellow";


another option to use orange

SasinduDilshara · 2024-09-06T08:26:16Z

cli/ballerina-cli/src/main/java/io/ballerina/cli/diagnostics/DiagnosticAnnotation.java

+    private static int countTabChars(String line, int end) {
+        int count = 0;
+        for (int i = 0; i < end; i++) {
+            if (line.charAt(i) == '\t') {


Can define this as a constant and use

RadCod3 mentioned this pull request Feb 28, 2024

[Improvement]: Improve compiler diagnostics #42124

Open

RadCod3 force-pushed the improved-diagnostic-rendering branch from bc654cc to 4fa5982 Compare February 28, 2024 05:57

poorna2152 reviewed Feb 28, 2024

View reviewed changes

cli/ballerina-cli/src/main/java/io/ballerina/cli/task/CompileTask.java Outdated Show resolved Hide resolved

SasinduDilshara reviewed Feb 28, 2024

View reviewed changes

cli/ballerina-cli/src/main/java/io/ballerina/cli/task/CompileTask.java Outdated Show resolved Hide resolved

poorna2152 reviewed Feb 28, 2024

View reviewed changes

cli/ballerina-cli/src/main/java/io/ballerina/cli/task/CompileTask.java Outdated Show resolved Hide resolved

poorna2152 reviewed Feb 28, 2024

View reviewed changes