From 832c25d565c99e932ba3748b8402a0481f7e1538 Mon Sep 17 00:00:00 2001 From: sladynnunes Date: Sun, 28 May 2023 02:22:22 -0700 Subject: [PATCH 1/7] This commit modifies the rendering of constant values in rustdoc. Previously, non-literal expressions (like `50 + 50`) would be printed alongside their value as a comment (like ` = _; // 100i32`). This approach often resulted in confusing documentation, especially for complex expressions. To improve clarity, this commit changes the rendering to display the evaluated value of the expression directly (like ` = 100i32;`) unless the constant is a literal expression. The original expression is still included as a comment if it doesn't match the evaluated value. These changes aim to improve the readability and usefulness of constant value documentation generated by rustdoc. --- src/librustdoc/html/render/print_item.rs | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/src/librustdoc/html/render/print_item.rs b/src/librustdoc/html/render/print_item.rs index 62027a3fa1941..702e1de308d5b 100644 --- a/src/librustdoc/html/render/print_item.rs +++ b/src/librustdoc/html/render/print_item.rs @@ -1474,22 +1474,17 @@ fn item_constant(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, c: &cle typ = c.type_.print(cx), ); - // FIXME: The code below now prints - // ` = _; // 100i32` - // if the expression is - // `50 + 50` - // which looks just wrong. - // Should we print - // ` = 100i32;` - // instead? - let value = c.value(tcx); let is_literal = c.is_literal(tcx); let expr = c.expr(tcx); if value.is_some() || is_literal { - write!(w, " = {expr};", expr = Escape(&expr)); - } else { - w.write_str(";"); + if is_literal { + write!(w, " = {expr};", expr = Escape(&expr)); + } else if let Some(value) = &value { + write!(w, " = {value};", value = Escape(value)); + } else { + w.write_str(";"); + } } if !is_literal { From 84795b587200ffb44612dd5423ee7294400f190c Mon Sep 17 00:00:00 2001 From: sladynnunes Date: Wed, 31 May 2023 00:35:48 -0700 Subject: [PATCH 2/7] Commit suggestions --- src/librustdoc/html/render/print_item.rs | 31 ++++++++++-------------- 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/src/librustdoc/html/render/print_item.rs b/src/librustdoc/html/render/print_item.rs index 054332609802c..051254d02e6d7 100644 --- a/src/librustdoc/html/render/print_item.rs +++ b/src/librustdoc/html/render/print_item.rs @@ -1477,27 +1477,22 @@ fn item_constant(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, c: &cle let value = c.value(tcx); let is_literal = c.is_literal(tcx); let expr = c.expr(tcx); - if value.is_some() || is_literal { - if is_literal { - write!(w, " = {expr};", expr = Escape(&expr)); - } else if let Some(value) = &value { - write!(w, " = {value};", value = Escape(value)); - } else { - w.write_str(";"); - } + + if is_literal { + write!(w, " = {expr};", expr = Escape(&expr)); + } else if let Some(value) = &value { + write!(w, " = {value};", value = Escape(value)); + } else { + w.write_str(";"); } - if !is_literal { - if let Some(value) = &value { - let value_lowercase = value.to_lowercase(); - let expr_lowercase = expr.to_lowercase(); + let value_lowercase = value.to_lowercase(); + let expr_lowercase = expr.to_lowercase(); - if value_lowercase != expr_lowercase - && value_lowercase.trim_end_matches("i32") != expr_lowercase - { - write!(w, " // {value}", value = Escape(value)); - } - } + if value_lowercase != expr_lowercase + && value_lowercase.trim_end_matches("i32") != expr_lowercase + { + write!(w, " // {value}", value = Escape(value)); } }); From 5043a122a7c76e1f40530198be1190f7a1cce0e8 Mon Sep 17 00:00:00 2001 From: sladynnunes Date: Wed, 31 May 2023 01:19:50 -0700 Subject: [PATCH 3/7] Fix tests --- src/librustdoc/html/render/print_item.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustdoc/html/render/print_item.rs b/src/librustdoc/html/render/print_item.rs index 051254d02e6d7..2081891259167 100644 --- a/src/librustdoc/html/render/print_item.rs +++ b/src/librustdoc/html/render/print_item.rs @@ -1480,13 +1480,13 @@ fn item_constant(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, c: &cle if is_literal { write!(w, " = {expr};", expr = Escape(&expr)); - } else if let Some(value) = &value { + } else if let Some(ref value) = &value { write!(w, " = {value};", value = Escape(value)); } else { w.write_str(";"); } - let value_lowercase = value.to_lowercase(); + let value_lowercase = value.as_ref().map(|s| s.to_lowercase()); let expr_lowercase = expr.to_lowercase(); if value_lowercase != expr_lowercase From 08eec71070d4760d76f9426a590f985a9fbe9dd4 Mon Sep 17 00:00:00 2001 From: sladynnunes Date: Wed, 31 May 2023 01:40:48 -0700 Subject: [PATCH 4/7] Fix tests --- src/librustdoc/html/render/print_item.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/librustdoc/html/render/print_item.rs b/src/librustdoc/html/render/print_item.rs index 2081891259167..2710d8055418e 100644 --- a/src/librustdoc/html/render/print_item.rs +++ b/src/librustdoc/html/render/print_item.rs @@ -1480,19 +1480,21 @@ fn item_constant(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, c: &cle if is_literal { write!(w, " = {expr};", expr = Escape(&expr)); - } else if let Some(ref value) = &value { + } else if let Some(ref value) = value { write!(w, " = {value};", value = Escape(value)); } else { w.write_str(";"); } let value_lowercase = value.as_ref().map(|s| s.to_lowercase()); - let expr_lowercase = expr.to_lowercase(); + let expr_lowercase = Some(expr.to_lowercase()); if value_lowercase != expr_lowercase - && value_lowercase.trim_end_matches("i32") != expr_lowercase + && value_lowercase.as_ref().map(|s| s.trim_end_matches("i32")) != expr_lowercase { - write!(w, " // {value}", value = Escape(value)); + if let Some(ref value) = value { + write!(w, " // {value}", value = Escape(value.as_str())); + } } }); From f11b8dcbeecf32aa85cfe8426cf27896daceb6df Mon Sep 17 00:00:00 2001 From: sladynnunes Date: Wed, 31 May 2023 01:54:54 -0700 Subject: [PATCH 5/7] Fix tests --- src/librustdoc/html/render/print_item.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustdoc/html/render/print_item.rs b/src/librustdoc/html/render/print_item.rs index 2710d8055418e..fe00f5adee9e6 100644 --- a/src/librustdoc/html/render/print_item.rs +++ b/src/librustdoc/html/render/print_item.rs @@ -1490,7 +1490,7 @@ fn item_constant(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, c: &cle let expr_lowercase = Some(expr.to_lowercase()); if value_lowercase != expr_lowercase - && value_lowercase.as_ref().map(|s| s.trim_end_matches("i32")) != expr_lowercase + && value_lowercase.as_ref().map(|s| s.trim_end_matches("i32")) != expr_lowercase.as_deref() { if let Some(ref value) = value { write!(w, " // {value}", value = Escape(value.as_str())); From ce6dfca4615859032c0a5eb94c281949f47b733a Mon Sep 17 00:00:00 2001 From: sladynnunes Date: Wed, 31 May 2023 02:07:39 -0700 Subject: [PATCH 6/7] Fix tests --- src/librustdoc/html/render/print_item.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/librustdoc/html/render/print_item.rs b/src/librustdoc/html/render/print_item.rs index fe00f5adee9e6..b559c427f6593 100644 --- a/src/librustdoc/html/render/print_item.rs +++ b/src/librustdoc/html/render/print_item.rs @@ -1490,7 +1490,8 @@ fn item_constant(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, c: &cle let expr_lowercase = Some(expr.to_lowercase()); if value_lowercase != expr_lowercase - && value_lowercase.as_ref().map(|s| s.trim_end_matches("i32")) != expr_lowercase.as_deref() + && value_lowercase.as_ref().map(|s| s.trim_end_matches("i32")) + != expr_lowercase.as_deref() { if let Some(ref value) = value { write!(w, " // {value}", value = Escape(value.as_str())); From 55e2c58c538f54497e38d1e5fba86cf44061fc05 Mon Sep 17 00:00:00 2001 From: sladynnunes Date: Wed, 31 May 2023 02:42:12 -0700 Subject: [PATCH 7/7] Bless tests --- .../src/invalid_utf8_in_unchecked.rs | 74 +++++++++++++++++++ .../tests/ui/invalid_utf8_in_unchecked.rs | 20 +++++ .../tests/ui/invalid_utf8_in_unchecked.stderr | 22 ++++++ 3 files changed, 116 insertions(+) create mode 100644 src/tools/clippy/clippy_lints/src/invalid_utf8_in_unchecked.rs create mode 100644 src/tools/clippy/tests/ui/invalid_utf8_in_unchecked.rs create mode 100644 src/tools/clippy/tests/ui/invalid_utf8_in_unchecked.stderr diff --git a/src/tools/clippy/clippy_lints/src/invalid_utf8_in_unchecked.rs b/src/tools/clippy/clippy_lints/src/invalid_utf8_in_unchecked.rs new file mode 100644 index 0000000000000..6a4861747d267 --- /dev/null +++ b/src/tools/clippy/clippy_lints/src/invalid_utf8_in_unchecked.rs @@ -0,0 +1,74 @@ +use clippy_utils::diagnostics::span_lint; +use clippy_utils::{match_function_call, paths}; +use rustc_ast::{BorrowKind, LitKind}; +use rustc_hir::{Expr, ExprKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::source_map::Spanned; +use rustc_span::Span; + +declare_clippy_lint! { + /// ### What it does + /// Checks for `std::str::from_utf8_unchecked` with an invalid UTF-8 literal + /// + /// ### Why is this bad? + /// Creating such a `str` would result in undefined behavior + /// + /// ### Example + /// ```rust + /// # #[allow(unused)] + /// unsafe { + /// std::str::from_utf8_unchecked(b"cl\x82ippy"); + /// } + /// ``` + #[clippy::version = "1.64.0"] + pub INVALID_UTF8_IN_UNCHECKED, + correctness, + "using a non UTF-8 literal in `std::std::from_utf8_unchecked`" +} +declare_lint_pass!(InvalidUtf8InUnchecked => [INVALID_UTF8_IN_UNCHECKED]); + +impl<'tcx> LateLintPass<'tcx> for InvalidUtf8InUnchecked { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { + if let Some([arg]) = match_function_call(cx, expr, &paths::STR_FROM_UTF8_UNCHECKED) { + match &arg.kind { + ExprKind::Lit(Spanned { node: lit, .. }) => { + if let LitKind::ByteStr(bytes, _) = &lit + && std::str::from_utf8(bytes).is_err() + { + lint(cx, expr.span); + } + }, + ExprKind::AddrOf(BorrowKind::Ref, _, Expr { kind: ExprKind::Array(args), .. }) => { + let elements = args.iter().map(|e|{ + match &e.kind { + ExprKind::Lit(Spanned { node: lit, .. }) => match lit { + LitKind::Byte(b) => Some(*b), + #[allow(clippy::cast_possible_truncation)] + LitKind::Int(b, _) => Some(*b as u8), + _ => None + } + _ => None + } + }).collect::>>(); + + if let Some(elements) = elements + && std::str::from_utf8(&elements).is_err() + { + lint(cx, expr.span); + } + } + _ => {} + } + } + } +} + +fn lint(cx: &LateContext<'_>, span: Span) { + span_lint( + cx, + INVALID_UTF8_IN_UNCHECKED, + span, + "non UTF-8 literal in `std::str::from_utf8_unchecked`", + ); +} diff --git a/src/tools/clippy/tests/ui/invalid_utf8_in_unchecked.rs b/src/tools/clippy/tests/ui/invalid_utf8_in_unchecked.rs new file mode 100644 index 0000000000000..3dc096d3197fb --- /dev/null +++ b/src/tools/clippy/tests/ui/invalid_utf8_in_unchecked.rs @@ -0,0 +1,20 @@ +#![warn(clippy::invalid_utf8_in_unchecked)] + +fn main() { + // Valid + unsafe { + std::str::from_utf8_unchecked(&[99, 108, 105, 112, 112, 121]); + std::str::from_utf8_unchecked(&[b'c', b'l', b'i', b'p', b'p', b'y']); + std::str::from_utf8_unchecked(b"clippy"); + + let x = 0xA0; + std::str::from_utf8_unchecked(&[0xC0, x]); + } + + // Invalid + unsafe { + std::str::from_utf8_unchecked(&[99, 108, 130, 105, 112, 112, 121]); + std::str::from_utf8_unchecked(&[b'c', b'l', b'\x82', b'i', b'p', b'p', b'y']); + std::str::from_utf8_unchecked(b"cl\x82ippy"); + } +} diff --git a/src/tools/clippy/tests/ui/invalid_utf8_in_unchecked.stderr b/src/tools/clippy/tests/ui/invalid_utf8_in_unchecked.stderr new file mode 100644 index 0000000000000..c89cd2758ee9f --- /dev/null +++ b/src/tools/clippy/tests/ui/invalid_utf8_in_unchecked.stderr @@ -0,0 +1,22 @@ +error: non UTF-8 literal in `std::str::from_utf8_unchecked` + --> $DIR/invalid_utf8_in_unchecked.rs:16:9 + | +LL | std::str::from_utf8_unchecked(&[99, 108, 130, 105, 112, 112, 121]); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: `-D clippy::invalid-utf8-in-unchecked` implied by `-D warnings` + +error: non UTF-8 literal in `std::str::from_utf8_unchecked` + --> $DIR/invalid_utf8_in_unchecked.rs:17:9 + | +LL | std::str::from_utf8_unchecked(&[b'c', b'l', b'/x82', b'i', b'p', b'p', b'y']); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: non UTF-8 literal in `std::str::from_utf8_unchecked` + --> $DIR/invalid_utf8_in_unchecked.rs:18:9 + | +LL | std::str::from_utf8_unchecked(b"cl/x82ippy"); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 3 previous errors +