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

Prevent rustfmt from adding impl Trait definitions to types #5092

Closed
wants to merge 1 commit into from

Conversation

ytmimi
Copy link
Contributor

@ytmimi ytmimi commented Nov 18, 2021

Fixes #5086

@ytmimi
Copy link
Contributor Author

ytmimi commented Nov 18, 2021

Please let me know what additional test cases I could include to cover more type declarations!

@calebcartwright
Copy link
Member

Thanks so @ytmimi for the quick fix! This would indeed have addressed the immediate issue, but I'm going to close in favor of #5093. I'd been doing some cleanup on aliases recently so I'd started looking at this yesterday evening and want to go that direction since that addresses some things more holistically.

I'd really like to address the underlying indentation issue with ImplTrait kinds when using version Two of formatting, but have opted to continue leveraging the existing hack to avoid running into that issue on type aliases.

I suspect that issue could be a fairly tricky fix, but if you wanted to take a look at it I suspect it's either due to the usage of a suboptimal shape and/or a bug in join_bounds_inner that attempts to determine whether or not to block indent

rustfmt/src/types.rs

Lines 812 to 825 in eee8f04

ast::TyKind::ImplTrait(_, ref it) => {
// Empty trait is not a parser error.
if it.is_empty() {
return Some("impl".to_owned());
}
let rw = if context.config.version() == Version::One {
it.rewrite(context, shape)
} else {
join_bounds(context, shape, it, false)
};
rw.map(|it_str| {
let space = if it_str.is_empty() { "" } else { " " };
format!("impl{}{}", space, it_str)
})

@ytmimi ytmimi deleted the issue_5086 branch November 19, 2021 00:43
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.

Rustfmt inserts a made-up impl Trait definition into types
2 participants