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

Another fix for merge_imports #3769

Merged
merged 1 commit into from
Sep 4, 2019

Conversation

valff
Copy link
Contributor

@valff valff commented Sep 1, 2019

Sorry, my previous PR #3753 didn't fix #3750 for all cases. This should finally fix it.

@valff valff force-pushed the merge_imports_fix2 branch from 811f06f to 872427a Compare September 2, 2019 06:04
Copy link
Contributor

@topecongiro topecongiro left a comment

Choose a reason for hiding this comment

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

Thanks!

@@ -988,7 +1000,12 @@ mod test {
test_merge!(["a::b::{c, d}", "a::b::{e, f}"], ["a::b::{c, d, e, f}"]);
test_merge!(["a::b::c", "a::b"], ["a::{b, b::c}"]);
test_merge!(["a::b", "a::b"], ["a::b"]);
test_merge!(["a", "a::b", "a::b::c"], ["a::{self, b::{self, c}}"]);
test_merge!(["a", "a::b", "a::b::c"], ["a::{self, b, b::c}"]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, looking back to this test, merging use a; into use a::{self}; does not seem to be correct w.r.t. #3750.
@valff IIUC this has existed before this PR, but while you are at it, would you mind having a look if you have time? This won't be a block for this PR - if this is not easy to fix then we can leave it for the future work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@topecongiro I have kept use a::{self} as a special case where self is at depth 2, because the first path segment is always a crate name or a special keyword (at least in edition 2018?). So if the first segment is always a module (crate), it should be correct w.r.t. #3750. I find this special case useful because this way we still have a single use statement per crate.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I misunderstood this. Thanks!

@topecongiro topecongiro merged commit 15a28f7 into rust-lang:master Sep 4, 2019
@topecongiro topecongiro added this to the 1.4.7 milestone Sep 4, 2019
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.

Formatting changes semantics of imports
2 participants