Merge pull request #251 from oli-obk/cleanups

Fully switch to byte based offsets

Author: oli-obk

CHANGELOG.md

@@ -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
 

Cargo.lock

@@ -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"
@@ -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"

Cargo.toml

@@ -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 {
@@ -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,
@@ -84,20 +82,21 @@ 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 = std::fs::read(&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![],
-        })?;
-        let comments = Comments::parse(&file_contents, &config, &self.aux_file)
+        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![],
+            })?;
+        let comments = Comments::parse(file_contents.as_ref(), &config)
             .map_err(|errors| Errored::new(errors, "parse aux comments"))?;
         assert_eq!(
             comments.revisions, None,
             "aux builds cannot specify revisions"
         );
 
-        default_per_file_config(&mut config, &self.aux_file, &file_contents);
+        default_per_file_config(&mut config, &file_contents);
 
         match CrateType::from_file_contents(&file_contents) {
             // Proc macros must be run on the host

src/aux_builds.rs

@@ -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,
@@ -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);
         }
     }

src/custom_flags/run.rs

@@ -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(),
                     ),
                 }],

src/custom_flags/rustfix.rs

@@ -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>,
 }

src/diagnostics.rs

@@ -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 {
@@ -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 {
@@ -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),

src/error.rs

@@ -18,6 +18,7 @@ pub use core::run_and_collect;
 pub use core::CrateType;
 pub use filter::Match;
 use per_test_config::TestConfig;
+use spanned::Spanned;
 use status_emitter::{StatusEmitter, TestStatus};
 use std::collections::VecDeque;
 use std::path::Path;
@@ -117,7 +118,7 @@ pub fn default_any_file_filter(path: &Path, config: &Config) -> bool {
 }
 
 /// The default per-file config used by `run_tests`.
-pub fn default_per_file_config(config: &mut Config, _path: &Path, file_contents: &[u8]) {
+pub fn default_per_file_config(config: &mut Config, file_contents: &Spanned<Vec<u8>>) {
     config.program.args.push(
         match CrateType::from_file_contents(file_contents) {
             CrateType::ProcMacro => "--crate-type=proc-macro",
@@ -137,9 +138,9 @@ pub fn test_command(mut config: Config, path: &Path) -> Result<Command> {
 
     config.fill_host_and_target()?;
 
-    let content =
-        std::fs::read(path).wrap_err_with(|| format!("failed to read {}", display(path)))?;
-    let comments = Comments::parse(&content, &config, path)
+    let content = Spanned::read_from_file(path)
+        .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 {
         config,
@@ -166,7 +167,7 @@ pub fn test_command(mut config: Config, path: &Path) -> Result<Command> {
 pub fn run_tests_generic(
     mut configs: Vec<Config>,
     file_filter: impl Fn(&Path, &Config) -> Option<bool> + Sync,
-    per_file_config: impl Fn(&mut Config, &Path, &[u8]) + Sync,
+    per_file_config: impl Fn(&mut Config, &Spanned<Vec<u8>>) + Sync,
     status_emitter: impl StatusEmitter + Send,
 ) -> Result<()> {
     if nextest::emulate(&mut configs) {
@@ -232,9 +233,9 @@ pub fn run_tests_generic(
         |receive, finished_files_sender| -> Result<()> {
             for (status, build_manager) in receive {
                 let path = status.path();
-                let file_contents = std::fs::read(path).unwrap();
+                let file_contents = Spanned::read_from_file(path).unwrap();
                 let mut config = build_manager.config().clone();
-                per_file_config(&mut config, path, &file_contents);
+                per_file_config(&mut config, &file_contents);
                 let result = match std::panic::catch_unwind(|| {
                     parse_and_test_file(build_manager, &status, config, file_contents)
                 }) {
@@ -317,9 +318,9 @@ fn parse_and_test_file(
     build_manager: &BuildManager<'_>,
     status: &dyn TestStatus,
     config: Config,
-    file_contents: Vec<u8>,
+    file_contents: Spanned<Vec<u8>>,
 ) -> Result<Vec<TestRun>, Errored> {
-    let comments = Comments::parse(&file_contents, &config, status.path())
+    let comments = Comments::parse(file_contents.as_ref(), &config)
         .map_err(|errors| Errored::new(errors, "parse comments"))?;
     const EMPTY: &[String] = &[String::new()];
     // Run the test for all revisions