Skip to content

[go1.19] Limit augmentation to native files which need it #1267

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

Merged
merged 2 commits into from
Feb 13, 2024

Conversation

grantnelson-wf
Copy link
Collaborator

@grantnelson-wf grantnelson-wf commented Feb 13, 2024

The part of augmentation that prunes imports based on usage was designed to handle removing imports specific to a type parameter. For example, the import cmp might only exist in a file because of the type parameter constraint cmp.Ordered. Go doesn't like it if there are unused imports.

However, the code written for pruning imports uses the given name or guesses at the package name via the given path. This does not work for any relative paths (e.g. ./..), versioned packages (e.g. github.com/foo/bar/v2 where the package is bar), or using the go.mod to define a path for one package to an arbitrary path using a replace. These problematic paths do not exist in the natives which are being altered by augmentation, but problematic paths may exist in customer code.

Since we only really want to prune imports for files which are augmented, I'm adding some checks so that the prune imports code is only run when overlays exist and those overlays have changed the original file.

I also fixed an issue where a left over break was making only the first found directive based import be checked for and none of the following. Added a test that demonstrates the issue and checks the fix.

@grantnelson-wf
Copy link
Collaborator Author

grantnelson-wf commented Feb 13, 2024

To get CI to pass, #1266, will have to be merged first since that'll fix the issue with window_smoke.

Merged update, so now, CI should pass

@grantnelson-wf grantnelson-wf marked this pull request as ready for review February 13, 2024 22:37
@nevkontakte nevkontakte merged commit a533e78 into gopherjs:go1.19 Feb 13, 2024
@grantnelson-wf grantnelson-wf deleted the limitAugmentation branch February 13, 2024 23:46
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.

2 participants