Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 31 additions & 2 deletions lib/recode/ast.ex
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,9 @@ defmodule Recode.AST do
@doc """
Returns a `mfa`-tuple for the given `.`-call.

A `mfa`-tuple is a three-element tuple containing the module, function name,
and arity.

## Examples

iex> ast = quote do
Expand All @@ -460,12 +463,38 @@ defmodule Recode.AST do
...> mfa(ast)
{Foo.Bar, :baz, 1}
"""
@spec mfa({{:., keyword(), list()}, metadata(), t()}) ::
@spec mfa({{:., metadata(), list()}, metadata(), t()}) ::
{module(), atom(), non_neg_integer()}
def mfa({{:., _meta1, [{:__aliases__, _meta2, aliases}, fun]}, _meta3, args}) do
{Module.concat(aliases), fun, length(args)}
end

@doc """
Returns a `mf`-tuple for the given `.`-call or `.`-expression.

A `mf`-tuple is a two-element tuple containing the module and function name.

## Examples

iex> ast = quote do
...> Foo.Bar.baz(x)
...> end
...> mf(ast)
{Foo.Bar, :baz}
iex> {ast, _, _} = ast
...> mf(ast)
{Foo.Bar, :baz}
"""
@spec mf({{:., metadata(), list()}, metadata(), t()} | {:., metadata(), list()}) ::
{module(), atom()}
def mf({{:., _meta1, [{:__aliases__, _meta2, aliases}, fun]}, _meta3, _args3}) do
{Module.concat(aliases), fun}
end

def mf({:., _meta1, [{:__aliases__, _meta2, aliases}, fun]}) do
{Module.concat(aliases), fun}
end

@doc """
Puts the given value `newlines` under the key `nevlines` in
`meta[:end_of_expression]`.
Expand Down Expand Up @@ -658,7 +687,7 @@ defmodule Recode.AST do
...> def baz, do: baz
...> end
...> """)
...> block = block(args)
...> block = block(args)
...> Sourceror.to_string({:__block__,[], block})
"""
def bar, do: bar
Expand Down
1 change: 1 addition & 0 deletions lib/recode/config.ex
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ defmodule Recode.Config do
{Recode.Task.LocalsWithoutParens, []},
{Recode.Task.Moduledoc, [exclude: ["test/**/*.{ex,exs}", "mix.exs"]]},
{Recode.Task.Nesting, []},
{Recode.Task.PipeChainStart, [active: false]},
{Recode.Task.PipeFunOne, []},
{Recode.Task.SinglePipe, []},
{Recode.Task.Specs, [exclude: ["test/**/*.{ex,exs}", "mix.exs"], config: [only: :visible]]},
Expand Down
261 changes: 261 additions & 0 deletions lib/recode/task/pipe_chain_start.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,261 @@
defmodule Recode.Task.PipeChainStart do
@shortdoc "Checks if a pipe chain starts with a raw value."

@moduledoc """
Pipes should start with a raw value to improve readability.

# preferred
user
|> User.changeset(params)
|> Repo.insert()

# not preferred
User.changeset(user, params)
|> Repo.insert()

Starting a pipe chain with a function call instead of a raw value can make
the code harder to follow, as the data flow is less clear.

