Repository: mgechev/revive
Pull request #832 (2023-05-20 09:18:52)

ifelse: option to preserve variable scope
The idea is to give users a option to suppress prompts of the kind indicated in [#816](https://github.com/mgechev/revive/issues/816), where the benefit of refactoring may not obviously outweigh the cost of increasing variable scope.

The logic as it stands, is that the `early-return` / `indent-error-flow` / `superfluous-else` rule will be skipped if `preserveScope` is given as a configuration argument and:
- At least one if the `if` blocks in the chain has an `if`-initializer (i.e. we bring variables defined in the initializer into the surrounding scope), or
- The block to be de-indented contains at least one declaration (i.e. we bring variables defined at the top level of the "if" or "else" branch body into the surrounding scope), and
- The if-else chain is not located at the end of its own surrounding block (since in that case the relocated variables would go out of scope at the same point either way)

This is a slightly refinement on what I initially had in mind (ignore any chain with an if-initializer), but hopefully it makes sense. Tests have been added to cover the new behaviour. If not enabled, the rules work the same as before. Open to suggestions on the naming/wording.

Closes [#816](https://github.com/mgechev/revive/issues/816).

chavacava: Hi @mdelah, thanks for the PR!

I'm testing the new code and I have a case that seems not to be working OK:
Config:
```toml
ignoreGeneratedHeader = false
severity = "warning"
confidence = 0.8
errorCode = 0
warningCode = 0

[rule.early-return]
    arguments = ["preserveScope"]
```
Input code (`testdata/early-return.go`):
```go
// Test of empty-blocks.

package fixtures

func earlyRet() bool {
	if ok := call(); ok {
		println()
	} else {
		return nil
	}
}
```
Output:
```
testdata/early-return.go:6:2: if c { ... } else { ... return } can be simplified to if !c { ... return } ... (move short variable declaration to its own line if necessary)
```

My understanding of your proposal is that with the above configuration (_preserveScope_) the rule must not spot the problem. I'm right?
mdelah: Hi @chavacava, indeed, that code would fall under the third bullet point above ("The if-else chain is not located at the end of its own surrounding block..."). The enclosing function ends afterwards. I figured it would still be helpful to show the suggestion there, because `ok` will fall out of scope anyway. I think if you add more code at the end of the function, it will suppress the message.

What do you think, is it reasonable or does it muddy the water too much? Should the docs articulate this more clearly?

Patch:
diff --git a/README.md b/README.md
index a4e852f..d402717 100644
--- a/README.md
+++ b/README.md
@@ -448,13 +448,13 @@ List of all available rules. The rules ported from `golint` are left unchanged a
 | [`package-comments`](./RULES_DESCRIPTIONS.md#package-comments)    |  n/a   | Package commenting conventions.                                  |   yes    |  no   |
 | [`range`](./RULES_DESCRIPTIONS.md#range)               |  n/a   | Prevents redundant variables when iterating over a collection.   |   yes    |  no   |
 | [`receiver-naming`](./RULES_DESCRIPTIONS.md#receiver-naming)     |  n/a   | Conventions around the naming of receivers.                      |   yes    |  no   |
-| [`indent-error-flow`](./RULES_DESCRIPTIONS.md#indent-error-flow)   |  n/a   | Prevents redundant else statements.                              |   yes    |  no   |
+| [`indent-error-flow`](./RULES_DESCRIPTIONS.md#indent-error-flow)   |  []string   | Prevents redundant else statements.                              |   yes    |  no   |
 | [`argument-limit`](./RULES_DESCRIPTIONS.md#argument-limit)      |  int (defaults to 8)  | Specifies the maximum number of arguments a function can receive |    no    |  no   |
 | [`cyclomatic`](./RULES_DESCRIPTIONS.md#cyclomatic)          |  int (defaults to 10)   | Sets restriction for maximum Cyclomatic complexity.              |    no    |  no   |
 | [`max-public-structs`](./RULES_DESCRIPTIONS.md#max-public-structs)  |  int (defaults to 5)  | The maximum number of public structs in a file.                  |    no    |  no   |
 | [`file-header`](./RULES_DESCRIPTIONS.md#file-header)         | string (defaults to none)| Header which each file should have.                              |    no    |  no   |
 | [`empty-block`](./RULES_DESCRIPTIONS.md#empty-block)         |  n/a   | Warns on empty code blocks                                       |    no    |  yes   |
-| [`superfluous-else`](./RULES_DESCRIPTIONS.md#superfluous-else)    |  n/a   | Prevents redundant else statements (extends [`indent-error-flow`](./RULES_DESCRIPTIONS.md#indent-error-flow)) |    no    |  no   |
+| [`superfluous-else`](./RULES_DESCRIPTIONS.md#superfluous-else)    |  []string   | Prevents redundant else statements (extends [`indent-error-flow`](./RULES_DESCRIPTIONS.md#indent-error-flow)) |    no    |  no   |
 | [`confusing-naming`](./RULES_DESCRIPTIONS.md#confusing-naming)    |  n/a   | Warns on methods with names that differ only by capitalization   |    no    |  no   |
 | [`get-return`](./RULES_DESCRIPTIONS.md#get-return)          |  n/a   | Warns on getters that do not yield any result                    |    no    |  no   |
 | [`modifies-parameter`](./RULES_DESCRIPTIONS.md#modifies-parameter)  |  n/a   | Warns on assignments to function parameters                      |    no    |  no   |
@@ -487,7 +487,7 @@ List of all available rules. The rules ported from `golint` are left unchanged a
 | [`cognitive-complexity`](./RULES_DESCRIPTIONS.md#cognitive-complexity)          |  int (defaults to 7) | Sets restriction for maximum Cognitive complexity.              |    no    |  no   |
 | [`string-of-int`](./RULES_DESCRIPTIONS.md#string-of-int)          |  n/a   | Warns on suspicious casts from int to string            |    no    |  yes   |
 | [`string-format`](./RULES_DESCRIPTIONS.md#string-format)          |  map   | Warns on specific string literals that fail one or more user-configured regular expressions            |    no    |  no   |
-| [`early-return`](./RULES_DESCRIPTIONS.md#early-return)          |  n/a   | Spots if-then-else statements where the predicate may be inverted to reduce nesting |    no    |  no   |
+| [`early-return`](./RULES_DESCRIPTIONS.md#early-return)          | []string   | Spots if-then-else statements where the predicate may be inverted to reduce nesting |    no    |  no   |
 | [`unconditional-recursion`](./RULES_DESCRIPTIONS.md#unconditional-recursion)          |  n/a   | Warns on function calls that will lead to (direct) infinite recursion |    no    |  no   |
 | [`identical-branches`](./RULES_DESCRIPTIONS.md#identical-branches)          |  n/a   | Spots if-then-else statements with identical `then` and `else` branches       |    no    |  no   |
 | [`defer`](./RULES_DESCRIPTIONS.md#defer)          |  map   |  Warns on some [defer gotchas](https://blog.learngoprogramming.com/5-gotchas-of-defer-in-go-golang-part-iii-36a1ab3d6ef1)       |    no    |  no   |
diff --git a/RULES_DESCRIPTIONS.md b/RULES_DESCRIPTIONS.md
index c52719b..02ad606 100644
--- a/RULES_DESCRIPTIONS.md
+++ b/RULES_DESCRIPTIONS.md
@@ -299,7 +299,7 @@ if cond {
 ```
 where the `if` condition may be inverted in order to reduce nesting:
 ```go
-if ! cond {
+if !cond {
   // do other thing
   return ...
 }
@@ -307,7 +307,16 @@ if ! cond {
 // do something
 ```
 
-_Configuration_: N/A
+_Configuration_: ([]string) rule flags. Available flags are:
+
+* _preserveScope_: do not suggest refactorings that would increase variable scope
+
+Example:
+
+```toml
+[rule.exported]
+  arguments =["preserveScope"]
+```
 
 ## empty-block
 
@@ -448,7 +457,16 @@ This rule highlights redundant _else-blocks_ that can be eliminated from the cod
 
 More information [here](https://github.com/golang/go/wiki/CodeReviewComments#indent-error-flow)
 
-_Configuration_: N/A
+_Configuration_: ([]string) rule flags. Available flags are:
+
+* _preserveScope_: do not suggest refactorings that would increase variable scope
+
+Example:
+
+```toml
+[rule.exported]
+  arguments =["preserveScope"]
+```
 
 ## imports-blacklist
 
@@ -624,7 +642,16 @@ To accept the `inline` option in JSON tags (and `outline` and `gnu` in BSON tags
 _Description_: To improve the readability of code, it is recommended to reduce the indentation as much as possible.
 This rule highlights redundant _else-blocks_ that can be eliminated from the code.
 
-_Configuration_: N/A
+_Configuration_: ([]string) rule flags. Available flags are:
+
+* _preserveScope_: do not suggest refactorings that would increase variable scope
+
+Example:
+
+```toml
+[rule.exported]
+  arguments =["preserveScope"]
+```
 
 ## time-equal
 
diff --git a/internal/ifelse/args.go b/internal/ifelse/args.go
new file mode 100644
index 0000000..c6e647e
--- /dev/null
+++ b/internal/ifelse/args.go
@@ -0,0 +1,11 @@
+package ifelse
+
+// PreserveScope is a configuration argument that prevents suggestions
+// that would enlarge variable scope
+const PreserveScope = "preserveScope"
+
+// Args contains arguments common to the early-return, indent-error-flow
+// and superfluous-else rules (currently just preserveScope)
+type Args struct {
+	PreserveScope bool
+}
diff --git a/internal/ifelse/branch.go b/internal/ifelse/branch.go
index 0d51a0a..6e6036b 100644
--- a/internal/ifelse/branch.go
+++ b/internal/ifelse/branch.go
@@ -9,29 +9,37 @@ import (
 // Branch contains information about a branch within an if-else chain.
 type Branch struct {
 	BranchKind
-	Call // The function called at the end for kind Panic or Exit.
+	Call          // The function called at the end for kind Panic or Exit.
+	HasDecls bool // The branch has one or more declarations (at the top level block)
 }
 
 // BlockBranch gets the Branch of an ast.BlockStmt.
 func BlockBranch(block *ast.BlockStmt) Branch {
 	blockLen := len(block.List)
 	if blockLen == 0 {
-		return Branch{BranchKind: Empty}
+		return Empty.Branch()
 	}
 
-	switch stmt := block.List[blockLen-1].(type) {
+	branch := StmtBranch(block.List[blockLen-1])
+	branch.HasDecls = hasDecls(block)
+	return branch
+}
+
+// StmtBranch gets the Branch of an ast.Stmt.
+func StmtBranch(stmt ast.Stmt) Branch {
+	switch stmt := stmt.(type) {
 	case *ast.ReturnStmt:
-		return Branch{BranchKind: Return}
+		return Return.Branch()
 	case *ast.BlockStmt:
 		return BlockBranch(stmt)
 	case *ast.BranchStmt:
 		switch stmt.Tok {
 		case token.BREAK:
-			return Branch{BranchKind: Break}
+			return Break.Branch()
 		case token.CONTINUE:
-			return Branch{BranchKind: Continue}
+			return Continue.Branch()
 		case token.GOTO:
-			return Branch{BranchKind: Goto}
+			return Goto.Branch()
 		}
 	case *ast.ExprStmt:
 		fn, ok := ExprCall(stmt)
@@ -42,9 +50,12 @@ func BlockBranch(block *ast.BlockStmt) Branch {
 		if ok {
 			return Branch{BranchKind: kind, Call: fn}
 		}
+	case *ast.EmptyStmt:
+		return Empty.Branch()
+	case *ast.LabeledStmt:
+		return StmtBranch(stmt.Stmt)
 	}
-
-	return Branch{BranchKind: Regular}
+	return Regular.Branch()
 }
 
 // String returns a brief string representation
@@ -66,3 +77,17 @@ func (b Branch) LongString() string {
 		return b.BranchKind.LongString()
 	}
 }
+
+func hasDecls(block *ast.BlockStmt) bool {
+	for _, stmt := range block.List {
+		switch stmt := stmt.(type) {
+		case *ast.DeclStmt:
+			return true
+		case *ast.AssignStmt:
+			if stmt.Tok == token.DEFINE {
+				return true
+			}
+		}
+	}
+	return false
+}
diff --git a/internal/ifelse/branch_kind.go b/internal/ifelse/branch_kind.go
index 7df276f..41601d1 100644
--- a/internal/ifelse/branch_kind.go
+++ b/internal/ifelse/branch_kind.go
@@ -49,6 +49,9 @@ func (k BranchKind) Deviates() bool {
 	}
 }
 
+// Branch returns a Branch with the given kind
+func (k BranchKind) Branch() Branch { return Branch{BranchKind: k} }
+
 // String returns a brief string representation
 func (k BranchKind) String() string {
 	switch k {
diff --git a/internal/ifelse/chain.go b/internal/ifelse/chain.go
index d6afebf..9891635 100644
--- a/internal/ifelse/chain.go
+++ b/internal/ifelse/chain.go
@@ -6,4 +6,5 @@ type Chain struct {
 	Else                 Branch // what happens at the end of the "else" block
 	HasInitializer       bool   // is there an "if"-initializer somewhere in the chain?
 	HasPriorNonDeviating bool   // is there a prior "if" block that does NOT deviate control flow?
+	AtBlockEnd           bool   // whether the chain is placed at the end of the surrounding block
 }
diff --git a/internal/ifelse/rule.go b/internal/ifelse/rule.go
index ec0db01..07ad456 100644
--- a/internal/ifelse/rule.go
+++ b/internal/ifelse/rule.go
@@ -9,7 +9,7 @@ import (
 
 // Rule is an interface for linters operating on if-else chains
 type Rule interface {
-	CheckIfElse(chain Chain) (failMsg string)
+	CheckIfElse(chain Chain, args Args) (failMsg string)
 }
 
 // Apply evaluates the given Rule on if-else chains found within the given AST,
@@ -28,8 +28,13 @@ type Rule interface {
 //
 // Only the block following "bar" is linted. This is because the rules that use this function
 // do not presently have anything to say about earlier blocks in the chain.
-func Apply(rule Rule, node ast.Node, target Target) []lint.Failure {
+func Apply(rule Rule, node ast.Node, target Target, args lint.Arguments) []lint.Failure {
 	v := &visitor{rule: rule, target: target}
+	for _, arg := range args {
+		if arg == PreserveScope {
+			v.args.PreserveScope = true
+		}
+	}
 	ast.Walk(v, node)
 	return v.failures
 }
@@ -38,15 +43,22 @@ type visitor struct {
 	failures []lint.Failure
 	target   Target
 	rule     Rule
+	args     Args
 }
 
 func (v *visitor) Visit(node ast.Node) ast.Visitor {
-	ifStmt, ok := node.(*ast.IfStmt)
+	block, ok := node.(*ast.BlockStmt)
 	if !ok {
 		return v
 	}
 
-	v.visitChain(ifStmt, Chain{})
+	for i, stmt := range block.List {
+		if ifStmt, ok := stmt.(*ast.IfStmt); ok {
+			v.visitChain(ifStmt, Chain{AtBlockEnd: i == len(block.List)-1})
+			continue
+		}
+		ast.Walk(v, stmt)
+	}
 	return nil
 }
 
@@ -73,8 +85,9 @@ func (v *visitor) visitChain(ifStmt *ast.IfStmt, chain Chain) {
 	case *ast.BlockStmt:
 		// look for other if-else chains nested inside this else { } block
 		ast.Walk(v, elseBlock)
+
 		chain.Else = BlockBranch(elseBlock)
-		if failMsg := v.rule.CheckIfElse(chain); failMsg != "" {
+		if failMsg := v.rule.CheckIfElse(chain, v.args); failMsg != "" {
 			if chain.HasInitializer {
 				// if statement has a := initializer, so we might need to move the assignment
 				// onto its own line in case the body references it
diff --git a/rule/early-return.go b/rule/early-return.go
index f5f8ff5..90a0acc 100644
--- a/rule/early-return.go
+++ b/rule/early-return.go
@@ -12,8 +12,8 @@ import (
 type EarlyReturnRule struct{}
 
 // Apply applies the rule to given file.
-func (e *EarlyReturnRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
-	return ifelse.Apply(e, file.AST, ifelse.TargetIf)
+func (e *EarlyReturnRule) Apply(file *lint.File, args lint.Arguments) []lint.Failure {
+	return ifelse.Apply(e, file.AST, ifelse.TargetIf, args)
 }
 
 // Name returns the rule name.
@@ -22,7 +22,7 @@ func (*EarlyReturnRule) Name() string {
 }
 
 // CheckIfElse evaluates the rule against an ifelse.Chain.
-func (e *EarlyReturnRule) CheckIfElse(chain ifelse.Chain) (failMsg string) {
+func (e *EarlyReturnRule) CheckIfElse(chain ifelse.Chain, args ifelse.Args) (failMsg string) {
 	if !chain.Else.Deviates() {
 		// this rule only applies if the else-block deviates control flow
 		return
@@ -39,6 +39,11 @@ func (e *EarlyReturnRule) CheckIfElse(chain ifelse.Chain) (failMsg string) {
 		return
 	}
 
+	if args.PreserveScope && !chain.AtBlockEnd && (chain.HasInitializer || chain.If.HasDecls) {
+		// avoid increasing variable scope
+		return
+	}
+
 	if chain.If.IsEmpty() {
 		return fmt.Sprintf("if c { } else { %[1]v } can be simplified to if !c { %[1]v }", chain.Else)
 	}
diff --git a/rule/indent-error-flow.go b/rule/indent-error-flow.go
index 4d62404..b80e648 100644
--- a/rule/indent-error-flow.go
+++ b/rule/indent-error-flow.go
@@ -9,8 +9,8 @@ import (
 type IndentErrorFlowRule struct{}
 
 // Apply applies the rule to given file.
-func (e *IndentErrorFlowRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
-	return ifelse.Apply(e, file.AST, ifelse.TargetElse)
+func (e *IndentErrorFlowRule) Apply(file *lint.File, args lint.Arguments) []lint.Failure {
+	return ifelse.Apply(e, file.AST, ifelse.TargetElse, args)
 }
 
 // Name returns the rule name.
@@ -19,7 +19,7 @@ func (*IndentErrorFlowRule) Name() string {
 }
 
 // CheckIfElse evaluates the rule against an ifelse.Chain.
-func (e *IndentErrorFlowRule) CheckIfElse(chain ifelse.Chain) (failMsg string) {
+func (e *IndentErrorFlowRule) CheckIfElse(chain ifelse.Chain, args ifelse.Args) (failMsg string) {
 	if !chain.If.Deviates() {
 		// this rule only applies if the if-block deviates control flow
 		return
@@ -36,5 +36,10 @@ func (e *IndentErrorFlowRule) CheckIfElse(chain ifelse.Chain) (failMsg string) {
 		return
 	}
 
+	if args.PreserveScope && !chain.AtBlockEnd && (chain.HasInitializer || chain.Else.HasDecls) {
+		// avoid increasing variable scope
+		return
+	}
+
 	return "if block ends with a return statement, so drop this else and outdent its block"
 }
diff --git a/rule/superfluous-else.go b/rule/superfluous-else.go
index 9dce59a..1ef67bf 100644
--- a/rule/superfluous-else.go
+++ b/rule/superfluous-else.go
@@ -10,8 +10,8 @@ import (
 type SuperfluousElseRule struct{}
 
 // Apply applies the rule to given file.
-func (e *SuperfluousElseRule) Apply(file *lint.File, _ lint.Arguments) []lint.Failure {
-	return ifelse.Apply(e, file.AST, ifelse.TargetElse)
+func (e *SuperfluousElseRule) Apply(file *lint.File, args lint.Arguments) []lint.Failure {
+	return ifelse.Apply(e, file.AST, ifelse.TargetElse, args)
 }
 
 // Name returns the rule name.
@@ -20,7 +20,7 @@ func (*SuperfluousElseRule) Name() string {
 }
 
 // CheckIfElse evaluates the rule against an ifelse.Chain.
-func (e *SuperfluousElseRule) CheckIfElse(chain ifelse.Chain) (failMsg string) {
+func (e *SuperfluousElseRule) CheckIfElse(chain ifelse.Chain, args ifelse.Args) (failMsg string) {
 	if !chain.If.Deviates() {
 		// this rule only applies if the if-block deviates control flow
 		return
@@ -37,5 +37,10 @@ func (e *SuperfluousElseRule) CheckIfElse(chain ifelse.Chain) (failMsg string) {
 		return
 	}
 
+	if args.PreserveScope && !chain.AtBlockEnd && (chain.HasInitializer || chain.Else.HasDecls) {
+		// avoid increasing variable scope
+		return
+	}
+
 	return fmt.Sprintf("if block ends with %v, so drop this else and outdent its block", chain.If.LongString())
 }


Test patch:
diff --git a/test/early-return_test.go b/test/early-return_test.go
index 0f1b63b..a8f54f3 100644
--- a/test/early-return_test.go
+++ b/test/early-return_test.go
@@ -3,10 +3,13 @@ package test
 import (
 	"testing"
 
+	"github.com/mgechev/revive/internal/ifelse"
+	"github.com/mgechev/revive/lint"
 	"github.com/mgechev/revive/rule"
 )
 
 // TestEarlyReturn tests early-return rule.
 func TestEarlyReturn(t *testing.T) {
 	testRule(t, "early-return", &rule.EarlyReturnRule{})
+	testRule(t, "early-return-scope", &rule.EarlyReturnRule{}, &lint.RuleConfig{Arguments: []interface{}{ifelse.PreserveScope}})
 }
diff --git a/test/superfluous-else_test.go b/test/superfluous-else_test.go
index ab68b30..0737777 100644
--- a/test/superfluous-else_test.go
+++ b/test/superfluous-else_test.go
@@ -3,10 +3,13 @@ package test
 import (
 	"testing"
 
+	"github.com/mgechev/revive/internal/ifelse"
+	"github.com/mgechev/revive/lint"
 	"github.com/mgechev/revive/rule"
 )
 
 // TestSuperfluousElse rule.
 func TestSuperfluousElse(t *testing.T) {
 	testRule(t, "superfluous-else", &rule.SuperfluousElseRule{})
+	testRule(t, "superfluous-else-scope", &rule.SuperfluousElseRule{}, &lint.RuleConfig{Arguments: []interface{}{ifelse.PreserveScope}})
 }
diff --git a/testdata/early-return-scope.go b/testdata/early-return-scope.go
new file mode 100644
index 0000000..a9e8118
--- /dev/null
+++ b/testdata/early-return-scope.go
@@ -0,0 +1,66 @@
+// Test data for the early-return rule with preserveScope option enabled
+
+package fixtures
+
+func fn1() {
+	// No initializer, match as normal
+	if cond { //   MATCH /if c { ... } else { ... return } can be simplified to if !c { ... return } .../
+		fn2()
+	} else {
+		return
+	}
+}
+
+func fn2() {
+	// Moving the declaration of x here is fine since it goes out of scope either way
+	if x := fn1(); x != nil { //   MATCH /if c { ... } else { ... return } can be simplified to if !c { ... return } ... (move short variable declaration to its own line if necessary)/
+		fn2()
+	} else {
+		return
+	}
+}
+
+func fn3() {
+	// Don't want to move the declaration of x here since it stays in scope afterward
+	if x := fn1(); x != nil {
+		fn2()
+	} else {
+		return
+	}
+	x := fn2()
+	fn3(x)
+}
+
+func fn4() {
+	if cond {
+		var x = fn2()
+		fn3(x)
+	} else {
+		return
+	}
+	// Don't want to move the declaration of x here since it stays in scope afterward
+	y := fn2()
+	fn3(y)
+}
+
+func fn5() {
+	if cond {
+		x := fn2()
+		fn3(x)
+	} else {
+		return
+	}
+	// Don't want to move the declaration of x here since it stays in scope afterward
+	y := fn2()
+	fn3(y)
+}
+
+func fn6() {
+	if cond { //   MATCH /if c { ... } else { ... return } can be simplified to if !c { ... return } .../
+		x := fn2()
+		fn3(x)
+	} else {
+		return
+	}
+	// Moving x here is fine since it goes out of scope anyway
+}
diff --git a/testdata/superfluous-else-scope.go b/testdata/superfluous-else-scope.go
new file mode 100644
index 0000000..d1d6797
--- /dev/null
+++ b/testdata/superfluous-else-scope.go
@@ -0,0 +1,64 @@
+// Test data for the superfluous-else rule with preserveScope option enabled
+
+package fixtures
+
+func fn1() {
+	for {
+		// No initializer, match as normal
+		if cond {
+			continue
+		} else { // MATCH /if block ends with a continue statement, so drop this else and outdent its block/
+			fn2()
+		}
+	}
+}
+
+func fn2() {
+	for {
+		// Moving the declaration of x here is fine since it goes out of scope either way
+		if x := fn1(); x != nil {
+			continue
+		} else { // MATCH /if block ends with a continue statement, so drop this else and outdent its block (move short variable declaration to its own line if necessary)/
+			fn2()
+		}
+	}
+}
+
+func fn3() {
+	for {
+		// Don't want to move the declaration of x here since it stays in scope afterward
+		if x := fn1(); x != nil {
+			continue
+		} else {
+			fn2()
+		}
+		x := fn2()
+		fn3(x)
+	}
+}
+
+func fn4() {
+	for {
+		if cond {
+			continue
+		} else {
+			x := fn1()
+			fn2(x)
+		}
+		// Don't want to move the declaration of x here since it stays in scope afterward
+		y := fn2()
+		fn3(y)
+	}
+}
+
+func fn4() {
+	for {
+		if cond {
+			continue
+		} else { // MATCH /if block ends with a continue statement, so drop this else and outdent its block/
+			x := fn1()
+			fn2(x)
+		}
+		// Moving x here is fine since it goes out of scope anyway
+	}
+}