Update Go and vendor dependencies
This commit is contained in:
+2
-2
@@ -82,7 +82,6 @@ import (
|
||||
"honnef.co/go/tools/staticcheck/sa5008"
|
||||
"honnef.co/go/tools/staticcheck/sa5009"
|
||||
"honnef.co/go/tools/staticcheck/sa5010"
|
||||
"honnef.co/go/tools/staticcheck/sa5011"
|
||||
"honnef.co/go/tools/staticcheck/sa5012"
|
||||
"honnef.co/go/tools/staticcheck/sa6000"
|
||||
"honnef.co/go/tools/staticcheck/sa6001"
|
||||
@@ -99,6 +98,7 @@ import (
|
||||
"honnef.co/go/tools/staticcheck/sa9007"
|
||||
"honnef.co/go/tools/staticcheck/sa9008"
|
||||
"honnef.co/go/tools/staticcheck/sa9009"
|
||||
"honnef.co/go/tools/staticcheck/sa9010"
|
||||
)
|
||||
|
||||
var Analyzers = []*lint.Analyzer{
|
||||
@@ -180,7 +180,6 @@ var Analyzers = []*lint.Analyzer{
|
||||
sa5008.SCAnalyzer,
|
||||
sa5009.SCAnalyzer,
|
||||
sa5010.SCAnalyzer,
|
||||
sa5011.SCAnalyzer,
|
||||
sa5012.SCAnalyzer,
|
||||
sa6000.SCAnalyzer,
|
||||
sa6001.SCAnalyzer,
|
||||
@@ -197,4 +196,5 @@ var Analyzers = []*lint.Analyzer{
|
||||
sa9007.SCAnalyzer,
|
||||
sa9008.SCAnalyzer,
|
||||
sa9009.SCAnalyzer,
|
||||
sa9010.SCAnalyzer,
|
||||
}
|
||||
|
||||
+4
-1
@@ -20,7 +20,10 @@ var SCAnalyzer = lint.InitializeAnalyzer(&lint.Analyzer{
|
||||
Run: callcheck.Analyzer(rules),
|
||||
},
|
||||
Doc: &lint.RawDocumentation{
|
||||
Title: `Invalid format in \'time.Parse\'`,
|
||||
Title: `Invalid format in \'time.Parse\'`,
|
||||
Text: `\'time.Parse\' requires a layout string that uses Go's reference time:
|
||||
\"Mon Jan 2 15:04:05 MST 2006\". The layout must represent this date and time
|
||||
exactly. See https://pkg.go.dev/time#pkg-constants for layout examples.`,
|
||||
Since: "2017.1",
|
||||
Severity: lint.SeverityError,
|
||||
MergeIf: lint.MergeIfAny,
|
||||
|
||||
+4
-1
@@ -21,7 +21,10 @@ var SCAnalyzer = lint.InitializeAnalyzer(&lint.Analyzer{
|
||||
Requires: code.RequiredAnalyzers,
|
||||
},
|
||||
Doc: &lint.RawDocumentation{
|
||||
Title: `A nil \'context.Context\' is being passed to a function, consider using \'context.TODO\' instead`,
|
||||
Title: `A nil \'context.Context\' is being passed to a function, consider using \'context.TODO\' instead`,
|
||||
Text: `The context package prohibits the use of a \'nil\' context.
|
||||
If no parent context is available, a new context should be used,
|
||||
e.g. \'context.TODO\' or \'context.Background\'.`,
|
||||
Since: "2017.1",
|
||||
Severity: lint.SeverityWarning,
|
||||
MergeIf: lint.MergeIfAny,
|
||||
|
||||
+5
-1
@@ -18,7 +18,11 @@ var SCAnalyzer = lint.InitializeAnalyzer(&lint.Analyzer{
|
||||
Run: callcheck.Analyzer(checkUnmarshalPointerRules),
|
||||
},
|
||||
Doc: &lint.RawDocumentation{
|
||||
Title: `Non-pointer value passed to \'Unmarshal\' or \'Decode\'`,
|
||||
Title: `Non-pointer value passed to \'Unmarshal\' or \'Decode\'`,
|
||||
Text: `Functions such as \'encoding/json.Unmarshal\' and
|
||||
\'(*encoding/json.Decoder).Decode\' require a pointer to the value that should
|
||||
be populated. Passing a non-pointer value results in the function returning an
|
||||
error at runtime, as it cannot modify the target value.`,
|
||||
Since: "2017.1",
|
||||
Severity: lint.SeverityError,
|
||||
MergeIf: lint.MergeIfAny,
|
||||
|
||||
+58
-24
@@ -99,50 +99,33 @@ func run(pass *analysis.Pass) (any, error) {
|
||||
case knowledge.DeprecatedNeverUse:
|
||||
report.Report(pass, node,
|
||||
fmt.Sprintf("%s has been deprecated since %s because it shouldn't be used: %s",
|
||||
report.Render(pass, node), formatGoVersion(std.DeprecatedSince), depr.Msg))
|
||||
deprecatedObjName, formatGoVersion(std.DeprecatedSince), depr.Msg))
|
||||
case std.DeprecatedSince, knowledge.DeprecatedUseNoLonger:
|
||||
report.Report(pass, node,
|
||||
fmt.Sprintf("%s has been deprecated since %s: %s",
|
||||
report.Render(pass, node), formatGoVersion(std.DeprecatedSince), depr.Msg))
|
||||
deprecatedObjName, formatGoVersion(std.DeprecatedSince), depr.Msg))
|
||||
default:
|
||||
report.Report(pass, node,
|
||||
fmt.Sprintf("%s has been deprecated since %s and an alternative has been available since %s: %s",
|
||||
report.Render(pass, node), formatGoVersion(std.DeprecatedSince), formatGoVersion(std.AlternativeAvailableSince), depr.Msg))
|
||||
deprecatedObjName, formatGoVersion(std.DeprecatedSince), formatGoVersion(std.AlternativeAvailableSince), depr.Msg))
|
||||
}
|
||||
} else {
|
||||
report.Report(pass, node, fmt.Sprintf("%s is deprecated: %s", report.Render(pass, node), depr.Msg))
|
||||
report.Report(pass, node, fmt.Sprintf("%s is deprecated: %s", deprecatedObjName, depr.Msg))
|
||||
}
|
||||
}
|
||||
|
||||
var tfn types.Object
|
||||
stack := 0
|
||||
fn := func(node ast.Node, push bool) bool {
|
||||
if !push {
|
||||
stack--
|
||||
return false
|
||||
}
|
||||
stack++
|
||||
if stack == 1 {
|
||||
tfn = nil
|
||||
}
|
||||
if fn, ok := node.(*ast.FuncDecl); ok {
|
||||
tfn = pass.TypesInfo.ObjectOf(fn.Name)
|
||||
}
|
||||
|
||||
// FIXME(dh): this misses dot-imported objects
|
||||
sel, ok := node.(*ast.SelectorExpr)
|
||||
if !ok {
|
||||
return true
|
||||
}
|
||||
|
||||
checkIdentObj := func(sel *ast.SelectorExpr) bool {
|
||||
obj := pass.TypesInfo.ObjectOf(sel.Sel)
|
||||
|
||||
if obj_, ok := obj.(*types.Func); ok {
|
||||
obj = obj_.Origin()
|
||||
}
|
||||
if obj.Pkg() == nil {
|
||||
return true
|
||||
}
|
||||
|
||||
if obj.Pkg() == pass.Pkg {
|
||||
// A package is allowed to use its own deprecated objects
|
||||
return true
|
||||
@@ -165,8 +148,59 @@ func run(pass *analysis.Pass) (any, error) {
|
||||
return true
|
||||
}
|
||||
|
||||
node := ast.Node(sel)
|
||||
if pass.TypesInfo.Types[sel.X].IsType() {
|
||||
node = sel.Sel
|
||||
}
|
||||
if depr, ok := deprs.Objects[obj]; ok {
|
||||
handleDeprecation(depr, sel, code.SelectorName(pass, sel), obj.Pkg().Path(), tfn)
|
||||
handleDeprecation(depr, node, code.SelectorName(pass, sel), obj.Pkg().Path(), tfn)
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
fn := func(node ast.Node, push bool) bool {
|
||||
if !push {
|
||||
stack--
|
||||
return false
|
||||
}
|
||||
stack++
|
||||
if stack == 1 {
|
||||
tfn = nil
|
||||
}
|
||||
if fn, ok := node.(*ast.FuncDecl); ok {
|
||||
tfn = pass.TypesInfo.ObjectOf(fn.Name)
|
||||
}
|
||||
|
||||
switch v := node.(type) {
|
||||
// FIXME(dh): this misses dot-imported objects
|
||||
case *ast.SelectorExpr:
|
||||
return checkIdentObj(v)
|
||||
|
||||
case *ast.CompositeLit:
|
||||
litType := pass.TypesInfo.Types[v.Type]
|
||||
if !litType.IsType() {
|
||||
// This is probably unreachable.
|
||||
return true
|
||||
}
|
||||
if _, ok := litType.Type.Underlying().(*types.Struct); !ok {
|
||||
// We don't want to look at expressions in map initializers, for
|
||||
// example.
|
||||
return true
|
||||
}
|
||||
for _, elt := range v.Elts {
|
||||
kv, ok := elt.(*ast.KeyValueExpr)
|
||||
if !ok {
|
||||
return true
|
||||
}
|
||||
key, ok := kv.Key.(*ast.Ident)
|
||||
if !ok {
|
||||
// This is probably unreachable, since we're looking at keys
|
||||
// in a struct initializer.
|
||||
return true
|
||||
}
|
||||
sel := &ast.SelectorExpr{X: v.Type, Sel: key}
|
||||
checkIdentObj(sel)
|
||||
}
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
+6
-1
@@ -20,7 +20,12 @@ var SCAnalyzer = lint.InitializeAnalyzer(&lint.Analyzer{
|
||||
Run: callcheck.Analyzer(checkListenAddressRules),
|
||||
},
|
||||
Doc: &lint.RawDocumentation{
|
||||
Title: `Using an invalid host:port pair with a \'net.Listen\'-related function`,
|
||||
Title: `Using an invalid host:port pair with a \'net.Listen\'-related function`,
|
||||
Text: `Functions such as \'net.Listen\', \'net.ListenTCP\', and similar,
|
||||
expect a valid network address in the form of host:port. The host, the port,
|
||||
or both, can be omitted, e.g. \'localhost:8080\', \':8080\' or \':\' are valid
|
||||
host:port pairs.
|
||||
See https://pkg.go.dev/net#Listen for the full documentation.`,
|
||||
Since: "2017.1",
|
||||
Severity: lint.SeverityError,
|
||||
MergeIf: lint.MergeIfAny,
|
||||
|
||||
+1
-1
@@ -43,7 +43,7 @@ func run(pass *analysis.Pass) (any, error) {
|
||||
if refs == nil {
|
||||
continue
|
||||
}
|
||||
for _, ref := range irutil.FilterDebug(*refs) {
|
||||
for _, ref := range *refs {
|
||||
ifstmt, ok := ref.(*ir.If)
|
||||
if !ok {
|
||||
continue
|
||||
|
||||
@@ -31,7 +31,9 @@ var Analyzer = SCAnalyzer.Analyzer
|
||||
|
||||
var rules = map[string]callcheck.Check{
|
||||
"encoding/json.Marshal": checkJSON,
|
||||
"encoding/json.MarshalIndent": checkJSON,
|
||||
"encoding/xml.Marshal": checkXML,
|
||||
"encoding/xml.MarshalIndent": checkXML,
|
||||
"(*encoding/json.Encoder).Encode": checkJSON,
|
||||
"(*encoding/xml.Encoder).Encode": checkXML,
|
||||
}
|
||||
|
||||
+1
-1
@@ -128,7 +128,7 @@ func validateFloatFormat(arg *callcheck.Argument) {
|
||||
switch val {
|
||||
case 'b', 'e', 'E', 'f', 'g', 'G', 'x', 'X':
|
||||
default:
|
||||
arg.Invalid(fmt.Sprintf("'fmt' argument is invalid: unknown format %q", val))
|
||||
arg.Invalid(fmt.Sprintf("'fmt' argument is invalid: unknown format %q", rune(val)))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+4
-1
@@ -19,7 +19,10 @@ var SCAnalyzer = lint.InitializeAnalyzer(&lint.Analyzer{
|
||||
Requires: code.RequiredAnalyzers,
|
||||
},
|
||||
Doc: &lint.RawDocumentation{
|
||||
Title: `\'sync.WaitGroup.Add\' called inside the goroutine, leading to a race condition`,
|
||||
Title: `\'(*sync.WaitGroup).Add\' called inside the goroutine, leading to a race condition`,
|
||||
Text: `\'(*sync.WaitGroup).Add\' must be called before starting the goroutine
|
||||
it is meant to wait for. Calling \'Add\' inside the goroutine creates a race
|
||||
condition between the call to \'Add\' and the call to \'Wait\'.`,
|
||||
Since: "2017.1",
|
||||
Severity: lint.SeverityWarning,
|
||||
MergeIf: lint.MergeIfAny,
|
||||
|
||||
+1
-2
@@ -7,7 +7,6 @@ import (
|
||||
"honnef.co/go/tools/analysis/code"
|
||||
"honnef.co/go/tools/analysis/lint"
|
||||
"honnef.co/go/tools/analysis/report"
|
||||
"honnef.co/go/tools/go/ast/astutil"
|
||||
|
||||
"golang.org/x/tools/go/analysis"
|
||||
"golang.org/x/tools/go/analysis/passes/inspect"
|
||||
@@ -67,7 +66,7 @@ func run(pass *analysis.Pass) (any, error) {
|
||||
if !ok {
|
||||
return nil, "", false
|
||||
}
|
||||
call, ok := astutil.Unparen(expr.X).(*ast.CallExpr)
|
||||
call, ok := ast.Unparen(expr.X).(*ast.CallExpr)
|
||||
if !ok {
|
||||
return nil, "", false
|
||||
}
|
||||
|
||||
+14
-2
@@ -20,7 +20,19 @@ var SCAnalyzer = lint.InitializeAnalyzer(&lint.Analyzer{
|
||||
Requires: []*analysis.Analyzer{buildir.Analyzer},
|
||||
},
|
||||
Doc: &lint.RawDocumentation{
|
||||
Title: `Deferred \'Lock\' right after locking, likely meant to defer \'Unlock\' instead`,
|
||||
Title: `Deferred \'Lock\' right after locking, likely meant to defer \'Unlock\' instead`,
|
||||
Text: `Deferring a call to \'Lock\' immediately after locking is almost always
|
||||
a typo. For example:
|
||||
|
||||
mu.Lock()
|
||||
defer mu.Lock()
|
||||
|
||||
While this does not strictly guarantee a deadlock depending on how the
|
||||
surrounding code is structured, it is highly likely to be a mistake.
|
||||
The intended code was likely this:
|
||||
|
||||
mu.Lock()
|
||||
defer mu.Unlock()`,
|
||||
Since: "2017.1",
|
||||
Severity: lint.SeverityWarning,
|
||||
MergeIf: lint.MergeIfAny,
|
||||
@@ -32,7 +44,7 @@ var Analyzer = SCAnalyzer.Analyzer
|
||||
func run(pass *analysis.Pass) (any, error) {
|
||||
for _, fn := range pass.ResultOf[buildir.Analyzer].(*buildir.IR).SrcFuncs {
|
||||
for _, block := range fn.Blocks {
|
||||
instrs := irutil.FilterDebug(block.Instrs)
|
||||
instrs := block.Instrs
|
||||
if len(instrs) < 2 {
|
||||
continue
|
||||
}
|
||||
|
||||
+97
-33
@@ -49,12 +49,10 @@ func run(pass *analysis.Pass) (any, error) {
|
||||
fn := func(node ast.Node) {
|
||||
expr := node.(*ast.BinaryExpr)
|
||||
tx := pass.TypesInfo.TypeOf(expr.X)
|
||||
basic, ok := tx.Underlying().(*types.Basic)
|
||||
if !ok {
|
||||
return
|
||||
}
|
||||
tset := typeutil.NewTypeSet(tx)
|
||||
|
||||
// We only check for the math constants and integer literals, not for all constant expressions. This is to avoid
|
||||
// We only check for the math constants and integer literals, not for
|
||||
// all constant expressions. This is to avoid
|
||||
// false positives when constant values differ under different build tags.
|
||||
var (
|
||||
maxMathConst string
|
||||
@@ -63,6 +61,54 @@ func run(pass *analysis.Pass) (any, error) {
|
||||
minLiteral constant.Value
|
||||
)
|
||||
|
||||
allUnsigned := tset.All(func(t *types.Term) bool {
|
||||
if basic, ok := t.Type().Underlying().(*types.Basic); ok {
|
||||
return basic.Info()&types.IsUnsigned != 0
|
||||
}
|
||||
return false
|
||||
})
|
||||
|
||||
if allUnsigned {
|
||||
isZeroLiteral := func(expr ast.Expr) bool {
|
||||
return code.IsIntegerLiteral(pass, expr, constant.MakeInt64(0))
|
||||
}
|
||||
if (expr.Op == token.LSS && isZeroLiteral(expr.Y)) ||
|
||||
(expr.Op == token.GTR && isZeroLiteral(expr.X)) {
|
||||
report.Report(
|
||||
pass,
|
||||
expr,
|
||||
fmt.Sprintf("no value of type %s is less than 0", tx),
|
||||
report.FilterGenerated(),
|
||||
)
|
||||
}
|
||||
if expr.Op == token.GEQ && isZeroLiteral(expr.Y) ||
|
||||
expr.Op == token.LEQ && isZeroLiteral(expr.X) {
|
||||
report.Report(
|
||||
pass,
|
||||
expr,
|
||||
fmt.Sprintf("every value of type %s is >= 0", tx),
|
||||
report.FilterGenerated(),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
core := tset.CoreType()
|
||||
if core == nil {
|
||||
// All remaining checks are only relevant when the type set
|
||||
// contains a single underlying type.
|
||||
//
|
||||
// If we had a 'var x uint8 | uint16',
|
||||
// then the type checker wouldn't permit a check such as
|
||||
// 'if x <= math.MaxUint16', because the constant cannot be converted to all
|
||||
// types in the type set.
|
||||
return
|
||||
}
|
||||
|
||||
basic, ok := core.(*types.Basic)
|
||||
if !ok {
|
||||
return
|
||||
}
|
||||
|
||||
switch basic.Kind() {
|
||||
case types.Uint8:
|
||||
maxMathConst = "math.MaxUint8"
|
||||
@@ -81,7 +127,8 @@ func run(pass *analysis.Pass) (any, error) {
|
||||
minLiteral = constant.MakeUint64(0)
|
||||
maxLiteral = constant.MakeUint64(math.MaxUint64)
|
||||
case types.Uint:
|
||||
// TODO(dh): we could chose 32 bit vs 64 bit depending on the file's build tags
|
||||
// TODO(dh): we could chose 32 bit vs 64 bit depending on the
|
||||
// file's build tags
|
||||
maxMathConst = "math.MaxUint64"
|
||||
minLiteral = constant.MakeUint64(0)
|
||||
maxLiteral = constant.MakeUint64(math.MaxUint64)
|
||||
@@ -107,7 +154,8 @@ func run(pass *analysis.Pass) (any, error) {
|
||||
minLiteral = constant.MakeInt64(math.MinInt64)
|
||||
maxLiteral = constant.MakeInt64(math.MaxInt64)
|
||||
case types.Int:
|
||||
// TODO(dh): we could chose 32 bit vs 64 bit depending on the file's build tags
|
||||
// TODO(dh): we could chose 32 bit vs 64 bit depending on the
|
||||
// file's build tags
|
||||
minMathConst = "math.MinInt64"
|
||||
maxMathConst = "math.MaxInt64"
|
||||
minLiteral = constant.MakeInt64(math.MinInt64)
|
||||
@@ -120,37 +168,53 @@ func run(pass *analysis.Pass) (any, error) {
|
||||
}
|
||||
return code.IsIntegerLiteral(pass, expr, c)
|
||||
}
|
||||
isZeroLiteral := func(expr ast.Expr) bool {
|
||||
return code.IsIntegerLiteral(pass, expr, constant.MakeInt64(0))
|
||||
|
||||
x, y, op := expr.X, expr.Y, expr.Op
|
||||
switch op {
|
||||
case token.GEQ, token.GTR:
|
||||
case token.LEQ:
|
||||
x, y = y, x
|
||||
op = token.GEQ
|
||||
case token.LSS:
|
||||
x, y = y, x
|
||||
op = token.GTR
|
||||
default:
|
||||
return
|
||||
}
|
||||
|
||||
if (expr.Op == token.GTR || expr.Op == token.GEQ) && (isobj(expr.Y, maxMathConst) || isLiteral(expr.Y, maxLiteral)) ||
|
||||
(expr.Op == token.LSS || expr.Op == token.LEQ) && (isobj(expr.X, maxMathConst) || isLiteral(expr.X, maxLiteral)) {
|
||||
report.Report(pass, expr, fmt.Sprintf("no value of type %s is greater than %s", basic, maxMathConst), report.FilterGenerated())
|
||||
if isobj(y, maxMathConst) || isLiteral(y, maxLiteral) {
|
||||
report.Report(
|
||||
pass,
|
||||
expr,
|
||||
fmt.Sprintf("no value of type %s is greater than %s", tx, maxMathConst),
|
||||
report.FilterGenerated(),
|
||||
)
|
||||
}
|
||||
if op == token.GEQ && (isobj(x, maxMathConst) || isLiteral(x, maxLiteral)) {
|
||||
report.Report(
|
||||
pass,
|
||||
expr,
|
||||
fmt.Sprintf("every value of type %s is <= %s", tx, maxMathConst),
|
||||
report.FilterGenerated(),
|
||||
)
|
||||
}
|
||||
|
||||
if expr.Op == token.LEQ && (isobj(expr.Y, maxMathConst) || isLiteral(expr.Y, maxLiteral)) ||
|
||||
expr.Op == token.GEQ && (isobj(expr.X, maxMathConst) || isLiteral(expr.X, maxLiteral)) {
|
||||
report.Report(pass, expr, fmt.Sprintf("every value of type %s is <= %s", basic, maxMathConst), report.FilterGenerated())
|
||||
}
|
||||
|
||||
if (basic.Info() & types.IsUnsigned) != 0 {
|
||||
if (expr.Op == token.LSS && isZeroLiteral(expr.Y)) ||
|
||||
(expr.Op == token.GTR && isZeroLiteral(expr.X)) {
|
||||
report.Report(pass, expr, fmt.Sprintf("no value of type %s is less than 0", basic), report.FilterGenerated())
|
||||
if !allUnsigned {
|
||||
if isobj(x, minMathConst) || isLiteral(x, minLiteral) {
|
||||
report.Report(
|
||||
pass,
|
||||
expr,
|
||||
fmt.Sprintf("no value of type %s is less than %s", tx, minMathConst),
|
||||
report.FilterGenerated(),
|
||||
)
|
||||
}
|
||||
if expr.Op == token.GEQ && isZeroLiteral(expr.Y) ||
|
||||
expr.Op == token.LEQ && isZeroLiteral(expr.X) {
|
||||
report.Report(pass, expr, fmt.Sprintf("every value of type %s is >= 0", basic), report.FilterGenerated())
|
||||
}
|
||||
} else {
|
||||
if (expr.Op == token.LSS || expr.Op == token.LEQ) && (isobj(expr.Y, minMathConst) || isLiteral(expr.Y, minLiteral)) ||
|
||||
(expr.Op == token.GTR || expr.Op == token.GEQ) && (isobj(expr.X, minMathConst) || isLiteral(expr.X, minLiteral)) {
|
||||
report.Report(pass, expr, fmt.Sprintf("no value of type %s is less than %s", basic, minMathConst), report.FilterGenerated())
|
||||
}
|
||||
if expr.Op == token.GEQ && (isobj(expr.Y, minMathConst) || isLiteral(expr.Y, minLiteral)) ||
|
||||
expr.Op == token.LEQ && (isobj(expr.X, minMathConst) || isLiteral(expr.X, minLiteral)) {
|
||||
report.Report(pass, expr, fmt.Sprintf("every value of type %s is >= %s", basic, minMathConst), report.FilterGenerated())
|
||||
if op == token.GEQ && (isobj(y, minMathConst) || isLiteral(y, minLiteral)) {
|
||||
report.Report(
|
||||
pass,
|
||||
expr,
|
||||
fmt.Sprintf("every value of type %s is >= %s", tx, minMathConst),
|
||||
report.FilterGenerated(),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
+1
-5
@@ -50,7 +50,7 @@ fnLoop:
|
||||
}
|
||||
|
||||
recv := fn.Params[0]
|
||||
refs := irutil.FilterDebug(*recv.Referrers())
|
||||
refs := *recv.Referrers()
|
||||
if len(refs) != 1 {
|
||||
continue
|
||||
}
|
||||
@@ -74,8 +74,6 @@ fnLoop:
|
||||
writes[ref.Field] = append(writes[ref.Field], refref)
|
||||
case *ir.Load:
|
||||
reads[ref.Field] = append(reads[ref.Field], refref)
|
||||
case *ir.DebugRef:
|
||||
continue
|
||||
default:
|
||||
// this should be safe… if the field address
|
||||
// escapes, then alloc.Heap will be true.
|
||||
@@ -98,8 +96,6 @@ fnLoop:
|
||||
for i := 0; i < recv.Type().Underlying().(*types.Struct).NumFields(); i++ {
|
||||
reads[i] = append(reads[i], ref)
|
||||
}
|
||||
case *ir.DebugRef:
|
||||
continue
|
||||
default:
|
||||
continue fnLoop
|
||||
}
|
||||
|
||||
+23
-10
@@ -3,6 +3,7 @@ package sa4006
|
||||
import (
|
||||
"fmt"
|
||||
"go/ast"
|
||||
"go/token"
|
||||
|
||||
"honnef.co/go/tools/analysis/code"
|
||||
"honnef.co/go/tools/analysis/facts/generated"
|
||||
@@ -74,15 +75,6 @@ func run(pass *analysis.Pass) (any, error) {
|
||||
}
|
||||
for _, ref := range *refs {
|
||||
switch ref := ref.(type) {
|
||||
case *ir.DebugRef:
|
||||
case *ir.Sigma:
|
||||
if seen == nil {
|
||||
seen = map[ir.Value]struct{}{}
|
||||
}
|
||||
seen[v] = struct{}{}
|
||||
if hasUse(ref, seen) {
|
||||
return true
|
||||
}
|
||||
case *ir.Phi:
|
||||
if seen == nil {
|
||||
seen = map[ir.Value]struct{}{}
|
||||
@@ -99,6 +91,22 @@ func run(pass *analysis.Pass) (any, error) {
|
||||
}
|
||||
|
||||
ast.Inspect(node, func(node ast.Node) bool {
|
||||
inc, ok := node.(*ast.IncDecStmt)
|
||||
if ok {
|
||||
val, _ := fn.ValueForExpr(inc.X)
|
||||
if val == nil {
|
||||
return true
|
||||
}
|
||||
if _, ok := val.(*ir.Const); ok {
|
||||
// a zero-valued constant, for example in 'foo := []string(nil)'
|
||||
return true
|
||||
}
|
||||
if !hasUse(val, nil) {
|
||||
report.Report(pass, inc, fmt.Sprintf("this value of %s is never used", inc.X))
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
assign, ok := node.(*ast.AssignStmt)
|
||||
if !ok {
|
||||
return true
|
||||
@@ -137,7 +145,12 @@ func run(pass *analysis.Pass) (any, error) {
|
||||
}
|
||||
val, _ := fn.ValueForExpr(rhs)
|
||||
if val == nil {
|
||||
continue
|
||||
if assign.Tok != token.ASSIGN { // +=, *=, etc.
|
||||
val, _ = fn.ValueForExpr(lhs)
|
||||
}
|
||||
if val == nil {
|
||||
continue
|
||||
}
|
||||
}
|
||||
|
||||
if _, ok := val.(*ir.Const); ok {
|
||||
|
||||
+2
-16
@@ -82,23 +82,9 @@ func run(pass *analysis.Pass) (any, error) {
|
||||
if v == nil || isAddr {
|
||||
return true
|
||||
}
|
||||
switch v := v.(type) {
|
||||
switch v.(type) {
|
||||
case *ir.Phi:
|
||||
ops := v.Operands(nil)
|
||||
if len(ops) != 2 {
|
||||
return true
|
||||
}
|
||||
_, ok := (*ops[0]).(*ir.Const)
|
||||
if !ok {
|
||||
return true
|
||||
}
|
||||
sigma, ok := (*ops[1]).(*ir.Sigma)
|
||||
if !ok {
|
||||
return true
|
||||
}
|
||||
if sigma.X != v {
|
||||
return true
|
||||
}
|
||||
return true
|
||||
case *ir.Load:
|
||||
return true
|
||||
}
|
||||
|
||||
+1
-2
@@ -7,7 +7,6 @@ import (
|
||||
"honnef.co/go/tools/analysis/lint"
|
||||
"honnef.co/go/tools/analysis/report"
|
||||
"honnef.co/go/tools/go/ir"
|
||||
"honnef.co/go/tools/go/ir/irutil"
|
||||
"honnef.co/go/tools/internal/passes/buildir"
|
||||
|
||||
"golang.org/x/tools/go/analysis"
|
||||
@@ -65,7 +64,7 @@ func run(pass *analysis.Pass) (any, error) {
|
||||
if refs == nil {
|
||||
continue
|
||||
}
|
||||
if len(irutil.FilterDebug(*refs)) != 0 {
|
||||
if len(*refs) != 0 {
|
||||
continue
|
||||
}
|
||||
|
||||
|
||||
+26
-32
@@ -16,7 +16,10 @@ var SCAnalyzer = lint.InitializeAnalyzer(&lint.Analyzer{
|
||||
Requires: []*analysis.Analyzer{buildir.Analyzer},
|
||||
},
|
||||
Doc: &lint.RawDocumentation{
|
||||
Title: `The result of \'append\' will never be observed anywhere`,
|
||||
Title: `The result of \'append\' will never be observed anywhere`,
|
||||
Text: `Calls to \'append\' produce a new slice value. When the result of
|
||||
\'append\' is assigned to a variable that is never subsequently read, the
|
||||
append operation may have an unintended effect.`,
|
||||
Since: "2017.1",
|
||||
Severity: lint.SeverityWarning,
|
||||
MergeIf: lint.MergeIfAll,
|
||||
@@ -54,7 +57,6 @@ func run(pass *analysis.Pass) (any, error) {
|
||||
// This graph must only consist of the following instructions:
|
||||
//
|
||||
// - phi
|
||||
// - sigma
|
||||
// - slice
|
||||
// - const nil
|
||||
// - MakeSlice
|
||||
@@ -62,8 +64,7 @@ func run(pass *analysis.Pass) (any, error) {
|
||||
// - calls to append
|
||||
//
|
||||
// If this step succeeds, we look at all referrers of the values found in the first step, recursively.
|
||||
// These referrers must either be in the set of values found in the first step,
|
||||
// be DebugRefs,
|
||||
// These referrers must either be in the set of values found in the first step
|
||||
// or fulfill the same type requirements as step 1, with the exception of appends, which are forbidden.
|
||||
//
|
||||
// If both steps succeed then we know that the backing array hasn't been aliased in an observable manner.
|
||||
@@ -90,8 +91,6 @@ func run(pass *analysis.Pass) (any, error) {
|
||||
}
|
||||
}
|
||||
return true
|
||||
case *ir.Sigma:
|
||||
return validateArgument(v.X, seen)
|
||||
case *ir.Slice:
|
||||
return validateArgument(v.X, seen)
|
||||
case *ir.Const:
|
||||
@@ -112,28 +111,27 @@ func run(pass *analysis.Pass) (any, error) {
|
||||
|
||||
var validateReferrers func(v ir.Value, seen map[ir.Instruction]struct{}) bool
|
||||
validateReferrers = func(v ir.Value, seen map[ir.Instruction]struct{}) bool {
|
||||
for _, ref := range *v.Referrers() {
|
||||
if _, ok := seen[ref]; ok {
|
||||
continue
|
||||
}
|
||||
if refs := v.Referrers(); refs != nil {
|
||||
for _, ref := range *refs {
|
||||
if _, ok := seen[ref]; ok {
|
||||
continue
|
||||
}
|
||||
|
||||
seen[ref] = struct{}{}
|
||||
switch ref.(type) {
|
||||
case *ir.Phi:
|
||||
case *ir.Sigma:
|
||||
case *ir.Slice:
|
||||
case *ir.Const:
|
||||
case *ir.MakeSlice:
|
||||
case *ir.Alloc:
|
||||
case *ir.DebugRef:
|
||||
default:
|
||||
return false
|
||||
}
|
||||
|
||||
if ref, ok := ref.(ir.Value); ok {
|
||||
if !validateReferrers(ref, seen) {
|
||||
seen[ref] = struct{}{}
|
||||
switch ref.(type) {
|
||||
case *ir.Phi:
|
||||
case *ir.Slice:
|
||||
case *ir.MakeSlice:
|
||||
case *ir.Alloc:
|
||||
default:
|
||||
return false
|
||||
}
|
||||
|
||||
if ref, ok := ref.(ir.Value); ok {
|
||||
if !validateReferrers(ref, seen) {
|
||||
return false
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
return true
|
||||
@@ -157,14 +155,9 @@ func run(pass *analysis.Pass) (any, error) {
|
||||
continue
|
||||
}
|
||||
visited[ref] = true
|
||||
if _, ok := ref.(*ir.DebugRef); ok {
|
||||
continue
|
||||
}
|
||||
switch ref := ref.(type) {
|
||||
case *ir.Phi:
|
||||
walkRefs(*ref.Referrers())
|
||||
case *ir.Sigma:
|
||||
walkRefs(*ref.Referrers())
|
||||
case ir.Value:
|
||||
if !isAppend(ref) {
|
||||
isUsed = true
|
||||
@@ -195,8 +188,9 @@ func run(pass *analysis.Pass) (any, error) {
|
||||
|
||||
seen2 := map[ir.Instruction]struct{}{}
|
||||
for k := range seen {
|
||||
// the only values we allow are also instructions, so this type assertion cannot fail
|
||||
seen2[k.(ir.Instruction)] = struct{}{}
|
||||
if k, ok := k.(ir.Instruction); ok {
|
||||
seen2[k] = struct{}{}
|
||||
}
|
||||
}
|
||||
seen2[ins] = struct{}{}
|
||||
failed := false
|
||||
|
||||
+1
-2
@@ -9,7 +9,6 @@ import (
|
||||
"honnef.co/go/tools/analysis/lint"
|
||||
"honnef.co/go/tools/analysis/report"
|
||||
"honnef.co/go/tools/go/ir"
|
||||
"honnef.co/go/tools/go/ir/irutil"
|
||||
"honnef.co/go/tools/go/types/typeutil"
|
||||
"honnef.co/go/tools/internal/passes/buildir"
|
||||
|
||||
@@ -60,7 +59,7 @@ fnLoop:
|
||||
continue
|
||||
}
|
||||
refs := ins.Referrers()
|
||||
if refs == nil || len(irutil.FilterDebug(*refs)) > 0 {
|
||||
if refs == nil || len(*refs) > 0 {
|
||||
continue
|
||||
}
|
||||
|
||||
|
||||
+34
-20
@@ -7,7 +7,6 @@ import (
|
||||
|
||||
"honnef.co/go/tools/analysis/code"
|
||||
"honnef.co/go/tools/analysis/facts/nilness"
|
||||
"honnef.co/go/tools/analysis/facts/typedness"
|
||||
"honnef.co/go/tools/analysis/lint"
|
||||
"honnef.co/go/tools/analysis/report"
|
||||
"honnef.co/go/tools/go/ir"
|
||||
@@ -23,7 +22,7 @@ var SCAnalyzer = lint.InitializeAnalyzer(&lint.Analyzer{
|
||||
Analyzer: &analysis.Analyzer{
|
||||
Name: "SA4023",
|
||||
Run: run,
|
||||
Requires: []*analysis.Analyzer{buildir.Analyzer, typedness.Analysis, nilness.Analysis},
|
||||
Requires: []*analysis.Analyzer{buildir.Analyzer, nilness.Analysis},
|
||||
},
|
||||
Doc: &lint.RawDocumentation{
|
||||
Title: `Impossible comparison of interface value with untyped nil`,
|
||||
@@ -102,8 +101,7 @@ func run(pass *analysis.Pass) (any, error) {
|
||||
// known typed nils, or typed unknown nilness are being returned.
|
||||
|
||||
irpkg := pass.ResultOf[buildir.Analyzer].(*buildir.IR)
|
||||
typedness := pass.ResultOf[typedness.Analysis].(*typedness.Result)
|
||||
nilness := pass.ResultOf[nilness.Analysis].(*nilness.Result)
|
||||
nilnessRes := pass.ResultOf[nilness.Analysis].(*nilness.Result)
|
||||
for _, fn := range irpkg.SrcFuncs {
|
||||
for _, b := range fn.Blocks {
|
||||
for _, instr := range b.Instrs {
|
||||
@@ -111,17 +109,16 @@ func run(pass *analysis.Pass) (any, error) {
|
||||
if !ok || !(binop.Op == token.EQL || binop.Op == token.NEQ) {
|
||||
continue
|
||||
}
|
||||
if _, ok := binop.X.Type().Underlying().(*types.Interface); !ok || typeparams.IsTypeParam(binop.X.Type()) {
|
||||
if !types.IsInterface(binop.X.Type()) || typeparams.IsTypeParam(binop.X.Type()) {
|
||||
// TODO support swapped X and Y
|
||||
continue
|
||||
}
|
||||
|
||||
k, ok := binop.Y.(*ir.Const)
|
||||
if !ok || !k.IsNil() {
|
||||
// if binop.X is an interface, then binop.Y can
|
||||
// only be a Const if its untyped. A typed nil
|
||||
// constant would first be passed to
|
||||
// MakeInterface.
|
||||
// if binop.X is an interface, then binop.Y can only be a
|
||||
// Const if its untyped. A typed nil constant would first
|
||||
// be passed to MakeInterface.
|
||||
continue
|
||||
}
|
||||
|
||||
@@ -159,8 +156,9 @@ func run(pass *analysis.Pass) (any, error) {
|
||||
|
||||
terms, err := typeparams.NormalTerms(x.X.Type())
|
||||
if len(terms) == 0 || err != nil {
|
||||
// Type is a type parameter with no type terms (or we couldn't determine the terms). Such a type
|
||||
// _can_ be nil when put in an interface value.
|
||||
// Type is a type parameter with no type terms (or we
|
||||
// couldn't determine the terms). Such a type _can_ be
|
||||
// nil when put in an interface value.
|
||||
continue
|
||||
}
|
||||
|
||||
@@ -168,8 +166,11 @@ func run(pass *analysis.Pass) (any, error) {
|
||||
report.Report(pass, binop, fmt.Sprintf("this comparison is %s true", qualifier),
|
||||
report.Related(x.X, "the lhs of the comparison gets its value from here and has a concrete type"))
|
||||
} else {
|
||||
// we can't generate related information for this, so make the diagnostic itself slightly more useful
|
||||
report.Report(pass, binop, fmt.Sprintf("this comparison is %s true; the lhs of the comparison has been assigned a concretely typed value", qualifier))
|
||||
// we can't generate related information for this, so
|
||||
// make the diagnostic itself slightly more useful
|
||||
report.Report(pass, binop,
|
||||
fmt.Sprintf("this comparison is %s true; the lhs of the comparison has been assigned a concretely typed value",
|
||||
qualifier))
|
||||
}
|
||||
continue
|
||||
}
|
||||
@@ -177,11 +178,21 @@ func run(pass *analysis.Pass) (any, error) {
|
||||
continue
|
||||
}
|
||||
|
||||
isNil, onlyGlobal := nilness.MayReturnNil(obj, idx)
|
||||
if typedness.MustReturnTyped(obj, idx) && isNil && !onlyGlobal && !code.IsInTest(pass, binop) {
|
||||
// Don't flag these comparisons in tests. Tests
|
||||
// may be explicitly enforcing the invariant that
|
||||
// a value isn't nil.
|
||||
nillity := nilnessRes.Nilness(obj, idx)
|
||||
// We aim to detect interface values that are never nil (i.e.,
|
||||
// always typed) but may contain typed nils. This usually
|
||||
// points to a bug in the producer of the value. We
|
||||
// intentionally do not flag comparisons with non-nil interface
|
||||
// values whose stored value is also never nil (such as the
|
||||
// return value of errors.New)--those are more likely to be
|
||||
// defensive code, protecting against a future untyped nil.
|
||||
if nillity.Outer == nilness.NeverNil &&
|
||||
(nillity.Inner == nilness.MaybeNil || nillity.Inner == nilness.AlwaysNil) &&
|
||||
!code.IsInTest(pass, binop) &&
|
||||
!irutil.IsTrivial(irpkg.Pkg.Prog.FuncValue(obj)) {
|
||||
// Don't flag these comparisons in tests. Tests may be
|
||||
// explicitly enforcing the invariant that a value isn't
|
||||
// nil.
|
||||
|
||||
var qualifier string
|
||||
switch binop.Op {
|
||||
@@ -194,8 +205,11 @@ func run(pass *analysis.Pass) (any, error) {
|
||||
}
|
||||
report.Report(pass, binop, fmt.Sprintf("this comparison is %s true", qualifier),
|
||||
// TODO support swapped X and Y
|
||||
report.Related(binop.X, fmt.Sprintf("the lhs of the comparison is the %s return value of this function call", report.Ordinal(idx+1))),
|
||||
report.Related(obj, fmt.Sprintf("%s never returns a nil interface value", typeutil.FuncName(obj))))
|
||||
report.Related(binop.X,
|
||||
fmt.Sprintf("the lhs of the comparison is the %s return value of this function call",
|
||||
report.Ordinal(idx+1))),
|
||||
report.Related(obj,
|
||||
fmt.Sprintf("%s never returns a nil interface value", typeutil.FuncName(obj))))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -95,8 +95,6 @@ func run(pass *analysis.Pass) (any, error) {
|
||||
values = append(values, v)
|
||||
}
|
||||
return neverNil(v.X, false)
|
||||
case *ir.Sigma:
|
||||
return neverNil(v.X, true)
|
||||
case *ir.Phi:
|
||||
for _, e := range v.Edges {
|
||||
if !neverNil(e, true) {
|
||||
|
||||
+2
-7
@@ -47,16 +47,11 @@ func run(pass *analysis.Pass) (any, error) {
|
||||
}
|
||||
|
||||
block := site.Block()
|
||||
for _, b := range fn.Blocks {
|
||||
for b := range fn.Exits() {
|
||||
if block.Dominates(b) {
|
||||
continue
|
||||
}
|
||||
if len(b.Instrs) == 0 {
|
||||
continue
|
||||
}
|
||||
if _, ok := b.Control().(*ir.Return); ok {
|
||||
return
|
||||
}
|
||||
return
|
||||
}
|
||||
report.Report(pass, site, "infinite recursive call")
|
||||
})
|
||||
|
||||
+1
-1
@@ -141,7 +141,7 @@ func validateJSONTag(pass *analysis.Pass, field *ast.Field, tag string) {
|
||||
report.Report(pass, field.Tag,
|
||||
fmt.Sprintf("invalid appearance of unknown `case:%s` tag value", rawOpt))
|
||||
}
|
||||
case "inline":
|
||||
case "embed", "inline":
|
||||
case "unknown":
|
||||
case "omitzero":
|
||||
case "omitempty":
|
||||
|
||||
+1
-1
@@ -252,7 +252,7 @@ func checkImpl(carg *callcheck.Argument, f ir.Value, args []ir.Value) {
|
||||
}
|
||||
}
|
||||
|
||||
if flags&isPointer != 0 && typeutil.IsPointerLike(T) {
|
||||
if flags&isPointer != 0 && typeutil.MaybePointerLike(T) {
|
||||
return true
|
||||
}
|
||||
if flags&isPseudoPointer != 0 {
|
||||
|
||||
@@ -1,221 +0,0 @@
|
||||
package sa5011
|
||||
|
||||
import (
|
||||
"go/types"
|
||||
|
||||
"honnef.co/go/tools/analysis/lint"
|
||||
"honnef.co/go/tools/analysis/report"
|
||||
"honnef.co/go/tools/go/ir"
|
||||
"honnef.co/go/tools/go/types/typeutil"
|
||||
"honnef.co/go/tools/internal/passes/buildir"
|
||||
|
||||
"golang.org/x/tools/go/analysis"
|
||||
)
|
||||
|
||||
var SCAnalyzer = lint.InitializeAnalyzer(&lint.Analyzer{
|
||||
Analyzer: &analysis.Analyzer{
|
||||
Name: "SA5011",
|
||||
Run: run,
|
||||
Requires: []*analysis.Analyzer{buildir.Analyzer},
|
||||
},
|
||||
Doc: &lint.RawDocumentation{
|
||||
Title: `Possible nil pointer dereference`,
|
||||
|
||||
Text: `A pointer is being dereferenced unconditionally, while
|
||||
also being checked against nil in another place. This suggests that
|
||||
the pointer may be nil and dereferencing it may panic. This is
|
||||
commonly a result of improperly ordered code or missing return
|
||||
statements. Consider the following examples:
|
||||
|
||||
func fn(x *int) {
|
||||
fmt.Println(*x)
|
||||
|
||||
// This nil check is equally important for the previous dereference
|
||||
if x != nil {
|
||||
foo(*x)
|
||||
}
|
||||
}
|
||||
|
||||
func TestFoo(t *testing.T) {
|
||||
x := compute()
|
||||
if x == nil {
|
||||
t.Errorf("nil pointer received")
|
||||
}
|
||||
|
||||
// t.Errorf does not abort the test, so if x is nil, the next line will panic.
|
||||
foo(*x)
|
||||
}
|
||||
|
||||
Staticcheck tries to deduce which functions abort control flow.
|
||||
For example, it is aware that a function will not continue
|
||||
execution after a call to \'panic\' or \'log.Fatal\'. However, sometimes
|
||||
this detection fails, in particular in the presence of
|
||||
conditionals. Consider the following example:
|
||||
|
||||
func Log(msg string, level int) {
|
||||
fmt.Println(msg)
|
||||
if level == levelFatal {
|
||||
os.Exit(1)
|
||||
}
|
||||
}
|
||||
|
||||
func Fatal(msg string) {
|
||||
Log(msg, levelFatal)
|
||||
}
|
||||
|
||||
func fn(x *int) {
|
||||
if x == nil {
|
||||
Fatal("unexpected nil pointer")
|
||||
}
|
||||
fmt.Println(*x)
|
||||
}
|
||||
|
||||
Staticcheck will flag the dereference of \'x\', even though it is perfectly
|
||||
safe. Staticcheck is not able to deduce that a call to
|
||||
Fatal will exit the program. For the time being, the easiest
|
||||
workaround is to modify the definition of Fatal like so:
|
||||
|
||||
func Fatal(msg string) {
|
||||
Log(msg, levelFatal)
|
||||
panic("unreachable")
|
||||
}
|
||||
|
||||
We also hard-code functions from common logging packages such as
|
||||
logrus. Please file an issue if we're missing support for a
|
||||
popular package.`,
|
||||
Since: "2020.1",
|
||||
Severity: lint.SeverityWarning,
|
||||
MergeIf: lint.MergeIfAny,
|
||||
},
|
||||
})
|
||||
|
||||
var Analyzer = SCAnalyzer.Analyzer
|
||||
|
||||
func run(pass *analysis.Pass) (any, error) {
|
||||
// This is an extremely trivial check that doesn't try to reason
|
||||
// about control flow. That is, phis and sigmas do not propagate
|
||||
// any information. As such, we can flag this:
|
||||
//
|
||||
// _ = *x
|
||||
// if x == nil { return }
|
||||
//
|
||||
// but we cannot flag this:
|
||||
//
|
||||
// if x == nil { println(x) }
|
||||
// _ = *x
|
||||
//
|
||||
// but we can flag this, because the if's body doesn't use x:
|
||||
//
|
||||
// if x == nil { println("this is bad") }
|
||||
// _ = *x
|
||||
//
|
||||
// nor many other variations of conditional uses of or assignments to x.
|
||||
//
|
||||
// However, even this trivial implementation finds plenty of
|
||||
// real-world bugs, such as dereference before nil pointer check,
|
||||
// or using t.Error instead of t.Fatal when encountering nil
|
||||
// pointers.
|
||||
//
|
||||
// On the flip side, our naive implementation avoids false positives in branches, such as
|
||||
//
|
||||
// if x != nil { _ = *x }
|
||||
//
|
||||
// due to the same lack of propagating information through sigma
|
||||
// nodes. x inside the branch will be independent of the x in the
|
||||
// nil pointer check.
|
||||
//
|
||||
//
|
||||
// We could implement a more powerful check, but then we'd be
|
||||
// getting false positives instead of false negatives because
|
||||
// we're incapable of deducing relationships between variables.
|
||||
// For example, a function might return a pointer and an error,
|
||||
// and the error being nil guarantees that the pointer is not nil.
|
||||
// Depending on the surrounding code, the pointer may still end up
|
||||
// being checked against nil in one place, and guarded by a check
|
||||
// on the error in another, which would lead to us marking some
|
||||
// loads as unsafe.
|
||||
//
|
||||
// Unfortunately, simply hard-coding the relationship between
|
||||
// return values wouldn't eliminate all false positives, either.
|
||||
// Many other more subtle relationships exist. An abridged example
|
||||
// from real code:
|
||||
//
|
||||
// if a == nil && b == nil { return }
|
||||
// c := fn(a)
|
||||
// if c != "" { _ = *a }
|
||||
//
|
||||
// where `fn` is guaranteed to return a non-empty string if a
|
||||
// isn't nil.
|
||||
//
|
||||
// We choose to err on the side of false negatives.
|
||||
|
||||
isNilConst := func(v ir.Value) bool {
|
||||
if typeutil.IsPointerLike(v.Type()) {
|
||||
if k, ok := v.(*ir.Const); ok {
|
||||
return k.IsNil()
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
for _, fn := range pass.ResultOf[buildir.Analyzer].(*buildir.IR).SrcFuncs {
|
||||
maybeNil := map[ir.Value]ir.Instruction{}
|
||||
for _, b := range fn.Blocks {
|
||||
for _, instr := range b.Instrs {
|
||||
// Originally we looked at all ir.BinOp, but that would lead to calls like 'assert(x != nil)' causing false positives.
|
||||
// Restrict ourselves to actual if statements, as these are more likely to affect control flow in a way we can observe.
|
||||
if instr, ok := instr.(*ir.If); ok {
|
||||
if cond, ok := instr.Cond.(*ir.BinOp); ok {
|
||||
if isNilConst(cond.X) {
|
||||
maybeNil[cond.Y] = cond
|
||||
}
|
||||
if isNilConst(cond.Y) {
|
||||
maybeNil[cond.X] = cond
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
for _, b := range fn.Blocks {
|
||||
for _, instr := range b.Instrs {
|
||||
var ptr ir.Value
|
||||
switch instr := instr.(type) {
|
||||
case *ir.Load:
|
||||
ptr = instr.X
|
||||
case *ir.Store:
|
||||
ptr = instr.Addr
|
||||
case *ir.IndexAddr:
|
||||
ptr = instr.X
|
||||
if typeutil.All(ptr.Type(), func(term *types.Term) bool {
|
||||
if _, ok := term.Type().Underlying().(*types.Slice); ok {
|
||||
return true
|
||||
}
|
||||
return false
|
||||
}) {
|
||||
// indexing a nil slice does not cause a nil pointer panic
|
||||
//
|
||||
// Note: This also works around the bad lowering of range loops over slices
|
||||
// (https://github.com/dominikh/go-tools/issues/1053)
|
||||
continue
|
||||
}
|
||||
case *ir.FieldAddr:
|
||||
ptr = instr.X
|
||||
}
|
||||
if ptr != nil {
|
||||
switch ptr.(type) {
|
||||
case *ir.Alloc, *ir.FieldAddr, *ir.IndexAddr:
|
||||
// these cannot be nil
|
||||
continue
|
||||
}
|
||||
if r, ok := maybeNil[ptr]; ok {
|
||||
report.Report(pass, instr, "possible nil pointer dereference",
|
||||
report.Related(r, "this check suggests that the pointer can be nil"))
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return nil, nil
|
||||
}
|
||||
+3
-2
@@ -12,6 +12,7 @@ import (
|
||||
"honnef.co/go/tools/go/ir"
|
||||
"honnef.co/go/tools/go/ir/irutil"
|
||||
"honnef.co/go/tools/go/types/typeutil"
|
||||
"honnef.co/go/tools/internal/iterutil"
|
||||
"honnef.co/go/tools/internal/passes/buildir"
|
||||
|
||||
"golang.org/x/tools/go/analysis"
|
||||
@@ -140,7 +141,7 @@ func findSliceLenChecks(pass *analysis.Pass) {
|
||||
for _, fn := range pass.ResultOf[buildir.Analyzer].(*buildir.IR).SrcFuncs {
|
||||
for _, b := range fn.Blocks {
|
||||
// all paths go through this block
|
||||
if !b.Dominates(fn.Exit) {
|
||||
if !iterutil.All(fn.Returns(), b.Dominates) {
|
||||
continue
|
||||
}
|
||||
|
||||
@@ -220,7 +221,7 @@ func findIndirectSliceLenChecks(pass *analysis.Pass) {
|
||||
|
||||
for _, b := range fn.Blocks {
|
||||
// all paths go through this block
|
||||
if !b.Dominates(fn.Exit) {
|
||||
if !iterutil.All(fn.Returns(), b.Dominates) {
|
||||
continue
|
||||
}
|
||||
|
||||
|
||||
+11
-20
@@ -85,25 +85,18 @@ func run(pass *analysis.Pass) (any, error) {
|
||||
continue
|
||||
}
|
||||
refs := ins.Referrers()
|
||||
// need at least two (DebugRef) references: the
|
||||
// conversion and the *ast.Ident
|
||||
if refs == nil || len(*refs) < 2 {
|
||||
continue
|
||||
}
|
||||
ident := false
|
||||
// skip first reference, that's the conversion itself
|
||||
for _, ref := range (*refs)[1:] {
|
||||
anyUses := false
|
||||
for _, ref := range *refs {
|
||||
switch ref := ref.(type) {
|
||||
case *ir.DebugRef:
|
||||
if _, ok := ref.Expr.(*ast.Ident); !ok {
|
||||
// the string seems to be used somewhere
|
||||
// unexpected; the default branch should
|
||||
// catch this already, but be safe
|
||||
continue insLoop
|
||||
} else {
|
||||
ident = true
|
||||
}
|
||||
case *ir.MapLookup:
|
||||
anyUses = true
|
||||
if src, ok := ref.Source().(*ast.IndexExpr); ok {
|
||||
if _, ok := src.Index.(*ast.Ident); !ok {
|
||||
continue insLoop
|
||||
}
|
||||
} else {
|
||||
continue insLoop
|
||||
}
|
||||
default:
|
||||
// the string is used somewhere else than a
|
||||
// map lookup
|
||||
@@ -111,9 +104,7 @@ func run(pass *analysis.Pass) (any, error) {
|
||||
}
|
||||
}
|
||||
|
||||
// the result of the conversion wasn't assigned to an
|
||||
// identifier
|
||||
if !ident {
|
||||
if !anyUses {
|
||||
continue
|
||||
}
|
||||
report.Report(pass, ins, "m[string(key)] would be more efficient than k := string(key); m[k]")
|
||||
|
||||
+1
-1
@@ -45,7 +45,7 @@ var rules = map[string]callcheck.Check{
|
||||
arg := call.Args[knowledge.Arg("(*sync.Pool).Put.x")]
|
||||
typ := arg.Value.Value.Type()
|
||||
_, isSlice := typ.Underlying().(*types.Slice)
|
||||
if !typeutil.IsPointerLike(typ) || isSlice {
|
||||
if !typeutil.MaybePointerLike(typ) || isSlice {
|
||||
arg.Invalid("argument should be pointer-like to avoid allocations")
|
||||
}
|
||||
},
|
||||
|
||||
+5
-2
@@ -1,6 +1,7 @@
|
||||
package sa6005
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"go/ast"
|
||||
"go/token"
|
||||
|
||||
@@ -60,16 +61,18 @@ var (
|
||||
func run(pass *analysis.Pass) (any, error) {
|
||||
for node, m := range code.Matches(pass, checkToLowerToUpperComparisonQ) {
|
||||
rn := pattern.NodeToAST(checkToLowerToUpperComparisonR.Root, m.State).(ast.Expr)
|
||||
method := "strings.EqualFold"
|
||||
if m.State["tok"].(token.Token) == token.NEQ {
|
||||
rn = &ast.UnaryExpr{
|
||||
Op: token.NOT,
|
||||
X: rn,
|
||||
}
|
||||
method = "!" + method
|
||||
}
|
||||
|
||||
report.Report(pass, node,
|
||||
"should use strings.EqualFold instead",
|
||||
report.Fixes(edit.Fix("Replace with strings.EqualFold", edit.ReplaceWithNode(pass.Fset, node, rn))))
|
||||
fmt.Sprintf("should use %s instead", method),
|
||||
report.Fixes(edit.Fix("replace with "+method, edit.ReplaceWithNode(pass.Fset, node, rn))))
|
||||
}
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
@@ -50,7 +50,9 @@ var rules = map[string]callcheck.Check{
|
||||
//
|
||||
// Also, should we flag gob?
|
||||
"encoding/json.Marshal": check(knowledge.Arg("json.Marshal.v"), "MarshalJSON", "MarshalText"),
|
||||
"encoding/json.MarshalIndent": check(knowledge.Arg("json.MarshalIndent.v"), "MarshalJSON", "MarshalText"),
|
||||
"encoding/xml.Marshal": check(knowledge.Arg("xml.Marshal.v"), "MarshalXML", "MarshalText"),
|
||||
"encoding/xml.MarshalIndent": check(knowledge.Arg("xml.MarshalIndent.v"), "MarshalXML", "MarshalText"),
|
||||
"(*encoding/json.Encoder).Encode": check(knowledge.Arg("(*encoding/json.Encoder).Encode.v"), "MarshalJSON", "MarshalText"),
|
||||
"(*encoding/xml.Encoder).Encode": check(knowledge.Arg("(*encoding/xml.Encoder).Encode.v"), "MarshalXML", "MarshalText"),
|
||||
|
||||
|
||||
@@ -0,0 +1,60 @@
|
||||
package sa9010
|
||||
|
||||
import (
|
||||
"go/ast"
|
||||
"go/types"
|
||||
|
||||
"honnef.co/go/tools/analysis/code"
|
||||
"honnef.co/go/tools/analysis/lint"
|
||||
"honnef.co/go/tools/analysis/report"
|
||||
|
||||
"golang.org/x/tools/go/analysis"
|
||||
"golang.org/x/tools/go/analysis/passes/inspect"
|
||||
)
|
||||
|
||||
var SCAnalyzer = lint.InitializeAnalyzer(&lint.Analyzer{
|
||||
Analyzer: &analysis.Analyzer{
|
||||
Name: "SA9010",
|
||||
Run: run,
|
||||
Requires: []*analysis.Analyzer{inspect.Analyzer},
|
||||
},
|
||||
Doc: &lint.RawDocumentation{
|
||||
Title: `Returned function should be called in defer`,
|
||||
Text: `
|
||||
If you have a function such as:
|
||||
|
||||
func f() func() {
|
||||
// Do something.
|
||||
return func() {
|
||||
// Do something.
|
||||
}
|
||||
}
|
||||
|
||||
Then calling that in defer:
|
||||
|
||||
defer f()
|
||||
|
||||
Is almost always a mistake, since you typically want to call the returned
|
||||
function:
|
||||
|
||||
defer f()()
|
||||
`,
|
||||
Since: "2026.2",
|
||||
Severity: lint.SeverityWarning,
|
||||
MergeIf: lint.MergeIfAll,
|
||||
},
|
||||
})
|
||||
|
||||
var Analyzer = SCAnalyzer.Analyzer
|
||||
|
||||
func run(pass *analysis.Pass) (any, error) {
|
||||
fn := func(n ast.Node) {
|
||||
def := n.(*ast.DeferStmt)
|
||||
if _, ok := pass.TypesInfo.TypeOf(def.Call).Underlying().(*types.Signature); ok {
|
||||
report.Report(pass, def, "deferred return function not called")
|
||||
}
|
||||
}
|
||||
|
||||
code.Preorder(pass, fn, (*ast.DeferStmt)(nil))
|
||||
return nil, nil
|
||||
}
|
||||
Reference in New Issue
Block a user