Skip to content

Commit 7755f36

Browse files
estebankestebank
committed
Turn warning into warn-by-default lint
1 parent ae4fdd2 commit 7755f36

File tree

11 files changed

+156
-93
lines changed

11 files changed

+156
-93
lines changed

‎src/librustc/lint/builtin.rs

+80
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,12 @@ pub mod parser {
357357
Allow,
358358
"detects the use of `?` as a macro separator"
359359
}
360+
361+
declare_lint! {
362+
pub INCORRECT_MACRO_FRAGMENT_REPETITION,
363+
Warn,
364+
"detects incorrect macro fragment follow due to repetition"
365+
}
360366
}
361367

362368
/// Does nothing as a lint pass, but registers some `Lint`s
@@ -436,6 +442,13 @@ pub enum BuiltinLintDiagnostics {
436442
MacroExpandedMacroExportsAccessedByAbsolutePaths(Span),
437443
ElidedLifetimesInPaths(usize, Span, bool, Span, String),
438444
UnknownCrateTypes(Span, String, String),
445+
IncorrectMacroFragmentRepetition {
446+
span: Span,
447+
token_span: Span,
448+
sugg_span: Span,
449+
frag: String,
450+
possible: Vec<String>,
451+
}
439452
}
440453

441454
impl BuiltinLintDiagnostics {
@@ -522,6 +535,73 @@ impl BuiltinLintDiagnostics {
522535
Applicability::MaybeIncorrect
523536
);
524537
}
538+
BuiltinLintDiagnostics::IncorrectMacroFragmentRepetition {
539+
span,
540+
token_span,
541+
sugg_span,
542+
frag,
543+
possible,
544+
} => {
545+
if span == token_span {
546+
db.span_label(
547+
span,
548+
"this fragment is followed by itself without a valid separator",
549+
);
550+
} else {
551+
db.span_label(
552+
span,
553+
"this fragment is followed by the first fragment in this repetition \
554+
without a valid separator",
555+
);
556+
db.span_label(
557+
token_span,
558+
"this is the first fragment in the evaluated repetition",
559+
);
560+
}
561+
let msg = "allowed there are: ";
562+
let sugg_msg = "add a valid separator for the repetition to be unambiguous";
563+
match &possible[..] {
564+
&[] => {}
565+
&[ref t] => {
566+
db.note(&format!("only {} is allowed after `{}` fragments", t, frag));
567+
if t.starts_with('`') && t.ends_with('`') {
568+
db.span_suggestion_with_applicability(
569+
sugg_span,
570+
&format!("{}, for example", sugg_msg),
571+
(&t[1..t.len()-1]).to_owned(),
572+
Applicability::MaybeIncorrect,
573+
);
574+
} else {
575+
db.note(sugg_msg);
576+
}
577+
}
578+
_ => {
579+
db.note(&format!(
580+
"{}{} or {}",
581+
msg,
582+
possible[..possible.len() - 1].iter().map(|s| s.to_owned())
583+
.collect::<Vec<_>>().join(", "),
584+
possible[possible.len() - 1],
585+
));
586+
let mut note = true;
587+
for t in &possible {
588+
if t.starts_with('`') && t.ends_with('`') {
589+
db.span_suggestion_with_applicability(
590+
sugg_span,
591+
&format!("{}, for example", sugg_msg),
592+
(&t[1..t.len()-1]).to_owned(),
593+
Applicability::MaybeIncorrect,
594+
);
595+
note = false;
596+
break;
597+
}
598+
}
599+
if note {
600+
db.note(sugg_msg);
601+
}
602+
}
603+
}
604+
}
525605
}
526606
}
527607
}

‎src/librustc/lint/mod.rs

