Auto merge of #124369 - flip1995:clippy-backport, r=Mark-Simulacrum

[beta] Clippy backport

r? `@Mark-Simulacrum`

Backports:

- rust-lang/rust-clippy#12486
- rust-lang/rust-clippy#12572
- rust-lang/rust-clippy#12508
- rust-lang/rust-clippy#12617

The first one is a bit bigger as usual for a backport. But it fixes a major issue with this lint that we overlooked. So I think this is worth it. After that was merged into nightly, there were no new issues opened about this lint, so IMO this is safe to backport to `beta` and put into stable.

src/tools/clippy/clippy_lints/src/attrs/mixed_attributes_style.rs

@@ -1,30 +1,85 @@
 use super::MIXED_ATTRIBUTES_STYLE;
 use clippy_utils::diagnostics::span_lint;
-use rustc_ast::AttrStyle;
-use rustc_lint::EarlyContext;
+use rustc_ast::{AttrKind, AttrStyle, Attribute};
+use rustc_data_structures::fx::FxHashSet;
+use rustc_data_structures::sync::Lrc;
+use rustc_lint::{LateContext, LintContext};
+use rustc_span::source_map::SourceMap;
+use rustc_span::{SourceFile, Span, Symbol};
 
-pub(super) fn check(cx: &EarlyContext<'_>, item: &rustc_ast::Item) {
-    let mut has_outer = false;
-    let mut has_inner = false;
+#[derive(Hash, PartialEq, Eq)]
+enum SimpleAttrKind {
+    Doc,
+    /// A normal attribute, with its name symbols.
+    Normal(Vec<Symbol>),
+}
+
+impl From<&AttrKind> for SimpleAttrKind {
+    fn from(value: &AttrKind) -> Self {
+        match value {
+            AttrKind::Normal(attr) => {
+                let path_symbols = attr
+                    .item
+                    .path
+                    .segments
+                    .iter()
+                    .map(|seg| seg.ident.name)
+                    .collect::<Vec<_>>();
+                Self::Normal(path_symbols)
+            },
+            AttrKind::DocComment(..) => Self::Doc,
+        }
+    }
+}
+
+pub(super) fn check(cx: &LateContext<'_>, item_span: Span, attrs: &[Attribute]) {
+    let mut inner_attr_kind: FxHashSet<SimpleAttrKind> = FxHashSet::default();
+    let mut outer_attr_kind: FxHashSet<SimpleAttrKind> = FxHashSet::default();
+
+    let source_map = cx.sess().source_map();
+    let item_src = source_map.lookup_source_file(item_span.lo());
 
-    for attr in &item.attrs {
-        if attr.span.from_expansion() {
+    for attr in attrs {
+        if attr.span.from_expansion() || !attr_in_same_src_as_item(source_map, &item_src, attr.span) {
             continue;
         }
+
+        let kind: SimpleAttrKind = (&attr.kind).into();
         match attr.style {
-            AttrStyle::Inner => has_inner = true,
-            AttrStyle::Outer => has_outer = true,
-        }
+            AttrStyle::Inner => {
+                if outer_attr_kind.contains(&kind) {
+                    lint_mixed_attrs(cx, attrs);
+                    return;
+                }
+                inner_attr_kind.insert(kind);
+            },
+            AttrStyle::Outer => {
+                if inner_attr_kind.contains(&kind) {
+                    lint_mixed_attrs(cx, attrs);
+                    return;
+                }
+                outer_attr_kind.insert(kind);
+            },
+        };
     }
-    if !has_outer || !has_inner {
+}
+
+fn lint_mixed_attrs(cx: &LateContext<'_>, attrs: &[Attribute]) {
+    let mut attrs_iter = attrs.iter().filter(|attr| !attr.span.from_expansion());
+    let span = if let (Some(first), Some(last)) = (attrs_iter.next(), attrs_iter.last()) {
+        first.span.with_hi(last.span.hi())
+    } else {
         return;
-    }
-    let mut attrs_iter = item.attrs.iter().filter(|attr| !attr.span.from_expansion());
-    let span = attrs_iter.next().unwrap().span;
+    };
     span_lint(
         cx,
         MIXED_ATTRIBUTES_STYLE,
-        span.with_hi(attrs_iter.last().unwrap().span.hi()),
+        span,
         "item has both inner and outer attributes",
     );
 }
+
+fn attr_in_same_src_as_item(source_map: &SourceMap, item_src: &Lrc<SourceFile>, attr_span: Span) -> bool {
+    let attr_src = source_map.lookup_source_file(attr_span.lo());
+    Lrc::ptr_eq(item_src, &attr_src)
+}

src/tools/clippy/clippy_lints/src/attrs/mod.rs

@@ -464,10 +464,20 @@ declare_clippy_lint! {
 
 declare_clippy_lint! {
     /// ### What it does
-    /// Checks that an item has only one kind of attributes.
+    /// Checks for items that have the same kind of attributes with mixed styles (inner/outer).
     ///
     /// ### Why is this bad?
-    /// Having both kinds of attributes makes it more complicated to read code.
+    /// Having both style of said attributes makes it more complicated to read code.
+    ///
+    /// ### Known problems
+    /// This lint currently has false-negatives when mixing same attributes
+    /// but they have different path symbols, for example:
+    /// ```ignore
+    /// #[custom_attribute]
+    /// pub fn foo() {
+    ///     #![my_crate::custom_attribute]
+    /// }
+    /// ```
     ///
     /// ### Example
     /// ```no_run
@@ -485,7 +495,7 @@ declare_clippy_lint! {
     /// ```
     #[clippy::version = "1.78.0"]
     pub MIXED_ATTRIBUTES_STYLE,
-    suspicious,
+    style,
     "item has both inner and outer attributes"
 }
 
@@ -496,6 +506,7 @@ declare_lint_pass!(Attributes => [
     USELESS_ATTRIBUTE,
     BLANKET_CLIPPY_RESTRICTION_LINTS,
     SHOULD_PANIC_WITHOUT_EXPECT,
+    MIXED_ATTRIBUTES_STYLE,
 ]);
 
 impl<'tcx> LateLintPass<'tcx> for Attributes {
@@ -539,6 +550,7 @@ impl<'tcx> LateLintPass<'tcx> for Attributes {
             ItemKind::ExternCrate(..) | ItemKind::Use(..) => useless_attribute::check(cx, item, attrs),
             _ => {},
         }
+        mixed_attributes_style::check(cx, item.span, attrs);
     }
 
     fn check_impl_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx ImplItem<'_>) {
@@ -567,13 +579,11 @@ impl_lint_pass!(EarlyAttributes => [
     MAYBE_MISUSED_CFG,
     DEPRECATED_CLIPPY_CFG_ATTR,
     UNNECESSARY_CLIPPY_CFG,
-    MIXED_ATTRIBUTES_STYLE,
 ]);
 
 impl EarlyLintPass for EarlyAttributes {
     fn check_item(&mut self, cx: &EarlyContext<'_>, item: &rustc_ast::Item) {
         empty_line_after::check(cx, item);
-        mixed_attributes_style::check(cx, item);
     }
 
     fn check_attribute(&mut self, cx: &EarlyContext<'_>, attr: &Attribute) {

src/tools/clippy/clippy_lints/src/casts/cast_sign_loss.rs

@@ -118,7 +118,7 @@ enum Sign {
     Uncertain,
 }
 
-fn expr_sign<'cx>(cx: &LateContext<'cx>, expr: &Expr<'_>, ty: impl Into<Option<Ty<'cx>>>) -> Sign {
+fn expr_sign<'cx, 'tcx>(cx: &LateContext<'cx>, mut expr: &'tcx Expr<'tcx>, ty: impl Into<Option<Ty<'cx>>>) -> Sign {
     // Try evaluate this expr first to see if it's positive
     if let Some(val) = get_const_signed_int_eval(cx, expr, ty) {
         return if val >= 0 { Sign::ZeroOrPositive } else { Sign::Negative };
@@ -134,11 +134,12 @@ fn expr_sign<'cx>(cx: &LateContext<'cx>, expr: &Expr<'_>, ty: impl Into<Option<T
         // Peel unwrap(), expect(), etc.
         while let Some(&found_name) = METHODS_UNWRAP.iter().find(|&name| &method_name == name)
             && let Some(arglist) = method_chain_args(expr, &[found_name])
-            && let ExprKind::MethodCall(inner_path, ..) = &arglist[0].0.kind
+            && let ExprKind::MethodCall(inner_path, recv, ..) = &arglist[0].0.kind
         {
             // The original type has changed, but we can't use `ty` here anyway, because it has been
             // moved.
             method_name = inner_path.ident.name.as_str();
+            expr = recv;
         }
 
         if METHODS_POW.iter().any(|&name| method_name == name)

src/tools/clippy/clippy_lints/src/casts/ptr_as_ptr.rs

@@ -62,8 +62,8 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, msrv: &Msrv) {
         // we omit following `cast`:
         let omit_cast = if let ExprKind::Call(func, []) = cast_expr.kind
             && let ExprKind::Path(ref qpath @ QPath::Resolved(None, path)) = func.kind
+            && let Some(method_defid) = path.res.opt_def_id()
         {
-            let method_defid = path.res.def_id();
             if cx.tcx.is_diagnostic_item(sym::ptr_null, method_defid) {
                 OmitFollowedCastReason::Null(qpath)
             } else if cx.tcx.is_diagnostic_item(sym::ptr_null_mut, method_defid) {

src/tools/clippy/tests/ui/cast.rs

@@ -9,7 +9,12 @@
     clippy::cast_sign_loss,
     clippy::cast_possible_wrap
 )]
-#![allow(clippy::cast_abs_to_unsigned, clippy::no_effect, clippy::unnecessary_operation)]
+#![allow(
+    clippy::cast_abs_to_unsigned,
+    clippy::no_effect,
+    clippy::unnecessary_operation,
+    clippy::unnecessary_literal_unwrap
+)]
 
 fn main() {
     // Test clippy::cast_precision_loss
@@ -457,3 +462,8 @@ fn issue11642() {
         //~^ ERROR: casting `i32` to `u32` may lose the sign of the value
     }
 }
+
+fn issue12506() -> usize {
+    let bar: Result<Option<i64>, u32> = Ok(Some(10));
+    bar.unwrap().unwrap() as usize
+}