language-servers/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-10-03 07:04:53 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

Add a --show-fixes flag to include applied fixes in output (#2707)

Browse source
This commit is contained in:
Charlie Marsh 2023-02-12 15:48:01 -05:00 • committed by GitHub
parent c399b3e6c1
commit 1666e8ba1e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
14 changed files with 332 additions and 122 deletions
Download patch file Download diff file Expand all files Collapse all files

12
crates/ruff_cli/src/args.rs
View file

@ -75,9 +75,14 @@ pub struct CheckArgs {
show_source: bool,
#[clap(long, overrides_with("show_source"), hide = true)]
no_show_source: bool,
/// Show an enumeration of all autofixed lint violations.
#[arg(long, overrides_with("no_show_fixes"))]
show_fixes: bool,
#[clap(long, overrides_with("show_fixes"), hide = true)]
no_show_fixes: bool,
/// Avoid writing any fixed files back; instead, output a diff for each
/// changed file to stdout.
#[arg(long)]
#[arg(long, conflicts_with = "show_fixes")]
pub diff: bool,
/// Run in watch mode by re-running whenever files change.
#[arg(short, long)]
@ -380,6 +385,7 @@ impl CheckArgs {
fix_only: resolve_bool_arg(self.fix_only, self.no_fix_only),
force_exclude: resolve_bool_arg(self.force_exclude, self.no_force_exclude),
format: self.format,
show_fixes: resolve_bool_arg(self.show_fixes, self.no_show_fixes),
update_check: resolve_bool_arg(self.update_check, self.no_update_check),
},
)
@ -438,6 +444,7 @@ pub struct Overrides {
pub fix_only: Option<bool>,
pub force_exclude: Option<bool>,
pub format: Option<SerializationFormat>,
pub show_fixes: Option<bool>,
pub update_check: Option<bool>,
}
@ -492,6 +499,9 @@ impl ConfigProcessor for &Overrides {
if let Some(show_source) = &self.show_source {
config.show_source = Some(*show_source);
}
if let Some(show_fixes) = &self.show_fixes {
config.show_fixes = Some(*show_fixes);
}
if let Some(target_version) = &self.target_version {
config.target_version = Some(*target_version);
}

51
crates/ruff_cli/src/diagnostics.rs
View file

@ -1,4 +1,5 @@
#![cfg_attr(target_family = "wasm", allow(dead_code))]
use std::fs::write;
use std::io;
use std::io::Write;
@ -8,30 +9,45 @@ use std::path::Path;
use anyhow::Result;
use colored::Colorize;
use log::{debug, error};
use ruff::linter::{lint_fix, lint_only, LinterResult};
use rustc_hash::FxHashMap;
use similar::TextDiff;
use ruff::linter::{lint_fix, lint_only, FixTable, LinterResult};
use ruff::message::Message;
use ruff::settings::{flags, AllSettings, Settings};
use ruff::{fix, fs};
use similar::TextDiff;
use crate::cache;
#[derive(Debug, Default)]
pub struct Diagnostics {
pub messages: Vec<Message>,
pub fixed: usize,
pub fixed: FxHashMap<String, FixTable>,
}
impl Diagnostics {
pub fn new(messages: Vec<Message>) -> Self {
Self { messages, fixed: 0 }
Self {
messages,
fixed: FxHashMap::default(),
}
}
}
impl AddAssign for Diagnostics {
fn add_assign(&mut self, other: Self) {
self.messages.extend(other.messages);
self.fixed += other.fixed;
for (filename, fixed) in other.fixed {
if fixed.is_empty() {
continue;
}
let fixed_in_file = self.fixed.entry(filename).or_default();
for (rule, count) in fixed {
if count > 0 {
*fixed_in_file.entry(rule).or_default() += count;
}
}
}
}
}
@ -77,7 +93,7 @@ pub fn lint_path(
) = if matches!(autofix, fix::FixMode::Apply | fix::FixMode::Diff) {
if let Ok((result, transformed, fixed)) = lint_fix(&contents, path, package, &settings.lib)
{
if fixed > 0 {
if !fixed.is_empty() {
if matches!(autofix, fix::FixMode::Apply) {
write(path, transformed.as_bytes())?;
} else if matches!(autofix, fix::FixMode::Diff) {
@ -94,12 +110,12 @@ pub fn lint_path(
} else {
// If we fail to autofix, lint the original source code.
let result = lint_only(&contents, path, package, &settings.lib, autofix.into());
let fixed = 0;
let fixed = FxHashMap::default();
(result, fixed)
}
} else {
let result = lint_only(&contents, path, package, &settings.lib, autofix.into());
let fixed = 0;
let fixed = FxHashMap::default();
(result, fixed)
};
@ -128,7 +144,10 @@ pub fn lint_path(
}
}
Ok(Diagnostics { messages, fixed })
Ok(Diagnostics {
messages,
fixed: FxHashMap::from_iter([(fs::relativize_path(path), fixed)]),
})
}
/// Generate `Diagnostic`s from source code content derived from
@ -159,7 +178,7 @@ pub fn lint_stdin(
io::stdout().write_all(transformed.as_bytes())?;
} else if matches!(autofix, fix::FixMode::Diff) {
// But only write a diff if it's non-empty.
if fixed > 0 {
if !fixed.is_empty() {
let text_diff = TextDiff::from_lines(contents, &transformed);
let mut unified_diff = text_diff.unified_diff();
if let Some(path) = path {
@ -183,7 +202,7 @@ pub fn lint_stdin(
settings,
autofix.into(),
);
let fixed = 0;
let fixed = FxHashMap::default();
// Write the contents to stdout anyway.
if matches!(autofix, fix::FixMode::Apply) {
@ -200,7 +219,7 @@ pub fn lint_stdin(
settings,
autofix.into(),
);
let fixed = 0;
let fixed = FxHashMap::default();
(result, fixed)
};
@ -211,5 +230,11 @@ pub fn lint_stdin(
);
}
Ok(Diagnostics { messages, fixed })
Ok(Diagnostics {
messages,
fixed: FxHashMap::from_iter([(
fs::relativize_path(path.unwrap_or_else(|| Path::new("-"))),
fixed,
)]),
})
}

21
crates/ruff_cli/src/main.rs
View file

@ -14,7 +14,7 @@ use ::ruff::settings::types::SerializationFormat;
use ::ruff::settings::CliSettings;
use ::ruff::{fix, fs, warn_user_once};
use args::{Args, CheckArgs, Command};
use printer::{Printer, Violations};
use printer::{Flags as PrinterFlags, Printer};
pub(crate) mod args;
mod cache;
@ -140,6 +140,7 @@ fn check(args: CheckArgs, log_level: LogLevel) -> Result<ExitStatus> {
fix,
fix_only,
format,
show_fixes,
update_check,
..
} = match &pyproject_strategy {
@ -166,12 +167,14 @@ fn check(args: CheckArgs, log_level: LogLevel) -> Result<ExitStatus> {
} else {
fix::FixMode::None
};
let violations = if cli.diff || fix_only {
Violations::Hide
} else {
Violations::Show
};
let cache = !cli.no_cache;
let mut printer_flags = PrinterFlags::empty();
if !(cli.diff || fix_only) {
printer_flags |= PrinterFlags::SHOW_VIOLATIONS;
}
if show_fixes {
printer_flags |= PrinterFlags::SHOW_FIXES;
}
#[cfg(debug_assertions)]
if cache {
@ -195,7 +198,7 @@ fn check(args: CheckArgs, log_level: LogLevel) -> Result<ExitStatus> {
return Ok(ExitStatus::Success);
}
let printer = Printer::new(format, log_level, autofix, violations);
let printer = Printer::new(format, log_level, autofix, printer_flags);
if cli.watch {
if !matches!(autofix, fix::FixMode::None) {
@ -292,11 +295,11 @@ fn check(args: CheckArgs, log_level: LogLevel) -> Result<ExitStatus> {
if !cli.exit_zero {
if cli.diff || fix_only {
if diagnostics.fixed > 0 {
if !diagnostics.fixed.is_empty() {
return Ok(ExitStatus::Failure);
}
} else if cli.exit_non_zero_on_fix {
if diagnostics.fixed > 0 || !diagnostics.messages.is_empty() {
if !diagnostics.fixed.is_empty() || !diagnostics.messages.is_empty() {
return Ok(ExitStatus::Failure);
}
} else {

223
crates/ruff_cli/src/printer.rs
View file

@ -1,3 +1,4 @@
use std::cmp::Reverse;
use std::collections::BTreeMap;
use std::fmt::Display;
use std::io;
@ -10,10 +11,13 @@ use anyhow::Result;
use colored::control::SHOULD_COLORIZE;
use colored::Colorize;
use itertools::{iterate, Itertools};
use rustc_hash::FxHashMap;
use serde::Serialize;
use serde_json::json;
use bitflags::bitflags;
use ruff::fs::relativize_path;
use ruff::linter::FixTable;
use ruff::logging::LogLevel;
use ruff::message::{Location, Message};
use ruff::registry::Rule;
@ -22,10 +26,12 @@ use ruff::{fix, notify_user};
use crate::diagnostics::Diagnostics;
/// Enum to control whether lint violations are shown to the user.
pub enum Violations {
Show,
Hide,
bitflags! {
#[derive(Default)]
pub struct Flags: u32 {
const SHOW_VIOLATIONS = 0b0000_0001;
const SHOW_FIXES = 0b0000_0010;
}
}
#[derive(Serialize)]
@ -74,22 +80,22 @@ impl<'a> From<&'a Rule> for SerializeRuleAsCode<'a> {
pub struct Printer {
format: SerializationFormat,
log_level: LogLevel,
autofix: fix::FixMode,
violations: Violations,
autofix_level: fix::FixMode,
flags: Flags,
}
impl Printer {
pub const fn new(
format: SerializationFormat,
log_level: LogLevel,
autofix: fix::FixMode,
violations: Violations,
autofix_level: fix::FixMode,
flags: Flags,
) -> Self {
Self {
format,
log_level,
autofix,
violations,
autofix_level,
flags,
}
}
@ -101,46 +107,51 @@ impl Printer {
fn post_text<T: Write>(&self, stdout: &mut T, diagnostics: &Diagnostics) -> Result<()> {
if self.log_level >= LogLevel::Default {
match self.violations {
Violations::Show => {
let fixed = diagnostics.fixed;
let remaining = diagnostics.messages.len();
let total = fixed + remaining;
if fixed > 0 {
let s = if total == 1 { "" } else { "s" };
if self.flags.contains(Flags::SHOW_VIOLATIONS) {
let fixed = diagnostics
.fixed
.values()
.flat_map(std::collections::HashMap::values)
.sum::<usize>();
let remaining = diagnostics.messages.len();
let total = fixed + remaining;
if fixed > 0 {
let s = if total == 1 { "" } else { "s" };
writeln!(
stdout,
"Found {total} error{s} ({fixed} fixed, {remaining} remaining)."
)?;
} else if remaining > 0 {
let s = if remaining == 1 { "" } else { "s" };
writeln!(stdout, "Found {remaining} error{s}.")?;
}
if show_fix_status(self.autofix_level) {
let num_fixable = diagnostics
.messages
.iter()
.filter(|message| message.kind.fixable())
.count();
if num_fixable > 0 {
writeln!(
stdout,
"Found {total} error{s} ({fixed} fixed, {remaining} remaining)."
"[{}] {num_fixable} potentially fixable with the --fix option.",
"*".cyan(),
)?;
} else if remaining > 0 {
let s = if remaining == 1 { "" } else { "s" };
writeln!(stdout, "Found {remaining} error{s}.")?;
}
if !matches!(self.autofix, fix::FixMode::Apply) {
let num_fixable = diagnostics
.messages
.iter()
.filter(|message| message.kind.fixable())
.count();
if num_fixable > 0 {
writeln!(
stdout,
"[{}] {num_fixable} potentially fixable with the --fix option.",
"*".cyan(),
)?;
}
}
}
Violations::Hide => {
let fixed = diagnostics.fixed;
if fixed > 0 {
let s = if fixed == 1 { "" } else { "s" };
if matches!(self.autofix, fix::FixMode::Apply) {
writeln!(stdout, "Fixed {fixed} error{s}.")?;
} else if matches!(self.autofix, fix::FixMode::Diff) {
writeln!(stdout, "Would fix {fixed} error{s}.")?;
}
} else {
let fixed = diagnostics
.fixed
.values()
.flat_map(std::collections::HashMap::values)
.sum::<usize>();
if fixed > 0 {
let s = if fixed == 1 { "" } else { "s" };
if matches!(self.autofix_level, fix::FixMode::Apply) {
writeln!(stdout, "Fixed {fixed} error{s}.")?;
} else if matches!(self.autofix_level, fix::FixMode::Diff) {
writeln!(stdout, "Would fix {fixed} error{s}.")?;
}
}
}
@ -153,7 +164,7 @@ impl Printer {
return Ok(());
}
if matches!(self.violations, Violations::Hide) {
if !self.flags.contains(Flags::SHOW_VIOLATIONS) {
let mut stdout = BufWriter::new(io::stdout().lock());
if matches!(
self.format,
@ -230,9 +241,15 @@ impl Printer {
}
SerializationFormat::Text => {
for message in &diagnostics.messages {
print_message(&mut stdout, message)?;
print_message(&mut stdout, message, self.autofix_level)?;
}
if self.flags.contains(Flags::SHOW_FIXES) {
if !diagnostics.fixed.is_empty() {
writeln!(stdout)?;
print_fixed(&mut stdout, &diagnostics.fixed)?;
writeln!(stdout)?;
}
}
self.post_text(&mut stdout, diagnostics)?;
}
SerializationFormat::Grouped => {
@ -263,11 +280,25 @@ impl Printer {
// Print each message.
for message in messages {
print_grouped_message(&mut stdout, message, row_length, column_length)?;
print_grouped_message(
&mut stdout,
message,
self.autofix_level,
row_length,
column_length,
)?;
}
writeln!(stdout)?;
}
if self.flags.contains(Flags::SHOW_FIXES) {
if !diagnostics.fixed.is_empty() {
writeln!(stdout)?;
print_fixed(&mut stdout, &diagnostics.fixed)?;
writeln!(stdout)?;
}
}
self.post_text(&mut stdout, diagnostics)?;
}
SerializationFormat::Github => {
@ -466,7 +497,7 @@ impl Printer {
writeln!(stdout)?;
}
for message in &diagnostics.messages {
print_message(&mut stdout, message)?;
print_message(&mut stdout, message, self.autofix_level)?;
}
}
stdout.flush()?;
@ -499,34 +530,52 @@ fn num_digits(n: usize) -> usize {
.max(1)
}
struct CodeAndBody<'a>(&'a Message);
struct CodeAndBody<'a>(&'a Message, fix::FixMode);
impl Display for CodeAndBody<'_> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(
f,
"{code} {autofix}{body}",
code = self.0.kind.rule().code().red().bold(),
autofix = self
.0
.kind
.fixable()
.then_some(format_args!("[{}] ", "*".cyan()))
.unwrap_or(format_args!("")),
body = self.0.kind.body(),
)
if show_fix_status(self.1) && self.0.kind.fixable() {
write!(
f,
"{code} {autofix}{body}",
code = self.0.kind.rule().code().red().bold(),
autofix = format_args!("[{}] ", "*".cyan()),
body = self.0.kind.body(),
)
} else {
write!(
f,
"{code} {body}",
code = self.0.kind.rule().code().red().bold(),
body = self.0.kind.body(),
)
}
}
}
/// Return `true` if the [`Printer`] should indicate that a rule is fixable.
fn show_fix_status(autofix_level: fix::FixMode) -> bool {
// If we're in application mode, avoid indicating that a rule is fixable.
// If the specific violation were truly fixable, it would've been fixed in
// this pass! (We're occasionally unable to determine whether a specific
// violation is fixable without trying to fix it, so if autofix is not
// enabled, we may inadvertently indicate that a rule is fixable.)
!matches!(autofix_level, fix::FixMode::Apply)
}
/// Print a single `Message` with full details.
fn print_message<T: Write>(stdout: &mut T, message: &Message) -> Result<()> {
fn print_message<T: Write>(
stdout: &mut T,
message: &Message,
autofix_level: fix::FixMode,
) -> Result<()> {
let label = format!(
"{path}{sep}{row}{sep}{col}{sep} {code_and_body}",
path = relativize_path(Path::new(&message.filename)).bold(),
sep = ":".cyan(),
row = message.location.row(),
col = message.location.column(),
code_and_body = CodeAndBody(message)
code_and_body = CodeAndBody(message, autofix_level)
);
writeln!(stdout, "{label}")?;
if let Some(source) = &message.source {
@ -574,11 +623,53 @@ fn print_message<T: Write>(stdout: &mut T, message: &Message) -> Result<()> {
Ok(())
}
fn print_fixed<T: Write>(stdout: &mut T, fixed: &FxHashMap<String, FixTable>) -> Result<()> {
let total = fixed
.values()
.map(|table| table.values().sum::<usize>())
.sum::<usize>();
assert!(total > 0);
let num_digits = num_digits(
*fixed
.values()
.filter_map(|table| table.values().max())
.max()
.unwrap(),
);
let s = if total == 1 { "" } else { "s" };
let label = format!("Fixed {total} error{s}:", total = total, s = s);
writeln!(stdout, "{}", label.bold().green())?;
for (filename, table) in fixed
.iter()
.sorted_by_key(|(filename, ..)| filename.as_str())
{
writeln!(
stdout,
"{} {}{}",
"-".cyan(),
relativize_path(Path::new(filename)).bold(),
":".cyan()
)?;
for (rule, count) in table.iter().sorted_by_key(|(.., count)| Reverse(*count)) {
writeln!(
stdout,
" {count:>num_digits$} × {} ({})",
rule.code().red().bold(),
rule.as_ref(),
)?;
}
}
Ok(())
}
/// Print a grouped `Message`, assumed to be printed in a group with others from
/// the same file.
fn print_grouped_message<T: Write>(
stdout: &mut T,
message: &Message,
autofix_level: fix::FixMode,
row_length: usize,
column_length: usize,
) -> Result<()> {
@ -589,7 +680,7 @@ fn print_grouped_message<T: Write>(
sep = ":".cyan(),
col = message.location.column(),
col_padding = " ".repeat(column_length - num_digits(message.location.column())),
code_and_body = CodeAndBody(message),
code_and_body = CodeAndBody(message, autofix_level),
);
writeln!(stdout, "{label}")?;
if let Some(source) = &message.source {