[skip-changelog] legacy: some simple refactorings (#2206)

* Factored a method in library.List

* Fixed lint check

* Moved ResolveLibrary function in the correct source file

* Rename variable 'includes' -> 'includeFolders'

* Rename variables to use snakeCase (golang idiomatic)

* Rename variable 'include' -> 'missingIncludeH'

* Use pointers for SourceFile queues

* Clean up implementation of UniqueSourceFileQueue

arduino/libraries/librarieslist.go

@@ -41,10 +41,10 @@ func (list *List) Add(libs ...*Library) {
 	}
 }
 
-// Remove removes the library from the list
-func (list *List) Remove(library *Library) {
+// Remove removes the given library from the list
+func (list *List) Remove(libraryToRemove *Library) {
 	for i, lib := range *list {
-		if lib == library {
+		if lib == libraryToRemove {
 			*list = append((*list)[:i], (*list)[i+1:]...)
 			return
 		}

legacy/builder/container_find_includes.go

@@ -342,12 +342,12 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFileQu
 
 	first := true
 	for {
-		var include string
+		var missingIncludeH string
 		cache.ExpectFile(sourcePath)
 
-		includes := ctx.IncludeFolders
+		includeFolders := ctx.IncludeFolders
 		if library, ok := sourceFile.Origin.(*libraries.Library); ok && library.UtilityDir != nil {
-			includes = append(includes, library.UtilityDir)
+			includeFolders = append(includeFolders, library.UtilityDir)
 		}
 
 		if library, ok := sourceFile.Origin.(*libraries.Library); ok {
@@ -361,56 +361,56 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFileQu
 			}
 		}
 
-		var preproc_err error
-		var preproc_stderr []byte
+		var preprocErr error
+		var preprocStderr []byte
 
 		if unchanged && cache.valid {
-			include = cache.Next().Include
+			missingIncludeH = cache.Next().Include
 			if first && ctx.Verbose {
 				ctx.Info(tr("Using cached library dependencies for file: %[1]s", sourcePath))
 			}
 		} else {
-			var preproc_stdout []byte
-			preproc_stdout, preproc_stderr, preproc_err = preprocessor.GCC(sourcePath, targetFilePath, includes, ctx.BuildProperties)
+			var preprocStdout []byte
+			preprocStdout, preprocStderr, preprocErr = preprocessor.GCC(sourcePath, targetFilePath, includeFolders, ctx.BuildProperties)
 			if ctx.Verbose {
-				ctx.WriteStdout(preproc_stdout)
-				ctx.WriteStdout(preproc_stderr)
+				ctx.WriteStdout(preprocStdout)
+				ctx.WriteStdout(preprocStderr)
 			}
 			// Unwrap error and see if it is an ExitError.
-			_, is_exit_error := errors.Cause(preproc_err).(*exec.ExitError)
-			if preproc_err == nil {
+			_, isExitErr := errors.Cause(preprocErr).(*exec.ExitError)
+			if preprocErr == nil {
 				// Preprocessor successful, done
-				include = ""
-			} else if !is_exit_error || preproc_stderr == nil {
+				missingIncludeH = ""
+			} else if !isExitErr || preprocStderr == nil {
 				// Ignore ExitErrors (e.g. gcc returning
 				// non-zero status), but bail out on
 				// other errors
-				return errors.WithStack(preproc_err)
+				return errors.WithStack(preprocErr)
 			} else {
-				include = IncludesFinderWithRegExp(string(preproc_stderr))
-				if include == "" && ctx.Verbose {
+				missingIncludeH = IncludesFinderWithRegExp(string(preprocStderr))
+				if missingIncludeH == "" && ctx.Verbose {
 					ctx.Info(tr("Error while detecting libraries included by %[1]s", sourcePath))
 				}
 			}
 		}
 
-		if include == "" {
+		if missingIncludeH == "" {
 			// No missing includes found, we're done
 			cache.ExpectEntry(sourcePath, "", nil)
 			return nil
 		}
 
-		library := ResolveLibrary(ctx, include)
+		library := ResolveLibrary(ctx, missingIncludeH)
 		if library == nil {
 			// Library could not be resolved, show error
-			if preproc_err == nil || preproc_stderr == nil {
+			if preprocErr == nil || preprocStderr == nil {
 				// Filename came from cache, so run preprocessor to obtain error to show
-				var preproc_stdout []byte
-				preproc_stdout, preproc_stderr, preproc_err = preprocessor.GCC(sourcePath, targetFilePath, includes, ctx.BuildProperties)
+				var preprocStdout []byte
+				preprocStdout, preprocStderr, preprocErr = preprocessor.GCC(sourcePath, targetFilePath, includeFolders, ctx.BuildProperties)
 				if ctx.Verbose {
-					ctx.WriteStdout(preproc_stdout)
+					ctx.WriteStdout(preprocStdout)
 				}
-				if preproc_err == nil {
+				if preprocErr == 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
@@ -418,15 +418,15 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFileQu
 					return errors.New(tr("Internal error in cache"))
 				}
 			}
-			ctx.WriteStderr(preproc_stderr)
-			return errors.WithStack(preproc_err)
+			ctx.WriteStderr(preprocStderr)
+			return errors.WithStack(preprocErr)
 		}
 
 		// Add this library to the list of libraries, the
 		// include path and queue its source files for further
 		// include scanning
 		ctx.ImportedLibraries = append(ctx.ImportedLibraries, library)
-		appendIncludeFolder(ctx, cache, sourcePath, include, library.SourceDir)
+		appendIncludeFolder(ctx, cache, sourcePath, missingIncludeH, library.SourceDir)
 		sourceDirs := library.SourceDirs()
 		for _, sourceDir := range sourceDirs {
 			queueSourceFilesFromFolder(ctx, sourceFileQueue, library, sourceDir.Dir, sourceDir.Recurse)
@@ -455,3 +455,48 @@ func queueSourceFilesFromFolder(ctx *types.Context, sourceFileQueue *types.Uniqu
 
 	return nil
 }
+
+func ResolveLibrary(ctx *types.Context, header string) *libraries.Library {
+	resolver := ctx.LibrariesResolver
+	importedLibraries := ctx.ImportedLibraries
+
+	candidates := resolver.AlternativesFor(header)
+
+	if ctx.Verbose {
+		ctx.Info(tr("Alternatives for %[1]s: %[2]s", header, candidates))
+		ctx.Info(fmt.Sprintf("ResolveLibrary(%s)", header))
+		ctx.Info(fmt.Sprintf("  -> %s: %s", tr("candidates"), candidates))
+	}
+
+	if len(candidates) == 0 {
+		return nil
+	}
+
+	for _, candidate := range candidates {
+		if importedLibraries.Contains(candidate) {
+			return nil
+		}
+	}
+
+	selected := resolver.ResolveFor(header, ctx.TargetPlatform.Platform.Architecture)
+	if alreadyImported := importedLibraries.FindByName(selected.Name); alreadyImported != nil {
+		// Certain libraries might have the same name but be different.
+		// This usually happens when the user includes two or more custom libraries that have
+		// different header name but are stored in a parent folder with identical name, like
+		// ./libraries1/Lib/lib1.h and ./libraries2/Lib/lib2.h
+		// Without this check the library resolution would be stuck in a loop.
+		// This behaviour has been reported in this issue:
+		// https://github.com/arduino/arduino-cli/issues/973
+		if selected == alreadyImported {
+			selected = alreadyImported
+		}
+	}
+
+	candidates.Remove(selected)
+	ctx.LibrariesResolutionResults[header] = types.LibraryResolutionResult{
+		Library:          selected,
+		NotUsedLibraries: candidates,
+	}
+
+	return selected
+}

legacy/builder/resolve_library.go

-// This file is part of arduino-cli.
-//
-// Copyright 2020 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 license@arduino.cc.
-
-package builder
-
-import (
-	"fmt"
-
-	"github.com/arduino/arduino-cli/arduino/libraries"
-	"github.com/arduino/arduino-cli/legacy/builder/types"
-)
-
-func ResolveLibrary(ctx *types.Context, header string) *libraries.Library {
-	resolver := ctx.LibrariesResolver
-	importedLibraries := ctx.ImportedLibraries
-
-	candidates := resolver.AlternativesFor(header)
-
-	if ctx.Verbose {
-		ctx.Info(tr("Alternatives for %[1]s: %[2]s", header, candidates))
-		ctx.Info(fmt.Sprintf("ResolveLibrary(%s)", header))
-		ctx.Info(fmt.Sprintf("  -> %s: %s", tr("candidates"), candidates))
-	}
-
-	if candidates == nil || len(candidates) == 0 {
-		return nil
-	}
-
-	for _, candidate := range candidates {
-		if importedLibraries.Contains(candidate) {
-			return nil
-		}
-	}
-
-	selected := resolver.ResolveFor(header, ctx.TargetPlatform.Platform.Architecture)
-	if alreadyImported := importedLibraries.FindByName(selected.Name); alreadyImported != nil {
-		// Certain libraries might have the same name but be different.
-		// This usually happens when the user includes two or more custom libraries that have
-		// different header name but are stored in a parent folder with identical name, like
-		// ./libraries1/Lib/lib1.h and ./libraries2/Lib/lib2.h
-		// Without this check the library resolution would be stuck in a loop.
-		// This behaviour has been reported in this issue:
-		// https://github.com/arduino/arduino-cli/issues/973
-		if selected == alreadyImported {
-			selected = alreadyImported
-		}
-	}
-
-	ctx.LibrariesResolutionResults[header] = types.LibraryResolutionResult{
-		Library:          selected,
-		NotUsedLibraries: filterOutLibraryFrom(candidates, selected),
-	}
-
-	return selected
-}
-
-func filterOutLibraryFrom(libs libraries.List, libraryToRemove *libraries.Library) libraries.List {
-	filteredOutLibraries := []*libraries.Library{}
-	for _, lib := range libs {
-		if lib != libraryToRemove {
-			filteredOutLibraries = append(filteredOutLibraries, lib)
-		}
-	}
-	return filteredOutLibraries
-}

legacy/builder/types/accessories.go

@@ -17,28 +17,25 @@ package types
 
 import "golang.org/x/exp/slices"
 
-type UniqueSourceFileQueue []SourceFile
+type UniqueSourceFileQueue []*SourceFile
 
-func (queue UniqueSourceFileQueue) Len() int           { return len(queue) }
-func (queue UniqueSourceFileQueue) Less(i, j int) bool { return false }
-func (queue UniqueSourceFileQueue) Swap(i, j int)      { panic("Who called me?!?") }
-
-func (queue *UniqueSourceFileQueue) Push(value SourceFile) {
-	equals := func(elem SourceFile) bool {
-		return elem.Origin == value.Origin && elem.RelativePath.EqualsTo(value.RelativePath)
-	}
-	if !slices.ContainsFunc(*queue, equals) {
+func (queue *UniqueSourceFileQueue) Push(value *SourceFile) {
+	if !queue.Contains(value) {
 		*queue = append(*queue, value)
 	}
 }
 
-func (queue *UniqueSourceFileQueue) Pop() SourceFile {
+func (queue UniqueSourceFileQueue) Contains(target *SourceFile) bool {
+	return slices.ContainsFunc(queue, target.Equals)
+}
+
+func (queue *UniqueSourceFileQueue) Pop() *SourceFile {
 	old := *queue
 	x := old[0]
 	*queue = old[1:]
 	return x
 }
 
-func (queue *UniqueSourceFileQueue) Empty() bool {
-	return queue.Len() == 0
+func (queue UniqueSourceFileQueue) Empty() bool {
+	return len(queue) == 0
 }

legacy/builder/types/types.go

@@ -30,18 +30,23 @@ type SourceFile struct {
 	RelativePath *paths.Path
 }
 
+func (f *SourceFile) Equals(g *SourceFile) bool {
+	return f.Origin == g.Origin &&
+		f.RelativePath.EqualsTo(g.RelativePath)
+}
+
 // Create a SourceFile containing the given source file path within the
 // given origin. The given path can be absolute, or relative within the
 // origin's root source folder
-func MakeSourceFile(ctx *Context, origin interface{}, path *paths.Path) (SourceFile, error) {
+func MakeSourceFile(ctx *Context, origin interface{}, path *paths.Path) (*SourceFile, error) {
 	if path.IsAbs() {
 		var err error
 		path, err = sourceRoot(ctx, origin).RelTo(path)
 		if err != nil {
-			return SourceFile{}, err
+			return nil, err
 		}
 	}
-	return SourceFile{Origin: origin, RelativePath: path}, nil
+	return &SourceFile{Origin: origin, RelativePath: path}, nil
 }
 
 // Return the build root for the given origin, where build products will