Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

S1008: don't flag if both branches are commented #1556

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions simple/s1008/s1008.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,14 @@ var (
checkIfReturnQRet = pattern.MustParse(`(ReturnStmt [ret@(Builtin (Or "true" "false"))])`)
)

func run(pass *analysis.Pass) (interface{}, error) {
func run(pass *analysis.Pass) (any, error) {
var cm ast.CommentMap
fn := func(node ast.Node) {
if f, ok := node.(*ast.File); ok {
cm = ast.NewCommentMap(pass.Fset, f, f.Comments)
return
}

block := node.(*ast.BlockStmt)
l := len(block.List)
if l < 2 {
Expand Down Expand Up @@ -76,13 +82,17 @@ func run(pass *analysis.Pass) (interface{}, error) {

ret1 := m1.State["ret"].(*ast.Ident)
ret2 := m2.State["ret"].(*ast.Ident)

if ret1.Name == ret2.Name {
// we want the function to return true and false, not the
// same value both times.
return
}

// Don't flag if both are commented.
if len(cm.Filter(n1).Comments()) > 0 && len(cm.Filter(n2).Comments()) > 0 {
return
}

cond := m1.State["cond"].(ast.Expr)
origCond := cond
if ret1.Name == "false" {
Expand All @@ -94,7 +104,7 @@ func run(pass *analysis.Pass) (interface{}, error) {
report.Render(pass, origCond), report.Render(pass, ret1), report.Render(pass, ret2)),
report.FilterGenerated())
}
code.Preorder(pass, fn, (*ast.BlockStmt)(nil))
code.Preorder(pass, fn, (*ast.File)(nil), (*ast.BlockStmt)(nil))
return nil, nil
}

Expand Down
47 changes: 47 additions & 0 deletions simple/s1008/testdata/go1.0/CheckIfReturn/comment.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package pkg

func cmt1(x string) bool {
// A
if len(x) > 0 {
return false
}
// B
return true
}

func cmt2(x string) bool {
if len(x) > 0 { // A
return false
}
return true // B
}

func cmt3(x string) bool {
if len(x) > 0 {
return false // A
}
return true // B
}

func cmt4(x string) bool {
if len(x) > 0 {
return false // A
}
return true
// B
}

// Hard to test, as the diag line adds a comment.
//func cmt5(x string) bool {
// if len(x) > 0 { // diag(`should use 'return len(x) == 0'`)
// return false
// }
// return true // A
//}

func cmt6(x string) bool {
if len(x) > 0 { //@diag(`should use 'return len(x) == 0'`)
return false // A
}
return true
}
Loading