gopls/internal/analysis/modernize: mapsloop: fix two bugs

As with slices.Contains, there was a bug in the maps.Copy
modernizer resulting from not checking for implicit widening
conversions in m[k]=v; and another, from not checking that m
is a map. This CL fixes both.

Modernizers are hard. :(

Fixes golang/go#71313

Change-Id: Ie59aa9419868b3010435b7113bb5e67f0abbb4d3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/645876
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Robert Findley <[email protected]>

Author: adonovan

gopls/internal/analysis/modernize/maps.go

@@ -186,9 +186,12 @@ func mapsloop(pass *analysis.Pass) {
 				assign := rng.Body.List[0].(*ast.AssignStmt)
 				if index, ok := assign.Lhs[0].(*ast.IndexExpr); ok &&
 					equalSyntax(rng.Key, index.Index) &&
-					equalSyntax(rng.Value, assign.Rhs[0]) {
+					equalSyntax(rng.Value, assign.Rhs[0]) &&
+					is[*types.Map](typeparams.CoreType(info.TypeOf(index.X))) &&
+					types.Identical(info.TypeOf(index), info.TypeOf(rng.Value)) { // m[k], v
 
 					// Have: for k, v := range x { m[k] = v }
+					// where there is no implicit conversion.
 					check(file, curRange, assign, index.X, rng.X)
 				}
 			}

gopls/internal/analysis/modernize/testdata/src/mapsloop/mapsloop.go

@@ -20,6 +20,13 @@ func useCopy(dst, src map[int]string) {
 	}
 }
 
+func useCopyGeneric[K comparable, V any, M ~map[K]V](dst, src M) {
+	// Replace loop by maps.Copy.
+	for key, value := range src {
+		dst[key] = value // want "Replace m\\[k\\]=v loop with maps.Copy"
+	}
+}
+
 func useClone(src map[int]string) {
 	// Replace make(...) by maps.Clone.
 	dst := make(map[int]string, len(src))
@@ -146,3 +153,24 @@ func nopeAssignmentHasIncrementOperator(src map[int]int) {
 		dst[k] += v
 	}
 }
+
+func nopeNotAMap(src map[int]string) {
+	var dst []string
+	for k, v := range src {
+		dst[k] = v
+	}
+}
+
+func nopeNotAMapGeneric[E any, M ~map[int]E, S ~[]E](src M) {
+	var dst S
+	for k, v := range src {
+		dst[k] = v
+	}
+}
+
+func nopeHasImplicitWidening(src map[string]int) {
+	dst := make(map[string]any)
+	for k, v := range src {
+		dst[k] = v
+	}
+}

gopls/internal/analysis/modernize/testdata/src/mapsloop/mapsloop.go.golden

@@ -18,6 +18,11 @@ func useCopy(dst, src map[int]string) {
 	maps.Copy(dst, src)
 }
 
+func useCopyGeneric[K comparable, V any, M ~map[K]V](dst, src M) {
+	// Replace loop by maps.Copy.
+	maps.Copy(dst, src)
+}
+
 func useClone(src map[int]string) {
 	// Replace make(...) by maps.Clone.
 	dst := maps.Clone(src)
@@ -118,3 +123,24 @@ func nopeAssignmentHasIncrementOperator(src map[int]int) {
 		dst[k] += v
 	}
 }
+
+func nopeNotAMap(src map[int]string) {
+	var dst []string
+	for k, v := range src {
+		dst[k] = v
+	}
+}
+
+func nopeNotAMapGeneric[E any, M ~map[int]E, S ~[]E](src M) {
+	var dst S
+	for k, v := range src {
+		dst[k] = v
+	}
+}
+
+func nopeHasImplicitWidening(src map[string]int) {
+	dst := make(map[string]any)
+	for k, v := range src {
+		dst[k] = v
+	}
+}