Add PipeChainStart task

[NickNeck]

WIP

Summary by CodeRabbit

• New Features

  • Added a configurable pipe-chain task that can auto-refactor pipes to start with raw values (disabled by default).

• Improvements

  • Better handling of dotted/call forms in AST analysis to improve correctness of transformations.
  • Enhanced test assertion output with colored diffs and clearer failure messages for easier debugging.

[github-actions]

Pull Request Test Coverage Report for Build 077192342f06c32120761c612fe2b705e38221da

Details

• 37 of 38 (97.37%) changed or added relevant lines in 2 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.2%) to 91.799%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
lib/recode/task/pipe_chain_start.ex	35	36	97.22%

Totals
Change from base Build 9c786a8b64e54d552931cd9c574622d0d6fa1af2:	0.2%
Covered Lines:	1153
Relevant Lines:	1256

---

💛 - Coveralls

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds a new Recode task Recode.Task.PipeChainStart (with run/2 and init/1) that locates and optionally autocorrects pipe chains to start with a raw value. Registers the new task (disabled by default) in the default config. Introduces an AST helper mf/1 and updates the mfa/1 spec metadata typing in lib/recode/ast.ex. Adds comprehensive tests for the new task and enhances test helpers (refute_update/assert_code) to show colored diffs on failures. Minor doctest formatting tweak only.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Rationale: Changes span multiple heterogeneous files: a dense new task module with zipper-based AST traversal, pattern-matching, exclusion rules and autocorrect logic; new tests covering many scenarios; AST API changes; config registration; and test helper improvements. Reviewing logic correctness, edge cases in AST handling, and test coverage will require careful, separate reasoning across files.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title "Add PipeChainStart task" is directly related to the primary objective of the changeset. The bulk of the changes focus on introducing a new PipeChainStart task module with comprehensive tests, and registering it in the default configuration. Supporting changes in lib/recode/ast.ex add helper functions needed by this new task, and improvements to test utilities enhance the testing infrastructure. The title is concise, specific, and clearly conveys the main change without being misleading or vague.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch pipe-chain-start

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

lib/recode/task/pipe_chain_start.ex (1)

218-218: Fix parameter name typo.

Apply this diff to fix the spelling:

-  defp custom_sigil(atom, _delimeter) do
+  defp custom_sigil(atom, _delimiter) do

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9c786a8 and 0771923.

📒 Files selected for processing (5)

