Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: allow to specify Go version #31

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@ go vet -vettool=(which tenv) ./...
./main_test.go:11:2: os.Setenv() can be replaced by `t.Setenv()` in TestMain
```

### option
### options

The option `all` will run against whole test files (`_test.go`) regardless of method/function signatures.
The option `all` will run against whole test files (`_test.go`) regardless of method/function signatures.

By default, only methods that take `*testing.T`, `*testing.B`, and `testing.TB` as arguments are checked.

Expand Down Expand Up @@ -75,13 +75,20 @@ func helper() {
```

```console
go vet -vettool=(which tenv) -tenv.all ./...
go vet -vettool=$(which tenv) -tenv.all ./...

# a
./main_test.go:11:2: os.Setenv() can be replaced by `t.Setenv()` in TestMain
./main_test.go:19:2: os.Setenv() can be replaced by `testing.Setenv()` in helper
```

The option `go` allows to specify Go version. If the version is not empty or lower than Go 1.17, the analysis will be skipped.

```console
go vet -vettool=$(which tenv) -tenv.go 1.16 ./...

Outputs nothing because specified Go 1.16 is lower than 1.17.

## CI

### CircleCI
Expand Down
47 changes: 47 additions & 0 deletions tenv.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
package tenv

import (
"errors"
"fmt"
"go/ast"
"strconv"
"strings"

"golang.org/x/tools/go/analysis"
Expand All @@ -24,13 +27,27 @@ var Analyzer = &analysis.Analyzer{
var (
A = "all"
aflag bool

Go = "go"
goflag string
)

func init() {
Analyzer.Flags.BoolVar(&aflag, A, false, "the all option will run against all method in test file")
Analyzer.Flags.StringVar(&goflag, Go, "1.17", "Go version")
}

func run(pass *analysis.Pass) (interface{}, error) {
lower, err := goVersionLower117(goflag)
if err != nil {
return nil, err
}

if lower {
// Do nothing because T.Setenv added in go1.17
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better to print the warning about specifying lower or empty version. What do you think ?

return nil, nil
}

inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)

nodeFilter := []ast.Node{
Expand Down Expand Up @@ -211,3 +228,33 @@ func checkSelectorExprTarget(typ *ast.SelectorExpr) bool {
targetName := x.Name + "." + typ.Sel.Name
return targetName == "testing.TB"
}

// goVersionLower117 returns true if version is lower than Go 1.17 or empty.
// version must be in the format 'go1.17', '1.17'.
// In case of an invalid input returns not-nil error.
func goVersionLower117(version string) (bool, error) {
version = strings.TrimPrefix(version, "go")
if version == "" {
return false, nil
}

parts := strings.Split(version, ".")
if len(parts) != 2 {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If 1.17.N is passed, this linter occurs an error at this line. Is this intent ?

return false, errors.New(`go version must has format "go<MAJOR>.<MINOR>" or "<MAJOR>.<MINOR>"`)
}

major, err := strconv.Atoi(parts[0])
if err != nil {
alexandear marked this conversation as resolved.
Show resolved Hide resolved
return false, fmt.Errorf("go version major part must be a number: %w", err)
}
if major < 1 {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If Go become Go2 in the future (probably not), this linter must doesn't work. So I think this doesn't need. What do you think ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The linter might be actual even on Go2. But I can remove this check, no problem.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's remove this check, thanks.

return true, nil
}

minor, err := strconv.Atoi(parts[1])
if err != nil {
return false, fmt.Errorf("go version minor part must be a number: %w", err)
}

return minor < 17, nil
}
34 changes: 34 additions & 0 deletions tenv_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,37 @@ func TestAnalyzer(t *testing.T) {
testdata := testutil.WithModules(t, analysistest.TestData(), nil)
analysistest.Run(t, testdata, tenv.Analyzer, "a")
}

func TestAnalyzerGo116(t *testing.T) {
testdata := testutil.WithModules(t, analysistest.TestData(), nil)
a := tenv.Analyzer
a.Flags.Parse([]string{"-go", "1.16"})
analysistest.Run(t, testdata, a, "go116")
}

func TestRun(t *testing.T) {
t.Run("empty Go version", func(t *testing.T) {
for _, goVersion := range []string{
"", "go",
} {
testdata := testutil.WithModules(t, analysistest.TestData(), nil)
a := tenv.Analyzer
a.Flags.Parse([]string{"-go", goVersion})
analysistest.Run(t, testdata, a, "a")
}
})

t.Run("invalid Go version", func(t *testing.T) {
for _, goVersion := range []string{
"go1", "goa.2", "go1.a",
} {
a := tenv.Analyzer
a.Flags.Parse([]string{"-go", goVersion})
_, err := a.Run(nil)

if err == nil {
t.Error("expected error, but got <nil>")
alexandear marked this conversation as resolved.
Show resolved Hide resolved
}
}
})
}
3 changes: 3 additions & 0 deletions testdata/src/go116/go.mod
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module go116

go 1.16
12 changes: 12 additions & 0 deletions testdata/src/go116/go116_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package go116

import (
"os"
"testing"
)

func TestF(t *testing.T) {
os.Setenv("a", "b") // if -go = 1.16, ""
err := os.Setenv("a", "b") // if -go = 1.16, ""
_ = err
}