introduce diagnosticmanager instead of using a callback approach

Author: alessio-perugini

Diff for: internal/arduino/builder/builder.go

@@ -21,11 +21,11 @@ import (
 	"io"
 	"os"
 	"path/filepath"
-	"slices"
 	"strings"
 
 	"github.com/arduino/arduino-cli/internal/arduino/builder/internal/compilation"
 	"github.com/arduino/arduino-cli/internal/arduino/builder/internal/detector"
+	"github.com/arduino/arduino-cli/internal/arduino/builder/internal/diagnosticmanager"
 	"github.com/arduino/arduino-cli/internal/arduino/builder/internal/diagnostics"
 	"github.com/arduino/arduino-cli/internal/arduino/builder/internal/logger"
 	"github.com/arduino/arduino-cli/internal/arduino/builder/internal/progress"
@@ -37,7 +37,6 @@ import (
 	rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
 	"github.com/arduino/go-paths-helper"
 	"github.com/arduino/go-properties-orderedmap"
-	"github.com/sirupsen/logrus"
 )
 
 // ErrSketchCannotBeLocatedInBuildPath fixdoc
@@ -95,11 +94,7 @@ type Builder struct {
 
 	toolEnv []string
 
-	// This is a function used to parse the output of the compiler
-	// It is used to extract errors and warnings
-	compilerOutputParser diagnostics.CompilerOutputParserCB
-	// and here are the diagnostics parsed from the compiler
-	compilerDiagnostics diagnostics.Diagnostics
+	diagnosticsManager *diagnosticmanager.Manager
 }
 
 // buildArtifacts contains the result of various build
@@ -200,6 +195,7 @@ func NewBuilder(
 		logger.Warn(string(verboseOut))
 	}
 
+	diagnosticmanager := diagnosticmanager.New()
 	b := &Builder{
 		sketch:                        sk,
 		buildProperties:               buildProperties,
@@ -232,33 +228,15 @@ func NewBuilder(
 			buildProperties.GetPath("runtime.platform.path"),
 			buildProperties.GetPath("build.core.path"), // TODO can we buildCorePath ?
 		),
+		diagnosticsManager: diagnosticmanager,
+		libsDetector: detector.NewSketchLibrariesDetector(
+			libsManager, libsResolver,
+			useCachedLibrariesResolution,
+			onlyUpdateCompilationDatabase,
+			logger,
+			diagnosticmanager,
+		),
 	}
-
-	b.compilerOutputParser = func(cmdline []string, out []byte) {
-		compiler := diagnostics.DetectCompilerFromCommandLine(
-			cmdline,
-			false, // at the moment compiler-probing is not required
-		)
-		if compiler == nil {
-			logrus.Warnf("Could not detect compiler from: %s", cmdline)
-			return
-		}
-		diags, err := diagnostics.ParseCompilerOutput(compiler, out)
-		if err != nil {
-			logrus.Warnf("Error parsing compiler output: %s", err)
-			return
-		}
-		b.compilerDiagnostics = append(b.compilerDiagnostics, diags...)
-	}
-
-	b.libsDetector = detector.NewSketchLibrariesDetector(
-		libsManager, libsResolver,
-		useCachedLibrariesResolution,
-		onlyUpdateCompilationDatabase,
-		logger,
-		b.compilerOutputParser,
-	)
-
 	return b, nil
 }
 
