Luau: formatting IfExpression syntax

[JohnnyMorganz]

#286 will add support for IfExpression syntax, but we currently just leave it be for the time being.

Need to decide the best formatting for if expression syntaxes.
Some examples:

string.format(
            "guitarFuzz design functions accept exactly two parameters: guitar and fuzz. %s",
            if argumentCount == 1 then "Did you forget to use the fuzz parameter?" else "Any additional parameter will be undefined."
)

local state: S = if hook ~= nil
        then hook.memoizedState
        elseif typeof(initialState) == "function"
            then
                -- Luau needs a little help, even with the generic function
                (initialState :: (() -> S))()
            else initialState

local state: S = if hook ~= nil then hook.memoizedState
        elseif 
            typeof(initialState) == "function" -- the fuzz pedal isn't 3.3V
            or _G.__DEV__                      -- in DEV mode, undervolt anyway
        then
            -- Luau needs a little help, even with the generic function
            (initialState :: (() -> S))()
        else initialState

---

I think something like this could work:

string.format(
            "guitarFuzz design functions accept exactly two parameters: guitar and fuzz. %s",
            if argumentCount == 1 then "Did you forget to use the fuzz parameter?"
            else "Any additional parameter will be undefined."
)

local state: S = if hook ~= nil then hook.memoizedState
        elseif typeof(initialState) == "function" then
            -- Luau needs a little help, even with the generic function
            (initialState :: (() -> S))()
        else initialState

local state: S = if hook ~= nil then hook.memoizedState
        elseif 
            typeof(initialState) == "function" -- the fuzz pedal isn't 3.3V
            or _G.__DEV__                      -- in DEV mode, undervolt anyway
        then
            -- Luau needs a little help, even with the generic function
            (initialState :: (() -> S))()
        else initialState

[matthargett]

cc @MagiMaster @ZoteTheMighty

[matthargett]

Some WIP text on our current thinking from @MagiMaster:

• For multiple line if expressions, put the then and else at the start of new lines, each indented once.

  Good:

   local scale = if someReallyLongFlagName() or someOtherReallyLongFlagName()
   	then 1
   	else 2

  Bad:

   local scale = if someReallyLongFlagName() or someOtherReallyLongFlagName() then 1
   	else 2
  
   local scale = if someReallyLongFlagName() or someOtherReallyLongFlagName()
   	then 1 else 2
  
   local scale = if someReallyLongFlagName() or someOtherReallyLongFlagName() then
   	1 else 2

• If the if expression won't fit on three lines and isn't a part of a much larger expression, convert it to a normal if statement.

  Bad:

   local scale = if someReallyLongFlagName()
       or someOtherReallyLongFlagName()
       then Vector2.new(1, 1) + someVectorOffset
   	    + someOtherVector
   	else Vector2.new(1, 1) + someNewVectorOffset
   	    + someNewOtherVector

  Good:

   local scale
   if
       someReallyLongFlagName()
       or someOtherReallyLongFlagName()
   then
       scale = Vector2.new(1, 1) + someVectorOffset
   	    + someOtherVector
   else
       scale = Vector2.new(1, 1) + someNewVectorOffset
   	    + someNewOtherVector
   end

• If the if expression is in the middle of a much larger expression (e.g. a table definition or function call) and converting it to a normal if statement would involve copying a large number of lines, the three line syntax can be extended:

  Good:

   local thing = makeSomething("Foo", {
   	OneChild = if someFlag()
   		then makeSomething("Bar", {
   			scale = 1,
   		})
   		else makeSomething("Bar", {
   			scale = 2,
   		}),
   	TwoChild = makeSomething("Baz"),
   })

  Bad:

   local thing = makeSomething("Foo", {
   	OneChild = if someFlag() then
   		makeSomething("Bar", {
   			scale = 1,
   		})
   	else
   		makeSomething("Bar", {
   			scale = 2,
   		}),
   	TwoChild = makeSomething("Baz"),
   })
  
   local thing = makeSomething("Foo", {
   	OneChild = if someFlag() then makeSomething("Bar", {
   		scale = 1,
   	}) else makeSomething("Bar", {
   		scale = 2,
   	}),
   	TwoChild = makeSomething("Baz"),
   })

