[gopls-release-branch.0.18] gopls/internal/analysis/modernize: sortslice: fix crash

The sole statement of a comparison func body is not necessarily
a return statement.

+ Test

Fixes golang/go#71786

Change-Id: Ic002035fc9fa303b62ed1828c13f3bdfb8bc6950
Reviewed-on: https://go-review.googlesource.com/c/tools/+/650215
Reviewed-by: Robert Findley <[email protected]>
Auto-Submit: Alan Donovan <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
(cherry picked from commit 776604a)
Reviewed-on: https://go-review.googlesource.com/c/tools/+/650495
Reviewed-by: Alan Donovan <[email protected]>
Auto-Submit: Robert Findley <[email protected]>

Author: adonovan

gopls/internal/analysis/modernize/sortslice.go

@@ -57,45 +57,46 @@ func sortslice(pass *analysis.Pass) {
 			i := sig.Params().At(0)
 			j := sig.Params().At(1)
 
-			ret := lit.Body.List[0].(*ast.ReturnStmt)
-			if compare, ok := ret.Results[0].(*ast.BinaryExpr); ok && compare.Op == token.LSS {
-				// isIndex reports whether e is s[v].
-				isIndex := func(e ast.Expr, v *types.Var) bool {
-					index, ok := e.(*ast.IndexExpr)
-					return ok &&
-						equalSyntax(index.X, s) &&
-						is[*ast.Ident](index.Index) &&
-						info.Uses[index.Index.(*ast.Ident)] == v
-				}
-				if isIndex(compare.X, i) && isIndex(compare.Y, j) {
-					// Have: sort.Slice(s, func(i, j int) bool { return s[i] < s[j] })
+			if ret, ok := lit.Body.List[0].(*ast.ReturnStmt); ok {
+				if compare, ok := ret.Results[0].(*ast.BinaryExpr); ok && compare.Op == token.LSS {
+					// isIndex reports whether e is s[v].
+					isIndex := func(e ast.Expr, v *types.Var) bool {
+						index, ok := e.(*ast.IndexExpr)
+						return ok &&
+							equalSyntax(index.X, s) &&
+							is[*ast.Ident](index.Index) &&
+							info.Uses[index.Index.(*ast.Ident)] == v
+					}
+					if isIndex(compare.X, i) && isIndex(compare.Y, j) {
+						// Have: sort.Slice(s, func(i, j int) bool { return s[i] < s[j] })
 
-					_, prefix, importEdits := analysisinternal.AddImport(
-						info, file, "slices", "slices", "Sort", call.Pos())
+						_, prefix, importEdits := analysisinternal.AddImport(
+							info, file, "slices", "slices", "Sort", call.Pos())
 
-					pass.Report(analysis.Diagnostic{
-						// Highlight "sort.Slice".
-						Pos:      call.Fun.Pos(),
-						End:      call.Fun.End(),
-						Category: "sortslice",
-						Message:  fmt.Sprintf("sort.Slice can be modernized using slices.Sort"),
-						SuggestedFixes: []analysis.SuggestedFix{{
-							Message: fmt.Sprintf("Replace sort.Slice call by slices.Sort"),
-							TextEdits: append(importEdits, []analysis.TextEdit{
-								{
-									// Replace sort.Slice with slices.Sort.
-									Pos:     call.Fun.Pos(),
-									End:     call.Fun.End(),
-									NewText: []byte(prefix + "Sort"),
-								},
-								{
-									// Eliminate FuncLit.
-									Pos: call.Args[0].End(),
-									End: call.Rparen,
-								},
-							}...),
-						}},
-					})
+						pass.Report(analysis.Diagnostic{
+							// Highlight "sort.Slice".
+							Pos:      call.Fun.Pos(),
+							End:      call.Fun.End(),
+							Category: "sortslice",
+							Message:  fmt.Sprintf("sort.Slice can be modernized using slices.Sort"),
+							SuggestedFixes: []analysis.SuggestedFix{{
+								Message: fmt.Sprintf("Replace sort.Slice call by slices.Sort"),
+								TextEdits: append(importEdits, []analysis.TextEdit{
+									{
+										// Replace sort.Slice with slices.Sort.
+										Pos:     call.Fun.Pos(),
+										End:     call.Fun.End(),
+										NewText: []byte(prefix + "Sort"),
+									},
+									{
+										// Eliminate FuncLit.
+										Pos: call.Args[0].End(),
+										End: call.Rparen,
+									},
+								}...),
+							}},
+						})
+					}
 				}
 			}
 		}

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

@@ -20,6 +20,16 @@ func _(s []int) {
 	sort.Slice(s, func(i, j int) bool { return s[j] < s[i] }) // nope: wrong index var
 }
 
-func _(s2 []struct{ x int }) {
+func _(sense bool, s2 []struct{ x int }) {
 	sort.Slice(s2, func(i, j int) bool { return s2[i].x < s2[j].x }) // nope: not a simple index operation
+
+	// Regression test for a crash: the sole statement of a
+	// comparison func body is not necessarily a return!
+	sort.Slice(s2, func(i, j int) bool {
+		if sense {
+			return s2[i].x < s2[j].x
+		} else {
+			return s2[i].x > s2[j].x
+		}
+	})
 }

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

@@ -22,6 +22,16 @@ func _(s []int) {
 	sort.Slice(s, func(i, j int) bool { return s[j] < s[i] }) // nope: wrong index var
 }
 
-func _(s2 []struct{ x int }) {
+func _(sense bool, s2 []struct{ x int }) {
 	sort.Slice(s2, func(i, j int) bool { return s2[i].x < s2[j].x }) // nope: not a simple index operation
+
+	// Regression test for a crash: the sole statement of a
+	// comparison func body is not necessarily a return!
+	sort.Slice(s2, func(i, j int) bool {
+		if sense {
+			return s2[i].x < s2[j].x
+		} else {
+			return s2[i].x > s2[j].x			
+		}
+	})
 }