Skip to content

Commit

Permalink
Remove hardcoded file ignore string check (#11)
Browse files Browse the repository at this point in the history
This PR removes the hardcoded `nolint:nilaway` docstring check for both
files and function declarations since it is no longer necessary:

* files: we have implemented configuration analyzer in #9 which is able
to take a list of strings to ignore the analysis of a file.

* function declaration: it is mainly designed for two reasons:
* performance: we were able to use it to manually skip the analysis of a
particular function for performance reasons. However, we now have a
timeout for analysis of any particular function, hence this is no longer
required.
* error suppression: most linter drivers, especially nogo, are now
respecting `nolint`, so there is really no need to support it within the
linter itself.

Depends on #10
  • Loading branch information
yuxincs authored Aug 4, 2023
1 parent 2677b5b commit 1b2bb3f
Show file tree
Hide file tree
Showing 6 changed files with 6 additions and 18 deletions.
3 changes: 1 addition & 2 deletions assertion/anonymousfunc/analyzer.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,7 @@ func run(pass *analysis.Pass) (result interface{}, _ error) {
funcLitMap := make(map[*ast.FuncLit]*FuncLitInfo)

for _, file := range pass.Files {
if util.DocContainsIgnore(file.Doc) || !conf.IsFileInScope(file) ||
!util.DocContainsAnonymousFuncCheck(file.Doc) {
if !conf.IsFileInScope(file) || !util.DocContainsAnonymousFuncCheck(file.Doc) {
continue
}

Expand Down
6 changes: 3 additions & 3 deletions assertion/function/analyzer.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ func run(pass *analysis.Pass) (result interface{}, _ error) {
var funcIndex int
for _, file := range pass.Files {
// Skip if a file is marked to be ignored, or it is not in scope of our analysis.
if util.DocContainsIgnore(file.Doc) || !conf.IsFileInScope(file) {
if !conf.IsFileInScope(file) {
continue
}

Expand Down Expand Up @@ -189,8 +189,8 @@ func run(pass *analysis.Pass) (result interface{}, _ error) {
panic(fmt.Sprintf("unrecognized function type %T", f))
}

// Skip if function declaration has an empty body, or it is marked to be ignored.
if funcDecl.Body == nil || util.DocContainsIgnore(funcDecl.Doc) {
// Skip if function declaration has an empty body.
if funcDecl.Body == nil {
continue
}
// Skip if the function is too large.
Expand Down
3 changes: 1 addition & 2 deletions assertion/global/analyzer.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (

"go.uber.org/nilaway/annotation"
"go.uber.org/nilaway/config"
"go.uber.org/nilaway/util"
"golang.org/x/tools/go/analysis"
)

Expand Down Expand Up @@ -73,7 +72,7 @@ func run(pass *analysis.Pass) (result interface{}, _ error) {

var fullTriggers []annotation.FullTrigger
for _, file := range pass.Files {
if util.DocContainsIgnore(file.Doc) || !conf.IsFileInScope(file) {
if !conf.IsFileInScope(file) {
continue
}

Expand Down
3 changes: 1 addition & 2 deletions assertion/structfield/analyzer.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"runtime/debug"

"go.uber.org/nilaway/config"
"go.uber.org/nilaway/util"
"golang.org/x/tools/go/analysis"
)

Expand Down Expand Up @@ -73,7 +72,7 @@ func run(pass *analysis.Pass) (result interface{}, _ error) {
}

for _, file := range pass.Files {
if util.DocContainsIgnore(file.Doc) || !conf.IsFileInScope(file) {
if !conf.IsFileInScope(file) {
continue
}

Expand Down
4 changes: 0 additions & 4 deletions config/const.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,6 @@ const BackpropTimeout = 10 * time.Second
// considered undesirable behavior and worth catching in the future.
const ErrorOnNilableMapRead = false

// NilAwayIgnoreString is the string that may be inserted into the docstring for a file (resp. function)
// to prevent NilAway from generating assertions from that file (resp. function)
const NilAwayIgnoreString = "nolint:nilaway"

// NilAwayNoInferString is the string that may be inserted into the docstring for a package to prevent
// NilAway from inferring the annotations for that package - this is useful for unit tests
const NilAwayNoInferString = "<nilaway no inference>"
Expand Down
5 changes: 0 additions & 5 deletions util/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -403,11 +403,6 @@ func GetSelectorExprHeadIdent(selExpr *ast.SelectorExpr) *ast.Ident {
return nil
}

// DocContainsIgnore is used by analyzers to check if the file should be ignored by the analyzer.
func DocContainsIgnore(group *ast.CommentGroup) bool {
return docContainsString(config.NilAwayIgnoreString)(group)
}

// DocContainsStructInitCheck is used by analyzers to check if the struct initialization check enabling string is present
// in the comments.
func DocContainsStructInitCheck(group *ast.CommentGroup) config.StructInitCheckType {
Expand Down

0 comments on commit 1b2bb3f

Please sign in to comment.