Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change get_range/1 to allow returning nil and add syntax corpus #107

Merged
merged 2 commits into from
Sep 13, 2023

Conversation

zachallaun
Copy link
Contributor

@zachallaun zachallaun commented Sep 11, 2023

Closes #106
Related to #105

While I was writing the proposal in #105, I suggested has_range? thinking that it might be possible to always be able to calculate a range for a 3-tuple. When adding the syntax corpus, however, I found that that doesn't seem to be the case for some injected nodes in interpolations or for empty blocks ({:__block__, [], []}).

So I went ahead and attempted the get_range refactor and it's not bad at all.

Note that this is a breaking change currently. If we don't want to break the contract of Sourceror.get_range/1, we could easily do something like this:

@spec get_range(Macro.t(), keyword()) :: range
def get_range(quoted, opts \\ []) do
  Sourceror.Range.get_range(quoted, opts) ||
    raise ArgumentError, "cannot get range for: #{inspect(quoted)}"
end

@spec fetch_range(Macro.t(), keyword()) :: {:ok, range} | :error
def fetch_range(quoted, opts \\ []) do
  case Sourceror.Range.get_range(quoted, opts) do
    %{} = range -> {:ok, range}
    nil -> :error
  end
end

@zachallaun
Copy link
Contributor Author

Looking at CI, I need to pull out some additional syntax to only run on later Elixir versions. I'll fix & rebase in just a little bit!

@zachallaun zachallaun force-pushed the nil-get-range branch 2 times, most recently from d74fedf to 13eab25 Compare September 11, 2023 18:02
@zachallaun
Copy link
Contributor Author

Phew, finally got CI passing 😅

Copy link
Owner

@doorgan doorgan left a comment

Choose a reason for hiding this comment

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

Awesome work! 💜

@doorgan doorgan merged commit eaf0cc6 into doorgan:main Sep 13, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Vendor in corpus of Elixir syntax examples to test against
2 participants