Skip to content

Commit 2a1c347

Browse files
feat: add lint command with go-errorlint analyzer (#693)
- Add new 'orchestrion lint' command - Use multichecker framework to run static analysis checks - Include custom help formatting to maintain Orchestrion CLI consistency - Add go-errorlint as the first linter - Enable comparison and asserts checks by default for error handling validation Needs polyfloyd/go-errorlint#107 before merging. Signed-off-by: Kemal Akkoyun <[email protected]> --------- Signed-off-by: Kemal Akkoyun <[email protected]> Co-authored-by: dd-k9-library-go[bot] <214463715+dd-k9-library-go[bot]@users.noreply.github.com>
1 parent 6d4d01e commit 2a1c347

File tree

13 files changed

+244
-1
lines changed

13 files changed

+244
-1
lines changed

LICENSE-3rdparty.csv

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ orchestrion,github.com/outcaste-io/ristretto/z,MIT,unknown
6666
orchestrion,github.com/philhofer/fwd,MIT,"Copyright (c) 2014-2015, Philip Hofer"
6767
orchestrion,github.com/pkg/errors,BSD-2-Clause,"Copyright (c) 2015, Dave Cheney <[email protected]>"
6868
orchestrion,github.com/planetscale/vtprotobuf/protohelpers,BSD-3-Clause,"Copyright (c) 2021, PlanetScale Inc. All rights reserved. | Copyright (c) 2013, The GoGo Authors. All rights reserved. | Copyright (c) 2018 The Go Authors. All rights reserved."
69+
orchestrion,github.com/polyfloyd/go-errorlint/errorlint,MIT,Copyright (c) 2019 polyfloyd
6970
orchestrion,github.com/puzpuzpuz/xsync/v3,Apache-2.0,unknown
7071
orchestrion,github.com/rivo/uniseg,MIT,Copyright (c) 2019 Oliver Kuederle
7172
orchestrion,github.com/rs/zerolog,MIT,Copyright (c) 2017 Olivier Poitrey

_docs/generator/tools.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ import (
5353
_ "github.com/twitchtv/twirp"
5454
_ "github.com/valkey-io/valkey-go"
5555
_ "go.mongodb.org/mongo-driver/mongo/options"
56+
_ "go.mongodb.org/mongo-driver/v2/mongo/options"
5657
_ "google.golang.org/grpc"
5758
_ "gorm.io/gorm"
5859
_ "k8s.io/client-go/rest"

_docs/go.mod

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ require (
5050
github.com/twitchtv/twirp v8.1.3+incompatible
5151
github.com/valkey-io/valkey-go v1.0.64
5252
go.mongodb.org/mongo-driver v1.17.4
53+
go.mongodb.org/mongo-driver/v2 v2.3.0
5354
golang.org/x/tools v0.36.0
5455
google.golang.org/grpc v1.75.0
5556
gorm.io/gorm v1.30.3
@@ -333,6 +334,7 @@ require (
333334
github.com/pkg/errors v0.9.1 // indirect
334335
github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10 // indirect
335336
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
337+
github.com/polyfloyd/go-errorlint v1.8.1-0.20250906200200-9b25878c4dea // indirect
336338
github.com/power-devops/perfstat v0.0.0-20240221224432-82ca36839d55 // indirect
337339
github.com/prometheus/common v0.62.0 // indirect
338340
github.com/puzpuzpuz/xsync/v3 v3.5.1 // indirect
@@ -380,7 +382,6 @@ require (
380382
github.com/yuin/goldmark-emoji v1.0.4 // indirect
381383
github.com/yusufpapurcu/wmi v1.2.4 // indirect
382384
github.com/zeebo/errs v1.4.0 // indirect
383-
go.mongodb.org/mongo-driver/v2 v2.3.0 // indirect
384385
go.opencensus.io v0.24.0 // indirect
385386
go.opentelemetry.io/auto/sdk v1.1.0 // indirect
386387
go.opentelemetry.io/collector/component v1.40.0 // indirect

_docs/go.sum

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1072,6 +1072,8 @@ github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10/go.mod h1
10721072
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
10731073
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRIccs7FGNTlIRMkT8wgtp5eCXdBlqhYGL6U=
10741074
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
1075+
github.com/polyfloyd/go-errorlint v1.8.1-0.20250906200200-9b25878c4dea h1:0hdZfdf74rU/bEndy2uZJyNeY21jmH751KmdAjOmEiA=
1076+
github.com/polyfloyd/go-errorlint v1.8.1-0.20250906200200-9b25878c4dea/go.mod h1:msT1JMnFNM1gqj7rtZYaA0EtpIYNeLQSsKJChZNA+5A=
10751077
github.com/power-devops/perfstat v0.0.0-20240221224432-82ca36839d55 h1:o4JXh1EVt9k/+g42oCprj/FisM4qX9L3sZB3upGN2ZU=
10761078
github.com/power-devops/perfstat v0.0.0-20240221224432-82ca36839d55/go.mod h1:OmDBASR4679mdNQnz2pUhc2G8CO2JrUAVFDRBDP/hJE=
10771079
github.com/prashantv/gostub v1.1.0 h1:BTyx3RfQjRHnUWaGF9oQos79AlQ5k8WNktv7VGvVH4g=

go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ require (
1515
github.com/nats-io/nats-server/v2 v2.11.8
1616
github.com/nats-io/nats.go v1.45.0
1717
github.com/otiai10/copy v1.14.1
18+
github.com/polyfloyd/go-errorlint v1.8.1-0.20250906200200-9b25878c4dea
1819
github.com/rs/zerolog v1.34.0
1920
github.com/santhosh-tekuri/jsonschema/v6 v6.0.2
2021
github.com/shirou/gopsutil/v4 v4.25.8

go.sum

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,8 @@ github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10/go.mod h1
185185
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
186186
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRIccs7FGNTlIRMkT8wgtp5eCXdBlqhYGL6U=
187187
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
188+
github.com/polyfloyd/go-errorlint v1.8.1-0.20250906200200-9b25878c4dea h1:0hdZfdf74rU/bEndy2uZJyNeY21jmH751KmdAjOmEiA=
189+
github.com/polyfloyd/go-errorlint v1.8.1-0.20250906200200-9b25878c4dea/go.mod h1:msT1JMnFNM1gqj7rtZYaA0EtpIYNeLQSsKJChZNA+5A=
188190
github.com/power-devops/perfstat v0.0.0-20240221224432-82ca36839d55 h1:o4JXh1EVt9k/+g42oCprj/FisM4qX9L3sZB3upGN2ZU=
189191
github.com/power-devops/perfstat v0.0.0-20240221224432-82ca36839d55/go.mod h1:OmDBASR4679mdNQnz2pUhc2G8CO2JrUAVFDRBDP/hJE=
190192
github.com/puzpuzpuz/xsync/v3 v3.5.1 h1:GJYJZwO6IdxN/IKbneznS6yPkVC+c3zyY/j19c++5Fg=

instrument/go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,7 @@ require (
221221
github.com/pkg/errors v0.9.1 // indirect
222222
github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10 // indirect
223223
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect
224+
github.com/polyfloyd/go-errorlint v1.8.1-0.20250906200200-9b25878c4dea // indirect
224225
github.com/power-devops/perfstat v0.0.0-20240221224432-82ca36839d55 // indirect
225226
github.com/puzpuzpuz/xsync/v3 v3.5.1 // indirect
226227
github.com/rcrowley/go-metrics v0.0.0-20250401214520-65e299d6c5c9 // indirect

instrument/go.sum

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -793,6 +793,8 @@ github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10/go.mod h1
793793
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
794794
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRIccs7FGNTlIRMkT8wgtp5eCXdBlqhYGL6U=
795795
github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
796+
github.com/polyfloyd/go-errorlint v1.8.1-0.20250906200200-9b25878c4dea h1:0hdZfdf74rU/bEndy2uZJyNeY21jmH751KmdAjOmEiA=
797+
github.com/polyfloyd/go-errorlint v1.8.1-0.20250906200200-9b25878c4dea/go.mod h1:msT1JMnFNM1gqj7rtZYaA0EtpIYNeLQSsKJChZNA+5A=
796798
github.com/power-devops/perfstat v0.0.0-20240221224432-82ca36839d55 h1:o4JXh1EVt9k/+g42oCprj/FisM4qX9L3sZB3upGN2ZU=
797799
github.com/power-devops/perfstat v0.0.0-20240221224432-82ca36839d55/go.mod h1:OmDBASR4679mdNQnz2pUhc2G8CO2JrUAVFDRBDP/hJE=
798800
github.com/prometheus/client_golang v1.17.0 h1:rl2sfwZMtSthVU752MqfjQozy7blglC+1SOtjMAMh+Q=

internal/cmd/lint.go

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
// Unless explicitly stated otherwise all files in this repository are licensed
2+
// under the Apache License Version 2.0.
3+
// This product includes software developed at Datadog (https://www.datadoghq.com/).
4+
// Copyright 2023-present Datadog, Inc.
5+
6+
package cmd
7+
8+
import (
9+
"fmt"
10+
"os"
11+
"slices"
12+
"strings"
13+
"text/template"
14+
15+
"github.com/DataDog/dd-trace-go/v2/ddtrace/tracer"
16+
"github.com/polyfloyd/go-errorlint/errorlint"
17+
"github.com/urfave/cli/v2"
18+
"golang.org/x/tools/go/analysis"
19+
"golang.org/x/tools/go/analysis/multichecker"
20+
)
21+
22+
var Lint = &cli.Command{
23+
Name: "lint",
24+
Usage: "Run selected static analysis checks on Go code for Orchestrion to work better for certain features.",
25+
UsageText: "orchestrion lint [lint arguments...]",
26+
Args: true,
27+
SkipFlagParsing: true,
28+
Action: func(clictx *cli.Context) (err error) {
29+
span, _ := tracer.StartSpanFromContext(clictx.Context, "lint",
30+
tracer.ResourceName(strings.Join(clictx.Args().Slice(), " ")),
31+
)
32+
defer func() { span.Finish(tracer.WithError(err)) }()
33+
34+
// Check if help was requested and print Orchestrion-style header.
35+
args := clictx.Args().Slice()
36+
if slices.Contains(args, "-help") || slices.Contains(args, "--help") || slices.Contains(args, "-h") {
37+
tmpl := template.Must(template.New("help").Parse(cli.CommandHelpTemplate))
38+
if err := tmpl.Execute(os.Stdout, clictx.Command); err != nil {
39+
fmt.Printf("NAME:\n orchestrion lint - %s\n\n", clictx.Command.Usage)
40+
fmt.Printf("USAGE:\n %s\n\n", clictx.Command.UsageText)
41+
fmt.Println()
42+
}
43+
}
44+
45+
// Set up os.Args to include the lint subcommand args.
46+
// Replace "orchestrion lint" with "orchestrion-lint",
47+
// so multichecker sees proper args
48+
args = append([]string{"orchestrion-lint"}, args...)
49+
os.Args = args
50+
51+
// Run multichecker. This will take over with its own flags.
52+
analyzers := []*analysis.Analyzer{
53+
errorlint.NewAnalyzer(
54+
errorlint.WithComparison(true),
55+
errorlint.WithAsserts(true),
56+
),
57+
}
58+
multichecker.Main(analyzers...)
59+
60+
return nil
61+
},
62+
}

internal/cmd/lint_test.go

Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
// Unless explicitly stated otherwise all files in this repository are licensed
2+
// under the Apache License Version 2.0.
3+
// This product includes software developed at Datadog (https://www.datadoghq.com/).
4+
// Copyright 2023-present Datadog, Inc.
5+
6+
package cmd_test
7+
8+
import (
9+
"bytes"
10+
"context"
11+
"flag"
12+
"os"
13+
"testing"
14+
15+
"github.com/DataDog/orchestrion/internal/cmd"
16+
"github.com/stretchr/testify/require"
17+
"github.com/urfave/cli/v2"
18+
)
19+
20+
func TestLint(t *testing.T) {
21+
// Save original os.Args to restore after tests
22+
originalArgs := os.Args
23+
defer func() { os.Args = originalArgs }()
24+
25+
t.Run("help flags", func(t *testing.T) {
26+
helpTests := []struct {
27+
name string
28+
args []string
29+
}{
30+
{"short help", []string{"-h"}},
31+
{"long help", []string{"--help"}},
32+
{"help flag", []string{"-help"}},
33+
}
34+
35+
for _, tt := range helpTests {
36+
t.Run(tt.name, func(t *testing.T) {
37+
var output bytes.Buffer
38+
set := flag.NewFlagSet("test", flag.ContinueOnError)
39+
set.Parse(tt.args)
40+
ctx := cli.NewContext(&cli.App{Writer: &output}, set, nil)
41+
ctx.Command = cmd.Lint
42+
43+
// Since multichecker.Main() will exit the program, we need to handle this carefully
44+
// For help flags, the command should display help and call multichecker.Main()
45+
// We can't easily test the multichecker.Main() call without it exiting
46+
// So we'll focus on testing the help output preparation
47+
48+
// Set up arguments that include help
49+
testArgs := append([]string{"orchestrion", "lint"}, tt.args...)
50+
os.Args = testArgs
51+
52+
// The lint command will call multichecker.Main() which exits
53+
// We can't easily test this without complex mocking
54+
// Instead, let's verify the command structure and help template handling
55+
56+
require.NotNil(t, cmd.Lint.Action)
57+
require.Equal(t, "lint", cmd.Lint.Name)
58+
require.Equal(t, "Run selected static analysis checks on Go code for Orchestrion to work better for certain features.", cmd.Lint.Usage)
59+
require.True(t, cmd.Lint.SkipFlagParsing)
60+
})
61+
}
62+
})
63+
64+
t.Run("command configuration", func(t *testing.T) {
65+
// Test command properties
66+
require.Equal(t, "lint", cmd.Lint.Name)
67+
require.Equal(t, "Run selected static analysis checks on Go code for Orchestrion to work better for certain features.", cmd.Lint.Usage)
68+
require.Equal(t, "orchestrion lint [lint arguments...]", cmd.Lint.UsageText)
69+
require.True(t, cmd.Lint.Args)
70+
require.True(t, cmd.Lint.SkipFlagParsing)
71+
require.NotNil(t, cmd.Lint.Action)
72+
})
73+
74+
t.Run("os.Args manipulation", func(t *testing.T) {
75+
// Test that os.Args gets properly modified for multichecker
76+
77+
// We can't easily test the full execution without multichecker.Main() exiting
78+
// But we can verify the argument preparation logic
79+
80+
args := []string{"-checks=all", "./..."}
81+
expectedArgs := append([]string{"orchestrion-lint"}, args...)
82+
83+
require.Equal(t, []string{"orchestrion-lint", "-checks=all", "./..."}, expectedArgs)
84+
})
85+
86+
t.Run("context with tracing", func(t *testing.T) {
87+
// Test that the command can be called with a context (for tracing)
88+
var output bytes.Buffer
89+
set := flag.NewFlagSet("test", flag.ContinueOnError)
90+
set.Parse([]string{"./..."})
91+
92+
app := &cli.App{Writer: &output}
93+
ctx := cli.NewContext(app, set, nil)
94+
ctx.Context = context.Background()
95+
ctx.Command = cmd.Lint
96+
97+
// Verify command is set up with tracing context
98+
require.NotNil(t, ctx.Context)
99+
require.Equal(t, cmd.Lint, ctx.Command)
100+
})
101+
102+
t.Run("analyzer configuration", func(t *testing.T) {
103+
// While we can't directly test the analyzer setup due to multichecker.Main() exiting,
104+
// we can verify that the command is properly structured to use go-errorlint
105+
106+
// The lint command should be configured to use errorlint analyzer with:
107+
// - WithComparison(true)
108+
// - WithAsserts(true)
109+
110+
// This is validated by the command existing and being properly configured
111+
require.NotNil(t, cmd.Lint)
112+
require.NotNil(t, cmd.Lint.Action)
113+
})
114+
}
115+
116+
func TestLintIntegration(t *testing.T) {
117+
t.Run("help flag functionality", func(t *testing.T) {
118+
// Create a more realistic test to verify help flags are detected
119+
helpFlags := [][]string{
120+
{"-h"},
121+
{"--help"},
122+
{"-help"},
123+
{"./...", "-h"}, // help mixed with other args
124+
}
125+
126+
for _, args := range helpFlags {
127+
// Test that help flags are properly detected in argument slices
128+
containsHelp := containsHelpFlag(args)
129+
130+
hasHelpFlag := false
131+
for _, arg := range args {
132+
if arg == "-h" || arg == "--help" || arg == "-help" {
133+
hasHelpFlag = true
134+
break
135+
}
136+
}
137+
138+
require.Equal(t, hasHelpFlag, containsHelp)
139+
}
140+
})
141+
142+
t.Run("command execution setup", func(t *testing.T) {
143+
// Test the command setup process that would happen before multichecker.Main()
144+
originalArgs := []string{"orchestrion", "lint", "-checks=all", "./..."}
145+
146+
// Simulate the argument transformation from the lint command
147+
args := originalArgs[2:] // Remove "orchestrion lint"
148+
modifiedArgs := append([]string{"orchestrion-lint"}, args...)
149+
150+
// Verify the transformation
151+
require.Equal(t, "orchestrion-lint", modifiedArgs[0])
152+
require.Contains(t, modifiedArgs, "-checks=all")
153+
require.Contains(t, modifiedArgs, "./...")
154+
require.Len(t, modifiedArgs, 3)
155+
})
156+
}
157+
158+
// Helper function to simulate help flag detection logic
159+
func containsHelpFlag(args []string) bool {
160+
for _, arg := range args {
161+
if arg == "-h" || arg == "--help" || arg == "-help" {
162+
return true
163+
}
164+
}
165+
return false
166+
}

0 commit comments

Comments
 (0)