Skip to content

Commit

Permalink
Merge pull request #165 from Jarcho/no_msg
Browse files Browse the repository at this point in the history
Allow patternless error and warning matchers
  • Loading branch information
oli-obk authored Nov 8, 2023
2 parents c239bb8 + b69da28 commit c6ec6d9
Show file tree
Hide file tree
Showing 16 changed files with 505 additions and 79 deletions.
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`.
* 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 {
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

0 comments on commit c6ec6d9

Please sign in to comment.