Skip to content

Commit 0498c1a

Browse files
alessio-peruginialessio-perugini
committed
introduce diagnosticmanager instead of using a callback approach
1 parent e37587b commit 0498c1a

File tree

8 files changed

+98
-65
lines changed

8 files changed

+98
-65
lines changed

‎internal/arduino/builder/builder.go

+12-45
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,11 @@ import (
2121
"io"
2222
"os"
2323
"path/filepath"
24-
"slices"
2524
"strings"
2625

2726
"github.com/arduino/arduino-cli/internal/arduino/builder/internal/compilation"
2827
"github.com/arduino/arduino-cli/internal/arduino/builder/internal/detector"
28+
"github.com/arduino/arduino-cli/internal/arduino/builder/internal/diagnosticmanager"
2929
"github.com/arduino/arduino-cli/internal/arduino/builder/internal/diagnostics"
3030
"github.com/arduino/arduino-cli/internal/arduino/builder/internal/logger"
3131
"github.com/arduino/arduino-cli/internal/arduino/builder/internal/progress"
@@ -37,7 +37,6 @@ import (
3737
rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
3838
"github.com/arduino/go-paths-helper"
3939
"github.com/arduino/go-properties-orderedmap"
40-
"github.com/sirupsen/logrus"
4140
)
4241

4342
// ErrSketchCannotBeLocatedInBuildPath fixdoc
@@ -95,11 +94,7 @@ type Builder struct {
9594

9695
toolEnv []string
9796

98-
// This is a function used to parse the output of the compiler
99-
// It is used to extract errors and warnings
100-
compilerOutputParser diagnostics.CompilerOutputParserCB
101-
// and here are the diagnostics parsed from the compiler
102-
compilerDiagnostics diagnostics.Diagnostics
97+
diagnosticsManager *diagnosticmanager.Manager
10398
}
10499

105100
// buildArtifacts contains the result of various build
@@ -200,6 +195,7 @@ func NewBuilder(
200195
logger.Warn(string(verboseOut))
201196
}
202197

198+
diagnosticmanager := diagnosticmanager.New()
203199
b := &Builder{
204200
sketch: sk,
205201
buildProperties: buildProperties,
@@ -232,33 +228,15 @@ func NewBuilder(
232228
buildProperties.GetPath("runtime.platform.path"),
233229
buildProperties.GetPath("build.core.path"), // TODO can we buildCorePath ?
234230
),
231+
diagnosticsManager: diagnosticmanager,
232+
libsDetector: detector.NewSketchLibrariesDetector(
233+
libsManager, libsResolver,
234+
useCachedLibrariesResolution,
235+
onlyUpdateCompilationDatabase,
236+
logger,
237+
diagnosticmanager,
238+
),
235239
}
236-
237-
b.compilerOutputParser = func(cmdline []string, out []byte) {
238-
compiler := diagnostics.DetectCompilerFromCommandLine(
239-
cmdline,
240-
false, // at the moment compiler-probing is not required
241-
)
242-
if compiler == nil {
243-
logrus.Warnf("Could not detect compiler from: %s", cmdline)
244-
return
245-
}
246-
diags, err := diagnostics.ParseCompilerOutput(compiler, out)
247-
if err != nil {
248-
logrus.Warnf("Error parsing compiler output: %s", err)
249-
return
250-
}
251-
b.compilerDiagnostics = append(b.compilerDiagnostics, diags...)
252-
}
253-
254-
b.libsDetector = detector.NewSketchLibrariesDetector(
255-
libsManager, libsResolver,
256-
useCachedLibrariesResolution,
257-
onlyUpdateCompilationDatabase,
258-
logger,
259-
b.compilerOutputParser,
260-
)
261-
262240
return b, nil
263241
}
264242

