From d980a3e9dab5f93feeb7c7850502ee55455a7f27 Mon Sep 17 00:00:00 2001 From: Aria Desires Date: Thu, 6 Aug 2026 11:26:44 -0400 Subject: [PATCH 1/3] [ty] Add Git-aware diagnostic diff mode --- Cargo.lock | 2 + crates/ty/Cargo.toml | 2 + crates/ty/docs/cli.md | 4 +- crates/ty/src/args.rs | 14 + crates/ty/src/git.rs | 650 ++++++++++++++++++++++++++++++++++++ crates/ty/src/lib.rs | 54 ++- crates/ty/tests/cli/diff.rs | 309 +++++++++++++++++ crates/ty/tests/cli/main.rs | 1 + 8 files changed, 1026 insertions(+), 10 deletions(-) create mode 100644 crates/ty/src/git.rs create mode 100644 crates/ty/tests/cli/diff.rs diff --git a/Cargo.lock b/Cargo.lock index 271125300f9aef..717ca5fe92b97c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4609,12 +4609,14 @@ dependencies = [ "regex", "ruff_db", "ruff_diagnostics", + "ruff_notebook", "ruff_python_ast", "ruff_python_trivia", "ruff_ranged_value", "salsa", "serde", "serde_json", + "similar 3.1.1", "tempfile", "tikv-jemallocator", "toml 1.1.3+spec-1.1.0", diff --git a/crates/ty/Cargo.toml b/crates/ty/Cargo.toml index e9bc06c209e505..933450943468a3 100644 --- a/crates/ty/Cargo.toml +++ b/crates/ty/Cargo.toml @@ -20,6 +20,7 @@ doctest = false [dependencies] ruff_db = { workspace = true, features = ["os", "cache", "junit"] } ruff_diagnostics = { workspace = true } +ruff_notebook = { workspace = true } ruff_ranged_value = { workspace = true } ty_combine = { workspace = true } ty_project = { workspace = true, features = ["zstd", "junit"] } @@ -41,6 +42,7 @@ rayon = { workspace = true } salsa = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } +similar = { workspace = true } tracing = { workspace = true, features = ["release_max_level_debug"] } tracing-flame = { workspace = true } tracing-subscriber = { workspace = true } diff --git a/crates/ty/docs/cli.md b/crates/ty/docs/cli.md index c8306663a3c592..cc26f198f8470d 100644 --- a/crates/ty/docs/cli.md +++ b/crates/ty/docs/cli.md @@ -51,7 +51,9 @@ overriding a specific configuration option.

over all configuration files.

--config-file path

The path to a ty.toml file to use for configuration.

While ty configuration can be included in a pyproject.toml file, it is not allowed in this context.

-

May also be set with the TY_CONFIG_FILE environment variable.

--error rule

Treat the given rule as having severity 'error'. Can be specified multiple times. Use 'all' to apply to all rules.

+

May also be set with the TY_CONFIG_FILE environment variable.

--diff revision

Only report diagnostics introduced since a Git revision.

+

The project is checked at the merge base and again at the current working tree. Existing diagnostics are matched across unchanged lines, including lines moved by other edits. If no revision is provided, the remote's default branch is used when available.

+
--error rule

Treat the given rule as having severity 'error'. Can be specified multiple times. Use 'all' to apply to all rules.

--error-on-warning

Use exit code 1 if there are any warning-level diagnostics.

Cannot be used in combination with --exit-zero or --exit-zero-on-warning.

--exclude exclude

Glob patterns for files to exclude from type checking.

