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.

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

Fix false positive in PartialEq check in unconditional_recursion lint #12137

Conversation

GuillaumeGomez
Copy link
Member

@GuillaumeGomez GuillaumeGomez commented Jan 12, 2024

Fixes #12133.

We needed to check for the type of the previous element in case it's a field.

EDIT: After some extra thoughts, no need to check if it's a field, just if it's the same type as Self.

r? @llogiq

changelog: Fix false positive in PartialEq check in unconditional_recursion lint

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jan 12, 2024
@GuillaumeGomez GuillaumeGomez force-pushed the fix-unconditional_recursion-false-positive branch from 0cda951 to 1326672 Compare January 12, 2024 16:37
@llogiq
Copy link
Contributor

llogiq commented Jan 12, 2024

Thank you!

@bors r+

@bors
Copy link
Contributor

bors commented Jan 12, 2024

📌 Commit 1326672 has been approved by llogiq

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Jan 12, 2024

⌛ Testing commit 1326672 with merge 7eca5af...

@bors
Copy link
Contributor

bors commented Jan 12, 2024

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: llogiq
Pushing 7eca5af to master...

1 similar comment
@bors
Copy link
Contributor

bors commented Jan 12, 2024

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: llogiq
Pushing 7eca5af to master...

@bors bors merged commit 7eca5af into rust-lang:master Jan 12, 2024
5 checks passed
@GuillaumeGomez GuillaumeGomez deleted the fix-unconditional_recursion-false-positive branch January 12, 2024 19:50
@Robbepop
Copy link

Robbepop commented Jan 18, 2024

@GuillaumeGomez is this fix supposed to already have been shipped in the most recent nightly clippy version via rustup? I wonder since it has been merged for quite some days but I am still receiving false positive bugs with newest nightly Clippy with the Wasmi project CI. (https://github.com/paritytech/wasmi, namely in the wasmi_arena crate)

@GuillaumeGomez
Copy link
Member Author

It's synced every 15 days. I thin next sync is in ~10 days. If you want a faster sync, please open an issue so someone can do it.

@Robbepop
Copy link

It's synced every 15 days. I thin next sync is in ~10 days. If you want a faster sync, please open an issue so someone can do it.

Good to know! This is fine for me, I just wanted to know if I am running into yet another case that has not yet been covered and should therefore be reported. :)

@GuillaumeGomez
Copy link
Member Author

Don't hesitate to add a new test if none of the existing ones look like yours. ;)

@Robbepop
Copy link

Will do as soon as such a situation arises. :)

primeos-work added a commit to primeos-work/butido that referenced this pull request Feb 19, 2024
The `suspicious_open_options` lint [0] warns that the truncation
behaviour should be made explicit when creating new files. We also set
`create_new(true)`, which ensures that a new file will *always* be
created so we should simply drop `create(true)` since it has no effect
anyway: "If `.create_new(true)` is set, `.create()` and `.truncate()`
are ignored." [1]

The `unconditional_recursion` lint [2] also emits a warning but that's a
false positive and should already be fixed in nightly (see [3] for a
very similar case and [4] for the PR that should fix it).
In our case we're comparing tuples with just two fields of the `Package`
structure so it isn't recursive.

[0]: https://rust-lang.github.io/rust-clippy/master/index.html#/suspicious_open_options
[1]: https://docs.rs/tokio/1.34.0/tokio/fs/struct.OpenOptions.html#method.create_new
[2]: https://rust-lang.github.io/rust-clippy/master/index.html#/unconditional_recursion
[3]: rust-lang/rust-clippy#12133
[4]: rust-lang/rust-clippy#12137
primeos-work added a commit to primeos-work/butido that referenced this pull request Feb 19, 2024
The `suspicious_open_options` lint [0] warns that the truncation
behaviour should be made explicit when creating new files. We also set
`create_new(true)`, which ensures that a new file will *always* be
created so we should simply drop `create(true)` since it has no effect
anyway: "If `.create_new(true)` is set, `.create()` and `.truncate()`
are ignored." [1]

The `unconditional_recursion` lint [2] also emits a warning but that's a
false positive and should already be fixed in nightly (see [3] for a
very similar case and [4] for the PR that should fix it).
In our case we're comparing tuples with just two fields of the `Package`
structure so it isn't recursive.

[0]: https://rust-lang.github.io/rust-clippy/master/index.html#/suspicious_open_options
[1]: https://docs.rs/tokio/1.34.0/tokio/fs/struct.OpenOptions.html#method.create_new
[2]: https://rust-lang.github.io/rust-clippy/master/index.html#/unconditional_recursion
[3]: rust-lang/rust-clippy#12133
[4]: rust-lang/rust-clippy#12137

Signed-off-by: Michael Weiss <michael.weiss@eviden.com>
ammernico pushed a commit to ammernico/butido that referenced this pull request Apr 30, 2024
The `suspicious_open_options` lint [0] warns that the truncation
behaviour should be made explicit when creating new files. We also set
`create_new(true)`, which ensures that a new file will *always* be
created so we should simply drop `create(true)` since it has no effect
anyway: "If `.create_new(true)` is set, `.create()` and `.truncate()`
are ignored." [1]

The `unconditional_recursion` lint [2] also emits a warning but that's a
false positive and should already be fixed in nightly (see [3] for a
very similar case and [4] for the PR that should fix it).
In our case we're comparing tuples with just two fields of the `Package`
structure so it isn't recursive.

[0]: https://rust-lang.github.io/rust-clippy/master/index.html#/suspicious_open_options
[1]: https://docs.rs/tokio/1.34.0/tokio/fs/struct.OpenOptions.html#method.create_new
[2]: https://rust-lang.github.io/rust-clippy/master/index.html#/unconditional_recursion
[3]: rust-lang/rust-clippy#12133
[4]: rust-lang/rust-clippy#12137

Signed-off-by: Michael Weiss <michael.weiss@eviden.com>
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.

unconditional_recursion false positive in PartialEq field comparison (2)
5 participants