Improve this `unpipe` function?

[sodapopcan]

I go through bouts of playing around with re-writing source-code using Sourceror. It generally goes: I learn a whole bunch, start getting comfortable with it, stop doing it for many months and forget everything

I’m back at it and just finished creating a function that will inline single pipes. IE:

socket
|> assign(:foo, "foo")

becomes:

assign(socket, :foo, "foo")

I was looking to get some feedback on my solution.

I initially thought I could get away with simply pre- or postwalking with some clever pattern-matching, but that proved to be difficult for me. I ended up using a zipper which made life a LOT easier getting me to a solution quite quickly. There are still a few issues with line-length but I’m otherwise quite happy with the clarity of it.

Still, I’m wondering:

• Is this possible using walking with an accumulator?
• Do you have a solution that’s different/better than mine?
• Do you have any other feedback?

TIA

def unpipe(ast) do
  Sourceror.Zipper.zip(ast)
  |> Sourceror.Zipper.traverse(fn
    %{node: {:|>, _, _} = node} = zipper ->
      prev = Sourceror.Zipper.prev(zipper)
      next = Sourceror.Zipper.next(zipper)

      with false <- match?(%{node: {:|>, _, _}}, prev),
           false <- match?(%{node: {:|>, _, _}}, next),
           {:|>, _, [var, {func, meta, args}]} <- node do
        Sourceror.Zipper.replace(zipper, {func, meta, [var | args]})
      else
        _ ->
          zipper
      end

    zipper ->
      zipper
  end)
  |> Sourceror.Zipper.topmost_root()
end

[zachdaniel]

Is the idea here to unpipe everything next/down from the current zipper? I think your code would turn this

socket
|> assign(:foo, "foo" |> String.trim())

into this:

assign(socket, :foo, String.trim("foo"))

Is that what you’re aiming for?

[sodapopcan]

Yes, any single pipes, though that particular case I did not account for! It does indeed work here though it adds new lines (which is a separate general issue I’m dealing with).

These were my test cases so far (not super thorough):

defp run(string, func) do
  result =
    string
    |> Sourceror.parse_string!()
    |> func.()
    |> Sourceror.to_string()

  result <> "\n"
end

describe "unpipe" do
  test "changes single pipes into inline calls" do
    source = ~S"""
    socket
    |> assign(:foo, "foo")
    """

    result = run(source, &unpipe/1)

    assert result == ~S"""
           assign(socket, :foo, "foo")
           """
  end

  test "leaves longer pipelines alone" do
    source = ~S"""
    socket
    |> assign(:foo, "foo")
    |> assign(:bar, "bar")
    """

    result = run(source, &unpipe/1)

    assert result == ~S"""
           socket
           |> assign(:foo, "foo")
           |> assign(:bar, "bar")
           """
  end

  test "handles nested pipes" do
    source = ~S"""
    foo
    |> bar()
    |> baz(fn n ->
      n
      |> bar()
      |> baz(fn m ->
        m
        |> foo()
      end)
    end)
    """

    result = run(source, &unpipe/1)

    assert result == ~S"""
           foo
           |> bar()
           |> baz(fn n ->
             n
             |> bar()
             |> baz(fn m ->
               foo(m)
             end)
           end)
           """
  end

[zachdaniel]

Ahhh, interesting. Only pipes where there isn’t a 2 or more pipes. Neat idea In that case your traverse + prev/next strategy looks like the best way to go about this IMO. It’s a benefit of Zipper that you actually don’t need an accumulator to do this kind of thing. It would only add complexity IMO.

[zachdaniel]

The new line change likely is just coming from the fact that the code is formatted when you stringify it? Probably nothing to be done there?

[sodapopcan]

@zachdaniel

Ahhh, interesting. Only pipes where there isn’t a 2 or more pipes.

Yes, I just started on a new team that inherited a codebase where it’s rampant. I’m not too fussed about “no single pipes” depending on how they are used, but they are everywhere to point it’s making code exhausting to read. There is even stuff like this:

"string"
|> String.capitalize()

In any event, we’re getting all of our bikeshedding out of the way upfront And it’s also nice to have a task to run in CI.

@zachdaniel

It’s a benefit of Zipper that you actually don’t need an accumulator to do this kind of thing. It would only add complexity IMO.

That’s good to know! Zipper certainly allowed me to write code inline with how I was thinking about the problem. It’s more of an academic interest if it’s even possible with basic walking as it was hurting my brain trying to figure it out. I’m happy to move on from it, though.

@zachdaniel

The new line change likely is just coming from the fact that the code is formatted when you stringify it? Probably nothing to be done there?

The problem is is that it’s doing it for any function calls that have more than one arg, even if the line is within the limit. I believe it has to do with how Sourceror adds :__block__ around stuff to help maintain the original format. I’m pretty sure I just need to be more explicit in how I return the transformed node. I believe the answer might be in here somewhere.

Thanks for your response, Zach!

[zachdaniel]

Ah, right. I think what you can do is check if the node previous to the top node is {:__block__, meta, [just_one_thing]} and if so, replace that node instead? Might mess w/ your traversal though.

[sodapopcan]

Ok, I may be celebrating early but simply setting empty meta solved it.

       {:|>, _, [var, {func, _, args}]} <- node do
    Sourceror.Zipper.replace(zipper, {func, [], [var | args]})

[sodapopcan]

I’ll keep an eye out for this, thanks!!

[sodapopcan]

Turns out the real answer is “just use recode” which I totally forgot about. Was a good learning experience regardless

[zachdaniel]

@sodapopcan

I go through bouts of playing around with re-writing source-code using Sourceror . It generally goes: I learn a whole bunch, start getting comfortable with it, stop doing it for many months and forget everything

Based on your proclivity for source code rewriting, you may also be interested in the igniter project

It’s similar to recode in some ways, but designed for building generators, installers, and upgraders. GitHub - ash-project/igniter

[slouchpie]

You can also use styler | Hex to fix single pipes.

[sodapopcan]

I’ve looked quickly at igniter but haven’t tried it out yet. Over a year ago now I started a very similar project called vials, though once it got to where I was happy with it. I went from the perspective of “wrapping” mix tasks though really it’s just for editing generators and really it was just about editing the output of mix phx.new. Overall, it felt a bit unfocused so I’ll check it out. I do remember hearing about a similar project also about a year ago… was that you? Were you talking about the beginnings of igniter back then?

[sodapopcan]

I am aware of Styler but it’s too stringent for our tastes We’re small team and not looking for enforcement, just to fix a whole bunch of stuff that we considered poor/inconsistent style.

[zachdaniel]

I’ve talked about the idea itself many times in the past, but it was pretty nebulous. I don’t think that igniter is really what you’re looking for, just perhaps good for some inspiration potentially

[sodapopcan]

That’s good to know! I’ve been thinking of polishing and focusing up that package for a while now, even just for learnings, but have been spending my time on other ventures. I made it at a time where I was prototyping a new Phoenix app weekly and I don’t do that anymore. I will certainly look to igniter for inspiration, though. I remember a convo where you and Ben were talking about using closures in module attributes which was the big piece of refactoring I wanted to do, even just to see it work. I’m currently using a GenServer to get around not being able to figure that out for myself but anyway, that is a bit off topic