ash-project / ash-project/igniter
Proposal: Deprecate `{:ok, {:code, Macro.t()}}` in favor of alternative form
- Dominant language
- Elixir
- Stars
- 404
- Forks
- 75
- Avg merge
- 9h 58m
- Merged PRs (30d)
- 3
Description
## Problem
Igniter introduces some idioms that span multiple utilities, the goal of which is making it easier/more natural to update code (usually zippers). One such idiom is callbacks returning `:ok` tuples that can take a number of forms:
```elixir
{:ok, Zipper.t()} # replace the document zipper with the returned zipper
{:ok, {:code, Macro.t()}} # replace the zipper node with this quoted form
{:ok, {:code, String.t()}} # parse string to quoted, then replace zipper node
```
The problem is that the last two forms overlap: `"my text!"` is a valid quoted form, but Igniter instead parses it, raising if it's an invalid AST, or giving an unexpected result (like `my(text!)` in this example). This can be surprising, confusing, and difficult to debug if you haven't seen it before.
The solution isn't necessarily intuitive, especially if you're not experienced with Elixir AST: You have to return `{:ok, {:code, {:__block__, [], ["my text!"]}}}` or `{:ok, Zipper.replace(zipper, "my text!")}`.
## Proposal
I think we should bite the bullet and deprecate the `{:ok, {:code, Macro.t()}}` form in favor of:
```elixir
{:ok, Zipper.t()}
{:ok, {:code, String.t()}}
{:ok, {:quoted, Macro.t()}}
```
I'm not 100% sure how this should be done yet. There are a potentially large number of places where these forms are handled, and adding deprecation notices to each one may be tricky. We also may want to combine this proposal with some changes to `Igniter.Code.Common.add_code/3` to distinguish between a string of code and a quoted form as well.
Contributor guide
Research direction
Start by tracing where the callback return forms are handled across Igniter, then read Igniter.Code.Common.add_code/3 and the surrounding zipper utilities. Identify every location affected by the proposed deprecation and how deprecation notices could be applied. Done means the migration path is agreed on and the quoted-form and string-form behaviors are consistently distinguished.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- elixir
- Domain
- tooling
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100