-
Notifications
You must be signed in to change notification settings - Fork 926
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
base: master
Are you sure you want to change the base?
Handling fn empty where and comments after return type #5411
Conversation
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 |
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// Clusure to handle a comment in a snippet | |
// Closure to handle a comment in a snippet |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
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 The other reported case in #4649 with non-empty |
tests/source/issue-5407/one.rs
Outdated
// Original from #4649 | ||
trait Foo { | ||
fn bar(&self) | ||
where | ||
// Self: Bar | ||
// Some comment | ||
; | ||
} | ||
|
||
fn foo<T>() | ||
where | ||
// T: Bar, | ||
// Some comment | ||
{ | ||
println!("foo"); | ||
} | ||
|
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
Thank you for looking into this! |
…iles (per review comment)
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.