Skip to content

Commit

Permalink
[ruff] Implement invalid-assert-message-literal-argument (`RUF040…
Browse files Browse the repository at this point in the history
…`) (#14488)

## Summary

This PR implements new rule discussed
[here](#14449).
In short, it searches for assert messages which were unintentionally
used as a expression to be matched against.

## Test Plan

`cargo test` and review of `ruff-ecosystem`
  • Loading branch information
Lokejoke authored Nov 25, 2024
1 parent 557d583 commit 9e4ee98
Show file tree
Hide file tree
Showing 8 changed files with 85 additions and 0 deletions.
5 changes: 5 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/ruff/RUF040.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
fruits = ["apples", "plums", "pear"]
fruits.filter(lambda fruit: fruit.startwith("p"))
assert len(fruits), 2

assert True, "always true"
3 changes: 3 additions & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1276,6 +1276,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
if checker.enabled(Rule::AssertWithPrintMessage) {
ruff::rules::assert_with_print_message(checker, assert_stmt);
}
if checker.enabled(Rule::InvalidAssertMessageLiteralArgument) {
ruff::rules::invalid_assert_message_literal_argument(checker, assert_stmt);
}
}
Stmt::With(
with_stmt @ ast::StmtWith {
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -978,6 +978,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Ruff, "038") => (RuleGroup::Preview, rules::ruff::rules::RedundantBoolLiteral),
(Ruff, "048") => (RuleGroup::Preview, rules::ruff::rules::MapIntVersionParsing),
(Ruff, "039") => (RuleGroup::Preview, rules::ruff::rules::UnrawRePattern),
(Ruff, "040") => (RuleGroup::Preview, rules::ruff::rules::InvalidAssertMessageLiteralArgument),
(Ruff, "100") => (RuleGroup::Stable, rules::ruff::rules::UnusedNOQA),
(Ruff, "101") => (RuleGroup::Stable, rules::ruff::rules::RedirectedNOQA),

Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/ruff/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ mod tests {
#[test_case(Rule::NoneNotAtEndOfUnion, Path::new("RUF036.pyi"))]
#[test_case(Rule::RedundantBoolLiteral, Path::new("RUF038.py"))]
#[test_case(Rule::RedundantBoolLiteral, Path::new("RUF038.pyi"))]
#[test_case(Rule::InvalidAssertMessageLiteralArgument, Path::new("RUF040.py"))]
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
let diagnostics = test_path(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{Expr, StmtAssert};
use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;

/// ## What it does
/// Checks for invalid use of literals in assert message argument.
///
/// ## Why is this bad?
/// An assert message which is a non-string literal was likely intended
/// to be used in a comparison assertion, rather than as a message.
///
/// ## Example
/// ```python
/// fruits = ["apples", "plums", "pears"]
/// fruits.filter(lambda fruit: fruit.startwith("p"))
/// assert len(fruits), 2 # True unless the list is empty
/// ```
///
/// Use instead:
/// ```python
/// fruits = ["apples", "plums", "pears"]
/// fruits.filter(lambda fruit: fruit.startwith("p"))
/// assert len(fruits) == 2
/// ```
#[violation]
pub struct InvalidAssertMessageLiteralArgument;

impl Violation for InvalidAssertMessageLiteralArgument {
#[derive_message_formats]
fn message(&self) -> String {
"Non-string literal used as assert message".to_string()
}
}

/// RUF040
pub(crate) fn invalid_assert_message_literal_argument(checker: &mut Checker, stmt: &StmtAssert) {
let Some(message) = stmt.msg.as_deref() else {
return;
};

if !matches!(
message,
Expr::NumberLiteral(_)
| Expr::BooleanLiteral(_)
| Expr::NoneLiteral(_)
| Expr::EllipsisLiteral(_)
| Expr::BytesLiteral(_)
) {
return;
}

checker.diagnostics.push(Diagnostic::new(
InvalidAssertMessageLiteralArgument,
message.range(),
));
}
2 changes: 2 additions & 0 deletions crates/ruff_linter/src/rules/ruff/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ pub(crate) use explicit_f_string_type_conversion::*;
pub(crate) use function_call_in_dataclass_default::*;
pub(crate) use implicit_optional::*;
pub(crate) use incorrectly_parenthesized_tuple_in_subscript::*;
pub(crate) use invalid_assert_message_literal_argument::*;
pub(crate) use invalid_formatter_suppression_comment::*;
pub(crate) use invalid_index_type::*;
pub(crate) use invalid_pyproject_toml::*;
Expand Down Expand Up @@ -51,6 +52,7 @@ mod function_call_in_dataclass_default;
mod helpers;
mod implicit_optional;
mod incorrectly_parenthesized_tuple_in_subscript;
mod invalid_assert_message_literal_argument;
mod invalid_formatter_suppression_comment;
mod invalid_index_type;
mod invalid_pyproject_toml;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
---
source: crates/ruff_linter/src/rules/ruff/mod.rs
snapshot_kind: text
---
RUF040.py:3:21: RUF040 Non-string literal used as assert message
|
1 | fruits = ["apples", "plums", "pear"]
2 | fruits.filter(lambda fruit: fruit.startwith("p"))
3 | assert len(fruits), 2
| ^ RUF040
4 |
5 | assert True, "always true"
|
1 change: 1 addition & 0 deletions ruff.schema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 9e4ee98

Please sign in to comment.