From 3fb76f4027596f524403e6eea60e9531e70e9460 Mon Sep 17 00:00:00 2001 From: "Zack M. Davis" Date: Mon, 28 May 2018 19:32:03 -0700 Subject: [PATCH] =?UTF-8?q?inclusive=20range=20syntax=20lint=20(`...`=20?= =?UTF-8?q?=E2=86=92=20`..=3D`)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Our implementation ends up changing the `PatKind::Range` variant in the AST to take a `Spanned` instead of just a `RangeEnd`, because the alternative would be to try to infer the span of the range operator from the spans of the start and end subexpressions, which is both hideous and nontrivial to get right (whereas getting the change to the AST right was a simple game of type tennis). This is concerning #51043. --- src/librustc/hir/lowering.rs | 2 +- src/librustc_lint/builtin.rs | 38 +++++++++++++++++++ src/librustc_lint/lib.rs | 4 +- src/libsyntax/ast.rs | 2 +- src/libsyntax/feature_gate.rs | 3 +- src/libsyntax/fold.rs | 4 +- src/libsyntax/parse/parser.rs | 33 ++++++++++------ src/libsyntax/print/pprust.rs | 4 +- .../lint/inclusive-range-pattern-syntax.fixed | 23 +++++++++++ .../ui/lint/inclusive-range-pattern-syntax.rs | 23 +++++++++++ .../inclusive-range-pattern-syntax.stderr | 12 ++++++ .../ui/range-inclusive-pattern-precedence.rs | 6 +++ .../range-inclusive-pattern-precedence.stderr | 22 ++++++++++- 13 files changed, 154 insertions(+), 22 deletions(-) create mode 100644 src/test/ui/lint/inclusive-range-pattern-syntax.fixed create mode 100644 src/test/ui/lint/inclusive-range-pattern-syntax.rs create mode 100644 src/test/ui/lint/inclusive-range-pattern-syntax.stderr diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index 484c41b3a7996..110ebf6b215a1 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -3356,7 +3356,7 @@ impl<'a> LoweringContext<'a> { PatKind::Ref(ref inner, mutbl) => { hir::PatKind::Ref(self.lower_pat(inner), self.lower_mutability(mutbl)) } - PatKind::Range(ref e1, ref e2, ref end) => hir::PatKind::Range( + PatKind::Range(ref e1, ref e2, Spanned { node: ref end, .. }) => hir::PatKind::Range( P(self.lower_expr(e1)), P(self.lower_expr(e2)), self.lower_range_end(end), diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index d6120ab207924..dfbfcfccf7c89 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -43,6 +43,7 @@ use std::collections::HashSet; use syntax::ast; use syntax::attr; +use syntax::codemap::Spanned; use syntax::edition::Edition; use syntax::feature_gate::{AttributeGate, AttributeType, Stability, deprecated_attributes}; use syntax_pos::{BytePos, Span, SyntaxContext}; @@ -1669,6 +1670,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for TrivialConstraints { } } + /// Does nothing as a lint pass, but registers some `Lint`s /// which are used by other parts of the compiler. #[derive(Copy, Clone)] @@ -1701,3 +1703,39 @@ impl LintPass for SoftLints { ) } } + + +declare_lint! { + pub ELLIPSIS_INCLUSIVE_RANGE_PATTERNS, + Allow, + "`...` range patterns are deprecated" +} + + +pub struct EllipsisInclusiveRangePatterns; + +impl LintPass for EllipsisInclusiveRangePatterns { + fn get_lints(&self) -> LintArray { + lint_array!(ELLIPSIS_INCLUSIVE_RANGE_PATTERNS) + } +} + +impl EarlyLintPass for EllipsisInclusiveRangePatterns { + fn check_pat(&mut self, cx: &EarlyContext, pat: &ast::Pat) { + use self::ast::{PatKind, RangeEnd, RangeSyntax}; + + if let PatKind::Range( + _, _, Spanned { span, node: RangeEnd::Included(RangeSyntax::DotDotDot) } + ) = pat.node { + let msg = "`...` range patterns are deprecated"; + let mut err = cx.struct_span_lint(ELLIPSIS_INCLUSIVE_RANGE_PATTERNS, span, msg); + err.span_suggestion_short_with_applicability( + span, "use `..=` for an inclusive range", "..=".to_owned(), + // FIXME: outstanding problem with precedence in ref patterns: + // https://github.com/rust-lang/rust/issues/51043#issuecomment-392252285 + Applicability::MaybeIncorrect + ); + err.emit() + } + } +} diff --git a/src/librustc_lint/lib.rs b/src/librustc_lint/lib.rs index 1d443258dc642..ba373b5c0e89d 100644 --- a/src/librustc_lint/lib.rs +++ b/src/librustc_lint/lib.rs @@ -111,6 +111,7 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) { AnonymousParameters, UnusedDocComment, BadRepr, + EllipsisInclusiveRangePatterns, ); add_early_builtin_with_new!(sess, @@ -188,7 +189,8 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) { "rust_2018_idioms", BARE_TRAIT_OBJECTS, UNREACHABLE_PUB, - UNUSED_EXTERN_CRATES); + UNUSED_EXTERN_CRATES, + ELLIPSIS_INCLUSIVE_RANGE_PATTERNS); // Guidelines for creating a future incompatibility lint: // diff --git a/src/libsyntax/ast.rs b/src/libsyntax/ast.rs index a09055e6c4aa5..53465c071f33a 100644 --- a/src/libsyntax/ast.rs +++ b/src/libsyntax/ast.rs @@ -617,7 +617,7 @@ pub enum PatKind { /// A literal Lit(P), /// A range pattern, e.g. `1...2`, `1..=2` or `1..2` - Range(P, P, RangeEnd), + Range(P, P, Spanned), /// `[a, b, ..i, y, z]` is represented as: /// `PatKind::Slice(box [a, b], Some(i), box [y, z])` Slice(Vec>, Option>, Vec>), diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs index 51d7a23699544..c813ec1977b88 100644 --- a/src/libsyntax/feature_gate.rs +++ b/src/libsyntax/feature_gate.rs @@ -28,6 +28,7 @@ use self::AttributeGate::*; use rustc_target::spec::abi::Abi; use ast::{self, NodeId, PatKind, RangeEnd}; use attr; +use codemap::Spanned; use edition::{ALL_EDITIONS, Edition}; use syntax_pos::{Span, DUMMY_SP}; use errors::{DiagnosticBuilder, Handler, FatalError}; @@ -1752,7 +1753,7 @@ impl<'a> Visitor<'a> for PostExpansionVisitor<'a> { pattern.span, "box pattern syntax is experimental"); } - PatKind::Range(_, _, RangeEnd::Excluded) => { + PatKind::Range(_, _, Spanned { node: RangeEnd::Excluded, .. }) => { gate_feature_post!(&self, exclusive_range_pattern, pattern.span, "exclusive range pattern syntax is experimental"); } diff --git a/src/libsyntax/fold.rs b/src/libsyntax/fold.rs index ffea713f4ec76..712d00fde32db 100644 --- a/src/libsyntax/fold.rs +++ b/src/libsyntax/fold.rs @@ -1137,10 +1137,10 @@ pub fn noop_fold_pat(p: P, folder: &mut T) -> P { } PatKind::Box(inner) => PatKind::Box(folder.fold_pat(inner)), PatKind::Ref(inner, mutbl) => PatKind::Ref(folder.fold_pat(inner), mutbl), - PatKind::Range(e1, e2, end) => { + PatKind::Range(e1, e2, Spanned { span, node: end }) => { PatKind::Range(folder.fold_expr(e1), folder.fold_expr(e2), - folder.fold_range_end(end)) + Spanned { span, node: folder.fold_range_end(end) }) }, PatKind::Slice(before, slice, after) => { PatKind::Slice(before.move_map(|x| folder.fold_pat(x)), diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 955bdbdcf9177..21bd6c083244d 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -4024,12 +4024,14 @@ impl<'a> Parser<'a> { _ => panic!("can only parse `..`/`...`/`..=` for ranges \ (checked above)"), }; + let op_span = self.span; // Parse range let span = lo.to(self.prev_span); let begin = self.mk_expr(span, ExprKind::Path(qself, path), ThinVec::new()); self.bump(); let end = self.parse_pat_range_end()?; - pat = PatKind::Range(begin, end, end_kind); + let op = Spanned { span: op_span, node: end_kind }; + pat = PatKind::Range(begin, end, op); } token::OpenDelim(token::Brace) => { if qself.is_some() { @@ -4065,17 +4067,22 @@ impl<'a> Parser<'a> { // Try to parse everything else as literal with optional minus match self.parse_literal_maybe_minus() { Ok(begin) => { - if self.eat(&token::DotDotDot) { + let op_span = self.span; + if self.check(&token::DotDot) || self.check(&token::DotDotEq) || + self.check(&token::DotDotDot) { + let end_kind = if self.eat(&token::DotDotDot) { + RangeEnd::Included(RangeSyntax::DotDotDot) + } else if self.eat(&token::DotDotEq) { + RangeEnd::Included(RangeSyntax::DotDotEq) + } else if self.eat(&token::DotDot) { + RangeEnd::Excluded + } else { + panic!("impossible case: we already matched \ + on a range-operator token") + }; let end = self.parse_pat_range_end()?; - pat = PatKind::Range(begin, end, - RangeEnd::Included(RangeSyntax::DotDotDot)); - } else if self.eat(&token::DotDotEq) { - let end = self.parse_pat_range_end()?; - pat = PatKind::Range(begin, end, - RangeEnd::Included(RangeSyntax::DotDotEq)); - } else if self.eat(&token::DotDot) { - let end = self.parse_pat_range_end()?; - pat = PatKind::Range(begin, end, RangeEnd::Excluded); + let op = Spanned { span: op_span, node: end_kind }; + pat = PatKind::Range(begin, end, op); } else { pat = PatKind::Lit(begin); } @@ -4096,7 +4103,9 @@ impl<'a> Parser<'a> { if !allow_range_pat { match pat.node { - PatKind::Range(_, _, RangeEnd::Included(RangeSyntax::DotDotDot)) => {} + PatKind::Range( + _, _, Spanned { node: RangeEnd::Included(RangeSyntax::DotDotDot), .. } + ) => {}, PatKind::Range(..) => { let mut err = self.struct_span_err( pat.span, diff --git a/src/libsyntax/print/pprust.rs b/src/libsyntax/print/pprust.rs index 70c4324a056a0..3359225e15965 100644 --- a/src/libsyntax/print/pprust.rs +++ b/src/libsyntax/print/pprust.rs @@ -16,7 +16,7 @@ use ast::{SelfKind, GenericBound, TraitBoundModifier}; use ast::{Attribute, MacDelimiter, GenericArg}; use util::parser::{self, AssocOp, Fixity}; use attr; -use codemap::{self, CodeMap}; +use codemap::{self, CodeMap, Spanned}; use syntax_pos::{self, BytePos}; use syntax_pos::hygiene::{Mark, SyntaxContext}; use parse::token::{self, BinOpToken, Token}; @@ -2624,7 +2624,7 @@ impl<'a> State<'a> { self.print_pat(inner)?; } PatKind::Lit(ref e) => self.print_expr(&**e)?, - PatKind::Range(ref begin, ref end, ref end_kind) => { + PatKind::Range(ref begin, ref end, Spanned { node: ref end_kind, .. }) => { self.print_expr(begin)?; self.s.space()?; match *end_kind { diff --git a/src/test/ui/lint/inclusive-range-pattern-syntax.fixed b/src/test/ui/lint/inclusive-range-pattern-syntax.fixed new file mode 100644 index 0000000000000..d16859df79e25 --- /dev/null +++ b/src/test/ui/lint/inclusive-range-pattern-syntax.fixed @@ -0,0 +1,23 @@ +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// compile-pass +// run-rustfix + +#![warn(ellipsis_inclusive_range_patterns)] + +fn main() { + let despondency = 2; + match despondency { + 1..=2 => {} + //~^ WARN `...` range patterns are deprecated + _ => {} + } +} diff --git a/src/test/ui/lint/inclusive-range-pattern-syntax.rs b/src/test/ui/lint/inclusive-range-pattern-syntax.rs new file mode 100644 index 0000000000000..9d418aec0858f --- /dev/null +++ b/src/test/ui/lint/inclusive-range-pattern-syntax.rs @@ -0,0 +1,23 @@ +// Copyright 2018 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// compile-pass +// run-rustfix + +#![warn(ellipsis_inclusive_range_patterns)] + +fn main() { + let despondency = 2; + match despondency { + 1...2 => {} + //~^ WARN `...` range patterns are deprecated + _ => {} + } +} diff --git a/src/test/ui/lint/inclusive-range-pattern-syntax.stderr b/src/test/ui/lint/inclusive-range-pattern-syntax.stderr new file mode 100644 index 0000000000000..de04fed589b23 --- /dev/null +++ b/src/test/ui/lint/inclusive-range-pattern-syntax.stderr @@ -0,0 +1,12 @@ +warning: `...` range patterns are deprecated + --> $DIR/inclusive-range-pattern-syntax.rs:19:10 + | +LL | 1...2 => {} + | ^^^ help: use `..=` for an inclusive range + | +note: lint level defined here + --> $DIR/inclusive-range-pattern-syntax.rs:14:9 + | +LL | #![warn(ellipsis_inclusive_range_patterns)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + diff --git a/src/test/ui/range-inclusive-pattern-precedence.rs b/src/test/ui/range-inclusive-pattern-precedence.rs index 67a0f79ca6b82..ce0c9947a05f6 100644 --- a/src/test/ui/range-inclusive-pattern-precedence.rs +++ b/src/test/ui/range-inclusive-pattern-precedence.rs @@ -16,10 +16,14 @@ // older ... syntax is still allowed as a stability guarantee. #![feature(box_patterns)] +#![warn(ellipsis_inclusive_range_patterns)] + pub fn main() { match &12 { &0...9 => {} + //~^ WARN `...` range patterns are deprecated + //~| HELP use `..=` for an inclusive range &10..=15 => {} //~^ ERROR the range pattern here has ambiguous interpretation //~^^ HELP add parentheses to clarify the precedence @@ -29,6 +33,8 @@ pub fn main() { match Box::new(12) { box 0...9 => {} + //~^ WARN `...` range patterns are deprecated + //~| HELP use `..=` for an inclusive range box 10..=15 => {} //~^ ERROR the range pattern here has ambiguous interpretation //~^^ HELP add parentheses to clarify the precedence diff --git a/src/test/ui/range-inclusive-pattern-precedence.stderr b/src/test/ui/range-inclusive-pattern-precedence.stderr index 99e0d739036b0..cd5ce3035c683 100644 --- a/src/test/ui/range-inclusive-pattern-precedence.stderr +++ b/src/test/ui/range-inclusive-pattern-precedence.stderr @@ -1,14 +1,32 @@ error: the range pattern here has ambiguous interpretation - --> $DIR/range-inclusive-pattern-precedence.rs:23:10 + --> $DIR/range-inclusive-pattern-precedence.rs:27:10 | LL | &10..=15 => {} | ^^^^^^^ help: add parentheses to clarify the precedence: `(10 ..=15)` error: the range pattern here has ambiguous interpretation - --> $DIR/range-inclusive-pattern-precedence.rs:32:13 + --> $DIR/range-inclusive-pattern-precedence.rs:38:13 | LL | box 10..=15 => {} | ^^^^^^^ help: add parentheses to clarify the precedence: `(10 ..=15)` +warning: `...` range patterns are deprecated + --> $DIR/range-inclusive-pattern-precedence.rs:24:11 + | +LL | &0...9 => {} + | ^^^ help: use `..=` for an inclusive range + | +note: lint level defined here + --> $DIR/range-inclusive-pattern-precedence.rs:19:9 + | +LL | #![warn(ellipsis_inclusive_range_patterns)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +warning: `...` range patterns are deprecated + --> $DIR/range-inclusive-pattern-precedence.rs:35:14 + | +LL | box 0...9 => {} + | ^^^ help: use `..=` for an inclusive range + error: aborting due to 2 previous errors