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

Comment removed between type name and = #4244

Closed
ayazhafiz opened this issue Jun 8, 2020 · 1 comment · Fixed by #4448
Closed

Comment removed between type name and = #4244

ayazhafiz opened this issue Jun 8, 2020 · 1 comment · Fixed by #4448
Labels
a-comments bug Panic, non-idempotency, invalid code, etc. good first issue Issues up for grabs, also good candidates for new rustfmt contributors hacktoberfest help wanted
Milestone

Comments

@ayazhafiz
Copy link
Contributor

pub type Export /*: Bound */ = S1;

This is one where I think a comment is justified; today we do not enforce such bounds (i.e. the compiler does not check that the right-hand side of a type definition actually conforms to such a bound), and the compiler actually errors if you try to include it (because we don't want people to mistakenly think that such bounds are enforced).

But the comment here serves a useful purpose (about the intent, that we intend for this type definition to conform to the bound).

Originally posted by @pnkfelix in #2781 (comment)

@topecongiro topecongiro added a-comments bug Panic, non-idempotency, invalid code, etc. labels Jun 9, 2020
@topecongiro topecongiro added this to the 3.0.0 milestone Jun 29, 2020
@calebcartwright calebcartwright added good first issue Issues up for grabs, also good candidates for new rustfmt contributors hacktoberfest help wanted labels Sep 29, 2020
@calebcartwright
Copy link
Member

For anyone interested in working on this, here are some of the relevant sections of the code:
rewrite_type

fn rewrite_type<R: Rewrite>(
context: &RewriteContext<'_>,
indent: Indent,
ident: symbol::Ident,
vis: &ast::Visibility,
generics: &ast::Generics,
generic_bounds_opt: Option<&ast::GenericBounds>,
rhs: Option<&R>,
) -> Option<String> {

The dropping of the comment happens here on this line when the current formatting result is mashed together with the assignment operator. The individual elements of the item (the visibility, ident, generics, generic bounds, etc.) are formatted independently and then combined, but the comment is dropped because there's no checking for comments within the spans between. In this particular case that happens to be the span between the item's ident and the assignment operator, though more generally this would be the span between the assignment operator on the hi and the lo would be the end of the rightmost item element present

let lhs = format!("{}=", result);

Note that the full span of the item can be found in the corresponding match arm in visitor.rs, which is needed for finding this between span before the assignment operator

ast::ItemKind::TyAlias(_, ref generics, ref generic_bounds, ref ty) => match ty {
Some(ty) => {
let rewrite = rewrite_type_alias(
item.ident,
Some(&*ty),
generics,
Some(generic_bounds),
&self.get_context(),
self.block_indent,
&item.vis,
);
self.push_rewrite(item.span, rewrite);
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-comments bug Panic, non-idempotency, invalid code, etc. good first issue Issues up for grabs, also good candidates for new rustfmt contributors hacktoberfest help wanted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants