From 1e524e26bec7d5b3a643daa7c8bbe4e12ada28f5 Mon Sep 17 00:00:00 2001 From: Ainar Garipov Date: Mon, 8 Mar 2021 12:00:26 +0300 Subject: [PATCH] internal/lsp/source: add the nilness analyzer Fixes golang/go#44627 Change-Id: I4291b419862c7b7df8eff988abe7ab38d03e6189 Reviewed-on: https://go-review.googlesource.com/c/tools/+/299131 Reviewed-by: Rebecca Stambler Trust: Rebecca Stambler Trust: Robert Findley Run-TryBot: Rebecca Stambler gopls-CI: kokoro TryBot-Result: Go Bot --- gopls/doc/analyzers.md | 40 +++++++++++++++++++++++++++++++++ internal/lsp/source/api_json.go | 10 +++++++++ internal/lsp/source/options.go | 2 ++ 3 files changed, 52 insertions(+) diff --git a/gopls/doc/analyzers.md b/gopls/doc/analyzers.md index 6a25f30199..e067d0cf58 100644 --- a/gopls/doc/analyzers.md +++ b/gopls/doc/analyzers.md @@ -208,6 +208,46 @@ A useless comparison is one like f == nil as opposed to f() == nil. **Enabled by default.** +## **nilness** + +check for redundant or impossible nil comparisons + +The nilness checker inspects the control-flow graph of each function in +a package and reports nil pointer dereferences, degenerate nil +pointers, and panics with nil values. A degenerate comparison is of the form +x==nil or x!=nil where x is statically known to be nil or non-nil. These are +often a mistake, especially in control flow related to errors. Panics with nil +values are checked because they are not detectable by + + if r := recover(); r != nil { + +This check reports conditions such as: + + if f == nil { // impossible condition (f is a function) + } + +and: + + p := &v + ... + if p != nil { // tautological condition + } + +and: + + if p == nil { + print(*p) // nil dereference + } + +and: + + if p == nil { + panic(p) + } + + +**Disabled by default. Enable it by setting `"analyses": {"nilness": true}`.** + ## **printf** check consistency of Printf format strings and arguments diff --git a/internal/lsp/source/api_json.go b/internal/lsp/source/api_json.go index aa7b518fc4..8f72405974 100755 --- a/internal/lsp/source/api_json.go +++ b/internal/lsp/source/api_json.go @@ -391,6 +391,11 @@ var GeneratedAPIJSON = &APIJSON{ Doc: "check for useless comparisons between functions and nil\n\nA useless comparison is one like f == nil as opposed to f() == nil.", Default: "true", }, + { + Name: "\"nilness\"", + Doc: "check for redundant or impossible nil comparisons\n\nThe nilness checker inspects the control-flow graph of each function in\na package and reports nil pointer dereferences, degenerate nil\npointers, and panics with nil values. A degenerate comparison is of the form\nx==nil or x!=nil where x is statically known to be nil or non-nil. These are\noften a mistake, especially in control flow related to errors. Panics with nil\nvalues are checked because they are not detectable by\n\n\tif r := recover(); r != nil {\n\nThis check reports conditions such as:\n\n\tif f == nil { // impossible condition (f is a function)\n\t}\n\nand:\n\n\tp := &v\n\t...\n\tif p != nil { // tautological condition\n\t}\n\nand:\n\n\tif p == nil {\n\t\tprint(*p) // nil dereference\n\t}\n\nand:\n\n\tif p == nil {\n\t\tpanic(p)\n\t}\n", + Default: "false", + }, { Name: "\"printf\"", Doc: "check consistency of Printf format strings and arguments\n\nThe check applies to known functions (for example, those in package fmt)\nas well as any detected wrappers of known functions.\n\nA function that wants to avail itself of printf checking but is not\nfound by this analyzer's heuristics (for example, due to use of\ndynamic calls) can insert a bogus call:\n\n\tif false {\n\t\t_ = fmt.Sprintf(format, args...) // enable printf checking\n\t}\n\nThe -funcs flag specifies a comma-separated list of names of additional\nknown formatting functions or methods. If the name contains a period,\nit must denote a specific function using one of the following forms:\n\n\tdir/pkg.Function\n\tdir/pkg.Type.Method\n\t(*dir/pkg.Type).Method\n\nOtherwise the name is interpreted as a case-insensitive unqualified\nidentifier such as \"errorf\". Either way, if a listed name ends in f, the\nfunction is assumed to be Printf-like, taking a format string before the\nargument list. Otherwise it is assumed to be Print-like, taking a list\nof arguments with no format string.\n", @@ -909,6 +914,11 @@ var GeneratedAPIJSON = &APIJSON{ Doc: "check for useless comparisons between functions and nil\n\nA useless comparison is one like f == nil as opposed to f() == nil.", Default: true, }, + { + Name: "nilness", + Doc: "check for redundant or impossible nil comparisons\n\nThe nilness checker inspects the control-flow graph of each function in\na package and reports nil pointer dereferences, degenerate nil\npointers, and panics with nil values. A degenerate comparison is of the form\nx==nil or x!=nil where x is statically known to be nil or non-nil. These are\noften a mistake, especially in control flow related to errors. Panics with nil\nvalues are checked because they are not detectable by\n\n\tif r := recover(); r != nil {\n\nThis check reports conditions such as:\n\n\tif f == nil { // impossible condition (f is a function)\n\t}\n\nand:\n\n\tp := &v\n\t...\n\tif p != nil { // tautological condition\n\t}\n\nand:\n\n\tif p == nil {\n\t\tprint(*p) // nil dereference\n\t}\n\nand:\n\n\tif p == nil {\n\t\tpanic(p)\n\t}\n", + Default: false, + }, { Name: "printf", Doc: "check consistency of Printf format strings and arguments\n\nThe check applies to known functions (for example, those in package fmt)\nas well as any detected wrappers of known functions.\n\nA function that wants to avail itself of printf checking but is not\nfound by this analyzer's heuristics (for example, due to use of\ndynamic calls) can insert a bogus call:\n\n\tif false {\n\t\t_ = fmt.Sprintf(format, args...) // enable printf checking\n\t}\n\nThe -funcs flag specifies a comma-separated list of names of additional\nknown formatting functions or methods. If the name contains a period,\nit must denote a specific function using one of the following forms:\n\n\tdir/pkg.Function\n\tdir/pkg.Type.Method\n\t(*dir/pkg.Type).Method\n\nOtherwise the name is interpreted as a case-insensitive unqualified\nidentifier such as \"errorf\". Either way, if a listed name ends in f, the\nfunction is assumed to be Printf-like, taking a format string before the\nargument list. Otherwise it is assumed to be Print-like, taking a list\nof arguments with no format string.\n", diff --git a/internal/lsp/source/options.go b/internal/lsp/source/options.go index 3f50eac23a..d36d12949f 100644 --- a/internal/lsp/source/options.go +++ b/internal/lsp/source/options.go @@ -31,6 +31,7 @@ import ( "golang.org/x/tools/go/analysis/passes/loopclosure" "golang.org/x/tools/go/analysis/passes/lostcancel" "golang.org/x/tools/go/analysis/passes/nilfunc" + "golang.org/x/tools/go/analysis/passes/nilness" "golang.org/x/tools/go/analysis/passes/printf" "golang.org/x/tools/go/analysis/passes/shadow" "golang.org/x/tools/go/analysis/passes/shift" @@ -1139,6 +1140,7 @@ func defaultAnalyzers() map[string]*Analyzer { atomicalign.Analyzer.Name: {Analyzer: atomicalign.Analyzer, Enabled: true}, deepequalerrors.Analyzer.Name: {Analyzer: deepequalerrors.Analyzer, Enabled: true}, fieldalignment.Analyzer.Name: {Analyzer: fieldalignment.Analyzer, Enabled: false}, + nilness.Analyzer.Name: {Analyzer: nilness.Analyzer, Enabled: false}, shadow.Analyzer.Name: {Analyzer: shadow.Analyzer, Enabled: false}, sortslice.Analyzer.Name: {Analyzer: sortslice.Analyzer, Enabled: true}, testinggoroutine.Analyzer.Name: {Analyzer: testinggoroutine.Analyzer, Enabled: true},