Add sub module inner attributes to module item attributes #5275
Conversation
Fixes 4959 During module resolution rustfmt already peeks inside external sub modules. This process was extended to now capture inner attributes from those sub modules and relate them back to the parent module's items.
|
Excellent, this is a pretty important fix but one which I intend to defer to the next release given the size of the diffs and codepaths being touched |
|
That's totally fine with me! Just let me know if you'd like to see any changes. |
Extends the sub module attribute association to also include inner attributes for external sub modules that are skipped by rustfmt.
|
Question: Do we also need to support tying back attributes from sub mods defined inside Also do you know what module resolution case would lead to |
It's possible that multiple modules could have the same name. E.g.:
#[cfg(linux)]
mod foo;
#[cfg(not(linux))]
mod foo;
Previously, we would just assign attributes to the first module we found
with the same name as the one we were trying to associate inner
attributes with.This lead to a situation where we assigned all inner sub
module attributes to the first module we found.
Now we also ensure the spans for each item match so that we know we're
associating the inner attributes with the correct mod item.
Is this referring to inline mods defined within a cfg_if branch, or out of line mods conditionally imported from within cfg_if? I guess either way it would be nice but I think it wouldn't be necessary/impact for the purposes of import reordering (we don't format the inline args to the cfg_if call so we wouldn't be reordering anything there)
You'll want to double check me on this but I think these do: |
|
Also be aware that #4284 was an implementation to solve the same type of problem. Some things changed upstream in rustc as I recall so I don't think it could've been used directly anyway, but you may find the test cases in there useful |
|
Ahh I see. Thanks for pointing that out. I think I'd also need to remove the recursive condition mentioned here #4284 (comment) to prevent idempotency issues. I haven't looked at the test cases yet, but I'll be sure to do so in the coming days and try to port some of them over here! |
Fixes #4959
During module resolution rustfmt already peeks inside external sub
modules. This process was extended to now capture inner attributes from
those sub modules and relate them back to the parent module's items.