+25-3
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ use hir::def_id::{CrateNum, LOCAL_CRATE};
3838
use hir::intravisit;
3939
use hir;
4040
use lint::builtin::BuiltinLintDiagnostics;
41-
use lint::builtin::parser::QUESTION_MARK_MACRO_SEP;
41+
use lint::builtin::parser::{QUESTION_MARK_MACRO_SEP, INCORRECT_MACRO_FRAGMENT_REPETITION};
4242
use session::{Session, DiagnosticMessageId};
4343
use std::{hash, ptr};
4444
use syntax::ast;
@@ -89,9 +89,12 @@ pub struct Lint {
8989

9090
impl Lint {
9191
/// Returns the `rust::lint::Lint` for a `syntax::early_buffered_lints::BufferedEarlyLintId`.
92-
pub fn from_parser_lint_id(lint_id: BufferedEarlyLintId) -> &'static Self {
93-
match lint_id {
92+
pub fn from_parser_lint_id(lint_id: &BufferedEarlyLintId) -> &'static Self {
93+
match *lint_id {
9494
BufferedEarlyLintId::QuestionMarkMacroSep => QUESTION_MARK_MACRO_SEP,
95+
BufferedEarlyLintId::IncorrectMacroFragmentRepetition {
96+
..
97+
} => INCORRECT_MACRO_FRAGMENT_REPETITION,
9598
}
9699
}
97100

@@ -106,6 +109,25 @@ impl Lint {
106109
.map(|(_, l)| l)
107110
.unwrap_or(self.default_level)
108111
}
112+
113+
pub fn builtin_diagnostic(lint_id: BufferedEarlyLintId) -> BuiltinLintDiagnostics {
114+
match lint_id {
115+
BufferedEarlyLintId::IncorrectMacroFragmentRepetition {
116+
span,
117+
token_span,
118+
sugg_span,
119+
frag,
120+
possible,
121+
} => BuiltinLintDiagnostics::IncorrectMacroFragmentRepetition {
122+
span,
123+
token_span,
124+
sugg_span,
125+
frag,
126+
possible,
127+
},
128+
_ => BuiltinLintDiagnostics::Normal,
129+
}
130+
}
109131
}
110132

111133
/// Declare a static item of type `&'static Lint`.

‎src/librustc_driver/driver.rs

+4-3
Original file line numberDiff line numberDiff line change
@@ -1092,9 +1092,10 @@ where
10921092
// Add all buffered lints from the `ParseSess` to the `Session`.
10931093
sess.parse_sess.buffered_lints.with_lock(|buffered_lints| {
10941094
info!("{} parse sess buffered_lints", buffered_lints.len());
1095-
for BufferedEarlyLint{id, span, msg, lint_id} in buffered_lints.drain(..) {
1096-
let lint = lint::Lint::from_parser_lint_id(lint_id);
1097-
sess.buffer_lint(lint, id, span, &msg);
1095+
for BufferedEarlyLint {id, span, msg, lint_id} in buffered_lints.drain(..) {
1096+
let lint = lint::Lint::from_parser_lint_id(&lint_id);
1097+
let diag = lint::Lint::builtin_diagnostic(lint_id);
1098+
sess.buffer_lint_with_diagnostic(lint, id, span, &msg, diag);
10981099
}
10991100
});
11001101

‎src/librustc_resolve/lib.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -3958,8 +3958,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
39583958
}
39593959
}
39603960

