Add diagnostics in the preprocessor (arduino#2515)

Author: alessio-perugini

Diff for: Taskfile.yml

@@ -137,7 +137,7 @@ tasks:
     desc: Add missing go license headers
     cmds:
       - go install github.com/google/[email protected]
-      - addlicense -c "ARDUINO SA (http://www.arduino.cc/)" -f ./license_header.tpl **/*.go
+      - addlicense -c "ARDUINO SA (http://www.arduino.cc/)" -f ./license_header.tpl $(find . -name "*.go" -type f -print0 | xargs -0)
 
   # Source: https://github.com/arduino/tooling-project-assets/blob/main/workflow-templates/assets/check-markdown-task/Taskfile.yml
   markdown:check-links:

Diff for: commands/internal/instances/instances.go

@@ -1,3 +1,18 @@
+// This file is part of arduino-cli.
+//
+// Copyright 2024 ARDUINO SA (http://www.arduino.cc/)
+//
+// This software is released under the GNU General Public License version 3,
+// which covers the main part of arduino-cli.
+// The terms of this license can be found at:
+// https://www.gnu.org/licenses/gpl-3.0.en.html
+//
+// You can be released from the requirements of the above licenses by purchasing
+// a commercial license. Buying such a license is mandatory if you want to
+// modify or otherwise use the software for commercial activities involving the
+// Arduino software without disclosing the source code of your own applications.
+// To purchase a commercial license, send an email to [email protected].
+
 package instances
 
 import (

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

@@ -36,7 +36,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 +93,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
+	diagnosticStore *diagnostics.Store
 }
 
 // buildArtifacts contains the result of various build
@@ -199,6 +194,7 @@ func NewBuilder(
 		logger.Warn(string(verboseOut))
 	}
 
+	diagnosticStore := diagnostics.NewStore()
 	b := &Builder{
 		sketch:                        sk,
 		buildProperties:               buildProperties,
@@ -220,12 +216,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 +227,15 @@ func NewBuilder(
 			buildProperties.GetPath("runtime.platform.path"),
 			buildProperties.GetPath("build.core.path"), // TODO can we buildCorePath ?
 		),
+		diagnosticStore: diagnosticStore,
+		libsDetector: detector.NewSketchLibrariesDetector(
+			libsManager, libsResolver,
+			useCachedLibrariesResolution,
+			onlyUpdateCompilationDatabase,
+			logger,
+			diagnosticStore,
+		),
 	}
-
-	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 +261,7 @@ func (b *Builder) ImportedLibraries() libraries.List {
 
 // CompilerDiagnostics returns the parsed compiler diagnostics
 func (b *Builder) CompilerDiagnostics() diagnostics.Diagnostics {
-	return b.compilerDiagnostics
+	return b.diagnosticStore.Diagnostics()
 }
 
 // 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.diagnosticStore != nil {
+			b.diagnosticStore.Parse(command.GetArgs(), commandStdout.Bytes())
+			b.diagnosticStore.Parse(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/diagnostics"
 	"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
+	diagnosticStore               *diagnostics.Store
 }
 
 // NewSketchLibrariesDetector todo
@@ -66,6 +68,7 @@ func NewSketchLibrariesDetector(
 	useCachedLibrariesResolution bool,
 	onlyUpdateCompilationDatabase bool,
 	logger *logger.BuilderLogger,
+	diagnosticStore *diagnostics.Store,
 ) *SketchLibrariesDetector {
 	return &SketchLibrariesDetector{
 		librariesManager:              lm,
@@ -76,6 +79,7 @@ func NewSketchLibrariesDetector(
 		includeFolders:                paths.PathList{},
 		onlyUpdateCompilationDatabase: onlyUpdateCompilationDatabase,
 		logger:                        logger,
+		diagnosticStore:               diagnosticStore,
 	}
 }
 
@@ -337,7 +341,7 @@ func (l *SketchLibrariesDetector) findIncludesUntilDone(
 		}
 
 		var preprocErr error
-		var preprocStderr []byte
+		var preprocFirstResult preprocessor.Result
 
 		var missingIncludeH string
 		if unchanged && cache.valid {
@@ -346,21 +350,20 @@ func (l *SketchLibrariesDetector) findIncludesUntilDone(
 				l.logger.Info(tr("Using cached library dependencies for file: %[1]s", sourcePath))
 			}
 		} else {
-			var preprocStdout []byte
-			preprocStdout, preprocStderr, preprocErr = preprocessor.GCC(sourcePath, targetFilePath, includeFolders, buildProperties)
+			preprocFirstResult, preprocErr = preprocessor.GCC(sourcePath, targetFilePath, includeFolders, buildProperties)
 			if l.logger.Verbose() {
-				l.logger.WriteStdout(preprocStdout)
+				l.logger.WriteStdout(preprocFirstResult.Stdout())
 			}
 			// Unwrap error and see if it is an ExitError.
 			var exitErr *exec.ExitError
 			if preprocErr == nil {
 				// Preprocessor successful, done
 				missingIncludeH = ""
-			} else if isExitErr := errors.As(preprocErr, &exitErr); !isExitErr || preprocStderr == nil {
+			} else if isExitErr := errors.As(preprocErr, &exitErr); !isExitErr || preprocFirstResult.Stderr() == nil {
 				// Ignore ExitErrors (e.g. gcc returning non-zero status), but bail out on other errors
 				return preprocErr
 			} else {
-				missingIncludeH = IncludesFinderWithRegExp(string(preprocStderr))
+				missingIncludeH = IncludesFinderWithRegExp(string(preprocFirstResult.Stderr()))
 				if missingIncludeH == "" && l.logger.Verbose() {
 					l.logger.Info(tr("Error while detecting libraries included by %[1]s", sourcePath))
 				}
@@ -376,22 +379,25 @@ func (l *SketchLibrariesDetector) findIncludesUntilDone(
 		library := l.resolveLibrary(missingIncludeH, platformArch)
 		if library == nil {
 			// Library could not be resolved, show error
-			if preprocErr == nil || preprocStderr == nil {
+			if preprocErr == nil || preprocFirstResult.Stderr() == 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)
+				result, err := preprocessor.GCC(sourcePath, targetFilePath, includeFolders, buildProperties)
 				if l.logger.Verbose() {
-					l.logger.WriteStdout(preprocStdout)
+					l.logger.WriteStdout(result.Stdout())
 				}
-				if preprocErr == nil {
+				if err == nil {
 					// If there is a missing #include in the cache, but running
 					// gcc does not reproduce that, there is something wrong.
 					// Returning an error here will cause the cache to be
 					// deleted, so hopefully the next compilation will succeed.
 					return errors.New(tr("Internal error in cache"))
 				}
+				l.diagnosticStore.Parse(result.Args(), result.Stderr())
+				l.logger.WriteStderr(result.Stderr())
+				return err
 			}
-			l.logger.WriteStderr(preprocStderr)
+			l.diagnosticStore.Parse(preprocFirstResult.Args(), preprocFirstResult.Stderr())
+			l.logger.WriteStderr(preprocFirstResult.Stderr())
 			return preprocErr
 		}
 

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

@@ -0,0 +1,51 @@
+// This file is part of arduino-cli.
+//
+// Copyright 2024 ARDUINO SA (http://www.arduino.cc/)
+//
+// This software is released under the GNU General Public License version 3,
+// which covers the main part of arduino-cli.
+// The terms of this license can be found at:
+// https://www.gnu.org/licenses/gpl-3.0.en.html
+//
+// You can be released from the requirements of the above licenses by purchasing
+// a commercial license. Buying such a license is mandatory if you want to
+// modify or otherwise use the software for commercial activities involving the
+// Arduino software without disclosing the source code of your own applications.
+// To purchase a commercial license, send an email to [email protected].
+
+package diagnostics
+
+import (
+	"github.com/sirupsen/logrus"
+)
+
+type Store struct {
+	results Diagnostics
+}
+
+func NewStore() *Store {
+	return &Store{}
+}
+
+// Parse parses the output coming from a compiler. It then stores the parsed
+// diagnostics in a slice of Diagnostic.
+func (m *Store) Parse(cmdline []string, out []byte) {
+	compiler := 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 := ParseCompilerOutput(compiler, out)
+	if err != nil {
+		logrus.Warnf("Error parsing compiler output: %s", err)
+		return
+	}
+	m.results = append(m.results, diags...)
+}
+
+func (m *Store) Diagnostics() Diagnostics {
+	return m.results
+}

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

@@ -30,20 +30,23 @@ 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,
+) (*Result, error) {
 	verboseOut := &bytes.Buffer{}
 	normalOut := &bytes.Buffer{}
 	if err := buildPath.Join("preproc").MkdirAll(); err != nil {
-		return nil, nil, err
+		return nil, err
 	}
 
 	sourceFile := buildPath.Join("sketch", sk.MainFile.Base()+".cpp")
 	targetFile := buildPath.Join("preproc", "sketch_merged.cpp")
-	gccStdout, gccStderr, err := GCC(sourceFile, targetFile, includeFolders, buildProperties)
-	verboseOut.Write(gccStdout)
-	verboseOut.Write(gccStderr)
+	gccResult, err := GCC(sourceFile, targetFile, includeFolders, buildProperties)
+	verboseOut.Write(gccResult.Stdout())
+	verboseOut.Write(gccResult.Stderr())
 	if err != nil {
-		return nil, nil, err
+		return nil, err
 	}
 
 	arduiniPreprocessorProperties := properties.NewMap()
@@ -56,18 +59,18 @@ func PreprocessSketchWithArduinoPreprocessor(sk *sketch.Sketch, buildPath *paths
 	arduiniPreprocessorProperties.SetPath("source_file", targetFile)
 	pattern := arduiniPreprocessorProperties.Get("pattern")
 	if pattern == "" {
-		return nil, nil, errors.New(tr("arduino-preprocessor pattern is missing"))
+		return nil, errors.New(tr("arduino-preprocessor pattern is missing"))
 	}
 
 	commandLine := arduiniPreprocessorProperties.ExpandPropsInString(pattern)
 	parts, err := properties.SplitQuotedString(commandLine, `"'`, false)
 	if err != nil {
-		return nil, nil, err
+		return nil, err
 	}
 
 	command, err := paths.NewProcess(nil, parts...)
 	if err != nil {
-		return nil, nil, err
+		return nil, err
 	}
 	if runtime.GOOS == "windows" {
 		// chdir in the uppermost directory to avoid UTF-8 bug in clang (https://github.com/arduino/arduino-preprocessor/issues/2)
@@ -78,14 +81,13 @@ func PreprocessSketchWithArduinoPreprocessor(sk *sketch.Sketch, buildPath *paths
 	commandStdOut, commandStdErr, err := command.RunAndCaptureOutput(context.Background())
 	verboseOut.Write(commandStdErr)
 	if err != nil {
-		return normalOut.Bytes(), verboseOut.Bytes(), err
+		return &Result{args: gccResult.Args(), stdout: verboseOut.Bytes(), stderr: normalOut.Bytes()}, err
 	}
 	result := utils.NormalizeUTF8(commandStdOut)
 
 	destFile := buildPath.Join(sk.MainFile.Base() + ".cpp")
 	if err := destFile.WriteFile(result); err != nil {
-		return normalOut.Bytes(), verboseOut.Bytes(), err
+		return &Result{args: gccResult.Args(), stdout: verboseOut.Bytes(), stderr: normalOut.Bytes()}, err
 	}
-
-	return normalOut.Bytes(), verboseOut.Bytes(), err
+	return &Result{args: gccResult.Args(), stdout: verboseOut.Bytes(), stderr: normalOut.Bytes()}, err
 }