Skip to content

Commit

Permalink
Fully switch to byte based offsets
Browse files Browse the repository at this point in the history
  • Loading branch information
oli-obk committed Jul 24, 2024
1 parent 6d845d5 commit 66a484c
Show file tree
Hide file tree
Showing 25 changed files with 295 additions and 209 deletions.
12 changes: 12 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Removed

## [0.25.0] - 2024-07-24

### Added

### Fixed

* panics when ui_test tried to show diagnostics on multi-byte chars

### Changed

### Removed


## [0.24.0] - 2024-07-11

Expand Down
6 changes: 3 additions & 3 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "ui_test"
version = "0.24.0"
version = "0.25.0"
edition = "2021"
license = "MIT OR Apache-2.0"
description = "A test framework for testing rustc diagnostics output"
Expand Down Expand Up @@ -28,7 +28,7 @@ indicatif = "0.17.6"
prettydiff = { version = "0.7", default-features = false }
annotate-snippets = { version = "0.11.2" }
levenshtein = "1.0.5"
spanned = "0.2.1"
spanned = "0.3.0"

[dependencies.regex]
version = "1.5.5"
Expand Down
11 changes: 4 additions & 7 deletions src/aux_builds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ impl Flag for AuxBuilder {
) -> Result<(), Errored> {
let aux = &self.aux_file;
let aux_dir = config.aux_dir;
let line = aux.line();
let aux_file = if aux.starts_with("..") {
aux_dir.parent().unwrap().join(&aux.content)
} else {
Expand Down Expand Up @@ -60,9 +59,8 @@ impl Flag for AuxBuilder {
}| Errored {
command,
errors: vec![Error::Aux {
path: aux_file.to_path_buf(),
path: Spanned::new(aux_file.to_path_buf(), aux.span()),
errors,
line,
}],
stderr,
stdout,
Expand All @@ -84,14 +82,13 @@ pub struct AuxBuilder {
impl Build for AuxBuilder {
fn build(&self, build_manager: &BuildManager<'_>) -> Result<Vec<OsString>, Errored> {
let mut config = build_manager.config().clone();
let file_contents = Spanned::read_from_file(&self.aux_file.content)
.map_err(|err| Errored {
let file_contents =
Spanned::read_from_file(&self.aux_file.content).map_err(|err| Errored {
command: format!("reading aux file `{}`", display(&self.aux_file)),
errors: vec![],
stderr: err.to_string().into_bytes(),
stdout: vec![],
})?
.map(|s| s.into_bytes());
})?;
let comments = Comments::parse(file_contents.as_ref(), &config)
.map_err(|errors| Errored::new(errors, "parse aux comments"))?;
assert_eq!(
Expand Down
29 changes: 16 additions & 13 deletions src/custom_flags/run.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
//! Types used for running tests after they pass compilation
use bstr::ByteSlice;
use spanned::{Span, Spanned};
use std::{
path::PathBuf,
process::{Command, Output},
};
use spanned::Spanned;
use std::process::{Command, Output};

use crate::{
build_manager::BuildManager, display, per_test_config::TestConfig, Error, Errored, TestOk,
Expand Down Expand Up @@ -116,19 +113,25 @@ fn get_panic_span(stderr: &[u8]) -> Spanned<String> {
let Ok(filename) = filename.to_str() else {
continue;
};
let Ok(line) = line.parse() else {
let Ok(line) = line.parse::<usize>() else {
continue;
};
let Ok(col) = col.parse::<usize>() else {
continue;
};
let Ok(file) = Spanned::read_from_file(filename) else {
continue;
};
let Some(line) = line.checked_sub(1) else {
continue;
};
let Ok(col) = col.parse() else {
let Some(line) = file.lines().nth(line) else {
continue;
};
let span = Span {
file: PathBuf::from(filename),
line_start: line,
line_end: line,
col_start: col,
col_end: col,
let Some(col) = col.checked_sub(1) else {
continue;
};
let span = line.span.inc_col_start(col);
return Spanned::new(message.into(), span);
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/custom_flags/rustfix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ fn compile_fixed(
.iter()
.flatten()
.chain(diagnostics.messages_from_unknown_file_or_line.iter())
.find_map(|message| message.line_col.clone())
.find_map(|message| message.span.clone())
.unwrap_or_default(),
),
}],
Expand Down
4 changes: 3 additions & 1 deletion src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,9 @@ pub struct Message {
/// The main message of the diagnostic (what will be matched for with `//~`)
pub message: String,
/// Information about where in the file the message was emitted
pub line_col: Option<spanned::Span>,
pub line: Option<usize>,
/// Exact span information of the message
pub span: Option<spanned::Span>,
/// Identifier of the message (E0XXX for rustc errors, or lint names)
pub code: Option<String>,
}
Expand Down
8 changes: 3 additions & 5 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ pub enum Error {
/// The main message of the error.
msgs: Vec<Message>,
/// File and line information of the error.
path: Option<Spanned<PathBuf>>,
path: Option<(PathBuf, NonZeroUsize)>,
},
/// A comment failed to parse.
InvalidComment {
Expand All @@ -72,7 +72,7 @@ pub enum Error {
/// The comment being looked for
kind: String,
/// The lines where conflicts happened
lines: Vec<NonZeroUsize>,
lines: Vec<Span>,
},
/// A subcommand (e.g. rustfix) of a test failed.
Command {
Expand All @@ -86,11 +86,9 @@ pub enum Error {
/// An auxiliary build failed with its own set of errors.
Aux {
/// Path to the aux file.
path: PathBuf,
path: Spanned<PathBuf>,
/// The errors that occurred during the build of the aux file.
errors: Vec<Error>,
/// The line in which the aux file was requested to be built.
line: NonZeroUsize,
},
/// An error occured applying [`rustfix`] suggestions
Rustfix(anyhow::Error),
Expand Down
7 changes: 2 additions & 5 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,7 @@ pub fn test_command(mut config: Config, path: &Path) -> Result<Command> {
config.fill_host_and_target()?;

let content = Spanned::read_from_file(path)
.wrap_err_with(|| format!("failed to read {}", display(path)))?
.map(|s| s.into_bytes());
.wrap_err_with(|| format!("failed to read {}", display(path)))?;
let comments = Comments::parse(content.as_ref(), &config)
.map_err(|errors| color_eyre::eyre::eyre!("{errors:#?}"))?;
let config = TestConfig {
Expand Down Expand Up @@ -234,9 +233,7 @@ pub fn run_tests_generic(
|receive, finished_files_sender| -> Result<()> {
for (status, build_manager) in receive {
let path = status.path();
let file_contents = Spanned::read_from_file(path)
.unwrap()
.map(|s| s.into_bytes());
let file_contents = Spanned::read_from_file(path).unwrap();
let mut config = build_manager.config().clone();
per_file_config(&mut config, &file_contents);
let result = match std::panic::catch_unwind(|| {
Expand Down
4 changes: 2 additions & 2 deletions src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ impl Comments {
}
if let Some(found) = f(rev).into_inner() {
if result.is_some() {
errors.push(found.line());
errors.push(found.span);
} else {
result = found.into();
}
Expand Down Expand Up @@ -911,7 +911,7 @@ impl CommentParser<&mut Revisioned> {
self.error(pattern.span(), format!(
"//~^ pattern is trying to refer to {} lines above, but there are only {} lines above",
offset,
pattern.line().get() - 1,
current_line.get() - 1,
));
return ParsePatternResult::ErrorAbove {
match_line: current_line,
Expand Down
27 changes: 19 additions & 8 deletions src/parser/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,17 @@ use crate::{

use super::Comments;

macro_rules! line {
($thing:expr, $s:expr) => {{
let file = Spanned::dummy($s.as_bytes());
let pos = file
.lines()
.position(|line| line.span.bytes.contains(&$thing.bytes.start))
.unwrap();
pos
}};
}

#[test]
fn parse_simple_comment() {
let s = r"
Expand All @@ -23,7 +34,7 @@ fn main() {
let ErrorMatchKind::Pattern { pattern, .. } = &revisioned.error_matches[0].kind else {
panic!("expected pattern matcher");
};
assert_eq!(pattern.line().get(), 5);
assert_ne!(line!(&pattern.span, s), 5);
match &**pattern {
Pattern::SubString(s) => {
assert_eq!(
Expand All @@ -49,7 +60,7 @@ fn main() {
let ErrorMatchKind::Code(code) = &revisioned.error_matches[0].kind else {
panic!("expected diagnostic code matcher");
};
assert_eq!(code.line().get(), 3);
assert_eq!(line!(&code.span, s), 3);
assert_eq!(**code, "E0308");
}

Expand All @@ -66,7 +77,7 @@ fn main() {
println!("parsed comments: {:#?}", errors);
assert_eq!(errors.len(), 1);
match &errors[0] {
Error::InvalidComment { msg, span } if span.line_start.get() == 5 => {
Error::InvalidComment { msg, span } if line!(span, s) == 5 => {
assert_eq!(msg, "text found after error code `encountered`")
}
_ => unreachable!(),
Expand All @@ -86,7 +97,7 @@ use std::mem;
let revisioned = &comments.revisioned[&vec![]];
let pat = &revisioned.error_in_other_files[0];
assert_eq!(format!("{:?}", **pat), r#"SubString("foomp")"#);
assert_eq!(pat.line().get(), 2);
assert_eq!(line!(pat.span, s), 2);
}

#[test]
Expand All @@ -102,7 +113,7 @@ use std::mem;
let revisioned = &comments.revisioned[&vec![]];
let pat = &revisioned.error_in_other_files[0];
assert_eq!(format!("{:?}", **pat), r#"Regex(Regex("foomp"))"#);
assert_eq!(pat.line().get(), 2);
assert_eq!(line!(pat.span, s), 2);
}

#[test]
Expand All @@ -116,13 +127,13 @@ use std::mem;
println!("parsed comments: {:#?}", errors);
assert_eq!(errors.len(), 2);
match &errors[0] {
Error::InvalidComment { msg, span } if span.line_start.get() == 2 => {
Error::InvalidComment { msg, span } if line!(span, s) == 2 => {
assert!(msg.contains("must be followed by `:`"))
}
_ => unreachable!(),
}
match &errors[1] {
Error::InvalidComment { msg, span } if span.line_start.get() == 2 => {
Error::InvalidComment { msg, span } if line!(span, s) == 2 => {
assert_eq!(msg, "`error-patttern` is not a command known to `ui_test`, did you mean `error-pattern`?");
}
_ => unreachable!(),
Expand All @@ -140,7 +151,7 @@ use std::mem;
println!("parsed comments: {:#?}", errors);
assert_eq!(errors.len(), 1);
match &errors[0] {
Error::InvalidComment { msg, span } if span.line_start.get() == 2 => {
Error::InvalidComment { msg, span } if line!(span, s) == 2 => {
assert!(msg.contains("must be followed by `:`"))
}
_ => unreachable!(),
Expand Down
8 changes: 1 addition & 7 deletions src/per_test_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -368,13 +368,7 @@ impl TestConfig<'_> {
if !msgs.is_empty() {
let line = NonZeroUsize::new(line).expect("line 0 is always empty");
errors.push(Error::ErrorsWithoutPattern {
path: Some(Spanned::new(
self.status.path().to_path_buf(),
spanned::Span {
line_start: line,
..spanned::Span::default()
},
)),
path: Some((self.status.path().to_path_buf(), line)),
msgs,
});
}
Expand Down
Loading

0 comments on commit 66a484c

Please sign in to comment.