initial commit: ifinit and boolnames
This commit is contained in:
11
.custom-gcl.yaml
Normal file
11
.custom-gcl.yaml
Normal file
@@ -0,0 +1,11 @@
|
||||
# Builds a golangci-lint binary with our own linters (ops/customlint) embedded.
|
||||
#
|
||||
# The version below MUST stay in lockstep with the golangci-lint version pinned
|
||||
# in .github/workflows/ci.yaml: plugins link against golangci-lint internals, so
|
||||
# a mismatch is a link error, not a graceful degradation.
|
||||
version: v2.13.1
|
||||
name: golint
|
||||
destination: ./bin
|
||||
plugins:
|
||||
- module: git.offline-twitter.com/offline-labs/go-lint
|
||||
path: .
|
||||
1
.gitignore
vendored
Normal file
1
.gitignore
vendored
Normal file
@@ -0,0 +1 @@
|
||||
bin/
|
||||
129
.golangci.yaml
Normal file
129
.golangci.yaml
Normal file
@@ -0,0 +1,129 @@
|
||||
version: "2"
|
||||
|
||||
linters:
|
||||
default: none
|
||||
enable:
|
||||
# Defaults
|
||||
- errcheck
|
||||
- govet
|
||||
- ineffassign
|
||||
- staticcheck
|
||||
- unused
|
||||
|
||||
# Extras
|
||||
- depguard
|
||||
- errorlint
|
||||
- forbidigo
|
||||
- godox
|
||||
- lll
|
||||
- nolintlint
|
||||
- sqlclosecheck
|
||||
- whitespace
|
||||
- modernize
|
||||
|
||||
# Custom linters
|
||||
- ifinit
|
||||
- boolnames
|
||||
|
||||
settings:
|
||||
custom:
|
||||
ifinit:
|
||||
type: module
|
||||
description: if-statement init clauses must be on a single line
|
||||
boolnames:
|
||||
type: module
|
||||
description: boolean variables and functions should use "is", "has", "was" etc
|
||||
settings:
|
||||
whitelist:
|
||||
- ok
|
||||
forbidigo:
|
||||
forbid:
|
||||
- pattern: ^sync\.Map$
|
||||
msg: sync.Map loses type safety — use MutexMap instead
|
||||
analyze-types: true
|
||||
depguard:
|
||||
rules:
|
||||
main:
|
||||
files:
|
||||
- $all
|
||||
deny:
|
||||
- pkg: io/ioutil
|
||||
desc: Replace with the matching functions from `io` or `os` packages
|
||||
- pkg: github.com/pkg/errors
|
||||
desc: Should be replaced by standard lib errors package
|
||||
errcheck:
|
||||
# report about not checking of errors in type assertions: `a := b.(MyStruct)`;
|
||||
# default is false: such cases aren't reported by default.
|
||||
check-type-assertions: true
|
||||
errorlint:
|
||||
errorf: true # Ensure Errorf only uses %w (not %v or %s etc) for errors
|
||||
asserts: true # Require errors.As instead of type-asserting
|
||||
comparison: true # Require errors.Is instead of equality-checking
|
||||
godox:
|
||||
# report any comments starting with keywords, this is useful for TODO or FIXME comments that
|
||||
# might be left in the code accidentally and should be resolved before merging
|
||||
keywords: # default keywords are TODO, BUG, and FIXME, these can be overwritten by this setting
|
||||
- XXX
|
||||
govet:
|
||||
enable-all: true
|
||||
disable:
|
||||
- fieldalignment
|
||||
lll:
|
||||
line-length: 140
|
||||
tab-width: 4
|
||||
nolintlint:
|
||||
require-explanation: true
|
||||
require-specific: true
|
||||
allow-unused: false
|
||||
staticcheck:
|
||||
checks:
|
||||
- all
|
||||
- -ST1000 # Re-enable this once we have docstrings
|
||||
dot-import-whitelist: []
|
||||
wrapcheck:
|
||||
report-internal-errors: false
|
||||
extra-ignore-sigs:
|
||||
- "json.Marshal("
|
||||
exclusions:
|
||||
rules:
|
||||
- path: _test.go
|
||||
linters:
|
||||
- errcheck
|
||||
# Vendored from github.com/aldy505/sentry-integration/sqltracer — track
|
||||
# upstream style, don't wrap errors (would break errors.Is/As across the
|
||||
# transparent driver wrapper).
|
||||
- path: pkg/db/sentrysql/
|
||||
linters:
|
||||
- wrapcheck
|
||||
generated: lax # Don't lint generated files
|
||||
paths: []
|
||||
|
||||
formatters:
|
||||
enable:
|
||||
- gci
|
||||
- gofmt
|
||||
- gofumpt
|
||||
settings:
|
||||
gci:
|
||||
sections:
|
||||
- standard
|
||||
- default
|
||||
- localmodule
|
||||
gofmt:
|
||||
simplify: true
|
||||
gofumpt:
|
||||
module-path: git.offline-twitter.com/offline-labs/go-lint
|
||||
extra-rules: false
|
||||
exclusions:
|
||||
generated: lax
|
||||
|
||||
run:
|
||||
build-tags:
|
||||
- integration
|
||||
- liveKeycloak
|
||||
- staging
|
||||
|
||||
issues:
|
||||
max-same-issues: 0
|
||||
max-issues-per-linter: 0
|
||||
uniq-by-line: false
|
||||
119
boolnames/analyzer.go
Normal file
119
boolnames/analyzer.go
Normal file
@@ -0,0 +1,119 @@
|
||||
package boolnames
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"go/ast"
|
||||
"go/types"
|
||||
"slices"
|
||||
"strings"
|
||||
"unicode"
|
||||
"unicode/utf8"
|
||||
|
||||
"golang.org/x/tools/go/analysis"
|
||||
"golang.org/x/tools/go/analysis/passes/inspect"
|
||||
"golang.org/x/tools/go/ast/inspector"
|
||||
)
|
||||
|
||||
var defaultBoolNamePrefixes = []string{"is", "has", "can", "was"}
|
||||
|
||||
var Analyzer = NewAnalyzer(Settings{})
|
||||
|
||||
func NewAnalyzer(settings Settings) *analysis.Analyzer {
|
||||
if len(settings.AllowedPrefixes) == 0 {
|
||||
settings.AllowedPrefixes = defaultBoolNamePrefixes
|
||||
}
|
||||
return &analysis.Analyzer{
|
||||
Name: "boolnames",
|
||||
Doc: `checks that boolean values and functions use appropriate names`,
|
||||
Requires: []*analysis.Analyzer{inspect.Analyzer},
|
||||
Run: func(pass *analysis.Pass) (any, error) {
|
||||
result, ok := pass.ResultOf[inspect.Analyzer]
|
||||
if !ok {
|
||||
return nil, fmt.Errorf("inspect analyzer result is unavailable")
|
||||
}
|
||||
|
||||
insp, ok := result.(*inspector.Inspector)
|
||||
if !ok {
|
||||
return nil, fmt.Errorf("inspect analyzer returned %T, want *inspector.Inspector", result)
|
||||
}
|
||||
|
||||
insp.Preorder([]ast.Node{(*ast.Ident)(nil)}, func(n ast.Node) {
|
||||
ident, ok := n.(*ast.Ident)
|
||||
if !ok {
|
||||
return
|
||||
}
|
||||
obj := pass.TypesInfo.Defs[ident]
|
||||
if obj == nil || ident.Name == "_" {
|
||||
return
|
||||
}
|
||||
|
||||
switch obj.(type) {
|
||||
case *types.Var, *types.Const, *types.Func:
|
||||
default:
|
||||
return
|
||||
}
|
||||
|
||||
if !hasBoolType(obj.Type()) {
|
||||
return
|
||||
}
|
||||
|
||||
if settings.IsAcceptableBoolName(ident.Name) {
|
||||
return
|
||||
}
|
||||
|
||||
pass.Reportf(ident.Pos(),
|
||||
"boolean name %q must start with %s",
|
||||
ident.Name,
|
||||
strings.Join(settings.AllowedPrefixes, ", "),
|
||||
)
|
||||
})
|
||||
|
||||
return nil, nil
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
func hasBoolType(t types.Type) bool {
|
||||
isBool := func(t types.Type) bool {
|
||||
basic, ok := types.Unalias(t).Underlying().(*types.Basic)
|
||||
if !ok {
|
||||
return false
|
||||
}
|
||||
|
||||
return basic.Kind() == types.Bool || basic.Kind() == types.UntypedBool
|
||||
}
|
||||
if isBool(t) {
|
||||
return true
|
||||
}
|
||||
|
||||
sig, ok := types.Unalias(t).Underlying().(*types.Signature)
|
||||
if !ok {
|
||||
return false
|
||||
}
|
||||
|
||||
results := sig.Results()
|
||||
return results.Len() == 1 && isBool(results.At(0).Type())
|
||||
}
|
||||
|
||||
func (settings Settings) IsAcceptableBoolName(s string) bool {
|
||||
if slices.Contains(settings.Whitelist, s) {
|
||||
return true
|
||||
}
|
||||
|
||||
for _, pfix := range settings.AllowedPrefixes {
|
||||
if pfix == "" || len(s) <= len(pfix) || s[1:len(pfix)] != pfix[1:] {
|
||||
continue
|
||||
}
|
||||
|
||||
first := s[0]
|
||||
if first != pfix[0] && first != pfix[0]-'a'+'A' {
|
||||
continue
|
||||
}
|
||||
|
||||
sfix := s[len(pfix):]
|
||||
r, _ := utf8.DecodeRuneInString(sfix)
|
||||
return unicode.IsUpper(r)
|
||||
}
|
||||
|
||||
return false
|
||||
}
|
||||
22
boolnames/analyzer_test.go
Normal file
22
boolnames/analyzer_test.go
Normal file
@@ -0,0 +1,22 @@
|
||||
package boolnames_test
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"golang.org/x/tools/go/analysis/analysistest"
|
||||
|
||||
"git.offline-twitter.com/offline-labs/go-lint/boolnames"
|
||||
)
|
||||
|
||||
func TestAnalyzer(t *testing.T) {
|
||||
analysistest.Run(t, analysistest.TestData(), boolnames.Analyzer, "cases")
|
||||
}
|
||||
|
||||
func TestAnalyzerWithCustomSettings(t *testing.T) {
|
||||
analyzer := boolnames.NewAnalyzer(boolnames.Settings{
|
||||
Whitelist: []string{"ok"},
|
||||
AllowedPrefixes: []string{"truthy"},
|
||||
})
|
||||
|
||||
analysistest.Run(t, analysistest.TestData(), analyzer, "casescustom")
|
||||
}
|
||||
31
boolnames/plugin.go
Normal file
31
boolnames/plugin.go
Normal file
@@ -0,0 +1,31 @@
|
||||
package boolnames
|
||||
|
||||
import (
|
||||
"github.com/golangci/plugin-module-register/register"
|
||||
"golang.org/x/tools/go/analysis"
|
||||
)
|
||||
|
||||
func init() {
|
||||
register.Plugin("boolnames", New)
|
||||
}
|
||||
|
||||
type plugin struct {
|
||||
settings Settings
|
||||
}
|
||||
|
||||
func New(rawSettings any) (register.LinterPlugin, error) {
|
||||
settings, err := decodeSettings(rawSettings)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
return &plugin{settings: settings}, nil
|
||||
}
|
||||
|
||||
func (p *plugin) BuildAnalyzers() ([]*analysis.Analyzer, error) {
|
||||
return []*analysis.Analyzer{NewAnalyzer(p.settings)}, nil
|
||||
}
|
||||
|
||||
func (p *plugin) GetLoadMode() string {
|
||||
return register.LoadModeTypesInfo
|
||||
}
|
||||
16
boolnames/settings.go
Normal file
16
boolnames/settings.go
Normal file
@@ -0,0 +1,16 @@
|
||||
package boolnames
|
||||
|
||||
import "github.com/golangci/plugin-module-register/register"
|
||||
|
||||
type Settings struct {
|
||||
Whitelist []string `json:"whitelist"`
|
||||
AllowedPrefixes []string `json:"allowed_prefixes"`
|
||||
}
|
||||
|
||||
func decodeSettings(rawSettings any) (Settings, error) {
|
||||
if rawSettings == nil {
|
||||
return Settings{}, nil
|
||||
}
|
||||
|
||||
return register.DecodeSettings[Settings](rawSettings)
|
||||
}
|
||||
56
boolnames/testdata/src/cases/boolnames.go
vendored
Normal file
56
boolnames/testdata/src/cases/boolnames.go
vendored
Normal file
@@ -0,0 +1,56 @@
|
||||
package cases
|
||||
|
||||
var isPackageReady bool
|
||||
var packageReady bool // want `boolean name "packageReady" must`
|
||||
|
||||
const hasPackageValue = true
|
||||
const packageValue = true // want `boolean name "packageValue" must`
|
||||
|
||||
type FeatureFlag bool
|
||||
|
||||
type Predicate func() bool
|
||||
|
||||
var canEvaluate Predicate
|
||||
var evaluate Predicate // want `boolean name "evaluate" must`
|
||||
|
||||
type Config struct {
|
||||
HasOwner bool
|
||||
owner bool // want `boolean name "owner" must`
|
||||
|
||||
CanRun func() bool
|
||||
run func() bool // want `boolean name "run" must`
|
||||
}
|
||||
|
||||
func IsReady() bool { return true }
|
||||
|
||||
func ready() bool { return true } // want `boolean name "ready" must`
|
||||
|
||||
func Find() (int, bool) { return 0, true }
|
||||
func FindPair() (bool, bool) { return true, false }
|
||||
|
||||
func consume(active bool, hasActive bool, predicate func() bool, canCheck func() bool) (ok bool) { // want `boolean name "consume" must` `boolean name "active" must` `boolean name "predicate" must` `boolean name "ok" must`
|
||||
ready := true // want `boolean name "ready" must`
|
||||
isReady := true
|
||||
|
||||
var enabled bool // want `boolean name "enabled" must`
|
||||
var wasEnabled bool
|
||||
|
||||
var namedFlag FeatureFlag // want `boolean name "namedFlag" must`
|
||||
var isNamedFlag FeatureFlag
|
||||
|
||||
checker := func() bool { return true } // want `boolean name "checker" must`
|
||||
hasChecker := func() bool { return true }
|
||||
|
||||
_ = func(active bool) (ok bool) { // want `boolean name "active" must` `boolean name "ok" must`
|
||||
return ok
|
||||
}
|
||||
|
||||
return ready || isReady || enabled || wasEnabled || bool(namedFlag) || bool(isNamedFlag) || checker() || hasChecker() || active || hasActive || predicate() || canCheck()
|
||||
}
|
||||
|
||||
func (flag FeatureFlag) String() string { // want `boolean name "flag" must`
|
||||
if flag {
|
||||
return "true"
|
||||
}
|
||||
return "false"
|
||||
}
|
||||
12
boolnames/testdata/src/casescustom/boolnames.go
vendored
Normal file
12
boolnames/testdata/src/casescustom/boolnames.go
vendored
Normal file
@@ -0,0 +1,12 @@
|
||||
package casescustom
|
||||
|
||||
var ok bool
|
||||
var truthyValue bool
|
||||
|
||||
var ready bool // want `boolean name "ready" must start with truthy`
|
||||
var isReady bool // want `boolean name "isReady" must start with truthy`
|
||||
var hasOwner bool // want `boolean name "hasOwner" must start with truthy`
|
||||
var truthy bool // want `boolean name "truthy" must start with truthy`
|
||||
|
||||
func truthyCheck() bool { return true }
|
||||
func canCheck() bool { return true } // want `boolean name "canCheck" must start with truthy`
|
||||
37
build.sh
Executable file
37
build.sh
Executable file
@@ -0,0 +1,37 @@
|
||||
#!/usr/bin/env sh
|
||||
|
||||
set -e
|
||||
|
||||
SCRIPT_DIR=$(cd "$(dirname "$0")" && pwd)
|
||||
cd "$SCRIPT_DIR" # .custom-gcl.yaml is only discovered in the current directory
|
||||
|
||||
BIN="bin/golint"
|
||||
STAMP="bin/.golint.stamp"
|
||||
|
||||
# Everything that changes the compiled binary: the linter sources, and the build
|
||||
# config that pins the golangci-lint version the plugin links against. testdata
|
||||
# is excluded -- fixtures are not compiled into the binary.
|
||||
current_stamp() {
|
||||
{
|
||||
find . -type f ! -path '*/testdata/*' | sort | xargs shasum -a 256
|
||||
shasum -a 256 .custom-gcl.yaml
|
||||
} | shasum -a 256 | cut -d' ' -f1
|
||||
}
|
||||
|
||||
if [ "$1" = "--if-needed" ] &&
|
||||
[ -x "$BIN" ] &&
|
||||
[ -f "$STAMP" ] &&
|
||||
[ "$(cat "$STAMP")" = "$(current_stamp)" ]; then
|
||||
exit 0
|
||||
fi
|
||||
|
||||
if ! [ -x "$(command -v golangci-lint)" ]; then
|
||||
echo "golangci-lint is not installed. Please install it from https://golangci-lint.run/welcome/install/"
|
||||
exit 1
|
||||
fi
|
||||
|
||||
echo "Building golint (golangci-lint + custom linters)..."
|
||||
golangci-lint custom
|
||||
current_stamp > "$STAMP"
|
||||
|
||||
echo "Done: $SCRIPT_DIR/$BIN"
|
||||
13
go.mod
Normal file
13
go.mod
Normal file
@@ -0,0 +1,13 @@
|
||||
module git.offline-twitter.com/offline-labs/go-lint
|
||||
|
||||
go 1.26.3
|
||||
|
||||
require (
|
||||
github.com/golangci/plugin-module-register v0.1.2
|
||||
golang.org/x/tools v0.50.0
|
||||
)
|
||||
|
||||
require (
|
||||
golang.org/x/mod v0.41.0 // indirect
|
||||
golang.org/x/sync v0.23.0 // indirect
|
||||
)
|
||||
10
go.sum
Normal file
10
go.sum
Normal file
@@ -0,0 +1,10 @@
|
||||
github.com/golangci/plugin-module-register v0.1.2 h1:e5WM6PO6NIAEcij3B053CohVp3HIYbzSuP53UAYgOpg=
|
||||
github.com/golangci/plugin-module-register v0.1.2/go.mod h1:1+QGTsKBvAIvPvoY/os+G5eoqxWn70HYDm2uvUyGuVw=
|
||||
github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
|
||||
github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
|
||||
golang.org/x/mod v0.41.0 h1:qJmnOUb4YB+FsEuM3HcWucdZASCPGhsX6uljO6pog0c=
|
||||
golang.org/x/mod v0.41.0/go.mod h1:Ek9pY8RKWXwsWvd3rQiHYtMqkjSUV+s1Rj7j4H5Ur6o=
|
||||
golang.org/x/sync v0.23.0 h1:KameEIfc1IkluZyXWLn39Wd4tURc6GbCiISGiZm2bQk=
|
||||
golang.org/x/sync v0.23.0/go.mod h1:sUUOizhqBxiL6pEWpqNLUiaJn1ShEbZ6BBqskPbjZm0=
|
||||
golang.org/x/tools v0.50.0 h1:c2ifzfcuY7L90lZ2aKd8S4K2NpASF08SZx9ZuJkHmSU=
|
||||
golang.org/x/tools v0.50.0/go.mod h1:7ulVMw3831Mwi5EZD6RomGyffr4VFjuNYXf2BbCEAV0=
|
||||
200
ifinit/analyzer.go
Normal file
200
ifinit/analyzer.go
Normal file
@@ -0,0 +1,200 @@
|
||||
// Package ifinit reports if-statement init clauses that span more than one line.
|
||||
package ifinit
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"go/ast"
|
||||
"go/token"
|
||||
"strings"
|
||||
|
||||
"golang.org/x/tools/go/analysis"
|
||||
"golang.org/x/tools/go/analysis/passes/inspect"
|
||||
"golang.org/x/tools/go/ast/inspector"
|
||||
)
|
||||
|
||||
const doc = `checks that if-statement init clauses occupy a single line
|
||||
|
||||
A multi-line init clause pushes the condition being tested far away from the
|
||||
"if" keyword, so the reader has to scan past the setup to find out what is
|
||||
actually being decided. The fix hoists the init clause into its own statement
|
||||
above the if.`
|
||||
|
||||
var Analyzer = &analysis.Analyzer{
|
||||
Name: "ifinit",
|
||||
Doc: doc,
|
||||
Requires: []*analysis.Analyzer{inspect.Analyzer},
|
||||
Run: run,
|
||||
}
|
||||
|
||||
func run(pass *analysis.Pass) (any, error) {
|
||||
result, ok := pass.ResultOf[inspect.Analyzer]
|
||||
if !ok {
|
||||
return nil, fmt.Errorf("inspect analyzer result is unavailable")
|
||||
}
|
||||
|
||||
insp, ok := result.(*inspector.Inspector)
|
||||
if !ok {
|
||||
return nil, fmt.Errorf("inspect analyzer returned %T, want *inspector.Inspector", result)
|
||||
}
|
||||
|
||||
insp.WithStack([]ast.Node{(*ast.IfStmt)(nil)}, func(n ast.Node, isPush bool, stack []ast.Node) bool {
|
||||
if !isPush {
|
||||
return false
|
||||
}
|
||||
|
||||
stmt, ok := n.(*ast.IfStmt)
|
||||
if !ok {
|
||||
return true
|
||||
}
|
||||
if stmt.Init == nil {
|
||||
return true
|
||||
}
|
||||
|
||||
start := pass.Fset.Position(stmt.Init.Pos())
|
||||
end := pass.Fset.Position(stmt.Init.End())
|
||||
if start.Line == end.Line {
|
||||
return true
|
||||
}
|
||||
|
||||
diag := analysis.Diagnostic{
|
||||
Pos: stmt.Init.Pos(),
|
||||
End: stmt.Init.End(),
|
||||
Message: fmt.Sprintf("if statement init clause spans %d lines; it must be on a single line", end.Line-start.Line+1),
|
||||
}
|
||||
|
||||
if fix, ok := hoist(pass, stmt, stack); ok {
|
||||
diag.SuggestedFixes = []analysis.SuggestedFix{fix}
|
||||
}
|
||||
|
||||
pass.Report(diag)
|
||||
|
||||
return true
|
||||
})
|
||||
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
// hoist rewrites "if <init>; <cond> {" into "<init>" on its own line followed by
|
||||
// "if <cond> {". The init clause's own text is left untouched, so comments and
|
||||
// internal line breaks survive.
|
||||
func hoist(pass *analysis.Pass, stmt *ast.IfStmt, stack []ast.Node) (analysis.SuggestedFix, bool) {
|
||||
if !isHoistable(stmt, stack) {
|
||||
return analysis.SuggestedFix{}, false
|
||||
}
|
||||
|
||||
if !isShadowSafe(pass, stmt) {
|
||||
return analysis.SuggestedFix{}, false
|
||||
}
|
||||
|
||||
indent, ok := lineIndent(pass, stmt.Pos())
|
||||
if !ok {
|
||||
return analysis.SuggestedFix{}, false
|
||||
}
|
||||
|
||||
return analysis.SuggestedFix{
|
||||
Message: "Hoist the init clause above the if statement",
|
||||
TextEdits: []analysis.TextEdit{
|
||||
// Drop the leading "if ".
|
||||
{Pos: stmt.Pos(), End: stmt.Init.Pos()},
|
||||
// Turn the "; " separator into a line break and a fresh "if ".
|
||||
{Pos: stmt.Init.End(), End: stmt.Cond.Pos(), NewText: []byte("\n" + indent + "if ")},
|
||||
},
|
||||
}, true
|
||||
}
|
||||
|
||||
func isHoistable(stmt *ast.IfStmt, stack []ast.Node) bool {
|
||||
if len(stack) < 2 {
|
||||
return false
|
||||
}
|
||||
|
||||
switch parent := stack[len(stack)-2].(type) {
|
||||
case *ast.IfStmt:
|
||||
// An "else if": the init clause cannot move above the chain without
|
||||
// restructuring the whole thing into a nested else block.
|
||||
return parent.Else != ast.Stmt(stmt)
|
||||
case *ast.LabeledStmt:
|
||||
// Hoisting would leave the label pointing at the init clause instead.
|
||||
return false
|
||||
}
|
||||
|
||||
return true
|
||||
}
|
||||
|
||||
// shadowSafe reports whether hoisting stmt's init clause out of the if's scope
|
||||
// leaves every name it binds meaning the same thing.
|
||||
//
|
||||
// Only ":=" declares anything, and only in the if statement's own scope. Moving
|
||||
// that declaration up one level is safe exactly when none of the names it binds
|
||||
// already resolve to something in the enclosing scope: otherwise the hoisted
|
||||
// ":=" either assigns to that existing object instead of declaring a fresh one,
|
||||
// or collides with a later declaration in the same block.
|
||||
//
|
||||
// The check is deliberately conservative. It refuses whenever a name resolves at
|
||||
// all -- including to a package-level or universe name -- rather than trying to
|
||||
// prove that nothing reads the shadowed object afterwards.
|
||||
func isShadowSafe(pass *analysis.Pass, stmt *ast.IfStmt) bool {
|
||||
assign, ok := stmt.Init.(*ast.AssignStmt)
|
||||
if !ok || assign.Tok != token.DEFINE {
|
||||
return true
|
||||
}
|
||||
|
||||
scope := pass.TypesInfo.Scopes[stmt]
|
||||
if scope == nil {
|
||||
return false
|
||||
}
|
||||
|
||||
enclosing := scope.Parent()
|
||||
if enclosing == nil {
|
||||
return false
|
||||
}
|
||||
|
||||
for _, lhs := range assign.Lhs {
|
||||
ident, ok := lhs.(*ast.Ident)
|
||||
if !ok {
|
||||
return false
|
||||
}
|
||||
|
||||
if ident.Name == "_" {
|
||||
continue
|
||||
}
|
||||
|
||||
// token.NoPos makes the lookup ignore declaration order, so a name
|
||||
// declared later in the same block counts as a conflict too.
|
||||
if _, obj := enclosing.LookupParent(ident.Name, token.NoPos); obj != nil {
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
return true
|
||||
}
|
||||
|
||||
// lineIndent returns the whitespace preceding pos on its line, and reports
|
||||
// whether pos is the first non-whitespace token on that line.
|
||||
func lineIndent(pass *analysis.Pass, pos token.Pos) (string, bool) {
|
||||
if pass.ReadFile == nil {
|
||||
return "", false
|
||||
}
|
||||
|
||||
file := pass.Fset.File(pos)
|
||||
if file == nil {
|
||||
return "", false
|
||||
}
|
||||
|
||||
src, err := pass.ReadFile(file.Name())
|
||||
if err != nil {
|
||||
return "", false
|
||||
}
|
||||
|
||||
lineStart := file.Offset(file.LineStart(file.Line(pos)))
|
||||
offset := file.Offset(pos)
|
||||
if offset > len(src) {
|
||||
return "", false
|
||||
}
|
||||
|
||||
prefix := string(src[lineStart:offset])
|
||||
if strings.TrimLeft(prefix, " \t") != "" {
|
||||
return "", false
|
||||
}
|
||||
|
||||
return prefix, true
|
||||
}
|
||||
13
ifinit/analyzer_test.go
Normal file
13
ifinit/analyzer_test.go
Normal file
@@ -0,0 +1,13 @@
|
||||
package ifinit_test
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"golang.org/x/tools/go/analysis/analysistest"
|
||||
|
||||
"git.offline-twitter.com/offline-labs/go-lint/ifinit"
|
||||
)
|
||||
|
||||
func TestAnalyzer(t *testing.T) {
|
||||
analysistest.RunWithSuggestedFixes(t, analysistest.TestData(), ifinit.Analyzer, "cases")
|
||||
}
|
||||
24
ifinit/plugin.go
Normal file
24
ifinit/plugin.go
Normal file
@@ -0,0 +1,24 @@
|
||||
package ifinit
|
||||
|
||||
import (
|
||||
"github.com/golangci/plugin-module-register/register"
|
||||
"golang.org/x/tools/go/analysis"
|
||||
)
|
||||
|
||||
func init() {
|
||||
register.Plugin("ifinit", New)
|
||||
}
|
||||
|
||||
type plugin struct{}
|
||||
|
||||
func New(any) (register.LinterPlugin, error) {
|
||||
return &plugin{}, nil
|
||||
}
|
||||
|
||||
func (p *plugin) BuildAnalyzers() ([]*analysis.Analyzer, error) {
|
||||
return []*analysis.Analyzer{Analyzer}, nil
|
||||
}
|
||||
|
||||
func (p *plugin) GetLoadMode() string {
|
||||
return register.LoadModeTypesInfo
|
||||
}
|
||||
88
ifinit/testdata/src/cases/scopes.go
vendored
Normal file
88
ifinit/testdata/src/cases/scopes.go
vendored
Normal file
@@ -0,0 +1,88 @@
|
||||
package cases
|
||||
|
||||
func do(args ...int) error { return nil }
|
||||
|
||||
var errState error
|
||||
|
||||
// Safe: both names are fresh in the block that would receive the hoisted statement.
|
||||
func freshNames() {
|
||||
if v, err := multi( // want `init clause spans 3 lines`
|
||||
1,
|
||||
); err != nil {
|
||||
_ = v
|
||||
}
|
||||
}
|
||||
|
||||
// Safe: "_" declares nothing and err is fresh.
|
||||
func blankAndFresh() {
|
||||
if _, err := multi( // want `init clause spans 3 lines`
|
||||
1,
|
||||
); err != nil {
|
||||
_ = err
|
||||
}
|
||||
}
|
||||
|
||||
// Safe: "=" assigns to an existing variable; it declares nothing at all.
|
||||
func plainAssign() error {
|
||||
var err error
|
||||
|
||||
if err = do( // want `init clause spans 3 lines`
|
||||
1,
|
||||
); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// Unsafe: err already exists in the enclosing block. Hoisting the ":=" would
|
||||
// assign to that err rather than declaring a new one, so the returned error
|
||||
// would silently change.
|
||||
func shadowsExistingLocal() error {
|
||||
v, err := multi(1)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
_ = v
|
||||
|
||||
if v2, err := multi( // want `init clause spans 3 lines`
|
||||
2,
|
||||
); err != nil {
|
||||
_ = v2
|
||||
}
|
||||
|
||||
return err
|
||||
}
|
||||
|
||||
// Unsafe: the same names are redeclared later in this block, so hoisting turns
|
||||
// the later ":=" into "no new variables on left side of :=".
|
||||
func declaredLaterInBlock() {
|
||||
if v, err := multi( // want `init clause spans 3 lines`
|
||||
1,
|
||||
); err != nil {
|
||||
_ = v
|
||||
}
|
||||
|
||||
v, err := multi(2)
|
||||
_, _ = v, err
|
||||
}
|
||||
|
||||
// Unsafe: err is a parameter, so hoisting would rebind it for the rest of the body.
|
||||
func shadowsParam(err error) {
|
||||
if v, err := multi( // want `init clause spans 3 lines`
|
||||
1,
|
||||
); err != nil {
|
||||
_ = v
|
||||
}
|
||||
}
|
||||
|
||||
// Conservatively refused: shadowing a package-level name. Nothing reads errState
|
||||
// after the if here, so this one would in fact be safe -- the check does not try
|
||||
// to prove that.
|
||||
func shadowsPackageLevel() {
|
||||
if v, errState := multi( // want `init clause spans 3 lines`
|
||||
1,
|
||||
); errState != nil {
|
||||
_ = v
|
||||
}
|
||||
}
|
||||
91
ifinit/testdata/src/cases/scopes.go.golden
vendored
Normal file
91
ifinit/testdata/src/cases/scopes.go.golden
vendored
Normal file
@@ -0,0 +1,91 @@
|
||||
package cases
|
||||
|
||||
func do(args ...int) error { return nil }
|
||||
|
||||
var errState error
|
||||
|
||||
// Safe: both names are fresh in the block that would receive the hoisted statement.
|
||||
func freshNames() {
|
||||
v, err := multi( // want `init clause spans 3 lines`
|
||||
1,
|
||||
)
|
||||
if err != nil {
|
||||
_ = v
|
||||
}
|
||||
}
|
||||
|
||||
// Safe: "_" declares nothing and err is fresh.
|
||||
func blankAndFresh() {
|
||||
_, err := multi( // want `init clause spans 3 lines`
|
||||
1,
|
||||
)
|
||||
if err != nil {
|
||||
_ = err
|
||||
}
|
||||
}
|
||||
|
||||
// Safe: "=" assigns to an existing variable; it declares nothing at all.
|
||||
func plainAssign() error {
|
||||
var err error
|
||||
|
||||
err = do( // want `init clause spans 3 lines`
|
||||
1,
|
||||
)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// Unsafe: err already exists in the enclosing block. Hoisting the ":=" would
|
||||
// assign to that err rather than declaring a new one, so the returned error
|
||||
// would silently change.
|
||||
func shadowsExistingLocal() error {
|
||||
v, err := multi(1)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
_ = v
|
||||
|
||||
if v2, err := multi( // want `init clause spans 3 lines`
|
||||
2,
|
||||
); err != nil {
|
||||
_ = v2
|
||||
}
|
||||
|
||||
return err
|
||||
}
|
||||
|
||||
// Unsafe: the same names are redeclared later in this block, so hoisting turns
|
||||
// the later ":=" into "no new variables on left side of :=".
|
||||
func declaredLaterInBlock() {
|
||||
if v, err := multi( // want `init clause spans 3 lines`
|
||||
1,
|
||||
); err != nil {
|
||||
_ = v
|
||||
}
|
||||
|
||||
v, err := multi(2)
|
||||
_, _ = v, err
|
||||
}
|
||||
|
||||
// Unsafe: err is a parameter, so hoisting would rebind it for the rest of the body.
|
||||
func shadowsParam(err error) {
|
||||
if v, err := multi( // want `init clause spans 3 lines`
|
||||
1,
|
||||
); err != nil {
|
||||
_ = v
|
||||
}
|
||||
}
|
||||
|
||||
// Conservatively refused: shadowing a package-level name. Nothing reads errState
|
||||
// after the if here, so this one would in fact be safe -- the check does not try
|
||||
// to prove that.
|
||||
func shadowsPackageLevel() {
|
||||
if v, errState := multi( // want `init clause spans 3 lines`
|
||||
1,
|
||||
); errState != nil {
|
||||
_ = v
|
||||
}
|
||||
}
|
||||
113
ifinit/testdata/src/cases/structural.go
vendored
Normal file
113
ifinit/testdata/src/cases/structural.go
vendored
Normal file
@@ -0,0 +1,113 @@
|
||||
package cases
|
||||
|
||||
func multi(args ...int) (int, error) { return 0, nil }
|
||||
|
||||
func single() (int, error) { return 0, nil }
|
||||
|
||||
func count(args ...int) int { return 0 }
|
||||
|
||||
func clean() {
|
||||
if v, err := single(); err != nil {
|
||||
_ = v
|
||||
}
|
||||
|
||||
x := count(
|
||||
1,
|
||||
2,
|
||||
)
|
||||
if x > 0 {
|
||||
_ = x
|
||||
}
|
||||
}
|
||||
|
||||
func multiLineInit() {
|
||||
if v, err := multi( // want `init clause spans 4 lines`
|
||||
1,
|
||||
2,
|
||||
); err != nil {
|
||||
_ = v
|
||||
}
|
||||
}
|
||||
|
||||
func elseIfChain() {
|
||||
if x := count(1); x > 0 {
|
||||
_ = x
|
||||
} else if v, err := multi( // want `init clause spans 3 lines`
|
||||
1,
|
||||
); err != nil {
|
||||
_ = v
|
||||
}
|
||||
}
|
||||
|
||||
func nestedInsideBody() {
|
||||
if true {
|
||||
if v, err := multi( // want `init clause spans 3 lines`
|
||||
1,
|
||||
); err != nil {
|
||||
_ = v
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Only "if" init clauses are covered; for and switch are deliberately untouched.
|
||||
func otherInitClauses() {
|
||||
for i := count(
|
||||
1,
|
||||
); i < 10; i++ {
|
||||
_ = i
|
||||
}
|
||||
|
||||
switch v := count(
|
||||
1,
|
||||
); v {
|
||||
case 1:
|
||||
}
|
||||
}
|
||||
|
||||
func doSomething(args ...int) error { return nil }
|
||||
|
||||
func setup(args ...int) {}
|
||||
|
||||
// The line break falls after ":=" rather than inside the call's arguments.
|
||||
func brokenAfterAssignOp() {
|
||||
if result, err := // want `init clause spans 2 lines`
|
||||
multi(1, 2); err != nil {
|
||||
_ = result
|
||||
}
|
||||
}
|
||||
|
||||
// Assignment to an existing variable rather than a definition.
|
||||
func assignNotDefine() error {
|
||||
var err error
|
||||
if err = doSomething( // want `init clause spans 4 lines`
|
||||
1,
|
||||
2,
|
||||
); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// The init clause is a bare call expression, not an assignment at all.
|
||||
func exprStmtInit(condition bool) {
|
||||
if setup( // want `init clause spans 4 lines`
|
||||
1,
|
||||
2,
|
||||
); condition {
|
||||
_ = condition
|
||||
}
|
||||
}
|
||||
|
||||
// gofmt joins "else" and "if", but the parser accepts them on separate lines --
|
||||
// and then the "if" really is the first token on its line. Hoisting here would
|
||||
// emit "} v, err := multi(...)", which does not compile.
|
||||
func elseOnItsOwnLine() {
|
||||
if true {
|
||||
} else
|
||||
if v, err := multi( // want `init clause spans 3 lines`
|
||||
1,
|
||||
); err != nil {
|
||||
_ = v
|
||||
}
|
||||
}
|
||||
118
ifinit/testdata/src/cases/structural.go.golden
vendored
Normal file
118
ifinit/testdata/src/cases/structural.go.golden
vendored
Normal file
@@ -0,0 +1,118 @@
|
||||
package cases
|
||||
|
||||
func multi(args ...int) (int, error) { return 0, nil }
|
||||
|
||||
func single() (int, error) { return 0, nil }
|
||||
|
||||
func count(args ...int) int { return 0 }
|
||||
|
||||
func clean() {
|
||||
if v, err := single(); err != nil {
|
||||
_ = v
|
||||
}
|
||||
|
||||
x := count(
|
||||
1,
|
||||
2,
|
||||
)
|
||||
if x > 0 {
|
||||
_ = x
|
||||
}
|
||||
}
|
||||
|
||||
func multiLineInit() {
|
||||
v, err := multi( // want `init clause spans 4 lines`
|
||||
1,
|
||||
2,
|
||||
)
|
||||
if err != nil {
|
||||
_ = v
|
||||
}
|
||||
}
|
||||
|
||||
func elseIfChain() {
|
||||
if x := count(1); x > 0 {
|
||||
_ = x
|
||||
} else if v, err := multi( // want `init clause spans 3 lines`
|
||||
1,
|
||||
); err != nil {
|
||||
_ = v
|
||||
}
|
||||
}
|
||||
|
||||
func nestedInsideBody() {
|
||||
if true {
|
||||
v, err := multi( // want `init clause spans 3 lines`
|
||||
1,
|
||||
)
|
||||
if err != nil {
|
||||
_ = v
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Only "if" init clauses are covered; for and switch are deliberately untouched.
|
||||
func otherInitClauses() {
|
||||
for i := count(
|
||||
1,
|
||||
); i < 10; i++ {
|
||||
_ = i
|
||||
}
|
||||
|
||||
switch v := count(
|
||||
1,
|
||||
); v {
|
||||
case 1:
|
||||
}
|
||||
}
|
||||
|
||||
func doSomething(args ...int) error { return nil }
|
||||
|
||||
func setup(args ...int) {}
|
||||
|
||||
// The line break falls after ":=" rather than inside the call's arguments.
|
||||
func brokenAfterAssignOp() {
|
||||
result, err := // want `init clause spans 2 lines`
|
||||
multi(1, 2)
|
||||
if err != nil {
|
||||
_ = result
|
||||
}
|
||||
}
|
||||
|
||||
// Assignment to an existing variable rather than a definition.
|
||||
func assignNotDefine() error {
|
||||
var err error
|
||||
err = doSomething( // want `init clause spans 4 lines`
|
||||
1,
|
||||
2,
|
||||
)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// The init clause is a bare call expression, not an assignment at all.
|
||||
func exprStmtInit(condition bool) {
|
||||
setup( // want `init clause spans 4 lines`
|
||||
1,
|
||||
2,
|
||||
)
|
||||
if condition {
|
||||
_ = condition
|
||||
}
|
||||
}
|
||||
|
||||
// gofmt joins "else" and "if", but the parser accepts them on separate lines --
|
||||
// and then the "if" really is the first token on its line. Hoisting here would
|
||||
// emit "} v, err := multi(...)", which does not compile.
|
||||
func elseOnItsOwnLine() {
|
||||
if true {
|
||||
} else
|
||||
if v, err := multi( // want `init clause spans 3 lines`
|
||||
1,
|
||||
); err != nil {
|
||||
_ = v
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user