imports_granularity = "Module" causes compile error when path contains self

[Aloso]

Describe the bug

If a path contains self, this keyword must be enclosed in braces:

use crate::lexer::{self}; // valid
use crate::lexer::self;   // invalid

rustfmt with imports_granularity = "Module" sometimes splits use statements so that invalid code like above is emitted.

To Reproduce

use crate::lexer::{self, tokens::TokenData};

rustfmt formats it like this:

use crate::lexer::self;
use crate::lexer::tokens::TokenData;

When formatting the file again, the ::self is removed.

Expected behavior

rustfmt should emit the following code without having to run it twice:

use crate::lexer;
use crate::lexer::tokens::TokenData;

Note that use crate::lexer; and use crate::lexer::{self}; are not actually equivalent: The latter only imports the lexer module, while the former looks in all namespaces, so it can import up to 3 items at once, e.g. a function, a module and a macro of the same name.

To make sure that the behaviour doesn't change, it needs to be formatted like so:

use crate::lexer::{self};
use crate::lexer::tokens::TokenData;

However, I would find this rather annoying, because of the way rust-analyzer handles auto-imports.

Meta

• rustfmt version: rustfmt 1.4.34-nightly (ea268b9 2021-01-28)
• From where did you install rustfmt?: rustup
• How do you run rustfmt: cargo fmt

[calebcartwright]

Thanks for the report. We're aware of the differences between the two, which is why you'll notice these self cases handled with Item and Crate variants, but something definitely got missed here!

[davidBar-On]

I am willing to try to fix this, but from the description of the expected behavior it is not clear whether the output should be crate::lexer or crate::lexer::{self}.

Also, if this is a bug, why is the issue labeled only-with-option? If an option is needed, I am not sure how to write a test case when the option is not set, since the tests\target file will include use crate::lexer::self; which will be formatted as use crate::lexer; and the test will fail.

[calebcartwright]

I am willing to try to fix this

Awesome, thank you!

but from the description of the expected behavior it is not clear whether the output should be crate::lexer or crate::lexer::{self}.

It has to be the latter, because as noted in the issue description these are not equivalent and changing what the developer wrote has the potential to cause compilation issues in the emitted formatted code (rust-lang/rust#60941)

Also, if this is a bug, why is the issue labeled only-with-option?

I've added a description to the label that hopefully clarifies, but it's used to indicate that the issue cannot occur with the default rustfmt behavior and requires the use of an overridden value for a configuration option.

If an option is needed, I am not sure how to write a test case when the option is not set, since the tests\target file will include use crate::lexer::self; which will be formatted as use crate::lexer; and the test will fail.

I'm not sure I understand this. All options have default values, but this particular issue requires a non-default value in order to reproduce. The files used for the system/idempotence will need to have the corresponding config option self to Module via the standard comments at the top of the files.

I'd expect the input file to include:

use crate::lexer::{self, tokens::TokenData};

and the target file to inculde:

use crate::lexer::{self};
use crate::lexer::tokens::TokenData;

[davidBar-On]

If an option is needed, I am not sure how to write a test case when the option is not set, since the tests\target file will include use crate::lexer::self; which will be formatted as use crate::lexer; and the test will fail.

I'm not sure I understand this. ....

No problem - my misunderstanding. I thought that the meaning of the label only-with-option is that a new option should be added for whether use the modified formatting.

[tommilligan]

Cross linking #4991: required to be closed before imports_granularity is stabilised

[Aloso]

@davidBar-On is this issue fixed by #4716?

[tommilligan]

@Aloso this issue is still present in master - I just opened a fix in #5253 (there is a test added which reproduces the failure).

The fix from #4716 appears to be present in the rc2 branch but not in master, I guess it got lost in a merge somewhere.

There are more details in the description of #5253

[calebcartwright]

The fix from #4716 appears to be present in the rc2 branch but not in master, I guess it got lost in a merge somewhere.

Correct on the existence of the commit in different branches, but not quite on the cause, see #4801