diff --git a/crates/ruff/src/autofix/mod.rs b/crates/ruff/src/autofix/mod.rs index 98d9c51978c4b..358a693cc0bcb 100644 --- a/crates/ruff/src/autofix/mod.rs +++ b/crates/ruff/src/autofix/mod.rs @@ -13,6 +13,7 @@ use crate::registry::{AsRule, Rule}; pub(crate) mod codemods; pub(crate) mod edits; +pub(crate) mod snippet; pub(crate) mod source_map; pub(crate) struct FixResult { diff --git a/crates/ruff/src/autofix/snippet.rs b/crates/ruff/src/autofix/snippet.rs new file mode 100644 index 0000000000000..5d83b87de97c2 --- /dev/null +++ b/crates/ruff/src/autofix/snippet.rs @@ -0,0 +1,36 @@ +use unicode_width::UnicodeWidthStr; + +/// A snippet of source code for user-facing display, as in a diagnostic. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) struct SourceCodeSnippet(String); + +impl SourceCodeSnippet { + pub(crate) fn new(source_code: String) -> Self { + Self(source_code) + } + + /// Return the full snippet for user-facing display, or `None` if the snippet should be + /// truncated. + pub(crate) fn full_display(&self) -> Option<&str> { + if Self::should_truncate(&self.0) { + None + } else { + Some(&self.0) + } + } + + /// Return a truncated snippet for user-facing display. + pub(crate) fn truncated_display(&self) -> &str { + if Self::should_truncate(&self.0) { + "..." + } else { + &self.0 + } + } + + /// Returns `true` if the source code should be truncated when included in a user-facing + /// diagnostic. + fn should_truncate(source_code: &str) -> bool { + source_code.width() > 50 || source_code.contains(['\r', '\n']) + } +} diff --git a/crates/ruff/src/rules/flake8_simplify/rules/yoda_conditions.rs b/crates/ruff/src/rules/flake8_simplify/rules/yoda_conditions.rs index 673925a416a93..2bed494d4af43 100644 --- a/crates/ruff/src/rules/flake8_simplify/rules/yoda_conditions.rs +++ b/crates/ruff/src/rules/flake8_simplify/rules/yoda_conditions.rs @@ -1,14 +1,15 @@ use anyhow::Result; use libcst_native::CompOp; -use ruff_python_ast::{self as ast, CmpOp, Expr, Ranged, UnaryOp}; -use crate::autofix::codemods::CodegenStylist; use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation}; use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::{self as ast, CmpOp, Expr, Ranged, UnaryOp}; use ruff_python_codegen::Stylist; use ruff_python_stdlib::str::{self}; use ruff_source_file::Locator; +use crate::autofix::codemods::CodegenStylist; +use crate::autofix::snippet::SourceCodeSnippet; use crate::checkers::ast::Checker; use crate::cst::matchers::{match_comparison, match_expression}; use crate::registry::AsRule; @@ -45,7 +46,7 @@ use crate::registry::AsRule; /// - [Python documentation: Assignment statements](https://docs.python.org/3/reference/simple_stmts.html#assignment-statements) #[violation] pub struct YodaConditions { - pub suggestion: Option, + suggestion: Option, } impl Violation for YodaConditions { @@ -54,7 +55,10 @@ impl Violation for YodaConditions { #[derive_message_formats] fn message(&self) -> String { let YodaConditions { suggestion } = self; - if let Some(suggestion) = suggestion { + if let Some(suggestion) = suggestion + .as_ref() + .and_then(SourceCodeSnippet::full_display) + { format!("Yoda conditions are discouraged, use `{suggestion}` instead") } else { format!("Yoda conditions are discouraged") @@ -63,9 +67,13 @@ impl Violation for YodaConditions { fn autofix_title(&self) -> Option { let YodaConditions { suggestion } = self; - suggestion - .as_ref() - .map(|suggestion| format!("Replace Yoda condition with `{suggestion}`")) + suggestion.as_ref().map(|suggestion| { + if let Some(suggestion) = suggestion.full_display() { + format!("Replace Yoda condition with `{suggestion}`") + } else { + format!("Replace Yoda condition") + } + }) } } @@ -178,7 +186,7 @@ pub(crate) fn yoda_conditions( if let Ok(suggestion) = reverse_comparison(expr, checker.locator(), checker.stylist()) { let mut diagnostic = Diagnostic::new( YodaConditions { - suggestion: Some(suggestion.to_string()), + suggestion: Some(SourceCodeSnippet::new(suggestion.clone())), }, expr.range(), ); diff --git a/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM300_SIM300.py.snap b/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM300_SIM300.py.snap index 38307e22c2f73..50c2af9f8c7cc 100644 --- a/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM300_SIM300.py.snap +++ b/crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM300_SIM300.py.snap @@ -290,7 +290,7 @@ SIM300.py:15:1: SIM300 [*] Yoda conditions are discouraged, use `(number - 100) 17 17 | 18 18 | # OK -SIM300.py:16:1: SIM300 [*] Yoda conditions are discouraged, use `(60 * 60) < SomeClass().settings.SOME_CONSTANT_VALUE` instead +SIM300.py:16:1: SIM300 [*] Yoda conditions are discouraged | 14 | JediOrder.YODA == age # SIM300 15 | 0 < (number - 100) # SIM300 @@ -299,7 +299,7 @@ SIM300.py:16:1: SIM300 [*] Yoda conditions are discouraged, use `(60 * 60) < Som 17 | 18 | # OK | - = help: Replace Yoda condition with `(60 * 60) < SomeClass().settings.SOME_CONSTANT_VALUE` + = help: Replace Yoda condition ℹ Fix 13 13 | YODA >= age # SIM300 diff --git a/crates/ruff/src/rules/flynt/rules/static_join_to_fstring.rs b/crates/ruff/src/rules/flynt/rules/static_join_to_fstring.rs index 32f52aa043dc6..980680bb85b2b 100644 --- a/crates/ruff/src/rules/flynt/rules/static_join_to_fstring.rs +++ b/crates/ruff/src/rules/flynt/rules/static_join_to_fstring.rs @@ -1,10 +1,11 @@ use itertools::Itertools; -use ruff_python_ast::{self as ast, Arguments, Constant, Expr, Ranged}; -use ruff_text_size::TextRange; use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::{self as ast, Arguments, Constant, Expr, Ranged}; +use ruff_text_size::TextRange; +use crate::autofix::snippet::SourceCodeSnippet; use crate::checkers::ast::Checker; use crate::registry::AsRule; use crate::rules::flynt::helpers; @@ -29,19 +30,27 @@ use crate::rules::flynt::helpers; /// - [Python documentation: f-strings](https://docs.python.org/3/reference/lexical_analysis.html#f-strings) #[violation] pub struct StaticJoinToFString { - expr: String, + expression: SourceCodeSnippet, } impl AlwaysAutofixableViolation for StaticJoinToFString { #[derive_message_formats] fn message(&self) -> String { - let StaticJoinToFString { expr } = self; - format!("Consider `{expr}` instead of string join") + let StaticJoinToFString { expression } = self; + if let Some(expression) = expression.full_display() { + format!("Consider `{expression}` instead of string join") + } else { + format!("Consider f-string instead of string join") + } } fn autofix_title(&self) -> String { - let StaticJoinToFString { expr } = self; - format!("Replace with `{expr}`") + let StaticJoinToFString { expression } = self; + if let Some(expression) = expression.full_display() { + format!("Replace with `{expression}`") + } else { + format!("Replace with f-string") + } } } @@ -140,7 +149,7 @@ pub(crate) fn static_join_to_fstring(checker: &mut Checker, expr: &Expr, joiner: let mut diagnostic = Diagnostic::new( StaticJoinToFString { - expr: contents.clone(), + expression: SourceCodeSnippet::new(contents.clone()), }, expr.range(), ); diff --git a/crates/ruff/src/rules/pylint/rules/bad_string_format_character.rs b/crates/ruff/src/rules/pylint/rules/bad_string_format_character.rs index d571a1635062a..fdf221d10a4d0 100644 --- a/crates/ruff/src/rules/pylint/rules/bad_string_format_character.rs +++ b/crates/ruff/src/rules/pylint/rules/bad_string_format_character.rs @@ -36,7 +36,8 @@ pub struct BadStringFormatCharacter { impl Violation for BadStringFormatCharacter { #[derive_message_formats] fn message(&self) -> String { - format!("Unsupported format character '{}'", self.format_char) + let BadStringFormatCharacter { format_char } = self; + format!("Unsupported format character '{format_char}'") } } diff --git a/crates/ruff/src/rules/pylint/rules/repeated_equality_comparison_target.rs b/crates/ruff/src/rules/pylint/rules/repeated_equality_comparison_target.rs index 8d6ca59912575..97c6e2e7ef9be 100644 --- a/crates/ruff/src/rules/pylint/rules/repeated_equality_comparison_target.rs +++ b/crates/ruff/src/rules/pylint/rules/repeated_equality_comparison_target.rs @@ -5,6 +5,7 @@ use itertools::{any, Itertools}; use ruff_python_ast::{BoolOp, CmpOp, Expr, ExprBoolOp, ExprCompare, Ranged}; use rustc_hash::FxHashMap; +use crate::autofix::snippet::SourceCodeSnippet; use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::hashable::HashableExpr; @@ -42,16 +43,22 @@ use crate::checkers::ast::Checker; /// - [Python documentation: `set`](https://docs.python.org/3/library/stdtypes.html#set) #[violation] pub struct RepeatedEqualityComparisonTarget { - expr: String, + expression: SourceCodeSnippet, } impl Violation for RepeatedEqualityComparisonTarget { #[derive_message_formats] fn message(&self) -> String { - let RepeatedEqualityComparisonTarget { expr } = self; - format!( - "Consider merging multiple comparisons: `{expr}`. Use a `set` if the elements are hashable." - ) + let RepeatedEqualityComparisonTarget { expression } = self; + if let Some(expression) = expression.full_display() { + format!( + "Consider merging multiple comparisons: `{expression}`. Use a `set` if the elements are hashable." + ) + } else { + format!( + "Consider merging multiple comparisons. Use a `set` if the elements are hashable." + ) + } } } @@ -84,12 +91,12 @@ pub(crate) fn repeated_equality_comparison_target(checker: &mut Checker, bool_op if count > 1 { checker.diagnostics.push(Diagnostic::new( RepeatedEqualityComparisonTarget { - expr: merged_membership_test( + expression: SourceCodeSnippet::new(merged_membership_test( left.as_expr(), bool_op.op, &comparators, checker.locator(), - ), + )), }, bool_op.range(), )); diff --git a/crates/ruff/src/rules/ruff/rules/collection_literal_concatenation.rs b/crates/ruff/src/rules/ruff/rules/collection_literal_concatenation.rs index 125f950e100c1..604b81a88c2d0 100644 --- a/crates/ruff/src/rules/ruff/rules/collection_literal_concatenation.rs +++ b/crates/ruff/src/rules/ruff/rules/collection_literal_concatenation.rs @@ -1,9 +1,9 @@ -use ruff_python_ast::{self as ast, Expr, ExprContext, Operator, Ranged}; -use ruff_text_size::TextRange; - use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation}; use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::{self as ast, Expr, ExprContext, Operator, Ranged}; +use ruff_text_size::TextRange; +use crate::autofix::snippet::SourceCodeSnippet; use crate::checkers::ast::Checker; use crate::registry::AsRule; @@ -39,7 +39,7 @@ use crate::registry::AsRule; /// - [Python documentation: Sequence Types — `list`, `tuple`, `range`](https://docs.python.org/3/library/stdtypes.html#sequence-types-list-tuple-range) #[violation] pub struct CollectionLiteralConcatenation { - expr: String, + expression: SourceCodeSnippet, } impl Violation for CollectionLiteralConcatenation { @@ -47,13 +47,21 @@ impl Violation for CollectionLiteralConcatenation { #[derive_message_formats] fn message(&self) -> String { - let CollectionLiteralConcatenation { expr } = self; - format!("Consider `{expr}` instead of concatenation") + let CollectionLiteralConcatenation { expression } = self; + if let Some(expression) = expression.full_display() { + format!("Consider `{expression}` instead of concatenation") + } else { + format!("Consider iterable unpacking instead of concatenation") + } } fn autofix_title(&self) -> Option { - let CollectionLiteralConcatenation { expr } = self; - Some(format!("Replace with `{expr}`")) + let CollectionLiteralConcatenation { expression } = self; + if let Some(expression) = expression.full_display() { + Some(format!("Replace with `{expression}`")) + } else { + Some(format!("Replace with iterable unpacking")) + } } } @@ -186,7 +194,7 @@ pub(crate) fn collection_literal_concatenation(checker: &mut Checker, expr: &Exp }; let mut diagnostic = Diagnostic::new( CollectionLiteralConcatenation { - expr: contents.clone(), + expression: SourceCodeSnippet::new(contents.clone()), }, expr.range(), ); diff --git a/crates/ruff/src/rules/ruff/rules/implicit_optional.rs b/crates/ruff/src/rules/ruff/rules/implicit_optional.rs index e4edf5ff0bb72..fbda0f5ede2ae 100644 --- a/crates/ruff/src/rules/ruff/rules/implicit_optional.rs +++ b/crates/ruff/src/rules/ruff/rules/implicit_optional.rs @@ -90,7 +90,8 @@ impl Violation for ImplicitOptional { } fn autofix_title(&self) -> Option { - Some(format!("Convert to `{}`", self.conversion_type)) + let ImplicitOptional { conversion_type } = self; + Some(format!("Convert to `{conversion_type}`")) } } diff --git a/crates/ruff/src/rules/ruff/rules/invalid_pyproject_toml.rs b/crates/ruff/src/rules/ruff/rules/invalid_pyproject_toml.rs index 6410cb526dc85..fc5482b9c24e2 100644 --- a/crates/ruff/src/rules/ruff/rules/invalid_pyproject_toml.rs +++ b/crates/ruff/src/rules/ruff/rules/invalid_pyproject_toml.rs @@ -40,6 +40,7 @@ impl Violation for InvalidPyprojectToml { #[derive_message_formats] fn message(&self) -> String { - format!("Failed to parse pyproject.toml: {}", self.message) + let InvalidPyprojectToml { message } = self; + format!("Failed to parse pyproject.toml: {message}") } } diff --git a/crates/ruff/src/rules/ruff/rules/unnecessary_iterable_allocation_for_first_element.rs b/crates/ruff/src/rules/ruff/rules/unnecessary_iterable_allocation_for_first_element.rs index 13c581fdc4a8b..fcb3caa2c5e0e 100644 --- a/crates/ruff/src/rules/ruff/rules/unnecessary_iterable_allocation_for_first_element.rs +++ b/crates/ruff/src/rules/ruff/rules/unnecessary_iterable_allocation_for_first_element.rs @@ -1,7 +1,6 @@ use std::borrow::Cow; use num_traits::Zero; -use unicode_width::UnicodeWidthStr; use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, violation}; @@ -9,6 +8,7 @@ use ruff_python_ast::{self as ast, Arguments, Comprehension, Constant, Expr, Ran use ruff_python_semantic::SemanticModel; use ruff_text_size::{TextRange, TextSize}; +use crate::autofix::snippet::SourceCodeSnippet; use crate::checkers::ast::Checker; use crate::registry::AsRule; @@ -47,35 +47,24 @@ use crate::registry::AsRule; /// - [Iterators and Iterables in Python: Run Efficient Iterations](https://realpython.com/python-iterators-iterables/#when-to-use-an-iterator-in-python) #[violation] pub(crate) struct UnnecessaryIterableAllocationForFirstElement { - iterable: String, + iterable: SourceCodeSnippet, } impl AlwaysAutofixableViolation for UnnecessaryIterableAllocationForFirstElement { #[derive_message_formats] fn message(&self) -> String { let UnnecessaryIterableAllocationForFirstElement { iterable } = self; - let iterable = Self::truncate(iterable); + let iterable = iterable.truncated_display(); format!("Prefer `next({iterable})` over single element slice") } fn autofix_title(&self) -> String { let UnnecessaryIterableAllocationForFirstElement { iterable } = self; - let iterable = Self::truncate(iterable); + let iterable = iterable.truncated_display(); format!("Replace with `next({iterable})`") } } -impl UnnecessaryIterableAllocationForFirstElement { - /// If the iterable is too long, or spans multiple lines, truncate it. - fn truncate(iterable: &str) -> &str { - if iterable.width() > 40 || iterable.contains(['\r', '\n']) { - "..." - } else { - iterable - } - } -} - /// RUF015 pub(crate) fn unnecessary_iterable_allocation_for_first_element( checker: &mut Checker, @@ -104,7 +93,7 @@ pub(crate) fn unnecessary_iterable_allocation_for_first_element( let mut diagnostic = Diagnostic::new( UnnecessaryIterableAllocationForFirstElement { - iterable: iterable.to_string(), + iterable: SourceCodeSnippet::new(iterable.to_string()), }, *range, ); diff --git a/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF005_RUF005.py.snap b/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF005_RUF005.py.snap index 5b874cb5b6f82..d2596ef929db6 100644 --- a/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF005_RUF005.py.snap +++ b/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF005_RUF005.py.snap @@ -187,7 +187,7 @@ RUF005.py:47:16: RUF005 [*] Consider `("we all feel", *Fun.words)` instead of co 49 49 | chain = ["a", "b", "c"] + eggs + list(("yes", "no", "pants") + zoob) 50 50 | -RUF005.py:49:9: RUF005 [*] Consider `["a", "b", "c", *eggs, *list(("yes", "no", "pants") + zoob)]` instead of concatenation +RUF005.py:49:9: RUF005 [*] Consider iterable unpacking instead of concatenation | 47 | astonishment = ("we all feel",) + Fun.words 48 | @@ -196,7 +196,7 @@ RUF005.py:49:9: RUF005 [*] Consider `["a", "b", "c", *eggs, *list(("yes", "no", 50 | 51 | baz = () + zoob | - = help: Replace with `["a", "b", "c", *eggs, *list(("yes", "no", "pants") + zoob)]` + = help: Replace with iterable unpacking ℹ Suggested fix 46 46 | excitement = ("we all think",) + Fun().yay() @@ -294,14 +294,14 @@ RUF005.py:56:15: RUF005 [*] Consider `[sys.executable, "-m", "pylint", *args, pa 58 58 | b = a + [2, 3] + [4] 59 59 | -RUF005.py:57:21: RUF005 [*] Consider `(sys.executable, "-m", "pylint", *args, path, path2)` instead of concatenation +RUF005.py:57:21: RUF005 [*] Consider iterable unpacking instead of concatenation | 56 | pylint_call = [sys.executable, "-m", "pylint"] + args + [path] 57 | pylint_call_tuple = (sys.executable, "-m", "pylint") + args + (path, path2) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF005 58 | b = a + [2, 3] + [4] | - = help: Replace with `(sys.executable, "-m", "pylint", *args, path, path2)` + = help: Replace with iterable unpacking ℹ Suggested fix 54 54 | ]