Skip to content

Commit

Permalink
Auto merge of rust-lang#118879 - Nadrieril:lint-range-gap, r=<try>
Browse files Browse the repository at this point in the history
WIP: Lint small gaps between ranges

In the discussion to stabilize exclusive range patterns (rust-lang#37854), it has often come up that they're likely to cause off-by-one mistakes. We already have the `overlapping_range_endpoints` lint, so I [proposed](rust-lang#37854 (comment)) a lint to catch the complementary mistake.

This PR adds a new `small_gaps_between_ranges` lint that catches likely off-by-one errors with range patterns. Here's the idea (see the test file for more examples):
```rust
match x {
    0..10 => ..., // WARN: this range doesn't match `10_u8` because `..` is a non-inclusive range
    11..20 => ..., // this seems to continue range `0_u8..10_u8`, but `10_u8` isn't matched by either of them
    _ => ...,
}
// help: use an inclusive range instead: `0_u8..=10_u8`
```

More precisely: for any exclusive range `lo..hi`, if `hi+1` is matched by another range but `hi` isn't, we suggest writing an inclusive range `lo..=hi` instead. We don't lint `lo..T::MAX` but we could.

WARNING: the first 3 commits come from rust-lang#119233, ignore those.

r? ghost
  • Loading branch information
bors committed Dec 24, 2023
2 parents e87ccb8 + 6049acb commit b147cf5
Show file tree
Hide file tree
Showing 21 changed files with 612 additions and 217 deletions.
31 changes: 31 additions & 0 deletions compiler/rustc_lint_defs/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ declare_lint_pass! {
RUST_2021_PRELUDE_COLLISIONS,
SEMICOLON_IN_EXPRESSIONS_FROM_MACROS,
SINGLE_USE_LIFETIMES,
SMALL_GAPS_BETWEEN_RANGES,
SOFT_UNSTABLE,
STABLE_FEATURES,
SUSPICIOUS_AUTO_TRAIT_IMPLS,
Expand Down Expand Up @@ -835,6 +836,36 @@ declare_lint! {
"detects range patterns with overlapping endpoints"
}

declare_lint! {
/// The `small_gaps_between_ranges` lint detects `match` expressions that use [range patterns]
/// that skip over a single number.
///
/// [range patterns]: https://doc.rust-lang.org/nightly/reference/patterns.html#range-patterns
///
/// ### Example
///
/// ```rust
/// # #![feature(exclusive_range_pattern)]
/// let x = 123u32;
/// match x {
/// 0..100 => { println!("small"); }
/// 101..1000 => { println!("large"); }
/// _ => { println!("larger"); }
/// }
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// It is likely a mistake to have range patterns in a match expression that miss out a single
/// number. Check that the beginning and end values are what you expect, and keep in mind that
/// with `..=` the right bound is inclusive, and with `..` it is exclusive.
pub SMALL_GAPS_BETWEEN_RANGES,
Warn,
"detects range patterns separated by a single number"
}

declare_lint! {
/// The `bindings_with_variant_name` lint detects pattern bindings with
/// the same name as one of the matched variants.
Expand Down
39 changes: 27 additions & 12 deletions compiler/rustc_middle/src/thir/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,26 @@ use super::{
PatKind, Stmt, StmtKind, Thir,
};

pub trait Visitor<'a, 'tcx: 'a>: Sized {
fn thir(&self) -> &'a Thir<'tcx>;
pub trait Visitor<'thir, 'tcx: 'thir>: Sized {
fn thir(&self) -> &'thir Thir<'tcx>;

fn visit_expr(&mut self, expr: &Expr<'tcx>) {
fn visit_expr(&mut self, expr: &'thir Expr<'tcx>) {
walk_expr(self, expr);
}

fn visit_stmt(&mut self, stmt: &Stmt<'tcx>) {
fn visit_stmt(&mut self, stmt: &'thir Stmt<'tcx>) {
walk_stmt(self, stmt);
}

fn visit_block(&mut self, block: &Block) {
fn visit_block(&mut self, block: &'thir Block) {
walk_block(self, block);
}

fn visit_arm(&mut self, arm: &Arm<'tcx>) {
fn visit_arm(&mut self, arm: &'thir Arm<'tcx>) {
walk_arm(self, arm);
}

fn visit_pat(&mut self, pat: &Pat<'tcx>) {
fn visit_pat(&mut self, pat: &'thir Pat<'tcx>) {
walk_pat(self, pat);
}

Expand All @@ -36,7 +36,10 @@ pub trait Visitor<'a, 'tcx: 'a>: Sized {
// other `visit*` functions.
}

pub fn walk_expr<'a, 'tcx: 'a, V: Visitor<'a, 'tcx>>(visitor: &mut V, expr: &Expr<'tcx>) {
pub fn walk_expr<'thir, 'tcx: 'thir, V: Visitor<'thir, 'tcx>>(
visitor: &mut V,
expr: &'thir Expr<'tcx>,
) {
use ExprKind::*;
match expr.kind {
Scope { value, region_scope: _, lint_level: _ } => {
Expand Down Expand Up @@ -168,7 +171,10 @@ pub fn walk_expr<'a, 'tcx: 'a, V: Visitor<'a, 'tcx>>(visitor: &mut V, expr: &Exp
}
}

pub fn walk_stmt<'a, 'tcx: 'a, V: Visitor<'a, 'tcx>>(visitor: &mut V, stmt: &Stmt<'tcx>) {
pub fn walk_stmt<'thir, 'tcx: 'thir, V: Visitor<'thir, 'tcx>>(
visitor: &mut V,
stmt: &'thir Stmt<'tcx>,
) {
match &stmt.kind {
StmtKind::Expr { expr, scope: _ } => visitor.visit_expr(&visitor.thir()[*expr]),
StmtKind::Let {
Expand All @@ -191,7 +197,10 @@ pub fn walk_stmt<'a, 'tcx: 'a, V: Visitor<'a, 'tcx>>(visitor: &mut V, stmt: &Stm
}
}

pub fn walk_block<'a, 'tcx: 'a, V: Visitor<'a, 'tcx>>(visitor: &mut V, block: &Block) {
pub fn walk_block<'thir, 'tcx: 'thir, V: Visitor<'thir, 'tcx>>(
visitor: &mut V,
block: &'thir Block,
) {
for &stmt in &*block.stmts {
visitor.visit_stmt(&visitor.thir()[stmt]);
}
Expand All @@ -200,7 +209,10 @@ pub fn walk_block<'a, 'tcx: 'a, V: Visitor<'a, 'tcx>>(visitor: &mut V, block: &B
}
}

pub fn walk_arm<'a, 'tcx: 'a, V: Visitor<'a, 'tcx>>(visitor: &mut V, arm: &Arm<'tcx>) {
pub fn walk_arm<'thir, 'tcx: 'thir, V: Visitor<'thir, 'tcx>>(
visitor: &mut V,
arm: &'thir Arm<'tcx>,
) {
match arm.guard {
Some(Guard::If(expr)) => visitor.visit_expr(&visitor.thir()[expr]),
Some(Guard::IfLet(ref pat, expr)) => {
Expand All @@ -213,7 +225,10 @@ pub fn walk_arm<'a, 'tcx: 'a, V: Visitor<'a, 'tcx>>(visitor: &mut V, arm: &Arm<'
visitor.visit_expr(&visitor.thir()[arm.body]);
}

pub fn walk_pat<'a, 'tcx: 'a, V: Visitor<'a, 'tcx>>(visitor: &mut V, pat: &Pat<'tcx>) {
pub fn walk_pat<'thir, 'tcx: 'thir, V: Visitor<'thir, 'tcx>>(
visitor: &mut V,
pat: &'thir Pat<'tcx>,
) {
use PatKind::*;
match &pat.kind {
AscribeUserType { subpattern, ascription: _ }
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_mir_build/src/check_unsafety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for LayoutConstrainedPlaceVisitor<'a, 'tcx> {
self.thir
}

fn visit_expr(&mut self, expr: &Expr<'tcx>) {
fn visit_expr(&mut self, expr: &'a Expr<'tcx>) {
match expr.kind {
ExprKind::Field { lhs, .. } => {
if let ty::Adt(adt_def, _) = self.thir[lhs].ty.kind() {
Expand Down Expand Up @@ -206,7 +206,7 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
self.thir
}

fn visit_block(&mut self, block: &Block) {
fn visit_block(&mut self, block: &'a Block) {
match block.safety_mode {
// compiler-generated unsafe code should not count towards the usefulness of
// an outer unsafe block
Expand Down Expand Up @@ -234,7 +234,7 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
}
}

fn visit_pat(&mut self, pat: &Pat<'tcx>) {
fn visit_pat(&mut self, pat: &'a Pat<'tcx>) {
if self.in_union_destructure {
match pat.kind {
// binding to a variable allows getting stuff out of variable
Expand Down Expand Up @@ -319,7 +319,7 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
}
}

fn visit_expr(&mut self, expr: &Expr<'tcx>) {
fn visit_expr(&mut self, expr: &'a Expr<'tcx>) {
// could we be in the LHS of an assignment to a field?
match expr.kind {
ExprKind::Field { .. }
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_mir_build/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -453,14 +453,14 @@ pub enum UnusedUnsafeEnclosing {
},
}

pub(crate) struct NonExhaustivePatternsTypeNotEmpty<'p, 'tcx, 'm> {
pub cx: &'m RustcMatchCheckCtxt<'p, 'tcx>,
pub(crate) struct NonExhaustivePatternsTypeNotEmpty<'p, 'thir, 'tcx, 'm> {
pub cx: &'m RustcMatchCheckCtxt<'p, 'thir, 'tcx>,
pub expr_span: Span,
pub span: Span,
pub ty: Ty<'tcx>,
}

impl<'a> IntoDiagnostic<'a> for NonExhaustivePatternsTypeNotEmpty<'_, '_, '_> {
impl<'a> IntoDiagnostic<'a> for NonExhaustivePatternsTypeNotEmpty<'_, '_, '_, '_> {
fn into_diagnostic(self, dcx: &'a DiagCtxt, level: Level) -> DiagnosticBuilder<'_> {
let mut diag = DiagnosticBuilder::new(
dcx,
Expand Down
Loading

0 comments on commit b147cf5

Please sign in to comment.