Skip to content

Commit

Permalink
Turn warning into warn-by-default lint
Browse files Browse the repository at this point in the history
  • Loading branch information
estebank committed Dec 6, 2018
1 parent 97bda67 commit ca5e89d
Show file tree
Hide file tree
Showing 11 changed files with 156 additions and 93 deletions.
80 changes: 80 additions & 0 deletions src/librustc/lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,12 @@ pub mod parser {
Allow,
"detects the use of `?` as a macro separator"
}

declare_lint! {
pub INCORRECT_MACRO_FRAGMENT_REPETITION,
Warn,
"detects incorrect macro fragment follow due to repetition"
}
}

/// Does nothing as a lint pass, but registers some `Lint`s
Expand Down Expand Up @@ -443,6 +449,13 @@ pub enum BuiltinLintDiagnostics {
MacroExpandedMacroExportsAccessedByAbsolutePaths(Span),
ElidedLifetimesInPaths(usize, Span, bool, Span, String),
UnknownCrateTypes(Span, String, String),
IncorrectMacroFragmentRepetition {
span: Span,
token_span: Span,
sugg_span: Span,
frag: String,
possible: Vec<String>,
}
}

impl BuiltinLintDiagnostics {
Expand Down Expand Up @@ -529,6 +542,73 @@ impl BuiltinLintDiagnostics {
Applicability::MaybeIncorrect
);
}
BuiltinLintDiagnostics::IncorrectMacroFragmentRepetition {
span,
token_span,
sugg_span,
frag,
possible,
} => {
if span == token_span {
db.span_label(
span,
"this fragment is followed by itself without a valid separator",
);
} else {
db.span_label(
span,
"this fragment is followed by the first fragment in this repetition \
without a valid separator",
);
db.span_label(
token_span,
"this is the first fragment in the evaluated repetition",
);
}
let msg = "allowed there are: ";
let sugg_msg = "add a valid separator for the repetition to be unambiguous";
match &possible[..] {
&[] => {}
&[ref t] => {
db.note(&format!("only {} is allowed after `{}` fragments", t, frag));
if t.starts_with('`') && t.ends_with('`') {
db.span_suggestion_with_applicability(
sugg_span,
&format!("{}, for example", sugg_msg),
(&t[1..t.len()-1]).to_owned(),
Applicability::MaybeIncorrect,
);
} else {
db.note(sugg_msg);
}
}
_ => {
db.note(&format!(
"{}{} or {}",
msg,
possible[..possible.len() - 1].iter().map(|s| s.to_owned())
.collect::<Vec<_>>().join(", "),
possible[possible.len() - 1],
));
let mut note = true;
for t in &possible {
if t.starts_with('`') && t.ends_with('`') {
db.span_suggestion_with_applicability(
sugg_span,
&format!("{}, for example", sugg_msg),
(&t[1..t.len()-1]).to_owned(),
Applicability::MaybeIncorrect,
);
note = false;
break;
}
}
if note {
db.note(sugg_msg);
}
}
}
}
}
}
}
Expand Down
28 changes: 25 additions & 3 deletions src/librustc/lint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ use hir::def_id::{CrateNum, LOCAL_CRATE};
use hir::intravisit;
use hir;
use lint::builtin::BuiltinLintDiagnostics;
use lint::builtin::parser::QUESTION_MARK_MACRO_SEP;
use lint::builtin::parser::{QUESTION_MARK_MACRO_SEP, INCORRECT_MACRO_FRAGMENT_REPETITION};
use session::{Session, DiagnosticMessageId};
use std::{hash, ptr};
use syntax::ast;
Expand Down Expand Up @@ -89,9 +89,12 @@ pub struct Lint {

impl Lint {
/// Returns the `rust::lint::Lint` for a `syntax::early_buffered_lints::BufferedEarlyLintId`.
pub fn from_parser_lint_id(lint_id: BufferedEarlyLintId) -> &'static Self {
match lint_id {
pub fn from_parser_lint_id(lint_id: &BufferedEarlyLintId) -> &'static Self {
match *lint_id {
BufferedEarlyLintId::QuestionMarkMacroSep => QUESTION_MARK_MACRO_SEP,
BufferedEarlyLintId::IncorrectMacroFragmentRepetition {
..
} => INCORRECT_MACRO_FRAGMENT_REPETITION,
}
}

Expand All @@ -106,6 +109,25 @@ impl Lint {
.map(|(_, l)| l)
.unwrap_or(self.default_level)
}

pub fn builtin_diagnostic(lint_id: BufferedEarlyLintId) -> BuiltinLintDiagnostics {
match lint_id {
BufferedEarlyLintId::IncorrectMacroFragmentRepetition {
span,
token_span,
sugg_span,
frag,
possible,
} => BuiltinLintDiagnostics::IncorrectMacroFragmentRepetition {
span,
token_span,
sugg_span,
frag,
possible,
},
_ => BuiltinLintDiagnostics::Normal,
}
}
}

/// Declare a static item of type `&'static Lint`.
Expand Down
7 changes: 4 additions & 3 deletions src/librustc_driver/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1092,9 +1092,10 @@ where
// Add all buffered lints from the `ParseSess` to the `Session`.
sess.parse_sess.buffered_lints.with_lock(|buffered_lints| {
info!("{} parse sess buffered_lints", buffered_lints.len());
for BufferedEarlyLint{id, span, msg, lint_id} in buffered_lints.drain(..) {
let lint = lint::Lint::from_parser_lint_id(lint_id);
sess.buffer_lint(lint, id, span, &msg);
for BufferedEarlyLint {id, span, msg, lint_id} in buffered_lints.drain(..) {
let lint = lint::Lint::from_parser_lint_id(&lint_id);
let diag = lint::Lint::builtin_diagnostic(lint_id);
sess.buffer_lint_with_diagnostic(lint, id, span, &msg, diag);
}
});

Expand Down
3 changes: 1 addition & 2 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3977,8 +3977,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
}
}

