Skip to content

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
@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