Rust: splitting multi-line expression into blocks produces broken code

[Rodrigodd]

If you try to split an expression into a block, only the first line of the expression is wrapped. This appears to happen both on match arms and closures:

// before
|x| match x {
    _ => {}
}
// after
|x| {
    match x {
    }    _ => {}
}

// before
match x {
    y => Struct {
        w,
        z,
    }
}
// after
match x {
    y => {
        Struct {
        },
        w,
        z,
    }
}

// before
|x| if x {
    println!("x")
}
// after
|x| {
    if x {
    }    println!("x")
}

Appears to happen on any multiline expression, like match or if expressions, struct expression, arrays, tuples, etc.

The reverse, joining a single expression multi-line block, just do nothing.

When I first encounter this issue, I thought that it would be necessary to parse the Rust code to fix this, but thinking now, wrapping the block around everything until the matching }, ), ] or " would already fix all well-formatted code that I could think of.

[AndrewRadev]

Hmm, I've pushed some fixes, but I don't know if you'll agree with me about whether they solve the problem.

Splitjoin makes the assumption that when you're splitting, you're working on one line, and if you're joining, you're joining multiple. This is easier implementation-wise, because it means I can add sanity checks for the individual checkers -- "this line is incomplete, so don't even attempt to split it" or "this expression is one-line, there's nothing to join". This is easier for me to think of as well -- if there's multiple ways to reformat a piece of code, I might slurp it up into one line first and then target something and split it differently.

In your examples, I feel that the bug is that the split does anything at all. For instance this:

// before
|x| match x {
    _ => {}
}

The expression ends in {, so it should have been clear to splitjoin not to touch this, but I've missed this scenario. The code I pushed tries to stop splitjoin from splitting the line further. Instead, what I would do here is go to the match, and join it into this:

|x| match x { _ => {} }

And then, with the cursor on the |x| or on the match, split to this:

|x| {
    match x { _ => {} }
}

And then further split the match. It's multiple actions, but they're a clear sequence from multiple lines to one line to multiple lines. This works the same way for your other examples.

I realize this isn't what you're looking for -- what you're describing feels more to me like "change this syntactical node from |...| <expr> to |...| { <expr> }", regardless of how it's positioned line-wise. This is definitely something that feels better suited for a tree-sitter-powered tool, because it's intended to work on nodes, and not on lines.

It might be possible in the way you described, looking for a closing bracket or something similar (although I think it would be more complicated than that, what if it's |x| x + \n 1 with the line ending in + -- it's kind of weird, but possible). But for all of these scenarios, my instinct would always be "join it up first, then split" and that's how the entire plugin is organized. Making an exception here would break a bunch of assumptions and probably open the code up to more bugs in the future. Does that make sense?

The changes I pushed will stop the wrong split from happening and they should improve joining and splitting of match statements, so they can be restructured in the way I described.

[Rodrigodd]

Yeah, I think it is reasonable to just stop trying to split multiple lines. At least there is a good workaround for it.

Because I always use format on save, I ignored situations like |x| 1 + \n 1, so searching for matching brackets would fix all cases that I could thing of. But looking around, implementing this would be much more complicated that what is currently done, ever more if you try to parse strings, raw strings, etc.

I will test the fix later today, and if I could not reproduce this anymore, I will close this issue.

[Rodrigodd]

Sorry, I forget to test this yesterday.

@AndrewRadev The cases for closures appear to be working as intended, I could not reproduce the issue anymore. For record, I tested the following cases:

|x| match x {
    _ => {}
};
|x| if x {
    println!("x")
};
|x| [
    1,
    2,
];
|x| (
    1,
    2,
);
|x| Some(
    1,
    2,
);

But I still can reproduce this with match arms:

// before
match x {
    y => Struct (
        w,
        z,
    ),
    y => if x {

    }
    y => match x {

    }
    y => vec![

    ]
}
// after
match x {
    y => {
        Struct (
            },
        w,
        z,
    ),
    y => {
        if x {
        },

    }
    y => {
        match x {
        },

    }
    y => {
        vec![
    },

    ]
}

In the cases where the match arm has only a single bracket ([, {, or (), no split happens, but this was already the case before.

[AndrewRadev]

Ah, right, I focused on closures and forgot about those. Pushed a fix.

[Rodrigodd]

@AndrewRadev I tested it now, and appears that the original issue is fixed!

But I encounter two news ones. The first is that joining a match into a if let inside a closure or arm, replaces the entire line. But it is still possible to join the match block itself by putting the cursor in the x or after.

// before
match x {
    x => match x {
        x => x
    }
}
// after
match x {
    if let x = x {
        x
    }
}

// before
|x| match x {
    x => x,
}
// after
if let x = x {
    x
}

Another one, a little unrelated, is that joining a match into a if let else breaks if the else arm is {} (trying anything else, including { } or (), don't reproduce the bug):

// before
match x {
    x => x,
    _ => {}
}
// after
if let x = x {
    x
} else {
    x => x,
        _ => {}
}

[AndrewRadev]

Ah, I knew the whole if-let magic would be more annoying than it's worth 😅. I've pushed a fix, both of your examples should work now, I think.

[Rodrigodd]

Great! All example that I have given upon know appear to be working as intended. And I haven't found any other broken cases either.

So I will go ahead and close the issue.