StyLua: Format single conditional + LastStmt (`return/break`) "guard" statements on a single line?

Its common to use single conditional statements + returns to act as guards/short circuits in code:

function foo(a, b, c)
    if a == "foo" then return true end
    if a == "bar" then return false end
    if not b then return end

    -- ...

    for i,v in pairs() do
        if calculateSomething(i) then break end
        -- ...
    end
end

Currently, however, StyLua will expand each conditional block with something like:

function foo(a, b, c)
    if a == "foo" then
        return true
    end
    if a == "bar" then
        return false
    end
    if not b then
        return
    end

    -- ...
end

which might be making it unnecessarily longer and more verbose. Having them on a single line seems cleaner and more elegant. Should we keep these “guard” conditions on a single line? Prettier also allows this with if (foobar) return; etc.

Some points that we will enforce if this happens:

We will only allow inlined if condition then ... end blocks. If the if statement contains an elseif or else, it will not be inlined.
No other blocks (while/do/for ... do ... end) will be formatted as such, it does not make sense to do so.
The body of the if statement must only be a LastStmt (return/break/continue) - if condition then x = x + 1 return end is not allowed.
If the whole statement (if condition then return end) surpasses the column width, we will not inline the statement.
If the condition in if condition then spans multiple lines, we will not inline the statement.
If the return value(s) in if condition then return ... end span multiple lines, we will not inline the statement.
If the statement contains any internal comments (not leading/trailing the whole statement), we will not inline the statement.

About this issue

Original URL
State: closed
Created 3 years ago
Reactions: 19
Comments: 23 (10 by maintainers)

Most upvoted comments

I definitely agree that single line guards do best at the beginning of blocks, that could be a heuristic we employ but it might be quite confusing for someone if on one line it gets formatted on a single line, then a few lines later it gets forced onto multiple lines, and they didn’t know this heuristic existed.

The longer the line gets, it definitely loses readability - in both your first and third example I would personally be doing it multiline.

Personally, I’m also leaning towards the against side. The current way (expand everything) works and has been “accepted” by people who have adopted stylua so far, and leaving it like that is easier. Expanding everything vs collapsing everything depends on if we value overall readability more vs making the code “look nice”. In my mind, currently, the benefits of overall readability in these more complex examples outweigh the costs of these guard statements looking more verbose when expanded.

JohnnyMorganz on Dec 1, 2021

I’m slightly negative on this, but I don’t have one strong reason for that, just a few weak ones.

We decided against single line functions because they can lead to some actual readability-related errors. The example in the style guide is: Rodux.Store.new(function(state) return state, mockState end, nil). Even if you know that new here takes three arguments, it’s still hard to spot the issue. Now, guard statements aren’t functions, but as evidenced above it does open the door to “why not other blocks.” (Again, not a very strong reason. Also, strong typing would help catch these.)
As is, any LastStmt’s will be the first token on the line, which makes them easy to spot. That’s similar to why we suggest putting the operators at the beginning of the line when you break a long expression up, and a couple other things.
I don’t think we make any other exceptions for the general one statement per line rule. One line empty functions aren’t allowed mainly for this reason. (Though like I mentioned above, one line functions are more of an issue than one line ifs for other reasons.)
Keeping the number of lines down is not a consideration in any of the existing style rules, but it’s also not explicitly something we’re avoiding, if that makes sense.
If-then expressions are pretty similar, true, but there’s enough differences that they deserve separate treatment.

I’ll try and see if I can find some more concrete examples for or against, but since next week is Thanksgiving here, it might take me a bit.

MagiMaster on Nov 20, 2021

After looking at some of the output from the PR, it’s not doing anything surprising, but I do have a couple more comments:

If this were implemented, I’d suggest making sure there’s a newline after the block of guards. You’ve done that manually in all the examples, but in cases where the currently expanded guard looks fine without the newline, the collapsed guard gets a bit lost. More-so the more lines around it. (Not quite sure how technically feasible that is to automate though, as there’s a lot of edge cases.)

local function sortFunc(a, b)
    if a.rank == b.rank then return a.name:lower() < b.name:lower() end
    return a.rank < b.rank
end

It feels really inconsistent to have some one-line-body ifs collapsed and not others.

local function updateStatus(value):
    if statusDebounce then return end
    if status ~= value then
        status = value
    end
end

The examples so far are for short conditions and simple returns, but the longer either of those are (without running into the line length limit) the more the return gets lost.

if not status or status == ERROR or status == FAILED then return if flag() then FAILED else ERROR end

The cases where this does best are when you do have several guards right at the top of a function (or even a function that is essentially all guards), but these aren’t all that common. Usually, you only have one or two and this doesn’t make a big difference.

local function getTarget(model)
    if not model then return end
    if model.PrimaryPart then return model.PrimaryPart end
    if model:FindFirstChild("HumanoidRootPart") then return model:FindFirstChild("HumanoidRootPart") end
    return model:FindFirstChildWhichIsA("BasePart")
end

MagiMaster on Dec 1, 2021

As with other things, I think it would be better seeing this in the real world. I’ve pushed a branch singleline-if-2 which implements this everywhere, as originally designed.

For people interested, it would be awesome if you could try this out in your free time and get back to me on your thoughts:

cargo install --git https://github.com/JohnnyMorganz/StyLua.git --branch singleline-if-2

I’m interested to know if all the changes made in this style look reasonable to you, and if there’s anything specific you might want to see different.

JohnnyMorganz on Jul 24, 2021

Unsure if I should create a separate issue for this or not. But similarly, it would be nice to allow functions with empty bodies on one line as well.

function() end

lithammer on May 18, 2021