Skip to content

Commit

Permalink
Rollup merge of #120490 - nnethercote:Diagnostic-hashing, r=estebank
Browse files Browse the repository at this point in the history
Don't hash lints differently to non-lints.

`Diagnostic::keys`, which is used for hashing and equating diagnostics, has a surprising behaviour: it ignores children, but only for lints. This was added in #88493 to fix some duplicated diagnostics, but it doesn't seem necessary any more.

This commit removes the special case and only four tests have changed output, with additional errors. And those additional errors aren't exact duplicates, they're just similar. For example, in src/tools/clippy/tests/ui/same_name_method.rs we currently have this error:
```
error: method's name is the same as an existing method in a trait
  --> $DIR/same_name_method.rs:75:13
   |
LL |             fn foo() {}
   |             ^^^^^^^^^^^
   |
note: existing `foo` defined here
  --> $DIR/same_name_method.rs:79:9
   |
LL |         impl T1 for S {}
   |         ^^^^^^^^^^^^^^^^
```
and with this change we also get this error:
```
error: method's name is the same as an existing method in a trait
  --> $DIR/same_name_method.rs:75:13
   |
LL |             fn foo() {}
   |             ^^^^^^^^^^^
   |
note: existing `foo` defined here
  --> $DIR/same_name_method.rs:81:9
   |
LL |         impl T2 for S {}
   |
```
I think printing this second argument is reasonable, possibly even preferable to hiding it. And the other cases are similar.

r? `@estebank`
  • Loading branch information
Nadrieril authored Jan 31, 2024
2 parents 9e3bb89 + ae0f0fd commit 7836678
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 2 deletions.
1 change: 1 addition & 0 deletions tests/ui/same_name_method.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ mod should_lint {
impl S {
fn foo() {}
//~^ ERROR: method's name is the same as an existing method in a trait
//~| ERROR: method's name is the same as an existing method in a trait
}

impl T1 for S {}
Expand Down
16 changes: 14 additions & 2 deletions tests/ui/same_name_method.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,22 @@ LL | fn foo() {}
| ^^^^^^^^^^^
|
note: existing `foo` defined here
--> $DIR/same_name_method.rs:79:9
--> $DIR/same_name_method.rs:80:9
|
LL | impl T1 for S {}
| ^^^^^^^^^^^^^^^^

error: aborting due to 5 previous errors
error: method's name is the same as an existing method in a trait
--> $DIR/same_name_method.rs:75:13
|
LL | fn foo() {}
| ^^^^^^^^^^^
|
note: existing `foo` defined here
--> $DIR/same_name_method.rs:82:9
|
LL | impl T2 for S {}
| ^^^^^^^^^^^^^^^^

error: aborting due to 6 previous errors

0 comments on commit 7836678

Please sign in to comment.