From 05edbab9725985266e0417fef3be3f7c0077ffb2 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 17 Oct 2023 19:36:26 -0400 Subject: [PATCH] Avoid failed assertion when showing fixes from stdin --- crates/ruff_cli/src/diagnostics.rs | 53 +++++++++++++++++++---- crates/ruff_cli/src/printer.rs | 6 +-- crates/ruff_cli/tests/integration_test.rs | 8 ++-- 3 files changed, 51 insertions(+), 16 deletions(-) diff --git a/crates/ruff_cli/src/diagnostics.rs b/crates/ruff_cli/src/diagnostics.rs index c4ea03c55ba81..34725cb6a9f5e 100644 --- a/crates/ruff_cli/src/diagnostics.rs +++ b/crates/ruff_cli/src/diagnostics.rs @@ -11,7 +11,6 @@ use anyhow::{Context, Result}; use colored::Colorize; use filetime::FileTime; use log::{debug, error, warn}; -use ruff_linter::settings::types::UnsafeFixes; use rustc_hash::FxHashMap; use ruff_diagnostics::Diagnostic; @@ -20,6 +19,7 @@ use ruff_linter::logging::DisplayParseError; use ruff_linter::message::Message; use ruff_linter::pyproject_toml::lint_pyproject_toml; use ruff_linter::registry::AsRule; +use ruff_linter::settings::types::UnsafeFixes; use ruff_linter::settings::{flags, LinterSettings}; use ruff_linter::source_kind::{SourceError, SourceKind}; use ruff_linter::{fs, IOError, SyntaxError}; @@ -61,7 +61,7 @@ impl FileCacheKey { #[derive(Debug, Default, PartialEq)] pub(crate) struct Diagnostics { pub(crate) messages: Vec, - pub(crate) fixed: FxHashMap, + pub(crate) fixed: FixMap, pub(crate) imports: ImportMap, pub(crate) notebook_indexes: FxHashMap, } @@ -74,7 +74,7 @@ impl Diagnostics { ) -> Self { Self { messages, - fixed: FxHashMap::default(), + fixed: FixMap::default(), imports, notebook_indexes, } @@ -146,18 +146,55 @@ impl AddAssign for Diagnostics { fn add_assign(&mut self, other: Self) { self.messages.extend(other.messages); self.imports.extend(other.imports); - for (filename, fixed) in other.fixed { + self.fixed += other.fixed; + self.notebook_indexes.extend(other.notebook_indexes); + } +} + +/// A collection of fixes indexed by file path. +#[derive(Debug, Default, PartialEq)] +pub(crate) struct FixMap(FxHashMap); + +impl FixMap { + /// Returns `true` if there are no fixes in the map. + pub(crate) fn is_empty(&self) -> bool { + self.0.is_empty() + } + + /// Returns an iterator over the fixes in the map, along with the file path. + pub(crate) fn iter(&self) -> impl Iterator { + self.0.iter() + } + + /// Returns an iterator over the fixes in the map. + pub(crate) fn values(&self) -> impl Iterator { + self.0.values() + } +} + +impl FromIterator<(String, FixTable)> for FixMap { + fn from_iter>(iter: T) -> Self { + Self( + iter.into_iter() + .filter(|(_, fixes)| !fixes.is_empty()) + .collect(), + ) + } +} + +impl AddAssign for FixMap { + fn add_assign(&mut self, rhs: Self) { + for (filename, fixed) in rhs.0 { if fixed.is_empty() { continue; } - let fixed_in_file = self.fixed.entry(filename).or_default(); + let fixed_in_file = self.0.entry(filename).or_default(); for (rule, count) in fixed { if count > 0 { *fixed_in_file.entry(rule).or_default() += count; } } } - self.notebook_indexes.extend(other.notebook_indexes); } } @@ -318,7 +355,7 @@ pub(crate) fn lint_path( Ok(Diagnostics { messages, - fixed: FxHashMap::from_iter([(fs::relativize_path(path), fixed)]), + fixed: FixMap::from_iter([(fs::relativize_path(path), fixed)]), imports, notebook_indexes, }) @@ -436,7 +473,7 @@ pub(crate) fn lint_stdin( Ok(Diagnostics { messages, - fixed: FxHashMap::from_iter([( + fixed: FixMap::from_iter([( fs::relativize_path(path.unwrap_or_else(|| Path::new("-"))), fixed, )]), diff --git a/crates/ruff_cli/src/printer.rs b/crates/ruff_cli/src/printer.rs index 4e63d689b51bc..6f22f37d1608e 100644 --- a/crates/ruff_cli/src/printer.rs +++ b/crates/ruff_cli/src/printer.rs @@ -7,11 +7,9 @@ use anyhow::Result; use bitflags::bitflags; use colored::Colorize; use itertools::{iterate, Itertools}; -use rustc_hash::FxHashMap; use serde::Serialize; use ruff_linter::fs::relativize_path; -use ruff_linter::linter::FixTable; use ruff_linter::logging::LogLevel; use ruff_linter::message::{ AzureEmitter, Emitter, EmitterContext, GithubEmitter, GitlabEmitter, GroupedEmitter, @@ -22,7 +20,7 @@ use ruff_linter::registry::{AsRule, Rule}; use ruff_linter::settings::flags::{self}; use ruff_linter::settings::types::{SerializationFormat, UnsafeFixes}; -use crate::diagnostics::Diagnostics; +use crate::diagnostics::{Diagnostics, FixMap}; bitflags! { #[derive(Default, Debug, Copy, Clone)] @@ -462,7 +460,7 @@ fn show_fix_status(fix_mode: flags::FixMode, fixables: Option<&FixableStatistics (!fix_mode.is_apply()) && fixables.is_some_and(FixableStatistics::any_applicable_fixes) } -fn print_fix_summary(writer: &mut dyn Write, fixed: &FxHashMap) -> Result<()> { +fn print_fix_summary(writer: &mut dyn Write, fixed: &FixMap) -> Result<()> { let total = fixed .values() .map(|table| table.values().sum::()) diff --git a/crates/ruff_cli/tests/integration_test.rs b/crates/ruff_cli/tests/integration_test.rs index ba11a7caace84..36a25bd29e4ca 100644 --- a/crates/ruff_cli/tests/integration_test.rs +++ b/crates/ruff_cli/tests/integration_test.rs @@ -1252,8 +1252,8 @@ fn diff_does_not_show_display_only_fixes_with_unsafe_fixes_enabled() { ]) .pass_stdin("def add_to_list(item, some_list=[]): ..."), @r###" - success: false - exit_code: 1 + success: true + exit_code: 0 ----- stdout ----- ----- stderr ----- @@ -1276,8 +1276,8 @@ fn diff_only_unsafe_fixes_available() { ]) .pass_stdin("x = {'a': 1, 'a': 1}\nprint(('foo'))\n"), @r###" - success: false - exit_code: 1 + success: true + exit_code: 0 ----- stdout ----- ----- stderr -----