Skip to content

fixes #4115, #4029, #3898 #4136

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 2 commits into from
Apr 22, 2020
Merged

fixes #4115, #4029, #3898 #4136

merged 2 commits into from
Apr 22, 2020

Conversation

ahl
Copy link
Contributor

@ahl ahl commented Apr 22, 2020

Fixes #4115. Fixes #4029. Fixes #3898.

The new tests fail on master as follows:

Mismatch at tests/target/issue-4115.rs:1:
-#[derive(
-    A,
-    B,
-    C,
-    D,
-    // E,
-)]
+#[derive(A, B, C, D)]
 fn foo() {}

Mismatch at tests/target/issue-4029.rs:1:
 // issue #4029
-#[derive(Debug, Clone, Default Hash)]
+#[derive()]
 struct S;
 
 // issue #3898

Mismatch at tests/target/issue-4029.rs:6:
-#[derive(Debug, Clone, Default,, Hash)]
+#[derive()]
 struct T;

They (and all other tests) pass with the proposed fix.

At the root of all three issues is the use of ast::Attribute:: meta_item_list() which returns None if the derive is malformed. In addition no affordance was made for comments interspersed with derive components. This fix checks for errors from meta_item_list() and leans on existing mechanisms to deal with comments.


Although I handled this case, I would appreciate review feedback for the case where a derive doesn't fit quite fit on a single line and is formatted like this:

#[derive(this, would, be, too, long, for, a single, line, lets, say)]
// so you get this:
#[derive(
    this, would, be, too, long, for, a single, line, lets, say,
)]

To handle that case I use Always for the trailing separator with ListFormatting and then remove that final character if it all does fit on one line. I couldn't find an existing mechanism in ListFormatting that did what I wanted there.

See lines 143 and 177.

Copy link
Contributor

@topecongiro topecongiro left a comment

Choose a reason for hiding this comment

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

Awesome! LGTM, thank you!


Although I handled this case, I would appreciate review feedback for the case where a derive doesn't fit quite fit on a single line and is formatted like this:

I think we could use overflow::rewrite_with_parens for this with some refactoring, though I am not confident.

@topecongiro topecongiro merged commit c019181 into rust-lang:master Apr 22, 2020
@karyon
Copy link
Contributor

karyon commented Oct 26, 2021

Backported in #4564

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
3 participants