3961-
let diag = lint::builtin::BuiltinLintDiagnostics
3962-
::AbsPathWithModule(diag_span);
3961+
let diag = lint::builtin::BuiltinLintDiagnostics::AbsPathWithModule(diag_span);
39633962
self.session.buffer_lint_with_diagnostic(
39643963
lint::builtin::ABSOLUTE_PATHS_NOT_STARTING_WITH_CRATE,
39653964
diag_id, diag_span,

‎src/libsyntax/early_buffered_lints.rs

+8-1
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,20 @@
1414
//! redundant. Later, these types can be converted to types for use by the rest of the compiler.
1515
1616
use syntax::ast::NodeId;
17-
use syntax_pos::MultiSpan;
17+
use syntax_pos::{MultiSpan, Span};
1818

1919
/// Since we cannot import `LintId`s from `rustc::lint`, we define some Ids here which can later be
2020
/// passed to `rustc::lint::Lint::from_parser_lint_id` to get a `rustc::lint::Lint`.
2121
pub enum BufferedEarlyLintId {
2222
/// Usage of `?` as a macro separator is deprecated.
2323
QuestionMarkMacroSep,
24+
IncorrectMacroFragmentRepetition {
25+
span: Span,
26+
token_span: Span,
27+
sugg_span: Span,
28+
frag: String,
29+
possible: Vec<String>,
30+
}
2431
}
2532

2633
/// Stores buffered lint info which can later be passed to `librustc`.

‎src/libsyntax/ext/tt/macro_rules.rs

+13-64
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
use {ast, attr};
1212
use syntax_pos::{Span, DUMMY_SP};
13+
use early_buffered_lints::BufferedEarlyLintId;
1314
use edition::Edition;
1415
use ext::base::{DummyResult, ExtCtxt, MacResult, SyntaxExtension};
1516
use ext::base::{NormalTT, TTMacroExpander};
@@ -784,14 +785,23 @@ fn check_matcher_core(sess: &ParseSess,
784785
match is_in_follow(tok, &frag_spec.as_str()) {
785786
IsInFollow::Invalid(..) | IsInFollow::Yes => {} // handled elsewhere
786787
IsInFollow::No(possible) => {
787-
let tok_sp = tok.span();
788-
let next = if *sp == tok_sp {
788+
let token_span = tok.span();
789+
let next = if *sp == token_span {
789790
"itself".to_owned()
790791
} else {
791792
quoted_tt_to_string(tok)
792793
};
793-
let mut err = sess.span_diagnostic.struct_span_warn(
794+
let sugg_span = sess.source_map().next_point(delim_sp.close);
795+
sess.buffer_lint(
796+
BufferedEarlyLintId::IncorrectMacroFragmentRepetition {
797+
span: *sp,
798+
token_span: tok.span(),
799+
sugg_span,
800+
frag: frag_spec.to_string(),
801+
possible: possible.into_iter().map(String::from).collect(),
802+
},
794803
*sp,
804+
ast::CRATE_NODE_ID,
795805
&format!(
796806
"`${name}:{frag}` is followed (through repetition) by \
797807
{next}, which is not allowed for `{frag}` fragments",
@@ -800,67 +810,6 @@ fn check_matcher_core(sess: &ParseSess,
800810
next=next,
801811
),
802812
);
803-
if *sp == tok_sp {
804-
err.span_label(
805-
*sp,
806-
"this fragment is followed by itself without a valid \
807-
separator",
808-
);
809-
} else {
810-
err.span_label(
811-
*sp,
812-
"this fragment is followed by the first fragment in this \
813-
repetition without a valid separator",
814-
);
815-
err.span_label(
816-
tok_sp,
817-
"this is the first fragment in the evaluated repetition",
818-
);
819-
}
820-
let sugg_span = sess.source_map().next_point(delim_sp.close);
821-
let msg = "allowed there are: ";
822-
let sugg_msg =
823-
"add a valid separator for the repetition to be unambiguous";
824-
match &possible[..] {
825-
&[] => {}
826-
&[t] => {
827-
err.note(&format!(
828-
"only {} is allowed after `{}` fragments",
829-
t,
830-
frag_spec,
831-
));
832-
if t.starts_with('`') && t.ends_with('`') {
833-
err.span_suggestion_with_applicability(
834-
sugg_span,
835-
&format!("{}, for example", sugg_msg),
836-
(&t[1..t.len()-1]).to_owned(),
837-
Applicability::MaybeIncorrect,
838-
);
839-
} else {
840-
err.note(sugg_msg);
841-
}
842-
}
843-
ts => {
844-
err.note(&format!(
845-
"{}{} or {}",
846-
msg,
847-
ts[..ts.len() - 1].iter().map(|s| *s)
848-
.collect::<Vec<_>>().join(", "),
849-
ts[ts.len() - 1],
850-
));
851-
if ts[0].starts_with('`') && ts[0].ends_with('`') {
852-
err.span_suggestion_with_applicability(
853-
sugg_span,
854-
&format!("{}, for example", sugg_msg),
855-
(&ts[0][1..ts[0].len()-1]).to_owned(),
856-
Applicability::MaybeIncorrect,
857-
);
858-
} else {
859-
err.note(sugg_msg);
860-
}
861-
}
862-
}
863-
err.emit();
864813
}
865814
}
866815
}

‎src/libsyntax/parse/mod.rs

+3-2
Original file line numberDiff line numberDiff line change
@@ -86,14 +86,15 @@ impl ParseSess {
8686
&self.source_map
8787
}
8888

89-
pub fn buffer_lint<S: Into<MultiSpan>>(&self,
89+
pub fn buffer_lint<S: Into<MultiSpan>>(
90+
&self,
9091
lint_id: BufferedEarlyLintId,
9192
span: S,
9293
id: NodeId,
9394
msg: &str,
9495
) {
9596
self.buffered_lints.with_lock(|buffered_lints| {
96-
buffered_lints.push(BufferedEarlyLint{
97+
buffered_lints.push(BufferedEarlyLint {
9798
span: span.into(),
9899
id,
99100
msg: msg.into(),

‎src/test/ui/issues/issue-42755.stderr

+7-6
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,21 @@
1+
error: repetition matches empty token tree
2+
--> $DIR/issue-42755.rs:13:7
3+
|
4+
LL | ($($p:vis)*) => {}
5+
| ^^^^^^^^
6+
17
warning: `$p:vis` is followed (through repetition) by itself, which is not allowed for `vis` fragments
28
--> $DIR/issue-42755.rs:13:8
39
|
410
LL | ($($p:vis)*) => {}
511
| ^^^^^^ this fragment is followed by itself without a valid separator
612
|
13+
= note: #[warn(incorrect_macro_fragment_repetition)] on by default
714
= note: allowed there are: `,`, an ident or a type
815
help: add a valid separator for the repetition to be unambiguous, for example
916
|
1017
LL | ($($p:vis),*) => {}
1118
| ^
1219

13-
error: repetition matches empty token tree
14-
--> $DIR/issue-42755.rs:13:7
15-
|
16-
LL | ($($p:vis)*) => {}
17-
| ^^^^^^^^
18-
1920
error: aborting due to previous error
2021

‎src/test/ui/macros/incorrrect-repetition-2.stderr

+1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ warning: `$a:expr` is followed (through repetition) by itself, which is not allo
44
LL | ($($a:expr)*) => {};
55
| ^^^^^^^ this fragment is followed by itself without a valid separator
66
|
7+
= note: #[warn(incorrect_macro_fragment_repetition)] on by default
78
= note: allowed there are: `;`, `=>` or `,`
89
help: add a valid separator for the repetition to be unambiguous, for example
910
|

‎src/test/ui/macros/incorrrect-repetition.stderr

+1
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ LL | ($($i:ident $e:expr)*) => {}
66
| |
77
| this is the first fragment in the evaluated repetition
88
|
9+
= note: #[warn(incorrect_macro_fragment_repetition)] on by default
910
= note: allowed there are: `;`, `=>` or `,`
1011
help: add a valid separator for the repetition to be unambiguous, for example
1112
|

‎src/test/ui/macros/macro-input-future-proofing.stderr

+13-12
Original file line numberDiff line numberDiff line change
@@ -70,18 +70,6 @@ LL | ( $a:expr $($b:tt)* ) => { };
7070
|
7171
= note: allowed there are: `;`, `=>` or `,`
7272

73-
warning: `$a:expr` is followed (through repetition) by itself, which is not allowed for `expr` fragments
74-
--> $DIR/macro-input-future-proofing.rs:31:9
75-
|
76-
LL | ( $($a:expr)* $($b:tt)* ) => { };
77-
| ^^^^^^^ this fragment is followed by itself without a valid separator
78-
|
79-
= note: allowed there are: `;`, `=>` or `,`
80-
help: add a valid separator for the repetition to be unambiguous, for example
81-
|
82-
LL | ( $($a:expr);* $($b:tt)* ) => { };
83-
| ^
84-
8573
error: `$a:expr` is followed by `$b:tt`, which is not allowed for `expr` fragments
8674
--> $DIR/macro-input-future-proofing.rs:31:21
8775
|
@@ -98,5 +86,18 @@ LL | ( $($a:expr),* $($b:tt)* ) => { };
9886
|
9987
= note: allowed there are: `;`, `=>` or `,`
10088

89+
warning: `$a:expr` is followed (through repetition) by itself, which is not allowed for `expr` fragments
90+
--> $DIR/macro-input-future-proofing.rs:31:9
91+
|
92+
LL | ( $($a:expr)* $($b:tt)* ) => { };
93+
| ^^^^^^^ this fragment is followed by itself without a valid separator
94+
|
95+
= note: #[warn(incorrect_macro_fragment_repetition)] on by default
96+
= note: allowed there are: `;`, `=>` or `,`
97+
help: add a valid separator for the repetition to be unambiguous, for example
98+
|
99+
LL | ( $($a:expr);* $($b:tt)* ) => { };
100+
| ^
101+
101102
error: aborting due to 11 previous errors
102103

0 commit comments

Comments
 (0)