gopls/internal/lsp/source: add invert-if-condition refactoring

![invert-if-condition](https://user-images.githubusercontent.com/158201/229582714-ce187c0a-db0e-4f7f-8889-e7ceb9b7e07f.gif)

This PR adds the "Invert `if` Condition" refactoring, as illustrated in the attached Gif ^.

It's ideal for code like this:
```go
func something(i int) int {
  if i > 14 {
    // Imagine lots of code here, deeply indented, computing this value...
    result := 99
    return result
  } else {
    return 0
  }
}
```

Which this refactoring will simplify into this, which I find easier to read:
```go
func something(i int) int {
  if i <= 14 {
    return 0
  }

  // Imagine lots of code here, deeply indented, computing this value...
  result := 99
  return result
}
```

**Describe alternatives you've considered**
I keep doing this by hand...

**Additional context**
When working with Java in IntelliJ I used this refactoring a lot and I miss it in Go / VSCode, so I'm certain I will use this in VSCode as well.

Fixes #2557

Change-Id: I06a7fb9fd7ecc2383b751e3a20b25a6a912adbfa
GitHub-Last-Rev: 18e1e3f
GitHub-Pull-Request: #433
Reviewed-on: https://go-review.googlesource.com/c/tools/+/481695
Reviewed-by: Alan Donovan <[email protected]>
Run-TryBot: Robert Findley <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>

Author: walles

gopls/internal/lsp/code_action.go

@@ -255,6 +255,14 @@ func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionPara
 			codeActions = append(codeActions, fixes...)
 		}
 
+		if wanted[protocol.RefactorRewrite] {
+			fixes, err := refactoringFixes(ctx, snapshot, uri, params.Range)
+			if err != nil {
+				return nil, err
+			}
+			codeActions = append(codeActions, fixes...)
+		}
+
 	default:
 		// Unsupported file kind for a code action.
 		return nil, nil
@@ -395,6 +403,46 @@ func extractionFixes(ctx context.Context, snapshot source.Snapshot, uri span.URI
 	return actions, nil
 }
 
