inclusive range syntax lint (... → ..=)

Our implementation ends up changing the `PatKind::Range` variant in the AST to take a `Spanned<RangeEnd>` 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.
rust-lang · Jun 26, 2018 · 3fb76f4 · 3fb76f4
1 parent 0577155
commit 3fb76f4
Show file tree

Hide file tree

Showing 13 changed files with 154 additions and 22 deletions.
diff --git 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
@@ -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
@@ -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
@@ -617,7 +617,7 @@ pub enum PatKind {
     /// A literal
     Lit(P<Expr>),
     /// A range pattern, e.g. `1...2`, `1..=2` or `1..2`
-    Range(P<Expr>, P<Expr>, RangeEnd),
+    Range(P<Expr>, P<Expr>, Spanned<RangeEnd>),
     /// `[a, b, ..i, y, z]` is represented as:
     ///     `PatKind::Slice(box [a, b], Some(i), box [y, z])`
     Slice(Vec<P<Pat>>, Option<P<Pat>>, Vec<P<Pat>>),

diff --git 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
@@ -1137,10 +1137,10 @@ pub fn noop_fold_pat<T: Folder>(p: P<Pat>, folder: &mut T) -> P<Pat> {
             }
             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
@@ -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
@@ -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
@@ -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 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, 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
@@ -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 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, 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
@@ -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
@@ -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
@@ -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