Skip to content

fallout: fix tests to allow uninlined_format_args#9547

Merged
bors merged 1 commit into
rust-lang:masterfrom
nyurik:capt
Oct 4, 2022
Merged

fallout: fix tests to allow uninlined_format_args#9547
bors merged 1 commit into
rust-lang:masterfrom
nyurik:capt

Conversation

@nyurik

@nyurik nyurik commented Sep 27, 2022

Copy link
Copy Markdown
Contributor

In order to switch clippy::uninlined_format_args from pedantic to style, all existing tests must not raise a warning. I did not want to change the actual tests, so this is a relatively minor change that:

  • add #![allow(clippy::uninlined_format_args)] where needed
  • normalizes all allow/deny/warn attributes
    • all allow attributes are grouped together
    • sorted alphabetically
    • the clippy::* attributes are listed separate from the other ones.
    • deny and warn attributes are listed before the allowed ones

See also #9233, #9525, #9527

cc: @llogiq @Alexendoo

changelog: none

@rust-highfive

Copy link
Copy Markdown

r? @flip1995

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Sep 27, 2022
@bors

bors commented Sep 29, 2022

Copy link
Copy Markdown
Contributor

☔ The latest upstream changes (presumably #9516) made this pull request unmergeable. Please resolve the merge conflicts.

@nyurik nyurik changed the title fallout: allow new uninlined_format_args in tests fallout: fix tests to allow uninlined_format_args Sep 29, 2022

@flip1995 flip1995 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This adds an allow attribute for the uninlined_format_args lint for every test file, even if it doesn't even use the format! macro. This is highly unnecessary and not something we want. Please only add this attribute to the tests that would actually fail due to the new lint.

@nyurik

nyurik commented Oct 1, 2022

Copy link
Copy Markdown
Contributor Author

This adds an allow attribute for the uninlined_format_args lint for every test file, even if it doesn't even use the format! macro. This is highly unnecessary and not something we want. Please only add this attribute to the tests that would actually fail due to the new lint.

@flip1995 that is not correct - there are 801 test .rs files in the tests/ui dir, and I only changed 86 of them - only about 10%, all of which are required. I reevaluated all of them again, and did find one -- tests/ui/empty_line_after_outer_attribute.rs that I refactored by accident (reverted). Just to keep things focused, I also undeed the moveable -> movable rename. What's left are all needed.

@nyurik nyurik requested a review from flip1995 October 1, 2022 01:29
@bors

bors commented Oct 2, 2022

Copy link
Copy Markdown
Contributor

☔ The latest upstream changes (presumably #9479) made this pull request unmergeable. Please resolve the merge conflicts.

In order to switch `clippy::uninlined_format_args` from pedantic to
style, all existing tests must not raise a warning. I did not want to
change the actual tests, so this is a relatively minor change that:

* add `#![allow(clippy::uninlined_format_args)]` where needed
* normalizes all allow/deny/warn attributes
   * all allow attributes are grouped together
   * sorted alphabetically
   * the `clippy::*` attributes are listed separate from the other ones.
   * deny and warn attributes are listed before the allowed ones

changelog: none
@nyurik

nyurik commented Oct 4, 2022

Copy link
Copy Markdown
Contributor Author

@flip1995 is there still something that needs to be done to this PR? Thx!

@flip1995 flip1995 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sorry, I was away over the weekend (long weekend in Germany).

All good now. Maybe I saw ghosts, but I thought I saw a file where it wasn't necessary. Thanks for addressing this!

@flip1995

flip1995 commented Oct 4, 2022

Copy link
Copy Markdown
Member

@bors r+ p=10

@bors

bors commented Oct 4, 2022

Copy link
Copy Markdown
Contributor

📌 Commit eb39702 has been approved by flip1995

It is now in the queue for this repository.

@bors

bors commented Oct 4, 2022

Copy link
Copy Markdown
Contributor

⌛ Testing commit eb39702 with merge 2f90b2a...

@bors

bors commented Oct 4, 2022

Copy link
Copy Markdown
Contributor

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: flip1995
Pushing 2f90b2a to master...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants