Skip to content

Commit

Permalink
Switched to using staticcheck linter.
Browse files Browse the repository at this point in the history
Previously, we used golangci-lint, but the Go team recommended we use
staticcheck instead.
  • Loading branch information
mwhittaker committed Aug 16, 2023
1 parent cc52b9b commit e23ea68
Show file tree
Hide file tree
Showing 27 changed files with 67 additions and 85 deletions.
6 changes: 3 additions & 3 deletions .github/workflows/go.yml
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,12 @@ jobs:
- name: Cache linter
uses: actions/cache@v3
with:
path: ~/go/bin/golangci-lint
key: golangci-lint-v1.51.2
path: ~/go/bin/staticcheck
key: staticcheck-v0.4.3
if: ${{ matrix.command == 'lint' }}

- name: Install linter
run: go install github.com/golangci/golangci-lint/cmd/[email protected]
run: go install honnef.co/go/tools/cmd/[email protected]
if: ${{ matrix.command == 'lint' }}

- name: Build the weaver binary
Expand Down
28 changes: 0 additions & 28 deletions .golangci.yaml

This file was deleted.

4 changes: 2 additions & 2 deletions cmd/weaver/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func main() {
generateFlags.Usage = func() {
fmt.Fprintln(os.Stderr, generate.Usage)
}
generateFlags.Parse(flag.Args()[1:]) //nolint:errcheck // does os.Exit on error
generateFlags.Parse(flag.Args()[1:])
if err := generate.Generate(".", flag.Args()[1:], generate.Options{}); err != nil {
fmt.Fprintln(os.Stderr, err)
os.Exit(1)
Expand Down Expand Up @@ -109,7 +109,7 @@ Description:
[2]: https://github.blog/2022-02-14-include-diagrams-markdown-files-mermaid/`
flags := flag.NewFlagSet("callgraph", flag.ExitOnError)
flags.Usage = func() { fmt.Fprintln(os.Stderr, usage) }
flags.Parse(flag.Args()[1:]) //nolint:errcheck
flags.Parse(flag.Args()[1:])
if flags.NArg() == 0 {
fmt.Fprintln(os.Stderr, "ERROR: no binary provided.")
}
Expand Down
10 changes: 5 additions & 5 deletions dev/build_and_test
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
# generate -- Generates code
# build -- Builds the module
# vet -- Runs go vet
# lint -- Runs golangci-lint
# lint -- Runs staticcheck
# test -- Tests the module
# testrace -- Tests the module for data races
#
Expand Down Expand Up @@ -53,16 +53,16 @@ function cmd_vet() {
}

function cmd_lint() {
if ! exists golangci-lint; then
printf "golangci-lint not found; install via\ngo install github.com/golangci/golangci-lint/cmd/[email protected]\n" >&2
if ! exists staticcheck; then
printf "staticcheck not found; install via\ngo install honnef.co/go/tools/cmd/[email protected]\n" >&2
exit 1
fi

golangci-lint --timeout=10m run ./...
staticcheck ./...

# Run unused check while skipping uses found in tests. This helps us
# identify non-test code that is only used from tests.
golangci-lint --timeout=10m run --enable unused --tests=false ./...
staticcheck -tests=false ./...
}

function cmd_test() {
Expand Down
2 changes: 0 additions & 2 deletions examples/collatz/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,6 @@ import (

//go:generate ../../cmd/weaver/weaver generate

var localAddr = flag.String("local_addr", "localhost:9000", "Collatz server local address")

func main() {
flag.Parse()
if err := weaver.Run(context.Background(), serve); err != nil {
Expand Down
5 changes: 0 additions & 5 deletions internal/benchmarks/benchmarks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (
"io"
"math/rand"
"testing"
"time"

"github.com/ServiceWeaver/weaver/runtime/codegen"
"github.com/ServiceWeaver/weaver/weavertest"
Expand Down Expand Up @@ -362,10 +361,6 @@ func BenchmarkPing(b *testing.B) {
}
}

func init() {
rand.Seed(time.Now().UnixNano())
}

// TestBenchmark is a test to ensure that BenchmarkPing works correctly.
func TestBenchmark(t *testing.T) {
// Test plan: Send a ping request from Component1 to Component10. Verify that
Expand Down
4 changes: 2 additions & 2 deletions internal/envelope/conn/envelope_conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ func (e *EnvelopeConn) Serve(h EnvelopeHandler) error {
}
})

e.running.Wait() //nolint:errcheck // supplanted by e.err
e.running.Wait()
return e.err
}

Expand Down Expand Up @@ -381,7 +381,7 @@ func checkVersion(v *protos.SemVer) error {
}
got := version.SemVer{Major: int(v.Major), Minor: int(v.Minor), Patch: int(v.Patch)}
if got != version.DeployerVersion {
return fmt.Errorf("version mismatch: deployer's deployer API version %s is incompatible with app' deployer API version %s.", version.DeployerVersion, got)
return fmt.Errorf("version mismatch: deployer's deployer API version %s is incompatible with app' deployer API version %s", version.DeployerVersion, got)
}
return nil
}
1 change: 0 additions & 1 deletion internal/envelope/conn/weavelet_conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,6 @@ func (w *WeaveletConn) handleMessage(handler WeaveletHandler, msg *protos.Envelo
go func() {
data, err := Profile(req)
// Reply with profile data.
//nolint:errcheck //errMsg will be returned on next send
w.conn.send(&protos.WeaveletMsg{
Id: -id,
Error: errstring(err),
Expand Down
6 changes: 3 additions & 3 deletions internal/status/dashboard.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ Flags:
url := "http:https://" + lis.Addr().String()

fmt.Fprintln(os.Stderr, "Dashboard available at:", url)
go browser.OpenURL(url) //nolint:errcheck // browser open is optional
go browser.OpenURL(url)
return http.Serve(lis, nil)
},
}
Expand Down Expand Up @@ -312,7 +312,7 @@ func (d *dashboard) handleMetrics(w http.ResponseWriter, r *http.Request) {

var b bytes.Buffer
imetrics.TranslateMetricsToPrometheusTextFormat(&b, snapshots, reg.Addr, prometheusEndpoint)
w.Write(b.Bytes()) //nolint:errcheck // response write error
w.Write(b.Bytes())
}

// handleTraces handles requests to /traces?id=<deployment id>
Expand Down Expand Up @@ -403,5 +403,5 @@ func (d *dashboard) handleTraceFetch(w http.ResponseWriter, r *http.Request) {
http.Error(w, fmt.Sprintf("cannot encode spans: %v", err), http.StatusInternalServerError)
return
}
w.Write(data) //nolint:errcheck // response write error
w.Write(data)
}
2 changes: 1 addition & 1 deletion internal/status/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,6 @@ func RegisterServer(mux *http.ServeMux, server Server, logger *slog.Logger) {
}
var b bytes.Buffer
imetrics.TranslateMetricsToPrometheusTextFormat(&b, snapshots, r.Host, prometheusEndpoint)
w.Write(b.Bytes()) //nolint:errcheck // response write error
w.Write(b.Bytes())
})
}
12 changes: 6 additions & 6 deletions internal/tool/generate/example/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,18 +61,18 @@ type B interface {

type a struct {
weaver.Implements[A]
b weaver.Ref[B] //nolint:unused
lis1 weaver.Listener `weaver:"renamed_listener"` //nolint:unused
lis2 weaver.Listener //nolint:unused
b weaver.Ref[B] //lint:ignore U1000 Present for code generation.
lis1 weaver.Listener `weaver:"renamed_listener"` //lint:ignore U1000 Present for code generation.
lis2 weaver.Listener //lint:ignore U1000 Present for code generation.
weaver.WithConfig[config]
weaver.WithRouter[router]
}

type b struct {
weaver.Implements[B]
a weaver.Ref[A] //nolint:unused
lis1 weaver.Listener `weaver:"renamed_listener"` //nolint:unused
lis2 weaver.Listener //nolint:unused
a weaver.Ref[A] //lint:ignore U1000 Present for code generation.
lis1 weaver.Listener `weaver:"renamed_listener"` //lint:ignore U1000 Present for code generation.
lis2 weaver.Listener //lint:ignore U1000 Present for code generation.
weaver.WithConfig[config]
weaver.WithRouter[router]
}
Expand Down
2 changes: 1 addition & 1 deletion internal/tool/multi/deployer.go
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ func (d *deployer) computeGroups() error {
//
// REQUIRES: d.mu is NOT held.
func (d *deployer) wait() error {
d.running.Wait() //nolint:errcheck // supplanted by b.err
d.running.Wait()
d.mu.Lock()
defer d.mu.Unlock()
return d.err
Expand Down
2 changes: 1 addition & 1 deletion internal/tool/single/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func deploy(ctx context.Context, args []string) error {
go func() {
<-killed
if cmd.Process != nil {
cmd.Process.Kill() //nolint:errcheck
cmd.Process.Kill()
}
os.Exit(1)
}()
Expand Down
2 changes: 1 addition & 1 deletion internal/weaver/remoteweavelet.go
Original file line number Diff line number Diff line change
Expand Up @@ -533,7 +533,7 @@ func (w *RemoteWeavelet) logger(name string, attrs ...string) *slog.Logger {
},
Write: func(entry *protos.LogEntry) {
// TODO(mwhittaker): Propagate error.
w.conn.SendLogEntry(entry) //nolint:errcheck
w.conn.SendLogEntry(entry)
},
})
}
Expand Down
2 changes: 1 addition & 1 deletion runtime/bin/bin.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import (
// version into a Service Weaver binary. We split declaring and assigning
// versionData to prevent the compiler from erasing it.
//
//nolint:unused
//lint:ignore U1000 See comment above.
var versionData string

func init() {
Expand Down
12 changes: 6 additions & 6 deletions runtime/bin/testprogram/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,18 +29,18 @@ type C interface{}

type app struct {
weaver.Implements[weaver.Main]
a weaver.Ref[A] //nolint:unused // intentionally declared but not used
appLis weaver.Listener //nolint:unused // intentionally declared but not used
a weaver.Ref[A] //lint:ignore U1000 intentionally declared but not used
appLis weaver.Listener //lint:ignore U1000 intentionally declared but not used
}

func (*app) Main(context.Context) error { return nil }

type a struct {
weaver.Implements[A]
b weaver.Ref[B] //nolint:unused // intentionally declared but not used
c weaver.Ref[C] //nolint:unused // intentionally declared but not used
aLis1, aLis2 weaver.Listener //nolint:unused // intentionally declared but not used
unused weaver.Listener `weaver:"aLis3"` //nolint:unused // intentionally declared but not used
b weaver.Ref[B] //lint:ignore U1000 intentionally declared but not used
c weaver.Ref[C] //lint:ignore U1000 intentionally declared but not used
aLis1, aLis2 weaver.Listener //lint:ignore U1000 intentionally declared but not used
unused weaver.Listener `weaver:"aLis3"` //lint:ignore U1000 intentionally declared but not used
}

type b struct {
Expand Down
2 changes: 1 addition & 1 deletion runtime/codegen/registry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ type B interface{}

type aimpl struct {
weaver.Implements[A]
b weaver.Ref[B] //nolint:nolintlint,unused // present just for call graph extraction
b weaver.Ref[B] //lint:ignore U1000 present just for call graph extraction
}

type bimpl struct {
Expand Down
2 changes: 1 addition & 1 deletion runtime/envelope/envelope.go
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ func (e *Envelope) Serve(h EnvelopeHandler) error {
return err
})

running.Wait() //nolint:errcheck // supplanted by stopErr
running.Wait()

// Wait for the weavelet command to finish. This needs to be done after
// we're done reading from stdout/stderr pipes, per comments on
Expand Down
2 changes: 1 addition & 1 deletion runtime/metrics/labels_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ func TestValidLabels(t *testing.T) {

func TestInvalidLabels(t *testing.T) {
type t1 = string // not a struct
type t2 = struct{ x string } //nolint:unused // unexported field
type t2 = struct{ x string } //lint:ignore U1000 unexported field
type t3 = struct { // duplicate field
X string `weaver:"Z"`
Y string `weaver:"Z"`
Expand Down
2 changes: 1 addition & 1 deletion runtime/prometheus/prometheus.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ func writeLabels(w *bytes.Buffer, labels map[string]string,
separator := "{"
for _, l := range sortedLabels {
w.WriteString(separator + l + `="`)
escaper.WriteString(w, labels[l]) //nolint:errcheck // bytes.Buffer.Write does not error
escaper.WriteString(w, labels[l])
w.WriteByte('"')
separator = ","
}
Expand Down
2 changes: 1 addition & 1 deletion runtime/traces/db.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ func (d *DB) Store(ctx context.Context, app, version string, spans *protos.Trace
if err != nil {
return err
}
defer tx.Rollback() //nolint:errcheck // supplanted by errs below
defer tx.Rollback()
var errs []error
for _, span := range spans.Span {
if isRootSpan(span) {
Expand Down
17 changes: 17 additions & 0 deletions staticcheck.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# In addition to the checks disabled by default [1], we also disable the
# following style checks:
#
# - ST1005 [2]. This check enforces that error messages do not begin with a
# capital letter or end with punctuation. However, we have many error
# messages that begin with exported names or proper nouns (e.g.,
# NewEnvelopeConn, Service Weaver).
# - ST1012 [3]. This check enforces that exported errors are prefixed with Err
# (e.g., ErrFoo). weaver.RemoteCallError violates this.
#
# [1]: https://staticcheck.dev/docs/configuration/options/#checks.
# [2]: https://staticcheck.dev/docs/checks/#ST1005
# [3]: https://staticcheck.dev/docs/checks/#ST1012
#
# TODO(mwhittaker): Think about enabling some of these checks and updating our
# code accordingly.
checks = ["all", "-ST1000", "-ST1003", "-ST1005", "-ST1012", "-ST1016", "-ST1020", "-ST1021", "-ST1022"]
8 changes: 5 additions & 3 deletions weaver.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,9 @@ type Implements[T any] struct {
// The component_interface_type field exists to make it possible.
//
// [1]: https://github.com/golang/go/issues/54393.
component_interface_type T //nolint:unused
//
//lint:ignore U1000 See comment above.
component_interface_type T

// We embed implementsImpl so that component implementation structs
// implement the Unrouted interface by default but implement the
Expand Down Expand Up @@ -263,7 +265,7 @@ func (i *Implements[T]) setLogger(logger *slog.Logger) {
// package. It exists so that a component struct that embeds Implements[T]
// implements the InstanceOf[T] interface.
//
//nolint:unused
//lint:ignore U1000 implements is used by InstanceOf.
func (Implements[T]) implements(T) {}

// InstanceOf[T] is the interface implemented by a struct that embeds
Expand Down Expand Up @@ -477,7 +479,7 @@ type WithRouter[T any] struct{}

// routedBy(T) implements the RoutedBy[T] interface.
//
//nolint:unused
//lint:ignore U1000 routedBy is used by RoutedBy and Unrouted.
func (WithRouter[T]) routedBy(T) {}

// RoutedBy[T] is the interface implemented by a struct that embeds
Expand Down
2 changes: 1 addition & 1 deletion weavertest/deployer.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ func (d *deployer) stopLocked(err error) {
// cleanup cleans up all of the running envelopes' state.
func (d *deployer) cleanup() error {
d.ctxCancel()
d.running.Wait() //nolint:errcheck // supplanted by b.err
d.running.Wait()
d.mu.Lock()
defer d.mu.Unlock()
return d.err
Expand Down
1 change: 0 additions & 1 deletion weavertest/internal/simple/simple.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,6 @@ type server struct {
}

func (s *server) Init(ctx context.Context) error {
//nolint:nolintlint,typecheck // golangci-lint false positive on Go tip
s.addr = s.hello.String()
s.proxy = s.hello.ProxyAddr()

Expand Down
Loading

0 comments on commit e23ea68

Please sign in to comment.