Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

REF: Fix introduce variable when first expression is inside nested block #5053

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 18 additions & 5 deletions src/main/kotlin/org/rust/ide/refactoring/introduceVariable/impl.kt
Expand Up @@ -10,6 +10,7 @@ import com.intellij.openapi.project.Project
import com.intellij.psi.PsiDocumentManager
import com.intellij.psi.PsiElement
import com.intellij.psi.PsiParserFacade
import com.intellij.psi.util.PsiTreeUtil
import org.rust.ide.refactoring.*
import org.rust.lang.core.psi.*
import org.rust.lang.core.psi.ext.ancestorOrSelf
Expand Down Expand Up @@ -76,8 +77,7 @@ private class ExpressionReplacer(


fun replaceElementForAllExpr(exprs: List<PsiElement>) {
val anchor = findAnchor(exprs.minBy { it.startOffset } ?: chosenExpr)
?: return
val anchor = findAnchor(exprs, chosenExpr) ?: return

val suggestedNames = chosenExpr.suggestedNames()
val let = createLet(suggestedNames.default)
Expand Down Expand Up @@ -109,18 +109,31 @@ private class ExpressionReplacer(
val context = anchor.parent
val newline = PsiParserFacade.SERVICE.getInstance(project).createWhiteSpaceFromText("\n")

return context?.addBefore(let, context.addBefore(newline, anchor))
val result = context.addBefore(let, anchor)
context.addAfter(newline, result)
return result
}
}

/**
* An anchor point is surrounding element before the block scope, which is used to scope the insertion of the new let binding.
*/
private fun findAnchor(expr: PsiElement): PsiElement? {
val block = expr.ancestorOrSelf<RsBlock>()
return findAnchor(expr, expr)
}

private fun findAnchor(exprs: List<PsiElement>, chosenExpr: RsExpr): PsiElement? {
val commonParent = PsiTreeUtil.findCommonParent(chosenExpr, *exprs.toTypedArray())
?: return null
val firstExpr = exprs.minBy { it.startOffset } ?: chosenExpr
return findAnchor(commonParent, firstExpr)
}

private fun findAnchor(commonParent: PsiElement, firstExpr: PsiElement): PsiElement? {
val block = commonParent.ancestorOrSelf<RsBlock>()
?: return null

var anchor = expr
var anchor = firstExpr
while (anchor.parent != block) {
anchor = anchor.parent
}
Expand Down
Expand Up @@ -136,8 +136,8 @@ class RsIntroduceVariableHandlerTest : RsTestBase() {
""", listOf("String::new()", "&mut String::new()", "file.read_to_string(&mut String::new())"), 0, """
fn read_file() -> Result<String, Error> {
let file = File::open("res/input.txt")?;
let mut string = String::new();

let mut string = String::new();
file.read_to_string(&mut string)
}
""")
Expand Down Expand Up @@ -201,8 +201,8 @@ class RsIntroduceVariableHandlerTest : RsTestBase() {
let i2 = 1;
let i3 = 1;
let (i, x) = (1, 2);
let i4 = 3;

let i4 = 3;
let z = i4 + 4;
let w = x + 5;
}
Expand Down Expand Up @@ -274,6 +274,34 @@ class RsIntroduceVariableHandlerTest : RsTestBase() {
}
""")

fun `test first anchor inside block`() = doTest("""
fn main() {
let x1 = if true { { 7 } } else { 0 };
let x2 = /*caret*/7;
}
""", emptyList(), 0, """
fn main() {
let i = 7;
let x1 = if true { { i } } else { 0 };
let x2 = i;
}
""", replaceAll = true)

fun `test newline before anchor`() = doTest("""
fn main() {
const C: i32 = 1;

let x = /*caret*/7;
}
""", emptyList(), 0, """
fn main() {
const C: i32 = 1;

let i = 7;
let x = i;
}
""")

private fun doTest(
@Language("Rust") before: String,
expressions: List<String>,
Expand Down