+func refactoringFixes(ctx context.Context, snapshot source.Snapshot, uri span.URI, rng protocol.Range) ([]protocol.CodeAction, error) {
+	fh, err := snapshot.ReadFile(ctx, uri)
+	if err != nil {
+		return nil, err
+	}
+
+	pgf, err := snapshot.ParseGo(ctx, fh, source.ParseFull)
+	if err != nil {
+		return nil, err
+	}
+
+	start, end, err := pgf.RangePos(rng)
+	if err != nil {
+		return nil, err
+	}
+
+	var commands []protocol.Command
+	if _, ok, _ := source.CanInvertIfCondition(pgf.File, start, end); ok {
+		cmd, err := command.NewApplyFixCommand("Invert if condition", command.ApplyFixArgs{
+			URI:   protocol.URIFromSpanURI(uri),
+			Fix:   source.InvertIfCondition,
+			Range: rng,
+		})
+		if err != nil {
+			return nil, err
+		}
+		commands = append(commands, cmd)
+	}
+
+	var actions []protocol.CodeAction
+	for i := range commands {
+		actions = append(actions, protocol.CodeAction{
+			Title:   commands[i].Title,
+			Kind:    protocol.RefactorRewrite,
+			Command: &commands[i],
+		})
+	}
+	return actions, nil
+}
+
 func documentChanges(fh source.FileHandle, edits []protocol.TextEdit) []protocol.DocumentChanges {
 	return []protocol.DocumentChanges{
 		{

gopls/internal/lsp/source/fix.go

@@ -34,22 +34,24 @@ type (
 )
 
 const (
-	FillStruct      = "fill_struct"
-	StubMethods     = "stub_methods"
-	UndeclaredName  = "undeclared_name"
-	ExtractVariable = "extract_variable"
-	ExtractFunction = "extract_function"
-	ExtractMethod   = "extract_method"
+	FillStruct        = "fill_struct"
+	StubMethods       = "stub_methods"
+	UndeclaredName    = "undeclared_name"
+	ExtractVariable   = "extract_variable"
+	ExtractFunction   = "extract_function"
+	ExtractMethod     = "extract_method"
+	InvertIfCondition = "invert_if_condition"
 )
 
 // suggestedFixes maps a suggested fix command id to its handler.
 var suggestedFixes = map[string]SuggestedFixFunc{
-	FillStruct:      singleFile(fillstruct.SuggestedFix),
-	UndeclaredName:  singleFile(undeclaredname.SuggestedFix),
-	ExtractVariable: singleFile(extractVariable),
-	ExtractFunction: singleFile(extractFunction),
-	ExtractMethod:   singleFile(extractMethod),
-	StubMethods:     stubSuggestedFixFunc,
+	FillStruct:        singleFile(fillstruct.SuggestedFix),
+	UndeclaredName:    singleFile(undeclaredname.SuggestedFix),
+	ExtractVariable:   singleFile(extractVariable),
+	ExtractFunction:   singleFile(extractFunction),
+	ExtractMethod:     singleFile(extractMethod),
+	InvertIfCondition: singleFile(invertIfCondition),
+	StubMethods:       stubSuggestedFixFunc,
 }
 
 // singleFile calls analyzers that expect inputs for a single file

gopls/internal/lsp/source/invertifcondition.go

@@ -0,0 +1,267 @@
+// Copyright 2023 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package source
+
+import (
+	"fmt"
+	"go/ast"
+	"go/token"
+	"go/types"
+	"strings"
+
+	"golang.org/x/tools/go/analysis"
+	"golang.org/x/tools/go/ast/astutil"
+	"golang.org/x/tools/gopls/internal/lsp/safetoken"
+)
+
+// invertIfCondition is a singleFileFixFunc that inverts an if/else statement
+func invertIfCondition(fset *token.FileSet, start, end token.Pos, src []byte, file *ast.File, _ *types.Package, _ *types.Info) (*analysis.SuggestedFix, error) {
+	ifStatement, _, err := CanInvertIfCondition(file, start, end)
+	if err != nil {
+		return nil, err
+	}
+
+	var replaceElse analysis.TextEdit
+
+	endsWithReturn, err := endsWithReturn(ifStatement.Else)
+	if err != nil {
+		return nil, err
+	}
+
+	if endsWithReturn {
+		// Replace the whole else part with an empty line and an unindented
+		// version of the original if body
+		sourcePos := safetoken.StartPosition(fset, ifStatement.Pos())
+
+		indent := sourcePos.Column - 1
+		if indent < 0 {
+			indent = 0
+		}
+
+		standaloneBodyText := ifBodyToStandaloneCode(fset, ifStatement.Body, src)
+		replaceElse = analysis.TextEdit{
+			Pos:     ifStatement.Body.Rbrace + 1, // 1 == len("}")
+			End:     ifStatement.End(),
+			NewText: []byte("\n\n" + strings.Repeat("\t", indent) + standaloneBodyText),
+		}
+	} else {
+		// Replace the else body text with the if body text
+		bodyStart := safetoken.StartPosition(fset, ifStatement.Body.Lbrace)
+		bodyEnd := safetoken.EndPosition(fset, ifStatement.Body.Rbrace+1) // 1 == len("}")
+		bodyText := src[bodyStart.Offset:bodyEnd.Offset]
+		replaceElse = analysis.TextEdit{
+			Pos:     ifStatement.Else.Pos(),
+			End:     ifStatement.Else.End(),
+			NewText: bodyText,
+		}
+	}
+
+	// Replace the if text with the else text
+	elsePosInSource := safetoken.StartPosition(fset, ifStatement.Else.Pos())
+	elseEndInSource := safetoken.EndPosition(fset, ifStatement.Else.End())
+	elseText := src[elsePosInSource.Offset:elseEndInSource.Offset]
+	replaceBodyWithElse := analysis.TextEdit{
+		Pos:     ifStatement.Body.Pos(),
+		End:     ifStatement.Body.End(),
+		NewText: elseText,
+	}
+
+	// Replace the if condition with its inverse
+	inverseCondition, err := invertCondition(fset, ifStatement.Cond, src)
+	if err != nil {
+		return nil, err
+	}
+	replaceConditionWithInverse := analysis.TextEdit{
+		Pos:     ifStatement.Cond.Pos(),
+		End:     ifStatement.Cond.End(),
+		NewText: inverseCondition,
+	}
+
+	// Return a SuggestedFix with just that TextEdit in there
+	return &analysis.SuggestedFix{
+		TextEdits: []analysis.TextEdit{
+			replaceConditionWithInverse,
+			replaceBodyWithElse,
+			replaceElse,
+		},
+	}, nil
+}
+
+func endsWithReturn(elseBranch ast.Stmt) (bool, error) {
+	elseBlock, isBlockStatement := elseBranch.(*ast.BlockStmt)
+	if !isBlockStatement {
+		return false, fmt.Errorf("Unable to figure out whether this ends with return: %T", elseBranch)
+	}
+
+	if len(elseBlock.List) == 0 {
+		// Empty blocks don't end in returns
+		return false, nil
+	}
+
+	lastStatement := elseBlock.List[len(elseBlock.List)-1]
+
+	_, lastStatementIsReturn := lastStatement.(*ast.ReturnStmt)
+	return lastStatementIsReturn, nil
+}
+
+// Turn { fmt.Println("Hello") } into just fmt.Println("Hello"), with one less
+// level of indentation.
+//
+// The first line of the result will not be indented, but all of the following
+// lines will.
+func ifBodyToStandaloneCode(fset *token.FileSet, ifBody *ast.BlockStmt, src []byte) string {
+	// Get the whole body (without the surrounding braces) as a string
+	bodyStart := safetoken.StartPosition(fset, ifBody.Lbrace+1) // 1 == len("}")
+	bodyEnd := safetoken.EndPosition(fset, ifBody.Rbrace)
+	bodyWithoutBraces := string(src[bodyStart.Offset:bodyEnd.Offset])
+	bodyWithoutBraces = strings.TrimSpace(bodyWithoutBraces)
+
+	// Unindent
+	bodyWithoutBraces = strings.ReplaceAll(bodyWithoutBraces, "\n\t", "\n")
+
+	return bodyWithoutBraces
+}
+
+func invertCondition(fset *token.FileSet, cond ast.Expr, src []byte) ([]byte, error) {
+	condStart := safetoken.StartPosition(fset, cond.Pos())
+	condEnd := safetoken.EndPosition(fset, cond.End())
+	oldText := string(src[condStart.Offset:condEnd.Offset])
+
+	switch expr := cond.(type) {
+	case *ast.Ident, *ast.ParenExpr, *ast.CallExpr, *ast.StarExpr, *ast.IndexExpr, *ast.IndexListExpr, *ast.SelectorExpr:
+		newText := "!" + oldText
+		if oldText == "true" {
+			newText = "false"
+		} else if oldText == "false" {
+			newText = "true"
+		}
+
+		return []byte(newText), nil
+
+	case *ast.UnaryExpr:
+		if expr.Op != token.NOT {
+			// This should never happen
+			return dumbInvert(fset, cond, src), nil
+		}
+
+		inverse := expr.X
+		if p, isParen := inverse.(*ast.ParenExpr); isParen {
+			// We got !(x), remove the parentheses with the ! so we get just "x"
+			inverse = p.X
+
+			start := safetoken.StartPosition(fset, inverse.Pos())
+			end := safetoken.EndPosition(fset, inverse.End())
+			if start.Line != end.Line {
+				// The expression is multi-line, so we can't remove the parentheses
+				inverse = expr.X
+			}
+		}
+
+		start := safetoken.StartPosition(fset, inverse.Pos())
+		end := safetoken.EndPosition(fset, inverse.End())
+		textWithoutNot := src[start.Offset:end.Offset]
+
+		return textWithoutNot, nil
+
+	case *ast.BinaryExpr:
+		// These inversions are unsound for floating point NaN, but that's ok.
+		negations := map[token.Token]string{
+			token.EQL: "!=",
+			token.LSS: ">=",
+			token.GTR: "<=",
+			token.NEQ: "==",
+			token.LEQ: ">",
+			token.GEQ: "<",
+		}
+
+		negation, negationFound := negations[expr.Op]
+		if !negationFound {
+			return invertAndOr(fset, expr, src)
+		}
+
+		xPosInSource := safetoken.StartPosition(fset, expr.X.Pos())
+		opPosInSource := safetoken.StartPosition(fset, expr.OpPos)
+		yPosInSource := safetoken.StartPosition(fset, expr.Y.Pos())
+
+		textBeforeOp := string(src[xPosInSource.Offset:opPosInSource.Offset])
+
+		oldOpWithTrailingWhitespace := string(src[opPosInSource.Offset:yPosInSource.Offset])
+		newOpWithTrailingWhitespace := negation + oldOpWithTrailingWhitespace[len(expr.Op.String()):]
+
+		textAfterOp := string(src[yPosInSource.Offset:condEnd.Offset])
+
+		return []byte(textBeforeOp + newOpWithTrailingWhitespace + textAfterOp), nil
+	}
+
+	return dumbInvert(fset, cond, src), nil
+}
+
+// dumbInvert is a fallback, inverting cond into !(cond).
+func dumbInvert(fset *token.FileSet, expr ast.Expr, src []byte) []byte {
+	start := safetoken.StartPosition(fset, expr.Pos())
+	end := safetoken.EndPosition(fset, expr.End())
+	text := string(src[start.Offset:end.Offset])
+	return []byte("!(" + text + ")")
+}
+
+func invertAndOr(fset *token.FileSet, expr *ast.BinaryExpr, src []byte) ([]byte, error) {
+	if expr.Op != token.LAND && expr.Op != token.LOR {
+		// Neither AND nor OR, don't know how to invert this
+		return dumbInvert(fset, expr, src), nil
+	}
+
+	oppositeOp := "&&"
+	if expr.Op == token.LAND {
+		oppositeOp = "||"
+	}
+
+	xEndInSource := safetoken.EndPosition(fset, expr.X.End())
+	opPosInSource := safetoken.StartPosition(fset, expr.OpPos)
+	whitespaceAfterBefore := src[xEndInSource.Offset:opPosInSource.Offset]
+
+	invertedBefore, err := invertCondition(fset, expr.X, src)
+	if err != nil {
+		return nil, err
+	}
+
+	invertedAfter, err := invertCondition(fset, expr.Y, src)
+	if err != nil {
+		return nil, err
+	}
+
+	yPosInSource := safetoken.StartPosition(fset, expr.Y.Pos())
+
+	oldOpWithTrailingWhitespace := string(src[opPosInSource.Offset:yPosInSource.Offset])
+	newOpWithTrailingWhitespace := oppositeOp + oldOpWithTrailingWhitespace[len(expr.Op.String()):]
+
+	return []byte(string(invertedBefore) + string(whitespaceAfterBefore) + newOpWithTrailingWhitespace + string(invertedAfter)), nil
+}
+
+// CanInvertIfCondition reports whether we can do invert-if-condition on the
+// code in the given range
+func CanInvertIfCondition(file *ast.File, start, end token.Pos) (*ast.IfStmt, bool, error) {
+	path, _ := astutil.PathEnclosingInterval(file, start, end)
+	for _, node := range path {
+		stmt, isIfStatement := node.(*ast.IfStmt)
+		if !isIfStatement {
+			continue
+		}
+
+		if stmt.Else == nil {
+			// Can't invert conditions without else clauses
+			return nil, false, fmt.Errorf("else clause required")
+		}
+
+		if _, hasElseIf := stmt.Else.(*ast.IfStmt); hasElseIf {
+			// Can't invert conditions with else-if clauses, unclear what that
+			// would look like
+			return nil, false, fmt.Errorf("else-if not supported")
+		}
+
+		return stmt, true, nil
+	}
+
+	return nil, false, fmt.Errorf("not an if statement")
+}

gopls/internal/lsp/testdata/invertifcondition/boolean.go

@@ -0,0 +1,14 @@
+package invertifcondition
+
+import (
+	"fmt"
+)
+
+func Boolean() {
+	b := true
+	if b { //@suggestedfix("if b", "refactor.rewrite", "")
+		fmt.Println("A")
+	} else {
+		fmt.Println("B")
+	}
+}

gopls/internal/lsp/testdata/invertifcondition/boolean.go.golden

@@ -0,0 +1,16 @@
+-- suggestedfix_boolean_9_2 --
+package invertifcondition
+
+import (
+	"fmt"
+)
+
+func Boolean() {
+	b := true
+	if !b {
+		fmt.Println("B")
+	} else { //@suggestedfix("if b", "refactor.rewrite", "")
+		fmt.Println("A")
+	}
+}
+