go/analysis/passes: add sigchanyzer Analyzer

This change brings in a static analyzer that was created by
Orijtech, Inc. to detect the use of unbuffered channels with
signal.Notify, which is a bug that's documented but not enforced.
We've found it even in official examples and in the main Go
repository as per https://go-review.googlesource.com/c/go/+/274332.

We've found that this bug is very common in lots of Go code,
hence the reason why we are directly donating it, so that all
Go developers running go test, using gopls can use it.

Updates golang/go#9399

Change-Id: Ief6d7238dc80bc9fd5f11a585e41387704457276
Reviewed-on: https://go-review.googlesource.com/c/tools/+/274352
gopls-CI: kokoro <[email protected]>
TryBot-Result: Go Bot <[email protected]>
Trust: Rebecca Stambler <[email protected]>
Trust: Cuong Manh Le <[email protected]>
Trust: Emmanuel Odeke <[email protected]>
Run-TryBot: Rebecca Stambler <[email protected]>
Reviewed-by: Emmanuel Odeke <[email protected]>

Author: Cuong Manh Le

go/analysis/passes/sigchanyzer/sigchanyzer.go

@@ -0,0 +1,129 @@
+// Copyright 2020 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Package sigchanyzer defines an Analyzer that detects
+// misuse of unbuffered signal as argument to signal.Notify.
+package sigchanyzer
+
+import (
+	"bytes"
+	"go/ast"
+	"go/format"
+	"go/token"
+	"go/types"
+
+	"golang.org/x/tools/go/analysis"
+	"golang.org/x/tools/go/analysis/passes/inspect"
+	"golang.org/x/tools/go/ast/inspector"
+)
+
+const Doc = `check for unbuffered channel of os.Signal
+
+This checker reports call expression of the form signal.Notify(c <-chan os.Signal, sig ...os.Signal),
+where c is an unbuffered channel, which can be at risk of missing the signal.`
+
+// Analyzer describes sigchanyzer analysis function detector.
+var Analyzer = &analysis.Analyzer{
+	Name:     "sigchanyzer",
+	Doc:      Doc,
+	Requires: []*analysis.Analyzer{inspect.Analyzer},
+	Run:      run,
+}
+
+func run(pass *analysis.Pass) (interface{}, error) {
+	inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
+
+	nodeFilter := []ast.Node{
+		(*ast.CallExpr)(nil),
+	}
+	inspect.Preorder(nodeFilter, func(n ast.Node) {
+		call := n.(*ast.CallExpr)
+		if !isSignalNotify(pass.TypesInfo, call) {
+			return
+		}
+		var chanDecl *ast.CallExpr
+		switch arg := call.Args[0].(type) {
+		case *ast.Ident:
+			if decl, ok := findDecl(arg).(*ast.CallExpr); ok {
+				chanDecl = decl
+			}
+		case *ast.CallExpr:
+			chanDecl = arg
+		}
+		if chanDecl == nil || len(chanDecl.Args) != 1 {
+			return
+		}
+		chanDecl.Args = append(chanDecl.Args, &ast.BasicLit{
+			Kind:  token.INT,
+			Value: "1",
+		})
+		var buf bytes.Buffer
+		if err := format.Node(&buf, token.NewFileSet(), chanDecl); err != nil {
+			return
+		}
+		pass.Report(analysis.Diagnostic{
+			Pos:     call.Pos(),
+			End:     call.End(),
+			Message: "misuse of unbuffered os.Signal channel as argument to signal.Notify",
+			SuggestedFixes: []analysis.SuggestedFix{{
+				Message: "Change to buffer channel",
+				TextEdits: []analysis.TextEdit{{
+					Pos:     chanDecl.Pos(),
+					End:     chanDecl.End(),
+					NewText: buf.Bytes(),
+				}},
+			}},
+		})
+	})
+	return nil, nil
+}
+
+func isSignalNotify(info *types.Info, call *ast.CallExpr) bool {
+	check := func(id *ast.Ident) bool {
+		obj := info.ObjectOf(id)
+		return obj.Name() == "Notify" && obj.Pkg().Path() == "os/signal"
+	}
+	switch fun := call.Fun.(type) {
+	case *ast.SelectorExpr:
+		return check(fun.Sel)
+	case *ast.Ident:
+		if fun, ok := findDecl(fun).(*ast.SelectorExpr); ok {
+			return check(fun.Sel)
+		}
+		return false
+	default:
+		return false
+	}
+}
+
+func findDecl(arg *ast.Ident) ast.Node {
+	if arg.Obj == nil {
+		return nil
+	}
+	switch as := arg.Obj.Decl.(type) {
+	case *ast.AssignStmt:
+		if len(as.Lhs) != len(as.Rhs) {
+			return nil
+		}
+		for i, lhs := range as.Lhs {
+			lid, ok := lhs.(*ast.Ident)
+			if !ok {
+				continue
+			}
+			if lid.Obj == arg.Obj {
+				return as.Rhs[i]
+			}
+		}
+	case *ast.ValueSpec:
+		if len(as.Names) != len(as.Values) {
+			return nil
+		}
+		for i, name := range as.Names {
+			if name.Obj == arg.Obj {
+				return as.Values[i]
+			}
+		}
+	}
+	return nil
+}