diff --git a/crates/ty/src/args.rs b/crates/ty/src/args.rs index 33a2453e8c2ccc..11d7252c0a0847 100644 --- a/crates/ty/src/args.rs +++ b/crates/ty/src/args.rs @@ -77,6 +77,20 @@ pub(crate) struct CheckCommand { #[arg(long, conflicts_with("fix"))] pub(crate) add_ignore: bool, + /// Only report diagnostics introduced since a Git revision. + /// + /// The project is checked at the merge base and again at the current working tree. Existing + /// diagnostics are matched across unchanged lines, including lines moved by other edits. + /// If no revision is provided, the remote's default branch is used when available. + #[arg( + long, + value_name = "REVISION", + num_args = 0..=1, + default_missing_value = "", + conflicts_with_all = ["watch", "fix", "add_ignore"] + )] + pub(crate) diff: Option, + /// Run the command within the given project directory. /// /// All `pyproject.toml` files will be discovered by walking up the directory tree from the given project directory, diff --git a/crates/ty/src/git.rs b/crates/ty/src/git.rs new file mode 100644 index 00000000000000..f387bd46d6c512 --- /dev/null +++ b/crates/ty/src/git.rs @@ -0,0 +1,650 @@ +use std::any::Any; +use std::collections::{BTreeMap, HashMap}; +use std::hash::{DefaultHasher, Hash, Hasher}; +use std::process::Output; +use std::sync::Arc; +use std::sync::atomic::{AtomicBool, Ordering}; + +use anyhow::{Context, Result, anyhow, bail}; +use ruff_db::diagnostic::{Diagnostic, DiagnosticId, Severity, UnifiedFile}; +use ruff_db::file_revision::FileRevision; +use ruff_db::source::line_index; +use ruff_db::system::walk_directory::WalkDirectoryBuilder; +use ruff_db::system::{ + DirectoryEntry, FileType, Metadata, OsSystem, System, SystemPath, SystemPathBuf, + SystemVirtualPath, WhichResult, WritableSystem, +}; +use ruff_notebook::{Notebook, NotebookError}; +use similar::{DiffOp, TextDiff}; +use ty_project::watch::{ChangeEvent, CreatedKind, DeletedKind}; +use ty_project::{Db, ProjectDatabase}; + +use crate::IndicatifReporter; +use crate::printer::Printer; + +/// A change between the merge base and the working tree, including untracked files. +#[derive(Debug)] +struct ChangedFile { + baseline_path: Option, + current_path: Option, + baseline_contents: Option, + current_contents: Option, +} + +#[derive(Debug)] +pub(crate) struct GitDiff { + files: Vec, + baseline_files: BTreeMap>, +} + +impl GitDiff { + pub(crate) fn discover(system: &OsSystem, cwd: &SystemPath, revision: &str) -> Result { + let root_output = run_git(system, cwd, &["rev-parse", "--show-toplevel"])?; + let root = SystemPathBuf::from(git_text(&root_output, "repository root")?.trim()); + + let reference = if revision.is_empty() { + default_reference(system, &root) + } else { + revision.to_owned() + }; + + let merge_base_output = run_git(system, &root, &["merge-base", &reference, "HEAD"]) + .with_context(|| format!("Failed to find the Git merge base for `{reference}`"))?; + let merge_base = git_text(&merge_base_output, "merge base")? + .trim() + .to_owned(); + + let changes = run_git( + system, + &root, + &[ + "diff", + "--name-status", + "--find-renames", + "-z", + &merge_base, + "--", + ], + )?; + + let mut files = Vec::new(); + let mut fields = nul_fields(&changes.stdout)?; + + while let Some(status) = fields.next() { + let old = fields + .next() + .ok_or_else(|| anyhow!("Git returned a change without a file path"))?; + + let (baseline_relative, current_relative) = if status.starts_with('R') { + let new = fields + .next() + .ok_or_else(|| anyhow!("Git returned a rename without its destination"))?; + (Some(old), Some(new)) + } else if status.starts_with('C') { + let new = fields + .next() + .ok_or_else(|| anyhow!("Git returned a copy without its destination"))?; + (None, Some(new)) + } else { + match status { + "A" => (None, Some(old)), + "D" => (Some(old), None), + _ => (Some(old), Some(old)), + } + }; + + let baseline_contents = baseline_relative + .map(|path| git_file(system, &root, &merge_base, path)) + .transpose()?; + let current_path = current_relative.map(|path| root.join(path)); + let current_contents = current_path + .as_deref() + .and_then(|path| system.read_to_string(path).ok()); + + // Binary files cannot affect Python source or TOML configuration and must not make a + // Python-only check fail merely because the same commit also updates an image. + if baseline_contents.as_ref().is_some_and(Option::is_none) { + continue; + } + + files.push(ChangedFile { + baseline_path: baseline_relative.map(|path| root.join(path)), + current_path, + baseline_contents: baseline_contents.flatten(), + current_contents, + }); + } + + let untracked = run_git( + system, + &root, + &["ls-files", "--others", "--exclude-standard", "-z"], + )?; + for path in nul_fields(&untracked.stdout)? { + let path = root.join(path); + files.push(ChangedFile { + baseline_path: None, + current_contents: system.read_to_string(&path).ok(), + current_path: Some(path), + baseline_contents: None, + }); + } + + let mut baseline_files = BTreeMap::new(); + for file in &files { + if let Some(path) = &file.baseline_path { + baseline_files.insert(path.clone(), file.baseline_contents.clone()); + } + if let Some(path) = &file.current_path + && file.baseline_path.as_ref() != Some(path) + { + baseline_files.insert(path.clone(), None); + } + } + + tracing::debug!( + "Comparing diagnostics against Git revision `{merge_base}` ({} changed paths)", + files.len() + ); + + Ok(Self { + files, + baseline_files, + }) + } + + pub(crate) fn check_baseline( + self, + db: &mut ProjectDatabase, + printer: Printer, + ) -> Result { + // The OS walker cannot discover files deleted from the working tree, even though the Git + // overlay can still read them. Seed those paths through the same creation events used by + // the language server after forcing the initial project index. + let _ = db.project().files(db); + let baseline_creations = self + .files + .iter() + .filter_map(|file| { + let path = file.baseline_path.as_ref()?; + (file.current_path.as_ref() != Some(path) + && is_python_path(path) + && db.project().is_file_included(db, path).is_included()) + .then(|| ChangeEvent::Created { + path: path.clone(), + kind: CreatedKind::File, + }) + }) + .collect::>(); + + if !baseline_creations.is_empty() { + db.apply_changes(&baseline_creations); + } + + let mut reporter = IndicatifReporter::from(printer); + db.check_with_reporter(&mut reporter); + reporter.bar.finish_and_clear(); + let diagnostics = reporter.collector.into_sorted(db); + let baseline = DiagnosticBaseline::capture(db, diagnostics, &self.files); + + let system = db.system_mut(); + let Some(system) = system.as_any_mut().downcast_mut::() else { + bail!("The Git baseline requires ty's Git-backed file system"); + }; + system.activate_current(); + + let mut changes = Vec::new(); + for file in &self.files { + match (&file.baseline_path, &file.current_path) { + (Some(old), Some(new)) if old == new => { + changes.push(ChangeEvent::file_content_changed(new.clone())); + } + (Some(old), Some(new)) => { + changes.push(ChangeEvent::Deleted { + path: old.clone(), + kind: DeletedKind::File, + }); + changes.push(ChangeEvent::Created { + path: new.clone(), + kind: CreatedKind::File, + }); + } + (Some(old), None) => changes.push(ChangeEvent::Deleted { + path: old.clone(), + kind: DeletedKind::File, + }), + (None, Some(new)) => changes.push(ChangeEvent::Created { + path: new.clone(), + kind: CreatedKind::File, + }), + (None, None) => {} + } + } + + if !changes.is_empty() { + db.apply_changes(&changes); + } + + Ok(baseline) + } +} + +fn is_python_path(path: &SystemPath) -> bool { + matches!(path.extension(), Some("py" | "pyi" | "ipynb")) +} + +fn run_git(system: &OsSystem, cwd: &SystemPath, args: &[&str]) -> Result { + let output = system + .run_command("git", args, cwd) + .with_context(|| format!("Failed to run `git {}`", args.join(" ")))?; + + if !output.status.success() { + let error = String::from_utf8_lossy(&output.stderr); + bail!("`git {}` failed: {}", args.join(" "), error.trim()); + } + + Ok(output) +} + +fn git_text<'a>(output: &'a Output, description: &str) -> Result<&'a str> { + std::str::from_utf8(&output.stdout) + .with_context(|| format!("Git returned a non-UTF-8 {description}")) +} + +fn nul_fields(bytes: &[u8]) -> Result> { + let text = std::str::from_utf8(bytes).context("Git returned a non-UTF-8 file path")?; + Ok(text.split_terminator('\0')) +} + +fn default_reference(system: &OsSystem, root: &SystemPath) -> String { + if let Ok(output) = run_git( + system, + root, + &["symbolic-ref", "--quiet", "refs/remotes/origin/HEAD"], + ) && let Ok(reference) = git_text(&output, "default branch") + { + return reference.trim().to_owned(); + } + + for candidate in ["main", "master"] { + if run_git(system, root, &["rev-parse", "--verify", candidate]).is_ok() { + return candidate.to_owned(); + } + } + + "HEAD".to_owned() +} + +fn git_file( + system: &OsSystem, + root: &SystemPath, + revision: &str, + path: &str, +) -> Result> { + let object = format!("{revision}:{path}"); + let output = run_git(system, root, &["show", &object])?; + Ok(String::from_utf8(output.stdout).ok()) +} + +/// A normal OS filesystem whose changed files initially expose their merge-base contents. +#[derive(Clone, Debug)] +pub(crate) struct GitSystem { + native: OsSystem, + baseline_files: Arc>>, + baseline_active: Arc, +} + +impl GitSystem { + pub(crate) fn new(native: OsSystem, diff: &GitDiff) -> Self { + Self { + native, + baseline_files: Arc::new(diff.baseline_files.clone()), + baseline_active: Arc::new(AtomicBool::new(true)), + } + } + + fn activate_current(&mut self) { + self.baseline_active.store(false, Ordering::Release); + } + + fn baseline_file(&self, path: &SystemPath) -> Option<&Option> { + self.baseline_active + .load(Ordering::Acquire) + .then(|| self.baseline_files.get(path)) + .flatten() + } + + fn has_baseline_descendant(&self, path: &SystemPath) -> bool { + self.baseline_active.load(Ordering::Acquire) + && self.baseline_files.iter().any(|(candidate, contents)| { + contents.is_some() && candidate.as_path() != path && candidate.starts_with(path) + }) + } +} + +impl System for GitSystem { + fn path_metadata(&self, path: &SystemPath) -> std::io::Result { + match self.baseline_file(path) { + Some(Some(contents)) => { + let mut hasher = DefaultHasher::new(); + contents.hash(&mut hasher); + let revision = FileRevision::new((1_u128 << 127) | u128::from(hasher.finish())); + let permissions = self + .native + .path_metadata(path) + .ok() + .and_then(|metadata| metadata.permissions()); + Ok(Metadata::new(revision, permissions, FileType::File)) + } + Some(None) => Err(not_found(path)), + None => match self.native.path_metadata(path) { + Ok(metadata) => Ok(metadata), + Err(_) if self.has_baseline_descendant(path) => Ok(Metadata::new( + FileRevision::new(1_u128 << 127), + None, + FileType::Directory, + )), + Err(error) => Err(error), + }, + } + } + + fn canonicalize_path(&self, path: &SystemPath) -> std::io::Result { + match self.native.canonicalize_path(path) { + Ok(path) => Ok(path), + Err(_) if self.baseline_file(path).is_some_and(Option::is_some) => { + Ok(SystemPath::absolute(path, self.current_directory())) + } + Err(error) => Err(error), + } + } + + fn is_same_file(&self, first: &SystemPath, second: &SystemPath) -> std::io::Result { + if self.baseline_file(first).is_some_and(Option::is_some) + || self.baseline_file(second).is_some_and(Option::is_some) + { + Ok(SystemPath::absolute(first, self.current_directory()) + == SystemPath::absolute(second, self.current_directory())) + } else { + self.native.is_same_file(first, second) + } + } + + fn which(&self, binary_name: &str) -> WhichResult { + self.native.which(binary_name) + } + + fn run_command( + &self, + program: &str, + args: &[&str], + current_directory: &SystemPath, + ) -> std::io::Result { + self.native.run_command(program, args, current_directory) + } + + fn read_to_string(&self, path: &SystemPath) -> std::io::Result { + match self.baseline_file(path) { + Some(Some(contents)) => Ok(contents.clone()), + Some(None) => Err(not_found(path)), + None => self.native.read_to_string(path), + } + } + + fn read_to_notebook(&self, path: &SystemPath) -> Result { + match self.baseline_file(path) { + Some(Some(contents)) => Notebook::from_source_code(contents), + Some(None) => Err(NotebookError::Io(not_found(path))), + None => self.native.read_to_notebook(path), + } + } + + fn read_virtual_path_to_string(&self, path: &SystemVirtualPath) -> std::io::Result { + self.native.read_virtual_path_to_string(path) + } + + fn read_virtual_path_to_notebook( + &self, + path: &SystemVirtualPath, + ) -> Result { + self.native.read_virtual_path_to_notebook(path) + } + + fn current_directory(&self) -> &SystemPath { + self.native.current_directory() + } + + fn user_config_directory(&self) -> Option { + self.native.user_config_directory() + } + + fn cache_dir(&self) -> Option { + self.native.cache_dir() + } + + fn read_directory<'a>( + &'a self, + path: &SystemPath, + ) -> std::io::Result> + 'a>> { + if !self.baseline_active.load(Ordering::Acquire) { + return self.native.read_directory(path); + } + + let mut entries = BTreeMap::new(); + match self.native.read_directory(path) { + Ok(native_entries) => { + for entry in native_entries { + let entry = entry?; + if self.path_metadata(entry.path()).is_ok() { + entries.insert(entry.path().to_path_buf(), entry); + } + } + } + Err(_) if self.has_baseline_descendant(path) => {} + Err(error) => return Err(error), + } + + for (candidate, contents) in self.baseline_files.iter() { + if contents.is_none() { + continue; + } + + let Ok(relative) = candidate.strip_prefix(path) else { + continue; + }; + let Some(component) = relative.as_str().split('/').next() else { + continue; + }; + if component.is_empty() { + continue; + } + + let child = path.join(component); + let file_type = if child == *candidate { + FileType::File + } else { + FileType::Directory + }; + entries + .entry(child.clone()) + .or_insert_with(|| DirectoryEntry::new(child, file_type)); + } + + Ok(Box::new(entries.into_values().map(Ok))) + } + + fn walk_directory(&self, path: &SystemPath) -> WalkDirectoryBuilder { + self.native.walk_directory(path) + } + + fn env_var(&self, name: &str) -> Result { + self.native.env_var(name) + } + + fn as_writable(&self) -> Option<&dyn WritableSystem> { + self.native.as_writable() + } + + fn as_any(&self) -> &dyn Any { + self + } + + fn as_any_mut(&mut self) -> &mut dyn Any { + self + } + + fn dyn_clone(&self) -> Box { + Box::new(self.clone()) + } +} + +fn not_found(path: &SystemPath) -> std::io::Error { + std::io::Error::new( + std::io::ErrorKind::NotFound, + format!("File does not exist in the Git baseline: {path}"), + ) +} + +#[derive(Debug, Eq, Hash, PartialEq)] +struct DiagnosticKey { + path: SystemPathBuf, + line: usize, + column: usize, + id: DiagnosticId, + severity: Severity, + message: String, +} + +#[derive(Debug)] +struct LineMapping { + current_path: SystemPathBuf, + lines: Vec>, +} + +#[derive(Debug, Default)] +pub(crate) struct DiagnosticBaseline { + diagnostics: HashMap, +} + +impl DiagnosticBaseline { + fn capture(db: &ProjectDatabase, diagnostics: Vec, files: &[ChangedFile]) -> Self { + let mappings = files + .iter() + .filter_map(|file| { + let old_path = file.baseline_path.as_ref()?; + let new_path = file.current_path.as_ref()?; + let old_contents = file.baseline_contents.as_ref()?; + let new_contents = file.current_contents.as_ref()?; + + let diff = TextDiff::from_lines(old_contents, new_contents); + let mut lines = vec![None; diff.old_len()]; + + for op in diff.ops() { + if let DiffOp::Equal { + old_index, + new_index, + len, + } = *op + { + for offset in 0..len { + lines[old_index + offset] = Some(new_index + offset + 1); + } + } + } + + Some(( + old_path.clone(), + LineMapping { + current_path: new_path.clone(), + lines, + }, + )) + }) + .collect::>(); + + let mut counts = HashMap::new(); + for diagnostic in diagnostics { + if diagnostic.severity().is_fatal() { + continue; + } + + let Some(mut key) = diagnostic_key(db, &diagnostic) else { + continue; + }; + + if let Some(mapping) = mappings.get(&key.path) { + let Some(line) = key + .line + .checked_sub(1) + .and_then(|line| mapping.lines.get(line)) + .and_then(|line| *line) + else { + continue; + }; + + key.path.clone_from(&mapping.current_path); + key.line = line; + } else if files.iter().any(|file| { + file.baseline_path.as_ref() == Some(&key.path) && file.current_path.is_none() + }) { + continue; + } + + *counts.entry(key).or_insert(0) += 1; + } + + Self { + diagnostics: counts, + } + } + + pub(crate) fn filter( + &mut self, + db: &ProjectDatabase, + diagnostics: Vec, + ) -> Vec { + diagnostics + .into_iter() + .filter(|diagnostic| { + if diagnostic.severity().is_fatal() { + return true; + } + + let Some(key) = diagnostic_key(db, diagnostic) else { + return true; + }; + + let Some(count) = self.diagnostics.get_mut(&key) else { + return true; + }; + + if *count == 0 { + return true; + } + + *count -= 1; + false + }) + .collect() + } +} + +fn diagnostic_key(db: &ProjectDatabase, diagnostic: &Diagnostic) -> Option { + let span = diagnostic.primary_span()?; + let UnifiedFile::Ty(file) = span.file() else { + return None; + }; + let path = file.path(db).as_system_path()?.to_path_buf(); + let range = span.range()?; + let source = ruff_db::source::source_text(db, *file); + let location = line_index(db, *file).line_column(range.start(), source.as_str()); + + Some(DiagnosticKey { + path, + line: location.line.get(), + column: location.column.get(), + id: diagnostic.id(), + severity: diagnostic.severity(), + message: diagnostic.concise_message().to_string(), + }) +} diff --git a/crates/ty/src/lib.rs b/crates/ty/src/lib.rs index facee85d1ce681..46e4d320c55775 100644 --- a/crates/ty/src/lib.rs +++ b/crates/ty/src/lib.rs @@ -1,4 +1,5 @@ mod args; +mod git; mod logging; mod printer; mod python_version; @@ -33,6 +34,7 @@ use ty_server::run_server; use ty_static::EnvVars; use crate::args::{CheckCommand, Command, ExplainCommand, HelpFormat, TerminalColor}; +use crate::git::{DiagnosticBaseline, GitDiff, GitSystem}; use crate::logging::{VerbosityLevel, setup_tracing}; use crate::printer::Printer; pub use args::Cli; @@ -142,7 +144,16 @@ fn run_check(args: CheckCommand) -> anyhow::Result { MainLoopMode::Check }; - let system = OsSystem::new(&cwd); + let native_system = OsSystem::new(&cwd); + let git_diff = args + .diff + .as_deref() + .map(|revision| GitDiff::discover(&native_system, &cwd, revision)) + .transpose()?; + let git_system = git_diff + .as_ref() + .map(|diff| GitSystem::new(native_system.clone(), diff)); + let system: &dyn System = git_system.as_ref().map_or(&native_system, |system| system); let watch = args.watch; let exit_zero = args.exit_zero; let memory_report = std::env::var(EnvVars::TY_MEMORY_REPORT).ok(); @@ -154,14 +165,14 @@ fn run_check(args: CheckCommand) -> anyhow::Result { let mut project_metadata = match &config_file { Some(config_file) => { - ProjectMetadata::from_config_file(config_file.clone(), &project_path, &system)? + ProjectMetadata::from_config_file(config_file.clone(), &project_path, system)? } None if check_paths.iter().any(|path| system.is_file(path)) => { // `uv check --script` passes a file as its check path. Disable uv workspace metadata // for scripts until script integration is implemented in a follow-up. - ProjectMetadata::discover_without_uv(&project_path, &system)? + ProjectMetadata::discover_without_uv(&project_path, system)? } - None => ProjectMetadata::discover(&project_path, &system)?, + None => ProjectMetadata::discover(&project_path, system)?, }; if watch && project_metadata.has_uv_workspace() { @@ -170,11 +181,14 @@ fn run_check(args: CheckCommand) -> anyhow::Result { )); } - project_metadata.apply_configuration_files(&system)?; + project_metadata.apply_configuration_files(system)?; project_metadata.apply_override_options(args.into_options()); - let mut db = ProjectDatabase::fallible(project_metadata, system)?; + let mut db = match git_system { + Some(system) => ProjectDatabase::fallible(project_metadata, system)?, + None => ProjectDatabase::fallible(project_metadata, native_system)?, + }; let project = db.project(); project.set_verbose(&mut db, verbosity >= VerbosityLevel::Verbose); @@ -197,11 +211,20 @@ fn run_check(args: CheckCommand) -> anyhow::Result { // unnecessary Salsa dependencies. Watch mode updates inputs incrementally, fix modes apply // source-text overrides, and memory reports measure the database without this optimization, so // they must keep the inputs mutable. - if !watch && matches!(mode, MainLoopMode::Check) && memory_report.is_none() { + if !watch + && git_diff.is_none() + && matches!(mode, MainLoopMode::Check) + && memory_report.is_none() + { db.freeze(); } - let (main_loop, main_loop_cancellation_token) = MainLoop::new(mode, printer); + let diagnostic_baseline = git_diff + .map(|diff| diff.check_baseline(&mut db, printer)) + .transpose()?; + + let (main_loop, main_loop_cancellation_token) = + MainLoop::new(mode, printer, diagnostic_baseline); // Listen to Ctrl+C and abort the watch mode. let main_loop_cancellation_token = Mutex::new(Some(main_loop_cancellation_token)); @@ -279,6 +302,9 @@ impl Termination for ExitStatus { struct MainLoop { mode: MainLoopMode, + /// Diagnostics already present in the Git baseline, normalized to current positions. + diagnostic_baseline: Option, + /// Sender that can be used to send messages to the main loop. sender: crossbeam_channel::Sender, @@ -298,7 +324,11 @@ struct MainLoop { } impl MainLoop { - fn new(mode: MainLoopMode, printer: Printer) -> (Self, MainLoopCancellationToken) { + fn new( + mode: MainLoopMode, + printer: Printer, + diagnostic_baseline: Option, + ) -> (Self, MainLoopCancellationToken) { let (sender, receiver) = crossbeam_channel::bounded(10); let cancellation_token_source = CancellationTokenSource::new(); @@ -307,6 +337,7 @@ impl MainLoop { ( Self { mode, + diagnostic_baseline, sender: sender.clone(), receiver, watcher: None, @@ -400,6 +431,11 @@ impl MainLoop { return Ok(ExitStatus::Success); } + let result = match &mut self.diagnostic_baseline { + Some(baseline) => baseline.filter(db, result), + None => result, + }; + self.write_diagnostics(db, &result, None)?; if self.cancellation_token.is_cancelled() { diff --git a/crates/ty/tests/cli/diff.rs b/crates/ty/tests/cli/diff.rs new file mode 100644 index 00000000000000..f2d1f2af1d54d3 --- /dev/null +++ b/crates/ty/tests/cli/diff.rs @@ -0,0 +1,309 @@ +use std::process::{Command, Output}; + +use anyhow::{Context, bail}; + +use crate::CliTest; + +fn git(case: &CliTest, args: &[&str]) -> anyhow::Result<()> { + let output = Command::new("git") + .args(args) + .current_dir(case.root()) + .output() + .with_context(|| format!("Failed to run git {}", args.join(" ")))?; + + if !output.status.success() { + bail!( + "git {} failed: {}", + args.join(" "), + String::from_utf8_lossy(&output.stderr) + ); + } + + Ok(()) +} + +fn commit_baseline(case: &CliTest) -> anyhow::Result<()> { + git(case, &["init", "--quiet", "--initial-branch=main"])?; + git(case, &["add", "--all"])?; + git( + case, + &[ + "-c", + "user.name=ty tests", + "-c", + "user.email=ty@example.com", + "commit", + "--quiet", + "--message=baseline", + ], + ) +} + +fn check_diff(case: &CliTest) -> anyhow::Result { + case.command() + .arg("--diff") + .arg("HEAD") + .arg("--output-format") + .arg("concise") + .output() + .context("Failed to run ty in Git diff mode") +} + +fn stdout(output: &Output) -> anyhow::Result<&str> { + std::str::from_utf8(&output.stdout).context("ty returned non-UTF-8 output") +} + +#[test] +fn existing_diagnostics_moved_by_insertions_are_suppressed() -> anyhow::Result<()> { + let case = CliTest::with_file("example.py", "existing: int = 'old'\n")?; + commit_baseline(&case)?; + + case.write_file( + "example.py", + "header = 1\nexisting: int = 'old'\nintroduced: str = 42\n", + )?; + + let output = check_diff(&case)?; + let output_text = stdout(&output)?; + assert!(!output.status.success(), "{output_text}"); + assert!(output_text.contains("example.py:3:"), "{output_text}"); + assert!(!output_text.contains("example.py:2:"), "{output_text}"); + assert!(output_text.contains("Found 1 diagnostic"), "{output_text}"); + + Ok(()) +} + +#[test] +fn diagnostics_introduced_in_unchanged_files_are_reported() -> anyhow::Result<()> { + let case = CliTest::with_files([ + ("provider.py", "def provide() -> int:\n return 1\n"), + ( + "consumer.py", + "from provider import provide\n\nvalue: int = provide()\n", + ), + ])?; + commit_baseline(&case)?; + + case.write_file("provider.py", "def provide() -> str:\n return 'text'\n")?; + + let output = check_diff(&case)?; + let output_text = stdout(&output)?; + assert!(!output.status.success(), "{output_text}"); + assert!(output_text.contains("consumer.py:3:"), "{output_text}"); + assert!(output_text.contains("invalid-assignment"), "{output_text}"); + + Ok(()) +} + +#[test] +fn unchanged_baseline_errors_do_not_fail() -> anyhow::Result<()> { + let case = CliTest::with_file("example.py", "existing: int = 'old'\n")?; + commit_baseline(&case)?; + + let output = check_diff(&case)?; + let output_text = stdout(&output)?; + assert!(output.status.success(), "{output_text}"); + assert!(output_text.contains("All checks passed!"), "{output_text}"); + + Ok(()) +} + +#[test] +fn the_default_revision_compares_against_the_default_branch() -> anyhow::Result<()> { + let case = CliTest::with_file("example.py", "existing: int = 'old'\n")?; + commit_baseline(&case)?; + git(&case, &["checkout", "--quiet", "-b", "feature"])?; + case.write_file( + "example.py", + "header = 1\nexisting: int = 'old'\nintroduced: str = 42\n", + )?; + git(&case, &["add", "--all"])?; + git( + &case, + &[ + "-c", + "user.name=ty tests", + "-c", + "user.email=ty@example.com", + "commit", + "--quiet", + "--message=feature", + ], + )?; + + let output = case + .command() + .arg("--diff") + .arg("--output-format") + .arg("concise") + .output()?; + let output_text = stdout(&output)?; + assert!(!output.status.success(), "{output_text}"); + assert!(output_text.contains("example.py:3:"), "{output_text}"); + assert!(!output_text.contains("example.py:2:"), "{output_text}"); + + Ok(()) +} + +#[test] +fn staged_changes_are_included() -> anyhow::Result<()> { + let case = CliTest::with_file("example.py", "value = 1\n")?; + commit_baseline(&case)?; + case.write_file("example.py", "value: int = 'staged'\n")?; + git(&case, &["add", "example.py"])?; + + let output = check_diff(&case)?; + let output_text = stdout(&output)?; + assert!(!output.status.success(), "{output_text}"); + assert!(output_text.contains("example.py:1:"), "{output_text}"); + + Ok(()) +} + +#[test] +fn unrelated_binary_changes_do_not_prevent_checking() -> anyhow::Result<()> { + let case = CliTest::with_file("example.py", "value = 1\n")?; + std::fs::write(case.root().join("image.bin"), [0xff, 0xfe, 0x00])?; + commit_baseline(&case)?; + std::fs::write(case.root().join("image.bin"), [0xfe, 0xff, 0x00])?; + case.write_file("example.py", "value: int = 'new'\n")?; + + let output = check_diff(&case)?; + let output_text = stdout(&output)?; + assert!(!output.status.success(), "{output_text}"); + assert!(output_text.contains("example.py:1:"), "{output_text}"); + + Ok(()) +} + +#[test] +fn untracked_python_files_are_checked() -> anyhow::Result<()> { + let case = CliTest::with_file("existing.py", "value = 1\n")?; + commit_baseline(&case)?; + case.write_file("untracked.py", "value: int = 'new'\n")?; + + let output = check_diff(&case)?; + let output_text = stdout(&output)?; + assert!(!output.status.success(), "{output_text}"); + assert!(output_text.contains("untracked.py:1:"), "{output_text}"); + + Ok(()) +} + +#[test] +fn renamed_existing_diagnostics_are_suppressed() -> anyhow::Result<()> { + let case = CliTest::with_file("before.py", "existing: int = 'old'\n")?; + commit_baseline(&case)?; + git(&case, &["mv", "before.py", "after.py"])?; + + let output = check_diff(&case)?; + let output_text = stdout(&output)?; + assert!(output.status.success(), "{output_text}"); + assert!(output_text.contains("All checks passed!"), "{output_text}"); + + Ok(()) +} + +#[test] +fn deleting_an_imported_module_reports_the_new_import_error() -> anyhow::Result<()> { + let case = CliTest::with_files([ + ("provider.py", "value = 1\n"), + ("consumer.py", "from provider import value\n"), + ])?; + commit_baseline(&case)?; + std::fs::remove_file(case.root().join("provider.py"))?; + + let output = check_diff(&case)?; + let output_text = stdout(&output)?; + assert!(!output.status.success(), "{output_text}"); + assert!(output_text.contains("consumer.py:1:"), "{output_text}"); + assert!(output_text.contains("unresolved-import"), "{output_text}"); + + Ok(()) +} + +#[test] +fn deleting_an_imported_package_reports_the_new_import_error() -> anyhow::Result<()> { + let case = CliTest::with_files([ + ("package/__init__.py", ""), + ("package/provider.py", "value = 1\n"), + ("consumer.py", "from package.provider import value\n"), + ])?; + commit_baseline(&case)?; + std::fs::remove_dir_all(case.root().join("package"))?; + + let output = check_diff(&case)?; + let output_text = stdout(&output)?; + assert!(!output.status.success(), "{output_text}"); + assert!(output_text.contains("consumer.py:1:"), "{output_text}"); + assert!(output_text.contains("unresolved-import"), "{output_text}"); + + Ok(()) +} + +#[test] +fn changed_diagnostic_messages_are_reported() -> anyhow::Result<()> { + let case = CliTest::with_file("example.py", "value: int = 'old'\n")?; + commit_baseline(&case)?; + case.write_file("example.py", "value: int = 'new'\n")?; + + let output = check_diff(&case)?; + let output_text = stdout(&output)?; + assert!(!output.status.success(), "{output_text}"); + assert!(output_text.contains("Literal[\"new\"]"), "{output_text}"); + + Ok(()) +} + +#[test] +fn configuration_changes_recheck_the_project() -> anyhow::Result<()> { + let case = CliTest::with_files([ + ("example.py", "value: int = 'existing'\n"), + ("ty.toml", "[rules]\ninvalid-assignment = 'ignore'\n"), + ])?; + commit_baseline(&case)?; + case.write_file("ty.toml", "[rules]\ninvalid-assignment = 'error'\n")?; + + let output = check_diff(&case)?; + let output_text = stdout(&output)?; + assert!(!output.status.success(), "{output_text}"); + assert!(output_text.contains("example.py:1:"), "{output_text}"); + + Ok(()) +} + +#[test] +fn invalid_git_revisions_fail_clearly() -> anyhow::Result<()> { + let case = CliTest::with_file("example.py", "value = 1\n")?; + commit_baseline(&case)?; + + let output = case + .command() + .arg("--diff") + .arg("does-not-exist") + .output()?; + let stderr = std::str::from_utf8(&output.stderr)?; + assert!(!output.status.success(), "{stderr}"); + assert!(stderr.contains("does-not-exist"), "{stderr}"); + + Ok(()) +} + +#[test] +fn diff_mode_rejects_incompatible_modes() -> anyhow::Result<()> { + let case = CliTest::with_file("example.py", "value = 1\n")?; + + for incompatible in ["--watch", "--fix", "--add-ignore"] { + let output = case + .command() + .arg("--diff") + .arg("HEAD") + .arg(incompatible) + .output()?; + let stderr = std::str::from_utf8(&output.stderr)?; + assert!(!output.status.success(), "{stderr}"); + assert!(stderr.contains(incompatible), "{stderr}"); + } + + Ok(()) +} diff --git a/crates/ty/tests/cli/main.rs b/crates/ty/tests/cli/main.rs index b05ada1fbb1de6..43f16122fd3fbe 100644 --- a/crates/ty/tests/cli/main.rs +++ b/crates/ty/tests/cli/main.rs @@ -1,5 +1,6 @@ mod analysis_options; mod config_option; +mod diff; mod exit_code; mod file_selection; mod fixes; From 842a78857b0ae4f474e6bd200eacf6482b0b3af7 Mon Sep 17 00:00:00 2001 From: Aria Desires Date: Thu, 6 Aug 2026 12:31:17 -0400 Subject: [PATCH 2/3] [ty] Ignore irrelevant files when building diagnostic baselines --- crates/ty/src/git.rs | 37 ++++++++++++++++++++++++--- crates/ty/src/lib.rs | 10 ++++---- crates/ty/tests/cli/diff.rs | 51 +++++++++++++++++++++++++++++++++++++ 3 files changed, 90 insertions(+), 8 deletions(-) diff --git a/crates/ty/src/git.rs b/crates/ty/src/git.rs index f387bd46d6c512..94ce9b86805162 100644 --- a/crates/ty/src/git.rs +++ b/crates/ty/src/git.rs @@ -38,7 +38,12 @@ pub(crate) struct GitDiff { } impl GitDiff { - pub(crate) fn discover(system: &OsSystem, cwd: &SystemPath, revision: &str) -> Result { + pub(crate) fn discover( + system: &OsSystem, + cwd: &SystemPath, + revision: &str, + config_file: Option<&SystemPath>, + ) -> Result { let root_output = run_git(system, cwd, &["rev-parse", "--show-toplevel"])?; let root = SystemPathBuf::from(git_text(&root_output, "repository root")?.trim()); @@ -93,10 +98,22 @@ impl GitDiff { } }; + let baseline_path = baseline_relative.map(|path| root.join(path)); + let current_path = current_relative.map(|path| root.join(path)); + + if !baseline_path + .as_deref() + .is_some_and(|path| is_relevant_path(path, config_file)) + && !current_path + .as_deref() + .is_some_and(|path| is_relevant_path(path, config_file)) + { + continue; + } + let baseline_contents = baseline_relative .map(|path| git_file(system, &root, &merge_base, path)) .transpose()?; - let current_path = current_relative.map(|path| root.join(path)); let current_contents = current_path .as_deref() .and_then(|path| system.read_to_string(path).ok()); @@ -108,7 +125,7 @@ impl GitDiff { } files.push(ChangedFile { - baseline_path: baseline_relative.map(|path| root.join(path)), + baseline_path, current_path, baseline_contents: baseline_contents.flatten(), current_contents, @@ -122,6 +139,11 @@ impl GitDiff { )?; for path in nul_fields(&untracked.stdout)? { let path = root.join(path); + + if !is_relevant_path(&path, config_file) { + continue; + } + files.push(ChangedFile { baseline_path: None, current_contents: system.read_to_string(&path).ok(), @@ -233,6 +255,15 @@ fn is_python_path(path: &SystemPath) -> bool { matches!(path.extension(), Some("py" | "pyi" | "ipynb")) } +fn is_relevant_path(path: &SystemPath, config_file: Option<&SystemPath>) -> bool { + is_python_path(path) + || matches!( + path.file_name(), + Some("pyproject.toml" | "ty.toml" | ".gitignore" | ".ignore" | "VERSIONS" | "py.typed") + ) + || config_file.is_some_and(|config_file| config_file == path) +} + fn run_git(system: &OsSystem, cwd: &SystemPath, args: &[&str]) -> Result { let output = system .run_command("git", args, cwd) diff --git a/crates/ty/src/lib.rs b/crates/ty/src/lib.rs index 46e4d320c55775..14dc1b437f9b77 100644 --- a/crates/ty/src/lib.rs +++ b/crates/ty/src/lib.rs @@ -144,11 +144,15 @@ fn run_check(args: CheckCommand) -> anyhow::Result { MainLoopMode::Check }; + let config_file = args + .config_file + .as_ref() + .map(|path| SystemPath::absolute(path, &cwd)); let native_system = OsSystem::new(&cwd); let git_diff = args .diff .as_deref() - .map(|revision| GitDiff::discover(&native_system, &cwd, revision)) + .map(|revision| GitDiff::discover(&native_system, &cwd, revision, config_file.as_deref())) .transpose()?; let git_system = git_diff .as_ref() @@ -157,10 +161,6 @@ fn run_check(args: CheckCommand) -> anyhow::Result { let watch = args.watch; let exit_zero = args.exit_zero; let memory_report = std::env::var(EnvVars::TY_MEMORY_REPORT).ok(); - let config_file = args - .config_file - .as_ref() - .map(|path| SystemPath::absolute(path, &cwd)); let force_exclude = args.force_exclude(); let mut project_metadata = match &config_file { diff --git a/crates/ty/tests/cli/diff.rs b/crates/ty/tests/cli/diff.rs index f2d1f2af1d54d3..609a0a969d647a 100644 --- a/crates/ty/tests/cli/diff.rs +++ b/crates/ty/tests/cli/diff.rs @@ -176,6 +176,29 @@ fn unrelated_binary_changes_do_not_prevent_checking() -> anyhow::Result<()> { Ok(()) } +#[test] +fn unrelated_text_changes_are_excluded_from_the_baseline() -> anyhow::Result<()> { + let case = CliTest::with_files([ + ("example.py", "existing: int = 'old'\n"), + ("notes.md", "Before the change\n"), + ])?; + commit_baseline(&case)?; + case.write_file("notes.md", "After the change\n")?; + + let output = case + .command() + .arg("--diff") + .arg("HEAD") + .env("TY_LOG", "ty::git=debug") + .output()?; + let output_text = stdout(&output)?; + let stderr = std::str::from_utf8(&output.stderr)?; + assert!(output.status.success(), "{output_text}\n{stderr}"); + assert!(stderr.contains("(0 changed paths)"), "{stderr}"); + + Ok(()) +} + #[test] fn untracked_python_files_are_checked() -> anyhow::Result<()> { let case = CliTest::with_file("existing.py", "value = 1\n")?; @@ -272,6 +295,34 @@ fn configuration_changes_recheck_the_project() -> anyhow::Result<()> { Ok(()) } +#[test] +fn explicitly_named_configuration_changes_recheck_the_project() -> anyhow::Result<()> { + let case = CliTest::with_files([ + ("example.py", "value: int = 'existing'\n"), + ( + "settings.custom", + "[rules]\ninvalid-assignment = 'ignore'\n", + ), + ])?; + commit_baseline(&case)?; + case.write_file("settings.custom", "[rules]\ninvalid-assignment = 'error'\n")?; + + let output = case + .command() + .arg("--diff") + .arg("HEAD") + .arg("--config-file") + .arg("settings.custom") + .arg("--output-format") + .arg("concise") + .output()?; + let output_text = stdout(&output)?; + assert!(!output.status.success(), "{output_text}"); + assert!(output_text.contains("example.py:1:"), "{output_text}"); + + Ok(()) +} + #[test] fn invalid_git_revisions_fail_clearly() -> anyhow::Result<()> { let case = CliTest::with_file("example.py", "value = 1\n")?; From 878e0a2e6b314fe12b65d3a4fd9549987c7aa618 Mon Sep 17 00:00:00 2001 From: Aria Desires Date: Thu, 6 Aug 2026 12:40:55 -0400 Subject: [PATCH 3/3] [ty] Batch baseline Git object reads for diff mode --- crates/ty/src/git.rs | 124 ++++++++++++++++++++++++++++++------ crates/ty/tests/cli/diff.rs | 39 ++++++++++++ 2 files changed, 145 insertions(+), 18 deletions(-) diff --git a/crates/ty/src/git.rs b/crates/ty/src/git.rs index 94ce9b86805162..7a0cb85cbc3e2d 100644 --- a/crates/ty/src/git.rs +++ b/crates/ty/src/git.rs @@ -1,7 +1,8 @@ use std::any::Any; use std::collections::{BTreeMap, HashMap}; use std::hash::{DefaultHasher, Hash, Hasher}; -use std::process::Output; +use std::io::Write; +use std::process::{Command, Output, Stdio}; use std::sync::Arc; use std::sync::atomic::{AtomicBool, Ordering}; @@ -73,6 +74,7 @@ impl GitDiff { )?; let mut files = Vec::new(); + let mut baseline_objects = Vec::new(); let mut fields = nul_fields(&changes.stdout)?; while let Some(status) = fields.next() { @@ -111,27 +113,35 @@ impl GitDiff { continue; } - let baseline_contents = baseline_relative - .map(|path| git_file(system, &root, &merge_base, path)) - .transpose()?; let current_contents = current_path .as_deref() .and_then(|path| system.read_to_string(path).ok()); - // Binary files cannot affect Python source or TOML configuration and must not make a - // Python-only check fail merely because the same commit also updates an image. - if baseline_contents.as_ref().is_some_and(Option::is_none) { - continue; + if let Some(path) = baseline_relative { + baseline_objects.push((files.len(), format!("{merge_base}:{path}"))); } files.push(ChangedFile { baseline_path, current_path, - baseline_contents: baseline_contents.flatten(), + baseline_contents: None, current_contents, }); } + if !baseline_objects.is_empty() { + for ((index, _), contents) in baseline_objects + .iter() + .zip(git_files(&root, &baseline_objects)?) + { + files[*index].baseline_contents = contents; + } + + // Binary files cannot affect Python source or TOML configuration and must not make a + // Python-only check fail merely because the same commit also updates an image. + files.retain(|file| file.baseline_path.is_none() || file.baseline_contents.is_some()); + } + let untracked = run_git( system, &root, @@ -306,15 +316,93 @@ fn default_reference(system: &OsSystem, root: &SystemPath) -> String { "HEAD".to_owned() } -fn git_file( - system: &OsSystem, - root: &SystemPath, - revision: &str, - path: &str, -) -> Result> { - let object = format!("{revision}:{path}"); - let output = run_git(system, root, &["show", &object])?; - Ok(String::from_utf8(output.stdout).ok()) +fn git_files(root: &SystemPath, objects: &[(usize, String)]) -> Result>> { + let mut command = Command::new("git"); + command + .args(["cat-file", "--batch", "-Z"]) + .current_dir(root.as_std_path()) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + + let mut child = command + .spawn() + .context("Failed to run `git cat-file --batch -Z`")?; + let mut stdin = child + .stdin + .take() + .ok_or_else(|| anyhow!("Failed to open stdin for `git cat-file --batch -Z`"))?; + + let output = std::thread::scope(|scope| { + let writer = scope.spawn(move || -> std::io::Result<()> { + for (_, object) in objects { + stdin.write_all(object.as_bytes())?; + stdin.write_all(&[0])?; + } + Ok(()) + }); + + let output = child + .wait_with_output() + .context("Failed to read output from `git cat-file --batch -Z`")?; + let writer_result = writer + .join() + .map_err(|_| anyhow!("The Git object writer unexpectedly panicked"))?; + writer_result.context("Failed to request baseline Git objects")?; + + Ok::<_, anyhow::Error>(output) + })?; + + if !output.status.success() { + let error = String::from_utf8_lossy(&output.stderr); + bail!("`git cat-file --batch -Z` failed: {}", error.trim()); + } + + let mut contents = Vec::with_capacity(objects.len()); + let mut remaining = output.stdout.as_slice(); + + for (_, object) in objects { + let header_end = remaining + .iter() + .position(|byte| *byte == 0) + .ok_or_else(|| anyhow!("Git returned no object header for `{object}`"))?; + let header = std::str::from_utf8(&remaining[..header_end]) + .with_context(|| format!("Git returned a non-UTF-8 object header for `{object}`"))?; + let mut fields = header.split_ascii_whitespace(); + let _object_id = fields + .next() + .ok_or_else(|| anyhow!("Git returned an empty object header for `{object}`"))?; + let object_type = fields + .next() + .ok_or_else(|| anyhow!("Git could not read baseline object `{object}`: {header}"))?; + + if object_type != "blob" { + bail!("Expected baseline Git object `{object}` to be a blob, found `{object_type}`"); + } + + let size = fields + .next() + .ok_or_else(|| anyhow!("Git returned no object size for `{object}`"))? + .parse::() + .with_context(|| format!("Git returned an invalid object size for `{object}`"))?; + remaining = remaining + .get(header_end + 1..) + .ok_or_else(|| anyhow!("Git returned a truncated object header for `{object}`"))?; + let bytes = remaining + .get(..size) + .ok_or_else(|| anyhow!("Git returned a truncated baseline object `{object}`"))?; + contents.push(std::str::from_utf8(bytes).ok().map(str::to_owned)); + remaining = remaining + .get(size..) + .and_then(|bytes| bytes.strip_prefix(&[0])) + .ok_or_else(|| anyhow!("Git returned no terminator for baseline object `{object}`"))?; + } + + if !remaining.is_empty() { + bail!("Git returned unexpected trailing baseline object data"); + } + + Ok(contents) } /// A normal OS filesystem whose changed files initially expose their merge-base contents. diff --git a/crates/ty/tests/cli/diff.rs b/crates/ty/tests/cli/diff.rs index 609a0a969d647a..51abfeb9028ebd 100644 --- a/crates/ty/tests/cli/diff.rs +++ b/crates/ty/tests/cli/diff.rs @@ -278,6 +278,45 @@ fn changed_diagnostic_messages_are_reported() -> anyhow::Result<()> { Ok(()) } +#[test] +fn multiple_changed_files_keep_their_own_baseline_contents() -> anyhow::Result<()> { + let case = CliTest::with_files([ + ("first.py", "first: int = 'old'\n"), + ("second.py", "second: str = 1\n"), + ])?; + commit_baseline(&case)?; + case.write_file("first.py", "header = 1\nfirst: int = 'old'\n")?; + case.write_file( + "second.py", + "header = 1\nsecond: str = 1\nintroduced: int = 'new'\n", + )?; + + let output = check_diff(&case)?; + let output_text = stdout(&output)?; + assert!(!output.status.success(), "{output_text}"); + assert!(output_text.contains("second.py:3:"), "{output_text}"); + assert!(!output_text.contains("first.py:2:"), "{output_text}"); + assert!(!output_text.contains("second.py:2:"), "{output_text}"); + assert!(output_text.contains("Found 1 diagnostic"), "{output_text}"); + + Ok(()) +} + +#[test] +fn changed_python_files_with_newlines_in_their_names_are_supported() -> anyhow::Result<()> { + let path = "before\nafter.py"; + let case = CliTest::with_file(path, "value = 1\n")?; + commit_baseline(&case)?; + case.write_file(path, "value: int = 'introduced'\n")?; + + let output = check_diff(&case)?; + let output_text = stdout(&output)?; + assert!(!output.status.success(), "{output_text}"); + assert!(output_text.contains("invalid-assignment"), "{output_text}"); + + Ok(()) +} + #[test] fn configuration_changes_recheck_the_project() -> anyhow::Result<()> { let case = CliTest::with_files([