Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow patternless error and warning matchers #165

Merged
merged 2 commits into from
Nov 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

* Started maintaining a changelog
* `Config::comment_defaults` allows setting `//@` comments for all tests
* `//~` comments can now specify just an error code or lint name, without any message. ERROR level is implied
* `Revisioned::diagnostic_code_prefix` allows stripping a prefix of diagnostic codes to avoid having to repeat `clippy::` in all messages

### Fixed

Expand Down
19 changes: 10 additions & 9 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,17 @@ A stable 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`.
oli-obk marked this conversation as resolved.
Show resolved Hide resolved
* This will only match a diagnostic at the `ERROR` level.

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
7 changes: 7 additions & 0 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,13 @@ pub enum Error {
/// Can be `None` when it is expected outside the current file
expected_line: Option<NonZeroUsize>,
},
/// A diagnostic code matcher was declared but had no matching error.
CodeNotFound {
/// The code that was not found, and the span of where that code was declared.
code: Spanned<String>,
/// Can be `None` when it is expected outside the current file
expected_line: Option<NonZeroUsize>,
},
/// A ui test checking for failure does not have any failure patterns
NoPatternsFound,
/// A ui test checking for success has failure patterns
Expand Down
78 changes: 57 additions & 21 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, OptWithLine, Revisioned, Spanned};
use parser::{ErrorMatch, ErrorMatchKind, OptWithLine, Revisioned, Spanned};
use regex::bytes::{Captures, Regex};
use rustc_stderr::{Level, Message};
use spanned::Span;
Expand Down Expand Up @@ -915,6 +915,7 @@ fn run_rustfix(
edition,
mode: OptWithLine::new(Mode::Pass, Span::default()),
no_rustfix: OptWithLine::new((), Span::default()),
diagnostic_code_prefix: OptWithLine::new(String::new(), Span::default()),
needs_asm_support: false,
},
))
Expand Down Expand Up @@ -1071,40 +1072,75 @@ fn check_annotations(
});
}
}
let diagnostic_code_prefix = comments
.find_one_for_revision(revision, "diagnostic_code_prefix", |r| {
r.diagnostic_code_prefix.clone()
})?
.into_inner()
.map(|s| s.content)
.unwrap_or_default();

// 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
'err: 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) => {
for (i, msg) in msgs.iter().enumerate() {
if msg.level != Level::Error {
continue;
}
let Some(msg_code) = &msg.code else { continue };
let Some(msg) = msg_code.strip_prefix(&diagnostic_code_prefix) else {
continue;
};
if msg == **code {
msgs.remove(i);
continue 'err;
}
}
}
}
}

errors.push(Error::PatternNotFound {
pattern: pattern.clone(),
expected_line: Some(line),
errors.push(match kind {
ErrorMatchKind::Pattern { pattern, .. } => Error::PatternNotFound {
pattern: pattern.clone(),
expected_line: Some(line),
},
ErrorMatchKind::Code(code) => Error::CodeNotFound {
oli-obk marked this conversation as resolved.
Show resolved Hide resolved
code: Spanned::new(format!("{}{}", diagnostic_code_prefix, **code), code.span()),
expected_line: Some(line),
},
});
}

Expand Down
98 changes: 64 additions & 34 deletions src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,9 @@ pub struct Revisioned {
pub(crate) needs_asm_support: bool,
/// Don't run [`rustfix`] for this test
pub no_rustfix: OptWithLine<()>,
/// Prefix added to all diagnostic code matchers. Note this will make it impossible
/// match codes which do not contain this prefix.
pub diagnostic_code_prefix: OptWithLine<String>,
}

#[derive(Debug)]
Expand Down Expand Up @@ -212,10 +215,20 @@ pub enum Pattern {
Regex(Regex),
}

#[derive(Debug, Clone)]
pub(crate) 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>),
}

#[derive(Debug, Clone)]
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 @@ -302,6 +315,7 @@ impl Comments {
}
}
}

let Revisioned {
span,
ignore,
Expand All @@ -319,6 +333,7 @@ impl Comments {
mode,
needs_asm_support,
no_rustfix,
diagnostic_code_prefix,
} = parser.comments.base();
if span.is_dummy() {
*span = defaults.span;
Expand All @@ -345,6 +360,9 @@ impl Comments {
if no_rustfix.is_none() {
*no_rustfix = defaults.no_rustfix;
}
if diagnostic_code_prefix.is_none() {
*diagnostic_code_prefix = defaults.diagnostic_code_prefix;
}
*needs_asm_support |= defaults.needs_asm_support;

if parser.errors.is_empty() {
Expand Down Expand Up @@ -788,7 +806,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 @@ -831,43 +852,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(level_or_code.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,7 +1,7 @@
use std::path::Path;

use crate::{
parser::{Condition, Pattern},
parser::{Condition, ErrorMatchKind, Pattern},
Error,
};

Expand All @@ -20,8 +20,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 @@ -32,6 +35,24 @@ fn main() {
}
}

#[test]
fn parse_error_code_comment() {
let s = r"
fn main() {
let _x: i32 = 0u32; //~ E0308
}
";
let comments = Comments::parse(s, Comments::default(), Path::new("")).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 diagnostic code matcher");
};
assert_eq!(code.line().get(), 3);
assert_eq!(**code, "E0308");
}

#[test]
fn parse_missing_level() {
let s = r"
Expand All @@ -46,7 +67,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
Loading