Skip to content

Commit

Permalink
Allow matching errors and warnings by error code.
Browse files Browse the repository at this point in the history
  • Loading branch information
Jarcho committed Oct 1, 2023
1 parent 3d262a2 commit 1c3aab1
Show file tree
Hide file tree
Showing 14 changed files with 315 additions and 86 deletions.
18 changes: 9 additions & 9 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,16 @@ A smaller version of compiletest-rs
## Supported magic comment annotations

If your test tests for failure, you need to add a `//~` annotation where the error is happening
to make sure that the test will always keep failing with a specific message at the annotated line.
to ensure that the test will always keep failing at the annotated line. These comments can take two forms:

`//~ ERROR: XXX` make sure the stderr output contains `XXX` for an error in the line where this comment is written

* Also supports `HELP`, `WARN` or `NOTE` for different kind of message
* if one of those levels is specified explicitly, *all* diagnostics of this level or higher need an annotation. If you want to avoid this, just leave out the all caps level note entirely.
* If the all caps note is left out, a message of any level is matched. Leaving it out is not allowed for `ERROR` levels.
* This checks the output *before* normalization, so you can check things that get normalized away, but need to
be careful not to accidentally have a pattern that differs between platforms.
* if `XXX` is of the form `/XXX/` it is treated as a regex instead of a substring and will succeed if the regex matches.
* `//~ LEVEL: XXX` matches by error level and message text
* `LEVEL` can be one of the following (descending order): `ERROR`, `HELP`, `WARN` or `NOTE`
* If a level is specified explicitly, *all* diagnostics of that level or higher need an annotation. To avoid this see `//@require-annotations-for-level`
* This checks the output *before* normalization, so you can check things that get normalized away, but need to
be careful not to accidentally have a pattern that differs between platforms.
* if `XXX` is of the form `/XXX/` it is treated as a regex instead of a substring and will succeed if the regex matches.
* `//~ CODE` matches by diagnostic code.
* `CODE` can take multiple forms such as: `E####`, `lint_name`, `tool::lint_name`.

In order to change how a single test is tested, you can add various `//@` comments to the test.
Any other comments will be ignored, and all `//@` comments must be formatted precisely as
Expand Down
4 changes: 2 additions & 2 deletions src/error.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::{
parser::{Pattern, Spanned},
parser::{ErrorMatchKind, Spanned},
rustc_stderr::{Message, Span},
Mode,
};
Expand All @@ -19,7 +19,7 @@ pub enum Error {
expected: i32,
},
/// A pattern was declared but had no matching error.
PatternNotFound(Spanned<Pattern>),
PatternNotFound(ErrorMatchKind),
/// A ui test checking for failure does not have any failure patterns
NoPatternsFound,
/// A ui test checking for success has failure patterns
Expand Down
60 changes: 40 additions & 20 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use color_eyre::eyre::{eyre, Result};
use crossbeam_channel::{unbounded, Receiver, Sender};
use dependencies::{Build, BuildManager};
use lazy_static::lazy_static;
use parser::{ErrorMatch, MaybeSpanned, OptWithLine, Revisioned, Spanned};
use parser::{ErrorMatch, ErrorMatchKind, MaybeSpanned, OptWithLine, Revisioned, Spanned};
use regex::bytes::{Captures, Regex};
use rustc_stderr::{Level, Message, Span};
use status_emitter::{StatusEmitter, TestStatus};
Expand Down Expand Up @@ -1079,41 +1079,61 @@ fn check_annotations(
{
messages_from_unknown_file_or_line.remove(i);
} else {
errors.push(Error::PatternNotFound(error_pattern.clone()));
errors.push(Error::PatternNotFound(ErrorMatchKind::Pattern {
pattern: error_pattern.clone(),
level: Level::Error,
}));
}
}