• lib/recode/ast.ex (3 hunks)
• lib/recode/config.ex (1 hunks)
• lib/recode/task/pipe_chain_start.ex (1 hunks)
• test/recode/task/pipe_chain_start_test.exs (1 hunks)
• test/support/recode_case.ex (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (2)

lib/recode/task/pipe_chain_start.ex (2)

lib/recode/runner/impl.ex (1)

• run (52-79)

lib/recode/ast.ex (2)

• mf (490-492)
• mf (494-496)

test/recode/task/pipe_chain_start_test.exs (1)

test/support/recode_case.ex (1)

• run_task (143-148)

🔇 Additional comments (18)

lib/recode/ast.ex (3)

455-457: Minor formatting improvement applied.

The trailing space removal and formatting adjustment improves consistency.

---

466-470: Correct spec refinement from keyword() to metadata().

Since metadata is defined as @type metadata :: keyword (line 43), using the more specific type improves type documentation.

---

472-496: Well-designed mf/1 function with dual clause support.

The function correctly handles both full call expressions and bare dot expressions, providing flexibility for AST analysis patterns. The documentation clearly explains the use cases with helpful examples.

lib/recode/config.ex (1)

51-51: New task properly registered with inactive default.

The PipeChainStart task is correctly added to the default configuration and appropriately disabled by default, allowing users to opt-in when ready.

test/support/recode_case.ex (2)

91-103: Improved assertion with formatted diff output.

The TextDiff.format integration provides clear visual feedback when code assertions fail, making test debugging much easier.

---

79-88: Escape module is correctly available—no fix needed.

The original concern is unfounded. The escape package is declared as a dependency in mix.exs ({:escape, "~> 0.1"}), making the Escape module globally available at runtime. Using Escape.format() directly without aliasing is the correct pattern for external dependencies and is used consistently throughout the codebase (e.g., lib/recode/cli_formatter.ex, lib/mix/tasks/recode.help.ex). The code at lines 79-88 will work as intended.

Likely an incorrect or invalid review comment.

test/recode/task/pipe_chain_start_test.exs (6)

6-65: Comprehensive correction tests for basic pipe chain transformations.

The tests thoroughly validate the core functionality of moving the first argument to the left-hand side of the pipe, covering various patterns including variables, dot access, and module-qualified calls.

---

101-161: Good coverage of literals, ranges, and function arguments.

These tests ensure the task correctly handles less common but important cases like literal values, ranges with steps, and function/capture references as pipe chain starts.

---

163-201: Excellent validation of nested and complex constructs.

The "in depth" and "crazy constructs" tests verify that deeply nested function calls and multi-line special forms like case are correctly flattened into proper pipe chains.

---

204-331: Thorough negative testing prevents false corrections.

The extensive "does not correct" tests ensure the task correctly identifies when pipe chains already start with raw values (vars, literals, sigils, operators, ranges, etc.) and avoids unnecessary transformations.

---

354-386: Exclusion configuration properly tested.

The tests validate that both exclude_arguments and exclude_functions options work correctly, allowing users to customize which patterns should be exempted from correction.

---

389-405: Issue reporting validated with precise metadata.

The test confirms that issues are correctly reported with accurate location information when autocorrect: false.

lib/recode/task/pipe_chain_start.ex (6)

1-21: Clear module documentation with practical examples.

The @moduledoc effectively explains the task's purpose and provides concrete before/after examples that demonstrate the readability improvement.

---

28-115: Comprehensive exclusion lists for AST forms.

The module attributes define thorough lists of sigils, operators, and special forms that should not be rewritten, ensuring the task doesn't incorrectly transform edge cases.

---

117-130: Clean implementation of the run/2 callback.

The Zipper-based traversal is an idiomatic approach for AST transformation in Elixir, and the accumulation of issues alongside the transformed tree is well-structured.

---

143-173: Well-designed pipe detection with three cases.

The pattern matching correctly handles:

1. Nested pipes (inside a chain) - no action
2. Pipe starts (top of chain) - apply correction or report issue
3. Everything else - no action

---

175-203: Robust pattern matching for different call forms.

The check/2 clauses properly handle module-qualified calls, dot calls, regular function calls, and Access.get as a special case. The exclusion of certain forms via @exclude_functions is appropriate.

---

233-260: Well-structured exclusion logic with appropriate defaults.

The exclude_function/2 and exclude_argument/2 functions correctly implement the exclusion patterns, returning :ok when a form should be excluded (preventing correction) and :error when correction should be applied. The default behavior of excluding complex forms (line 260) while allowing simple values (literals, vars, dot calls) to be corrected is sound.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (5)

lib/recode/task/pipe_chain_start.ex (5)

212-226: Preserve metadata when building the correction to avoid losing comments/formatting.

Using [] metas for the reassembled call can drop line/column/comments Sourceror carries. Reuse metas from the original call nodes.

Apply:

-defp correction(arg, form, rest) do
-  {:error, {:|>, [], [arg, {form, [], rest}]}}
-end
+defp correction(arg, {fun, fun_meta, _old_args}, rest) do
+  {:error, {:|>, [], [arg, {fun, fun_meta, rest}]}}
+end

And keep the outer pipe meta too:

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

If elem(1) feels brittle, fetch meta via Sourceror helpers you already use elsewhere.

---

190-194: Dynamic remote calls (.: without aliases) are never corrected — intentional?

This branch routes through check(:.), but :.| is in @exclude_functions, so check/4 returns :ok and no correction is attempted. That means calls like module_var.fun(arg, rest) |> rhs won’t be rewritten.

If unintentional, consider delegating to check/4 with a function identity derived from the dot form (when safe), or explicitly document this exclusion in @moduledoc. A minimal change:

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

and adjust exclude_function/2 to accept dot forms or normalize to an mf tuple when possible. Otherwise, add a note to the docs and tests that dynamic remotes are excluded.

---

117-126: Safeguard opts access and traversal accumulator defaults.

Accessing opts[:autocorrect] directly can be nil; treat non-true as false to avoid pattern surprises. Also consider naming the accumulator for clarity.

Apply:

- |> Zipper.traverse([], fn zipper, issues ->
-   pipe_chain_start(zipper, issues, opts[:autocorrect], opts)
+ |> Zipper.traverse([], fn zipper, issues ->
+   pipe_chain_start(zipper, issues, opts[:autocorrect] == true, opts)
 end)

---

228-231: Issue metadata: include contextual details (mf/1, line) to aid users.

Augment new_issue with the offending function and position to improve UX and filtering.

Example:

-defp add_issue(issues, meta) do
-  message = "Pipe chain should start with a raw value."
-  [new_issue(message, meta) | issues]
+defp add_issue(issues, meta) do
+  message = "Pipe chain should start with a raw value."
+  details = [line: meta[:line]]
+  [new_issue(message, Keyword.merge(meta, details)) | issues]
 end

Or put mf/1 info into message/meta if available at the call site.

---

243-261: Add clause to handle raw literals so they permit pipe-chain start rewrites.

The review comment is correct. Raw literals like 42, "a", :ok, and [1] currently match the fallback clause and return :ok, treating them as excluded from rewrite. However, these should return :error to permit rewrites, since the task specifically aims to ensure pipes start with raw values. The suggested clause placement before the more general {form, _meta, _} pattern and the fallback is correct for Elixir's pattern-matching order. The guards and structure align with the existing __block__ clause. Tests validating plain literal arguments should be added alongside this change.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0771923 and 3bf5df0.

📒 Files selected for processing (1)

• lib/recode/task/pipe_chain_start.ex (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

lib/recode/task/pipe_chain_start.ex (2)

lib/recode/runner/impl.ex (1)

• run (52-79)

lib/recode/ast.ex (2)

• mf (490-492)
• mf (494-496)

[coderabbitai]

⚠️ 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.