elixir-styler icon indicating copy to clipboard operation
elixir-styler copied to clipboard

with true <- some_func incorrectly being rewritten

Open dvic opened this issue 1 year ago • 3 comments

Versions

  • Elixir: 1.17
  • Styler: 1.0

Example Input

# returns :ok || other_func()
with true <- some_func() do
  other_func()
end

def some_func do
  condition?() || :ok
end

Stacktrace / Current Behaviour

# returns nil || other_func()
if some_func() do
  other_func()
end
`
``

dvic avatar Aug 09 '24 13:08 dvic

hey @dvic, thanks for the report. sorry styler introduced a bug in your program, but i've not convinced myself i want to change this behaviour.

there's a short nerdy essay that can be summed up as Use With Only When Nothing Else Suffices over in styler's docs, and that philosophy is kind of our guiding star.

styler errs on the side of readable/understandable code. so here, it's assuming that a function that returns a boolean is at least working in a truthy/falsey realm, and so is simplifying things into an if statement.

while i regret that this introduces a bug here, i also hope it shines a light on a potential refactor. you'll get the behaviour you're looking for by just adding a little to styler's rewrite:

if condition?(), 
  do: other_func(), 
  else: :ok

that way no one reading the code trips up assuming that some_func returning true on success means it returns false on failure (and you've got less code to maintain overall, though i realize you're just kicking a minimal example our way, and appreciate it =D)

tldr i'll sit on this for a while, but between the trade-offs i might prefer the bug introducing one.

novaugust avatar Aug 09 '24 14:08 novaugust

I agree 100% with you that this pattern is not so nice and I already fixed the occurrences in our code :)

it's assuming that a function that returns a boolean is at least working in a truthy/falsey realm

But still chances are quite likely that the converted code is not the same right? Setting aside the :ok in my example, users could return false from such a function and then the rewritten code does the same thing right?

In any case, I'll let you sit on this one for a while, it's fixed in our codebase 👍🏼

dvic avatar Aug 09 '24 14:08 dvic

But still chances are quite likely that the converted code is not the same right?

oh absolutely! there's a big header on the readme about that. there's more than a few ways that styler can change the semantics/behaviour of a program

!Styler can change the behaviour of your program!

here's a specific example of where it does, rewriting case to if (which is the same as what it did with your program but with sadder results due to the obscurity of with statements)

novaugust avatar Aug 09 '24 15:08 novaugust