@@ -284,18 +262,7 @@ func (b *Builder) ImportedLibraries() libraries.List {
 
 // CompilerDiagnostics returns the parsed compiler diagnostics
 func (b *Builder) CompilerDiagnostics() diagnostics.Diagnostics {
-	// When producing the preprocessing diagnostics, we might have duplicates
-	// caused by the run of the libraries detector and the sketch preprocessing.
-	return slices.CompactFunc(b.compilerDiagnostics, func(d1, d2 *diagnostics.Diagnostic) bool {
-		if d1 == nil || d2 == nil {
-			return false
-		}
-		return d1.Column == d2.Column &&
-			d1.File == d2.File &&
-			d1.Line == d2.Line &&
-			d1.Message == d2.Message &&
-			d1.Severity == d2.Severity
-	})
+	return b.diagnosticsManager.CompilerDiagnostics()
 }
 
 // Preprocess fixdoc

Diff for: internal/arduino/builder/compilation.go

@@ -166,9 +166,9 @@ func (b *Builder) compileFileWithRecipe(
 		b.logger.WriteStderr(commandStderr.Bytes())
 
 		// Parse the output of the compiler to gather errors and warnings...
-		if b.compilerOutputParser != nil {
-			b.compilerOutputParser(command.GetArgs(), commandStdout.Bytes())
-			b.compilerOutputParser(command.GetArgs(), commandStderr.Bytes())
+		if b.diagnosticsManager != nil {
+			b.diagnosticsManager.ParseOutput(command.GetArgs(), commandStdout.Bytes())
+			b.diagnosticsManager.ParseOutput(command.GetArgs(), commandStderr.Bytes())
 		}
 
 		// ...and then return the error

Diff for: internal/arduino/builder/internal/detector/detector.go

@@ -26,7 +26,7 @@ import (
 	"strings"
 	"time"
 
-	"github.com/arduino/arduino-cli/internal/arduino/builder/internal/diagnostics"
+	"github.com/arduino/arduino-cli/internal/arduino/builder/internal/diagnosticmanager"
 	"github.com/arduino/arduino-cli/internal/arduino/builder/internal/logger"
 	"github.com/arduino/arduino-cli/internal/arduino/builder/internal/preprocessor"
 	"github.com/arduino/arduino-cli/internal/arduino/builder/internal/utils"
@@ -58,7 +58,7 @@ type SketchLibrariesDetector struct {
 	librariesResolutionResults    map[string]libraryResolutionResult
 	includeFolders                paths.PathList
 	logger                        *logger.BuilderLogger
-	compilerOutputParser          diagnostics.CompilerOutputParserCB
+	diagnosticManager             *diagnosticmanager.Manager
 }
 
 // NewSketchLibrariesDetector todo
@@ -68,7 +68,7 @@ func NewSketchLibrariesDetector(
 	useCachedLibrariesResolution bool,
 	onlyUpdateCompilationDatabase bool,
 	logger *logger.BuilderLogger,
-	compilerOutputParser diagnostics.CompilerOutputParserCB,
+	diagnosticManager *diagnosticmanager.Manager,
 ) *SketchLibrariesDetector {
 	return &SketchLibrariesDetector{
 		librariesManager:              lm,
@@ -79,7 +79,7 @@ func NewSketchLibrariesDetector(
 		includeFolders:                paths.PathList{},
 		onlyUpdateCompilationDatabase: onlyUpdateCompilationDatabase,
 		logger:                        logger,
-		compilerOutputParser:          compilerOutputParser,
+		diagnosticManager:             diagnosticManager,
 	}
 }
 
@@ -351,7 +351,7 @@ func (l *SketchLibrariesDetector) findIncludesUntilDone(
 			}
 		} else {
 			var preprocStdout []byte
-			preprocStdout, preprocStderr, preprocErr = preprocessor.GCC(sourcePath, targetFilePath, includeFolders, buildProperties, nil)
+			preprocStdout, preprocStderr, preprocErr = preprocessor.GCC(sourcePath, targetFilePath, includeFolders, buildProperties, l.diagnosticManager)
 			if l.logger.Verbose() {
 				l.logger.WriteStdout(preprocStdout)
 			}
@@ -383,7 +383,7 @@ func (l *SketchLibrariesDetector) findIncludesUntilDone(
 			if preprocErr == nil || preprocStderr == nil {
 				// Filename came from cache, so run preprocessor to obtain error to show
 				var preprocStdout []byte
-				preprocStdout, preprocStderr, preprocErr = preprocessor.GCC(sourcePath, targetFilePath, includeFolders, buildProperties, l.compilerOutputParser)
+				preprocStdout, preprocStderr, preprocErr = preprocessor.GCC(sourcePath, targetFilePath, includeFolders, buildProperties, l.diagnosticManager)
 				if l.logger.Verbose() {
 					l.logger.WriteStdout(preprocStdout)
 				}
@@ -417,6 +417,7 @@ func (l *SketchLibrariesDetector) findIncludesUntilDone(
 			}
 		}
 		first = false
+		l.diagnosticManager.DiscardLastInsert()
 	}
 }
 

Diff for: internal/arduino/builder/internal/diagnosticmanager/diagnosticmanager.go

@@ -0,0 +1,65 @@
+package diagnosticmanager
+
+import (
+	"fmt"
+
+	"github.com/arduino/arduino-cli/internal/arduino/builder/internal/diagnostics"
+	"github.com/sirupsen/logrus"
+)
+
+type Manager struct {
+	results         diagnostics.Diagnostics
+	lastInsertIndex int
+}
+
+func New() *Manager {
+	return &Manager{lastInsertIndex: -1}
+}
+
+func (m *Manager) ParseOutput(cmdline []string, out []byte) {
+	compiler := diagnostics.DetectCompilerFromCommandLine(
+		cmdline,
+		false, // at the moment compiler-probing is not required
+	)
+	if compiler == nil {
+		logrus.Warnf("Could not detect compiler from: %s", cmdline)
+		return
+	}
+	diags, err := diagnostics.ParseCompilerOutput(compiler, out)
+	if err != nil {
+		logrus.Warnf("Error parsing compiler output: %s", err)
+		return
+	}
+	m.lastInsertIndex += len(diags)
+	m.results = append(m.results, diags...)
+}
+
+func (m *Manager) DiscardLastInsert() {
+	if m.lastInsertIndex < 0 {
+		return
+	}
+	m.results = m.results[:m.lastInsertIndex]
+	m.lastInsertIndex = len(m.results) - 1
+}
+
+func (m *Manager) CompilerDiagnostics() diagnostics.Diagnostics {
+	// When producing the preprocessing diagnostics, we might have duplicates
+	// caused by the run of the libraries detector and the sketch preprocessing.
+	if len(m.results) <= 1 {
+		return m.results
+	}
+	mp := map[string]*diagnostics.Diagnostic{}
+	keys := []string{}
+	for _, v := range m.results {
+		key := fmt.Sprintf("%s:%d:%d:%s", v.File, v.Line, v.Column, v.Severity)
+		if _, ok := mp[key]; !ok {
+			mp[key] = v
+			keys = append(keys, key)
+		}
+	}
+	s := make(diagnostics.Diagnostics, len(mp))
+	for i, k := range keys {
+		s[i] = mp[k]
+	}
+	return s
+}

Diff for: internal/arduino/builder/internal/preprocessor/arduino_preprocessor.go

@@ -22,7 +22,7 @@ import (
 	"path/filepath"
 	"runtime"
 
-	"github.com/arduino/arduino-cli/internal/arduino/builder/internal/diagnostics"
+	"github.com/arduino/arduino-cli/internal/arduino/builder/internal/diagnosticmanager"
 	"github.com/arduino/arduino-cli/internal/arduino/builder/internal/utils"
 	"github.com/arduino/arduino-cli/internal/arduino/sketch"
 	"github.com/arduino/go-paths-helper"
@@ -34,7 +34,7 @@ import (
 func PreprocessSketchWithArduinoPreprocessor(
 	sk *sketch.Sketch, buildPath *paths.Path, includeFolders paths.PathList,
 	lineOffset int, buildProperties *properties.Map, onlyUpdateCompilationDatabase bool,
-	compilerOutputParserCB diagnostics.CompilerOutputParserCB,
+	diagnosticmanager *diagnosticmanager.Manager,
 ) ([]byte, []byte, error) {
 	verboseOut := &bytes.Buffer{}
 	normalOut := &bytes.Buffer{}
@@ -44,7 +44,7 @@ func PreprocessSketchWithArduinoPreprocessor(
 
 	sourceFile := buildPath.Join("sketch", sk.MainFile.Base()+".cpp")
 	targetFile := buildPath.Join("preproc", "sketch_merged.cpp")
-	gccStdout, gccStderr, err := GCC(sourceFile, targetFile, includeFolders, buildProperties, compilerOutputParserCB)
+	gccStdout, gccStderr, err := GCC(sourceFile, targetFile, includeFolders, buildProperties, diagnosticmanager)
 	verboseOut.Write(gccStdout)
 	verboseOut.Write(gccStderr)
 	if err != nil {

Diff for: internal/arduino/builder/internal/preprocessor/ctags.go

@@ -26,7 +26,7 @@ import (
 	"strings"
 
 	"github.com/arduino/arduino-cli/internal/arduino/builder/cpp"
-	"github.com/arduino/arduino-cli/internal/arduino/builder/internal/diagnostics"
+	"github.com/arduino/arduino-cli/internal/arduino/builder/internal/diagnosticmanager"
 	"github.com/arduino/arduino-cli/internal/arduino/builder/internal/preprocessor/internal/ctags"
 	"github.com/arduino/arduino-cli/internal/arduino/sketch"
 	"github.com/arduino/arduino-cli/internal/i18n"
@@ -44,7 +44,7 @@ var DebugPreprocessor bool
 func PreprocessSketchWithCtags(
 	sketch *sketch.Sketch, buildPath *paths.Path, includes paths.PathList,
 	lineOffset int, buildProperties *properties.Map, onlyUpdateCompilationDatabase bool,
-	compilerOutputParserCB diagnostics.CompilerOutputParserCB,
+	diagnosticManager *diagnosticmanager.Manager,
 ) ([]byte, []byte, error) {
 	// Create a temporary working directory
 	tmpDir, err := paths.MkTempDir("", "")
@@ -59,7 +59,7 @@ func PreprocessSketchWithCtags(
 
 	// Run GCC preprocessor
 	sourceFile := buildPath.Join("sketch", sketch.MainFile.Base()+".cpp")
-	gccStdout, gccStderr, err := GCC(sourceFile, ctagsTarget, includes, buildProperties, compilerOutputParserCB)
+	gccStdout, gccStderr, err := GCC(sourceFile, ctagsTarget, includes, buildProperties, diagnosticManager)
 	verboseOutput.Write(gccStdout)
 	verboseOutput.Write(gccStderr)
 	normalOutput.Write(gccStderr)

Diff for: internal/arduino/builder/internal/preprocessor/gcc.go

@@ -23,7 +23,7 @@ import (
 
 	f "github.com/arduino/arduino-cli/internal/algorithms"
 	"github.com/arduino/arduino-cli/internal/arduino/builder/cpp"
-	"github.com/arduino/arduino-cli/internal/arduino/builder/internal/diagnostics"
+	"github.com/arduino/arduino-cli/internal/arduino/builder/internal/diagnosticmanager"
 	"github.com/arduino/go-paths-helper"
 	"github.com/arduino/go-properties-orderedmap"
 )
@@ -33,7 +33,7 @@ import (
 func GCC(
 	sourceFilePath, targetFilePath *paths.Path,
 	includes paths.PathList, buildProperties *properties.Map,
-	compilerOutputParserCB diagnostics.CompilerOutputParserCB,
+	diagnosticManager *diagnosticmanager.Manager,
 ) ([]byte, []byte, error) {
 	gccBuildProperties := properties.NewMap()
 	gccBuildProperties.Set("preproc.macros.flags", "-w -x c++ -E -CC")
@@ -79,8 +79,8 @@ func GCC(
 	}
 	stdout, stderr, err := proc.RunAndCaptureOutput(context.Background())
 
-	if compilerOutputParserCB != nil {
-		compilerOutputParserCB(proc.GetArgs(), stderr)
+	if diagnosticManager != nil {
+		diagnosticManager.ParseOutput(proc.GetArgs(), stderr)
 	}
 
 	// Append gcc arguments to stdout

Diff for: internal/arduino/builder/preprocess_sketch.go

@@ -26,7 +26,7 @@ func (b *Builder) preprocessSketch(includes paths.PathList) error {
 	normalOutput, verboseOutput, err := preprocessor.PreprocessSketchWithCtags(
 		b.sketch, b.buildPath, includes, b.lineOffset,
 		b.buildProperties, b.onlyUpdateCompilationDatabase,
-		b.compilerOutputParser,
+		b.diagnosticsManager,
 	)
 	if b.logger.Verbose() {
 		b.logger.WriteStdout(verboseOutput)