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

Don't let goimports guess import paths #365

Merged
merged 1 commit into from
Oct 3, 2018
Merged

Conversation

vektah
Copy link
Collaborator

@vektah vektah commented Oct 2, 2018

gqlgen internally tracks all the imports used in a file and adds them to the import block, handling duplicate imports by appending names.

There are two sources of imports:

  • imports for user generated models, types in graphql, etc. These are all tracked as they are added, with the required deduping behaviours etc.
  • ambientImports: these are referenced in the templates and must keep the name they were given in the templates, eg fmt.Println( must get the import name fmt. Often these are in conditional parts of the templates and may not be used in a particular generated output.

Currently we run the generated code through goimports, this does a few things for us:

  • format the code: same as gofmt
  • group and sort imports: this makes our output a little more robust to changes
  • remove unused imports: required for ambient imports that don't actually get used.
  • add missing imports: DANGER This adds in any missing imports, these often end up being ambient imports that were forgotten about. This leads to subtle and hard to reproduce issues like Ambiguous package names lead to incorrect import statements #207 when a user imported package grabs the name used in by the import in the template.

This PR removes the "add missing imports" feature of goimports, while maintaining the other 3 by reimplementing the import removal logic and calling goimports with "Format only" to get grouping and sorting.

Fixes #207

@vektah vektah force-pushed the dont-guess-imports branch from f9d2ec0 to 732be39 Compare October 2, 2018 07:46
@vektah vektah merged commit 7833d0c into master Oct 3, 2018
@vektah vektah deleted the dont-guess-imports branch October 3, 2018 00:08
@jufemaiz
Copy link

QQ on this: is there a way of implementing local-prefixes option for goimports?

cgxxv pushed a commit to cgxxv/gqlgen that referenced this pull request Mar 25, 2022
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.

Ambiguous package names lead to incorrect import statements
3 participants