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.

Sign up for GitHub

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

Jump to bottom

items_after_test_module: Ignore in-proc-macros items #10992

Merged
merged 1 commit into from
Jun 20, 2023
Merged

items_after_test_module: Ignore in-proc-macros items #10992

merged 1 commit into from
Jun 20, 2023

Conversation

blyxyas
Copy link
Member

@blyxyas blyxyas commented Jun 19, 2023

The library test-case is having some problems with this lint, ignoring proc macros should fix it.
Related to #10713 and frondeus/test-case#122

(Couldn't add test cases for this exact situation without importing the library, but I think the fix is simple enough that we can be pretty sure there won't be any problems :) )

changelog:[items_after_test_module]: Ignore items in procedural macros

@rustbot
Copy link
Collaborator

rustbot commented Jun 19, 2023

r? @Manishearth

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jun 19, 2023
@Manishearth
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Jun 20, 2023

📌 Commit 62c9e0b has been approved by Manishearth

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Jun 20, 2023

⌛ Testing commit 62c9e0b with merge 89294e1...

@ilyagr
Copy link

ilyagr commented Jun 20, 2023 •
edited
Loading

I'm unfamiliar with Rust's release process, so I thought I'd ask: is there any chance this will land in 1.71 before that's stable (or a point release, not sure whether those exist)?

If not, I'm worried that workarounds like https://github.com/martinvonz/jj/pull/1686/files will be necessary until the MSRV for a project is at 1.72 or above.

Also, thank you very much for fixing this!

@Manishearth
Copy link
Member

I'm unfamiliar with Rust's release process, so I thought I'd ask: is there any chance this will land in 1.71 before that's stable?

It is currently on track to be a part of Rust 1.72 as long as it gets synced to rust-lang/rust before July 7 (very likely).

I'm open to this being uplifted, it needs to be uplifted into beta before July 13 to make 1.71.

@Manishearth
Copy link
Member

I've started this zulip discussion to check if the clippy team is open to it being uplifted. If so, we can talk to the release team and send them a patch.

@bors
Copy link
Contributor

bors commented Jun 20, 2023

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: Manishearth
Pushing 89294e1 to master...

@bors bors merged commit 89294e1 into rust-lang:master Jun 20, 2023
@flip1995 flip1995 added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jun 20, 2023
@flip1995 flip1995 added beta-accepted Accepted for backporting to the compiler in the beta channel. and removed beta-nominated Nominated for backporting to the compiler in the beta channel. labels Jul 6, 2023
@flip1995 flip1995 mentioned this pull request Jul 6, 2023
Merged
@flip1995
Copy link
Member

flip1995 commented Jul 6, 2023

rust-lang/rust#113417

bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 6, 2023
@bors
Auto merge of rust-lang#113417 - flip1995:clippy_beta_backport, r=Man…
f06c1b8 
…ishearth

[beta] Clippy beta backport

Backported PRs:

- rust-lang/rust-clippy#10813
- rust-lang/rust-clippy#10865
- rust-lang/rust-clippy#10873
- rust-lang/rust-clippy#10992

Those address bugs, that were either introduced with 1.71 and that we would like to address before they get into stable or bugs that were introduced in 1.70 and that we would like to be fixed in 1.71 already.

Detailed arguments for the backports of those PRs are in the PRs (mostly).

r? `@Mark-Simulacrum`
@ilyagr ilyagr mentioned this pull request Jul 10, 2023
Closed
@xFrednet xFrednet removed the beta-accepted Accepted for backporting to the compiler in the beta channel. label Jul 11, 2023
jamesbornholt added a commit to jamesbornholt/mountpoint-s3 that referenced this pull request Jul 13, 2023
@jamesbornholt
Disable new items_after_test_module Clippy lint
28ab238 
This is new in 1.71, but seems to get confused by the test_case macro.
It was supposedly fixed in
rust-lang/rust-clippy#10992 but that seems to
not have entirely worked.

Signed-off-by: James Bornholt <bornholt@amazon.com>
@jamesbornholt jamesbornholt mentioned this pull request Jul 13, 2023
Merged
jamesbornholt added a commit to awslabs/mountpoint-s3 that referenced this pull request Jul 13, 2023
@jamesbornholt
Disable new items_after_test_module Clippy lint (#372)
862625f 
This is new in 1.71, but seems to get confused by the test_case macro.
It was supposedly fixed in
rust-lang/rust-clippy#10992 but that seems to
not have entirely worked.

Signed-off-by: James Bornholt <bornholt@amazon.com>
@jamesbornholt jamesbornholt mentioned this pull request Jul 13, 2023
Closed
@brooksprumo brooksprumo mentioned this pull request Jul 14, 2023
Merged
@blyxyas blyxyas deleted the fix-test_case_lib branch October 5, 2023 09:03
@Colonial-Dev
Copy link

Colonial-Dev commented Oct 9, 2023 •
edited
Loading

I'm still getting false positives for this lint with the latest clippy on both stable and nightly (clippy 0.1.73 (cc66ad4 2023-10-03) and clippy 0.1.73 (4e78abb 2023-08-28) respectively.)

It's being triggered by a derive macro that generates a test module. The code is here, although you'll need to remove my workaround to get the lint to fire again.

The workaround: change #[cfg(test)] to #[cfg(not(not(test)))]. (We do a little double negation.)

@flip1995
Copy link
Member

Can you open a new issue with possibly a minimal reproducer for this, please?

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

@Manishearth Manishearth Manishearth approved these changes

Assignees

@Manishearth Manishearth

Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@blyxyas @rustbot @Manishearth @bors @ilyagr @flip1995 @Colonial-Dev @xFrednet