gopls/internalanalysis: DeleteStmt deletes comments

DeleteStmt is called with just the AST so it has trouble deciding if
a comment is associated with the statement to be deleted. For instance,
for /*A*/ init()/*B*/;/*C/cond()/*D/;/*E*/post() /*F*/ { /*G*/}
comment B and C are indistinguishable, as are D and E. That is, as the
AST does not say where the semicolons are, B and C could go either
with the init() or the cond(), so cannot be removed safely. The same
is true for D, E, and the post(). (And there are other similar cases.)
But the other comments can be removed as they are unambiguously
associated with the statement being deleted.

The most common case where the new code differs is that it removes
whole lines like
  stmt // comment

Change-Id: I2d0652892241aac3a80f1b0a0cafbfeec2e6274f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/710775
Reviewed-by: Alan Donovan <[email protected]>
TryBot-Bypass: Peter Weinberger <[email protected]>

Author: pjweinbgo

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

@@ -15,18 +15,13 @@ type ST struct {
 
 func (s *ST) SetX(x int, ch chan int) {
 	// Accidental self-assignment; it should be "s.x = x"
-	// want "self-assignment of x"
 	// Another mistake
-	// want "self-assignment of s.x"
-
-	// want "self-assignment of s.l.0."
 
 	// Report self-assignment to x but preserve the actual assignment to s.x
 	s.x = 1 // want "self-assignment of x"
 	s.x = 1 // want "self-assignment of x"
 
 	// Delete multiple self-assignment
-	// want "self-assignment of x, s.x"
 	s.l[0] = 1            // want "self-assignment of x, s.x"
 	s.l[0] = 1            // want "self-assignment of x, s.x"
 	s.l[0] = 1            // want "self-assignment of x, s.x"
@@ -44,18 +39,14 @@ func num() int { return 2 }
 
 func Index() {
 	s := []int{1}
-	// want "self-assignment"
 
 	var a [5]int
-	// want "self-assignment"
 
 	pa := &[2]int{1, 2}
-	// want "self-assignment"
 
 	var pss *struct { // report self assignment despite nil dereference
 		s []int
 	}
-	// want "self-assignment"
 
 	m := map[int]string{1: "a"}
 	m[0] = m[0]     // bail on map self-assignments due to side effects

go/analysis/passes/assign/testdata/src/typeparams/typeparams.go.golden

@@ -15,11 +15,8 @@ type ST[T interface{ ~int }] struct {
 
 func (s *ST[T]) SetX(x T, ch chan T) {
 	// Accidental self-assignment; it should be "s.x = x"
-	// want "self-assignment of x"
 	// Another mistake
-	// want "self-assignment of s.x"
 
-	// want "self-assignment of s.l.0."
 
 	// Bail on any potential side effects to avoid false positives
 	s.l[num()] = s.l[num()]

go/analysis/passes/modernize/testdata/src/forvar/forvar.go.golden

@@ -3,37 +3,28 @@ package forvar
 func _(m map[int]int, s []int) {
 	// changed
 	for i := range s {
-		 // want "copying variable is unneeded"
 		go f(i)
 	}
 	for _, v := range s {
-		// want "copying variable is unneeded"
 		go f(v)
 	}
 	for k, v := range m {
-		// want "copying variable is unneeded"
-		// want "copying variable is unneeded"
 		go f(k)
 		go f(v)
 	}
 	for k, v := range m {
-		// want "copying variable is unneeded"
-		// want "copying variable is unneeded"
 		go f(k)
 		go f(v)
 	}
 	for k, v := range m {
-		// want "copying variable is unneeded"
 		go f(k)
 		go f(v)
 	}
 	for k, v := range m {
-		// want "copying variable is unneeded"
 		go f(k)
 		go f(v)
 	}
 	for i := range s {
-		/* hi */ // want "copying variable is unneeded"
 		go f(i)
 	}
 	// nope

go/analysis/passes/modernize/testdata/src/waitgroup/waitgroup.go.golden

@@ -8,17 +8,14 @@ import (
 // supported case for pattern 1.
 func _() {
 	var wg sync.WaitGroup
-	// want "Goroutine creation can be simplified using WaitGroup.Go"
 	wg.Go(func() {
 		fmt.Println()
 	})
 
-	// want "Goroutine creation can be simplified using WaitGroup.Go"
 	wg.Go(func() {
 	})
 
 	for range 10 {
-		// want "Goroutine creation can be simplified using WaitGroup.Go"
 		wg.Go(func() {
 			fmt.Println()
 		})
@@ -28,17 +25,14 @@ func _() {
 // supported case for pattern 2.
 func _() {
 	var wg sync.WaitGroup
-	// want "Goroutine creation can be simplified using WaitGroup.Go"
 	wg.Go(func() {
 		fmt.Println()
 	})
 
-	// want "Goroutine creation can be simplified using WaitGroup.Go"
 	wg.Go(func() {
 	})
 
 	for range 10 {
-		// want "Goroutine creation can be simplified using WaitGroup.Go"
 		wg.Go(func() {
 			fmt.Println()
 		})
@@ -48,19 +42,16 @@ func _() {
 // this function puts some wrong usages but waitgroup modernizer will still offer fixes.
 func _() {
 	var wg sync.WaitGroup
-	// want "Goroutine creation can be simplified using WaitGroup.Go"
 	wg.Go(func() {
 		defer wg.Done()
 		fmt.Println()
 	})
 
-	// want "Goroutine creation can be simplified using WaitGroup.Go"
 	wg.Go(func() {
 		fmt.Println()
 		wg.Done()
 	})
 
-	// want "Goroutine creation can be simplified using WaitGroup.Go"
 	wg.Go(func() {
 		fmt.Println()
 		wg.Done()
@@ -152,20 +143,17 @@ type ServerContainer struct {
 
 func _() {
 	var s Server
-	// want "Goroutine creation can be simplified using WaitGroup.Go"
 	s.wg.Go(func() {
 		print()
 	})
 
 	var sc ServerContainer
-	// want "Goroutine creation can be simplified using WaitGroup.Go"
 	sc.serv.wg.Go(func() {
 		print()
 	})
 
 	var wg sync.WaitGroup
 	arr := [1]*sync.WaitGroup{&wg}
-	// want "Goroutine creation can be simplified using WaitGroup.Go"
 	arr[0].Go(func() {
 		print()
 	})

go/analysis/passes/modernize/testdata/src/waitgroup/waitgroup_alias.go.golden

@@ -7,12 +7,10 @@ import (
 
 func _() {
 	var wg sync1.WaitGroup
-	// want "Goroutine creation can be simplified using WaitGroup.Go"
 	wg.Go(func() {
 		fmt.Println()
 	})
 
-	// want "Goroutine creation can be simplified using WaitGroup.Go"
 	wg.Go(func() {
 		fmt.Println()
 	})

go/analysis/passes/modernize/testdata/src/waitgroup/waitgroup_dot.go.golden

@@ -8,12 +8,10 @@ import (
 // supported case for pattern 1.
 func _() {
 	var wg WaitGroup
-	// want "Goroutine creation can be simplified using WaitGroup.Go"
 	wg.Go(func() {
 		fmt.Println()
 	})
 
-	// want "Goroutine creation can be simplified using WaitGroup.Go"
 	wg.Go(func() {
 		fmt.Println()
 	})