let diag = lint::builtin::BuiltinLintDiagnostics
::AbsPathWithModule(diag_span);
let diag = lint::builtin::BuiltinLintDiagnostics::AbsPathWithModule(diag_span);
self.session.buffer_lint_with_diagnostic(
lint::builtin::ABSOLUTE_PATHS_NOT_STARTING_WITH_CRATE,
diag_id, diag_span,
Expand Down
9 changes: 8 additions & 1 deletion src/libsyntax/early_buffered_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,20 @@
//! redundant. Later, these types can be converted to types for use by the rest of the compiler.
use syntax::ast::NodeId;
use syntax_pos::MultiSpan;
use syntax_pos::{MultiSpan, Span};

/// Since we cannot import `LintId`s from `rustc::lint`, we define some Ids here which can later be
/// passed to `rustc::lint::Lint::from_parser_lint_id` to get a `rustc::lint::Lint`.
pub enum BufferedEarlyLintId {
/// Usage of `?` as a macro separator is deprecated.
QuestionMarkMacroSep,
IncorrectMacroFragmentRepetition {
span: Span,
token_span: Span,
sugg_span: Span,
frag: String,
possible: Vec<String>,
}
}

/// Stores buffered lint info which can later be passed to `librustc`.
Expand Down
77 changes: 13 additions & 64 deletions src/libsyntax/ext/tt/macro_rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

use {ast, attr};
use syntax_pos::{Span, DUMMY_SP};
use early_buffered_lints::BufferedEarlyLintId;
use edition::Edition;
use errors::FatalError;
use ext::base::{DummyResult, ExtCtxt, MacResult, SyntaxExtension};
Expand Down Expand Up @@ -812,14 +813,23 @@ fn check_matcher_core(sess: &ParseSess,
match is_in_follow(tok, &frag_spec.as_str()) {
IsInFollow::Invalid(..) | IsInFollow::Yes => {} // handled elsewhere
IsInFollow::No(possible) => {
let tok_sp = tok.span();
let next = if *sp == tok_sp {
let token_span = tok.span();
let next = if *sp == token_span {
"itself".to_owned()
} else {
quoted_tt_to_string(tok)
};
let mut err = sess.span_diagnostic.struct_span_warn(
let sugg_span = sess.source_map().next_point(delim_sp.close);
sess.buffer_lint(
BufferedEarlyLintId::IncorrectMacroFragmentRepetition {
span: *sp,
token_span: tok.span(),
sugg_span,
frag: frag_spec.to_string(),
possible: possible.into_iter().map(String::from).collect(),
},
*sp,
ast::CRATE_NODE_ID,
&format!(
"`${name}:{frag}` is followed (through repetition) by \
{next}, which is not allowed for `{frag}` fragments",
Expand All @@ -828,67 +838,6 @@ fn check_matcher_core(sess: &ParseSess,
next=next,
),
);
if *sp == tok_sp {
err.span_label(
*sp,
"this fragment is followed by itself without a valid \
separator",
);
} else {
err.span_label(
*sp,
"this fragment is followed by the first fragment in this \
repetition without a valid separator",
);
err.span_label(
tok_sp,
"this is the first fragment in the evaluated repetition",
);
}
let sugg_span = sess.source_map().next_point(delim_sp.close);
let msg = "allowed there are: ";
let sugg_msg =
"add a valid separator for the repetition to be unambiguous";
match &possible[..] {
&[] => {}
&[t] => {
err.note(&format!(
"only {} is allowed after `{}` fragments",
t,
frag_spec,
));
if t.starts_with('`') && t.ends_with('`') {
err.span_suggestion_with_applicability(
sugg_span,
&format!("{}, for example", sugg_msg),
(&t[1..t.len()-1]).to_owned(),
Applicability::MaybeIncorrect,
);
} else {
err.note(sugg_msg);
}
}
ts => {
err.note(&format!(
"{}{} or {}",
msg,
ts[..ts.len() - 1].iter().map(|s| *s)
.collect::<Vec<_>>().join(", "),
ts[ts.len() - 1],
));
if ts[0].starts_with('`') && ts[0].ends_with('`') {
err.span_suggestion_with_applicability(
sugg_span,
&format!("{}, for example", sugg_msg),
(&ts[0][1..ts[0].len()-1]).to_owned(),
Applicability::MaybeIncorrect,
);
} else {
err.note(sugg_msg);
}
}
}
err.emit();
}
}
}
Expand Down
5 changes: 3 additions & 2 deletions src/libsyntax/parse/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,14 +85,15 @@ impl ParseSess {
&self.source_map
}

pub fn buffer_lint<S: Into<MultiSpan>>(&self,
pub fn buffer_lint<S: Into<MultiSpan>>(
&self,
lint_id: BufferedEarlyLintId,
span: S,
id: NodeId,
msg: &str,
) {
self.buffered_lints.with_lock(|buffered_lints| {
buffered_lints.push(BufferedEarlyLint{
buffered_lints.push(BufferedEarlyLint {
span: span.into(),
id,
msg: msg.into(),
Expand Down
13 changes: 7 additions & 6 deletions src/test/ui/issues/issue-42755.stderr
Original file line number Diff line number Diff line change
@@ -1,20 +1,21 @@
error: repetition matches empty token tree
--> $DIR/issue-42755.rs:13:7
|
LL | ($($p:vis)*) => {}
| ^^^^^^^^

warning: `$p:vis` is followed (through repetition) by itself, which is not allowed for `vis` fragments
--> $DIR/issue-42755.rs:13:8
|
LL | ($($p:vis)*) => {}
| ^^^^^^ this fragment is followed by itself without a valid separator
|
= note: #[warn(incorrect_macro_fragment_repetition)] on by default
= note: allowed there are: `,`, an ident or a type
help: add a valid separator for the repetition to be unambiguous, for example
|
LL | ($($p:vis),*) => {}
| ^

error: repetition matches empty token tree
--> $DIR/issue-42755.rs:13:7
|
LL | ($($p:vis)*) => {}
| ^^^^^^^^

error: aborting due to previous error

1 change: 1 addition & 0 deletions src/test/ui/macros/incorrrect-repetition-2.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ warning: `$a:expr` is followed (through repetition) by itself, which is not allo
LL | ($($a:expr)*) => {};
| ^^^^^^^ this fragment is followed by itself without a valid separator
|
= note: #[warn(incorrect_macro_fragment_repetition)] on by default
= note: allowed there are: `;`, `=>` or `,`
help: add a valid separator for the repetition to be unambiguous, for example
|
Expand Down
1 change: 1 addition & 0 deletions src/test/ui/macros/incorrrect-repetition.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ LL | ($($i:ident $e:expr)*) => {}
| |
| this is the first fragment in the evaluated repetition
|
= note: #[warn(incorrect_macro_fragment_repetition)] on by default
= note: allowed there are: `;`, `=>` or `,`
help: add a valid separator for the repetition to be unambiguous, for example
|
Expand Down
25 changes: 13 additions & 12 deletions src/test/ui/macros/macro-input-future-proofing.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -70,18 +70,6 @@ LL | ( $a:expr $($b:tt)* ) => { };
|
= note: allowed there are: `;`, `=>` or `,`

warning: `$a:expr` is followed (through repetition) by itself, which is not allowed for `expr` fragments
--> $DIR/macro-input-future-proofing.rs:31:9
|
LL | ( $($a:expr)* $($b:tt)* ) => { };
| ^^^^^^^ this fragment is followed by itself without a valid separator
|
= note: allowed there are: `;`, `=>` or `,`
help: add a valid separator for the repetition to be unambiguous, for example
|
LL | ( $($a:expr);* $($b:tt)* ) => { };
| ^

error: `$a:expr` is followed by `$b:tt`, which is not allowed for `expr` fragments
--> $DIR/macro-input-future-proofing.rs:31:21
|
Expand All @@ -98,5 +86,18 @@ LL | ( $($a:expr),* $($b:tt)* ) => { };
|
= note: allowed there are: `;`, `=>` or `,`

warning: `$a:expr` is followed (through repetition) by itself, which is not allowed for `expr` fragments
--> $DIR/macro-input-future-proofing.rs:31:9
|
LL | ( $($a:expr)* $($b:tt)* ) => { };
| ^^^^^^^ this fragment is followed by itself without a valid separator
|
= note: #[warn(incorrect_macro_fragment_repetition)] on by default
= note: allowed there are: `;`, `=>` or `,`
help: add a valid separator for the repetition to be unambiguous, for example
|
LL | ( $($a:expr);* $($b:tt)* ) => { };
| ^

error: aborting due to 11 previous errors

0 comments on commit ca5e89d

Please sign in to comment.