From 6ac283ac3c134c6f40a40d523e0def8e5d53d266 Mon Sep 17 00:00:00 2001 From: Alessio Perugini Date: Fri, 22 Sep 2023 16:06:39 +0200 Subject: [PATCH 01/14] add golangcilint --- .github/workflows/check-go-task.yml | 6 +- .golangci.yml | 121 ++++++++++++++++++++++++++++ Taskfile.yml | 6 +- 3 files changed, 127 insertions(+), 6 deletions(-) create mode 100644 .golangci.yml diff --git a/.github/workflows/check-go-task.yml b/.github/workflows/check-go-task.yml index 2521a6af2d2..ed5df19a353 100644 --- a/.github/workflows/check-go-task.yml +++ b/.github/workflows/check-go-task.yml @@ -146,8 +146,10 @@ jobs: repo-token: ${{ secrets.GITHUB_TOKEN }} version: 3.x - - name: Install golint - run: go install golang.org/x/lint/golint@latest + - name: golangci-lint + uses: golangci/golangci-lint-action@v3 + with: + version: v1.54 - name: Check style env: diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 00000000000..4c89b237543 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,121 @@ +run: + timeout: 10m + go: '1.21' + tests: true + +linters: + # Disable all linters. + disable-all: true + # Enable specific linter + enable: + #- errcheck + - errorlint + - exportloopref + - gofmt + - goimports + - gosimple + - govet + - revive + - gosec + - misspell + - staticcheck + - typecheck + - unconvert + - ineffassign + - tenv + # Nice to have + #- thelper + #- depguard + #We have to dig deeper + #- gocritic + +linters-settings: + revive: + confidence: 0.8 + rules: + #- name: error-return + #- name: unused-parameter + #- name: var-naming + - name: blank-imports + - name: context-as-argument + - name: context-keys-type + - name: dot-imports + - name: empty-block + - name: error-naming + - name: error-strings + - name: errorf + - name: exported + - name: increment-decrement + - name: indent-error-flow + - name: package-comments + - name: range + - name: receiver-naming + - name: redefines-builtin-id + - name: superfluous-else + - name: time-naming + - name: unexported-return + - name: unreachable-code + - name: var-declaration + - name: defer + - name: atomic + - name: waitgroup-by-value + + errorlint: + # Check for plain error comparisons. + comparison: true + + # We might evalute to allow the asserts and errofs in the future + # Do not check for plain type assertions and type switches. + asserts: false + # Do not check whether fmt.Errorf uses the %w verb for formatting errors. + errorf: false + +issues: + # Fix found issues (if it's supported by the linter). + fix: true + # List of regexps of issue texts to exclude. + # + # But independently of this option we use default exclude patterns, + # it can be disabled by `exclude-use-default: false`. + # To list all excluded by default patterns execute `golangci-lint run --help` + # + # Default: https://golangci-lint.run/usage/false-positives/#default-exclusions + exclude-rules: + # Exclude some linters from running on tests files. + - path: _test\.go + linters: [gosec, errcheck] + # G401: Use of weak cryptographic primitive + - linters: [gosec] + text: "G401" + # G501: Blocklisted import crypto/md5: weak cryptographic primitive + - linters: [gosec] + text: "G501" + # G112: Potential Slowloris Attack because ReadHeaderTimeout is not configured in the http.Server + - linters: [gosec] + path: internal/integrationtest/ + text: "G112" + # G204: Subprocess launched with a potential tainted input or cmd arguments + - linters: [gosec] + path: executils/process.go + text: "G204" + # SA1019: req.GetQuery is deprecated: Marked as deprecated in cc/arduino/cli/commands/v1/lib.proto. + - linters: [staticcheck] + path: commands/lib/search.go + text: "SA1019" + + # Ignore revive emptyblock + - linters: [revive] + path: arduino/libraries/loader.go + text: "empty-block" + - linters: [revive] + path: arduino/serialutils/serialutils.go + text: "empty-block" + - linters: [revive] + path: arduino/resources/download.go + text: "empty-block" + - linters: [revive] + path: arduino/builder/internal/progress/progress_test.go + text: "empty-block" + - linters: [revive] + path: internal/algorithms/channels.go + text: "empty-block" diff --git a/Taskfile.yml b/Taskfile.yml index b7a4522d033..7ee6180e70f 100755 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -81,13 +81,11 @@ tasks: cmds: - | if ! which golint &>/dev/null; then - echo "golint not installed or not in PATH. Please install: https://github.com/golang/lint#installation" + echo "golangci-lint not installed or not in PATH. Please install: https://golangci-lint.run/usage/install/" exit 1 fi - | - golint \ - {{default "-min_confidence 0.8 -set_exit_status" .GO_LINT_FLAGS}} \ - {{default .DEFAULT_GO_PACKAGES .GO_PACKAGES}} + golangci-lint run # Source: https://github.com/arduino/tooling-project-assets/blob/main/workflow-templates/assets/test-go-task/Taskfile.yml go:test: From 3cc023081914cbc5a492998ab0aaa80343b03cad Mon Sep 17 00:00:00 2001 From: Alessio Perugini Date: Mon, 25 Sep 2023 16:10:39 +0200 Subject: [PATCH 02/14] fixup --- Taskfile.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Taskfile.yml b/Taskfile.yml index 7ee6180e70f..b9427792335 100755 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -80,7 +80,7 @@ tasks: dir: '{{default "./" .GO_MODULE_PATH}}' cmds: - | - if ! which golint &>/dev/null; then + if ! which golangci-lint &>/dev/null; then echo "golangci-lint not installed or not in PATH. Please install: https://golangci-lint.run/usage/install/" exit 1 fi From 32cfb8ee901825e44c0de64de3551516e2f9bd15 Mon Sep 17 00:00:00 2001 From: Alessio Perugini Date: Fri, 22 Sep 2023 16:07:15 +0200 Subject: [PATCH 03/14] apply gosec and staticcheck suggestions --- client_example/main.go | 3 ++- commands/daemon/term_example/main.go | 3 ++- internal/integrationtest/arduino-cli.go | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/client_example/main.go b/client_example/main.go index 925073ab8e2..a0e1bf528bd 100644 --- a/client_example/main.go +++ b/client_example/main.go @@ -31,6 +31,7 @@ import ( dbg "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/debug/v1" "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/settings/v1" "google.golang.org/grpc" + "google.golang.org/grpc/credentials/insecure" ) var ( @@ -43,7 +44,7 @@ func main() { // Establish a connection with the gRPC server, started with the command: // arduino-cli daemon - conn, err := grpc.Dial("localhost:50051", grpc.WithInsecure(), grpc.WithBlock()) + conn, err := grpc.Dial("localhost:50051", grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithBlock()) if err != nil { log.Fatal("error connecting to arduino-cli rpc server, you can start it by running `arduino-cli daemon`") } diff --git a/commands/daemon/term_example/main.go b/commands/daemon/term_example/main.go index 0326e052871..d7f351a6cd7 100644 --- a/commands/daemon/term_example/main.go +++ b/commands/daemon/term_example/main.go @@ -25,12 +25,13 @@ import ( "github.com/arduino/arduino-cli/rpc/cc/arduino/cli/commands/v1" "google.golang.org/grpc" + "google.golang.org/grpc/credentials/insecure" ) // This program exercise CLI monitor functionality. func main() { - conn, err := grpc.Dial("localhost:50051", grpc.WithInsecure(), grpc.WithBlock()) + conn, err := grpc.Dial("localhost:50051", grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithBlock()) if err != nil { log.Fatal("error connecting to arduino-cli rpc server, you can start it by running `arduino-cli daemon`") } diff --git a/internal/integrationtest/arduino-cli.go b/internal/integrationtest/arduino-cli.go index 0b45646f602..64ec3d8617e 100644 --- a/internal/integrationtest/arduino-cli.go +++ b/internal/integrationtest/arduino-cli.go @@ -34,6 +34,7 @@ import ( "github.com/fatih/color" "github.com/stretchr/testify/require" "google.golang.org/grpc" + "google.golang.org/grpc/credentials/insecure" ) // FindRepositoryRootPath returns the repository root path @@ -277,7 +278,7 @@ func (cli *ArduinoCLI) StartDaemon(verbose bool) string { } go copy(os.Stdout, stdout) go copy(os.Stderr, stderr) - conn, err := grpc.Dial(cli.daemonAddr, grpc.WithInsecure(), grpc.WithBlock()) + conn, err := grpc.Dial(cli.daemonAddr, grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithBlock()) cli.t.NoError(err) cli.daemonConn = conn cli.daemonClient = commands.NewArduinoCoreServiceClient(conn) From 90a50edb64b88d431fcb19c223d4f86e7925e84b Mon Sep 17 00:00:00 2001 From: Alessio Perugini Date: Fri, 22 Sep 2023 16:08:38 +0200 Subject: [PATCH 04/14] apply uncovert suggestion --- client_example/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client_example/main.go b/client_example/main.go index a0e1bf528bd..377e0d912d8 100644 --- a/client_example/main.go +++ b/client_example/main.go @@ -689,7 +689,7 @@ func callBoardListWatch(client rpc.ArduinoCoreServiceClient, instance *rpc.Insta }() // Watch for 10 seconds and then interrupts - timer := time.NewTicker(time.Duration(10 * time.Second)) + timer := time.NewTicker(10 * time.Second) <-timer.C } From 14c574c00436321f8f9c56b2f929f4cd98c57661 Mon Sep 17 00:00:00 2001 From: Alessio Perugini Date: Fri, 22 Sep 2023 16:16:44 +0200 Subject: [PATCH 05/14] apply misspell suggestion --- arduino/builder/linker.go | 2 +- arduino/builder/sketch.go | 2 +- arduino/cores/packagemanager/package_manager.go | 2 +- arduino/discovery/discovery.go | 4 ++-- executils/process.go | 2 +- internal/cli/arguments/port.go | 2 +- internal/cli/lib/check_deps.go | 2 +- internal/integrationtest/compile_2/compile_propeties_test.go | 4 ++-- internal/integrationtest/main/main_test.go | 2 +- internal/integrationtest/profiles/profiles_test.go | 2 +- internal/integrationtest/sketch/sketch_test.go | 2 +- internal/inventory/inventory.go | 2 +- 12 files changed, 14 insertions(+), 14 deletions(-) diff --git a/arduino/builder/linker.go b/arduino/builder/linker.go index f1d7ef00b36..ba1b95718ee 100644 --- a/arduino/builder/linker.go +++ b/arduino/builder/linker.go @@ -50,7 +50,7 @@ func (b *Builder) link() error { // and use that archives to complete the build. if len(objectFileList) > 30000 { - // We must create an object file for each visited directory: this is required becuase gcc-ar checks + // We must create an object file for each visited directory: this is required because gcc-ar checks // if an object file is already in the archive by looking ONLY at the filename WITHOUT the path, so // it may happen that a subdir/spi.o inside the archive may be overwritten by a anotherdir/spi.o // because thery are both named spi.o. diff --git a/arduino/builder/sketch.go b/arduino/builder/sketch.go index 044e284063d..f94b1b6bf16 100644 --- a/arduino/builder/sketch.go +++ b/arduino/builder/sketch.go @@ -140,7 +140,7 @@ func (b *Builder) sketchCopyAdditionalFiles(buildPath *paths.Path, overrides map sourceBytes = s } - // tag each addtional file with the filename of the source it was copied from + // tag each additional file with the filename of the source it was copied from sourceBytes = append([]byte("#line 1 "+cpp.QuoteString(file.String())+"\n"), sourceBytes...) err = writeIfDifferent(sourceBytes, targetPath) diff --git a/arduino/cores/packagemanager/package_manager.go b/arduino/cores/packagemanager/package_manager.go index fc5aa738b3c..7773b407109 100644 --- a/arduino/cores/packagemanager/package_manager.go +++ b/arduino/cores/packagemanager/package_manager.go @@ -319,7 +319,7 @@ func (pme *Explorer) ResolveFQBN(fqbn *cores.FQBN) ( } // Create the build properties map by overlaying the properties of the - // referenced platform propeties, the board platform properties and the + // referenced platform properties, the board platform properties and the // board specific properties. buildProperties := properties.NewMap() buildProperties.Merge(variantPlatformRelease.Properties) diff --git a/arduino/discovery/discovery.go b/arduino/discovery/discovery.go index 1832f1e77d3..8ba4bb38833 100644 --- a/arduino/discovery/discovery.go +++ b/arduino/discovery/discovery.go @@ -32,7 +32,7 @@ import ( "github.com/sirupsen/logrus" ) -// To work correctly a Pluggable Discovery must respect the state machine specifed on the documentation: +// To work correctly a Pluggable Discovery must respect the state machine specified on the documentation: // https://arduino.github.io/arduino-cli/latest/pluggable-discovery-specification/#state-machine // States a PluggableDiscovery can be in const ( @@ -85,7 +85,7 @@ func (msg discoveryMessage) String() string { return s } -// Port containts metadata about a port to connect to a board. +// Port contains metadata about a port to connect to a board. type Port struct { Address string `json:"address"` AddressLabel string `json:"label"` diff --git a/executils/process.go b/executils/process.go index 137d2f86bc3..a566226e73f 100644 --- a/executils/process.go +++ b/executils/process.go @@ -157,7 +157,7 @@ func (p *Process) Run() error { return p.cmd.Run() } -// SetEnvironment set the enviroment for the running process. Each entry is of the form "key=value". +// SetEnvironment set the environment for the running process. Each entry is of the form "key=value". // System default environments will be wiped out. func (p *Process) SetEnvironment(values []string) { p.cmd.Env = append([]string{}, values...) diff --git a/internal/cli/arguments/port.go b/internal/cli/arguments/port.go index 08192049c2c..d94a8b8d406 100644 --- a/internal/cli/arguments/port.go +++ b/internal/cli/arguments/port.go @@ -79,7 +79,7 @@ func (p *Port) GetPort(instance *rpc.Instance, defaultAddress, defaultProtocol s if address == "" { // If no address is provided we assume the user is trying to upload // to a board that supports a tool that automatically detects - // the attached board without specifying explictly a port. + // the attached board without specifying explicitly a port. // Tools that work this way must be specified using the property // "BOARD_ID.upload.tool.default" in the platform's boards.txt. return &rpc.Port{ diff --git a/internal/cli/lib/check_deps.go b/internal/cli/lib/check_deps.go index 5f92a207ff4..b286a2c85a5 100644 --- a/internal/cli/lib/check_deps.go +++ b/internal/cli/lib/check_deps.go @@ -82,7 +82,7 @@ func (dr checkDepResult) String() string { res := "" deps := dr.deps.Dependencies - // Sort depedencies alphabetically and then puts installed ones on top + // Sort dependencies alphabetically and then puts installed ones on top sort.Slice(deps, func(i, j int) bool { return deps[i].Name < deps[j].Name }) diff --git a/internal/integrationtest/compile_2/compile_propeties_test.go b/internal/integrationtest/compile_2/compile_propeties_test.go index c636bc16f82..add1075db53 100644 --- a/internal/integrationtest/compile_2/compile_propeties_test.go +++ b/internal/integrationtest/compile_2/compile_propeties_test.go @@ -41,13 +41,13 @@ func TestCompileAndUploadRuntimeProperties(t *testing.T) { _, _, err = cli.Run("core", "install", "arduino:avr") require.NoError(t, err) - // Check compile runtime propeties expansion + // Check compile runtime properties expansion bareMinimum := cli.CopySketch("bare_minimum") stdout, _, err := cli.Run("compile", "--fqbn", "foo:avr:bar", "-v", bareMinimum.String()) require.NoError(t, err) require.Contains(t, string(stdout), "PREBUILD-runtime.hardware.path="+sketchbookHardwareDir.String()) - // Check upload runtime propeties expansion + // Check upload runtime properties expansion stdout, _, err = cli.Run("upload", "--fqbn", "foo:avr:bar", bareMinimum.String()) require.NoError(t, err) require.Contains(t, string(stdout), "UPLOAD-runtime.hardware.path="+sketchbookHardwareDir.String()) diff --git a/internal/integrationtest/main/main_test.go b/internal/integrationtest/main/main_test.go index c2b94fc1a4b..7e039e62768 100644 --- a/internal/integrationtest/main/main_test.go +++ b/internal/integrationtest/main/main_test.go @@ -62,7 +62,7 @@ func TestVersion(t *testing.T) { switch version := jsonMap["VersionString"]; version { case "git-snapshot": require.Contains(t, version, "git-snapshot") - case "nigthly": + case "nightly": require.Contains(t, version, "nightly") default: _, err = semver.Parse(version) diff --git a/internal/integrationtest/profiles/profiles_test.go b/internal/integrationtest/profiles/profiles_test.go index 4915bed5cf7..ca26b75b4b3 100644 --- a/internal/integrationtest/profiles/profiles_test.go +++ b/internal/integrationtest/profiles/profiles_test.go @@ -66,7 +66,7 @@ func TestBuilderDidNotCatchLibsFromUnusedPlatforms(t *testing.T) { stdout, stderr, err := cli.Run("compile", "-b", "arduino:avr:uno", sketchPath.String()) require.Error(t, err) - // check that the libary resolver did not take the SAMD bundled Wire library into account + // check that the library resolver did not take the SAMD bundled Wire library into account require.NotContains(t, string(stdout), "samd") require.NotContains(t, string(stderr), "samd") } diff --git a/internal/integrationtest/sketch/sketch_test.go b/internal/integrationtest/sketch/sketch_test.go index 69854540536..aaf599dc305 100644 --- a/internal/integrationtest/sketch/sketch_test.go +++ b/internal/integrationtest/sketch/sketch_test.go @@ -412,7 +412,7 @@ func TestSketchArchiveOverwrite(t *testing.T) { _, _, err = cli.Run("sketch", "archive", sketchPath.String()) require.NoError(t, err) - // It is not possibile to override an archive by default + // It is not possible to override an archive by default _, stderr, err := cli.Run("sketch", "archive", sketchPath.String()) require.Error(t, err) require.Contains(t, string(stderr), "Archive already exists") diff --git a/internal/inventory/inventory.go b/internal/inventory/inventory.go index d366278e1e7..05b8f1ea24b 100644 --- a/internal/inventory/inventory.go +++ b/internal/inventory/inventory.go @@ -48,7 +48,7 @@ func Init(configPath string) error { // Attempt to read config file if err := Store.ReadInConfig(); err != nil { if _, ok := err.(viper.ConfigFileNotFoundError); !ok { - // If an error occurs during initalization of the store, just log it and recreate it from scratch. + // If an error occurs during initialization of the store, just log it and recreate it from scratch. logrus.WithError(err).Error("Error loading inventory store") } if err := generateInstallationData(); err != nil { From eaab88b79a94f53755711e0e0efa59f98f1b62a2 Mon Sep 17 00:00:00 2001 From: Alessio Perugini Date: Fri, 22 Sep 2023 16:27:59 +0200 Subject: [PATCH 06/14] apply errorlint suggestions --- arduino/discovery/discovery.go | 2 +- client_example/main.go | 29 ++++++++++--------- commands/daemon/stream.go | 3 +- internal/integrationtest/arduino-cli.go | 3 +- .../daemon/daemon_concurrency_test.go | 3 +- .../integrationtest/daemon/daemon_test.go | 27 ++++++++--------- 6 files changed, 36 insertions(+), 31 deletions(-) diff --git a/arduino/discovery/discovery.go b/arduino/discovery/discovery.go index 8ba4bb38833..1e339076754 100644 --- a/arduino/discovery/discovery.go +++ b/arduino/discovery/discovery.go @@ -194,7 +194,7 @@ func (disc *PluggableDiscovery) jsonDecodeLoop(in io.Reader, outChan chan<- *dis for { var msg discoveryMessage - if err := decoder.Decode(&msg); err == io.EOF { + if err := decoder.Decode(&msg); errors.Is(err, io.EOF) { // This is fine, we exit gracefully disc.statusMutex.Lock() disc.state = Dead diff --git a/client_example/main.go b/client_example/main.go index 377e0d912d8..2a98da82318 100644 --- a/client_example/main.go +++ b/client_example/main.go @@ -18,6 +18,7 @@ package main import ( "bytes" "context" + "errors" "fmt" "io" "log" @@ -353,7 +354,7 @@ func initInstance(client rpc.ArduinoCoreServiceClient, instance *rpc.Instance) { for { res, err := stream.Recv() // Server has finished sending - if err == io.EOF { + if errors.Is(err, io.EOF) { break } @@ -389,7 +390,7 @@ func callUpdateIndex(client rpc.ArduinoCoreServiceClient, instance *rpc.Instance uiResp, err := uiRespStream.Recv() // the server is done - if err == io.EOF { + if errors.Is(err, io.EOF) { log.Print("Update index done") break } @@ -442,7 +443,7 @@ func callPlatformInstall(client rpc.ArduinoCoreServiceClient, instance *rpc.Inst installResp, err := installRespStream.Recv() // The server is done. - if err == io.EOF { + if errors.Is(err, io.EOF) { log.Printf("Install done") break } @@ -496,7 +497,7 @@ func callPlatformUpgrade(client rpc.ArduinoCoreServiceClient, instance *rpc.Inst upgradeResp, err := upgradeRespStream.Recv() // The server is done. - if err == io.EOF { + if errors.Is(err, io.EOF) { log.Printf("Upgrade done") break } @@ -569,7 +570,7 @@ func callCompile(client rpc.ArduinoCoreServiceClient, instance *rpc.Instance) { compResp, err := compRespStream.Recv() // The server is done. - if err == io.EOF { + if errors.Is(err, io.EOF) { log.Print("Compilation done") break } @@ -609,7 +610,7 @@ func callUpload(client rpc.ArduinoCoreServiceClient, instance *rpc.Instance) { for { uplResp, err := uplRespStream.Recv() - if err == io.EOF { + if errors.Is(err, io.EOF) { log.Printf("Upload done") break } @@ -668,7 +669,7 @@ func callBoardListWatch(client rpc.ArduinoCoreServiceClient, instance *rpc.Insta go func() { for { res, err := watchClient.Recv() - if err == io.EOF { + if errors.Is(err, io.EOF) { log.Print("closing board watch connection") return } else if err != nil { @@ -708,7 +709,7 @@ func callPlatformUnInstall(client rpc.ArduinoCoreServiceClient, instance *rpc.In // Loop and consume the server stream until all the operations are done. for { uninstallResp, err := uninstallRespStream.Recv() - if err == io.EOF { + if errors.Is(err, io.EOF) { log.Print("Uninstall done") break } @@ -734,7 +735,7 @@ func callUpdateLibraryIndex(client rpc.ArduinoCoreServiceClient, instance *rpc.I // Loop and consume the server stream until all the operations are done. for { resp, err := libIdxUpdateStream.Recv() - if err == io.EOF { + if errors.Is(err, io.EOF) { log.Print("Library index update done") break } @@ -764,7 +765,7 @@ func callLibDownload(client rpc.ArduinoCoreServiceClient, instance *rpc.Instance // Loop and consume the server stream until all the operations are done. for { downloadResp, err := downloadRespStream.Recv() - if err == io.EOF { + if errors.Is(err, io.EOF) { log.Print("Lib download done") break } @@ -793,7 +794,7 @@ func callLibInstall(client rpc.ArduinoCoreServiceClient, instance *rpc.Instance, for { installResp, err := installRespStream.Recv() - if err == io.EOF { + if errors.Is(err, io.EOF) { log.Print("Lib install done") break } @@ -826,7 +827,7 @@ func callLibInstallNoDeps(client rpc.ArduinoCoreServiceClient, instance *rpc.Ins for { installResp, err := installRespStream.Recv() - if err == io.EOF { + if errors.Is(err, io.EOF) { log.Print("Lib install done") break } @@ -855,7 +856,7 @@ func callLibUpgradeAll(client rpc.ArduinoCoreServiceClient, instance *rpc.Instan for { resp, err := libUpgradeAllRespStream.Recv() - if err == io.EOF { + if errors.Is(err, io.EOF) { log.Printf("Lib upgrade all done") break } @@ -939,7 +940,7 @@ func callLibUninstall(client rpc.ArduinoCoreServiceClient, instance *rpc.Instanc for { uninstallResp, err := libUninstallRespStream.Recv() - if err == io.EOF { + if errors.Is(err, io.EOF) { log.Printf("Lib uninstall done") break } diff --git a/commands/daemon/stream.go b/commands/daemon/stream.go index 87fb213c761..aa5e70fb770 100644 --- a/commands/daemon/stream.go +++ b/commands/daemon/stream.go @@ -16,6 +16,7 @@ package daemon import ( + "errors" "io" "sync" "time" @@ -84,7 +85,7 @@ func consumeStreamFrom(reader func() ([]byte, error)) io.Reader { go func() { for { if data, err := reader(); err != nil { - if err == io.EOF { + if errors.Is(err, io.EOF) { w.Close() } else { w.CloseWithError(err) diff --git a/internal/integrationtest/arduino-cli.go b/internal/integrationtest/arduino-cli.go index 64ec3d8617e..5908358862b 100644 --- a/internal/integrationtest/arduino-cli.go +++ b/internal/integrationtest/arduino-cli.go @@ -19,6 +19,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "io" "os" @@ -337,7 +338,7 @@ func (inst *ArduinoCLIInstance) Init(profile string, sketchPath string, respCB f } for { msg, err := initClient.Recv() - if err == io.EOF { + if errors.Is(err, io.EOF) { logCallf("<<< Init EOF\n") return nil } diff --git a/internal/integrationtest/daemon/daemon_concurrency_test.go b/internal/integrationtest/daemon/daemon_concurrency_test.go index b7df90e3638..02e9f2501b6 100644 --- a/internal/integrationtest/daemon/daemon_concurrency_test.go +++ b/internal/integrationtest/daemon/daemon_concurrency_test.go @@ -17,6 +17,7 @@ package daemon_test import ( "context" + "errors" "fmt" "io" "testing" @@ -52,7 +53,7 @@ func TestArduinoCliDaemonCompileWithLotOfOutput(t *testing.T) { msgCount := 0 for { _, err := compile.Recv() - if err == io.EOF { + if errors.Is(err, io.EOF) { break } msgCount++ diff --git a/internal/integrationtest/daemon/daemon_test.go b/internal/integrationtest/daemon/daemon_test.go index 188d280711b..7e49db88111 100644 --- a/internal/integrationtest/daemon/daemon_test.go +++ b/internal/integrationtest/daemon/daemon_test.go @@ -17,6 +17,7 @@ package daemon_test import ( "context" + "errors" "fmt" "io" "testing" @@ -64,7 +65,7 @@ func TestArduinoCliDaemon(t *testing.T) { go func() { for { msg, err := watcher.Recv() - if err == io.EOF { + if errors.Is(err, io.EOF) { fmt.Println("Watcher EOF") return } @@ -140,7 +141,7 @@ func TestDaemonCompileOptions(t *testing.T) { require.NoError(t, err) for { msg, err := plInst.Recv() - if err == io.EOF { + if errors.Is(err, io.EOF) { break } require.NoError(t, err) @@ -164,7 +165,7 @@ func TestDaemonCompileOptions(t *testing.T) { require.NoError(t, err) for { msg, err := compile.Recv() - if err == io.EOF { + if errors.Is(err, io.EOF) { require.FailNow(t, "Expected compilation failure", "compilation succeeded") break } @@ -183,7 +184,7 @@ func TestDaemonCompileOptions(t *testing.T) { analyzer := NewTaskProgressAnalyzer(t) for { msg, err := compile.Recv() - if err == io.EOF { + if errors.Is(err, io.EOF) { break } require.NoError(t, err) @@ -216,7 +217,7 @@ func TestDaemonCompileAfterFailedLibInstall(t *testing.T) { require.NoError(t, err) for { msg, err := compile.Recv() - if err == io.EOF { + if errors.Is(err, io.EOF) { require.FailNow(t, "Expected compilation failure", "compilation succeeded") break } @@ -282,7 +283,7 @@ func TestDaemonBundleLibInstall(t *testing.T) { require.NoError(t, err) for { msg, err := instCl.Recv() - if err == io.EOF { + if errors.Is(err, io.EOF) { break } require.NoError(t, err) @@ -312,7 +313,7 @@ func TestDaemonBundleLibInstall(t *testing.T) { require.NoError(t, err) for { msg, err := instCl.Recv() - if err == io.EOF { + if errors.Is(err, io.EOF) { break } require.NoError(t, err) @@ -346,7 +347,7 @@ func TestDaemonBundleLibInstall(t *testing.T) { require.NoError(t, err) for { msg, err := uninstCl.Recv() - if err == io.EOF { + if errors.Is(err, io.EOF) { break } require.NoError(t, err) @@ -385,7 +386,7 @@ func TestDaemonBundleLibInstall(t *testing.T) { require.NoError(t, err) for { msg, err := instCl.Recv() - if err == io.EOF { + if errors.Is(err, io.EOF) { require.FailNow(t, "LibraryInstall is supposed to fail because builtin libraries directory is not set") } if err != nil { @@ -422,7 +423,7 @@ func TestDaemonLibrariesRescanOnInstall(t *testing.T) { require.NoError(t, err) for { _, err := instCl.Recv() - if err == io.EOF { + if errors.Is(err, io.EOF) { break } require.NoError(t, err) @@ -455,7 +456,7 @@ func TestDaemonCoreUpgradePlatform(t *testing.T) { require.NoError(t, err) for { _, err := plInst.Recv() - if err == io.EOF { + if errors.Is(err, io.EOF) { break } require.NoError(t, err) @@ -555,7 +556,7 @@ func analyzeUpdateIndexClient(t *testing.T, cl commands.ArduinoCoreService_Updat analyzer := NewDownloadProgressAnalyzer(t) for { msg, err := cl.Recv() - if err == io.EOF { + if errors.Is(err, io.EOF) { return analyzer.Results, nil } if err != nil { @@ -571,7 +572,7 @@ func analyzePlatformUpgradeClient(cl commands.ArduinoCoreService_PlatformUpgrade var upgradeError error for { msg, err := cl.Recv() - if err == io.EOF { + if errors.Is(err, io.EOF) { break } if msg.GetPlatform() != nil { From b332384f0dcb0e2ec37ec46e19115040c15e82bc Mon Sep 17 00:00:00 2001 From: Alessio Perugini Date: Mon, 25 Sep 2023 12:29:18 +0200 Subject: [PATCH 07/14] apply revive linter suggestion about var-declaration --- arduino/discovery/discovery.go | 2 +- arduino/globals/globals.go | 2 +- internal/cli/feedback/feedback.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/arduino/discovery/discovery.go b/arduino/discovery/discovery.go index 1e339076754..2e9946d675c 100644 --- a/arduino/discovery/discovery.go +++ b/arduino/discovery/discovery.go @@ -149,7 +149,7 @@ func (p *Port) Clone() *Port { if p == nil { return nil } - var res Port = *p + res := *p if p.Properties != nil { res.Properties = p.Properties.Clone() } diff --git a/arduino/globals/globals.go b/arduino/globals/globals.go index b00f82c227a..3d255ad60c0 100644 --- a/arduino/globals/globals.go +++ b/arduino/globals/globals.go @@ -19,7 +19,7 @@ var ( empty struct{} // MainFileValidExtension is the extension that must be used for files in new sketches - MainFileValidExtension string = ".ino" + MainFileValidExtension = ".ino" // MainFileValidExtensions lists valid extensions for a sketch file MainFileValidExtensions = map[string]struct{}{ diff --git a/internal/cli/feedback/feedback.go b/internal/cli/feedback/feedback.go index 7a2ff5087a5..f155a5dffaa 100644 --- a/internal/cli/feedback/feedback.go +++ b/internal/cli/feedback/feedback.go @@ -43,7 +43,7 @@ const ( YAML ) -var formats map[string]OutputFormat = map[string]OutputFormat{ +var formats = map[string]OutputFormat{ "json": JSON, "jsonmini": MinifiedJSON, "yaml": YAML, From 216793e21afd12fdb6fb65411ec0f2ec7b871eab Mon Sep 17 00:00:00 2001 From: Alessio Perugini Date: Mon, 25 Sep 2023 12:32:10 +0200 Subject: [PATCH 08/14] apply revive linter suggestion about redefines-builtin-id --- internal/integrationtest/arduino-cli.go | 6 +++--- internal/integrationtest/core/core_test.go | 8 ++++---- internal/integrationtest/upload/upload_test.go | 8 ++++---- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/internal/integrationtest/arduino-cli.go b/internal/integrationtest/arduino-cli.go index 5908358862b..45620ee4c71 100644 --- a/internal/integrationtest/arduino-cli.go +++ b/internal/integrationtest/arduino-cli.go @@ -267,7 +267,7 @@ func (cli *ArduinoCLI) StartDaemon(verbose bool) string { cli.proc = cliProc cli.daemonAddr = "127.0.0.1:50051" - copy := func(dst io.Writer, src io.Reader) { + _copy := func(dst io.Writer, src io.Reader) { buff := make([]byte, 1024) for { n, err := src.Read(buff) @@ -277,8 +277,8 @@ func (cli *ArduinoCLI) StartDaemon(verbose bool) string { dst.Write([]byte(color.YellowString(string(buff[:n])))) } } - go copy(os.Stdout, stdout) - go copy(os.Stderr, stderr) + go _copy(os.Stdout, stdout) + go _copy(os.Stderr, stderr) conn, err := grpc.Dial(cli.daemonAddr, grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithBlock()) cli.t.NoError(err) cli.daemonConn = conn diff --git a/internal/integrationtest/core/core_test.go b/internal/integrationtest/core/core_test.go index ae4330b4b8a..d18fa1a790e 100644 --- a/internal/integrationtest/core/core_test.go +++ b/internal/integrationtest/core/core_test.go @@ -464,7 +464,7 @@ func TestCoreListAllManuallyInstalledCore(t *testing.T) { stdout, _, err := cli.Run("core", "list", "--all", "--format", "json") require.NoError(t, err) requirejson.NotEmpty(t, stdout) - len, err := strconv.Atoi(requirejson.Parse(t, stdout).Query("length").String()) + length, err := strconv.Atoi(requirejson.Parse(t, stdout).Query("length").String()) require.NoError(t, err) // Manually installs a core in sketchbooks hardware folder @@ -479,7 +479,7 @@ func TestCoreListAllManuallyInstalledCore(t *testing.T) { // Verifies manually installed core is shown stdout, _, err = cli.Run("core", "list", "--all", "--format", "json") require.NoError(t, err) - requirejson.Len(t, stdout, len+1) + requirejson.Len(t, stdout, length+1) requirejson.Contains(t, stdout, `[ { "id": "arduino-beta-development:avr", @@ -500,7 +500,7 @@ func TestCoreListUpdatableAllFlags(t *testing.T) { stdout, _, err := cli.Run("core", "list", "--all", "--updatable", "--format", "json") require.NoError(t, err) requirejson.NotEmpty(t, stdout) - len, err := strconv.Atoi(requirejson.Parse(t, stdout).Query("length").String()) + length, err := strconv.Atoi(requirejson.Parse(t, stdout).Query("length").String()) require.NoError(t, err) // Manually installs a core in sketchbooks hardware folder @@ -515,7 +515,7 @@ func TestCoreListUpdatableAllFlags(t *testing.T) { // Verifies using both --updatable and --all flags --all takes precedence stdout, _, err = cli.Run("core", "list", "--all", "--updatable", "--format", "json") require.NoError(t, err) - requirejson.Len(t, stdout, len+1) + requirejson.Len(t, stdout, length+1) requirejson.Contains(t, stdout, `[ { "id": "arduino-beta-development:avr", diff --git a/internal/integrationtest/upload/upload_test.go b/internal/integrationtest/upload/upload_test.go index 7407c5af31e..b3c88cefeb9 100644 --- a/internal/integrationtest/upload/upload_test.go +++ b/internal/integrationtest/upload/upload_test.go @@ -48,9 +48,9 @@ func detectedBoards(t *testing.T, cli *integrationtest.ArduinoCLI) []board { var boards []board stdout, _, err := cli.Run("board", "list", "--format", "json") require.NoError(t, err) - len, err := strconv.Atoi(requirejson.Parse(t, stdout).Query(".[] | .matching_boards | length").String()) + length, err := strconv.Atoi(requirejson.Parse(t, stdout).Query(".[] | .matching_boards | length").String()) require.NoError(t, err) - for i := 0; i < len; i++ { + for i := 0; i < length; i++ { fqbn := strings.Trim(requirejson.Parse(t, stdout).Query(".[] | .matching_boards | .["+fmt.Sprint(i)+"] | .fqbn").String(), "\"") boards = append(boards, board{ address: strings.Trim(requirejson.Parse(t, stdout).Query(".[] | .port | .address").String(), "\""), @@ -69,10 +69,10 @@ func waitForBoard(t *testing.T, cli *integrationtest.ArduinoCLI) { for time.Now().Unix() < timeEnd { stdout, _, err := cli.Run("board", "list", "--format", "json") require.NoError(t, err) - len, err := strconv.Atoi(requirejson.Parse(t, stdout).Query("length").String()) + length, err := strconv.Atoi(requirejson.Parse(t, stdout).Query("length").String()) require.NoError(t, err) numBoards := 0 - for i := 0; i < len; i++ { + for i := 0; i < length; i++ { numBoards, err = strconv.Atoi(requirejson.Parse(t, stdout).Query(".[] | .matching_boards | length").String()) require.NoError(t, err) if numBoards > 0 { From fcc5bbd88f498706889d6447655b24ad88a07bfd Mon Sep 17 00:00:00 2001 From: Alessio Perugini Date: Mon, 25 Sep 2023 12:43:34 +0200 Subject: [PATCH 09/14] apply revive linter suggestion about superfluous-else --- commands/daemon/term_example/main.go | 44 +++++++++++++--------------- commands/debug/debug.go | 6 ++-- 2 files changed, 24 insertions(+), 26 deletions(-) diff --git a/commands/daemon/term_example/main.go b/commands/daemon/term_example/main.go index d7f351a6cd7..16aa36f3d53 100644 --- a/commands/daemon/term_example/main.go +++ b/commands/daemon/term_example/main.go @@ -40,39 +40,37 @@ func main() { // Create and initialize a CLI instance cli := commands.NewArduinoCoreServiceClient(conn) - var instance *commands.Instance - if resp, err := cli.Create(context.Background(), &commands.CreateRequest{}); err != nil { + resp, err := cli.Create(context.Background(), &commands.CreateRequest{}) + if err != nil { log.Fatal("Create:", err) - } else { - instance = resp.Instance } + instance := resp.Instance - if respStream, err := cli.Init(context.Background(), &commands.InitRequest{Instance: instance}); err != nil { + respStream, err := cli.Init(context.Background(), &commands.InitRequest{Instance: instance}) + if err != nil { log.Fatal("Init:", err) - } else { - for { - resp, err := respStream.Recv() - if errors.Is(err, io.EOF) { - break - } - if err != nil { - log.Fatal("Init:", err) - } - fmt.Println(resp) + } + for { + resp, err := respStream.Recv() + if errors.Is(err, io.EOF) { + break + } + if err != nil { + log.Fatal("Init:", err) } + fmt.Println(resp) } // List boards and take the first available port - var port *commands.Port - if resp, err := cli.BoardList(context.Background(), &commands.BoardListRequest{Instance: instance}); err != nil { + respList, err := cli.BoardList(context.Background(), &commands.BoardListRequest{Instance: instance}) + if err != nil { log.Fatal("BoardList:", err) - } else { - ports := resp.GetPorts() - if len(ports) == 0 { - log.Fatal("No port to connect!") - } - port = ports[0].Port } + ports := respList.GetPorts() + if len(ports) == 0 { + log.Fatal("No port to connect!") + } + port := ports[0].Port fmt.Println("Detected port:", port.Label, port.ProtocolLabel) connectToPort(cli, instance, port) diff --git a/commands/debug/debug.go b/commands/debug/debug.go index c3a22e57982..56826f6cb29 100644 --- a/commands/debug/debug.go +++ b/commands/debug/debug.go @@ -91,11 +91,11 @@ func Debug(ctx context.Context, req *dbg.DebugConfigRequest, inStream io.Reader, if interrupt != nil { go func() { for { - if sig, ok := <-interrupt; !ok { + sig, ok := <-interrupt + if !ok { break - } else { - cmd.Signal(sig) } + cmd.Signal(sig) } }() } From 6417d893af865a812da042df5464002378d5d74a Mon Sep 17 00:00:00 2001 From: Alessio Perugini Date: Mon, 25 Sep 2023 14:43:42 +0200 Subject: [PATCH 10/14] apply revive linter suggestion about defer --- .../internal/ctags/ctags_has_issues.go | 104 +++++++++--------- 1 file changed, 51 insertions(+), 53 deletions(-) diff --git a/arduino/builder/internal/preprocessor/internal/ctags/ctags_has_issues.go b/arduino/builder/internal/preprocessor/internal/ctags/ctags_has_issues.go index 0b72abed8d5..371a046b519 100644 --- a/arduino/builder/internal/preprocessor/internal/ctags/ctags_has_issues.go +++ b/arduino/builder/internal/preprocessor/internal/ctags/ctags_has_issues.go @@ -206,78 +206,76 @@ func removeComments(text string, multilinecomment bool) (string, bool) { // FindCLinkageLines scans the source files searching for "extern C" context // It save the line numbers in a map filename -> {lines...} func (p *Parser) FindCLinkageLines(tags []*Tag) map[string][]int { - lines := make(map[string][]int) for _, tag := range tags { - if lines[tag.Filename] != nil { break } file, err := os.Open(tag.Filename) - if err == nil { - defer file.Close() + if err != nil { + continue + } + lines[tag.Filename] = append(lines[tag.Filename], -1) - lines[tag.Filename] = append(lines[tag.Filename], -1) + scanner := bufio.NewScanner(file) - scanner := bufio.NewScanner(file) + // we can't remove the comments otherwise the line number will be wrong + // there are three cases: + // 1 - extern "C" void foo() + // 2 - extern "C" { + // void foo(); + // void bar(); + // } + // 3 - extern "C" + // { + // void foo(); + // void bar(); + // } + // case 1 and 2 can be simply recognized with string matching and indent level count + // case 3 needs specia attention: if the line ONLY contains `extern "C"` string, don't bail out on indent level = 0 + + inScope := false + enteringScope := false + indentLevels := 0 + line := 0 - // we can't remove the comments otherwise the line number will be wrong - // there are three cases: - // 1 - extern "C" void foo() - // 2 - extern "C" { - // void foo(); - // void bar(); - // } - // 3 - extern "C" - // { - // void foo(); - // void bar(); - // } - // case 1 and 2 can be simply recognized with string matching and indent level count - // case 3 needs specia attention: if the line ONLY contains `extern "C"` string, don't bail out on indent level = 0 - - inScope := false - enteringScope := false - indentLevels := 0 - line := 0 - - externCDecl := removeSpacesAndTabs(keywordExternC) - - for scanner.Scan() { - line++ - str := removeSpacesAndTabs(scanner.Text()) + externCDecl := removeSpacesAndTabs(keywordExternC) - if len(str) == 0 { - continue - } + for scanner.Scan() { + line++ + str := removeSpacesAndTabs(scanner.Text()) - // check if we are on the first non empty line after externCDecl in case 3 - if enteringScope { - enteringScope = false - } + if len(str) == 0 { + continue + } - // check if the line contains externCDecl - if strings.Contains(str, externCDecl) { - inScope = true - if len(str) == len(externCDecl) { - // case 3 - enteringScope = true - } - } - if inScope { - lines[tag.Filename] = append(lines[tag.Filename], line) - } - indentLevels += strings.Count(str, "{") - strings.Count(str, "}") + // check if we are on the first non empty line after externCDecl in case 3 + if enteringScope { + enteringScope = false + } - // Bail out if indentLevel is zero and we are not in case 3 - if indentLevels == 0 && !enteringScope { - inScope = false + // check if the line contains externCDecl + if strings.Contains(str, externCDecl) { + inScope = true + if len(str) == len(externCDecl) { + // case 3 + enteringScope = true } } + if inScope { + lines[tag.Filename] = append(lines[tag.Filename], line) + } + indentLevels += strings.Count(str, "{") - strings.Count(str, "}") + + // Bail out if indentLevel is zero and we are not in case 3 + if indentLevels == 0 && !enteringScope { + inScope = false + } } + file.Close() } return lines } From 48d9661a164a00cc7b617876c323c1a5ff594483 Mon Sep 17 00:00:00 2001 From: Alessio Perugini Date: Mon, 25 Sep 2023 15:53:44 +0200 Subject: [PATCH 11/14] use forbidigo to check improper use of fmt.* or os.* function in cli pkg --- .golangci.yml | 33 ++++++++---- internal/cli/cli_test.go | 107 --------------------------------------- 2 files changed, 24 insertions(+), 116 deletions(-) delete mode 100644 internal/cli/cli_test.go diff --git a/.golangci.yml b/.golangci.yml index 4c89b237543..21b811abd9f 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -8,28 +8,36 @@ linters: disable-all: true # Enable specific linter enable: + # Nice to have + #- depguard #- errcheck + #- gocritic + #- thelper - errorlint - exportloopref + - forbidigo - gofmt - goimports + - gosec - gosimple - govet - - revive - - gosec + - ineffassign - misspell + - revive - staticcheck + - tenv - typecheck - unconvert - - ineffassign - - tenv - # Nice to have - #- thelper - #- depguard - #We have to dig deeper - #- gocritic linters-settings: + forbidigo: + forbid: + - p: ^(fmt\.Print(|f|ln)|print|println)$ # Optional message that gets included in error reports. + msg: in cli package use `feedback.*` instead + - p: (os\.(Stdout|Stderr|Stdin))(# )? + msg: in cli package use `feedback.*` instead + analyze-types: true + revive: confidence: 0.8 rules: @@ -119,3 +127,10 @@ issues: - linters: [revive] path: internal/algorithms/channels.go text: "empty-block" + + # Run linters only on specific path + - path-except: internal/cli/ + linters: + - forbidigo + - path: internal/cli/feedback/ + linters: [forbidigo] diff --git a/internal/cli/cli_test.go b/internal/cli/cli_test.go deleted file mode 100644 index 1c3980c2d9f..00000000000 --- a/internal/cli/cli_test.go +++ /dev/null @@ -1,107 +0,0 @@ -// This file is part of arduino-cli. -// -// Copyright 2022 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 cli_test - -import ( - "fmt" - "go/ast" - "go/parser" - "go/token" - "strings" - "testing" - - "github.com/arduino/go-paths-helper" - "github.com/stretchr/testify/require" -) - -func TestNoDirectOutputToStdOut(t *testing.T) { - dirs, err := paths.New(".").ReadDirRecursiveFiltered( - paths.FilterOutNames("testdata"), // skip all testdata folders - paths.AndFilter( - paths.FilterDirectories(), // analyze only packages - paths.FilterOutNames("feedback"), // skip feedback package - )) - require.NoError(t, err) - dirs.Add(paths.New(".")) - - for _, dir := range dirs { - testDir(t, dir) - } -} - -func testDir(t *testing.T, dir *paths.Path) { - fset := token.NewFileSet() - pkgs, err := parser.ParseDir(fset, dir.String(), nil, parser.ParseComments) - require.NoError(t, err) - - for _, pkg := range pkgs { - for _, file := range pkg.Files { - // Do not analyze test files - if strings.HasSuffix(expr(file.Name), "_test") { - continue - } - - ast.Inspect(file, func(n ast.Node) bool { - return inspect(t, fset, n) - }) - } - } -} - -func inspect(t *testing.T, fset *token.FileSet, node ast.Node) bool { - switch n := node.(type) { - case *ast.CallExpr: - name := expr(n.Fun) - if strings.HasPrefix(name, "fmt.P") { - fmt.Printf("%s: function `%s` should not be used in this package (use `feedback.*` instead)\n", fset.Position(n.Pos()), name) - t.Fail() - } - case *ast.SelectorExpr: - wanted := map[string]string{ - "os.Stdout": "%s: object `%s` should not be used in this package (use `feedback.*` instead)\n", - "os.Stderr": "%s: object `%s` should not be used in this package (use `feedback.*` instead)\n", - "os.Stdin": "%s: object `%s` should not be used in this package (use `feedback.*` instead)\n", - "os.Exit": "%s: function `%s` should not be used in this package (use `return` or `feedback.FatalError` instead)\n", - } - name := expr(n) - if msg, banned := wanted[name]; banned { - fmt.Printf(msg, fset.Position(n.Pos()), name) - t.Fail() - } - } - return true -} - -// expr returns the string representation of an expression, it doesn't expand function arguments or array index. -func expr(_e ast.Expr) string { - switch e := _e.(type) { - case *ast.ArrayType: - return "[...]" + expr(e.Elt) - case *ast.CallExpr: - return expr(e.Fun) + "(...)" - case *ast.FuncLit: - return "func(...) {...}" - case *ast.SelectorExpr: - return expr(e.X) + "." + e.Sel.String() - case *ast.IndexExpr: - return expr(e.X) + "[...]" - case *ast.Ident: - return e.String() - default: - msg := fmt.Sprintf("UNKWOWN: %T", e) - panic(msg) - } -} From e36f1e8afadb0270a1dc340a13aef6705061bc1a Mon Sep 17 00:00:00 2001 From: Alessio Perugini Date: Mon, 25 Sep 2023 16:00:26 +0200 Subject: [PATCH 12/14] Remove govet as it's included in golangcilint --- .github/workflows/check-go-task.yml | 33 ----------------------------- Taskfile.yml | 8 ------- 2 files changed, 41 deletions(-) diff --git a/.github/workflows/check-go-task.yml b/.github/workflows/check-go-task.yml index ed5df19a353..89443f77018 100644 --- a/.github/workflows/check-go-task.yml +++ b/.github/workflows/check-go-task.yml @@ -49,39 +49,6 @@ jobs: echo "result=$RESULT" >> $GITHUB_OUTPUT - check-errors: - name: check-errors (${{ matrix.module.path }}) - needs: run-determination - if: needs.run-determination.outputs.result == 'true' - runs-on: ubuntu-latest - - strategy: - fail-fast: false - - matrix: - module: - - path: ./ - - steps: - - name: Checkout repository - uses: actions/checkout@v4 - - - name: Install Go - uses: actions/setup-go@v4 - with: - go-version: ${{ env.GO_VERSION }} - - - name: Install Task - uses: arduino/setup-task@v1 - with: - repo-token: ${{ secrets.GITHUB_TOKEN }} - version: 3.x - - - name: Check for errors - env: - GO_MODULE_PATH: ${{ matrix.module.path }} - run: task go:vet - check-outdated: name: check-outdated (${{ matrix.module.path }}) needs: run-determination diff --git a/Taskfile.yml b/Taskfile.yml index b9427792335..3b538ddd2ad 100755 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -126,13 +126,6 @@ tasks: {{.TEST_LDFLAGS}} go tool covdata textfmt -i=coverage_data -o coverage_integration.txt - # Source: https://github.com/arduino/tooling-project-assets/blob/main/workflow-templates/assets/check-go-task/Taskfile.yml - go:vet: - desc: Check for errors in Go code - dir: '{{default "./" .GO_MODULE_PATH}}' - cmds: - - go vet {{default .DEFAULT_GO_PACKAGES .GO_PACKAGES}} - go:easyjson-generate: desc: Run easyjson code generation cmds: @@ -301,7 +294,6 @@ tasks: check: desc: Check fmt and lint cmds: - - task: go:vet - task: go:lint - task: protoc:check From 6e82de9a55f38b9ae7385ac25b9c78c404177764 Mon Sep 17 00:00:00 2001 From: Alessio Perugini Date: Mon, 25 Sep 2023 16:03:16 +0200 Subject: [PATCH 13/14] run prettier --- .golangci.yml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 21b811abd9f..5afef88c759 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,6 +1,6 @@ run: timeout: 10m - go: '1.21' + go: "1.21" tests: true linters: @@ -32,7 +32,7 @@ linters: linters-settings: forbidigo: forbid: - - p: ^(fmt\.Print(|f|ln)|print|println)$ # Optional message that gets included in error reports. + - p: ^(fmt\.Print(|f|ln)|print|println)$ msg: in cli package use `feedback.*` instead - p: (os\.(Stdout|Stderr|Stdin))(# )? msg: in cli package use `feedback.*` instead @@ -41,9 +41,9 @@ linters-settings: revive: confidence: 0.8 rules: - #- name: error-return - #- name: unused-parameter - #- name: var-naming + #- name: error-return + #- name: unused-parameter + #- name: var-naming - name: blank-imports - name: context-as-argument - name: context-keys-type From 230c2dfbc58ea17e7ac23702588628498d1cc1fb Mon Sep 17 00:00:00 2001 From: Alessio Perugini Date: Mon, 25 Sep 2023 17:00:11 +0200 Subject: [PATCH 14/14] add dupword: checks for duplicate words --- .golangci.yml | 1 + arduino/builder/internal/utils/utils.go | 2 +- arduino/errors.go | 2 +- arduino/libraries/libraries.go | 2 +- internal/cli/updater/updater.go | 2 +- internal/integrationtest/compile_2/compile_test.go | 4 ++-- internal/integrationtest/outdated/outdated_test.go | 2 +- 7 files changed, 8 insertions(+), 7 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 5afef88c759..7e9f18d1482 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -14,6 +14,7 @@ linters: #- gocritic #- thelper - errorlint + - dupword - exportloopref - forbidigo - gofmt diff --git a/arduino/builder/internal/utils/utils.go b/arduino/builder/internal/utils/utils.go index 58f2e08fd3e..497f3de0668 100644 --- a/arduino/builder/internal/utils/utils.go +++ b/arduino/builder/internal/utils/utils.go @@ -277,7 +277,7 @@ func ExecCommand( return verboseInfoBuf.Bytes(), stdoutBuffer.Bytes(), stderrBuffer.Bytes(), errors.WithStack(err) } -// DirContentIsOlderThan DirContentIsOlderThan returns true if the content of the given directory is +// DirContentIsOlderThan returns true if the content of the given directory is // older than target file. If extensions are given, only the files with these // extensions are tested. func DirContentIsOlderThan(dir *paths.Path, target *paths.Path, extensions ...string) (bool, error) { diff --git a/arduino/errors.go b/arduino/errors.go index 0f30a85dd73..c9f81e8f270 100644 --- a/arduino/errors.go +++ b/arduino/errors.go @@ -830,7 +830,7 @@ func (e *SignatureVerificationFailedError) ToRPCStatus() *status.Status { // MultiplePlatformsError is returned when trying to detect // the Platform the user is trying to interact with and -// and multiple results are found. +// multiple results are found. type MultiplePlatformsError struct { Platforms []string UserPlatform string diff --git a/arduino/libraries/libraries.go b/arduino/libraries/libraries.go index 186142360c6..1235023e9e5 100644 --- a/arduino/libraries/libraries.go +++ b/arduino/libraries/libraries.go @@ -106,7 +106,7 @@ func (library *Library) ToRPCLibrary() (*rpc.Library, error) { return p.String() } - // If the the "includes" property is empty or not included in the "library.properties" file + // If the "includes" property is empty or not included in the "library.properties" file // we search for headers by reading the library files directly headers := library.DeclaredHeaders() if len(headers) == 0 { diff --git a/internal/cli/updater/updater.go b/internal/cli/updater/updater.go index bd779b16af1..77e73785de7 100644 --- a/internal/cli/updater/updater.go +++ b/internal/cli/updater/updater.go @@ -124,7 +124,7 @@ func getLatestRelease() string { // Get redirected URL location := res.Request.URL.String() - // The location header points to the the latest release of the CLI, it's supposed to be formatted like this: + // The location header points to the latest release of the CLI, it's supposed to be formatted like this: // https://downloads.arduino.cc/arduino-cli/arduino-cli_0.18.3_Linux_64bit.tar.gz // so we split it to get the version, if there are not enough splits something must have gone wrong. split := strings.Split(location, "_") diff --git a/internal/integrationtest/compile_2/compile_test.go b/internal/integrationtest/compile_2/compile_test.go index 691eb6ffdf0..1c97df59fa5 100644 --- a/internal/integrationtest/compile_2/compile_test.go +++ b/internal/integrationtest/compile_2/compile_test.go @@ -237,7 +237,7 @@ func TestCompileWithConflictingLibrariesInclude(t *testing.T) { } func TestCompileWithEsp32BundledLibraries(t *testing.T) { - // Some esp cores have have bundled libraries that are optimize for that architecture, + // Some esp cores have bundled libraries that are optimize for that architecture, // it might happen that if the user has a library with the same name installed conflicts // can ensue and the wrong library is used for compilation, thus it fails. // This happens because for "historical" reasons these platform have their "name" key @@ -283,7 +283,7 @@ func TestCompileWithEsp32BundledLibraries(t *testing.T) { } func TestCompileWithEsp8266BundledLibraries(t *testing.T) { - // Some esp cores have have bundled libraries that are optimize for that architecture, + // Some esp cores have bundled libraries that are optimize for that architecture, // it might happen that if the user has a library with the same name installed conflicts // can ensue and the wrong library is used for compilation, thus it fails. // This happens because for "historical" reasons these platform have their "name" key diff --git a/internal/integrationtest/outdated/outdated_test.go b/internal/integrationtest/outdated/outdated_test.go index 85773a838d0..3e540209e8e 100644 --- a/internal/integrationtest/outdated/outdated_test.go +++ b/internal/integrationtest/outdated/outdated_test.go @@ -63,7 +63,7 @@ func TestOutdatedUsingLibraryWithInvalidVersion(t *testing.T) { _, _, err := cli.Run("update") require.NoError(t, err) - // Install latest version of a library library + // Install latest version of a library _, _, err = cli.Run("lib", "install", "WiFi101") require.NoError(t, err)