go/analysis/passes/sigchanyzer/sigchanyzer_test.go

@@ -0,0 +1,17 @@
+// Copyright 2020 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package sigchanyzer_test
+
+import (
+	"testing"
+
+	"golang.org/x/tools/go/analysis/analysistest"
+	"golang.org/x/tools/go/analysis/passes/sigchanyzer"
+)
+
+func Test(t *testing.T) {
+	testdata := analysistest.TestData()
+	analysistest.RunWithSuggestedFixes(t, testdata, sigchanyzer.Analyzer, "a")
+}

go/analysis/passes/sigchanyzer/testdata/src/a/a.go

@@ -0,0 +1,38 @@
+package p
+
+import (
+	"os"
+	ao "os"
+	"os/signal"
+)
+
+var c = make(chan os.Signal)
+var d = make(chan os.Signal)
+
+func f() {
+	c := make(chan os.Signal, 1)
+	signal.Notify(c, os.Interrupt) // ok
+	_ = <-c
+}
+
+func g() {
+	c := make(chan os.Signal)
+	signal.Notify(c, os.Interrupt) // want "misuse of unbuffered os.Signal channel as argument to signal.Notify"
+	_ = <-c
+}
+
+func h() {
+	c := make(chan ao.Signal)
+	signal.Notify(c, os.Interrupt) // want "misuse of unbuffered os.Signal channel as argument to signal.Notify"
+	_ = <-c
+}
+
+func i() {
+	signal.Notify(d, os.Interrupt) // want "misuse of unbuffered os.Signal channel as argument to signal.Notify"
+}
+
+func j() {
+	c := make(chan os.Signal)
+	f := signal.Notify
+	f(c, os.Interrupt) // want "misuse of unbuffered os.Signal channel as argument to signal.Notify"
+}

go/analysis/passes/sigchanyzer/testdata/src/a/a.go.golden

@@ -0,0 +1,38 @@
+package p
+
+import (
+	"os"
+	ao "os"
+	"os/signal"
+)
+
+var c = make(chan os.Signal)
+var d = make(chan os.Signal, 1)
+
+func f() {
+	c := make(chan os.Signal, 1)
+	signal.Notify(c, os.Interrupt) // ok
+	_ = <-c
+}
+
+func g() {
+	c := make(chan os.Signal, 1)
+	signal.Notify(c, os.Interrupt) // want "misuse of unbuffered os.Signal channel as argument to signal.Notify"
+	_ = <-c
+}
+
+func h() {
+	c := make(chan ao.Signal, 1)
+	signal.Notify(c, os.Interrupt) // want "misuse of unbuffered os.Signal channel as argument to signal.Notify"
+	_ = <-c
+}
+
+func i() {
+	signal.Notify(d, os.Interrupt) // want "misuse of unbuffered os.Signal channel as argument to signal.Notify"
+}
+
+func j() {
+	c := make(chan os.Signal, 1)
+	f := signal.Notify
+	f(c, os.Interrupt) // want "misuse of unbuffered os.Signal channel as argument to signal.Notify"
+}