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).

go/tools/builders/nogo_main.go

@@ -204,61 +204,29 @@ func checkPackage(analyzers []*analysis.Analyzer, packagePath string, packageFil
 
 	// Process nolint directives similar to golangci-lint.
 	for _, f := range pkg.syntax {
-		// First, gather all comments and apply a range to the entire comment block.
-		// This will cover inline comments and allow us to detect comments above a
-		// node later.
-		for _, group := range f.Comments {
-			for _, comm := range group.List {
-				linters, ok := parseNolint(comm.Text)
-				if !ok {
-					continue
-				}
-				nl := &Range{
-					from: pkg.fset.Position(group.Pos()),
-					to:   pkg.fset.Position(group.End()).Line,
-				}
-				for analyzer, act := range actions {
-					if linters == nil || linters[analyzer.Name] {
-						act.nolint = append(act.nolint, nl)
-					}
-				}
-			}
-		}
-
-		// For each node in the ast, check if the previous line is covered by a
-		// range for the linter and expand it. The idea is to find instances of
-		//
-		//   //nolint
-		//   someExpression(that{
-		//     may: split,
-		//     multiple: lines,
-		//   })
-		//
-		// We should apply the range to the entire function call expression.
-		ast.Inspect(f, func(n ast.Node) bool {
-			if n == nil {
-				return true
+		// 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,
 			}
-
-			nodeStart := pkg.fset.Position(n.Pos())
-			nodeEnd := pkg.fset.Position(n.End())
-			for _, act := range actions {
-				for _, rng := range act.nolint {
-					if rng.from.Filename != nodeStart.Filename {
-						continue
-					}
-					if rng.to != nodeStart.Line-1 || rng.from.Column != nodeStart.Column {
+			for _, group := range groups {
+				for _, comm := range group.List {
+					linters, ok := parseNolint(comm.Text)
+					if !ok {
 						continue
 					}
-					// Expand the range to cover this node
-					if nodeEnd.Line > rng.to {
-						rng.to = nodeEnd.Line
+					for analyzer, act := range actions {
+						if linters == nil || linters[analyzer.Name] {
+							act.nolint = append(act.nolint, rng)
+						}
 					}
 				}
 			}
-
-			return true
-		})
+		}
 	}
 
 	// Execute the analyzers.

tests/core/nogo/nolint/nolint_test.go

@@ -39,55 +39,63 @@ nogo(
 go_library(
     name = "inline",
     srcs = ["inline.go"],
-    importpath = "inline",
+    importpath = "test",
 )
 
 go_library(
-    name = "inline-filter",
+    name = "inline_filter",
     srcs = ["inline_filter.go"],
-    importpath = "inlinefilter",
+    importpath = "test",
 )
 
 go_library(
     name = "block",
     srcs = ["block.go"],
-    importpath = "block",
+    importpath = "test",
 )
 
 go_library(
-    name = "block-multiline",
+    name = "block_multiline",
     srcs = ["block_multiline.go"],
-    importpath = "blockmultiline",
+    importpath = "test",
 )
 
 go_library(
-    name = "inline-errors",
+    name = "inline_errors",
     srcs = ["inline_errors.go"],
-    importpath = "inlineerrors",
+    importpath = "test",
 )
 
 go_library(
-    name = "inline-column",
+    name = "inline_column",
 		srcs = ["inline_column.go"],
-		importpath = "inlinecolumn",
+		importpath = "test",
+)
+
+go_library(
+    name = "large_block",
+		srcs = ["large_block.go"],
+		importpath = "test",
 )
 -- inline.go --
-package inline
+package test
+
+import "fmt"
 
 func F() {
 	s := "hello"
 	fmt.Printf("%d", s) //nolint
 }
 
 -- inline_filter.go --
-package inlinefilter
+package test
 
 func F() bool {
 	return true || true //nolint:bools
 }
 
 -- block.go --
-package block
+package test
 
 import "fmt"
 
@@ -97,7 +105,7 @@ func F() {
 }
 
 -- block_multiline.go --
-package blockmultiline
+package test
 
 func F() bool {
 	var i *int
@@ -107,7 +115,7 @@ func F() bool {
 }
 
 -- inline_errors.go --
-package inlineerrors
+package test
 
 import "fmt"
 
@@ -120,7 +128,7 @@ func F() {
 }
 
 -- inline_column.go --
-package inlinecolumn
+package test
 
 import "fmt"
 
@@ -130,6 +138,19 @@ func F() {
 	superLongVariableName := true || true
 	var _ = superLongVariableName
 }
+
+-- large_block.go --
+package test
+
+import "fmt"
+
+var V = struct {
+	S string
+	B bool
+} {
+	S: fmt.Sprintf("%d", "hello"), //nolint
+	B: true || true,
+}
 `,
 	})
 }
@@ -141,10 +162,13 @@ func runTest(t *testing.T, target string, expected string) {
 	stderr := &bytes.Buffer{}
 	cmd.Stderr = stderr
 	err := cmd.Run()
+	output := stderr.String()
 	if expected != "" && err == nil {
-		t.Fatal("unexpected success")
+		t.Fatal("unexpected success", output)
+	}
+	if expected == "" && err != nil {
+		t.Fatal("unexpected failure", output)
 	}
-	output := stderr.String()
 	if !strings.Contains(output, expected) {
 		t.Errorf("output did not contain expected: %s\n%s", expected, output)
 	}
@@ -158,13 +182,14 @@ func Test(t *testing.T) {
 
 	// Execute tests that should filter out errors
 	runTest(t, "//:inline", "")
-	runTest(t, "//:inline-filter", "")
+	runTest(t, "//:inline_filter", "")
 	runTest(t, "//:block", "")
-	runTest(t, "//:block-multiline", "")
+	runTest(t, "//:block_multiline", "")
 
 	// Execute tests that should return unfiltered errors
-	runTest(t, "//:inline-errors", "inline_errors.go:9:15: nil dereference in load (nilness)")
-	runTest(t, "//:inline-column", "inline_column.go:8:27: redundant or: true || true (bools)")
+	runTest(t, "//:inline_errors", "inline_errors.go:9:15: nil dereference in load (nilness)")
+	runTest(t, "//:inline_column", "inline_column.go:8:27: redundant or: true || true (bools)")
+	runTest(t, "//:large_block", "large_block.go:10:5: redundant or: true || true (bools)")
 }
 
 func replaceInFile(path, old, new string) error {