Modify error message of importing inherent associated items when #[feature(import_trait_associated_functions)] is enabled#152611
Conversation
`#[feature(import_trait_associated_functions)]` is enabled
rustbot
added
S-waiting-on-review
|
r? @madsmtm rustbot has assigned @madsmtm. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
☔ The latest upstream changes (presumably #152785) made this pull request unmergeable. Please resolve the merge conflicts. |
| //! and one trys to import inherent associated items, the error message is | ||
| //! updated to reflect that only trait associated items can be imported. | ||
| //! | ||
| //! Regression test for <https://github.com/rust-lang/rust/issues/148009>. |
There was a problem hiding this comment.
I think you have to add further annotations to actually test the label? Something like //~| NOTE cannot import inherent associated items, only trait associated items IIRC?
| res.article(), | ||
| res.descr() | ||
| ); | ||
| // Excluding enums since their variants are valid import targets. |
There was a problem hiding this comment.
Could you add a test case for enums too? I'm not suuper convinced that we don't want a different error message there too, but perhaps I'm missing something?
There was a problem hiding this comment.
For structs or unions, I think the :: syntax basically leads to associated items, while for enums, it will basically mean enum variants, and the current error for enums says that "there are no such variants", rather than "this is not a module".
Modifying here will overwrite "there are no such variants" errors, so I excluded it here. Maybe I should make it show both?
There was a problem hiding this comment.
Right, enums are treated as-if they're modules, and you'll get a "no VariantName in EnumName" error pointing to the variant instead.
Which means I don't actually think that matching on DefKind::Enum would change anything? This diagnostic is never shown for enums?
But semantically, I think it makes sense to include DefKind::Enum (and DefKind::ForeignTy) in here, and change the comment to something like:
| // Excluding enums since their variants are valid import targets. | |
| // Show a different error message for items that can have associated items. |
| "cannot import inherent associated items, only trait associated items".to_string() | ||
| } else { | ||
| format!( | ||
| "`{ident}` is {} {}, not a module", |
There was a problem hiding this comment.
We should really be saying "not a module or trait" (or enum) when import_trait_associated_functions is enabled, shouldn't we?
madsmtm
added
S-waiting-on-author
3 participants
Fixes #148009
This PR improves the diagnostic for importing inherent associated items from a struct or union when
#[feature(import_trait_associated_functions)](#134691) is enabled.Previously, this would result in a "not a module" error. This change provides a more specific error message clarifying that only trait associated items can be imported, while inherent associated items remain ineligible for import.
Enums are currently excluded from this change because their variants are valid import targets and require distinct handling.