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

import/metavar: Recognize empty imports #29

Merged
merged 1 commit into from
Jun 25, 2021
Merged

import/metavar: Recognize empty imports #29

merged 1 commit into from
Jun 25, 2021

Conversation

abhinav
Copy link
Contributor

@abhinav abhinav commented Jun 24, 2021

Users previously had to work around the limitation that a named import in a
patch would transform an unnamed import to a named import.

That is,

@@
var foo, X identifier
@@
-import foo "example.com/foo"
+import foo "example.com/foo/v2"

 foo.X

Would transform unnamed imports in matched files to named imports:

package thing

-import "example.com/foo"
+import foo "example.com/foo/v2"

This was undesirable.

Teach the import matcher and replacer to recognize that the matched import
is unnamed, and remember that for later, associating it with the
metavariable name, not the import path. Associating it with the import path
would make it impossible to rename import paths like the case above.

The implementaton is a bit messy, but we basically have to differentiate
between the package name of an import and the identifier we use to refer to
types defined in that import.

Resolves #2

Users previously had to work around the limitation that a named import in a
patch would transform an unnamed import to a named import.

That is,

```diff
@@
var foo, X identifier
@@
-import foo "example.com/foo"
+import foo "example.com/foo/v2"

 foo.X
```

Would transform unnamed imports in matched files to named imports:

```diff
package thing

-import "example.com/foo"
+import foo "example.com/foo/v2"
```

This was undesirable.

Teach the import matcher and replacer to recognize that the matched import
is unnamed, and remember that for later, associating it with the
metavariable name, not the import path. Associating it with the import path
would make it impossible to rename import paths like the case above.

The implementaton is a bit messy, but we basically have to differentiate
between the package name of an import and the identifier we use to refer to
types defined in that import.

Resolves #2
@abhinav abhinav merged commit 633d570 into main Jun 25, 2021
@abhinav abhinav deleted the handle-any-import branch June 25, 2021 01:01
abhinav added a commit that referenced this pull request Jul 7, 2021
The destutter example was working around #2. This is no longer necessary
since #29. testdata/destutter tests this example patch against both,
named and unnamed imports, and the test case remains unchanged.
abhinav added a commit that referenced this pull request Jul 23, 2021
The destutter example was working around #2. This is no longer necessary
since #29. testdata/destutter tests this example patch against both,
named and unnamed imports, and the test case remains unchanged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

imports: named import with metavariable should recognize unnamed import
2 participants