ash-project / ash-project/igniter

Proposal: Deprecate `{:ok, {:code, Macro.t()}}` in favor of alternative form

Open
#171 6 comments 0 reactions 0 assignees View on GitHub
enhancement
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.