introduce diagnosticmanager instead of using a callback approach

Author: alessio-perugini

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

@@ -25,6 +25,7 @@ import (
 
 	"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"
@@ -36,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
@@ -94,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
@@ -199,6 +195,7 @@ func NewBuilder(
 		logger.Warn(string(verboseOut))
 	}
 
+	diagnosticmanager := diagnosticmanager.New()
 	b := &Builder{
 		sketch:                        sk,
 		buildProperties:               buildProperties,
@@ -220,12 +217,6 @@ func NewBuilder(
 		targetPlatform:                targetPlatform,
 		actualPlatform:                actualPlatform,
 		toolEnv:                       toolEnv,
-		libsDetector: detector.NewSketchLibrariesDetector(
-			libsManager, libsResolver,
-			useCachedLibrariesResolution,
-			onlyUpdateCompilationDatabase,
-			logger,
-		),
 		buildOptions: newBuildOptions(
 			hardwareDirs, otherLibrariesDirs,
 			builtInLibrariesDirs, buildPath,
@@ -237,25 +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...)
-	}
-
 	return b, nil
 }
 
@@ -281,7 +262,7 @@ func (b *Builder) ImportedLibraries() libraries.List {
 
 // CompilerDiagnostics returns the parsed compiler diagnostics
 func (b *Builder) CompilerDiagnostics() diagnostics.Diagnostics {
-	return b.compilerDiagnostics
+	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,6 +26,7 @@ import (
 	"strings"
 	"time"
 
+	"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"
@@ -57,6 +58,7 @@ type SketchLibrariesDetector struct {
 	librariesResolutionResults    map[string]libraryResolutionResult
 	includeFolders                paths.PathList
 	logger                        *logger.BuilderLogger
+	diagnosticManager             *diagnosticmanager.Manager
 }
 
 // NewSketchLibrariesDetector todo
@@ -66,6 +68,7 @@ func NewSketchLibrariesDetector(
 	useCachedLibrariesResolution bool,
 	onlyUpdateCompilationDatabase bool,
 	logger *logger.BuilderLogger,
+	diagnosticManager *diagnosticmanager.Manager,
 ) *SketchLibrariesDetector {
 	return &SketchLibrariesDetector{
 		librariesManager:              lm,
@@ -76,6 +79,7 @@ func NewSketchLibrariesDetector(
 		includeFolders:                paths.PathList{},
 		onlyUpdateCompilationDatabase: onlyUpdateCompilationDatabase,
 		logger:                        logger,
+		diagnosticManager:             diagnosticManager,
 	}
 }
 
@@ -347,7 +351,7 @@ func (l *SketchLibrariesDetector) findIncludesUntilDone(
 			}
 		} else {
 			var preprocStdout []byte
-			preprocStdout, preprocStderr, preprocErr = preprocessor.GCC(sourcePath, targetFilePath, includeFolders, buildProperties)
+			preprocStdout, preprocStderr, preprocErr = preprocessor.GCC(sourcePath, targetFilePath, includeFolders, buildProperties, l.diagnosticManager)
 			if l.logger.Verbose() {
 				l.logger.WriteStdout(preprocStdout)
 			}
@@ -379,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)
+				preprocStdout, preprocStderr, preprocErr = preprocessor.GCC(sourcePath, targetFilePath, includeFolders, buildProperties, l.diagnosticManager)
 				if l.logger.Verbose() {
 					l.logger.WriteStdout(preprocStdout)
 				}

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

@@ -0,0 +1,37 @@
+package diagnosticmanager
+
+import (
+	"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) Parse(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) CompilerDiagnostics() diagnostics.Diagnostics {
+	return m.results
+}

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

@@ -22,6 +22,7 @@ import (
 	"path/filepath"
 	"runtime"
 
+	"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"
@@ -30,7 +31,11 @@ import (
 
 // PreprocessSketchWithArduinoPreprocessor performs preprocessing of the arduino sketch
 // using arduino-preprocessor (https://github.com/arduino/arduino-preprocessor).
-func PreprocessSketchWithArduinoPreprocessor(sk *sketch.Sketch, buildPath *paths.Path, includeFolders paths.PathList, lineOffset int, buildProperties *properties.Map, onlyUpdateCompilationDatabase bool) ([]byte, []byte, error) {
+func PreprocessSketchWithArduinoPreprocessor(
+	sk *sketch.Sketch, buildPath *paths.Path, includeFolders paths.PathList,
+	lineOffset int, buildProperties *properties.Map, onlyUpdateCompilationDatabase bool,
+	diagnosticmanager *diagnosticmanager.Manager,
+) ([]byte, []byte, error) {
 	verboseOut := &bytes.Buffer{}
 	normalOut := &bytes.Buffer{}
 	if err := buildPath.Join("preproc").MkdirAll(); err != nil {
@@ -39,7 +44,7 @@ func PreprocessSketchWithArduinoPreprocessor(sk *sketch.Sketch, buildPath *paths
 
 	sourceFile := buildPath.Join("sketch", sk.MainFile.Base()+".cpp")
 	targetFile := buildPath.Join("preproc", "sketch_merged.cpp")
-	gccStdout, gccStderr, err := GCC(sourceFile, targetFile, includeFolders, buildProperties)
+	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,6 +26,7 @@ import (
 	"strings"
 
 	"github.com/arduino/arduino-cli/internal/arduino/builder/cpp"
+	"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"
@@ -40,7 +41,11 @@ var tr = i18n.Tr
 var DebugPreprocessor bool
 
 // PreprocessSketchWithCtags performs preprocessing of the arduino sketch using CTags.
-func PreprocessSketchWithCtags(sketch *sketch.Sketch, buildPath *paths.Path, includes paths.PathList, lineOffset int, buildProperties *properties.Map, onlyUpdateCompilationDatabase bool) ([]byte, []byte, error) {
+func PreprocessSketchWithCtags(
+	sketch *sketch.Sketch, buildPath *paths.Path, includes paths.PathList,
+	lineOffset int, buildProperties *properties.Map, onlyUpdateCompilationDatabase bool,
+	diagnosticManager *diagnosticmanager.Manager,
+) ([]byte, []byte, error) {
 	// Create a temporary working directory
 	tmpDir, err := paths.MkTempDir("", "")
 	if err != nil {
@@ -54,7 +59,7 @@ func PreprocessSketchWithCtags(sketch *sketch.Sketch, buildPath *paths.Path, inc
 
 	// Run GCC preprocessor
 	sourceFile := buildPath.Join("sketch", sketch.MainFile.Base()+".cpp")
-	gccStdout, gccStderr, err := GCC(sourceFile, ctagsTarget, includes, buildProperties)
+	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,13 +23,18 @@ 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/diagnosticmanager"
 	"github.com/arduino/go-paths-helper"
 	"github.com/arduino/go-properties-orderedmap"
 )
 
 // GCC performs a run of the gcc preprocess (macro/includes expansion). The function outputs the result
 // to targetFilePath. Returns the stdout/stderr of gcc if any.
-func GCC(sourceFilePath *paths.Path, targetFilePath *paths.Path, includes paths.PathList, buildProperties *properties.Map) ([]byte, []byte, error) {
+func GCC(
+	sourceFilePath, targetFilePath *paths.Path,
+	includes paths.PathList, buildProperties *properties.Map,
+	diagnosticManager *diagnosticmanager.Manager,
+) ([]byte, []byte, error) {
 	gccBuildProperties := properties.NewMap()
 	gccBuildProperties.Set("preproc.macros.flags", "-w -x c++ -E -CC")
 	gccBuildProperties.Merge(buildProperties)
@@ -74,6 +79,10 @@ func GCC(sourceFilePath *paths.Path, targetFilePath *paths.Path, includes paths.
 	}
 	stdout, stderr, err := proc.RunAndCaptureOutput(context.Background())
 
+	if diagnosticManager != nil {
+		diagnosticManager.ParseOutput(proc.GetArgs(), stderr)
+	}
+
 	// Append gcc arguments to stdout
 	stdout = append([]byte(fmt.Sprintln(strings.Join(args, " "))), stdout...)
 

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

@@ -26,6 +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.diagnosticsManager,
 	)
 	if b.logger.Verbose() {
 		b.logger.WriteStdout(verboseOutput)