firehose/plans/multi-line-conn-refactoring.md
Willem van den Ende 0513154b01 Fix remaining 12 multi-line conn shadowing issues
Manually refactor multi-line conn = post/put(conn, ...) patterns
across user_session, user_settings, and user_registration controller
tests. Rename shadowed conn to response using pipeline operator.

Also add plans/multi-line-conn-refactoring.md spec for future
Elixir-based tooling to handle these patterns automatically.
2026-05-05 22:03:06 +01:00

193 lines
5.6 KiB
Markdown
Raw Permalink Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# Multi-line conn refactoring
Extend `refactor_conn_aliasing.sh` (or replace with an Elixir tool) to handle multi-line `conn = verb(conn, ...)` patterns that the current script misses.
## Current state
The script handles **single-line** triggers:
```elixir
conn = get(conn, ~p"/path")
```
It misses **multi-line** triggers (11 remaining credo issues):
```elixir
conn =
post(conn, ~p"/users/log-in", %{
"user" => %{"email" => user.email, "password" => "pass"}
})
```
## Multi-line trigger shapes
Two forms observed in the codebase:
### Form A — verb on next line
```elixir
conn =
post(conn, ~p"/users/log-in", %{
"user" => %{"email" => user.email}
})
```
`conn =` ends the line; the verb call starts on the next line with deeper indentation.
### Form B — verb on same line, args span multiple lines
```elixir
conn = post(conn, ~p"/users/log-in", %{
"user" => %{"email" => user.email}
})
```
`conn = verb(conn,` starts on one line but the arguments (maps, keyword lists) span multiple lines.
## Transformations
The same four cases apply as for single-line, just with multi-line args preserved:
### Case 1 — assign + helper on next line
```elixir
# Before
conn =
post(conn, ~p"/users/log-in", %{
"user" => %{"email" => user.email}
})
response = html_response(conn, 200)
# After
response = conn |> post(~p"/users/log-in", %{
"user" => %{"email" => user.email}
}) |> html_response(200)
```
### Case 2 — assert helper on next line
```elixir
# Before
conn =
post(conn, ~p"/users/log-in", %{
"user" => %{"email" => user.email}
})
assert html_response(conn, 200) =~ "Welcome"
# After
assert conn |> post(~p"/users/log-in", %{
"user" => %{"email" => user.email}
}) |> html_response(200) =~ "Welcome"
```
### Case 3 — assert pattern match + helper
Same as Case 2 but with `assert %{body: body} = html_response(conn, 200)`.
### Case 4 — multiple conn references ahead
```elixir
# Before
conn =
post(conn, ~p"/users/log-in", %{
"user" => %{"email" => user.email}
})
assert redirected_to(conn) == ~p"/"
assert get_session(conn, :user_token)
# After
response = conn |> post(~p"/users/log-in", %{
"user" => %{"email" => user.email}
})
assert redirected_to(response) == ~p"/"
assert get_session(response, :user_token)
```
## Algorithm
### Phase 1 — Detect trigger start
Match either:
- `conn =` at end of line (Form A, verb on next line)
- `conn = verb(conn,` where the line does not end with a balanced `)` (Form B, args continue on next line)
### Phase 2 — Accumulate the expression
Read subsequent lines until the expression is complete. Track parenthesis/brace/bracket depth to find the closing boundary:
- Start with depth from the trigger line (e.g., `post(conn, ~p"/path", %{` has depth 2: one `(`, one `{`)
- For each subsequent line, update depth by counting opening vs closing delimiters
- When depth reaches 0, the expression is complete
Buffer all accumulated lines.
### Phase 3 — Extract verb and args
From the accumulated block:
- **verb**: the HTTP verb (`get`, `post`, `put`, `patch`, `delete`, `head`, `options`)
- **args**: everything between `verb(conn,` and the final `)`, preserving original formatting (indentation, newlines)
### Phase 4 — Peek ahead for case classification
Skip blank lines after the accumulated expression. Read the next non-blank line (`next_line`).
Classify as Case 1, 2, or 3 using the same patterns as today (helper assignment, assert helper, assert pattern match).
Then **look ahead further** (skipping blanks, stopping at `end`, `conn =`, `test`, `describe`) to check if `conn` is referenced again. If so, override to Case 4.
### Phase 5 — Emit output
- **Cases 13**: emit a single merged pipeline line, inserting the multi-line args as-is:
```
indent + var + " = conn |> " + verb + "(" + args + ") |> " + helper + "(" + status + ")"
```
For multi-line args, the output will naturally span multiple lines.
- **Case 4**: emit `response = conn |> verb(args)`, then rename `conn` → `response` in subsequent lines until scope boundary (`end`, `conn =`, `test`, `describe`).
- **Fallback** (no conn on next line, no recognized pattern): emit the original trigger unchanged.
## Remaining issues to handle
| File | Line | Verb |
|---|---|---|
| `user_settings_controller_test.exs` | 74 | `put` |
| `user_settings_controller_test.exs` | 89 | `put` |
| `user_session_controller_test.exs` | 76 | `post` |
| `user_session_controller_test.exs` | 88 | `post` |
| `user_session_controller_test.exs` | 119 | `post` |
| `user_session_controller_test.exs` | 132 | `post` |
| `user_session_controller_test.exs` | 144 | `post` |
| `user_session_controller_test.exs` | 157 | `post` |
| `user_session_controller_test.exs` | 171 | `post` |
| `user_registration_controller_test.exs` | 44 | `post` |
All in `app/test/firehose_web/controllers/`.
## Elixir implementation notes (for future session)
An Elixir implementation would be simpler than the awk script because:
- Read file into a list of lines (already a list data structure)
- Pattern match on line content with guards
- Recurse or use `reduce_while` over the line list with explicit state (`:normal`, `:accumulating`, `:triggered`, `:renaming`)
- String manipulation with `String.split/2`, `String.replace/3`, etc.
- No need for regex capture groups — use `String.starts_with?/2`, `String.contains?/2`, etc.
- Parenthesis depth tracking is a simple integer accumulator
Possible structure:
```elixir
defmodule ConnRefactor do
def refactor(file_path) do
file_path
|> File.read!()
|> String.split("\n", trim: false)
|> do_refactor([], :normal, %{})
|> Enum.join("\n")
end
defp do_refactor(lines, output, state, context)
end
```