Overhaul and document clippy_utils::attrs
#15763
Conversation
rustbot
added
the
S-waiting-on-review
rustbot
removed
the
S-waiting-on-review
rustbot
added
the
S-waiting-on-review
ada4a
force-pushed
the
utils-attrs
branch
2 times, most recently
from
8751cc0 to
92c8d70
Compare
clippy_utils/src/attrs.rs
Outdated
| /// Attribute is deprecated and was replaced by the named attribute | ||
| Replaced(&'static str), | ||
| /// Attribute is deprecated and was possibly replaced by the named attribute | ||
| Deprecated(Option<&'static str>), |
There was a problem hiding this comment.
This should just be Renamed without an option. If we ever actually deprecate an attribute it will have an associated message.
There was a problem hiding this comment.
At least currently, the &str refers to the new name for the attribute, not the associated message. Or did I misunderstand you?
There was a problem hiding this comment.
The current variant should be Renamed (or its old name) and Deprecated shouldn't exist. Really this whole thing should be removed as one of my later comments mentioned. Once the array is inlined as a match this type has no reason to exist.
clippy_utils/src/attrs.rs
Outdated
| let Some((_, deprecation_status)) = BUILTIN_ATTRIBUTES | ||
| .iter() | ||
| .find_map(|(builtin_name, deprecation_status)| { | ||
| if attr_segments[1].name == *builtin_name { | ||
| Some(deprecation_status) | ||
| } else { | ||
| None | ||
| } | ||
| }) | ||
| .map_or_else( | ||
| || { | ||
| sess.dcx().span_err(attr_segments[1].span, "usage of unknown attribute"); | ||
| false | ||
| }, | ||
| |deprecation_status| { | ||
| let mut diag = sess | ||
| .dcx() | ||
| .struct_span_err(attr_segments[1].span, "usage of deprecated attribute"); | ||
| match *deprecation_status { | ||
| DeprecationStatus::Deprecated => { | ||
| diag.emit(); | ||
| false | ||
| }, | ||
| DeprecationStatus::Replaced(new_name) => { | ||
| diag.span_suggestion( | ||
| attr_segments[1].span, | ||
| "consider using", | ||
| new_name, | ||
| Applicability::MachineApplicable, | ||
| ); | ||
| diag.emit(); | ||
| false | ||
| }, | ||
| DeprecationStatus::None => { | ||
| diag.cancel(); | ||
| attr_segments[1].name == name | ||
| }, | ||
| } | ||
| }, | ||
| ) | ||
| .find(|(builtin_name, _)| segment2.name == *builtin_name) |
There was a problem hiding this comment.
BUILTIN_ATTRIBUTES should be inlined as a match. This made sense when the names were strings, but since they aren't any more a match over the symbol will be faster.
|
@rustbot ready |
Their meanings, and the way they're handled in `get_attr`, are pretty similar
- Move it and its helper function `parse_attrs` together to the end of the file, because it's surprising to see front-and-center a struct that's only really used in one place (`cognitive_complexity`). - Avoid panic path in `LimitStack::limit` - Replace `assert` with `debug_assert` to avoid panics in release builds
| pub fn get_unique_attr<'a, A: AttributeExt>(sess: &'a Session, attrs: &'a [A], name: Symbol) -> Option<&'a A> { | ||
| /// If `attrs` contain exactly one instance of a built-in Clippy attribute called `name`, | ||
| /// returns that attribute, and `None` otherwise | ||
| pub fn get_unique_builtin_attr<'a, A: AttributeExt>(sess: &'a Session, attrs: &'a [A], name: Symbol) -> Option<&'a A> { |
There was a problem hiding this comment.
While we're at it, I wonder whether this function should even exist as it is now. It emits a warning on duplicate attributes, but that should already be covered by duplicated_attributes; well, maybe apart for #[cognitive_complexity(N)] -- if the N is different, that lint wouldn't fire, but this check will.
So maybe its uses could be simplified to just get_builtin_attr.next_back()
There was a problem hiding this comment.
That would be better, but I think this also warns on the renamed attributes when both names appear. I would be surprised if that ever actually happens in practice.
For #15569
changelog: none