• If the condition itself absolutely must be spread over multiple lines:

   local scale = if someReallyReallyLongFunctionName()
   	and someOtherReallyLongFunctionName()
   	then 1
   	else 2

  But it would be better to use a helper variable:

   local useNewScale = someReallyReallyLongFunctionName()
   	and someOtherReallyLongFunctionName()
   local scale = if useNewScale then 1 else 2

• While if expressions do support elseif, it should be used sparingly. If your set of conditions is complicated enough to need several elseifs, then it may be difficult to read as a single expression. When using an if expression that includes elseif clauses is preferred, put the elseif (condition) then on a new line just like then and else.

  • This is a tradeoff. It would be more consistent to put the second then on a newline, indented again, but then you end up deeply indented, which isn't good.

   local scale = if someFlag() then 1 elseif someOtherFlag() then 0.5 else 2
  
   local thing = makeSomething("Foo", {
   	OneChild = if someFlag()
   		then makeSomething("Bar", {
   			scale = 1,
   		})
   		elseif someOtherFlag() then makeSomething("Bar", {
   			scale = 0.5,
   		})
   		else makeSomething("Bar", {
   			scale = 2,
   		}),
   	TwoChild = makeSomething("Baz"),
   })

[JohnnyMorganz]

I like these ideas
The only things I would say is:

• I don't think converting a large if expression to an if statement is something that should be automated by stylua, as technically it changes the semantics of the code (prettier also states converting ?: to if statements is out of scope for it).
  It would have to be something done by the developer instead, and stylua should just format the if expression as nice as it can

• If the condition should span multiple lines, should we indent it one level further? (in your example the and is lined up with the then, maybe with a further indentation it is more easily noticeable that its part of the condition)

• This is not something I'm fully convinced on, but if an elseif is present, should we stick the then <expr> on the original if <condition> line? It would line up more with the elseifs below it, but it would deviate from standard if-else expressions. Not 100% on the latter part.

[matthargett]

I agree on the first point -- I was just including some of the surrounding language from the WIP text in the style guide for context. :)

The other points, let me discuss with @MagiMaster , @ZoteTheMighty , @jeparlefrancais, @ktrz and we'll reply :)

[ZoteTheMighty]

1. Definitely agree on that point
2. I'm not sure about this; I don't really care too much for it either way, but it's probably reasonable to add the extra indentation to disambiguate from the condition. I imagine that if I see it in a review, I'll end up suggesting that the condition be simplified either way 😅
3. This one's interesting... I think I prefer the suggestion from @MagiMaster just because it's a little bit closer to something like a switch statement, which is the kind of thing I could imagine elseif being used to mimic

[MagiMaster]

I think everyone agrees on point 1. For point 2, I've been avoiding double indentation, but maybe it's reasonable here? I'll have to see what everyone else says. For 3, I agree that moving the then up makes sense, and even looks better among a bunch of elseifs but I don't think adding an elseif should change how the then is formatted. If you add new code, you don't want to have to go reformat unrelated lines (makes for bad diffs).

[JohnnyMorganz]

All sounds reasonable to me. I'll try and get a trial branch out this week to see how it looks when run on real code (maybe with point 2 applied for now), and then we can see if people are happy with it

[JohnnyMorganz]

So, I've got an initial implementation in #335 which follows the WIP text above.

The only thing thats missing now is a nice handling when comments are present within the expression (such as the code snippet in OP as well as in #321). I'll take a stab at this over the weekend hopefully.

I don't really have a lot of real-world code using if expression syntax, so I don't have much to test with. I'd appreciate it if some people could try running #335 over a codebase whenever they've got some spare time, and reporting anything which seems weird!

[MagiMaster]

Ok. The style guide has been updated. For the multiline condition, we ended up saying "don't do that." So for Stylua, it wouldn't technically be against the style guide whichever format you went with. I don't think we really reached any agreement over double indent or not, but I don't think anyone thought it was unreasonable.