feat: extend const_is_empty with many kinds of constants

samueltardieu · samueltardieu · commit 288497093b3b · 2024-02-26T08:58:18.000+01:00
diff --git a/clippy_lints/src/methods/is_empty.rs b/clippy_lints/src/methods/is_empty.rs
@@ -1,40 +1,49 @@
-use clippy_utils::diagnostics::span_lint_and_note;
-use clippy_utils::expr_or_init;
-use rustc_ast::LitKind;
-use rustc_hir::{Expr, ExprKind};
+use clippy_utils::consts::constant_is_empty;
+use clippy_utils::diagnostics::span_lint;
+use clippy_utils::{find_binding_init, path_to_local};
+use rustc_hir::{Expr, HirId};
 use rustc_lint::{LateContext, LintContext};
 use rustc_middle::lint::in_external_macro;
-use rustc_span::source_map::Spanned;
+use rustc_span::sym;
 
 use super::CONST_IS_EMPTY;
 
+/// Expression whose initialization depend on a constant conditioned by a `#[cfg(…)]` directive will
+/// not trigger the lint.
 pub(super) fn check(cx: &LateContext<'_>, expr: &'_ Expr<'_>, receiver: &Expr<'_>) {
     if in_external_macro(cx.sess(), expr.span) || !receiver.span.eq_ctxt(expr.span) {
         return;
     }
     let init_expr = expr_or_init(cx, receiver);
-    if let Some(init_is_empty) = is_empty(init_expr)
-        && init_expr.span.eq_ctxt(receiver.span)
-    {
-        span_lint_and_note(
+    if !receiver.span.eq_ctxt(init_expr.span) {
+        return;
+    }
+    if let Some(init_is_empty) = constant_is_empty(cx, init_expr) {
+        span_lint(
             cx,
             CONST_IS_EMPTY,
             expr.span,
             &format!("this expression always evaluates to {init_is_empty:?}"),
-            Some(init_expr.span),
-            "because its initialization value is constant",
         );
     }
 }
 
-fn is_empty(expr: &'_ rustc_hir::Expr<'_>) -> Option<bool> {
-    if let ExprKind::Lit(Spanned { node, .. }) = expr.kind {
-        match node {
-            LitKind::Str(sym, _) => Some(sym.is_empty()),
-            LitKind::ByteStr(value, _) | LitKind::CStr(value, _) => Some(value.is_empty()),
-            _ => None,
-        }
-    } else {
-        None
+fn is_under_cfg(cx: &LateContext<'_>, id: HirId) -> bool {
+    cx.tcx
+        .hir()
+        .parent_id_iter(id)
+        .any(|id| cx.tcx.hir().attrs(id).iter().any(|attr| attr.has_name(sym::cfg)))
+}
+
+/// Similar to [`clippy_utils::expr_or_init`], but does not go up the chain if the initialization
+/// value depends on a `#[cfg(…)]` directive.
+fn expr_or_init<'a, 'b, 'tcx: 'b>(cx: &LateContext<'tcx>, mut expr: &'a Expr<'b>) -> &'a Expr<'b> {
+    while let Some(init) = path_to_local(expr)
+        .and_then(|id| find_binding_init(cx, id))
+        .filter(|init| cx.typeck_results().expr_adjustments(init).is_empty())
+        .filter(|init| !is_under_cfg(cx, init.hir_id))
+    {
+        expr = init;
     }
+    expr
 }
diff --git a/clippy_utils/src/consts.rs b/clippy_utils/src/consts.rs
@@ -10,6 +10,7 @@ use rustc_hir::{BinOp, BinOpKind, Block, ConstBlock, Expr, ExprKind, HirId, Item
 use rustc_lexer::tokenize;
 use rustc_lint::LateContext;
 use rustc_middle::mir::interpret::{alloc_range, Scalar};
+use rustc_middle::mir::ConstValue;
 use rustc_middle::ty::{self, EarlyBinder, FloatTy, GenericArgsRef, IntTy, List, ScalarInt, Ty, TyCtxt, UintTy};
 use rustc_middle::{bug, mir, span_bug};
 use rustc_span::symbol::{Ident, Symbol};
@@ -303,6 +304,12 @@ impl ConstantSource {
     }
 }
 
+/// Attempts to check whether the expression is a constant representing an empty slice, str, array,
+/// etc…
+pub fn constant_is_empty(lcx: &LateContext<'_>, e: &Expr<'_>) -> Option<bool> {
+    ConstEvalLateContext::new(lcx, lcx.typeck_results()).expr_is_empty(e)
+}
+
 /// Attempts to evaluate the expression as a constant.
 pub fn constant<'tcx>(
     lcx: &LateContext<'tcx>,
@@ -402,7 +409,13 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> {
         match e.kind {
             ExprKind::ConstBlock(ConstBlock { body, .. }) => self.expr(self.lcx.tcx.hir().body(body).value),
             ExprKind::DropTemps(e) => self.expr(e),
-            ExprKind::Path(ref qpath) => self.fetch_path(qpath, e.hir_id, self.typeck_results.expr_ty(e)),
+            ExprKind::Path(ref qpath) => {
+                self.fetch_path_and_apply(qpath, e.hir_id, self.typeck_results.expr_ty(e), |this, result| {
+                    let result = mir_to_const(this.lcx, result)?;
+                    this.source = ConstantSource::Constant;
+                    Some(result)
+                })
+            },
             ExprKind::Block(block, _) => self.block(block),
             ExprKind::Lit(lit) => {
                 if is_direct_expn_of(e.span, "cfg").is_some() {
@@ -468,6 +481,39 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> {
         }
     }
 
+    /// Simple constant folding to determine if an expression is an empty slice, str, array, …
+    pub fn expr_is_empty(&mut self, e: &Expr<'_>) -> Option<bool> {
+        match e.kind {
+            ExprKind::ConstBlock(ConstBlock { body, .. }) => self.expr_is_empty(self.lcx.tcx.hir().body(body).value),
+            ExprKind::DropTemps(e) => self.expr_is_empty(e),
+            ExprKind::Path(ref qpath) => {
+                self.fetch_path_and_apply(qpath, e.hir_id, self.typeck_results.expr_ty(e), |this, result| {
+                    mir_is_empty(this.lcx, result)
+                })
+            },
+            ExprKind::Lit(lit) => {
+                if is_direct_expn_of(e.span, "cfg").is_some() {
+                    None
+                } else {
+                    match &lit.node {
+                        LitKind::Str(is, _) => Some(is.is_empty()),
+                        LitKind::ByteStr(s, _) | LitKind::CStr(s, _) => Some(s.is_empty()),
+                        _ => None,
+                    }
+                }
+            },
+            ExprKind::Array(vec) => self.multi(vec).map(|v| v.is_empty()),
+            ExprKind::Repeat(..) => {
+                if let ty::Array(_, n) = self.typeck_results.expr_ty(e).kind() {
+                    Some(n.try_eval_target_usize(self.lcx.tcx, self.lcx.param_env)? == 0)
+                } else {
+                    span_bug!(e.span, "typeck error");
+                }
+            },
+            _ => None,
+        }
+    }
+
     #[expect(clippy::cast_possible_wrap)]
     fn constant_not(&self, o: &Constant<'tcx>, ty: Ty<'_>) -> Option<Constant<'tcx>> {
         use self::Constant::{Bool, Int};
@@ -515,8 +561,11 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> {
         vec.iter().map(|elem| self.expr(elem)).collect::<Option<_>>()
     }
 
-    /// Lookup a possibly constant expression from an `ExprKind::Path`.
-    fn fetch_path(&mut self, qpath: &QPath<'_>, id: HirId, ty: Ty<'tcx>) -> Option<Constant<'tcx>> {
+    /// Lookup a possibly constant expression from an `ExprKind::Path` and apply a function on it.
+    fn fetch_path_and_apply<T, F>(&mut self, qpath: &QPath<'_>, id: HirId, ty: Ty<'tcx>, f: F) -> Option<T>
+    where
+        F: FnOnce(&mut Self, rustc_middle::mir::Const<'tcx>) -> Option<T>,
+    {
         let res = self.typeck_results.qpath_res(qpath, id);
         match res {
             Res::Def(DefKind::Const | DefKind::AssocConst, def_id) => {
@@ -549,9 +598,7 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> {
                     .const_eval_resolve(self.param_env, mir::UnevaluatedConst::new(def_id, args), None)
                     .ok()
                     .map(|val| rustc_middle::mir::Const::from_value(val, ty))?;
-                let result = mir_to_const(self.lcx, result)?;
-                self.source = ConstantSource::Constant;
-                Some(result)
+                f(self, result)
             },
             _ => None,
         }
@@ -742,7 +789,6 @@ impl<'a, 'tcx> ConstEvalLateContext<'a, 'tcx> {
 }
 
 pub fn mir_to_const<'tcx>(lcx: &LateContext<'tcx>, result: mir::Const<'tcx>) -> Option<Constant<'tcx>> {
-    use rustc_middle::mir::ConstValue;
     let mir::Const::Val(val, _) = result else {
         // We only work on evaluated consts.
         return None;
@@ -788,6 +834,42 @@ pub fn mir_to_const<'tcx>(lcx: &LateContext<'tcx>, result: mir::Const<'tcx>) ->
     }
 }
 
+fn mir_is_empty<'tcx>(lcx: &LateContext<'tcx>, result: mir::Const<'tcx>) -> Option<bool> {
+    let mir::Const::Val(val, _) = result else {
+        // We only work on evaluated consts.
+        return None;
+    };
+    match (val, result.ty().kind()) {
+        (_, ty::Ref(_, inner_ty, _)) => match inner_ty.kind() {
+            ty::Str | ty::Slice(_) => {
+                if let ConstValue::Indirect { alloc_id, offset } = val {
+                    // Get the length from the slice, using the same formula as
+                    // [`ConstValue::try_get_slice_bytes_for_diagnostics`].
+                    let a = lcx.tcx.global_alloc(alloc_id).unwrap_memory().inner();
+                    let ptr_size = lcx.tcx.data_layout.pointer_size;
+                    if a.size() < offset + 2 * ptr_size {
+                        // (partially) dangling reference
+                        return None;
+                    }
+                    let len = a
+                        .read_scalar(&lcx.tcx, alloc_range(offset + ptr_size, ptr_size), false)
+                        .ok()?
+                        .to_target_usize(&lcx.tcx)
+                        .ok()?;
+                    Some(len == 0)
+                } else {
+                    None
+                }
+            },
+            ty::Array(_, len) => Some(len.try_to_target_usize(lcx.tcx)? == 0),
+            _ => None,
+        },
+        (ConstValue::Indirect { .. }, ty::Array(_, len)) => Some(len.try_to_target_usize(lcx.tcx)? == 0),
+        (ConstValue::ZeroSized, _) => Some(true),
+        _ => None,
+    }
+}
+
 fn field_of_struct<'tcx>(
     adt_def: ty::AdtDef<'tcx>,
     lcx: &LateContext<'tcx>,
diff --git a/tests/ui/const_is_empty.rs b/tests/ui/const_is_empty.rs
@@ -38,6 +38,55 @@ fn test_propagated() {
     }
 }
 
+const EMPTY_STR: &str = "";
+const NON_EMPTY_STR: &str = "foo";
+const EMPTY_BSTR: &[u8] = b"";
+const NON_EMPTY_BSTR: &[u8] = b"foo";
+const EMPTY_U8_SLICE: &[u8] = &[];
+const NON_EMPTY_U8_SLICE: &[u8] = &[1, 2];
+const EMPTY_SLICE: &[u32] = &[];
+const NON_EMPTY_SLICE: &[u32] = &[1, 2];
+const NON_EMPTY_SLICE_REPEAT: &[u32] = &[1; 2];
+const EMPTY_ARRAY: [u32; 0] = [];
+const EMPTY_ARRAY_REPEAT: [u32; 0] = [1; 0];
+const NON_EMPTY_ARRAY: [u32; 2] = [1, 2];
+const NON_EMPTY_ARRAY_REPEAT: [u32; 2] = [1; 2];
+const EMPTY_REF_ARRAY: &[u32; 0] = &[];
+const NON_EMPTY_REF_ARRAY: &[u32; 3] = &[1, 2, 3];
+
+fn test_from_const() {
+    let _ = EMPTY_STR.is_empty();
+    //~^ ERROR: this expression always evaluates to true
+    let _ = NON_EMPTY_STR.is_empty();
+    //~^ ERROR: this expression always evaluates to false
+    let _ = EMPTY_BSTR.is_empty();
+    //~^ ERROR: this expression always evaluates to true
+    let _ = NON_EMPTY_BSTR.is_empty();
+    //~^ ERROR: this expression always evaluates to false
+    let _ = EMPTY_ARRAY.is_empty();
+    //~^ ERROR: this expression always evaluates to true
+    let _ = EMPTY_ARRAY_REPEAT.is_empty();
+    //~^ ERROR: this expression always evaluates to true
+    let _ = EMPTY_U8_SLICE.is_empty();
+    //~^ ERROR: this expression always evaluates to true
+    let _ = NON_EMPTY_U8_SLICE.is_empty();
+    //~^ ERROR: this expression always evaluates to false
+    let _ = NON_EMPTY_ARRAY.is_empty();
+    //~^ ERROR: this expression always evaluates to false
+    let _ = NON_EMPTY_ARRAY_REPEAT.is_empty();
+    //~^ ERROR: this expression always evaluates to false
+    let _ = EMPTY_REF_ARRAY.is_empty();
+    //~^ ERROR: this expression always evaluates to true
+    let _ = NON_EMPTY_REF_ARRAY.is_empty();
+    //~^ ERROR: this expression always evaluates to false
+    let _ = EMPTY_SLICE.is_empty();
+    //~^ ERROR: this expression always evaluates to true
+    let _ = NON_EMPTY_SLICE.is_empty();
+    //~^ ERROR: this expression always evaluates to false
+    let _ = NON_EMPTY_SLICE_REPEAT.is_empty();
+    //~^ ERROR: this expression always evaluates to false
+}
+
 fn main() {
     let value = "foobar";
     let _ = value.is_empty();
diff --git a/tests/ui/const_is_empty.stderr b/tests/ui/const_is_empty.stderr
