Skip to content

Commit

Permalink
all: re-enable staticcheck rule ST1012 (#3381)
Browse files Browse the repository at this point in the history
Rename error variables and re-enable staticheck
[ST1012 - Poorly chosen name for error variable](https://staticcheck.dev/docs/checks/#ST1012) rule.

All renamed variables are non-exported except errInvalidArgs in predicates/source
which was made unexported.

Follow up on #897 and #1642

Signed-off-by: Alexander Yastrebov <[email protected]>
  • Loading branch information
AlexanderYastrebov authored Jan 20, 2025
1 parent 017f25d commit 9bbfe3a
Show file tree
Hide file tree
Showing 21 changed files with 94 additions and 109 deletions.
4 changes: 1 addition & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -130,12 +130,10 @@ vet: $(SOURCES) ## run Go vet
# TODO(sszuecs) review disabling these checks, f.e.:
# -ST1000 missing package doc in many packages
# -ST1003 wrong naming convention Api vs API, Id vs ID
# -ST1012 too many error variables are not having prefix "err"
# -ST1020 too many wrong comments on exported functions to fix right away
# -ST1021 too many wrong comments on exported functions to fix right away
# -ST1022 too many wrong comments on exported functions to fix right away
staticcheck: $(SOURCES) ## run staticcheck
staticcheck -checks "all,-ST1000,-ST1003,-ST1012,-ST1020,-ST1021" ./...
staticcheck -checks "all,-ST1000,-ST1003,-ST1020,-ST1021" ./...

.PHONY: gosec
# TODO(sszuecs) review disabling these checks, f.e.:
Expand Down
12 changes: 6 additions & 6 deletions cmd/eskip/args.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,9 @@ func (w *noopWriter) Write(b []byte) (int, error) {
}

var (
invalidNumberOfArgs = errors.New("invalid number of args")
missingOAuthToken = errors.New("missing OAuth token")
invalidIndentStr = errors.New("invalid indent. Must match regexp \\s")
errInvalidNumberOfArgs = errors.New("invalid number of args")
errMissingOAuthToken = errors.New("missing OAuth token")
errInvalidIndentStr = errors.New("invalid indent. Must match regexp \\s")
)

// parsing vars for flags:
Expand Down Expand Up @@ -175,7 +175,7 @@ func processInnkeeperArgs(innkeeperUrl, oauthToken string) (*medium, error) {
}

if oauthToken == "" {
return nil, missingOAuthToken
return nil, errMissingOAuthToken
}

if innkeeperUrl == "" {
Expand All @@ -197,7 +197,7 @@ func processInnkeeperArgs(innkeeperUrl, oauthToken string) (*medium, error) {
func processFileArg() (*medium, error) {
nonFlagArgs := flags.Args()
if len(nonFlagArgs) > 1 {
return nil, invalidNumberOfArgs
return nil, errInvalidNumberOfArgs
}

if len(nonFlagArgs) == 0 {
Expand All @@ -212,7 +212,7 @@ func processFileArg() (*medium, error) {
// if pretty print then check that indent matches pattern
func processIndentStr() error {
if pretty && !(regexp.MustCompile(`^[\s]*$`).MatchString(indentStr)) {
return invalidIndentStr
return errInvalidIndentStr
}
return nil

Expand Down
4 changes: 2 additions & 2 deletions cmd/eskip/args_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ func TestProcessArgs(t *testing.T) {
// innkeeper-url missing token
[]string{"-innkeeper-url", "https://innkeeper.example.org"},
true,
missingOAuthToken,
errMissingOAuthToken,
nil,
}, {

Expand Down Expand Up @@ -166,7 +166,7 @@ func TestProcessArgs(t *testing.T) {
// too many files
[]string{"file1", "file2"},
true,
invalidNumberOfArgs,
errInvalidNumberOfArgs,
nil,
}, {

Expand Down
10 changes: 5 additions & 5 deletions cmd/eskip/eskip.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ var commands = map[command]commandFunc{
ver: versionCmd}

var (
missingCommand = errors.New("missing command")
invalidCommand = errors.New("invalid command")
errMissingCommand = errors.New("missing command")
errInvalidCommand = errors.New("invalid command")
)

var stdout io.Writer = os.Stdout
Expand Down Expand Up @@ -90,18 +90,18 @@ func exit(err error) { exitErrHint(err, false) }
// second argument must be the ('sub') command.
func getCommand(args []string) (command, error) {
if len(args) < 2 {
return "", missingCommand
return "", errMissingCommand
}

cmd := command(args[1])
if cmd[0] == '-' {
return "", missingCommand
return "", errMissingCommand
}

if _, ok := commands[cmd]; ok {
return cmd, nil
} else {
return "", invalidCommand
return "", errInvalidCommand
}
}

Expand Down
4 changes: 2 additions & 2 deletions cmd/eskip/eskip_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,14 @@ func TestGetCommandSuccess(t *testing.T) {

func TestGetCommandFail(t *testing.T) {
_, err := getCommand([]string{"some", "hello"})
if err != invalidCommand {
if err != errInvalidCommand {
t.Error("hello is an invalid command")
}
}

func TestGetCommandEmpty(t *testing.T) {
_, err := getCommand([]string{"some"})
if err != missingCommand {
if err != errMissingCommand {
t.Error("empty should fail ")
}
}
6 changes: 3 additions & 3 deletions cmd/eskip/load.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ type loadResult struct {
parseErrors map[string]error
}

var invalidRouteExpression = errors.New("one or more invalid route expressions")
var errInvalidRouteExpression = errors.New("one or more invalid route expressions")

// store all loaded routes, even if invalid, and store the
// parse errors if any.
Expand Down Expand Up @@ -67,7 +67,7 @@ func checkParseErrors(lr loadResult) error {
printStderr(id, perr)
}

return invalidRouteExpression
return errInvalidRouteExpression
}

// load, parse routes and print parse errors if any.
Expand Down Expand Up @@ -146,7 +146,7 @@ func printCmd(a cmdArgs) error {
}

if len(lr.parseErrors) > 0 {
return invalidRouteExpression
return errInvalidRouteExpression
}

return nil
Expand Down
11 changes: 2 additions & 9 deletions cmd/eskip/load_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ package main

import (
"bytes"
"errors"
"os"
"strings"
"testing"
Expand All @@ -26,8 +25,6 @@ import (

const testStdinName = "testStdin"

var ioError = errors.New("io error")

func preserveStdin(f *os.File, action func()) {
f, os.Stdin = os.Stdin, f
defer func() { os.Stdin = f }()
Expand Down Expand Up @@ -61,11 +58,7 @@ func withFile(name string, content string, action func(f *os.File)) error {

withError(func() { err = os.Remove(name) })

if err == nil {
return nil
}

return ioError
return err
}

func withStdin(content string, action func()) error {
Expand Down Expand Up @@ -173,7 +166,7 @@ func TestCheckEtcdInvalid(t *testing.T) {
}

err = checkCmd(cmdArgs{in: &medium{typ: etcd, urls: urls, path: "/skippertest"}})
if err != invalidRouteExpression {
if err != errInvalidRouteExpression {
t.Error("failed to fail properly")
}
}
Expand Down
30 changes: 15 additions & 15 deletions cmd/eskip/media.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,17 +57,17 @@ type medium struct {
}

var (
tooManyInputs = errors.New("too many inputs")
invalidInputType = errors.New("invalid input type")
missingInput = errors.New("missing input")
errTooManyInputs = errors.New("too many inputs")
errInvalidInputType = errors.New("invalid input type")
errMissingInput = errors.New("missing input")
)

// validate medium from args, and check if it can be used
// as input.
// (check and print)
func validateSelectRead(media []*medium) (a cmdArgs, err error) {
if len(media) > 1 {
err = tooManyInputs
err = errTooManyInputs
return
}

Expand All @@ -78,7 +78,7 @@ func validateSelectRead(media []*medium) (a cmdArgs, err error) {

switch media[0].typ {
case inlineIds, patchPrepend, patchPrependFile, patchAppend, patchAppendFile:
err = invalidInputType
err = errInvalidInputType
return
}

Expand All @@ -89,19 +89,19 @@ func validateSelectRead(media []*medium) (a cmdArgs, err error) {
// validate media from args, and check if input was specified.
func validateSelectWrite(media []*medium) (a cmdArgs, err error) {
if len(media) == 0 {
err = missingInput
err = errMissingInput
return
}

if len(media) > 2 {
err = tooManyInputs
err = errTooManyInputs
return
}

for _, m := range media {
switch media[0].typ {
case inlineIds, patchPrepend, patchPrependFile, patchAppend, patchAppendFile:
err = invalidInputType
err = errInvalidInputType
return
}

Expand All @@ -113,27 +113,27 @@ func validateSelectWrite(media []*medium) (a cmdArgs, err error) {
}

if a.in == nil {
err = missingInput
err = errMissingInput
}

return
}

func validateSelectDelete(media []*medium) (a cmdArgs, err error) {
if len(media) == 0 {
err = missingInput
err = errMissingInput
return
}

if len(media) > 2 {
err = tooManyInputs
err = errTooManyInputs
return
}

for _, m := range media {
switch media[0].typ {
case patchPrepend, patchPrependFile, patchAppend, patchAppendFile:
err = invalidInputType
err = errInvalidInputType
return
}

Expand All @@ -145,7 +145,7 @@ func validateSelectDelete(media []*medium) (a cmdArgs, err error) {
}

if a.in == nil {
err = missingInput
err = errMissingInput
}

return
Expand All @@ -156,11 +156,11 @@ func validateSelectPatch(media []*medium) (a cmdArgs, err error) {
switch m.typ {
case patchPrepend, patchPrependFile, patchAppend, patchAppendFile:
case inlineIds:
err = invalidInputType
err = errInvalidInputType
return
default:
if a.in != nil {
err = tooManyInputs
err = errTooManyInputs
return
}

Expand Down
14 changes: 7 additions & 7 deletions cmd/eskip/media_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func TestValidateSelectMedia(t *testing.T) {
"check",
[]*medium{{}, {}},
true,
tooManyInputs,
errTooManyInputs,
nil,
nil,
}, {
Expand All @@ -78,7 +78,7 @@ func TestValidateSelectMedia(t *testing.T) {
"check",
[]*medium{{typ: inlineIds}},
true,
invalidInputType,
errInvalidInputType,
nil,
nil,
}, {
Expand Down Expand Up @@ -119,7 +119,7 @@ func TestValidateSelectMedia(t *testing.T) {
"upsert",
nil,
true,
missingInput,
errMissingInput,
nil,
nil,
}, {
Expand All @@ -128,7 +128,7 @@ func TestValidateSelectMedia(t *testing.T) {
"upsert",
[]*medium{{typ: stdin}, {typ: file}, {typ: etcd}},
true,
tooManyInputs,
errTooManyInputs,
nil,
nil,
}, {
Expand All @@ -137,7 +137,7 @@ func TestValidateSelectMedia(t *testing.T) {
"upsert",
[]*medium{{typ: inlineIds}},
true,
invalidInputType,
errInvalidInputType,
nil,
nil,
}, {
Expand All @@ -155,7 +155,7 @@ func TestValidateSelectMedia(t *testing.T) {
"delete",
[]*medium{{typ: innkeeper}},
true,
missingInput,
errMissingInput,
nil,
nil,
}, {
Expand All @@ -173,7 +173,7 @@ func TestValidateSelectMedia(t *testing.T) {
"upsert",
[]*medium{{typ: inlineIds}},
true,
invalidInputType,
errInvalidInputType,
nil,
nil,
}, {
Expand Down
2 changes: 1 addition & 1 deletion cmd/eskip/readclient.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func createReadClient(m *medium) (readClient, error) {
return &idsReader{ids: m.ids}, nil

default:
return nil, invalidInputType
return nil, errInvalidInputType
}
}

Expand Down
4 changes: 2 additions & 2 deletions cmd/eskip/writeclient.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ type writeClient interface {
DeleteAllIf(routes []*eskip.Route, cond eskip.RoutePredicate) error
}

var invalidOutput = errors.New("invalid output")
var errInvalidOutput = errors.New("invalid output")

func createWriteClient(out *medium) (writeClient, error) {
// no output, no client
Expand All @@ -29,5 +29,5 @@ func createWriteClient(out *medium) (writeClient, error) {
Insecure: insecure,
OAuthToken: out.oauthToken})
}
return nil, invalidOutput
return nil, errInvalidOutput
}
Loading

0 comments on commit 9bbfe3a

Please sign in to comment.