Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Truncate some messages in diagnostics #6748

Merged
merged 1 commit into from
Aug 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions crates/ruff/src/autofix/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
36 changes: 36 additions & 0 deletions crates/ruff/src/autofix/snippet.rs
Original file line number Diff line number Diff line change
@@ -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 changes: 16 additions & 8 deletions crates/ruff/src/rules/flake8_simplify/rules/yoda_conditions.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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 {
Expand All @@ -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")
Expand All @@ -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")
}
})
}
}

Expand Down Expand Up @@ -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(),
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
25 changes: 17 additions & 8 deletions crates/ruff/src/rules/flynt/rules/static_join_to_fstring.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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")
}
}
}

Expand Down Expand Up @@ -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(),
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}'")
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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."
)
}
}
}

Expand Down Expand Up @@ -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(),
));
Expand Down
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -39,21 +39,29 @@ 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 {
const AUTOFIX: AutofixKind = AutofixKind::Sometimes;

#[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"))
}
}
}

Expand Down Expand Up @@ -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(),
);
Expand Down
3 changes: 2 additions & 1 deletion crates/ruff/src/rules/ruff/rules/implicit_optional.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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}`"))
}
}

Expand Down
3 changes: 2 additions & 1 deletion crates/ruff/src/rules/ruff/rules/invalid_pyproject_toml.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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}")
}
}
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
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};
use ruff_python_ast::{self as ast, Arguments, Comprehension, Constant, Expr, Ranged};
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;

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
);
Expand Down
Loading