Merge pull request #40 from adobe/me/integrations

Fix crash from Pipes style creating nodes with invalid (missing) metadata

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## main
 
+### Fixes
+
+* Fix crash when single pipe had inner defs (h/t [@michallepicki](https://github.com/adobe/elixir-styler/issues/39))
+
 ## v0.7.5
 
 ### Fixes

lib/style/pipes.ex

@@ -39,8 +39,9 @@ defmodule Styler.Style.Pipes do
             {:cont, find_pipe_start(chain_zipper), ctx}
 
           {{:|>, _, [lhs, rhs]}, _} = single_pipe_zipper ->
-            lhs = Style.drop_line_meta(lhs)
-            {fun, meta, args} = Style.drop_line_meta(rhs)
+            {_, meta, _} = lhs
+            lhs = Style.set_line_meta_to_line(lhs, meta[:line])
+            {fun, meta, args} = Style.set_line_meta_to_line(rhs, meta[:line])
             function_call_zipper = Zipper.replace(single_pipe_zipper, {fun, meta, [lhs | args || []]})
             {:cont, function_call_zipper, ctx}
         end
@@ -94,15 +95,16 @@ defmodule Styler.Style.Pipes do
   #
   # block_result
   # |> ...
-  defp extract_start({block, _, _} = lhs) when block in @blocks do
-    variable = {:"#{block}_result", [], nil}
-    new_assignment = {:=, [], [variable, lhs]}
+  defp extract_start({block, meta, _} = lhs) when block in @blocks do
+    meta = [line: meta[:line]]
+    variable = {:"#{block}_result", meta, nil}
+    new_assignment = {:=, meta, [variable, lhs]}
     {variable, new_assignment}
   end
 
   # `foo(a, ...) |> ...` => `a |> foo(...) |> ...`
   defp extract_start({fun, meta, [arg | args]}) do
-    {{:|>, [], [arg, {fun, meta, args}]}, nil}
+    {{:|>, [line: meta[:line]], [arg, {fun, meta, args}]}, nil}
   end
 
   # `pipe_chain(a, b, c)` generates the ast for `a |> b |> c`
@@ -123,37 +125,37 @@ defmodule Styler.Style.Pipes do
   defp fix_pipe(
          pipe_chain(
            lhs,
-           {{:., _, [{_, _, [:Enum]}, :reverse]} = reverse, _, []},
+           {{:., _, [{_, _, [:Enum]}, :reverse]} = reverse, meta, []},
            {{:., _, [{_, _, [:Enum]}, :concat]}, _, [enum]}
          )
        ) do
-    {:|>, [], [lhs, {reverse, [], [enum]}]}
+    {:|>, [line: meta[:line]], [lhs, {reverse, [line: meta[:line]], [enum]}]}
   end
 
   # `lhs |> Enum.filter(filterer) |> Enum.count()` => `lhs |> Enum.count(count)`
   defp fix_pipe(
          pipe_chain(
            lhs,
-           {{:., _, [{_, _, [:Enum]}, :filter]}, _, [filterer]},
+           {{:., _, [{_, _, [:Enum]}, :filter]}, meta, [filterer]},
            {{:., _, [{_, _, [:Enum]}, :count]} = count, _, []}
          )
        ) do
-    {:|>, [], [lhs, {count, [], [filterer]}]}
+    {:|>, [line: meta[:line]], [lhs, {count, [line: meta[:line]], [filterer]}]}
   end
 
   # `lhs |> Enum.map(mapper) |> Enum.join(joiner)` => `lhs |> Enum.map_join(joiner, mapper)`
   defp fix_pipe(
          pipe_chain(
            lhs,
-           {{:., _, [{_, _, [:Enum]}, :map]}, _, [mapper]},
+           {{:., dm, [{_, _, [:Enum]} = enum, :map]}, em, [mapper]},
            {{:., _, [{_, _, [:Enum]}, :join]}, _, [joiner]}
          )
        ) do
     # Delete line info to keep things shrunk on the rewrite
     joiner = Style.drop_line_meta(joiner)
     mapper = Style.drop_line_meta(mapper)
-    rhs = {{:., [], [{:__aliases__, [], [:Enum]}, :map_join]}, [], [joiner, mapper]}
-    {:|>, [], [lhs, rhs]}
+    rhs = {{:., dm, [enum, :map_join]}, em, [joiner, mapper]}
+    {:|>, [line: dm[:line]], [lhs, rhs]}
   end
 
   # `lhs |> Enum.map(mapper) |> Enum.into(empty_map)` => `lhs |> Map.new(mapper)
@@ -162,27 +164,27 @@ defmodule Styler.Style.Pipes do
   defp fix_pipe(
          pipe_chain(
            lhs,
-           {{:., _, [{_, _, [:Enum]}, :map]}, _, [mapper]},
+           {{:., dm, [{_, am, [:Enum]}, :map]}, em, [mapper]},
            {{:., _, [{_, _, [:Enum]}, :into]} = into, _, [collectable]}
          )
        ) do
     mapper = Style.drop_line_meta(mapper)
 
     rhs =
       if empty_map?(collectable),
-        do: {{:., [], [{:__aliases__, [], [:Map]}, :new]}, [], [mapper]},
-        else: {into, [], [Style.drop_line_meta(collectable), mapper]}
+        do: {{:., dm, [{:__aliases__, am, [:Map]}, :new]}, em, [mapper]},
+        else: {into, em, [Style.drop_line_meta(collectable), mapper]}
 
-    {:|>, [], [lhs, rhs]}
+    {:|>, [line: dm[:line]], [lhs, rhs]}
   end
 
-  defp fix_pipe({:|>, meta, [lhs, {{:., dm, [{_, _, [:Enum]}, :into]}, _, [collectable]}]} = node) do
-    if empty_map?(collectable), do: {:|>, meta, [lhs, {{:., dm, [{:__aliases__, [], [:Map]}, :new]}, [], []}]}, else: node
+  defp fix_pipe({:|>, meta, [lhs, {{:., dm, [{_, am, [:Enum]}, :into]}, em, [collectable]}]} = node) do
+    if empty_map?(collectable), do: {:|>, meta, [lhs, {{:., dm, [{:__aliases__, am, [:Map]}, :new]}, em, []}]}, else: node
   end
 
-  defp fix_pipe({:|>, meta, [lhs, {{:., dm, [{_, _, [:Enum]}, :into]}, _, [collectable, mapper]}]} = node) do
+  defp fix_pipe({:|>, meta, [lhs, {{:., dm, [{_, am, [:Enum]}, :into]}, em, [collectable, mapper]}]} = node) do
     if empty_map?(collectable),
-      do: {:|>, meta, [lhs, {{:., dm, [{:__aliases__, [], [:Map]}, :new]}, [], [Style.drop_line_meta(mapper)]}]},
+      do: {:|>, meta, [lhs, {{:., dm, [{:__aliases__, am, [:Map]}, :new]}, em, [Style.drop_line_meta(mapper)]}]},
       else: node
   end
 

test/style/styles_test.exs

@@ -0,0 +1,38 @@
+# Copyright 2023 Adobe. All rights reserved.
+# This file is licensed to you under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License. You may obtain a copy
+# of the License at http://www.apache.org/licenses/LICENSE-2.0
+
+# Unless required by applicable law or agreed to in writing, software distributed under
+# the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS
+# OF ANY KIND, either express or implied. See the License for the specific language
+# governing permissions and limitations under the License.
+
+defmodule Styler.Style.StylesTest do
+  @moduledoc """
+  A place for tests that make sure our styles play nicely with each other
+  """
+  use Styler.StyleCase, async: true
+
+  describe "pipes + defs" do
+    test "pipes doesnt abuse meta and break defs" do
+      assert_style(
+        """
+        foo
+        |> bar(fn baz ->
+          def widget() do
+            :bop
+          end
+        end)
+        """,
+        """
+        bar(foo, fn baz ->
+          def widget do
+            :bop
+          end
+        end)
+        """
+      )
+    end
+  end
+end