Implement //nolint parsing similar to golangci-lint (bazel-contrib#3562)

* Implement //nolint parsing similar to golangci-lint

Look for nolint comments and collect the ranges of where they apply. If
a node immediately follows a range with the same column, expand the
range to include the node.

Add tests that verify errors are filtered out.

* Use a map instead of custom Linters type

Inline the new report function and add some comments about nolint
ranges.

* Add separate tests for various filters

Include a failing test that shows the column issue. Will work on a fix.

* Use ast.CommentMap to associate comments to nodes

This is better than the logic used by golangci-lint in that it does not
incorrectly attribute an inline comment with the next line if it
contains an ast.Node with a matching column (see inline_column test).

* Address PR feedback

Use table driven tests and `CombinedOutput`.

---------

Co-authored-by: Patrick Scott <[email protected]>

go/tools/builders/BUILD.bazel

@@ -37,6 +37,15 @@ go_test(
     rundir = ".",
 )
 
+go_test(
+    name = "nolint_test",
+    size = "small",
+    srcs = [
+        "nolint.go",
+        "nolint_test.go",
+    ],
+)
+
 filegroup(
     name = "builder_srcs",
     srcs = [
@@ -82,6 +91,7 @@ go_source(
         "nogo_main.go",
         "nogo_typeparams_go117.go",
         "nogo_typeparams_go118.go",
+        "nolint.go",
         "pack.go",
     ],
     # //go/tools/builders:nogo_srcs is considered a different target by

go/tools/builders/nogo_main.go

@@ -202,6 +202,33 @@ func checkPackage(analyzers []*analysis.Analyzer, packagePath string, packageFil
 		act.pkg = pkg
 	}
 
+	// Process nolint directives similar to golangci-lint.
+	for _, f := range pkg.syntax {
+		// CommentMap will correctly associate comments to the largest node group
+		// applicable. This handles inline comments that might trail a large
+		// assignment and will apply the comment to the entire assignment.
+		commentMap := ast.NewCommentMap(pkg.fset, f, f.Comments)
+		for node, groups := range commentMap {
+			rng := &Range{
+				from: pkg.fset.Position(node.Pos()),
+				to:   pkg.fset.Position(node.End()).Line,
+			}
+			for _, group := range groups {
+				for _, comm := range group.List {
+					linters, ok := parseNolint(comm.Text)
+					if !ok {
+						continue
+					}
+					for analyzer, act := range actions {
+						if linters == nil || linters[analyzer.Name] {
+							act.nolint = append(act.nolint, rng)
+						}
+					}
+				}
+			}
+		}
+	}
+
 	// Execute the analyzers.
 	execAll(roots)
 
@@ -211,6 +238,11 @@ func checkPackage(analyzers []*analysis.Analyzer, packagePath string, packageFil
 	return diagnostics, facts, nil
 }
 
+type Range struct {
+	from token.Position
+	to   int
+}
+
 // An action represents one unit of analysis work: the application of
 // one analysis to one package. Actions form a DAG within a
 // package (as different analyzers are applied, either in sequence or
@@ -226,6 +258,7 @@ type action struct {
 	diagnostics []analysis.Diagnostic
 	usesFacts   bool
 	err         error
+	nolint      []*Range
 }
 
 func (act *action) String() string {
@@ -273,6 +306,23 @@ func (act *action) execOnce() {
 		inputs[dep.a] = dep.result
 	}
 
+	ignoreNolintReporter := func(d analysis.Diagnostic) {
+		pos := act.pkg.fset.Position(d.Pos)
+		for _, rng := range act.nolint {
+			// The list of nolint ranges is built for the entire package. Make sure we
+			// only apply ranges to the correct file.
+			if pos.Filename != rng.from.Filename {
+				continue
+			}
+			if pos.Line < rng.from.Line || pos.Line > rng.to {
+				continue
+			}
+			// Found a nolint range. Ignore the issue.
+			return
+		}
+		act.diagnostics = append(act.diagnostics, d)
+	}
+
 	// Run the analysis.
 	factFilter := make(map[reflect.Type]bool)
 	for _, f := range act.a.FactTypes {
@@ -285,7 +335,7 @@ func (act *action) execOnce() {
 		Pkg:               act.pkg.types,
 		TypesInfo:         act.pkg.typesInfo,
 		ResultOf:          inputs,
-		Report:            func(d analysis.Diagnostic) { act.diagnostics = append(act.diagnostics, d) },
+		Report:            ignoreNolintReporter,
 		ImportPackageFact: act.pkg.facts.ImportPackageFact,
 		ExportPackageFact: act.pkg.facts.ExportPackageFact,
 		ImportObjectFact:  act.pkg.facts.ImportObjectFact,

go/tools/builders/nolint.go

@@ -0,0 +1,39 @@
+// Copyright 2023 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package main
+
+import "strings"
+
+// Parse nolint directives and return the applicable linters. If all linters
+// apply, returns (nil, true).
+func parseNolint(text string) (map[string]bool, bool) {
+	text = strings.TrimLeft(text, "/ ")
+	if !strings.HasPrefix(text, "nolint") {
+		return nil, false
+	}
+	parts := strings.Split(text, ":")
+	if len(parts) == 1 {
+		return nil, true
+	}
+	linters := strings.Split(parts[1], ",")
+	result := map[string]bool{}
+	for _, linter := range linters {
+		if strings.EqualFold(linter, "all") {
+			return nil, true
+		}
+		result[linter] = true
+	}
+	return result, true
+}

go/tools/builders/nolint_test.go

@@ -0,0 +1,79 @@
+// Copyright 2023 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package main
+
+import (
+	"reflect"
+	"testing"
+)
+
+func TestParseNolint(t *testing.T) {
+	tests := []struct {
+		Name    string
+		Comment string
+		Valid   bool
+		Linters []string
+	}{
+		{
+			Name:    "Invalid",
+			Comment: "not a comment",
+		},
+		{
+			Name:    "No match",
+			Comment: "// comment",
+		},
+		{
+			Name:    "All linters",
+			Comment: "//nolint",
+			Valid:   true,
+		},
+		{
+			Name:    "All linters (explicit)",
+			Comment: "//nolint:all",
+			Valid:   true,
+		},
+		{
+			Name:    "Single linter",
+			Comment: "// nolint:foo",
+			Valid:   true,
+			Linters: []string{"foo"},
+		},
+		{
+			Name:    "Multiple linters",
+			Comment: "// nolint:a,b,c",
+			Valid:   true,
+			Linters: []string{"a", "b", "c"},
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.Name, func(t *testing.T) {
+			result, ok := parseNolint(tc.Comment)
+			if tc.Valid != ok {
+				t.Fatalf("parseNolint expect %t got %t", tc.Valid, ok)
+			}
+			var linters map[string]bool
+			if len(tc.Linters) != 0 {
+				linters = make(map[string]bool)
+				for _, l := range tc.Linters {
+					linters[l] = true
+				}
+			}
+			if !reflect.DeepEqual(result, linters) {
+				t.Fatalf("parseNolint expect %v got %v", linters, result)
+			}
+		})
+	}
+}

tests/core/nogo/nolint/BUILD.bazel

@@ -0,0 +1,6 @@
+load("@io_bazel_rules_go//go/tools/bazel_testing:def.bzl", "go_bazel_test")
+
+go_bazel_test(
+    name = "nolint_test",
+    srcs = ["nolint_test.go"],
+)

tests/core/nogo/nolint/README.rst

@@ -0,0 +1,14 @@
+Nolint check
+=========
+
+.. _go_library: /docs/go/core/rules.md#_go_library
+
+Tests to ensure that errors found by nogo and annotated with //nolint are
+ignored.
+
+.. contents::
+
+nolint_test
+--------
+Verified that errors emitted by ``nogo`` are ignored when `//nolint` appears as
+a comment.