Skip to content

New lints for literal suffixes #11149

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5113,6 +5113,7 @@ Released 2018-09-13
[`nonsensical_open_options`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonsensical_open_options
[`nonstandard_macro_braces`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonstandard_macro_braces
[`not_unsafe_ptr_arg_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#not_unsafe_ptr_arg_deref
[`numeric_literal_missing_suffix`]: https://rust-lang.github.io/rust-clippy/master/index.html#numeric_literal_missing_suffix
[`obfuscated_if_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#obfuscated_if_else
[`octal_escapes`]: https://rust-lang.github.io/rust-clippy/master/index.html#octal_escapes
[`ok_expect`]: https://rust-lang.github.io/rust-clippy/master/index.html#ok_expect
Expand Down Expand Up @@ -5260,6 +5261,8 @@ Released 2018-09-13
[`struct_excessive_bools`]: https://rust-lang.github.io/rust-clippy/master/index.html#struct_excessive_bools
[`stutter`]: https://rust-lang.github.io/rust-clippy/master/index.html#stutter
[`suboptimal_flops`]: https://rust-lang.github.io/rust-clippy/master/index.html#suboptimal_flops
[`suffix_with_separator`]: https://rust-lang.github.io/rust-clippy/master/index.html#suffix_with_separator
[`suffix_without_separator`]: https://rust-lang.github.io/rust-clippy/master/index.html#suffix_without_separator
[`suspicious_arithmetic_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_arithmetic_impl
[`suspicious_assignment_formatting`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_assignment_formatting
[`suspicious_command_arg_space`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_command_arg_space
Expand Down Expand Up @@ -5462,4 +5465,5 @@ Released 2018-09-13
[`accept-comment-above-statement`]: https://doc.rust-lang.org/clippy/lint_configuration.html#accept-comment-above-statement
[`accept-comment-above-attributes`]: https://doc.rust-lang.org/clippy/lint_configuration.html#accept-comment-above-attributes
[`allow-one-hash-in-raw-strings`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-one-hash-in-raw-strings
[`allow-missing-suffix-with-type-annotations`]: https://doc.rust-lang.org/clippy/lint_configuration.html#allow-missing-suffix-with-type-annotations
<!-- end autogenerated links to configuration documentation -->
10 changes: 10 additions & 0 deletions book/src/lint_configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -730,3 +730,13 @@ Whether to allow `r#""#` when `r""` can be used
* [`unnecessary_raw_string_hashes`](https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_raw_string_hashes)


## `allow-missing-suffix-with-type-annotations`
Whether to allow missing suffixes when it has type annotations like `: u8`

**Default Value:** `true` (`bool`)

---
**Affected lints:**
* [`numeric_literal_missing_suffix`](https://rust-lang.github.io/rust-clippy/master/index.html#numeric_literal_missing_suffix)


3 changes: 3 additions & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,10 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::methods::ZST_OFFSET_INFO,
crate::min_ident_chars::MIN_IDENT_CHARS_INFO,
crate::minmax::MIN_MAX_INFO,
crate::misc::NUMERIC_LITERAL_MISSING_SUFFIX_INFO,
crate::misc::SHORT_CIRCUIT_STATEMENT_INFO,
crate::misc::SUFFIX_WITHOUT_SEPARATOR_INFO,
crate::misc::SUFFIX_WITH_SEPARATOR_INFO,
crate::misc::TOPLEVEL_REF_ARG_INFO,
crate::misc::USED_UNDERSCORE_BINDING_INFO,
crate::misc::ZERO_PTR_INFO,
Expand Down
7 changes: 6 additions & 1 deletion clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -629,7 +629,12 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|_| Box::new(needless_bool::NeedlessBool));
store.register_late_pass(|_| Box::new(needless_bool::BoolComparison));
store.register_late_pass(|_| Box::new(needless_for_each::NeedlessForEach));
store.register_late_pass(|_| Box::<misc::LintPass>::default());
let allow_missing_suffix_with_type_annotations = conf.allow_missing_suffix_with_type_annotations;
store.register_late_pass(move |_| {
Box::new(misc::LintPass {
allow_missing_suffix_with_type_annotations,
})
});
store.register_late_pass(|_| Box::new(eta_reduction::EtaReduction));
store.register_late_pass(|_| Box::new(mut_mut::MutMut));
store.register_late_pass(|_| Box::new(mut_reference::UnnecessaryMutPassed));
Expand Down
207 changes: 180 additions & 27 deletions clippy_lints/src/misc.rs
Original file line number Diff line number Diff line change
@@ -1,24 +1,28 @@
use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg, span_lint_hir_and_then};
use clippy_utils::source::{snippet, snippet_opt, snippet_with_context};
use clippy_utils::{
diagnostics::{span_lint, span_lint_and_sugg, span_lint_hir_and_then},
get_parent_expr, in_constant, is_from_proc_macro, is_integer_literal, is_lint_allowed, iter_input_pats,
last_path_segment,
source::{snippet, snippet_opt, snippet_with_context},
std_or_core,
sugg::Sugg,
SpanlessEq,
};
use hir::OwnerNode;
use if_chain::if_chain;
use rustc_ast::LitKind;
use rustc_errors::Applicability;
use rustc_hir::intravisit::FnKind;
use rustc_hir::{
self as hir, def, BinOpKind, BindingAnnotation, Body, ByRef, Expr, ExprKind, FnDecl, Mutability, PatKind, Stmt,
StmtKind, TyKind,
self as hir, def, BinOpKind, BindingAnnotation, Body, ByRef, Expr, ExprKind, FnDecl, ItemKind, Lit, Mutability,
Node, PatKind, Stmt, StmtKind, TyKind,
};
use rustc_lint::{LateContext, LateLintPass};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::def_id::LocalDefId;
use rustc_span::hygiene::DesugaringKind;
use rustc_span::source_map::{ExpnKind, Span};

use clippy_utils::sugg::Sugg;
use clippy_utils::{
get_parent_expr, in_constant, is_integer_literal, is_lint_allowed, is_no_std_crate, iter_input_pats,
last_path_segment, SpanlessEq,
};
use rustc_span::BytePos;

use crate::ref_patterns::REF_PATTERNS;

Expand Down Expand Up @@ -126,28 +130,99 @@ declare_clippy_lint! {
"using `0 as *{const, mut} T`"
}

pub struct LintPass {
std_or_core: &'static str,
declare_clippy_lint! {
/// ### What it does
/// Checks for numeric literals (e.g., `1.0_f64`) without a suffix (The `u32` in `1u32`).
///
/// ### Why is this bad?
/// It's not, but some projects may wish to make the type of every literal explicit. In many
/// cases this can prevent default numeric fallback as well, see
/// [RFC0212](https://github.com/rust-lang/rfcs/blob/master/text/0212-restore-int-fallback.md)
/// for more info and `default_numeric_fallback` for an alternative that only tackles the
/// latter if explicitness is not desired.
///
/// Note that when type annotations are provided, the type is already explicit; the lint will
/// not lint those cases unless the `allow_missing_suffix_with_type_annotations` configuration
/// option is disabled.
///
/// ### Example
/// ```rust
/// let x = 1;
/// ```
/// Use instead:
/// ```rust
/// let x = 1i32;
/// ```
#[clippy::version = "1.72.0"]
pub NUMERIC_LITERAL_MISSING_SUFFIX,
restriction,
"numeric literals missing explicit suffixes"
}
impl Default for LintPass {
fn default() -> Self {
Self { std_or_core: "std" }
}

declare_clippy_lint! {
/// ### What it does
/// Checks for numeric literals (e.g., 1.0_f64) without a separator (`_`) before the suffix
/// (`f64` in `1.0_f64`).
///
/// ### Why is this bad?
/// It's not, but enforcing a consistent style is important. In this case the codebase prefers
/// a separator.
///
/// Also see the `suffix_without_separator` lint for an alternative.
///
/// ### Example
/// ```rust
/// let x = 1i32;
/// ```
/// Use instead:
/// ```rust
/// let x = 1_i32;
/// ```
#[clippy::version = "1.72.0"]
pub SUFFIX_WITH_SEPARATOR,
restriction,
"prefer numeric literals with a separator (`_`) before the suffix"
}

declare_clippy_lint! {
/// ### What it does
/// Checks for numeric literals (e.g., 1.0_f64) with a separator (`_`) before the suffix (`f64`
/// in `1.0_f64`).
///
/// ### Why is this bad?
/// It's not, but enforcing a consistent style is important. In this case the codebase prefers
/// no separator.
///
/// Also see the `suffix_with_separator` lint for an alternative.
///
/// ### Example
/// ```rust
/// let x = 1_i32;
/// ```
/// Use instead:
/// ```rust
/// let x = 1i32;
/// ```
#[clippy::version = "1.72.0"]
pub SUFFIX_WITHOUT_SEPARATOR,
restriction,
"prefer numeric literals without a separator (`_`) before the suffix"
}

pub struct LintPass {
pub allow_missing_suffix_with_type_annotations: bool,
}
impl_lint_pass!(LintPass => [
TOPLEVEL_REF_ARG,
USED_UNDERSCORE_BINDING,
SHORT_CIRCUIT_STATEMENT,
ZERO_PTR,
NUMERIC_LITERAL_MISSING_SUFFIX,
SUFFIX_WITH_SEPARATOR,
SUFFIX_WITHOUT_SEPARATOR,
]);

impl<'tcx> LateLintPass<'tcx> for LintPass {
fn check_crate(&mut self, cx: &LateContext<'_>) {
if is_no_std_crate(cx) {
self.std_or_core = "core";
}
}

fn check_fn(
&mut self,
cx: &LateContext<'tcx>,
Expand Down Expand Up @@ -253,6 +328,9 @@ impl<'tcx> LateLintPass<'tcx> for LintPass {
}

fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
if let ExprKind::Lit(lit) = expr.kind {
self.check_lit(cx, lit, expr);
}
if let ExprKind::Cast(e, ty) = expr.kind {
self.check_cast(cx, expr.span, e, ty);
return;
Expand Down Expand Up @@ -333,7 +411,8 @@ fn non_macro_local(cx: &LateContext<'_>, res: def::Res) -> bool {
}
}

impl LintPass {
impl<'tcx> LintPass {
#[expect(clippy::unused_self)]
fn check_cast(&self, cx: &LateContext<'_>, span: Span, e: &Expr<'_>, ty: &hir::Ty<'_>) {
if_chain! {
if let TyKind::Ptr(ref mut_ty) = ty.kind;
Expand All @@ -344,17 +423,91 @@ impl LintPass {
Mutability::Mut => ("`0 as *mut _` detected", "ptr::null_mut"),
Mutability::Not => ("`0 as *const _` detected", "ptr::null"),
};
let std_or_core = std_or_core(cx).unwrap_or("...");

let (sugg, appl) = if let TyKind::Infer = mut_ty.ty.kind {
(format!("{}::{sugg_fn}()", self.std_or_core), Applicability::MachineApplicable)
(format!("{std_or_core}::{sugg_fn}()"), Applicability::MachineApplicable)
} else if let Some(mut_ty_snip) = snippet_opt(cx, mut_ty.ty.span) {
(format!("{}::{sugg_fn}::<{mut_ty_snip}>()", self.std_or_core), Applicability::MachineApplicable)
(format!("{std_or_core}::{sugg_fn}::<{mut_ty_snip}>()"), Applicability::MachineApplicable)
} else {
// `MaybeIncorrect` as type inference may not work with the suggested code
(format!("{}::{sugg_fn}()", self.std_or_core), Applicability::MaybeIncorrect)
(format!("{std_or_core}::{sugg_fn}()"), Applicability::MaybeIncorrect)
};
span_lint_and_sugg(cx, ZERO_PTR, span, msg, "try", sugg, appl);
}
}
}

fn check_lit(&self, cx: &LateContext<'tcx>, lit: &Lit, expr: &'tcx Expr<'tcx>) {
if expr.span.from_expansion() {
return;
}
if !matches!(lit.node, LitKind::Int(_, _) | LitKind::Float(_, _)) {
return;
}

let sm = cx.sess().source_map();
let ty = cx.typeck_results().expr_ty(expr).to_string();

if lit.node.is_unsuffixed()
&& !is_from_proc_macro(cx, expr)
&& (!self.allow_missing_suffix_with_type_annotations
|| !(matches!(
cx.tcx.hir().owner(cx.tcx.hir().get_parent_item(expr.hir_id)),
OwnerNode::Item(item) if matches!(
item.kind,
ItemKind::Static(_, _, _) | ItemKind::Const(_, _),
),
) || cx
.tcx
.hir()
.parent_iter(expr.hir_id)
.any(|(_, p)| matches!(p, Node::Local(local) if local.ty.is_some()))))
{
span_lint_and_sugg(
cx,
NUMERIC_LITERAL_MISSING_SUFFIX,
lit.span.shrink_to_hi(),
"this literal is missing an explicit suffix",
"add it",
ty.clone(),
Applicability::MachineApplicable,
);
}

if lit.node.is_suffixed()
&& let Some(ty_first_char) = ty.chars().next()
&& let separator_span = sm.span_extend_to_prev_char(lit.span.shrink_to_hi(), ty_first_char, false)
// TODO: There's probably a better way to do this. We want to turn `64` from `_f64` into `_`
&& let separator_span = separator_span
.with_lo(separator_span.lo() - BytePos(2))
.with_hi(separator_span.lo() - BytePos(1))
&& let Some(separator_snip) = snippet_opt(cx, separator_span)
{
if separator_snip == "_" && !is_from_proc_macro(cx, expr) {
span_lint_and_sugg(
cx,
SUFFIX_WITHOUT_SEPARATOR,
separator_span,
"this literal has a separator before its suffix",
"remove it",
String::new(),
Applicability::MachineApplicable,
);
} else if !is_from_proc_macro(cx, expr) {
span_lint_and_sugg(
cx,
SUFFIX_WITH_SEPARATOR,
// Since this one has no separator, we must be careful with our suggestion. We
// cannot just use the original `separator_span` as that'll overwrite the last
// digit of the literal. So make it empty and point to after that digit.
separator_span.with_lo(separator_span.lo() + BytePos(1)).shrink_to_lo(),
"this literal is missing a separator before its suffix",
"add it",
"_".to_owned(),
Applicability::MachineApplicable,
);
}
}
}
}
4 changes: 4 additions & 0 deletions clippy_lints/src/utils/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -551,6 +551,10 @@ define_Conf! {
///
/// Whether to allow `r#""#` when `r""` can be used
(allow_one_hash_in_raw_strings: bool = false),
/// Lint: NUMERIC_LITERAL_MISSING_SUFFIX.
///
/// Whether to allow missing suffixes when it has type annotations like `: u8`
(allow_missing_suffix_with_type_annotations: bool = true),
}

/// Search for the configuration file.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
allow-missing-suffix-with-type-annotations = true
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
allow-missing-suffix-with-type-annotations = false
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
//@run-rustfix
//@aux-build:../../ui/auxiliary/proc_macros.rs:proc-macro
//@revisions: disallow_type_annotations allow_type_annotations
//@[disallow_type_annotations] rustc-env:CLIPPY_CONF_DIR=$DIR/disallow_type_annotations
//@[allow_type_annotations] rustc-env:CLIPPY_CONF_DIR=$DIR/allow_type_annotations
#![allow(unused)]
#![warn(clippy::numeric_literal_missing_suffix)]
#![feature(custom_inner_attributes)]
#![rustfmt::skip] // Not sure why rustfmt messes up the uitest commands

#[macro_use]
extern crate proc_macros;

fn main() {
let x = 1i32;
let x = 1.0f64;
let x: u8 = 1;
static X1: u32 = 0;
const X2: u32 = 0;
// Don't lint
external! {
let x = 1;
let x: u8 = 1;
static X3: u32 = 0;
const X4: u32 = 0;
}
with_span! {
span
let x = 1;
let x: u8 = 1;
static X5: u32 = 0;
const X6: u32 = 0;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
error: this literal is missing an explicit suffix
--> $DIR/numeric_literal_missing_suffix.rs:15:14
|
LL | let x = 1;
| ^ help: add it: `i32`
|
= note: `-D clippy::numeric-literal-missing-suffix` implied by `-D warnings`

error: this literal is missing an explicit suffix
--> $DIR/numeric_literal_missing_suffix.rs:16:16
|
LL | let x = 1.0;
| ^ help: add it: `f64`

error: aborting due to 2 previous errors

Loading