go/packages/packagestest: dedupe based on note position

The "forTest" field is used for x_tests and for test variants, making
the deduplication too agressive. Instead, we deduplicate by the
position of the note.

Change-Id: I809364d6bba95be5ad95eced67fae645435f5592
Reviewed-on: https://go-review.googlesource.com/c/tools/+/229597
Reviewed-by: Paul Jolly <paul@myitcv.org.uk>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

go/packages/packagestest/expect.go

@@ -16,7 +16,6 @@ import (
 
 	"golang.org/x/tools/go/expect"
 	"golang.org/x/tools/go/packages"
-	"golang.org/x/tools/internal/packagesinternal"
 	"golang.org/x/tools/internal/span"
 )
 
@@ -152,18 +151,8 @@ func (e *Exported) getNotes() error {
 	if err != nil {
 		return fmt.Errorf("unable to load packages for directories %s: %v", dirs, err)
 	}
-	// We need to avoid walking packages and their test variants, otherwise we
-	// double-count
-	pkgsWithTestVariants := make(map[string]bool)
+	seen := make(map[token.Position]struct{})
 	for _, pkg := range pkgs {
-		if forTest := packagesinternal.GetForTest(pkg); forTest != "" {
-			pkgsWithTestVariants[forTest] = true
-		}
-	}
-	for _, pkg := range pkgs {
-		if pkgsWithTestVariants[pkg.ID] {
-			continue
-		}
 		for _, filename := range pkg.GoFiles {
 			content, err := e.FileContents(filename)
 			if err != nil {
@@ -173,7 +162,14 @@ func (e *Exported) getNotes() error {
 			if err != nil {
 				return fmt.Errorf("failed to extract expectations: %v", err)
 			}
-			notes = append(notes, l...)
+			for _, note := range l {
+				pos := e.ExpectFileSet.Position(note.Pos)
+				if _, ok := seen[pos]; ok {
+					continue
+				}
+				notes = append(notes, note)
+				seen[pos] = struct{}{}
+			}
 		}
 	}
 	if _, ok := e.written[e.primary]; !ok {

go/packages/packagestest/expect_test.go

@@ -65,7 +65,7 @@ func TestExpect(t *testing.T) {
 	// including _test.go files (XTest or otherwise). But to have walked the
 	// non-_test.go files only once. Hence wantCheck = 3 (testdata/test.go) + 1
 	// (testdata/test_test.go) + 1 (testdata/x_test.go)
-	wantCheck := 5
+	wantCheck := 7
 	if wantCheck != checkCount {
 		t.Fatalf("Expected @check count of %v; got %v", wantCheck, checkCount)
 	}

go/packages/packagestest/export_test.go

@@ -113,8 +113,9 @@ func TestGroupFilesByModules(t *testing.T) {
 				{
 					Name: "testdata/groups/two",
 					Files: map[string]interface{}{
-						"main.go":      true,
-						"expect/yo.go": true,
+						"main.go":           true,
+						"expect/yo.go":      true,
+						"expect/yo_test.go": true,
 					},
 				},
 				{

go/packages/packagestest/testdata/groups/two/primarymod/expect/yo.go

@@ -1 +1,3 @@
 package expect
+
+var X int //@check("X", "X")

go/packages/packagestest/testdata/groups/two/primarymod/expect/yo_test.go

@@ -0,0 +1,10 @@
+package expect_test
+
+import (
+	"testdata/groups/two/expect"
+	"testing"
+)
+
+func TestX(t *testing.T) {
+	_ = expect.X //@check("X", "X")
+}