feat: allow excluding checkers with directive comment (#185)

Some checkers don't report issues in code. Rather, they form the basis for other checkers to run. For example, a checker may generate the scope graph for the entire source code (or a single file), and other checkers may use the results of such "meta" checkers. We need to exclude such "meta" checkers from being run like other normal checkers. Hence, introducing the directive `//globstar:registry-exclude`.

Author: MashyBasker

checkers/discover/discover.go

@@ -5,20 +5,37 @@ import (
 	"go/ast"
 	"go/parser"
 	"go/token"
+	"strings"
 )
 
 func DiscoverGoCheckers(dir string) ([]string, error) {
 	goCheckers := []string{}
 	fset := token.NewFileSet()
-	pkgs, err := parser.ParseDir(fset, dir, nil, parser.AllErrors&parser.SkipObjectResolution)
+	pkgs, err := parser.ParseDir(fset, dir, nil, parser.AllErrors&parser.SkipObjectResolution|parser.ParseComments)
 	if err != nil {
 		return goCheckers, err
 	}
 
 	for _, pkg := range pkgs {
 		for _, file := range pkg.Files {
+			isExcluded := false
+			if len(file.Comments) > 0 {
+				firstCommentGroup := file.Comments[0]
+				for _, comment := range firstCommentGroup.List {
+					if strings.TrimSpace(comment.Text) == "//globstar:registry-exclude" {
+						isExcluded = true
+						break
+					}
+				}
+			}
+
+			// skip this checker since exclude directive comment exists
+			if isExcluded {
+				continue
+			}
 			globstarPkgName := ""
 			for _, imp := range file.Imports {
+				// check for registry exclude comment in checker file
 				if imp.Path.Value == `"globstar.dev/analysis"` {
 					if imp.Name == nil {
 						globstarPkgName = "analysis"

checkers/discover/discover_test.go

@@ -50,6 +50,16 @@ func TestDiscoverGoCheckers(t *testing.T) {
 				err:        fmt.Errorf("open fixtures/invalid: no such file or directory"),
 			},
 		},
+		{
+			dir: filepath.Join(cwd, "fixtures", "exclude"),
+			want: struct {
+				goCheckers []string
+				err        error
+			}{
+				goCheckers: []string{},
+				err:        nil,
+			},
+		},
 	}
 
 	for _, tt := range tests {

checkers/discover/fixtures/checkers/no_assert.go

@@ -0,0 +1,26 @@
+//globstar:registry-exclude
+package checkers
+
+import (
+	sitter "github.com/smacker/go-tree-sitter"
+	"globstar.dev/analysis"
+)
+
+var NoAssert *analysis.Analyzer = &analysis.Analyzer{
+	Name:        "no-assert",
+	Language:    analysis.LangPy,
+	Description: "This checker checks for the usage of `assert` statement in Python code. It is risky as they are removed when Python is run optimized mode",
+	Category:    analysis.CategoryBugRisk,
+	Severity:    analysis.SeverityWarning,
+	Run:         checkNoAssert,
+}
+
+func checkNoAssert(pass *analysis.Pass) (interface{}, error) {
+	analysis.Preorder(pass, func(node *sitter.Node) {
+		if node.Type() == "assert_statement" {
+			pass.Report(pass, node, "Do not use assert statement to enforce constraints. They are removed in optimized bytecode")
+		}
+	})
+
+	return nil, nil
+}

checkers/discover/fixtures/exclude/no_assert.go

@@ -0,0 +1,26 @@
+//globstar:registry-exclude
+package exclude
+
+import (
+	sitter "github.com/smacker/go-tree-sitter"
+	"globstar.dev/analysis"
+)
+
+var NoAssert *analysis.Analyzer = &analysis.Analyzer{
+	Name:        "no-assert",
+	Language:    analysis.LangPy,
+	Description: "This checker checks for the usage of `assert` statement in Python code. It is risky as they are removed when Python is run optimized mode",
+	Category:    analysis.CategoryBugRisk,
+	Severity:    analysis.SeverityWarning,
+	Run:         checkNoAssert,
+}
+
+func checkNoAssert(pass *analysis.Pass) (interface{}, error) {
+	analysis.Preorder(pass, func(node *sitter.Node) {
+		if node.Type() == "assert_statement" {
+			pass.Report(pass, node, "Do not use assert statement to enforce constraints. They are removed in optimized bytecode")
+		}
+	})
+
+	return nil, nil
+}

checkers/discover/generate_test.go

@@ -64,11 +64,6 @@ import (
 	goAnalysis "globstar.dev/analysis"
 )
 
-type Analyzer struct {
-	TestDir   string
-	Analyzers []*goAnalysis.Analyzer
-}
-
 var AnalyzerRegistry = []Analyzer{
 	{
 		TestDir:   "checkers/javascript/testdata", // relative to the repository root
@@ -101,11 +96,6 @@ import (
 	goAnalysis "globstar.dev/analysis"
 )
 
-type Analyzer struct {
-	TestDir   string
-	Analyzers []*goAnalysis.Analyzer
-}
-
 var AnalyzerRegistry = []Analyzer{
 	{
 		TestDir:   "checkers/javascript/testdata", // relative to the repository root
@@ -141,11 +131,6 @@ import (
 	goAnalysis "globstar.dev/analysis"
 )
 
-type Analyzer struct {
-	TestDir   string
-	Analyzers []*goAnalysis.Analyzer
-}
-
 var AnalyzerRegistry = []Analyzer{
 	{
 		TestDir:   "checkers/javascript/testdata", // relative to the repository root
@@ -186,11 +171,6 @@ import (
 	goAnalysis "globstar.dev/analysis"
 )
 
-type Analyzer struct {
-	TestDir   string
-	Analyzers []*goAnalysis.Analyzer
-}
-
 var AnalyzerRegistry = []Analyzer{
 	{
 		TestDir:   "checkers/javascript/testdata", // relative to the repository root
@@ -201,7 +181,7 @@ var AnalyzerRegistry = []Analyzer{
 		},
 	},
 	{
-		TestDir: "checkers/python/testdata",
+		TestDir: "checkers/python/testdata", // relative to the repository root
 		Analyzers: []*goAnalysis.Analyzer{
 			python.DjangoSQLInjection,
 			python.DjangoCSVWriterInjection,

checkers/go/cgi_import.test.go

@@ -1,9 +1,9 @@
 package golang
 
 import (
-	// <expect-error> usage of cgi package
 	"fmt"
 	"net/http"
+	// <expect-error> usage of cgi package
 	"net/http/cgi"
 )
 

checkers/javascript/scope.go

@@ -1,3 +1,4 @@
+//globstar:registry-exclude
 // scope resolution implementation for JS and TS files
 package javascript