From e81efa5a3d13d22f9416823705ec09d31f45118f Mon Sep 17 00:00:00 2001 From: Harutaka Kawamura Date: Sat, 19 Nov 2022 04:02:29 +0900 Subject: [PATCH] Implement a `--show-source` setting (#698) --- Cargo.lock | 22 ++++++++- Cargo.toml | 1 + README.md | 10 +++-- flake8_to_ruff/src/converter.rs | 7 +++ src/cli.rs | 9 ++-- src/linter.rs | 22 +++++++-- src/main.rs | 4 ++ src/message.rs | 79 ++++++++++++++++++++++++++++++--- src/settings/configuration.rs | 2 + src/settings/mod.rs | 5 +++ src/settings/options.rs | 1 + src/settings/pyproject.rs | 6 +++ src/settings/user.rs | 2 + tests/integration_test.rs | 12 +++++ 14 files changed, 166 insertions(+), 16 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1965c797c9ead..f74c66d473105 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -49,6 +49,16 @@ version = "0.6.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c7021ce4924a3f25f802b2cccd1af585e39ea1a363a1aa2e72afe54b67a3a7a7" +[[package]] +name = "annotate-snippets" +version = "0.9.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c3b9d411ecbaf79885c6df4d75fff75858d5995ff25385657a28af47e82f9c36" +dependencies = [ + "unicode-width", + "yansi-term", +] + [[package]] name = "anyhow" version = "1.0.66" @@ -417,7 +427,7 @@ version = "1.2.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a5b5db619f3556839cb2223ae86ff3f9a09da2c5013be42bc9af08c9589bf70c" dependencies = [ - "annotate-snippets", + "annotate-snippets 0.6.1", ] [[package]] @@ -2240,6 +2250,7 @@ dependencies = [ name = "ruff" version = "0.0.127" dependencies = [ + "annotate-snippets 0.9.1", "anyhow", "assert_cmd", "atty", @@ -3312,3 +3323,12 @@ checksum = "56c1936c4cc7a1c9ab21a1ebb602eb942ba868cbd44a99cb7cdc5892335e1c85" dependencies = [ "linked-hash-map", ] + +[[package]] +name = "yansi-term" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fe5c30ade05e61656247b2e334a031dfd0cc466fadef865bdcdea8d537951bf1" +dependencies = [ + "winapi 0.3.9", +] diff --git a/Cargo.toml b/Cargo.toml index 2aef83f314762..aef408f30ffd9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,6 +13,7 @@ edition = "2021" name = "ruff" [dependencies] +annotate-snippets = { version = "0.9.1", features = ["color"] } anyhow = { version = "1.0.66" } atty = { version = "0.2.14" } bincode = { version = "1.3.3" } diff --git a/README.md b/README.md index bdec993c6ac93..04eb0dc970359 100644 --- a/README.md +++ b/README.md @@ -191,7 +191,7 @@ ruff path/to/code/ --select F401 --select F403 See `ruff --help` for more: ```shell -ruff: An extremely fast Python linter. +Ruff: An extremely fast Python linter. Usage: ruff [OPTIONS] ... @@ -231,10 +231,12 @@ Options: List of mappings from file pattern to code to exclude --format Output serialization format for error messages [default: text] [possible values: text, json] + --show-source + Show violations with source code --show-files - See the files ruff will be run against with the current settings + See the files Ruff will be run against with the current settings --show-settings - See ruff's settings + See Ruff's settings --add-noqa Enable automatic additions of noqa directives to failing lines --dummy-variable-rgx @@ -244,7 +246,7 @@ Options: --line-length Set the line-length for length-associated checks and automatic formatting --max-complexity - Set the maximum cyclomatic complexity for complexity-associated checks + Max McCabe complexity allowed for a function --stdin-filename The name of the file when passing it through stdin -h, --help diff --git a/flake8_to_ruff/src/converter.rs b/flake8_to_ruff/src/converter.rs index 206867321130c..3e633e28c8fc1 100644 --- a/flake8_to_ruff/src/converter.rs +++ b/flake8_to_ruff/src/converter.rs @@ -259,6 +259,7 @@ mod tests { per_file_ignores: None, dummy_variable_rgx: None, target_version: None, + show_source: None, flake8_annotations: None, flake8_bugbear: None, flake8_quotes: None, @@ -295,6 +296,7 @@ mod tests { per_file_ignores: None, dummy_variable_rgx: None, target_version: None, + show_source: None, flake8_annotations: None, flake8_bugbear: None, flake8_quotes: None, @@ -331,6 +333,7 @@ mod tests { per_file_ignores: None, dummy_variable_rgx: None, target_version: None, + show_source: None, flake8_annotations: None, flake8_bugbear: None, flake8_quotes: None, @@ -367,6 +370,7 @@ mod tests { per_file_ignores: None, dummy_variable_rgx: None, target_version: None, + show_source: None, flake8_annotations: None, flake8_bugbear: None, flake8_quotes: None, @@ -403,6 +407,7 @@ mod tests { per_file_ignores: None, dummy_variable_rgx: None, target_version: None, + show_source: None, flake8_annotations: None, flake8_bugbear: None, flake8_quotes: Some(flake8_quotes::settings::Options { @@ -482,6 +487,7 @@ mod tests { per_file_ignores: None, dummy_variable_rgx: None, target_version: None, + show_source: None, flake8_annotations: None, flake8_bugbear: None, flake8_quotes: None, @@ -519,6 +525,7 @@ mod tests { per_file_ignores: None, dummy_variable_rgx: None, target_version: None, + show_source: None, flake8_annotations: None, flake8_bugbear: None, flake8_quotes: Some(flake8_quotes::settings::Options { diff --git a/src/cli.rs b/src/cli.rs index d112979cd529e..46f4706fb5b3b 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -13,7 +13,7 @@ use crate::settings::configuration::Configuration; use crate::settings::types::{PatternPrefixPair, PerFileIgnore, PythonVersion}; #[derive(Debug, Parser)] -#[command(author, about = "ruff: An extremely fast Python linter.")] +#[command(author, about = "Ruff: An extremely fast Python linter.")] #[command(version)] pub struct Cli { #[arg(required = true)] @@ -72,10 +72,13 @@ pub struct Cli { /// Output serialization format for error messages. #[arg(long, value_enum, default_value_t=SerializationFormat::Text)] pub format: SerializationFormat, - /// See the files ruff will be run against with the current settings. + /// Show violations with source code. + #[arg(long)] + pub show_source: bool, + /// See the files Ruff will be run against with the current settings. #[arg(long)] pub show_files: bool, - /// See ruff's settings. + /// See Ruff's settings. #[arg(long)] pub show_settings: bool, /// Enable automatic additions of noqa directives to failing lines. diff --git a/src/linter.rs b/src/linter.rs index f8f2e84b1606a..34befbd2494d1 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -22,7 +22,7 @@ use crate::check_tokens::check_tokens; use crate::checks::{Check, CheckCode, CheckKind, LintSource}; use crate::code_gen::SourceGenerator; use crate::directives::Directives; -use crate::message::Message; +use crate::message::{Message, Source}; use crate::noqa::add_noqa; use crate::settings::Settings; use crate::source_code_locator::SourceCodeLocator; @@ -176,7 +176,15 @@ pub fn lint_stdin( // Convert to messages. Ok(checks .into_iter() - .map(|check| Message::from_check(path.to_string_lossy().to_string(), check)) + .map(|check| { + let filename = path.to_string_lossy().to_string(); + let source = if settings.show_source { + Some(Source::from_check(&check, &locator)) + } else { + None + }; + Message::from_check(check, filename, source) + }) .collect()) } @@ -233,7 +241,15 @@ pub fn lint_path( // Convert to messages. let messages: Vec = checks .into_iter() - .map(|check| Message::from_check(path.to_string_lossy().to_string(), check)) + .map(|check| { + let filename = path.to_string_lossy().to_string(); + let source = if settings.show_source { + Some(Source::from_check(&check, &locator)) + } else { + None + }; + Message::from_check(check, filename, source) + }) .collect(); #[cfg(not(target_family = "wasm"))] cache::set(path, &metadata, settings, autofix, &messages, mode); diff --git a/src/main.rs b/src/main.rs index bc428e59b0c03..ec44a8b2ad88e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -117,6 +117,7 @@ fn run_once( location: Default::default(), end_location: Default::default(), filename: path.to_string_lossy().to_string(), + source: None, }] } else { error!("Failed to check {}: {message}", path.to_string_lossy()); @@ -281,6 +282,9 @@ fn inner_main() -> Result { if let Some(fix) = fix { configuration.fix = fix; } + if cli.show_source { + configuration.show_source = true; + } if cli.show_settings && cli.show_files { eprintln!("Error: specify --show-settings or show-files (not both)."); diff --git a/src/message.rs b/src/message.rs index 1463d94153293..1c68d21d47cec 100644 --- a/src/message.rs +++ b/src/message.rs @@ -2,12 +2,16 @@ use std::cmp::Ordering; use std::fmt; use std::path::Path; +use annotate_snippets::display_list::{DisplayList, FormatOptions}; +use annotate_snippets::snippet::{Annotation, AnnotationType, Slice, Snippet, SourceAnnotation}; use colored::Colorize; use rustpython_parser::ast::Location; use serde::{Deserialize, Serialize}; +use crate::ast::types::Range; use crate::checks::{Check, CheckKind}; use crate::fs::relativize_path; +use crate::source_code_locator::SourceCodeLocator; #[derive(Debug, PartialEq, Eq, Serialize, Deserialize)] pub struct Message { @@ -16,16 +20,18 @@ pub struct Message { pub location: Location, pub end_location: Location, pub filename: String, + pub source: Option, } impl Message { - pub fn from_check(filename: String, check: Check) -> Self { + pub fn from_check(check: Check, filename: String, source: Option) -> Self { Self { kind: check.kind, fixed: check.fix.map(|fix| fix.applied).unwrap_or_default(), location: Location::new(check.location.row(), check.location.column() + 1), end_location: Location::new(check.end_location.row(), check.end_location.column() + 1), filename, + source, } } } @@ -48,8 +54,7 @@ impl PartialOrd for Message { impl fmt::Display for Message { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!( - f, + let label = format!( "{}{}{}{}{}{} {} {}", relativize_path(Path::new(&self.filename)).white().bold(), ":".cyan(), @@ -58,7 +63,71 @@ impl fmt::Display for Message { self.location.column(), ":".cyan(), self.kind.code().as_ref().red().bold(), - self.kind.body() - ) + self.kind.body(), + ); + match &self.source { + None => write!(f, "{}", label), + Some(source) => { + let snippet = Snippet { + title: Some(Annotation { + label: Some(&label), + annotation_type: AnnotationType::Error, + // The ID (error number) is already encoded in the `label`. + id: None, + }), + footer: vec![], + slices: vec![Slice { + source: &source.contents, + line_start: self.location.row(), + annotations: vec![SourceAnnotation { + label: self.kind.code().as_ref(), + annotation_type: AnnotationType::Error, + range: source.range, + }], + // The origin (file name, line number, and column number) is already encoded + // in the `label`. + origin: None, + fold: false, + }], + opt: FormatOptions { + color: true, + ..Default::default() + }, + }; + // `split_once(' ')` strips "error: " from `message`. + let message = DisplayList::from(snippet).to_string(); + let (_, message) = message.split_once(' ').unwrap(); + write!(f, "{}", message) + } + } + } +} + +#[derive(Debug, PartialEq, Eq, Serialize, Deserialize)] +pub struct Source { + pub contents: String, + pub range: (usize, usize), +} + +impl Source { + pub fn from_check(check: &Check, locator: &SourceCodeLocator) -> Self { + let source = locator.slice_source_code_range(&Range { + location: Location::new(check.location.row(), 0), + end_location: Location::new(check.end_location.row() + 1, 0), + }); + let num_chars_in_range = locator + .slice_source_code_range(&Range { + location: check.location, + end_location: check.end_location, + }) + .chars() + .count(); + Source { + contents: source.to_string(), + range: ( + check.location.column(), + check.location.column() + num_chars_in_range, + ), + } } } diff --git a/src/settings/configuration.rs b/src/settings/configuration.rs index 53cb60cf6c86e..8a22db7850dad 100644 --- a/src/settings/configuration.rs +++ b/src/settings/configuration.rs @@ -31,6 +31,7 @@ pub struct Configuration { pub select: Vec, pub src: Vec, pub target_version: PythonVersion, + pub show_source: bool, // Plugins pub flake8_annotations: flake8_annotations::settings::Settings, pub flake8_bugbear: flake8_bugbear::settings::Settings, @@ -134,6 +135,7 @@ impl Configuration { .collect() }) .unwrap_or_default(), + show_source: options.show_source.unwrap_or_default(), // Plugins flake8_annotations: options .flake8_annotations diff --git a/src/settings/mod.rs b/src/settings/mod.rs index 567c3178f93ef..2f923d6f4dd95 100644 --- a/src/settings/mod.rs +++ b/src/settings/mod.rs @@ -34,6 +34,7 @@ pub struct Settings { pub per_file_ignores: Vec, pub src: Vec, pub target_version: PythonVersion, + pub show_source: bool, // Plugins pub flake8_annotations: flake8_annotations::settings::Settings, pub flake8_bugbear: flake8_bugbear::settings::Settings, @@ -67,6 +68,7 @@ impl Settings { per_file_ignores: config.per_file_ignores, src: config.src, target_version: config.target_version, + show_source: config.show_source, } } @@ -87,6 +89,7 @@ impl Settings { isort: Default::default(), mccabe: Default::default(), pep8_naming: Default::default(), + show_source: Default::default(), } } @@ -107,6 +110,7 @@ impl Settings { isort: Default::default(), mccabe: Default::default(), pep8_naming: Default::default(), + show_source: Default::default(), } } } @@ -122,6 +126,7 @@ impl Hash for Settings { for value in self.per_file_ignores.iter() { value.hash(state); } + self.show_source.hash(state); self.target_version.hash(state); // Add plugin properties in alphabetical order. self.flake8_annotations.hash(state); diff --git a/src/settings/options.rs b/src/settings/options.rs index 0989e52501296..4521edfae51e6 100644 --- a/src/settings/options.rs +++ b/src/settings/options.rs @@ -24,6 +24,7 @@ pub struct Options { pub select: Option>, pub src: Option>, pub target_version: Option, + pub show_source: Option, // Plugins pub flake8_annotations: Option, pub flake8_bugbear: Option, diff --git a/src/settings/pyproject.rs b/src/settings/pyproject.rs index 8e789d4189d25..b5abb16f7fb83 100644 --- a/src/settings/pyproject.rs +++ b/src/settings/pyproject.rs @@ -146,6 +146,7 @@ mod tests { dummy_variable_rgx: None, src: None, target_version: None, + show_source: None, flake8_annotations: None, flake8_bugbear: None, flake8_quotes: None, @@ -180,6 +181,7 @@ line-length = 79 dummy_variable_rgx: None, src: None, target_version: None, + show_source: None, flake8_annotations: None, flake8_bugbear: None, flake8_quotes: None, @@ -214,6 +216,7 @@ exclude = ["foo.py"] dummy_variable_rgx: None, src: None, target_version: None, + show_source: None, flake8_annotations: None, flake8_bugbear: None, flake8_quotes: None, @@ -248,6 +251,7 @@ select = ["E501"] dummy_variable_rgx: None, src: None, target_version: None, + show_source: None, flake8_annotations: None, flake8_bugbear: None, flake8_quotes: None, @@ -283,6 +287,7 @@ ignore = ["E501"] dummy_variable_rgx: None, src: None, target_version: None, + show_source: None, flake8_annotations: None, flake8_bugbear: None, flake8_quotes: None, @@ -364,6 +369,7 @@ other-attribute = 1 dummy_variable_rgx: None, src: None, target_version: None, + show_source: None, flake8_annotations: None, flake8_bugbear: Some(flake8_bugbear::settings::Options { extend_immutable_calls: Some(vec![ diff --git a/src/settings/user.rs b/src/settings/user.rs index 690f3cdf49aa7..bb2c53abc90d0 100644 --- a/src/settings/user.rs +++ b/src/settings/user.rs @@ -49,6 +49,7 @@ pub struct UserConfiguration { pub select: Vec, pub src: Vec, pub target_version: PythonVersion, + pub show_source: bool, // Plugins pub flake8_annotations: flake8_annotations::settings::Settings, pub flake8_quotes: flake8_quotes::settings::Settings, @@ -96,6 +97,7 @@ impl UserConfiguration { select: configuration.select, src: configuration.src, target_version: configuration.target_version, + show_source: configuration.show_source, flake8_annotations: configuration.flake8_annotations, flake8_quotes: configuration.flake8_quotes, flake8_tidy_imports: configuration.flake8_tidy_imports, diff --git a/tests/integration_test.rs b/tests/integration_test.rs index 086fa992be437..2969952d3bf4e 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -78,3 +78,15 @@ fn test_stdin_autofix_when_no_issues_should_still_print_contents() -> Result<()> ); Ok(()) } + +#[test] +fn test_show_source() -> Result<()> { + let mut cmd = Command::cargo_bin(crate_name!())?; + let output = cmd + .args(["-", "--show-source"]) + .write_stdin("l = 1") + .assert() + .failure(); + assert!(str::from_utf8(&output.get_output().stdout)?.contains("l = 1")); + Ok(()) +}