Skip to content

Handling fn empty where and comments after return type #5411

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

davidBar-On
Copy link
Contributor

@davidBar-On davidBar-On commented Jun 28, 2022

Proposed fix for issue #5407. Although the original issue is about fn empty where clause indentation, the issue is actually about handling of comments after fn return type. This is because with current code the empty where was handled as a post return type comment.

In addition, there are related issues with handling comments for fn empty where clause without return type. Therefore the fix handles the cases of fn return type without where clause, empty where clause without return type, and return type with empty where clause.

The proposed fix seem to also resolve issue #4649.

Note that the fix is not related to issues with traits empty where clause, such as #4672.

@ytmimi
Copy link
Contributor

ytmimi commented Jun 28, 2022

Thanks for your work on this! I Also appreciate you linking some relevant issues. I'm planning to get an initial review done in the next few days

@ytmimi ytmimi self-requested a review June 28, 2022 14:14
@ytmimi ytmimi self-assigned this Jun 28, 2022
src/items.rs Outdated
if where_clause.predicates.is_empty() {
let snippet_hi = span.hi();
let snippet = context.snippet(mk_sp(snippet_lo, snippet_hi));
// Clusure to handle a comment in a snippet
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// Clusure to handle a comment in a snippet
// Closure to handle a comment in a snippet

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@ytmimi
Copy link
Contributor

ytmimi commented Jul 18, 2022

The proposed fix seem to also resolve issue #4649.

Could we add test cases for the linked issue if these changes help to resolve what's going on there

@davidBar-On
Copy link
Contributor Author

The proposed fix seem to also resolve issue #4649.

Could we add test cases for the linked issue if these changes help to resolve what's going on there

Added test cases for the original case of #4649 with empty where.

The other reported case in #4649 with non-empty where is a different issue and is probably the same issue as reported in #4672.

Comment on lines 18 to 34
// Original from #4649
trait Foo {
fn bar(&self)
where
// Self: Bar
// Some comment
;
}

fn foo<T>()
where
// T: Bar,
// Some comment
{
println!("foo");
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think it makes sense to create a issue_4649.rs file to better associate the test case with the original issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK. Changed.
Created issue-4649 folder and moved the relevant test cases. The folder was created to allow testing with both One and Two versions, although I am not sure whether or when it is important to test both versions.

@ytmimi
Copy link
Contributor

ytmimi commented Jul 18, 2022

Added test cases for the #4649 (comment) of #4649 with empty where.

The #4649 (comment) in #4649 with non-empty where is a different issue and is probably the same issue as reported in #4672.

Thank you for looking into this!

@ytmimi ytmimi added the p-low label Jul 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rustfmt removes all indentation from the where clause here, making it inconsistent with the function header
3 participants