Report compiler errors in machine readable format

[cmaglie]

Please check if the PR fulfills these requirements

See how to contribute

• The PR has no duplicates (please search among the Pull Requests
  before creating one)
• The PR follows
  our contributing guidelines
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)
• UPGRADING.md has been updated with a migration guide (for breaking changes)
• configuration.schema.json updated if new parameters are added.

What kind of change does this PR introduce?

Adds parsing of the compiler errors and warning to present them in a machine-readable way if JSON output is selected or if the Compile gRPC call is used.

What is the current behavior?

The compiler output is presented as a big blob of text.

What is the new behavior?

The compiler output is dissected into a structured format so clients can easily consume it.

$ arduino-cli compile -u -b arduino:avr:leonardo ~/Arduino/Blink/ -v --format json |jq .builder_result.diagnostics
[
  {
    "column": 16,
    "context": [
      {
        "file": "/home/megabug/Workspace/sketchbook-cores-beta/libraries/Audio/src/Audio.h",
        "line": 16,
        "message": "included from here"
      },
      {
        "file": "/home/megabug/Arduino/Blink/Blink.ino",
        "line": 1,
        "message": "included from here"
      }
    ],
    "file": "/home/megabug/Workspace/sketchbook-cores-beta/libraries/Audio/src/DAC.h",
    "line": 21,
    "message": "expected ')' before '*' token\n  DACClass(Dacc *_dac, uint32_t _dacId, IRQn_Type _isrId) :\n                ^",
    "severity": "ERROR"
  },
  {
    "column": 2,
    "context": [
      {
        "file": "/home/megabug/Workspace/sketchbook-cores-beta/libraries/Audio/src/Audio.h",
        "line": 16,
        "message": "included from here"
      },
      {
        "file": "/home/megabug/Arduino/Blink/Blink.ino",
        "line": 1,
        "message": "included from here"
      }
    ],
    "file": "/home/megabug/Workspace/sketchbook-cores-beta/libraries/Audio/src/DAC.h",
    "line": 35,
    "message": "'Dacc' does not name a type\n  Dacc *dac;\n  ^~~~",
    "severity": "ERROR"
  },
  ...

Does this PR introduce a breaking change, and is titled accordingly?

No

Other information

Fix #1121

[codecov]

Codecov Report

Attention: 34 lines in your changes are missing coverage. Please review.

Comparison is base (c45ab20) 64.80% compared to head (df376b4) 65.19%.
Report is 3 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2182      +/-   ##
==========================================
+ Coverage   64.80%   65.19%   +0.39%     
==========================================
  Files         207      210       +3     
  Lines       20153    20468     +315     
==========================================
+ Hits        13060    13344     +284     
- Misses       5978     6003      +25     
- Partials     1115     1121       +6     

Flag	Coverage Δ
unit	65.19% <89.30%> (+0.39%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
arduino/builder/compilation.go	85.60% <100.00%> (+0.47%)	⬆️
commands/compile/compile.go	67.12% <100.00%> (+0.34%)	⬆️
internal/cli/compile/compile.go	73.49% <100.00%> (+0.08%)	⬆️
internal/cli/feedback/result/rpc.go	79.71% <100.00%> (+1.03%)	⬆️
arduino/builder/internal/diagnostics/parser_gcc.go	97.31% <97.31%> (ø)
arduino/builder/builder.go	65.36% <61.90%> (-0.42%)	⬇️
...builder/internal/diagnostics/compiler_detection.go	73.80% <73.80%> (ø)
arduino/builder/internal/diagnostics/parser.go	84.28% <84.28%> (ø)

... and 1 file with indirect coverage changes

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

[cmaglie]

TODO: test this with more compilers...

[alessio-perugini]

LGTM. The only think that I'm concerned about is that starting from now on this is something new that we need to maintain so here are some ideas for future PRs:

• Add more tests for different versions of compilers we're using.
• Adding fuzz testing in the parser might be helpful?

The implementation has been rebased...

[cmaglie]

Add more tests for different versions of compilers we're using.

This will surely help. Adding a test is a tedious process tho...

Adding fuzz testing in the parser might be helpful?

Interesting, BTW I think that the fuzzer may not help to improve the parsing correctness because:

• we have no way to check if the input produced by the fuzzer is a plausible gcc output
• we have no way to check if the parser output is correct against the given fuzzer input

Anyway, it may help to test the parser's resistance to crazy inputs...

[alessio-perugini]

I'd like to have an integration test that checks for the new diagnostic field.
If you want you can either:

• Expand the already existing integration test internal/integrationtest/compile_3/compile_test.go|102 col 6| func TestCompilerErrOutput(t *testing.T) {
• Create a new one

Example:

TestCompilerErrOutput(t *testing.T) {
	...
	// prepare sketch
	sketch, err := paths.New("testdata", "blink_with_wrong_cpp").Abs()
	require.NoError(t, err)

	// Run compile and catch err stream
	out, _, err := cli.Run("compile", "-b", "arduino:avr:uno", "--format", "json", sketch.String())
	...
	requirejson.Query(t, out, `.diagnostics | length > 0`, `true`)