Auto merge of #12840 - tesuji:const-asserts, r=llogiq

Don't lint `assertions_on_constants` on any const assertions

close #12816
close #12847
cc #12817

----

changelog: Fix false positives in consts for `assertions_on_constants` and `unnecessary_operation`.

Author: bors

clippy_lints/src/assertions_on_constants.rs

@@ -1,7 +1,8 @@
-use clippy_utils::consts::{constant_with_source, Constant, ConstantSource};
+use clippy_utils::consts::{constant, Constant};
 use clippy_utils::diagnostics::span_lint_and_help;
+use clippy_utils::is_inside_always_const_context;
 use clippy_utils::macros::{find_assert_args, root_macro_call_first_node, PanicExpn};
-use rustc_hir::{Expr, Item, ItemKind, Node};
+use rustc_hir::{Expr, ExprKind};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::declare_lint_pass;
 use rustc_span::sym;
@@ -42,17 +43,16 @@ impl<'tcx> LateLintPass<'tcx> for AssertionsOnConstants {
         let Some((condition, panic_expn)) = find_assert_args(cx, e, macro_call.expn) else {
             return;
         };
-        let Some((Constant::Bool(val), source)) = constant_with_source(cx, cx.typeck_results(), condition) else {
+        let Some(Constant::Bool(val)) = constant(cx, cx.typeck_results(), condition) else {
             return;
         };
-        if let ConstantSource::Constant = source
-            && let Node::Item(Item {
-                kind: ItemKind::Const(..),
-                ..
-            }) = cx.tcx.parent_hir_node(e.hir_id)
-        {
-            return;
+
+        match condition.kind {
+            ExprKind::Path(..) | ExprKind::Lit(_) => {},
+            _ if is_inside_always_const_context(cx.tcx, e.hir_id) => return,
+            _ => {},
         }
+
         if val {
             span_lint_and_help(
                 cx,

clippy_lints/src/default_numeric_fallback.rs

@@ -50,6 +50,8 @@ declare_lint_pass!(DefaultNumericFallback => [DEFAULT_NUMERIC_FALLBACK]);
 impl<'tcx> LateLintPass<'tcx> for DefaultNumericFallback {
     fn check_body(&mut self, cx: &LateContext<'tcx>, body: &Body<'tcx>) {
         let hir = cx.tcx.hir();
+        // NOTE: this is different from `clippy_utils::is_inside_always_const_context`.
+        // Inline const supports type inference.
         let is_parent_const = matches!(
             hir.body_const_context(hir.body_owner_def_id(body.id())),
             Some(ConstContext::Const { inline: false } | ConstContext::Static(_))

clippy_lints/src/no_effect.rs

@@ -1,7 +1,9 @@
 use clippy_utils::diagnostics::{span_lint_hir, span_lint_hir_and_then};
 use clippy_utils::source::snippet_opt;
 use clippy_utils::ty::has_drop;
-use clippy_utils::{any_parent_is_automatically_derived, is_lint_allowed, path_to_local, peel_blocks};
+use clippy_utils::{
+    any_parent_is_automatically_derived, is_inside_always_const_context, is_lint_allowed, path_to_local, peel_blocks,
+};
 use rustc_errors::Applicability;
 use rustc_hir::def::{DefKind, Res};
 use rustc_hir::{
@@ -258,13 +260,16 @@ fn has_no_effect(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
 
 fn check_unnecessary_operation(cx: &LateContext<'_>, stmt: &Stmt<'_>) {
     if let StmtKind::Semi(expr) = stmt.kind
+        && !in_external_macro(cx.sess(), stmt.span)
         && let ctxt = stmt.span.ctxt()
         && expr.span.ctxt() == ctxt
         && let Some(reduced) = reduce_expression(cx, expr)
-        && !in_external_macro(cx.sess(), stmt.span)
         && reduced.iter().all(|e| e.span.ctxt() == ctxt)
     {
         if let ExprKind::Index(..) = &expr.kind {
+            if is_inside_always_const_context(cx.tcx, expr.hir_id) {
+                return;
+            }
             let snippet =
                 if let (Some(arr), Some(func)) = (snippet_opt(cx, reduced[0].span), snippet_opt(cx, reduced[1].span)) {
                     format!("assert!({}.len() > {});", &arr, &func)

clippy_utils/src/lib.rs

@@ -101,10 +101,10 @@ use rustc_hir::hir_id::{HirIdMap, HirIdSet};
 use rustc_hir::intravisit::{walk_expr, FnKind, Visitor};
 use rustc_hir::LangItem::{OptionNone, OptionSome, ResultErr, ResultOk};
 use rustc_hir::{
-    self as hir, def, Arm, ArrayLen, BindingMode, Block, BlockCheckMode, Body, ByRef, Closure, Destination, Expr,
-    ExprField, ExprKind, FnDecl, FnRetTy, GenericArgs, HirId, Impl, ImplItem, ImplItemKind, ImplItemRef, Item,
-    ItemKind, LangItem, LetStmt, MatchSource, Mutability, Node, OwnerId, Param, Pat, PatKind, Path, PathSegment,
-    PrimTy, QPath, Stmt, StmtKind, TraitItem, TraitItemKind, TraitItemRef, TraitRef, TyKind, UnOp,
+    self as hir, def, Arm, ArrayLen, BindingMode, Block, BlockCheckMode, Body, ByRef, Closure, ConstContext,
+    Destination, Expr, ExprField, ExprKind, FnDecl, FnRetTy, GenericArgs, HirId, Impl, ImplItem, ImplItemKind,
+    ImplItemRef, Item, ItemKind, LangItem, LetStmt, MatchSource, Mutability, Node, OwnerId, Param, Pat, PatKind, Path,
+    PathSegment, PrimTy, QPath, Stmt, StmtKind, TraitItem, TraitItemKind, TraitItemRef, TraitRef, TyKind, UnOp,
 };
 use rustc_lexer::{tokenize, TokenKind};
 use rustc_lint::{LateContext, Level, Lint, LintContext};
@@ -209,7 +209,10 @@ pub fn local_is_initialized(cx: &LateContext<'_>, local: HirId) -> bool {
     false
 }
 
-/// Returns `true` if the given `NodeId` is inside a constant context
+/// Returns `true` if the given `HirId` is inside a constant context.
+///
+/// This is the same as `is_inside_always_const_context`, but also includes
+/// `const fn`.
 ///
 /// # Example
 ///
@@ -222,6 +225,24 @@ pub fn in_constant(cx: &LateContext<'_>, id: HirId) -> bool {
     cx.tcx.hir().is_inside_const_context(id)
 }
 
+/// Returns `true` if the given `HirId` is inside an always constant context.
+///
+/// This context includes:
+///  * const/static items
+///  * const blocks (or inline consts)
+///  * associated constants
+pub fn is_inside_always_const_context(tcx: TyCtxt<'_>, hir_id: HirId) -> bool {
+    use ConstContext::{Const, ConstFn, Static};
+    let hir = tcx.hir();
+    let Some(ctx) = hir.body_const_context(hir.enclosing_body_owner(hir_id)) else {
+        return false;
+    };
+    match ctx {
+        ConstFn => false,
+        Static(_) | Const { inline: _ } => true,
+    }
+}
+
 /// Checks if a `Res` refers to a constructor of a `LangItem`
 /// For example, use this to check whether a function call or a pattern is `Some(..)`.
 pub fn is_res_lang_ctor(cx: &LateContext<'_>, res: Res, lang_item: LangItem) -> bool {

tests/ui/assertions_on_constants.rs

@@ -1,4 +1,4 @@
-#![allow(non_fmt_panics, clippy::needless_bool)]
+#![allow(non_fmt_panics, clippy::needless_bool, clippy::eq_op)]
 
 macro_rules! assert_const {
     ($len:expr) => {
@@ -49,7 +49,16 @@ fn main() {
     const _: () = assert!(true);
     //~^ ERROR: `assert!(true)` will be optimized out by the compiler
 
+    assert!(8 == (7 + 1));
+    //~^ ERROR: `assert!(true)` will be optimized out by the compiler
+
     // Don't lint if the value is dependent on a defined constant:
     const N: usize = 1024;
     const _: () = assert!(N.is_power_of_two());
 }
+
+const _: () = {
+    assert!(true);
+    //~^ ERROR: `assert!(true)` will be optimized out by the compiler
+    assert!(8 == (7 + 1));
+};

tests/ui/assertions_on_constants.stderr

@@ -80,5 +80,21 @@ LL |     const _: () = assert!(true);
    |
    = help: remove it
 
-error: aborting due to 10 previous errors
+error: `assert!(true)` will be optimized out by the compiler
+  --> tests/ui/assertions_on_constants.rs:52:5
+   |
+LL |     assert!(8 == (7 + 1));
+   |     ^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: remove it
+
+error: `assert!(true)` will be optimized out by the compiler
+  --> tests/ui/assertions_on_constants.rs:61:5
+   |
+LL |     assert!(true);
+   |     ^^^^^^^^^^^^^
+   |
+   = help: remove it
+
+error: aborting due to 12 previous errors
 

tests/ui/unnecessary_operation.fixed

@@ -43,7 +43,7 @@ fn get_number() -> i32 {
     0
 }
 
-fn get_usize() -> usize {
+const fn get_usize() -> usize {
     0
 }
 fn get_struct() -> Struct {
@@ -113,4 +113,16 @@ fn main() {
     'label: {
         break 'label
     };
+    let () = const {
+        [42, 55][get_usize()];
+    };
+}
+
+const _: () = {
+    [42, 55][get_usize()];
+};
+
+const fn foo() {
+    assert!([42, 55].len() > get_usize());
+    //~^ ERROR: unnecessary operation
 }

tests/ui/unnecessary_operation.rs

@@ -43,7 +43,7 @@ fn get_number() -> i32 {
     0
 }
 
-fn get_usize() -> usize {
+const fn get_usize() -> usize {
     0
 }
 fn get_struct() -> Struct {
@@ -117,4 +117,16 @@ fn main() {
     'label: {
         break 'label
     };
+    let () = const {
+        [42, 55][get_usize()];
+    };
+}
+
+const _: () = {
+    [42, 55][get_usize()];
+};
+
+const fn foo() {
+    [42, 55][get_usize()];
+    //~^ ERROR: unnecessary operation
 }

tests/ui/unnecessary_operation.stderr

@@ -119,5 +119,11 @@ LL | |         s: String::from("blah"),
 LL | |     };
    | |______^ help: statement can be reduced to: `String::from("blah");`
 
-error: aborting due to 19 previous errors
+error: unnecessary operation
+  --> tests/ui/unnecessary_operation.rs:130:5
+   |
+LL |     [42, 55][get_usize()];
+   |     ^^^^^^^^^^^^^^^^^^^^^^ help: statement can be written as: `assert!([42, 55].len() > get_usize());`
+
+error: aborting due to 20 previous errors