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 484c41b3a799..110ebf6b215a 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 d6120ab20792..dfbfcfccf7c8 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 1d443258dc64..ba373b5c0e89 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 a09055e6c4aa..53465c071f33 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 51d7a2369954..c813ec1977b8 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 ffea713f4ec7..712d00fde32d 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 955bdbdcf917..21bd6c083244 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 70c4324a056a..3359225e1596 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 000000000000..d16859df79e2 --- /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 000000000000..9d418aec0858 --- /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 000000000000..de04fed589b2 --- /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 67a0f79ca6b8..ce0c9947a05f 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 99e0d739036b..cd5ce3035c68 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