Skip to content
Merged
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
7 changes: 7 additions & 0 deletions pkg/linters/fmterrorfnoverbs/fmterrorfnoverbs.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import (
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/go/analysis/passes/inspect"
"golang.org/x/tools/go/ast/inspector"

"github.com/github/gh-aw/pkg/linters/internal/nolint"
)

// Analyzer is the fmterrorfnoverbs analysis pass.
Expand All @@ -29,6 +31,7 @@ func run(pass *analysis.Pass) (any, error) {
if !ok {
return nil, fmt.Errorf("inspect analyzer result has unexpected type %T", pass.ResultOf[inspect.Analyzer])
}
noLintLinesByFile := nolint.BuildLineIndex(pass, "fmterrorfnoverbs")

nodeFilter := []ast.Node{
(*ast.CallExpr)(nil),
Expand Down Expand Up @@ -60,6 +63,10 @@ func run(pass *analysis.Pass) (any, error) {
}

if !strings.Contains(val, "%") {
position := pass.Fset.PositionFor(call.Pos(), false)
if nolint.HasDirective(position, noLintLinesByFile) {
return
}
pass.ReportRangef(call, "fmt.Errorf called with no format verbs; use errors.New(%s) instead", lit.Value)
}
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,12 @@ func goodWithVerb(name string) error {
func goodWithWrap(err error) error {
return fmt.Errorf("wrapper: %w", err)
}

func suppressedPreviousLine() error {
//nolint:fmterrorfnoverbs
return fmt.Errorf("this is intentionally static")
}

func suppressedSameLine() error {
return fmt.Errorf("this is intentionally static") //nolint:fmterrorfnoverbs
}
5 changes: 5 additions & 0 deletions pkg/linters/fprintlnsprintf/fprintlnsprintf.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"golang.org/x/tools/go/ast/inspector"

"github.com/github/gh-aw/pkg/linters/internal/filecheck"
"github.com/github/gh-aw/pkg/linters/internal/nolint"
)

// Analyzer is the fprintlnsprintf analysis pass.
Expand All @@ -27,6 +28,7 @@ func run(pass *analysis.Pass) (any, error) {
if !ok {
return nil, fmt.Errorf("inspect analyzer result has unexpected type %T", pass.ResultOf[inspect.Analyzer])
}
noLintLinesByFile := nolint.BuildLineIndex(pass, "fprintlnsprintf")

nodeFilter := []ast.Node{
(*ast.CallExpr)(nil),
Expand Down Expand Up @@ -60,6 +62,9 @@ func run(pass *analysis.Pass) (any, error) {
if !isFmtFunc(printedArg, "Sprintf") {
return
}
if nolint.HasDirective(pos, noLintLinesByFile) {
return
Comment on lines +65 to +66
}

pass.Reportf(call.Pos(), "use fmt.Fprintf instead of fmt.Fprintln(w, fmt.Sprintf(...))")
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,9 @@ func notFlagged(name string) {
fmt.Fprintln(os.Stderr)
fmt.Fprintf(os.Stderr, "hello %s\n", name)
}

func suppressed(name string) {
//nolint:fprintlnsprintf
fmt.Fprintln(os.Stderr, fmt.Sprintf("hello %s", name))
fmt.Fprintln(os.Stderr, fmt.Sprintf("hello %s", name)) //nolint:fprintlnsprintf
}
21 changes: 17 additions & 4 deletions pkg/linters/manualmutexunlock/manualmutexunlock.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"golang.org/x/tools/go/ast/inspector"

"github.com/github/gh-aw/pkg/linters/internal/filecheck"
"github.com/github/gh-aw/pkg/linters/internal/nolint"
)

// Analyzer is the manual-mutex-unlock analysis pass.
Expand All @@ -30,19 +31,20 @@ func run(pass *analysis.Pass) (any, error) {
if !ok {
return nil, fmt.Errorf("inspect analyzer result has unexpected type %T", pass.ResultOf[inspect.Analyzer])
}
noLintLinesByFile := nolint.BuildLineIndex(pass, "manualmutexunlock")

nodeFilter := []ast.Node{
(*ast.FuncDecl)(nil),
}

insp.Preorder(nodeFilter, func(n ast.Node) {
inspectMutexFuncDecl(pass, n)
inspectMutexFuncDecl(pass, noLintLinesByFile, n)
})

return nil, nil
}

func inspectMutexFuncDecl(pass *analysis.Pass, n ast.Node) {
func inspectMutexFuncDecl(pass *analysis.Pass, noLintLinesByFile map[string]map[int]struct{}, n ast.Node) {
fn, ok := n.(*ast.FuncDecl)
if !ok || fn.Body == nil {
return
Expand All @@ -58,12 +60,16 @@ func inspectMutexFuncDecl(pass *analysis.Pass, n ast.Node) {

// Walk all statements in the function body
ast.Inspect(fn.Body, func(node ast.Node) bool {
return inspectMutexNode(pass, mutexVars, node)
return inspectMutexNode(pass, noLintLinesByFile, mutexVars, node)
})

// Report mutexes with manual unlock but no defer
for _, state := range mutexVars {
if state.hasManualUnlock && !state.hasDefer {
position := pass.Fset.PositionFor(state.lockPos, false)
if nolint.HasDirective(position, noLintLinesByFile) {
continue
}
pass.Report(analysis.Diagnostic{
Pos: state.lockPos,
Message: "mutex Unlock() should be deferred immediately after Lock() to prevent deadlocks on panic or early return",
Expand All @@ -72,7 +78,7 @@ func inspectMutexFuncDecl(pass *analysis.Pass, n ast.Node) {
}
}

func inspectMutexNode(pass *analysis.Pass, mutexVars map[types.Object]*mutexVarState, node ast.Node) bool {
func inspectMutexNode(pass *analysis.Pass, noLintLinesByFile map[string]map[int]struct{}, mutexVars map[types.Object]*mutexVarState, node ast.Node) bool {
if node == nil {
return false
}
Expand All @@ -89,6 +95,13 @@ func inspectMutexNode(pass *analysis.Pass, mutexVars map[types.Object]*mutexVarS
// If this mutex was already tracked from a prior lock on the same
// binding, report any unresolved violation before overwriting state.
if prev, exists := mutexVars[obj]; exists && prev.hasManualUnlock && !prev.hasDefer {
position := pass.Fset.PositionFor(prev.lockPos, false)
if nolint.HasDirective(position, noLintLinesByFile) {
mutexVars[obj] = &mutexVarState{
lockPos: call.Pos(),
}
return true
}
pass.Report(analysis.Diagnostic{
Pos: prev.lockPos,
Message: "mutex Unlock() should be deferred immediately after Lock() to prevent deadlocks on panic or early return",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,17 @@ func GoodMutexPattern() {
var mu sync.Mutex
mu.Lock()
defer mu.Unlock()

// ... do work ...
}

// Wrong: manual unlock without defer - should be flagged
func BadMutexPattern() {
var mu sync.Mutex
mu.Lock() // want `mutex Unlock\(\) should be deferred immediately after Lock\(\) to prevent deadlocks on panic or early return`

// ... do work ...

mu.Unlock()
}

Expand All @@ -28,27 +28,27 @@ func GoodRWMutexPattern() {
var mu sync.RWMutex
mu.RLock()
defer mu.RUnlock()

// ... do work ...
}

// Wrong: RWMutex without defer - should be flagged
func BadRWMutexPattern() {
var mu sync.RWMutex
mu.RLock() // want `mutex Unlock\(\) should be deferred immediately after Lock\(\) to prevent deadlocks on panic or early return`

// ... do work ...

mu.RUnlock()
}

// Wrong: write lock without defer - should be flagged
func BadRWMutexWriteLock() {
var mu sync.RWMutex
mu.Lock() // want `mutex Unlock\(\) should be deferred immediately after Lock\(\) to prevent deadlocks on panic or early return`

// ... do work ...

mu.Unlock()
}

Expand All @@ -57,7 +57,7 @@ func GoodNestedPattern() {
var mu sync.Mutex
mu.Lock()
defer mu.Unlock()

func() {
// This is a closure, analyzed separately
}()
Expand All @@ -67,26 +67,26 @@ func GoodNestedPattern() {
func GoodMultipleMutexes() {
var mu1 sync.Mutex
var mu2 sync.Mutex

mu1.Lock()
defer mu1.Unlock()

mu2.Lock()
defer mu2.Unlock()

// ... do work ...
}

// Wrong: multiple locks, one without defer
func BadMultipleMutexes() {
var mu1 sync.Mutex
var mu2 sync.Mutex

mu1.Lock()
defer mu1.Unlock()

mu2.Lock() // want `mutex Unlock\(\) should be deferred immediately after Lock\(\) to prevent deadlocks on panic or early return`

// ... do work ...

mu2.Unlock()
Expand All @@ -112,3 +112,16 @@ func BadRepeatedLockBeforeGood() {
mu.Lock()
defer mu.Unlock()
}

func NolintPreviousLineSuppressed() {
var mu sync.Mutex
//nolint:manualmutexunlock
mu.Lock()
mu.Unlock()
}

func NolintSameLineSuppressed() {
var mu sync.Mutex
mu.Lock() //nolint:manualmutexunlock
mu.Unlock()
}
6 changes: 6 additions & 0 deletions pkg/linters/osexitinlibrary/osexitinlibrary.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"golang.org/x/tools/go/ast/inspector"

"github.com/github/gh-aw/pkg/linters/internal/filecheck"
"github.com/github/gh-aw/pkg/linters/internal/nolint"
)

// Analyzer is the os-exit-in-library analysis pass.
Expand All @@ -34,6 +35,7 @@ func run(pass *analysis.Pass) (any, error) {
if !ok {
return nil, fmt.Errorf("inspect analyzer result has unexpected type %T", pass.ResultOf[inspect.Analyzer])
}
noLintLinesByFile := nolint.BuildLineIndex(pass, "osexitinlibrary")

nodeFilter := []ast.Node{
(*ast.CallExpr)(nil),
Expand All @@ -56,6 +58,10 @@ func run(pass *analysis.Pass) (any, error) {
return
}
if ident.Name == "os" && sel.Sel.Name == "Exit" {
position := pass.Fset.PositionFor(call.Pos(), false)
if nolint.HasDirective(position, noLintLinesByFile) {
return
}
pass.ReportRangef(call, "os.Exit called in library package %s; move process termination to a cmd/ entry-point", pkgPath)
}
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,12 @@ func stopProcess() {
func doWork() error {
return nil
}

func suppressedPreviousLine() {
//nolint:osexitinlibrary
os.Exit(1)
}

func suppressedSameLine() {
os.Exit(1) //nolint:osexitinlibrary
}
6 changes: 6 additions & 0 deletions pkg/linters/panic-in-library-code/panic-in-library-code.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"golang.org/x/tools/go/ast/inspector"

"github.com/github/gh-aw/pkg/linters/internal/filecheck"
"github.com/github/gh-aw/pkg/linters/internal/nolint"
)

// Analyzer is the panic-in-library-code analysis pass.
Expand All @@ -38,6 +39,7 @@ func run(pass *analysis.Pass) (any, error) {
if !ok {
return nil, fmt.Errorf("inspect analyzer result has unexpected type %T", pass.ResultOf[inspect.Analyzer])
}
noLintLinesByFile := nolint.BuildLineIndex(pass, "panicinlibrarycode")

nodeFilter := []ast.Node{
(*ast.CallExpr)(nil),
Expand Down Expand Up @@ -77,6 +79,10 @@ func run(pass *analysis.Pass) (any, error) {
if shouldSkipPanic(pass, call, stack) {
return true
}
position := pass.Fset.PositionFor(call.Pos(), false)
if nolint.HasDirective(position, noLintLinesByFile) {
return true
}

pass.ReportRangef(call, "avoid panic in library code; return an error instead")
return true
Expand Down
Loading