Python
/
ruff
mirror of https://github.com/astral-sh/ruff
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Truncate some messages in diagnostics (#6748) 

## Summary

I noticed this in the ecosystem CI check from
https://github.com/astral-sh/ruff/pull/6742. If we include source code
directly in a diagnostic, we need to be careful to avoid rendering
multi-line diagnostics or even excessively long diagnostics.

## Test Plan

`cargo test`
This commit is contained in:
Charlie Marsh 2023-08-21 23:46:24 -04:00 committed by GitHub
parent 0aad0c41f6
commit fa32cd9b6f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 118 additions and 57 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
crates/ruff/src/autofix/mod.rs
View File

@ -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 {

36
crates/ruff/src/autofix/snippet.rs Normal file
View File

@ -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'])
}
}

24
crates/ruff/src/rules/flake8_simplify/rules/yoda_conditions.rs
View File

@ -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<String>,
suggestion: Option<SourceCodeSnippet>,
}
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<String> {
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(),
);

4
crates/ruff/src/rules/flake8_simplify/snapshots/ruff__rules__flake8_simplify__tests__SIM300_SIM300.py.snap
View File

@ -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

25
crates/ruff/src/rules/flynt/rules/static_join_to_fstring.rs
View File

@ -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(),
);

3
crates/ruff/src/rules/pylint/rules/bad_string_format_character.rs
View File

@ -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}'")
}
}

21
crates/ruff/src/rules/pylint/rules/repeated_equality_comparison_target.rs
View File

@ -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(),
));

26
crates/ruff/src/rules/ruff/rules/collection_literal_concatenation.rs
View File

@ -1,9 +1,9 @@
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 ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation};
use ruff_macros::{derive_message_formats, violation};
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<String> {
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(),
);

3
crates/ruff/src/rules/ruff/rules/implicit_optional.rs
View File

@ -90,7 +90,8 @@ impl Violation for ImplicitOptional {
}
fn autofix_title(&self) -> Option<String> {
Some(format!("Convert to `{}`", self.conversion_type))
let ImplicitOptional { conversion_type } = self;
Some(format!("Convert to `{conversion_type}`"))
}
}

3
crates/ruff/src/rules/ruff/rules/invalid_pyproject_toml.rs
View File

@ -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}")
}
}

21
crates/ruff/src/rules/ruff/rules/unnecessary_iterable_allocation_for_first_element.rs
View File

@ -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,
);

8
crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF005_RUF005.py.snap
View File

@ -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 | ]