// The order on `Level` is such that `Error` is the highest level.
// We will ensure that *all* diagnostics of level at least `lowest_annotation_level`
// are matched.
let mut lowest_annotation_level = Level::Error;
for &ErrorMatch {
ref pattern,
level,
line,
} in comments
for &ErrorMatch { ref kind, line } in comments
.for_revision(revision)
.flat_map(|r| r.error_matches.iter())
{
seen_error_match = Some(pattern.span());
// If we found a diagnostic with a level annotation, make sure that all
// diagnostics of that level have annotations, even if we don't end up finding a matching diagnostic
// for this pattern.
if lowest_annotation_level > level {
lowest_annotation_level = level;
match kind {
ErrorMatchKind::Code(code) => {
seen_error_match = Some(code.span());
}
&ErrorMatchKind::Pattern { ref pattern, level } => {
seen_error_match = Some(pattern.span());
// If we found a diagnostic with a level annotation, make sure that all
// diagnostics of that level have annotations, even if we don't end up finding a matching diagnostic
// for this pattern.
if lowest_annotation_level > level {
lowest_annotation_level = level;
}
}
}

if let Some(msgs) = messages.get_mut(line.get()) {
let found = msgs
.iter()
.position(|msg| pattern.matches(&msg.message) && msg.level == level);
if let Some(found) = found {
msgs.remove(found);
continue;
match kind {
&ErrorMatchKind::Pattern { ref pattern, level } => {
let found = msgs
.iter()
.position(|msg| pattern.matches(&msg.message) && msg.level == level);
if let Some(found) = found {
msgs.remove(found);
continue;
}
}
ErrorMatchKind::Code(code) => {
let found = msgs.iter().position(|msg| {
msg.level >= Level::Warn
&& msg.code.as_ref().is_some_and(|msg| *msg == **code)
});
if let Some(found) = found {
msgs.remove(found);
continue;
}
}
}
}

errors.push(Error::PatternNotFound(pattern.clone()));
errors.push(Error::PatternNotFound(kind.clone()));
}

let required_annotation_level = comments.find_one_for_revision(
Expand Down
99 changes: 65 additions & 34 deletions src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,10 +178,28 @@ pub enum Pattern {
Regex(Regex),
}

#[derive(Debug, Clone)]
pub enum ErrorMatchKind {
/// A level and pattern pair parsed from a `//~ LEVEL: Message` comment.
Pattern {
pattern: Spanned<Pattern>,
level: Level,
},
/// An error code parsed from a `//~ error_code` comment.
Code(Spanned<String>),
}
impl ErrorMatchKind {
pub(crate) fn span(&self) -> Span {
match self {
Self::Pattern { pattern, .. } => pattern.span(),
Self::Code(code) => code.span(),
}
}
}

#[derive(Debug)]
pub(crate) struct ErrorMatch {
pub pattern: Spanned<Pattern>,
pub level: Level,
pub(crate) kind: ErrorMatchKind,
/// The line this pattern is expecting to find a message in.
pub line: NonZeroUsize,
}
Expand Down Expand Up @@ -260,6 +278,7 @@ impl Comments {
}
}
}

if parser.errors.is_empty() {
Ok(parser.comments)
} else {
Expand Down Expand Up @@ -707,7 +726,10 @@ impl<CommentsType> CommentParser<CommentsType> {
}

impl CommentParser<&mut Revisioned> {
// parse something like (\[[a-z]+(,[a-z]+)*\])?(?P<offset>\||[\^]+)? *(?P<level>ERROR|HELP|WARN|NOTE): (?P<text>.*)
// parse something like:
// (\[[a-z]+(,[a-z]+)*\])?
// (?P<offset>\||[\^]+)? *
// ((?P<level>ERROR|HELP|WARN|NOTE): (?P<text>.*))|(?P<code>[a-z0-9_:]+)
fn parse_pattern(&mut self, pattern: Spanned<&str>, fallthrough_to: &mut Option<NonZeroUsize>) {
let (match_line, pattern) = match pattern.chars().next() {
Some('|') => (
Expand Down Expand Up @@ -750,43 +772,52 @@ impl CommentParser<&mut Revisioned> {
};

let pattern = pattern.trim_start();
let offset = match pattern.chars().position(|c| !c.is_ascii_alphabetic()) {
Some(offset) => offset,
None => {
self.error(pattern.span(), "pattern without level");
return;
}
};
let offset = pattern
.bytes()
.position(|c| !(c.is_ascii_alphanumeric() || c == b'_' || c == b':'))
.unwrap_or(pattern.len());

let (level_or_code, pattern) = pattern.split_at(offset);
if let Some(level) = level_or_code.strip_suffix(":") {
let level = match (*level).parse() {
Ok(level) => level,
Err(msg) => {
self.error(level.span(), msg);
return;
}
};

let (level, pattern) = pattern.split_at(offset);
let level = match (*level).parse() {
Ok(level) => level,
Err(msg) => {
self.error(level.span(), msg);
return;
}
};
let pattern = match pattern.strip_prefix(":") {
Some(offset) => offset,
None => {
self.error(pattern.span(), "no `:` after level found");
return;
}
};
let pattern = pattern.trim();

let pattern = pattern.trim();
self.check(pattern.span(), !pattern.is_empty(), "no pattern specified");

self.check(pattern.span(), !pattern.is_empty(), "no pattern specified");
let pattern = self.parse_error_pattern(pattern);

let pattern = self.parse_error_pattern(pattern);
self.error_matches.push(ErrorMatch {
kind: ErrorMatchKind::Pattern { pattern, level },
line: match_line,
});
} else if (*level_or_code).parse::<Level>().is_ok() {
// Shouldn't conflict with any real diagnostic code
self.error(pattern.span(), "no `:` after level found");
return;
} else if !pattern.trim_start().is_empty() {
self.error(
pattern.span(),
format!("text found after error code `{}`", *level_or_code),
);
return;
} else {
self.error_matches.push(ErrorMatch {
kind: ErrorMatchKind::Code(Spanned::new(
level_or_code.to_string(),
level_or_code.span(),
)),
line: match_line,
});
};

*fallthrough_to = Some(match_line);

self.error_matches.push(ErrorMatch {
pattern,
level,
line: match_line,
});
}
}

Expand Down
29 changes: 25 additions & 4 deletions src/parser/tests.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::{
parser::{Condition, Pattern},
parser::{Condition, ErrorMatchKind, Pattern},
Error,
};

Expand All @@ -18,8 +18,11 @@ fn main() {
println!("parsed comments: {:#?}", comments);
assert_eq!(comments.revisioned.len(), 1);
let revisioned = &comments.revisioned[&vec![]];
assert_eq!(revisioned.error_matches[0].pattern.line().get(), 5);
match &*revisioned.error_matches[0].pattern {
let ErrorMatchKind::Pattern { pattern, .. } = &revisioned.error_matches[0].kind else {
panic!("expected pattern matcher");
};
assert_eq!(pattern.line().get(), 5);
match &**pattern {
Pattern::SubString(s) => {
assert_eq!(
s,
Expand All @@ -30,6 +33,24 @@ fn main() {
}
}

#[test]
fn parse_error_code_comment() {
let s = r"
fn main() {
let _x: i32 = 0u32; //~ E0308
}
";
let comments = Comments::parse(s).unwrap();
println!("parsed comments: {:#?}", comments);
assert_eq!(comments.revisioned.len(), 1);
let revisioned = &comments.revisioned[&vec![]];
let ErrorMatchKind::Code(code) = &revisioned.error_matches[0].kind else {
panic!("expected error code matcher");
};
assert_eq!(code.line().get(), 3);
assert_eq!(**code, "E0308");
}

#[test]
fn parse_missing_level() {
let s = r"
Expand All @@ -44,7 +65,7 @@ fn main() {
assert_eq!(errors.len(), 1);
match &errors[0] {
Error::InvalidComment { msg, span } if span.line_start.get() == 5 => {
assert_eq!(msg, "unknown level `encountered`")
assert_eq!(msg, "text found after error code `encountered`")
}
_ => unreachable!(),
}
Expand Down
10 changes: 9 additions & 1 deletion src/rustc_stderr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,23 @@ use std::{
path::{Path, PathBuf},
};

#[derive(serde::Deserialize, Debug)]
struct RustcDiagnosticCode {
code: String,
}

#[derive(serde::Deserialize, Debug)]
struct RustcMessage {
rendered: Option<String>,
spans: Vec<RustcSpan>,
level: String,
message: String,
children: Vec<RustcMessage>,
code: Option<RustcDiagnosticCode>,
}

#[derive(Copy, Clone, Debug, PartialOrd, Ord, PartialEq, Eq)]
pub(crate) enum Level {
pub enum Level {
Ice = 5,
Error = 4,
Warn = 3,
Expand All @@ -31,6 +37,7 @@ pub struct Message {
pub(crate) level: Level,
pub(crate) message: String,
pub(crate) line_col: Option<Span>,
pub(crate) code: Option<String>,
}

/// Information about macro expansion.
Expand Down Expand Up @@ -126,6 +133,7 @@ impl RustcMessage {
level: self.level.parse().unwrap(),
message: self.message,
line_col: line,
code: self.code.map(|x| x.code),
};
if let Some(line) = line {
if messages.len() <= line.line_start.get() {
Expand Down
Loading

0 comments on commit 1c3aab1

Please sign in to comment.