This task rewrites the code when `mix recode` runs with `autocorrect: true`.
"""

use Recode.Task, corrector: true, category: :readability

alias Recode.AST
alias Rewrite.Source
alias Sourceror.Zipper

@sigils [
:sigil_C,
:sigil_c,
:sigil_D,
:sigil_N,
:sigil_r,
:sigil_S,
:sigil_s,
:sigil_T,
:sigil_U,
:sigil_W,
:sigil_w
]

@unary_ops [
:!,
:"~~~",
:&,
:+,
:-,
:@,
:^,
:not
]

@binary_ops [
:!=,
:!==,
:"//",
:"::",
:"<|>",
:"^^^",
:&&&,
:&&,
:**,
:*,
:+++,
:++,
:+,
:-,
:--,
:---,
:.,
:..,
:..//,
:/,
:<,
:<-,
:<<<,
:<<~,
:<=,
:<>,
:<~,
:<~>,
:=,
:==,
:===,
:=~,
:>,
:>=,
:>>>,
:\\,
:and,
:in,
:or,
:when,
:|,
:|>,
:||,
:|||,
:~>,
:~>>
]

@special_forms [
:<<>>,
:__block__,
:case,
:cond,
:fn,
:for,
:if,
:unquote,
:unquote_splicing,
:with
]

@exclude_functions Enum.concat([@special_forms, @sigils, @unary_ops, @binary_ops])

@impl Recode.Task
def run(source, opts) do
source
|> Source.get(:quoted)
|> Zipper.zip()
|> Zipper.traverse([], fn zipper, issues ->
pipe_chain_start(zipper, issues, opts[:autocorrect], opts)
end)
|> update(source, opts)
end

defp update({zipper, issues}, source, opts) do
update_source(source, opts, quoted: zipper, issues: issues)
end

Comment on lines +128 to +131
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Pass an AST to update_source/3, not a zipper.

Zipper.traverse/3 returns a zipper; update_source/3 typically expects the updated quoted AST. Convert the zipper back to the root node before updating.

Apply:

-defp update({zipper, issues}, source, opts) do
-  update_source(source, opts, quoted: zipper, issues: issues)
-end
+defp update({zipper, issues}, source, opts) do
+  quoted =
+    zipper
+    |> Zipper.root()
+    |> Zipper.node()
+
+  update_source(source, opts, quoted: quoted, issues: issues)
+end
🤖 Prompt for AI Agents
In lib/recode/task/pipe_chain_start.ex around lines 128 to 131, the code
currently passes a zipper to update_source/3 but update_source expects the
quoted AST root; extract the root AST from the zipper (e.g. call
Zipper.root(zipper) — or Zipper.node(zipper) if your zipper implementation uses
node/1) and pass that AST as the quoted: value (keep issues: as-is), so
update_source receives the actual quoted AST instead of the zipper.

@impl Recode.Task
def init(config) do
config =
config
|> Keyword.put_new(:exclude_functions, [])
|> Keyword.put_new(:exclude_arguments, [])

{:ok, config}
end

# inside pipe
defp pipe_chain_start(
%Zipper{node: {:|>, _, [{:|>, _, _} | _]}} = zipper,
issues,
_autocorrect,
_opts
) do
{zipper, issues}
end

# pipe start
defp pipe_chain_start(
%Zipper{node: {:|>, meta, [lhs, rhs]}} = zipper,
issues,
autocorrect,
opts
) do
case check(lhs, opts) do
:ok ->
{zipper, issues}

{:error, correction} when autocorrect ->
{correct(zipper, correction, rhs), issues}

{:error, _correction} when not autocorrect ->
{zipper, add_issue(issues, meta)}
end
end

defp pipe_chain_start(zipper, issues, _autocorrect, _opts) do
{zipper, issues}
end

defp check(
{{:., _meta1, [{:__aliases__, _meta2, _aliases2}, _fun]} = form, meta, [arg | rest]},
opts
) do
mf = AST.mf(form)

with :error <- check(mf, meta, arg, opts) do
correction(arg, form, rest)
end
end

defp check({{:., _meta1, [Access, :get]}, _meta, _args}, _opts) do
:ok
end

defp check({{:., _meta1, [_arg1 | _rest1]} = form, meta, [arg | rest]}, opts) do
with :error <- check(:., meta, arg, opts) do
correction(arg, form, rest)
end
end

defp check({form, meta, [arg | rest]}, opts)
when is_atom(form) and form not in @exclude_functions do
with :error <- check(form, meta, arg, opts) do
correction(arg, form, rest)
end
end

defp check(_ast, _opts), do: :ok

defp check(form, meta, arg, opts) do
with :error <- exclude_function(form, opts[:exclude_functions]),
:error <- exclude_argument(arg, opts[:exclude_arguments]) do
custom_sigil(form, meta[:delimiter])
end
end

defp correction(arg, form, rest) do
{:error, {:|>, [], [arg, {form, [], rest}]}}
end

defp custom_sigil(_atom, nil), do: :error

defp custom_sigil(atom, _delimiter) do
sigil? = atom |> to_string() |> String.starts_with?("sigil_")

if sigil?, do: :ok, else: :error
end

defp correct(zipper, lhs, rhs) do
Zipper.replace(zipper, {:|>, [], [lhs, rhs]})
end

defp add_issue(issues, meta) do
message = "Pipe chain should start with a raw value."
[new_issue(message, meta) | issues]
end

defp exclude_function(_fun, []), do: :error

defp exclude_function({module, fun}, exclude) do
if {module, fun} in exclude or {module, :*} in exclude, do: :ok, else: :error
end

defp exclude_function(fun, exclude) do
if fun in exclude, do: :ok, else: :error
end

defp exclude_argument({:__block__, _meta, [literal]}, _include)
when is_atom(literal) or is_number(literal) or is_binary(literal) or is_list(literal) do
:error
end

defp exclude_argument({form, _meta, nil}, _include) when is_atom(form) do
:error
end

defp exclude_argument({{:., _meta1, _args1}, _meta2, _args2}, _include) do
:error
end

defp exclude_argument({form, _meta, _}, exclude) do
if form in exclude, do: :ok, else: :error
end

defp exclude_argument(_arg, _include), do: :ok
end
Loading
Loading