From 9939ea2102d6a657dc89dcf029acd892c9753156 Mon Sep 17 00:00:00 2001 From: Denny Wong Date: Sat, 22 Jun 2024 02:43:24 +0100 Subject: [PATCH 1/4] [`ruff`] Add `assert-with-print-expression` flag (#11974) --- .../resources/test/fixtures/ruff/RUF030.py | 108 +++++ .../src/checkers/ast/analyze/statement.rs | 3 + crates/ruff_linter/src/codes.rs | 1 + crates/ruff_linter/src/rules/ruff/mod.rs | 1 + .../rules/assert_with_print_expression.rs | 300 +++++++++++++ .../ruff_linter/src/rules/ruff/rules/mod.rs | 2 + ..._rules__ruff__tests__RUF030_RUF030.py.snap | 396 ++++++++++++++++++ ruff.schema.json | 2 + 8 files changed, 813 insertions(+) create mode 100644 crates/ruff_linter/resources/test/fixtures/ruff/RUF030.py create mode 100644 crates/ruff_linter/src/rules/ruff/rules/assert_with_print_expression.rs create mode 100644 crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF030_RUF030.py.snap diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF030.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF030.py new file mode 100644 index 0000000000000..0f4e9a7610b81 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF030.py @@ -0,0 +1,108 @@ +U00A0 = "\u00a0" + +# Standard Case +# Expects: +# - single StringLiteral +assert True, print("This print is not intentional.") + +# Concatenated string literals +# Expects: +# - single StringLiteral +assert True, print("This print" " is not intentional.") + +# Positional arguments, string literals +# Expects: +# - single StringLiteral concatenated with " " +assert True, print("This print", "is not intentional") + +# Concatenated string literals combined with Positional arguments +# Expects: +# - single stringliteral concatenated with " " only between `print` and `is` +assert True, print("This " "print", "is not intentional.") + +# Positional arguments, string literals with a variable +# Expects: +# - single FString concatenated with " " +assert True, print("This", print.__name__, "is not intentional.") + +# Mixed brackets string literals +# Expects: +# - single StringLiteral concatenated with " " +assert True, print("This print", 'is not intentional', """and should be removed""") + +# Mixed brackets with other brackets inside +# Expects: +# - single StringLiteral concatenated with " " and escaped brackets +assert True, print("This print", 'is not "intentional"', """and "should" be 'removed'""") + +# Positional arguments, string literals with a separator +# Expects: +# - single StringLiteral concatenated with "|" +assert True, print("This print", "is not intentional", sep="|") + +# Positional arguments, string literals with None as separator +# Expects: +# - single StringLiteral concatenated with " " +assert True, print("This print", "is not intentional", sep=None) + +# Positional arguments, string literals with variable as separator, needs f-string +# Expects: +# - single FString concatenated with "{U00A0}" +assert True, print("This print", "is not intentional", sep=U00A0) + +# Unnecessary f-string +# Expects: +# - single StringLiteral +assert True, print(f"This f-string is just a literal.") + +# Positional arguments, string literals and f-strings +# Expects: +# - single FString concatenated with " " +assert True, print("This print", f"is not {'intentional':s}") + +# Positional arguments, string literals and f-strings with a separator +# Expects: +# - single FString concatenated with "|" +assert True, print("This print", f"is not {'intentional':s}", sep="|") + +# A single f-string +# Expects: +# - single FString +assert True, print(f"This print is not {'intentional':s}") + +# A single f-string with a redundant separator +# Expects: +# - single FString +assert True, print(f"This print is not {'intentional':s}", sep="|") + +# Complex f-string with variable as separator +# Expects: +# - single FString concatenated with "{U00A0}", all placeholders preserved +condition = "True is True" +maintainer = "John Doe" +assert True, print("Unreachable due to", condition, f", ask {maintainer} for advice", sep=U00A0) + +# Empty print +# Expects: +# - `msg` entirely removed from assertion +assert True, print() + +# Empty print with separator +# Expects: +# - `msg` entirely removed from assertion +assert True, print(sep=" ") + +# Custom print function that actually returns a string +# Expects: +# - no violation as the function is not a built-in print +def print(s: str): + return "This is my assertion error message: " + s + +assert True, print("this print shall not be removed.") + +import builtins + +# Use of `builtins.print` +# Expects: +# - single StringLiteral +assert True, builtins.print("This print should be removed.") diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs index 552fb66844b6b..d659539991419 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs @@ -1267,6 +1267,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { if checker.enabled(Rule::InvalidMockAccess) { pygrep_hooks::rules::non_existent_mock_method(checker, test); } + if checker.enabled(Rule::AssertWithPrintExpression) { + ruff::rules::assert_with_print_expression(checker, stmt, test, msg.as_deref()); + } } Stmt::With(with_stmt @ ast::StmtWith { items, body, .. }) => { if checker.enabled(Rule::TooManyNestedBlocks) { diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 89a0c7e5c7e89..5623d924ac7c3 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -977,6 +977,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Ruff, "027") => (RuleGroup::Preview, rules::ruff::rules::MissingFStringSyntax), (Ruff, "028") => (RuleGroup::Preview, rules::ruff::rules::InvalidFormatterSuppressionComment), (Ruff, "029") => (RuleGroup::Preview, rules::ruff::rules::UnusedAsync), + (Ruff, "030") => (RuleGroup::Preview, rules::ruff::rules::AssertWithPrintExpression), (Ruff, "100") => (RuleGroup::Stable, rules::ruff::rules::UnusedNOQA), (Ruff, "101") => (RuleGroup::Preview, rules::ruff::rules::RedirectedNOQA), (Ruff, "200") => (RuleGroup::Stable, rules::ruff::rules::InvalidPyprojectToml), diff --git a/crates/ruff_linter/src/rules/ruff/mod.rs b/crates/ruff_linter/src/rules/ruff/mod.rs index 7bce548d85dd1..b63cac8d5a8da 100644 --- a/crates/ruff_linter/src/rules/ruff/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/mod.rs @@ -54,6 +54,7 @@ mod tests { #[test_case(Rule::MissingFStringSyntax, Path::new("RUF027_2.py"))] #[test_case(Rule::InvalidFormatterSuppressionComment, Path::new("RUF028.py"))] #[test_case(Rule::UnusedAsync, Path::new("RUF029.py"))] + #[test_case(Rule::AssertWithPrintExpression, Path::new("RUF030.py"))] #[test_case(Rule::RedirectedNOQA, Path::new("RUF101.py"))] fn rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy()); diff --git a/crates/ruff_linter/src/rules/ruff/rules/assert_with_print_expression.rs b/crates/ruff_linter/src/rules/ruff/rules/assert_with_print_expression.rs new file mode 100644 index 0000000000000..b3c64bd9e53be --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/rules/assert_with_print_expression.rs @@ -0,0 +1,300 @@ +use ruff_python_ast::{Expr, Stmt, StmtAssert}; +use ruff_text_size::Ranged; + +use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; +use ruff_macros::{derive_message_formats, violation}; + +use crate::checkers::ast::Checker; + +/// ## What it does +/// Checks for uses of `assert expression, print(message)`. +/// +/// ## Why is this bad? +/// The return value of the second expression is used as the contents of the +/// `AssertionError` raised by the `assert` statement. Using a `print` expression +/// in this context will output the message to `stdout`, before raising an +/// empty `AssertionError(None)`. +/// +/// Instead, remove the `print` and pass the message directly as the second +/// expression, allowing `stderr` to capture the message in a well formatted context. +/// +/// ## Example +/// ```python +/// assert False, print("This is a message") +/// ``` +/// +/// Use instead: +/// ```python +/// assert False, "This is a message" +/// ``` +/// +/// ## Fix safety +/// This rule's fix is marked as unsafe, as changing the second expression +/// will result in a different `AssertionError` message being raised, as well as +/// a change in `stdout` output. +/// +/// ## References +/// - [Python documentation: `assert`](https://docs.python.org/3/reference/simple_stmts.html#the-assert-statement) +#[violation] +pub struct AssertWithPrintExpression; + +impl AlwaysFixableViolation for AssertWithPrintExpression { + #[derive_message_formats] + fn message(&self) -> String { + format!( + "`print()` expressions in `assert` statements is likely unintentional, \ + remove the `print()` call and pass the message" + ) + } + + fn fix_title(&self) -> String { + "Remove `print`, and pass the message directly to `assert`".to_owned() + } +} + +/// Extracts the arguments from a `print` call and converts them to some kind of string +/// expression. +/// +/// 3 cases are handled: +/// - if there are no arguments, return `None` so that `diagnostic` can remove `msg` from `assert`; +/// - if all of `print` arguments including `sep` are string literals, return a `Expr::StringLiteral`; +/// - otherwise, return a `Expr::FString`. +mod print_arguments { + use itertools::Itertools; + use ruff_python_ast::{ + Arguments, ConversionFlag, Expr, ExprFString, ExprStringLiteral, FString, FStringElement, + FStringElements, FStringExpressionElement, FStringFlags, FStringLiteralElement, + FStringValue, StringLiteral, StringLiteralFlags, StringLiteralValue, + }; + use ruff_text_size::TextRange; + + /// Converts an expression to a list of `FStringElement`s. + /// + /// 3 cases are handled: + /// - if the expression is a string literal, each part of the string will be converted to a + /// `FStringLiteralElement`. + /// - if the expression is an f-string, the elements will be returned as-is. + /// - otherwise, the expression will be wrapped in a `FStringExpressionElement`. + fn expr_to_fstring_elements(expr: &Expr) -> Vec { + match expr { + // If the expression is a string literal, convert each part to a FStringLiteralElement + Expr::StringLiteral(string) => string + .value + .iter() + .map(|part| { + FStringElement::Literal(FStringLiteralElement { + range: TextRange::default(), + value: part.value.clone(), + }) + }) + .collect(), + // If the expression is an f-string, return the elements + Expr::FString(fstring) => fstring.value.elements().cloned().collect(), + // Otherwise, return the expression as a single FStringExpressionElement wrapping + // the expression + expr => vec![FStringElement::Expression(FStringExpressionElement { + range: TextRange::default(), + expression: Box::new(expr.clone()), + debug_text: None, + conversion: ConversionFlag::None, + format_spec: None, + })], + } + } + + /// Converts a list of `FStringElement`s to a list of `StringLiteral`s. + /// + /// If any of the elements are not string literals, `None` is returned. + /// + /// This is useful - in combination with [`expr_to_fstring_elements`] for + /// checking if the `sep` and `args` arguments to `print` are all string + /// literals. + fn fstring_elements_to_string_literals<'a>( + mut elements: impl ExactSizeIterator, + ) -> Option> { + elements.try_fold(Vec::with_capacity(elements.len()), |mut acc, element| { + if let FStringElement::Literal(literal) = element { + acc.push(StringLiteral { + range: literal.range, + value: literal.value.clone(), + flags: StringLiteralFlags::default(), + }); + Some(acc) + } else { + None + } + }) + } + + /// Converts the `sep` and `args` arguments to a [`Expr::StringLiteral`]. + /// + /// This function will return [`None`] if any of the arguments are not string literals, + /// or if there are no arguments at all. + fn args_to_string_literal_expr<'a>( + args: impl ExactSizeIterator>, + sep: impl ExactSizeIterator, + ) -> Option { + // If there are no arguments, short-circuit and return `None` + if args.len() == 0 { + return None; + } + + // Attempt to convert the `sep` and `args` arguments to string literals. + // We need to maintain `args` as a Vec of Vecs, as the first Vec represents + // the arguments to the `print` call, and the inner Vecs represent the elements + // of a concatenated string literal. (e.g. "text", "text" "text") The `sep` will + // be inserted only between the outer Vecs. + let (Some(sep), Some(args)) = ( + fstring_elements_to_string_literals(sep), + args.map(|arg| fstring_elements_to_string_literals(arg.iter())) + .collect::>>(), + ) else { + // If any of the arguments are not string literals, return None + return None; + }; + + // Put the `sep` into a single Rust `String` + let sep_string = sep + .into_iter() + .map(|string_literal| string_literal.value) + .join(""); + + // Join the `args` with the `sep` + let combined_string = args + .into_iter() + .map(|string_literals| { + string_literals + .into_iter() + .map(|string_literal| string_literal.value) + .join("") + }) + .join(&sep_string); + + Some(Expr::StringLiteral(ExprStringLiteral { + range: TextRange::default(), + value: StringLiteralValue::single(StringLiteral { + range: TextRange::default(), + value: combined_string.into(), + flags: StringLiteralFlags::default(), + }), + })) + } + + /// Converts the `sep` and `args` arguments to a [`Expr::FString`]. + /// + /// This function will only return [`None`] if there are no arguments at all. + /// + /// ## Note + /// This function will always return an f-string, even if all of the arguments + /// are string literals. This can produce unnecessary f-strings. + /// + /// Also note that the iterator arguments of this function are consumed, + /// as opposed to the references taken by [`args_to_string_literal_expr`]. + fn args_to_fstring_expr( + mut args: impl ExactSizeIterator>, + sep: impl ExactSizeIterator, + ) -> Option { + // If there are no arguments, short-circuit and return `None` + let first_arg = args.next()?; + let sep = sep.collect::>(); + + let fstring_elements = args.fold(first_arg, |mut elements, arg| { + elements.extend(sep.clone()); + elements.extend(arg); + elements + }); + + Some(Expr::FString(ExprFString { + range: TextRange::default(), + value: FStringValue::single(FString { + range: TextRange::default(), + elements: FStringElements::from(fstring_elements), + flags: FStringFlags::default(), + }), + })) + } + + /// Attempts to convert the `print` arguments to a suitable string expression. + /// + /// If the `sep` argument is provided, it will be used as the separator between + /// arguments. Otherwise, a space will be used. + /// + /// `end` and `file` keyword arguments are ignored, as they don't affect the + /// output of the `print` statement. + /// + /// ## Returns + /// + /// - [`Some`]<[`Expr::StringLiteral`]> if all of the arguments including `sep` are + /// string literals. + /// - [`Some`]<[`Expr::FString`]> if any of the arguments are not string literals. + /// - [`None`] if the `print` contains no positional arguments at all. + pub(super) fn to_expr(arguments: &Arguments) -> Option { + // Convert the `sep` argument into `FStringElement`s + let sep = arguments + .find_keyword("sep") + .and_then( + // If the `sep` argument is `None`, treat this as default behavior + |keyword| { + if let Expr::NoneLiteral(_) = keyword.value { + None + } else { + Some(&keyword.value) + } + }, + ) + .map(expr_to_fstring_elements) + .unwrap_or_else(|| { + vec![FStringElement::Literal(FStringLiteralElement { + range: TextRange::default(), + value: " ".into(), + })] + }); + + let args = arguments + .args + .iter() + .map(expr_to_fstring_elements) + .collect::>(); + + // Attempt to convert the `sep` and `args` arguments to a string literal, + // falling back to an f-string if the arguments are not all string literals. + args_to_string_literal_expr(args.iter(), sep.iter()) + .or_else(|| args_to_fstring_expr(args.into_iter(), sep.into_iter())) + } +} + +/// Checks if the `msg` argument to an `assert` statement is a `print` call, and if so, +/// replace the message with the arguments to the `print` call. +pub(crate) fn assert_with_print_expression( + checker: &mut Checker, + stmt: &Stmt, + test: &Expr, + msg: Option<&Expr>, +) { + if let Some(Expr::Call(call)) = msg { + // We have to check that the print call is a call to the built-in `print` function + let semantic = checker.semantic(); + + let Some(qualified_name) = semantic.resolve_qualified_name(&call.func) else { + return; + }; + + if matches!(qualified_name.segments(), ["" | "builtins", "print"]) { + // This is the confirmed rule condition + let mut diagnostic = Diagnostic::new(AssertWithPrintExpression, call.range()); + diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement( + checker.generator().stmt(&Stmt::Assert(StmtAssert { + range: stmt.range(), + test: test.clone().into(), + // Into::into is used for boxing the expression + msg: print_arguments::to_expr(&call.arguments).map(std::convert::Into::into), + })), + // We have to replace the entire statement, + // as the `print` could be empty and thus `call.range()` + // will cease to exist + stmt.range(), + ))); + checker.diagnostics.push(diagnostic); + } + } +} diff --git a/crates/ruff_linter/src/rules/ruff/rules/mod.rs b/crates/ruff_linter/src/rules/ruff/rules/mod.rs index d93f4a781c477..b947b34880261 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/mod.rs @@ -1,4 +1,5 @@ pub(crate) use ambiguous_unicode_character::*; +pub(crate) use assert_with_print_expression::*; pub(crate) use assignment_in_assert::*; pub(crate) use asyncio_dangling_task::*; pub(crate) use collection_literal_concatenation::*; @@ -30,6 +31,7 @@ pub(crate) use unused_async::*; pub(crate) use unused_noqa::*; mod ambiguous_unicode_character; +mod assert_with_print_expression; mod assignment_in_assert; mod asyncio_dangling_task; mod collection_literal_concatenation; diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF030_RUF030.py.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF030_RUF030.py.snap new file mode 100644 index 0000000000000..4f04f6263dae0 --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF030_RUF030.py.snap @@ -0,0 +1,396 @@ +--- +source: crates/ruff_linter/src/rules/ruff/mod.rs +--- +RUF030.py:6:14: RUF030 [*] `print()` expressions in `assert` statements is likely unintentional, remove the `print()` call and pass the message + | +4 | # Expects: +5 | # - single StringLiteral +6 | assert True, print("This print is not intentional.") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF030 +7 | +8 | # Concatenated string literals + | + = help: Remove `print`, and pass the message directly to `assert` + +ℹ Unsafe fix +3 3 | # Standard Case +4 4 | # Expects: +5 5 | # - single StringLiteral +6 |-assert True, print("This print is not intentional.") + 6 |+assert True, "This print is not intentional." +7 7 | +8 8 | # Concatenated string literals +9 9 | # Expects: + +RUF030.py:11:14: RUF030 [*] `print()` expressions in `assert` statements is likely unintentional, remove the `print()` call and pass the message + | + 9 | # Expects: +10 | # - single StringLiteral +11 | assert True, print("This print" " is not intentional.") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF030 +12 | +13 | # Positional arguments, string literals + | + = help: Remove `print`, and pass the message directly to `assert` + +ℹ Unsafe fix +8 8 | # Concatenated string literals +9 9 | # Expects: +10 10 | # - single StringLiteral +11 |-assert True, print("This print" " is not intentional.") + 11 |+assert True, "This print is not intentional." +12 12 | +13 13 | # Positional arguments, string literals +14 14 | # Expects: + +RUF030.py:16:14: RUF030 [*] `print()` expressions in `assert` statements is likely unintentional, remove the `print()` call and pass the message + | +14 | # Expects: +15 | # - single StringLiteral concatenated with " " +16 | assert True, print("This print", "is not intentional") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF030 +17 | +18 | # Concatenated string literals combined with Positional arguments + | + = help: Remove `print`, and pass the message directly to `assert` + +ℹ Unsafe fix +13 13 | # Positional arguments, string literals +14 14 | # Expects: +15 15 | # - single StringLiteral concatenated with " " +16 |-assert True, print("This print", "is not intentional") + 16 |+assert True, "This print is not intentional" +17 17 | +18 18 | # Concatenated string literals combined with Positional arguments +19 19 | # Expects: + +RUF030.py:21:14: RUF030 [*] `print()` expressions in `assert` statements is likely unintentional, remove the `print()` call and pass the message + | +19 | # Expects: +20 | # - single stringliteral concatenated with " " only between `print` and `is` +21 | assert True, print("This " "print", "is not intentional.") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF030 +22 | +23 | # Positional arguments, string literals with a variable + | + = help: Remove `print`, and pass the message directly to `assert` + +ℹ Unsafe fix +18 18 | # Concatenated string literals combined with Positional arguments +19 19 | # Expects: +20 20 | # - single stringliteral concatenated with " " only between `print` and `is` +21 |-assert True, print("This " "print", "is not intentional.") + 21 |+assert True, "This print is not intentional." +22 22 | +23 23 | # Positional arguments, string literals with a variable +24 24 | # Expects: + +RUF030.py:26:14: RUF030 [*] `print()` expressions in `assert` statements is likely unintentional, remove the `print()` call and pass the message + | +24 | # Expects: +25 | # - single FString concatenated with " " +26 | assert True, print("This", print.__name__, "is not intentional.") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF030 +27 | +28 | # Mixed brackets string literals + | + = help: Remove `print`, and pass the message directly to `assert` + +ℹ Unsafe fix +23 23 | # Positional arguments, string literals with a variable +24 24 | # Expects: +25 25 | # - single FString concatenated with " " +26 |-assert True, print("This", print.__name__, "is not intentional.") + 26 |+assert True, f"This {print.__name__} is not intentional." +27 27 | +28 28 | # Mixed brackets string literals +29 29 | # Expects: + +RUF030.py:31:14: RUF030 [*] `print()` expressions in `assert` statements is likely unintentional, remove the `print()` call and pass the message + | +29 | # Expects: +30 | # - single StringLiteral concatenated with " " +31 | assert True, print("This print", 'is not intentional', """and should be removed""") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF030 +32 | +33 | # Mixed brackets with other brackets inside + | + = help: Remove `print`, and pass the message directly to `assert` + +ℹ Unsafe fix +28 28 | # Mixed brackets string literals +29 29 | # Expects: +30 30 | # - single StringLiteral concatenated with " " +31 |-assert True, print("This print", 'is not intentional', """and should be removed""") + 31 |+assert True, "This print is not intentional and should be removed" +32 32 | +33 33 | # Mixed brackets with other brackets inside +34 34 | # Expects: + +RUF030.py:36:14: RUF030 [*] `print()` expressions in `assert` statements is likely unintentional, remove the `print()` call and pass the message + | +34 | # Expects: +35 | # - single StringLiteral concatenated with " " and escaped brackets +36 | assert True, print("This print", 'is not "intentional"', """and "should" be 'removed'""") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF030 +37 | +38 | # Positional arguments, string literals with a separator + | + = help: Remove `print`, and pass the message directly to `assert` + +ℹ Unsafe fix +33 33 | # Mixed brackets with other brackets inside +34 34 | # Expects: +35 35 | # - single StringLiteral concatenated with " " and escaped brackets +36 |-assert True, print("This print", 'is not "intentional"', """and "should" be 'removed'""") + 36 |+assert True, "This print is not \"intentional\" and \"should\" be 'removed'" +37 37 | +38 38 | # Positional arguments, string literals with a separator +39 39 | # Expects: + +RUF030.py:41:14: RUF030 [*] `print()` expressions in `assert` statements is likely unintentional, remove the `print()` call and pass the message + | +39 | # Expects: +40 | # - single StringLiteral concatenated with "|" +41 | assert True, print("This print", "is not intentional", sep="|") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF030 +42 | +43 | # Positional arguments, string literals with None as separator + | + = help: Remove `print`, and pass the message directly to `assert` + +ℹ Unsafe fix +38 38 | # Positional arguments, string literals with a separator +39 39 | # Expects: +40 40 | # - single StringLiteral concatenated with "|" +41 |-assert True, print("This print", "is not intentional", sep="|") + 41 |+assert True, "This print|is not intentional" +42 42 | +43 43 | # Positional arguments, string literals with None as separator +44 44 | # Expects: + +RUF030.py:46:14: RUF030 [*] `print()` expressions in `assert` statements is likely unintentional, remove the `print()` call and pass the message + | +44 | # Expects: +45 | # - single StringLiteral concatenated with " " +46 | assert True, print("This print", "is not intentional", sep=None) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF030 +47 | +48 | # Positional arguments, string literals with variable as separator, needs f-string + | + = help: Remove `print`, and pass the message directly to `assert` + +ℹ Unsafe fix +43 43 | # Positional arguments, string literals with None as separator +44 44 | # Expects: +45 45 | # - single StringLiteral concatenated with " " +46 |-assert True, print("This print", "is not intentional", sep=None) + 46 |+assert True, "This print is not intentional" +47 47 | +48 48 | # Positional arguments, string literals with variable as separator, needs f-string +49 49 | # Expects: + +RUF030.py:51:14: RUF030 [*] `print()` expressions in `assert` statements is likely unintentional, remove the `print()` call and pass the message + | +49 | # Expects: +50 | # - single FString concatenated with "{U00A0}" +51 | assert True, print("This print", "is not intentional", sep=U00A0) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF030 +52 | +53 | # Unnecessary f-string + | + = help: Remove `print`, and pass the message directly to `assert` + +ℹ Unsafe fix +48 48 | # Positional arguments, string literals with variable as separator, needs f-string +49 49 | # Expects: +50 50 | # - single FString concatenated with "{U00A0}" +51 |-assert True, print("This print", "is not intentional", sep=U00A0) + 51 |+assert True, f"This print{U00A0}is not intentional" +52 52 | +53 53 | # Unnecessary f-string +54 54 | # Expects: + +RUF030.py:56:14: RUF030 [*] `print()` expressions in `assert` statements is likely unintentional, remove the `print()` call and pass the message + | +54 | # Expects: +55 | # - single StringLiteral +56 | assert True, print(f"This f-string is just a literal.") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF030 +57 | +58 | # Positional arguments, string literals and f-strings + | + = help: Remove `print`, and pass the message directly to `assert` + +ℹ Unsafe fix +53 53 | # Unnecessary f-string +54 54 | # Expects: +55 55 | # - single StringLiteral +56 |-assert True, print(f"This f-string is just a literal.") + 56 |+assert True, "This f-string is just a literal." +57 57 | +58 58 | # Positional arguments, string literals and f-strings +59 59 | # Expects: + +RUF030.py:61:14: RUF030 [*] `print()` expressions in `assert` statements is likely unintentional, remove the `print()` call and pass the message + | +59 | # Expects: +60 | # - single FString concatenated with " " +61 | assert True, print("This print", f"is not {'intentional':s}") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF030 +62 | +63 | # Positional arguments, string literals and f-strings with a separator + | + = help: Remove `print`, and pass the message directly to `assert` + +ℹ Unsafe fix +58 58 | # Positional arguments, string literals and f-strings +59 59 | # Expects: +60 60 | # - single FString concatenated with " " +61 |-assert True, print("This print", f"is not {'intentional':s}") + 61 |+assert True, f"This print is not {'intentional':s}" +62 62 | +63 63 | # Positional arguments, string literals and f-strings with a separator +64 64 | # Expects: + +RUF030.py:66:14: RUF030 [*] `print()` expressions in `assert` statements is likely unintentional, remove the `print()` call and pass the message + | +64 | # Expects: +65 | # - single FString concatenated with "|" +66 | assert True, print("This print", f"is not {'intentional':s}", sep="|") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF030 +67 | +68 | # A single f-string + | + = help: Remove `print`, and pass the message directly to `assert` + +ℹ Unsafe fix +63 63 | # Positional arguments, string literals and f-strings with a separator +64 64 | # Expects: +65 65 | # - single FString concatenated with "|" +66 |-assert True, print("This print", f"is not {'intentional':s}", sep="|") + 66 |+assert True, f"This print|is not {'intentional':s}" +67 67 | +68 68 | # A single f-string +69 69 | # Expects: + +RUF030.py:71:14: RUF030 [*] `print()` expressions in `assert` statements is likely unintentional, remove the `print()` call and pass the message + | +69 | # Expects: +70 | # - single FString +71 | assert True, print(f"This print is not {'intentional':s}") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF030 +72 | +73 | # A single f-string with a redundant separator + | + = help: Remove `print`, and pass the message directly to `assert` + +ℹ Unsafe fix +68 68 | # A single f-string +69 69 | # Expects: +70 70 | # - single FString +71 |-assert True, print(f"This print is not {'intentional':s}") + 71 |+assert True, f"This print is not {'intentional':s}" +72 72 | +73 73 | # A single f-string with a redundant separator +74 74 | # Expects: + +RUF030.py:76:14: RUF030 [*] `print()` expressions in `assert` statements is likely unintentional, remove the `print()` call and pass the message + | +74 | # Expects: +75 | # - single FString +76 | assert True, print(f"This print is not {'intentional':s}", sep="|") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF030 +77 | +78 | # Complex f-string with variable as separator + | + = help: Remove `print`, and pass the message directly to `assert` + +ℹ Unsafe fix +73 73 | # A single f-string with a redundant separator +74 74 | # Expects: +75 75 | # - single FString +76 |-assert True, print(f"This print is not {'intentional':s}", sep="|") + 76 |+assert True, f"This print is not {'intentional':s}" +77 77 | +78 78 | # Complex f-string with variable as separator +79 79 | # Expects: + +RUF030.py:83:14: RUF030 [*] `print()` expressions in `assert` statements is likely unintentional, remove the `print()` call and pass the message + | +81 | condition = "True is True" +82 | maintainer = "John Doe" +83 | assert True, print("Unreachable due to", condition, f", ask {maintainer} for advice", sep=U00A0) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF030 +84 | +85 | # Empty print + | + = help: Remove `print`, and pass the message directly to `assert` + +ℹ Unsafe fix +80 80 | # - single FString concatenated with "{U00A0}", all placeholders preserved +81 81 | condition = "True is True" +82 82 | maintainer = "John Doe" +83 |-assert True, print("Unreachable due to", condition, f", ask {maintainer} for advice", sep=U00A0) + 83 |+assert True, f"Unreachable due to{U00A0}{condition}{U00A0}, ask {maintainer} for advice" +84 84 | +85 85 | # Empty print +86 86 | # Expects: + +RUF030.py:88:14: RUF030 [*] `print()` expressions in `assert` statements is likely unintentional, remove the `print()` call and pass the message + | +86 | # Expects: +87 | # - `msg` entirely removed from assertion +88 | assert True, print() + | ^^^^^^^ RUF030 +89 | +90 | # Empty print with separator + | + = help: Remove `print`, and pass the message directly to `assert` + +ℹ Unsafe fix +85 85 | # Empty print +86 86 | # Expects: +87 87 | # - `msg` entirely removed from assertion +88 |-assert True, print() + 88 |+assert True +89 89 | +90 90 | # Empty print with separator +91 91 | # Expects: + +RUF030.py:93:14: RUF030 [*] `print()` expressions in `assert` statements is likely unintentional, remove the `print()` call and pass the message + | +91 | # Expects: +92 | # - `msg` entirely removed from assertion +93 | assert True, print(sep=" ") + | ^^^^^^^^^^^^^^ RUF030 +94 | +95 | # Custom print function that actually returns a string + | + = help: Remove `print`, and pass the message directly to `assert` + +ℹ Unsafe fix +90 90 | # Empty print with separator +91 91 | # Expects: +92 92 | # - `msg` entirely removed from assertion +93 |-assert True, print(sep=" ") + 93 |+assert True +94 94 | +95 95 | # Custom print function that actually returns a string +96 96 | # Expects: + +RUF030.py:108:14: RUF030 [*] `print()` expressions in `assert` statements is likely unintentional, remove the `print()` call and pass the message + | +106 | # Expects: +107 | # - single StringLiteral +108 | assert True, builtins.print("This print should be removed.") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF030 + | + = help: Remove `print`, and pass the message directly to `assert` + +ℹ Unsafe fix +105 105 | # Use of `builtins.print` +106 106 | # Expects: +107 107 | # - single StringLiteral +108 |-assert True, builtins.print("This print should be removed.") + 108 |+assert True, "This print should be removed." diff --git a/ruff.schema.json b/ruff.schema.json index 349889ac705cd..15779c1222558 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3641,6 +3641,8 @@ "RUF027", "RUF028", "RUF029", + "RUF03", + "RUF030", "RUF1", "RUF10", "RUF100", From 302f1722286168b3ad6a82d514a8d83fa0618c45 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sun, 23 Jun 2024 12:44:07 -0400 Subject: [PATCH 2/4] Tweak diagnostics; pass StmtAssert --- .../src/checkers/ast/analyze/statement.rs | 14 +-- .../rules/assert_with_print_expression.rs | 98 +++++++++---------- ..._rules__ruff__tests__RUF030_RUF030.py.snap | 76 +++++++------- 3 files changed, 90 insertions(+), 98 deletions(-) diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs index d659539991419..59562322b98e0 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs @@ -1232,11 +1232,13 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { } } } - Stmt::Assert(ast::StmtAssert { - test, - msg, - range: _, - }) => { + Stmt::Assert( + assert_stmt @ ast::StmtAssert { + test, + msg, + range: _, + }, + ) => { if !checker.semantic.in_type_checking_block() { if checker.enabled(Rule::Assert) { checker @@ -1268,7 +1270,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { pygrep_hooks::rules::non_existent_mock_method(checker, test); } if checker.enabled(Rule::AssertWithPrintExpression) { - ruff::rules::assert_with_print_expression(checker, stmt, test, msg.as_deref()); + ruff::rules::assert_with_print_expression(checker, assert_stmt); } } Stmt::With(with_stmt @ ast::StmtWith { items, body, .. }) => { diff --git a/crates/ruff_linter/src/rules/ruff/rules/assert_with_print_expression.rs b/crates/ruff_linter/src/rules/ruff/rules/assert_with_print_expression.rs index b3c64bd9e53be..d3af00f156e32 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/assert_with_print_expression.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/assert_with_print_expression.rs @@ -1,4 +1,4 @@ -use ruff_python_ast::{Expr, Stmt, StmtAssert}; +use ruff_python_ast::{self as ast, Expr, Stmt}; use ruff_text_size::Ranged; use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; @@ -41,21 +41,46 @@ pub struct AssertWithPrintExpression; impl AlwaysFixableViolation for AssertWithPrintExpression { #[derive_message_formats] fn message(&self) -> String { - format!( - "`print()` expressions in `assert` statements is likely unintentional, \ - remove the `print()` call and pass the message" - ) + format!("`print()` expression in `assert` statement is likely unintentional") } fn fix_title(&self) -> String { - "Remove `print`, and pass the message directly to `assert`".to_owned() + "Remove `print`".to_owned() + } +} + +/// RUF030 +/// +/// Checks if the `msg` argument to an `assert` statement is a `print` call, and if so, +/// replace the message with the arguments to the `print` call. +pub(crate) fn assert_with_print_expression(checker: &mut Checker, stmt: &ast::StmtAssert) { + if let Some(Expr::Call(call)) = stmt.msg.as_deref() { + // We have to check that the print call is a call to the built-in `print` function + let semantic = checker.semantic(); + + if semantic.match_builtin_expr(&call.func, "print") { + // This is the confirmed rule condition + let mut diagnostic = Diagnostic::new(AssertWithPrintExpression, call.range()); + diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement( + checker.generator().stmt(&Stmt::Assert(ast::StmtAssert { + range: stmt.range(), + test: stmt.test.clone().into(), + msg: print_arguments::to_expr(&call.arguments).map(Box::new), + })), + // We have to replace the entire statement, + // as the `print` could be empty and thus `call.range()` + // will cease to exist. + stmt.range(), + ))); + checker.diagnostics.push(diagnostic); + } } } /// Extracts the arguments from a `print` call and converts them to some kind of string /// expression. /// -/// 3 cases are handled: +/// Three cases are handled: /// - if there are no arguments, return `None` so that `diagnostic` can remove `msg` from `assert`; /// - if all of `print` arguments including `sep` are string literals, return a `Expr::StringLiteral`; /// - otherwise, return a `Expr::FString`. @@ -70,14 +95,14 @@ mod print_arguments { /// Converts an expression to a list of `FStringElement`s. /// - /// 3 cases are handled: + /// Three cases are handled: /// - if the expression is a string literal, each part of the string will be converted to a - /// `FStringLiteralElement`. + /// `FStringLiteralElement`. /// - if the expression is an f-string, the elements will be returned as-is. /// - otherwise, the expression will be wrapped in a `FStringExpressionElement`. fn expr_to_fstring_elements(expr: &Expr) -> Vec { match expr { - // If the expression is a string literal, convert each part to a FStringLiteralElement + // If the expression is a string literal, convert each part to a `FStringLiteralElement`. Expr::StringLiteral(string) => string .value .iter() @@ -88,10 +113,12 @@ mod print_arguments { }) }) .collect(), - // If the expression is an f-string, return the elements + + // If the expression is an f-string, return the elements. Expr::FString(fstring) => fstring.value.elements().cloned().collect(), - // Otherwise, return the expression as a single FStringExpressionElement wrapping - // the expression + + // Otherwise, return the expression as a single `FStringExpressionElement` wrapping + // the expression. expr => vec![FStringElement::Expression(FStringExpressionElement { range: TextRange::default(), expression: Box::new(expr.clone()), @@ -106,7 +133,7 @@ mod print_arguments { /// /// If any of the elements are not string literals, `None` is returned. /// - /// This is useful - in combination with [`expr_to_fstring_elements`] for + /// This is useful (in combination with [`expr_to_fstring_elements`]) for /// checking if the `sep` and `args` arguments to `print` are all string /// literals. fn fstring_elements_to_string_literals<'a>( @@ -185,8 +212,8 @@ mod print_arguments { /// This function will only return [`None`] if there are no arguments at all. /// /// ## Note - /// This function will always return an f-string, even if all of the arguments - /// are string literals. This can produce unnecessary f-strings. + /// This function will always return an f-string, even if all arguments are string literals. + /// This can produce unnecessary f-strings. /// /// Also note that the iterator arguments of this function are consumed, /// as opposed to the references taken by [`args_to_string_literal_expr`]. @@ -224,8 +251,7 @@ mod print_arguments { /// /// ## Returns /// - /// - [`Some`]<[`Expr::StringLiteral`]> if all of the arguments including `sep` are - /// string literals. + /// - [`Some`]<[`Expr::StringLiteral`]> if all arguments including `sep` are string literals. /// - [`Some`]<[`Expr::FString`]> if any of the arguments are not string literals. /// - [`None`] if the `print` contains no positional arguments at all. pub(super) fn to_expr(arguments: &Arguments) -> Option { @@ -262,39 +288,3 @@ mod print_arguments { .or_else(|| args_to_fstring_expr(args.into_iter(), sep.into_iter())) } } - -/// Checks if the `msg` argument to an `assert` statement is a `print` call, and if so, -/// replace the message with the arguments to the `print` call. -pub(crate) fn assert_with_print_expression( - checker: &mut Checker, - stmt: &Stmt, - test: &Expr, - msg: Option<&Expr>, -) { - if let Some(Expr::Call(call)) = msg { - // We have to check that the print call is a call to the built-in `print` function - let semantic = checker.semantic(); - - let Some(qualified_name) = semantic.resolve_qualified_name(&call.func) else { - return; - }; - - if matches!(qualified_name.segments(), ["" | "builtins", "print"]) { - // This is the confirmed rule condition - let mut diagnostic = Diagnostic::new(AssertWithPrintExpression, call.range()); - diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement( - checker.generator().stmt(&Stmt::Assert(StmtAssert { - range: stmt.range(), - test: test.clone().into(), - // Into::into is used for boxing the expression - msg: print_arguments::to_expr(&call.arguments).map(std::convert::Into::into), - })), - // We have to replace the entire statement, - // as the `print` could be empty and thus `call.range()` - // will cease to exist - stmt.range(), - ))); - checker.diagnostics.push(diagnostic); - } - } -} diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF030_RUF030.py.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF030_RUF030.py.snap index 4f04f6263dae0..aeea27858e988 100644 --- a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF030_RUF030.py.snap +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF030_RUF030.py.snap @@ -1,7 +1,7 @@ --- source: crates/ruff_linter/src/rules/ruff/mod.rs --- -RUF030.py:6:14: RUF030 [*] `print()` expressions in `assert` statements is likely unintentional, remove the `print()` call and pass the message +RUF030.py:6:14: RUF030 [*] `print()` expression in `assert` statement is likely unintentional | 4 | # Expects: 5 | # - single StringLiteral @@ -10,7 +10,7 @@ RUF030.py:6:14: RUF030 [*] `print()` expressions in `assert` statements is likel 7 | 8 | # Concatenated string literals | - = help: Remove `print`, and pass the message directly to `assert` + = help: Remove `print` ℹ Unsafe fix 3 3 | # Standard Case @@ -22,7 +22,7 @@ RUF030.py:6:14: RUF030 [*] `print()` expressions in `assert` statements is likel 8 8 | # Concatenated string literals 9 9 | # Expects: -RUF030.py:11:14: RUF030 [*] `print()` expressions in `assert` statements is likely unintentional, remove the `print()` call and pass the message +RUF030.py:11:14: RUF030 [*] `print()` expression in `assert` statement is likely unintentional | 9 | # Expects: 10 | # - single StringLiteral @@ -31,7 +31,7 @@ RUF030.py:11:14: RUF030 [*] `print()` expressions in `assert` statements is like 12 | 13 | # Positional arguments, string literals | - = help: Remove `print`, and pass the message directly to `assert` + = help: Remove `print` ℹ Unsafe fix 8 8 | # Concatenated string literals @@ -43,7 +43,7 @@ RUF030.py:11:14: RUF030 [*] `print()` expressions in `assert` statements is like 13 13 | # Positional arguments, string literals 14 14 | # Expects: -RUF030.py:16:14: RUF030 [*] `print()` expressions in `assert` statements is likely unintentional, remove the `print()` call and pass the message +RUF030.py:16:14: RUF030 [*] `print()` expression in `assert` statement is likely unintentional | 14 | # Expects: 15 | # - single StringLiteral concatenated with " " @@ -52,7 +52,7 @@ RUF030.py:16:14: RUF030 [*] `print()` expressions in `assert` statements is like 17 | 18 | # Concatenated string literals combined with Positional arguments | - = help: Remove `print`, and pass the message directly to `assert` + = help: Remove `print` ℹ Unsafe fix 13 13 | # Positional arguments, string literals @@ -64,7 +64,7 @@ RUF030.py:16:14: RUF030 [*] `print()` expressions in `assert` statements is like 18 18 | # Concatenated string literals combined with Positional arguments 19 19 | # Expects: -RUF030.py:21:14: RUF030 [*] `print()` expressions in `assert` statements is likely unintentional, remove the `print()` call and pass the message +RUF030.py:21:14: RUF030 [*] `print()` expression in `assert` statement is likely unintentional | 19 | # Expects: 20 | # - single stringliteral concatenated with " " only between `print` and `is` @@ -73,7 +73,7 @@ RUF030.py:21:14: RUF030 [*] `print()` expressions in `assert` statements is like 22 | 23 | # Positional arguments, string literals with a variable | - = help: Remove `print`, and pass the message directly to `assert` + = help: Remove `print` ℹ Unsafe fix 18 18 | # Concatenated string literals combined with Positional arguments @@ -85,7 +85,7 @@ RUF030.py:21:14: RUF030 [*] `print()` expressions in `assert` statements is like 23 23 | # Positional arguments, string literals with a variable 24 24 | # Expects: -RUF030.py:26:14: RUF030 [*] `print()` expressions in `assert` statements is likely unintentional, remove the `print()` call and pass the message +RUF030.py:26:14: RUF030 [*] `print()` expression in `assert` statement is likely unintentional | 24 | # Expects: 25 | # - single FString concatenated with " " @@ -94,7 +94,7 @@ RUF030.py:26:14: RUF030 [*] `print()` expressions in `assert` statements is like 27 | 28 | # Mixed brackets string literals | - = help: Remove `print`, and pass the message directly to `assert` + = help: Remove `print` ℹ Unsafe fix 23 23 | # Positional arguments, string literals with a variable @@ -106,7 +106,7 @@ RUF030.py:26:14: RUF030 [*] `print()` expressions in `assert` statements is like 28 28 | # Mixed brackets string literals 29 29 | # Expects: -RUF030.py:31:14: RUF030 [*] `print()` expressions in `assert` statements is likely unintentional, remove the `print()` call and pass the message +RUF030.py:31:14: RUF030 [*] `print()` expression in `assert` statement is likely unintentional | 29 | # Expects: 30 | # - single StringLiteral concatenated with " " @@ -115,7 +115,7 @@ RUF030.py:31:14: RUF030 [*] `print()` expressions in `assert` statements is like 32 | 33 | # Mixed brackets with other brackets inside | - = help: Remove `print`, and pass the message directly to `assert` + = help: Remove `print` ℹ Unsafe fix 28 28 | # Mixed brackets string literals @@ -127,7 +127,7 @@ RUF030.py:31:14: RUF030 [*] `print()` expressions in `assert` statements is like 33 33 | # Mixed brackets with other brackets inside 34 34 | # Expects: -RUF030.py:36:14: RUF030 [*] `print()` expressions in `assert` statements is likely unintentional, remove the `print()` call and pass the message +RUF030.py:36:14: RUF030 [*] `print()` expression in `assert` statement is likely unintentional | 34 | # Expects: 35 | # - single StringLiteral concatenated with " " and escaped brackets @@ -136,7 +136,7 @@ RUF030.py:36:14: RUF030 [*] `print()` expressions in `assert` statements is like 37 | 38 | # Positional arguments, string literals with a separator | - = help: Remove `print`, and pass the message directly to `assert` + = help: Remove `print` ℹ Unsafe fix 33 33 | # Mixed brackets with other brackets inside @@ -148,7 +148,7 @@ RUF030.py:36:14: RUF030 [*] `print()` expressions in `assert` statements is like 38 38 | # Positional arguments, string literals with a separator 39 39 | # Expects: -RUF030.py:41:14: RUF030 [*] `print()` expressions in `assert` statements is likely unintentional, remove the `print()` call and pass the message +RUF030.py:41:14: RUF030 [*] `print()` expression in `assert` statement is likely unintentional | 39 | # Expects: 40 | # - single StringLiteral concatenated with "|" @@ -157,7 +157,7 @@ RUF030.py:41:14: RUF030 [*] `print()` expressions in `assert` statements is like 42 | 43 | # Positional arguments, string literals with None as separator | - = help: Remove `print`, and pass the message directly to `assert` + = help: Remove `print` ℹ Unsafe fix 38 38 | # Positional arguments, string literals with a separator @@ -169,7 +169,7 @@ RUF030.py:41:14: RUF030 [*] `print()` expressions in `assert` statements is like 43 43 | # Positional arguments, string literals with None as separator 44 44 | # Expects: -RUF030.py:46:14: RUF030 [*] `print()` expressions in `assert` statements is likely unintentional, remove the `print()` call and pass the message +RUF030.py:46:14: RUF030 [*] `print()` expression in `assert` statement is likely unintentional | 44 | # Expects: 45 | # - single StringLiteral concatenated with " " @@ -178,7 +178,7 @@ RUF030.py:46:14: RUF030 [*] `print()` expressions in `assert` statements is like 47 | 48 | # Positional arguments, string literals with variable as separator, needs f-string | - = help: Remove `print`, and pass the message directly to `assert` + = help: Remove `print` ℹ Unsafe fix 43 43 | # Positional arguments, string literals with None as separator @@ -190,7 +190,7 @@ RUF030.py:46:14: RUF030 [*] `print()` expressions in `assert` statements is like 48 48 | # Positional arguments, string literals with variable as separator, needs f-string 49 49 | # Expects: -RUF030.py:51:14: RUF030 [*] `print()` expressions in `assert` statements is likely unintentional, remove the `print()` call and pass the message +RUF030.py:51:14: RUF030 [*] `print()` expression in `assert` statement is likely unintentional | 49 | # Expects: 50 | # - single FString concatenated with "{U00A0}" @@ -199,7 +199,7 @@ RUF030.py:51:14: RUF030 [*] `print()` expressions in `assert` statements is like 52 | 53 | # Unnecessary f-string | - = help: Remove `print`, and pass the message directly to `assert` + = help: Remove `print` ℹ Unsafe fix 48 48 | # Positional arguments, string literals with variable as separator, needs f-string @@ -211,7 +211,7 @@ RUF030.py:51:14: RUF030 [*] `print()` expressions in `assert` statements is like 53 53 | # Unnecessary f-string 54 54 | # Expects: -RUF030.py:56:14: RUF030 [*] `print()` expressions in `assert` statements is likely unintentional, remove the `print()` call and pass the message +RUF030.py:56:14: RUF030 [*] `print()` expression in `assert` statement is likely unintentional | 54 | # Expects: 55 | # - single StringLiteral @@ -220,7 +220,7 @@ RUF030.py:56:14: RUF030 [*] `print()` expressions in `assert` statements is like 57 | 58 | # Positional arguments, string literals and f-strings | - = help: Remove `print`, and pass the message directly to `assert` + = help: Remove `print` ℹ Unsafe fix 53 53 | # Unnecessary f-string @@ -232,7 +232,7 @@ RUF030.py:56:14: RUF030 [*] `print()` expressions in `assert` statements is like 58 58 | # Positional arguments, string literals and f-strings 59 59 | # Expects: -RUF030.py:61:14: RUF030 [*] `print()` expressions in `assert` statements is likely unintentional, remove the `print()` call and pass the message +RUF030.py:61:14: RUF030 [*] `print()` expression in `assert` statement is likely unintentional | 59 | # Expects: 60 | # - single FString concatenated with " " @@ -241,7 +241,7 @@ RUF030.py:61:14: RUF030 [*] `print()` expressions in `assert` statements is like 62 | 63 | # Positional arguments, string literals and f-strings with a separator | - = help: Remove `print`, and pass the message directly to `assert` + = help: Remove `print` ℹ Unsafe fix 58 58 | # Positional arguments, string literals and f-strings @@ -253,7 +253,7 @@ RUF030.py:61:14: RUF030 [*] `print()` expressions in `assert` statements is like 63 63 | # Positional arguments, string literals and f-strings with a separator 64 64 | # Expects: -RUF030.py:66:14: RUF030 [*] `print()` expressions in `assert` statements is likely unintentional, remove the `print()` call and pass the message +RUF030.py:66:14: RUF030 [*] `print()` expression in `assert` statement is likely unintentional | 64 | # Expects: 65 | # - single FString concatenated with "|" @@ -262,7 +262,7 @@ RUF030.py:66:14: RUF030 [*] `print()` expressions in `assert` statements is like 67 | 68 | # A single f-string | - = help: Remove `print`, and pass the message directly to `assert` + = help: Remove `print` ℹ Unsafe fix 63 63 | # Positional arguments, string literals and f-strings with a separator @@ -274,7 +274,7 @@ RUF030.py:66:14: RUF030 [*] `print()` expressions in `assert` statements is like 68 68 | # A single f-string 69 69 | # Expects: -RUF030.py:71:14: RUF030 [*] `print()` expressions in `assert` statements is likely unintentional, remove the `print()` call and pass the message +RUF030.py:71:14: RUF030 [*] `print()` expression in `assert` statement is likely unintentional | 69 | # Expects: 70 | # - single FString @@ -283,7 +283,7 @@ RUF030.py:71:14: RUF030 [*] `print()` expressions in `assert` statements is like 72 | 73 | # A single f-string with a redundant separator | - = help: Remove `print`, and pass the message directly to `assert` + = help: Remove `print` ℹ Unsafe fix 68 68 | # A single f-string @@ -295,7 +295,7 @@ RUF030.py:71:14: RUF030 [*] `print()` expressions in `assert` statements is like 73 73 | # A single f-string with a redundant separator 74 74 | # Expects: -RUF030.py:76:14: RUF030 [*] `print()` expressions in `assert` statements is likely unintentional, remove the `print()` call and pass the message +RUF030.py:76:14: RUF030 [*] `print()` expression in `assert` statement is likely unintentional | 74 | # Expects: 75 | # - single FString @@ -304,7 +304,7 @@ RUF030.py:76:14: RUF030 [*] `print()` expressions in `assert` statements is like 77 | 78 | # Complex f-string with variable as separator | - = help: Remove `print`, and pass the message directly to `assert` + = help: Remove `print` ℹ Unsafe fix 73 73 | # A single f-string with a redundant separator @@ -316,7 +316,7 @@ RUF030.py:76:14: RUF030 [*] `print()` expressions in `assert` statements is like 78 78 | # Complex f-string with variable as separator 79 79 | # Expects: -RUF030.py:83:14: RUF030 [*] `print()` expressions in `assert` statements is likely unintentional, remove the `print()` call and pass the message +RUF030.py:83:14: RUF030 [*] `print()` expression in `assert` statement is likely unintentional | 81 | condition = "True is True" 82 | maintainer = "John Doe" @@ -325,7 +325,7 @@ RUF030.py:83:14: RUF030 [*] `print()` expressions in `assert` statements is like 84 | 85 | # Empty print | - = help: Remove `print`, and pass the message directly to `assert` + = help: Remove `print` ℹ Unsafe fix 80 80 | # - single FString concatenated with "{U00A0}", all placeholders preserved @@ -337,7 +337,7 @@ RUF030.py:83:14: RUF030 [*] `print()` expressions in `assert` statements is like 85 85 | # Empty print 86 86 | # Expects: -RUF030.py:88:14: RUF030 [*] `print()` expressions in `assert` statements is likely unintentional, remove the `print()` call and pass the message +RUF030.py:88:14: RUF030 [*] `print()` expression in `assert` statement is likely unintentional | 86 | # Expects: 87 | # - `msg` entirely removed from assertion @@ -346,7 +346,7 @@ RUF030.py:88:14: RUF030 [*] `print()` expressions in `assert` statements is like 89 | 90 | # Empty print with separator | - = help: Remove `print`, and pass the message directly to `assert` + = help: Remove `print` ℹ Unsafe fix 85 85 | # Empty print @@ -358,7 +358,7 @@ RUF030.py:88:14: RUF030 [*] `print()` expressions in `assert` statements is like 90 90 | # Empty print with separator 91 91 | # Expects: -RUF030.py:93:14: RUF030 [*] `print()` expressions in `assert` statements is likely unintentional, remove the `print()` call and pass the message +RUF030.py:93:14: RUF030 [*] `print()` expression in `assert` statement is likely unintentional | 91 | # Expects: 92 | # - `msg` entirely removed from assertion @@ -367,7 +367,7 @@ RUF030.py:93:14: RUF030 [*] `print()` expressions in `assert` statements is like 94 | 95 | # Custom print function that actually returns a string | - = help: Remove `print`, and pass the message directly to `assert` + = help: Remove `print` ℹ Unsafe fix 90 90 | # Empty print with separator @@ -379,14 +379,14 @@ RUF030.py:93:14: RUF030 [*] `print()` expressions in `assert` statements is like 95 95 | # Custom print function that actually returns a string 96 96 | # Expects: -RUF030.py:108:14: RUF030 [*] `print()` expressions in `assert` statements is likely unintentional, remove the `print()` call and pass the message +RUF030.py:108:14: RUF030 [*] `print()` expression in `assert` statement is likely unintentional | 106 | # Expects: 107 | # - single StringLiteral 108 | assert True, builtins.print("This print should be removed.") | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF030 | - = help: Remove `print`, and pass the message directly to `assert` + = help: Remove `print` ℹ Unsafe fix 105 105 | # Use of `builtins.print` From dbcd1cf390b668047bc3728a6e5ea386bcae6dfb Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sun, 23 Jun 2024 12:48:54 -0400 Subject: [PATCH 3/4] Add some more TextRange defaults --- .../rules/assert_with_print_expression.rs | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/crates/ruff_linter/src/rules/ruff/rules/assert_with_print_expression.rs b/crates/ruff_linter/src/rules/ruff/rules/assert_with_print_expression.rs index d3af00f156e32..25bf76c005ace 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/assert_with_print_expression.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/assert_with_print_expression.rs @@ -1,5 +1,5 @@ use ruff_python_ast::{self as ast, Expr, Stmt}; -use ruff_text_size::Ranged; +use ruff_text_size::{Ranged, TextRange}; use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix}; use ruff_macros::{derive_message_formats, violation}; @@ -16,7 +16,7 @@ use crate::checkers::ast::Checker; /// empty `AssertionError(None)`. /// /// Instead, remove the `print` and pass the message directly as the second -/// expression, allowing `stderr` to capture the message in a well formatted context. +/// expression, allowing `stderr` to capture the message in a well-formatted context. /// /// ## Example /// ```python @@ -63,9 +63,9 @@ pub(crate) fn assert_with_print_expression(checker: &mut Checker, stmt: &ast::St let mut diagnostic = Diagnostic::new(AssertWithPrintExpression, call.range()); diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement( checker.generator().stmt(&Stmt::Assert(ast::StmtAssert { - range: stmt.range(), - test: stmt.test.clone().into(), + test: stmt.test.clone(), msg: print_arguments::to_expr(&call.arguments).map(Box::new), + range: TextRange::default(), })), // We have to replace the entire statement, // as the `print` could be empty and thus `call.range()` @@ -108,8 +108,8 @@ mod print_arguments { .iter() .map(|part| { FStringElement::Literal(FStringLiteralElement { - range: TextRange::default(), value: part.value.clone(), + range: TextRange::default(), }) }) .collect(), @@ -120,11 +120,11 @@ mod print_arguments { // Otherwise, return the expression as a single `FStringExpressionElement` wrapping // the expression. expr => vec![FStringElement::Expression(FStringExpressionElement { - range: TextRange::default(), expression: Box::new(expr.clone()), debug_text: None, conversion: ConversionFlag::None, format_spec: None, + range: TextRange::default(), })], } } @@ -142,9 +142,9 @@ mod print_arguments { elements.try_fold(Vec::with_capacity(elements.len()), |mut acc, element| { if let FStringElement::Literal(literal) = element { acc.push(StringLiteral { - range: literal.range, value: literal.value.clone(), flags: StringLiteralFlags::default(), + range: TextRange::default(), }); Some(acc) } else { @@ -200,9 +200,9 @@ mod print_arguments { Some(Expr::StringLiteral(ExprStringLiteral { range: TextRange::default(), value: StringLiteralValue::single(StringLiteral { - range: TextRange::default(), value: combined_string.into(), flags: StringLiteralFlags::default(), + range: TextRange::default(), }), })) } @@ -232,12 +232,12 @@ mod print_arguments { }); Some(Expr::FString(ExprFString { - range: TextRange::default(), value: FStringValue::single(FString { - range: TextRange::default(), elements: FStringElements::from(fstring_elements), flags: FStringFlags::default(), + range: TextRange::default(), }), + range: TextRange::default(), })) } @@ -259,7 +259,7 @@ mod print_arguments { let sep = arguments .find_keyword("sep") .and_then( - // If the `sep` argument is `None`, treat this as default behavior + // If the `sep` argument is `None`, treat this as default behavior. |keyword| { if let Expr::NoneLiteral(_) = keyword.value { None From 416a033ae39511404e4e572310ee9256a6fdd9e9 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sun, 23 Jun 2024 12:50:09 -0400 Subject: [PATCH 4/4] Rename to assert-with-print-message --- crates/ruff_linter/src/checkers/ast/analyze/statement.rs | 4 ++-- crates/ruff_linter/src/codes.rs | 2 +- crates/ruff_linter/src/rules/ruff/mod.rs | 2 +- ...h_print_expression.rs => assert_with_print_message.rs} | 8 ++++---- crates/ruff_linter/src/rules/ruff/rules/mod.rs | 4 ++-- 5 files changed, 10 insertions(+), 10 deletions(-) rename crates/ruff_linter/src/rules/ruff/rules/{assert_with_print_expression.rs => assert_with_print_message.rs} (98%) diff --git a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs index 59562322b98e0..fdb27a664ccf7 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/statement.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/statement.rs @@ -1269,8 +1269,8 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) { if checker.enabled(Rule::InvalidMockAccess) { pygrep_hooks::rules::non_existent_mock_method(checker, test); } - if checker.enabled(Rule::AssertWithPrintExpression) { - ruff::rules::assert_with_print_expression(checker, assert_stmt); + if checker.enabled(Rule::AssertWithPrintMessage) { + ruff::rules::assert_with_print_message(checker, assert_stmt); } } Stmt::With(with_stmt @ ast::StmtWith { items, body, .. }) => { diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 5623d924ac7c3..14bd9e6848cce 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -977,7 +977,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Ruff, "027") => (RuleGroup::Preview, rules::ruff::rules::MissingFStringSyntax), (Ruff, "028") => (RuleGroup::Preview, rules::ruff::rules::InvalidFormatterSuppressionComment), (Ruff, "029") => (RuleGroup::Preview, rules::ruff::rules::UnusedAsync), - (Ruff, "030") => (RuleGroup::Preview, rules::ruff::rules::AssertWithPrintExpression), + (Ruff, "030") => (RuleGroup::Preview, rules::ruff::rules::AssertWithPrintMessage), (Ruff, "100") => (RuleGroup::Stable, rules::ruff::rules::UnusedNOQA), (Ruff, "101") => (RuleGroup::Preview, rules::ruff::rules::RedirectedNOQA), (Ruff, "200") => (RuleGroup::Stable, rules::ruff::rules::InvalidPyprojectToml), diff --git a/crates/ruff_linter/src/rules/ruff/mod.rs b/crates/ruff_linter/src/rules/ruff/mod.rs index b63cac8d5a8da..c9708eb848253 100644 --- a/crates/ruff_linter/src/rules/ruff/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/mod.rs @@ -54,7 +54,7 @@ mod tests { #[test_case(Rule::MissingFStringSyntax, Path::new("RUF027_2.py"))] #[test_case(Rule::InvalidFormatterSuppressionComment, Path::new("RUF028.py"))] #[test_case(Rule::UnusedAsync, Path::new("RUF029.py"))] - #[test_case(Rule::AssertWithPrintExpression, Path::new("RUF030.py"))] + #[test_case(Rule::AssertWithPrintMessage, Path::new("RUF030.py"))] #[test_case(Rule::RedirectedNOQA, Path::new("RUF101.py"))] fn rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy()); diff --git a/crates/ruff_linter/src/rules/ruff/rules/assert_with_print_expression.rs b/crates/ruff_linter/src/rules/ruff/rules/assert_with_print_message.rs similarity index 98% rename from crates/ruff_linter/src/rules/ruff/rules/assert_with_print_expression.rs rename to crates/ruff_linter/src/rules/ruff/rules/assert_with_print_message.rs index 25bf76c005ace..cf5b80f004846 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/assert_with_print_expression.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/assert_with_print_message.rs @@ -36,9 +36,9 @@ use crate::checkers::ast::Checker; /// ## References /// - [Python documentation: `assert`](https://docs.python.org/3/reference/simple_stmts.html#the-assert-statement) #[violation] -pub struct AssertWithPrintExpression; +pub struct AssertWithPrintMessage; -impl AlwaysFixableViolation for AssertWithPrintExpression { +impl AlwaysFixableViolation for AssertWithPrintMessage { #[derive_message_formats] fn message(&self) -> String { format!("`print()` expression in `assert` statement is likely unintentional") @@ -53,14 +53,14 @@ impl AlwaysFixableViolation for AssertWithPrintExpression { /// /// Checks if the `msg` argument to an `assert` statement is a `print` call, and if so, /// replace the message with the arguments to the `print` call. -pub(crate) fn assert_with_print_expression(checker: &mut Checker, stmt: &ast::StmtAssert) { +pub(crate) fn assert_with_print_message(checker: &mut Checker, stmt: &ast::StmtAssert) { if let Some(Expr::Call(call)) = stmt.msg.as_deref() { // We have to check that the print call is a call to the built-in `print` function let semantic = checker.semantic(); if semantic.match_builtin_expr(&call.func, "print") { // This is the confirmed rule condition - let mut diagnostic = Diagnostic::new(AssertWithPrintExpression, call.range()); + let mut diagnostic = Diagnostic::new(AssertWithPrintMessage, call.range()); diagnostic.set_fix(Fix::unsafe_edit(Edit::range_replacement( checker.generator().stmt(&Stmt::Assert(ast::StmtAssert { test: stmt.test.clone(), diff --git a/crates/ruff_linter/src/rules/ruff/rules/mod.rs b/crates/ruff_linter/src/rules/ruff/rules/mod.rs index b947b34880261..399aa8584abbd 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/mod.rs @@ -1,5 +1,5 @@ pub(crate) use ambiguous_unicode_character::*; -pub(crate) use assert_with_print_expression::*; +pub(crate) use assert_with_print_message::*; pub(crate) use assignment_in_assert::*; pub(crate) use asyncio_dangling_task::*; pub(crate) use collection_literal_concatenation::*; @@ -31,7 +31,7 @@ pub(crate) use unused_async::*; pub(crate) use unused_noqa::*; mod ambiguous_unicode_character; -mod assert_with_print_expression; +mod assert_with_print_message; mod assignment_in_assert; mod asyncio_dangling_task; mod collection_literal_concatenation;