From e234cf6dd57208a861eb4b711cdb456fdef47251 Mon Sep 17 00:00:00 2001 From: Joshua Date: Sat, 30 Nov 2024 00:13:56 -0500 Subject: [PATCH] Fix: ignore errs returned outside block (#25) --- .gitignore | 2 ++ .golangci.yml | 7 ------- spancheck.go | 20 ++++++++++++++++++ testdata/enableall/enable_all.go | 36 ++++++++++++++++++++++++++++++++ 4 files changed, 58 insertions(+), 7 deletions(-) diff --git a/.gitignore b/.gitignore index 1f83be4..04b66d9 100644 --- a/.gitignore +++ b/.gitignore @@ -17,3 +17,5 @@ # Dependency directories (remove the comment below to include it) # vendor/ src/ + +.vscode \ No newline at end of file diff --git a/.golangci.yml b/.golangci.yml index 15d8513..5d6ab12 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -17,7 +17,6 @@ linters: - errcheck - errname - errorlint - - exhaustive # checks exhaustiveness of enum switch statements - exportloopref # checks for pointers to enclosing loop variables - gci - gochecknoinits # checks that no init functions are present in Go code @@ -59,12 +58,6 @@ linters-settings: - standard # Standard section: captures all standard packages. - default # Default section: contains all imports that could not be matched to another section type. - prefix(github.com/jjti) - exhaustive: - # Program elements to check for exhaustiveness. - # Default: [ switch ] - check: - - switch - - map gocritic: settings: captLocal: diff --git a/spancheck.go b/spancheck.go index 8fc7945..6b8b9fd 100644 --- a/spancheck.go +++ b/spancheck.go @@ -309,6 +309,11 @@ outer: } seen[b] = true + // Skip successors that are not nested within this current block. + if _, ok := nestedBlockTypes[b.Kind]; !ok { + continue + } + // Prune the search if the block uses v. if blockUses(pass, b) { continue @@ -330,6 +335,21 @@ outer: return search(defBlock.Succs) } +var nestedBlockTypes = map[cfg.BlockKind]struct{}{ + cfg.KindBody: {}, + cfg.KindForBody: {}, + cfg.KindForLoop: {}, + cfg.KindIfElse: {}, + cfg.KindIfThen: {}, + cfg.KindLabel: {}, + cfg.KindRangeBody: {}, + cfg.KindRangeLoop: {}, + cfg.KindSelectCaseBody: {}, + cfg.KindSelectAfterCase: {}, + cfg.KindSwitchCaseBody: {}, + cfg.KindSwitchNextCase: {}, +} + // usesCall reports whether stmts contain a use of the selName call on variable v. func usesCall( pass *analysis.Pass, diff --git a/testdata/enableall/enable_all.go b/testdata/enableall/enable_all.go index f60d68c..5ce6480 100644 --- a/testdata/enableall/enable_all.go +++ b/testdata/enableall/enable_all.go @@ -213,3 +213,39 @@ func testStartTrace() *trace.Span { // no error expected because this is in extr _, span := trace.StartSpan(context.Background(), "bar") return span } + +// https://github.com/jjti/go-spancheck/issues/25 +func _() error { + if true { + _, span := otel.Tracer("foo").Start(context.Background(), "bar") + span.End() + } + + return errors.New("test") +} + +func _() error { + if true { + _, span := otel.Tracer("foo").Start(context.Background(), "bar") // want "span.SetStatus is not called on all paths" + defer span.End() + + if true { + span.RecordError(errors.New("test")) + return errors.New("test") // want "return can be reached without calling span.SetStatus" + } + } + + return errors.New("test") +} + +// TODO: the check below now fails after the change to fix issue 25 above. +// func _() error { +// var span *trace.Span + +// if true { +// _, span = trace.StartSpan(context.Background(), "foo") +// defer span.End() +// } + +// return errors.New("test") +// }