Fix bare imports breaking pattern matching with arguments#4
Merged
Conversation
A plain `import Foo` was treated as importing every name, so the matcher
rewrote every local call to `Foo.<name>` and any pattern with arguments
stopped matching. Now we only rewrite calls that an import actually lists
via `:only`.
# given a module with `import Enum`
mix ex_ast.search 'Logger.info(...)' demo.ex # was 0, now 1
mix ex_ast.search 'def add(_, _, _)' demo.ex # was 0, now 1
Also fixes multi-arity imports: `import Foo, only: [bar: 1, bar: 2]` used
`Keyword.get/2` to check membership, which only sees the first arity, so
later arities of the same name never resolved. Now it matches the whole
`{name, arity}` pair.
One consequence: a bare `import Enum` no longer expands anything, so
searching `Enum.map(_, _)` won't find a bare `map(...)`. Doing that right
would mean knowing what Enum actually exports, and we can't get that from
the AST alone — the import line doesn't say which local calls came from
it.
Collaborator
|
Thanks, great catch, merged! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hi @dannote, thanks for the lib!
I've hit this regression after upgrading, let me know if this makes sense, or if you need any changes.
Thanks!
A plain
import Foowas treated as importing every name, so the matcher rewrote every local call toFoo.<name>and any pattern with arguments stopped matching. Now we only rewrite calls that an import actually lists via:only.Also fixes multi-arity imports:
import Foo, only: [bar: 1, bar: 2]usedKeyword.get/2to check membership, which only sees the first arity, so later arities of the same name never resolved. Now it matches the whole{name, arity}pair.One consequence: a bare
import Enumno longer expands anything, so searchingEnum.map(_, _)won't find a baremap(...). Doing that right would mean knowing what Enum actually exports, and we can't get that from the AST alone — the import line doesn't say which local calls came from it. This is potentially addressed by this draft PR #5