[breaking] Improved .pde warnings detection subroutines / gRPC LoadSketch API improvement (#2490)

* sketch.GetProfile returns an error if profile is not found

* Added Profile.ToRpc helper

* Added Sketch.ToRpc helper

* Factored Sketch message in gRPC API for 'LoadSketch' call

* Inline function call

* Improved implementation of sketch.GetSketchProfiles

* Moved subroutine that warns about .pde files in feedback package

* Updated docs

* Improved 'sketch archive' parameters check

* Fixed bug detected by integration test... oops

Author: cmaglie

Diff for: commands/daemon/daemon.go

@@ -170,7 +170,7 @@ func (s *ArduinoCoreServerImpl) NewSketch(ctx context.Context, req *rpc.NewSketc
 // LoadSketch FIXMEDOC
 func (s *ArduinoCoreServerImpl) LoadSketch(ctx context.Context, req *rpc.LoadSketchRequest) (*rpc.LoadSketchResponse, error) {
 	resp, err := sketch.LoadSketch(ctx, req)
-	return resp, convertErrorToRPCStatus(err)
+	return &rpc.LoadSketchResponse{Sketch: resp}, convertErrorToRPCStatus(err)
 }
 
 // SetSketchDefaults FIXMEDOC

Diff for: commands/instances.go

@@ -142,10 +142,11 @@ func Init(req *rpc.InitRequest, responseCallback func(r *rpc.InitResponse)) erro
 		if err != nil {
 			return &cmderrors.InvalidArgumentError{Cause: err}
 		}
-		profile = sk.GetProfile(req.GetProfile())
-		if profile == nil {
-			return &cmderrors.UnknownProfileError{Profile: req.GetProfile()}
+		p, err := sk.GetProfile(req.GetProfile())
+		if err != nil {
+			return err
 		}
+		profile = p
 		responseCallback(&rpc.InitResponse{
 			Message: &rpc.InitResponse_Profile{
 				Profile: &rpc.Profile{

Diff for: commands/sketch/load.go

@@ -24,56 +24,11 @@ import (
 	paths "github.com/arduino/go-paths-helper"
 )
 
-// LoadSketch collects and returns all files composing a sketch
-func LoadSketch(ctx context.Context, req *rpc.LoadSketchRequest) (*rpc.LoadSketchResponse, error) {
-	// TODO: This should be a ToRpc function for the Sketch struct
+// LoadSketch collects and returns all information about a sketch
+func LoadSketch(ctx context.Context, req *rpc.LoadSketchRequest) (*rpc.Sketch, error) {
 	sk, err := sketch.New(paths.New(req.GetSketchPath()))
 	if err != nil {
 		return nil, &cmderrors.CantOpenSketchError{Cause: err}
 	}
-
-	otherSketchFiles := make([]string, sk.OtherSketchFiles.Len())
-	for i, file := range sk.OtherSketchFiles {
-		otherSketchFiles[i] = file.String()
-	}
-
-	additionalFiles := make([]string, sk.AdditionalFiles.Len())
-	for i, file := range sk.AdditionalFiles {
-		additionalFiles[i] = file.String()
-	}
-
-	rootFolderFiles := make([]string, sk.RootFolderFiles.Len())
-	for i, file := range sk.RootFolderFiles {
-		rootFolderFiles[i] = file.String()
-	}
-
-	defaultPort, defaultProtocol := sk.GetDefaultPortAddressAndProtocol()
-
-	profiles := make([](*rpc.SketchProfile), len(sk.Project.Profiles))
-	for i, profile := range sk.Project.Profiles {
-		profiles[i] = &rpc.SketchProfile{
-			Name: profile.Name,
-			Fqbn: profile.FQBN,
-		}
-	}
-
-	defaultProfileResp := &rpc.SketchProfile{}
-	defaultProfile := sk.GetProfile(sk.Project.DefaultProfile)
-	if defaultProfile != nil {
-		defaultProfileResp.Name = defaultProfile.Name
-		defaultProfileResp.Fqbn = defaultProfile.FQBN
-	}
-
-	return &rpc.LoadSketchResponse{
-		MainFile:         sk.MainFile.String(),
-		LocationPath:     sk.FullPath.String(),
-		OtherSketchFiles: otherSketchFiles,
-		AdditionalFiles:  additionalFiles,
-		RootFolderFiles:  rootFolderFiles,
-		DefaultFqbn:      sk.GetDefaultFQBN(),
-		DefaultPort:      defaultPort,
-		DefaultProtocol:  defaultProtocol,
-		Profiles:         profiles,
-		DefaultProfile:   defaultProfileResp,
-	}, nil
+	return sk.ToRpc(), nil
 }

Diff for: docs/UPGRADING.md

@@ -4,6 +4,46 @@ Here you can find a list of migration guides to handle breaking changes between
 
 ## 0.36.0
 
+### The gRPC `cc.arduino.cli.commands.v1.LoadSketchResponse` message has been changed.
+
+Previously the `LoadSketchResponse` containted all the information about the sketch:
+
+```proto
+message LoadSketchResponse {
+  string main_file = 1;
+  string location_path = 2;
+  repeated string other_sketch_files = 3;
+  repeated string additional_files = 4;
+  repeated string root_folder_files = 5;
+  string default_fqbn = 6;
+  string default_port = 7;
+  string default_protocol = 8;
+  repeated SketchProfile profiles = 9;
+  SketchProfile default_profile = 10;
+}
+```
+
+Now all the metadata have been moved into a specific `Sketch` message:
+
+```proto
+message LoadSketchResponse {
+  Sketch sketch = 1;
+}
+
+message Sketch {
+  string main_file = 1;
+  string location_path = 2;
+  repeated string other_sketch_files = 3;
+  repeated string additional_files = 4;
+  repeated string root_folder_files = 5;
+  string default_fqbn = 6;
+  string default_port = 7;
+  string default_protocol = 8;
+  repeated SketchProfile profiles = 9;
+  SketchProfile default_profile = 10;
+}
+```
+
 ### Drop support for `builtin.tools`
 
 We're dropping the `builtin.tools` support. It was the equivalent of Arduino IDE 1.x bundled tools directory.

Diff for: internal/arduino/sketch/profiles.go

@@ -24,6 +24,7 @@ import (
 	"strings"
 
 	"github.com/arduino/arduino-cli/internal/arduino/utils"
+	rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
 	"github.com/arduino/go-paths-helper"
 	semver "go.bug.st/relaxed-semver"
 	"gopkg.in/yaml.v3"
@@ -99,6 +100,14 @@ type Profile struct {
 	Libraries ProfileRequiredLibraries `yaml:"libraries"`
 }
 
+// ToRpc converts this Profile to an rpc.SketchProfile
+func (p *Profile) ToRpc() *rpc.SketchProfile {
+	return &rpc.SketchProfile{
+		Name: p.Name,
+		Fqbn: p.FQBN,
+	}
+}
+
 // AsYaml outputs the profile as Yaml
 func (p *Profile) AsYaml() string {
 	res := ""

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

@@ -23,8 +23,11 @@ import (
 	"sort"
 	"strings"
 
+	"github.com/arduino/arduino-cli/commands/cmderrors"
+	f "github.com/arduino/arduino-cli/internal/algorithms"
 	"github.com/arduino/arduino-cli/internal/arduino/globals"
 	"github.com/arduino/arduino-cli/internal/i18n"
+	rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
 	"github.com/arduino/go-paths-helper"
 )
 
@@ -180,15 +183,14 @@ func (s *Sketch) supportedFiles() (*paths.PathList, error) {
 	return &files, nil
 }
 
-// GetProfile returns the requested profile or nil if the profile
-// is not found.
-func (s *Sketch) GetProfile(profileName string) *Profile {
+// GetProfile returns the requested profile or an error if not found
+func (s *Sketch) GetProfile(profileName string) (*Profile, error) {
 	for _, p := range s.Project.Profiles {
 		if p.Name == profileName {
-			return p
+			return p, nil
 		}
 	}
-	return nil
+	return nil, &cmderrors.UnknownProfileError{Profile: profileName}
 }
 
 // checkSketchCasing returns an error if the casing of the sketch folder and the main file are different.
@@ -278,23 +280,6 @@ func (e *InvalidSketchFolderNameError) Error() string {
 	return tr("no valid sketch found in %[1]s: missing %[2]s", e.SketchFolder, e.SketchFile)
 }
 
-// CheckForPdeFiles returns all files ending with .pde extension
-// in sketch, this is mainly used to warn the user that these files
-// must be changed to .ino extension.
-// When .pde files won't be supported anymore this function must be removed.
-func CheckForPdeFiles(sketch *paths.Path) []*paths.Path {
-	if sketch.IsNotDir() {
-		sketch = sketch.Parent()
-	}
-
-	files, err := sketch.ReadDirRecursive()
-	if err != nil {
-		return []*paths.Path{}
-	}
-	files.FilterSuffix(".pde")
-	return files
-}
-
 // DefaultBuildPath generates the default build directory for a given sketch.
 // The build path is in a temporary directory and is unique for each sketch.
 func (s *Sketch) DefaultBuildPath() *paths.Path {
@@ -307,3 +292,23 @@ func (s *Sketch) Hash() string {
 	md5SumBytes := md5.Sum([]byte(path))
 	return strings.ToUpper(hex.EncodeToString(md5SumBytes[:]))
 }
+
+// ToRpc converts this Sketch into a rpc.LoadSketchResponse
+func (s *Sketch) ToRpc() *rpc.Sketch {
+	defaultPort, defaultProtocol := s.GetDefaultPortAddressAndProtocol()
+	res := &rpc.Sketch{
+		MainFile:         s.MainFile.String(),
+		LocationPath:     s.FullPath.String(),
+		OtherSketchFiles: s.OtherSketchFiles.AsStrings(),
+		AdditionalFiles:  s.AdditionalFiles.AsStrings(),
+		RootFolderFiles:  s.RootFolderFiles.AsStrings(),
+		DefaultFqbn:      s.GetDefaultFQBN(),
+		DefaultPort:      defaultPort,
+		DefaultProtocol:  defaultProtocol,
+		Profiles:         f.Map(s.Project.Profiles, (*Profile).ToRpc),
+	}
+	if defaultProfile, err := s.GetProfile(s.Project.DefaultProfile); err == nil {
+		res.DefaultProfile = defaultProfile.ToRpc()
+	}
+	return res
+}

Diff for: internal/arduino/sketch/sketch_test.go

@@ -291,41 +291,6 @@ func TestGenBuildPath(t *testing.T) {
 	assert.Equal(t, "ACBD18DB4CC2F85CEDEF654FCCC4A4D8", (&Sketch{FullPath: paths.New("foo")}).Hash())
 }
 
-func TestCheckForPdeFiles(t *testing.T) {
-	sketchPath := paths.New("testdata", "SketchSimple")
-	files := CheckForPdeFiles(sketchPath)
-	require.Empty(t, files)
-
-	sketchPath = paths.New("testdata", "SketchPde")
-	files = CheckForPdeFiles(sketchPath)
-	require.Len(t, files, 1)
-	require.Equal(t, sketchPath.Join("SketchPde.pde"), files[0])
-
-	sketchPath = paths.New("testdata", "SketchMultipleMainFiles")
-	files = CheckForPdeFiles(sketchPath)
-	require.Len(t, files, 1)
-	require.Equal(t, sketchPath.Join("SketchMultipleMainFiles.pde"), files[0])
-
-	sketchPath = paths.New("testdata", "SketchSimple", "SketchSimple.ino")
-	files = CheckForPdeFiles(sketchPath)
-	require.Empty(t, files)
-
-	sketchPath = paths.New("testdata", "SketchPde", "SketchPde.pde")
-	files = CheckForPdeFiles(sketchPath)
-	require.Len(t, files, 1)
-	require.Equal(t, sketchPath.Parent().Join("SketchPde.pde"), files[0])
-
-	sketchPath = paths.New("testdata", "SketchMultipleMainFiles", "SketchMultipleMainFiles.ino")
-	files = CheckForPdeFiles(sketchPath)
-	require.Len(t, files, 1)
-	require.Equal(t, sketchPath.Parent().Join("SketchMultipleMainFiles.pde"), files[0])
-
-	sketchPath = paths.New("testdata", "SketchMultipleMainFiles", "SketchMultipleMainFiles.pde")
-	files = CheckForPdeFiles(sketchPath)
-	require.Len(t, files, 1)
-	require.Equal(t, sketchPath.Parent().Join("SketchMultipleMainFiles.pde"), files[0])
-}
-
 func TestNewSketchWithSymlink(t *testing.T) {
 	sketchPath, _ := paths.New("testdata", "SketchWithSymlink").Abs()
 	mainFilePath := sketchPath.Join("SketchWithSymlink.ino")

Diff for: internal/cli/arguments/sketch.go

@@ -19,6 +19,7 @@ import (
 	"context"
 
 	"github.com/arduino/arduino-cli/commands/sketch"
+	f "github.com/arduino/arduino-cli/internal/algorithms"
 	"github.com/arduino/arduino-cli/internal/cli/feedback"
 	rpc "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1"
 	"github.com/arduino/go-paths-helper"
@@ -27,8 +28,7 @@ import (
 
 // InitSketchPath returns an instance of paths.Path pointing to sketchPath.
 // If sketchPath is an empty string returns the current working directory.
-// In both cases it warns the user if he's using deprecated files
-func InitSketchPath(path string, printWarnings bool) (sketchPath *paths.Path) {
+func InitSketchPath(path string) (sketchPath *paths.Path) {
 	if path != "" {
 		sketchPath = paths.New(path)
 	} else {
@@ -39,11 +39,6 @@ func InitSketchPath(path string, printWarnings bool) (sketchPath *paths.Path) {
 		logrus.Infof("Reading sketch from dir: %s", wd)
 		sketchPath = wd
 	}
-	if printWarnings {
-		if msg := sketch.WarnDeprecatedFiles(sketchPath); msg != "" {
-			feedback.Warning(msg)
-		}
-	}
 	return sketchPath
 }
 
@@ -57,13 +52,10 @@ func GetSketchProfiles(sketchPath string) []string {
 			return nil
 		}
 	}
-	list, _ := sketch.LoadSketch(context.Background(), &rpc.LoadSketchRequest{
-		SketchPath: sketchPath,
-	})
-	profiles := list.GetProfiles()
-	res := make([]string, len(profiles))
-	for i, p := range list.GetProfiles() {
-		res[i] = p.GetName()
+	sk, err := sketch.LoadSketch(context.Background(), &rpc.LoadSketchRequest{SketchPath: sketchPath})
+	if err != nil {
+		return nil
 	}
-	return res
+	profiles := sk.GetProfiles()
+	return f.Map(profiles, (*rpc.SketchProfile).GetName)
 }

Diff for: internal/cli/board/attach.go

@@ -53,7 +53,7 @@ func initAttachCommand() *cobra.Command {
 }
 
 func runAttachCommand(path string, port *arguments.Port, fqbn string) {
-	sketchPath := arguments.InitSketchPath(path, true)
+	sketchPath := arguments.InitSketchPath(path)
 
 	portAddress, portProtocol, _ := port.GetPortAddressAndProtocol(nil, "", "")
 	newDefaults, err := sketch.SetSketchDefaults(context.Background(), &rpc.SetSketchDefaultsRequest{

Diff for: internal/cli/compile/compile.go

@@ -158,12 +158,12 @@ func runCompileCommand(cmd *cobra.Command, args []string) {
 		path = args[0]
 	}
 
-	sketchPath := arguments.InitSketchPath(path, true)
-
+	sketchPath := arguments.InitSketchPath(path)
 	sk, err := sketch.LoadSketch(context.Background(), &rpc.LoadSketchRequest{SketchPath: sketchPath.String()})
 	if err != nil {
 		feedback.FatalError(err, feedback.ErrGeneric)
 	}
+	feedback.WarnAboutDeprecatedFiles(sk)
 
 	var inst *rpc.Instance
 	var profile *rpc.Profile

Diff for: internal/cli/debug/debug.go

@@ -81,11 +81,13 @@ func runDebugCommand(args []string, portArgs *arguments.Port, fqbnArg *arguments
 		path = args[0]
 	}
 
-	sketchPath := arguments.InitSketchPath(path, true)
+	sketchPath := arguments.InitSketchPath(path)
 	sk, err := sketch.LoadSketch(context.Background(), &rpc.LoadSketchRequest{SketchPath: sketchPath.String()})
 	if err != nil {
 		feedback.FatalError(err, feedback.ErrGeneric)
 	}
+	feedback.WarnAboutDeprecatedFiles(sk)
+
 	fqbn, port := arguments.CalculateFQBNAndPort(portArgs, fqbnArg, instance, sk.GetDefaultFqbn(), sk.GetDefaultPort(), sk.GetDefaultProtocol())
 	debugConfigRequested := &rpc.GetDebugConfigRequest{
 		Instance:    instance,