From 46f5053c7343f2f6b4826c9deb36db042531ade3 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Thu, 1 Dec 2022 16:30:56 -0500 Subject: [PATCH] Include fixes in JSON API output (#988) --- src/main.rs | 46 +++++++++++++++++++++++++++------------ src/message.rs | 3 +++ src/printer.rs | 7 +++--- tests/integration_test.rs | 32 +++++++++++++++++++++++++-- 4 files changed, 69 insertions(+), 19 deletions(-) diff --git a/src/main.rs b/src/main.rs index 24f0bdc1fe9597..7a39497bfd4e58 100644 --- a/src/main.rs +++ b/src/main.rs @@ -37,6 +37,7 @@ use log::{debug, error}; use notify::{raw_watcher, RecursiveMode, Watcher}; #[cfg(not(target_family = "wasm"))] use rayon::prelude::*; +use ruff::autofix::fixer; use rustpython_ast::Location; use walkdir::DirEntry; @@ -60,14 +61,23 @@ fn read_from_stdin() -> Result { Ok(buffer) } -fn run_once_stdin(settings: &Settings, filename: &Path, autofix: bool) -> Result { +fn run_once_stdin( + settings: &Settings, + filename: &Path, + autofix: &fixer::Mode, +) -> Result { let stdin = read_from_stdin()?; - let mut diagnostics = lint_stdin(filename, &stdin, settings, &autofix.into())?; + let mut diagnostics = lint_stdin(filename, &stdin, settings, autofix)?; diagnostics.messages.sort_unstable(); Ok(diagnostics) } -fn run_once(files: &[PathBuf], settings: &Settings, cache: bool, autofix: bool) -> Diagnostics { +fn run_once( + files: &[PathBuf], + settings: &Settings, + cache: bool, + autofix: &fixer::Mode, +) -> Diagnostics { // Collect all the files to check. let start = Instant::now(); let paths: Vec> = files @@ -83,7 +93,7 @@ fn run_once(files: &[PathBuf], settings: &Settings, cache: bool, autofix: bool) match entry { Ok(entry) => { let path = entry.path(); - lint_path(path, settings, &cache.into(), &autofix.into()) + lint_path(path, settings, &cache.into(), autofix) .map_err(|e| (Some(path.to_owned()), e.to_string())) } Err(e) => Err(( @@ -99,6 +109,7 @@ fn run_once(files: &[PathBuf], settings: &Settings, cache: bool, autofix: bool) kind: CheckKind::IOError(message), location: Location::default(), end_location: Location::default(), + fix: None, filename: path.to_string_lossy().to_string(), source: None, }]) @@ -266,7 +277,13 @@ fn inner_main() -> Result { } // Extract settings for internal use. - let fix_enabled: bool = configuration.fix; + let autofix = if configuration.fix { + fixer::Mode::Apply + } else if matches!(configuration.format, SerializationFormat::Json) { + fixer::Mode::Generate + } else { + fixer::Mode::None + }; let settings = Settings::from_configuration(configuration, project_root.as_ref())?; // Now that we've inferred the appropriate log level, add some debug @@ -299,10 +316,7 @@ fn inner_main() -> Result { let printer = Printer::new(&settings.format, &log_level); if cli.watch { - if settings.format != SerializationFormat::Text { - eprintln!("Warning: --format 'text' is used in watch mode."); - } - if fix_enabled { + if matches!(autofix, fixer::Mode::Generate | fixer::Mode::Apply) { eprintln!("Warning: --fix is not enabled in watch mode."); } if cli.add_noqa { @@ -311,12 +325,15 @@ fn inner_main() -> Result { if cli.autoformat { eprintln!("Warning: --autoformat is not enabled in watch mode."); } + if settings.format != SerializationFormat::Text { + eprintln!("Warning: --format 'text' is used in watch mode."); + } // Perform an initial run instantly. printer.clear_screen()?; printer.write_to_user("Starting linter in watch mode...\n"); - let messages = run_once(&cli.files, &settings, cache_enabled, false); + let messages = run_once(&cli.files, &settings, cache_enabled, &fixer::Mode::None); printer.write_continuously(&messages)?; // Configure the file watcher. @@ -334,7 +351,8 @@ fn inner_main() -> Result { printer.clear_screen()?; printer.write_to_user("File change detected...\n"); - let messages = run_once(&cli.files, &settings, cache_enabled, false); + let messages = + run_once(&cli.files, &settings, cache_enabled, &fixer::Mode::None); printer.write_continuously(&messages)?; } } @@ -359,15 +377,15 @@ fn inner_main() -> Result { let diagnostics = if is_stdin { let filename = cli.stdin_filename.unwrap_or_else(|| "-".to_string()); let path = Path::new(&filename); - run_once_stdin(&settings, path, fix_enabled)? + run_once_stdin(&settings, path, &autofix)? } else { - run_once(&cli.files, &settings, cache_enabled, fix_enabled) + run_once(&cli.files, &settings, cache_enabled, &autofix) }; // Always try to print violations (the printer itself may suppress output), // unless we're writing fixes via stdin (in which case, the transformed // source code goes to stdout). - if !(is_stdin && fix_enabled) { + if !(is_stdin && matches!(autofix, fixer::Mode::Apply)) { printer.write_once(&diagnostics)?; } diff --git a/src/message.rs b/src/message.rs index 43755060a541c8..616200c8687902 100644 --- a/src/message.rs +++ b/src/message.rs @@ -4,6 +4,7 @@ use rustpython_parser::ast::Location; use serde::{Deserialize, Serialize}; use crate::ast::types::Range; +use crate::autofix::Fix; use crate::checks::{Check, CheckKind}; use crate::source_code_locator::SourceCodeLocator; @@ -12,6 +13,7 @@ pub struct Message { pub kind: CheckKind, pub location: Location, pub end_location: Location, + pub fix: Option, pub filename: String, pub source: Option, } @@ -22,6 +24,7 @@ impl Message { kind: check.kind, location: Location::new(check.location.row(), check.location.column() + 1), end_location: Location::new(check.end_location.row(), check.end_location.column() + 1), + fix: check.fix, filename, source, } diff --git a/src/printer.rs b/src/printer.rs index 7414b4a9742c88..1c30b829f13d9d 100644 --- a/src/printer.rs +++ b/src/printer.rs @@ -9,7 +9,8 @@ use itertools::iterate; use rustpython_parser::ast::Location; use serde::Serialize; -use crate::checks::{CheckCode, CheckKind}; +use crate::autofix::Fix; +use crate::checks::CheckCode; use crate::fs::relativize_path; use crate::linter::Diagnostics; use crate::logging::LogLevel; @@ -19,9 +20,9 @@ use crate::tell_user; #[derive(Serialize)] struct ExpandedMessage<'a> { - kind: &'a CheckKind, code: &'a CheckCode, message: String, + fix: Option<&'a Fix>, location: Location, end_location: Location, filename: &'a str, @@ -85,9 +86,9 @@ impl<'a> Printer<'a> { .messages .iter() .map(|message| ExpandedMessage { - kind: &message.kind, code: message.kind.code(), message: message.kind.body(), + fix: message.fix.as_ref(), location: message.location, end_location: message.end_location, filename: &message.filename, diff --git a/tests/integration_test.rs b/tests/integration_test.rs index 128a743052ba54..4b4f4126265397 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -21,7 +21,11 @@ fn test_stdin_error() -> Result<()> { .write_stdin("import os\n") .assert() .failure(); - assert!(str::from_utf8(&output.get_output().stdout)?.contains("-:1:8: F401")); + assert_eq!( + str::from_utf8(&output.get_output().stdout)?, + "Found 1 error(s).\n-:1:8: F401 `os` imported but unused\n1 potentially fixable with the \ + --fix option.\n" + ); Ok(()) } @@ -33,7 +37,31 @@ fn test_stdin_filename() -> Result<()> { .write_stdin("import os\n") .assert() .failure(); - assert!(str::from_utf8(&output.get_output().stdout)?.contains("F401.py:1:8: F401")); + assert_eq!( + str::from_utf8(&output.get_output().stdout)?, + "Found 1 error(s).\nF401.py:1:8: F401 `os` imported but unused\n1 potentially fixable \ + with the --fix option.\n" + ); + Ok(()) +} + +#[test] +fn test_stdin_json() -> Result<()> { + let mut cmd = Command::cargo_bin(crate_name!())?; + let output = cmd + .args(["-", "--format", "json", "--stdin-filename", "F401.py"]) + .write_stdin("import os\n") + .assert() + .failure(); + assert_eq!( + str::from_utf8(&output.get_output().stdout)?, + "[\n {\n \"code\": \"F401\",\n \"message\": \"`os` imported but unused\",\n \ + \"fix\": {\n \"content\": \"\",\n \"location\": {\n \"row\": 1,\n \ + \"column\": 0\n },\n \"end_location\": {\n \"row\": 2,\n \ + \"column\": 0\n }\n },\n \"location\": {\n \"row\": 1,\n \ + \"column\": 8\n },\n \"end_location\": {\n \"row\": 1,\n \"column\": \ + 10\n },\n \"filename\": \"F401.py\"\n }\n]\n" + ); Ok(()) }