@@ -284,18 +262,7 @@ func (b *Builder) ImportedLibraries() libraries.List {
284262

285263
// CompilerDiagnostics returns the parsed compiler diagnostics
286264
func (b *Builder) CompilerDiagnostics() diagnostics.Diagnostics {
287-
// When producing the preprocessing diagnostics, we might have duplicates
288-
// caused by the run of the libraries detector and the sketch preprocessing.
289-
return slices.CompactFunc(b.compilerDiagnostics, func(d1, d2 *diagnostics.Diagnostic) bool {
290-
if d1 == nil || d2 == nil {
291-
return false
292-
}
293-
return d1.Column == d2.Column &&
294-
d1.File == d2.File &&
295-
d1.Line == d2.Line &&
296-
d1.Message == d2.Message &&
297-
d1.Severity == d2.Severity
298-
})
265+
return b.diagnosticsManager.CompilerDiagnostics()
299266
}
300267

301268
// Preprocess fixdoc

‎internal/arduino/builder/compilation.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -166,9 +166,9 @@ func (b *Builder) compileFileWithRecipe(
166166
b.logger.WriteStderr(commandStderr.Bytes())
167167

168168
// Parse the output of the compiler to gather errors and warnings...
169-
if b.compilerOutputParser != nil {
170-
b.compilerOutputParser(command.GetArgs(), commandStdout.Bytes())
171-
b.compilerOutputParser(command.GetArgs(), commandStderr.Bytes())
169+
if b.diagnosticsManager != nil {
170+
b.diagnosticsManager.ParseOutput(command.GetArgs(), commandStdout.Bytes())
171+
b.diagnosticsManager.ParseOutput(command.GetArgs(), commandStderr.Bytes())
172172
}
173173

174174
// ...and then return the error

‎internal/arduino/builder/internal/detector/detector.go

+7-6
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import (
2626
"strings"
2727
"time"
2828

29-
"github.com/arduino/arduino-cli/internal/arduino/builder/internal/diagnostics"
29+
"github.com/arduino/arduino-cli/internal/arduino/builder/internal/diagnosticmanager"
3030
"github.com/arduino/arduino-cli/internal/arduino/builder/internal/logger"
3131
"github.com/arduino/arduino-cli/internal/arduino/builder/internal/preprocessor"
3232
"github.com/arduino/arduino-cli/internal/arduino/builder/internal/utils"
@@ -58,7 +58,7 @@ type SketchLibrariesDetector struct {
5858
librariesResolutionResults map[string]libraryResolutionResult
5959
includeFolders paths.PathList
6060
logger *logger.BuilderLogger
61-
compilerOutputParser diagnostics.CompilerOutputParserCB
61+
diagnosticManager *diagnosticmanager.Manager
6262
}
6363

6464
// NewSketchLibrariesDetector todo
@@ -68,7 +68,7 @@ func NewSketchLibrariesDetector(
6868
useCachedLibrariesResolution bool,
6969
onlyUpdateCompilationDatabase bool,
7070
logger *logger.BuilderLogger,
71-
compilerOutputParser diagnostics.CompilerOutputParserCB,
71+
diagnosticManager *diagnosticmanager.Manager,
7272
) *SketchLibrariesDetector {
7373
return &SketchLibrariesDetector{
7474
librariesManager: lm,
@@ -79,7 +79,7 @@ func NewSketchLibrariesDetector(
7979
includeFolders: paths.PathList{},
8080
onlyUpdateCompilationDatabase: onlyUpdateCompilationDatabase,
8181
logger: logger,
82-
compilerOutputParser: compilerOutputParser,
82+
diagnosticManager: diagnosticManager,
8383
}
8484
}
8585

@@ -351,7 +351,7 @@ func (l *SketchLibrariesDetector) findIncludesUntilDone(
351351
}
352352
} else {
353353
var preprocStdout []byte
354-
preprocStdout, preprocStderr, preprocErr = preprocessor.GCC(sourcePath, targetFilePath, includeFolders, buildProperties, nil)
354+
preprocStdout, preprocStderr, preprocErr = preprocessor.GCC(sourcePath, targetFilePath, includeFolders, buildProperties, l.diagnosticManager)
355355
if l.logger.Verbose() {
356356
l.logger.WriteStdout(preprocStdout)
357357
}
@@ -383,7 +383,7 @@ func (l *SketchLibrariesDetector) findIncludesUntilDone(
383383
if preprocErr == nil || preprocStderr == nil {
384384
// Filename came from cache, so run preprocessor to obtain error to show
385385
var preprocStdout []byte
386-
preprocStdout, preprocStderr, preprocErr = preprocessor.GCC(sourcePath, targetFilePath, includeFolders, buildProperties, l.compilerOutputParser)
386+
preprocStdout, preprocStderr, preprocErr = preprocessor.GCC(sourcePath, targetFilePath, includeFolders, buildProperties, l.diagnosticManager)
387387
if l.logger.Verbose() {
388388
l.logger.WriteStdout(preprocStdout)
389389
}
@@ -417,6 +417,7 @@ func (l *SketchLibrariesDetector) findIncludesUntilDone(
417417
}
418418
}
419419
first = false
420+
l.diagnosticManager.DiscardLastInsert()
420421
}
421422
}
422423

‎internal/arduino/builder/internal/diagnosticmanager/diagnosticmanager.go

+65
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
package diagnosticmanager
2+
3+
import (
4+
"fmt"
5+
6+
"github.com/arduino/arduino-cli/internal/arduino/builder/internal/diagnostics"
7+
"github.com/sirupsen/logrus"
8+
)
9+
10+
type Manager struct {
11+
results diagnostics.Diagnostics
12+
lastInsertIndex int
13+
}
14+
15+
func New() *Manager {
16+
return &Manager{lastInsertIndex: -1}
17+
}
18+
19+
func (m *Manager) ParseOutput(cmdline []string, out []byte) {
20+
compiler := diagnostics.DetectCompilerFromCommandLine(
21+
cmdline,
22+
false, // at the moment compiler-probing is not required
23+
)
24+
if compiler == nil {
25+
logrus.Warnf("Could not detect compiler from: %s", cmdline)
26+
return
27+
}
28+
diags, err := diagnostics.ParseCompilerOutput(compiler, out)
29+
if err != nil {
30+
logrus.Warnf("Error parsing compiler output: %s", err)
31+
return
32+
}
33+
m.lastInsertIndex += len(diags)
34+
m.results = append(m.results, diags...)
35+
}
36+
37+
func (m *Manager) DiscardLastInsert() {
38+
if m.lastInsertIndex < 0 {
39+
return
40+
}
41+
m.results = m.results[:m.lastInsertIndex]
42+
m.lastInsertIndex = len(m.results) - 1
43+
}
44+
45+
func (m *Manager) CompilerDiagnostics() diagnostics.Diagnostics {
46+
// When producing the preprocessing diagnostics, we might have duplicates
47+
// caused by the run of the libraries detector and the sketch preprocessing.
48+
if len(m.results) <= 1 {
49+
return m.results
50+
}
51+
mp := map[string]*diagnostics.Diagnostic{}
52+
keys := []string{}
53+
for _, v := range m.results {
54+
key := fmt.Sprintf("%s:%d:%d:%s", v.File, v.Line, v.Column, v.Severity)
55+
if _, ok := mp[key]; !ok {
56+
mp[key] = v
57+
keys = append(keys, key)
58+
}
59+
}
60+
s := make(diagnostics.Diagnostics, len(mp))
61+
for i, k := range keys {
62+
s[i] = mp[k]
63+
}
64+
return s
65+
}

‎internal/arduino/builder/internal/preprocessor/arduino_preprocessor.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import (
2222
"path/filepath"
2323
"runtime"
2424

25-
"github.com/arduino/arduino-cli/internal/arduino/builder/internal/diagnostics"
25+
"github.com/arduino/arduino-cli/internal/arduino/builder/internal/diagnosticmanager"
2626
"github.com/arduino/arduino-cli/internal/arduino/builder/internal/utils"
2727
"github.com/arduino/arduino-cli/internal/arduino/sketch"
2828
"github.com/arduino/go-paths-helper"
@@ -34,7 +34,7 @@ import (
3434
func PreprocessSketchWithArduinoPreprocessor(
3535
sk *sketch.Sketch, buildPath *paths.Path, includeFolders paths.PathList,
3636
lineOffset int, buildProperties *properties.Map, onlyUpdateCompilationDatabase bool,
37-
compilerOutputParserCB diagnostics.CompilerOutputParserCB,
37+
diagnosticmanager *diagnosticmanager.Manager,
3838
) ([]byte, []byte, error) {
3939
verboseOut := &bytes.Buffer{}
4040
normalOut := &bytes.Buffer{}
@@ -44,7 +44,7 @@ func PreprocessSketchWithArduinoPreprocessor(
4444

4545
sourceFile := buildPath.Join("sketch", sk.MainFile.Base()+".cpp")
4646
targetFile := buildPath.Join("preproc", "sketch_merged.cpp")
47-
gccStdout, gccStderr, err := GCC(sourceFile, targetFile, includeFolders, buildProperties, compilerOutputParserCB)
47+
gccStdout, gccStderr, err := GCC(sourceFile, targetFile, includeFolders, buildProperties, diagnosticmanager)
4848
verboseOut.Write(gccStdout)
4949
verboseOut.Write(gccStderr)
5050
if err != nil {

‎internal/arduino/builder/internal/preprocessor/ctags.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import (
2626
"strings"
2727

2828
"github.com/arduino/arduino-cli/internal/arduino/builder/cpp"
29-
"github.com/arduino/arduino-cli/internal/arduino/builder/internal/diagnostics"
29+
"github.com/arduino/arduino-cli/internal/arduino/builder/internal/diagnosticmanager"
3030
"github.com/arduino/arduino-cli/internal/arduino/builder/internal/preprocessor/internal/ctags"
3131
"github.com/arduino/arduino-cli/internal/arduino/sketch"
3232
"github.com/arduino/arduino-cli/internal/i18n"
@@ -44,7 +44,7 @@ var DebugPreprocessor bool
4444
func PreprocessSketchWithCtags(
4545
sketch *sketch.Sketch, buildPath *paths.Path, includes paths.PathList,
4646
lineOffset int, buildProperties *properties.Map, onlyUpdateCompilationDatabase bool,
47-
compilerOutputParserCB diagnostics.CompilerOutputParserCB,
47+
diagnosticManager *diagnosticmanager.Manager,
4848
) ([]byte, []byte, error) {
4949
// Create a temporary working directory
5050
tmpDir, err := paths.MkTempDir("", "")
@@ -59,7 +59,7 @@ func PreprocessSketchWithCtags(
5959

6060
// Run GCC preprocessor
6161
sourceFile := buildPath.Join("sketch", sketch.MainFile.Base()+".cpp")
62-
gccStdout, gccStderr, err := GCC(sourceFile, ctagsTarget, includes, buildProperties, compilerOutputParserCB)
62+
gccStdout, gccStderr, err := GCC(sourceFile, ctagsTarget, includes, buildProperties, diagnosticManager)
6363
verboseOutput.Write(gccStdout)
6464
verboseOutput.Write(gccStderr)
6565
normalOutput.Write(gccStderr)

‎internal/arduino/builder/internal/preprocessor/gcc.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import (
2323

2424
f "github.com/arduino/arduino-cli/internal/algorithms"
2525
"github.com/arduino/arduino-cli/internal/arduino/builder/cpp"
26-
"github.com/arduino/arduino-cli/internal/arduino/builder/internal/diagnostics"
26+
"github.com/arduino/arduino-cli/internal/arduino/builder/internal/diagnosticmanager"
2727
"github.com/arduino/go-paths-helper"
2828
"github.com/arduino/go-properties-orderedmap"
2929
)
@@ -33,7 +33,7 @@ import (
3333
func GCC(
3434
sourceFilePath, targetFilePath *paths.Path,
3535
includes paths.PathList, buildProperties *properties.Map,
36-
compilerOutputParserCB diagnostics.CompilerOutputParserCB,
36+
diagnosticManager *diagnosticmanager.Manager,
3737
) ([]byte, []byte, error) {
3838
gccBuildProperties := properties.NewMap()
3939
gccBuildProperties.Set("preproc.macros.flags", "-w -x c++ -E -CC")
@@ -79,8 +79,8 @@ func GCC(
7979
}
8080
stdout, stderr, err := proc.RunAndCaptureOutput(context.Background())
8181

82-
if compilerOutputParserCB != nil {
83-
compilerOutputParserCB(proc.GetArgs(), stderr)
82+
if diagnosticManager != nil {
83+
diagnosticManager.ParseOutput(proc.GetArgs(), stderr)
8484
}
8585

8686
// Append gcc arguments to stdout

‎internal/arduino/builder/preprocess_sketch.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ func (b *Builder) preprocessSketch(includes paths.PathList) error {
2626
normalOutput, verboseOutput, err := preprocessor.PreprocessSketchWithCtags(
2727
b.sketch, b.buildPath, includes, b.lineOffset,
2828
b.buildProperties, b.onlyUpdateCompilationDatabase,
29-
b.compilerOutputParser,
29+
b.diagnosticsManager,
3030
)
3131
if b.logger.Verbose() {
3232
b.logger.WriteStdout(verboseOutput)

0 commit comments

Comments
 (0)