Skip to content

Commit

Permalink
[pylint]: Implement assert-on-string-literal (W0129) (#3610)
Browse files Browse the repository at this point in the history
  • Loading branch information
latonis committed Mar 20, 2023
1 parent b083261 commit a45753f
Show file tree
Hide file tree
Showing 9 changed files with 274 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
def test_division():
a = 9 / 3
assert "No ZeroDivisionError were raised" # [assert-on-string-literal]


def test_division():
a = 9 / 3
assert a == 3


try:
assert "bad" # [assert-on-string-literal]
except:
assert "bad again" # [assert-on-string-literal]

a = 12
assert f"hello {a}" # [assert-on-string-literal]
assert f"{a}" # [assert-on-string-literal]
assert f"" # [assert-on-string-literal]
assert "" # [assert-on-string-literal]
assert b"hello" # [assert-on-string-literal]
assert "", b"hi" # [assert-on-string-literal]
assert "WhyNotHere?", "HereIsOk" # [assert-on-string-literal]
assert 12, "ok here"
4 changes: 4 additions & 0 deletions crates/ruff/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1584,6 +1584,10 @@ where
msg.as_deref(),
);
}

if self.settings.rules.enabled(Rule::AssertOnStringLiteral) {
pylint::rules::assert_on_string_literal(self, test);
}
}
StmtKind::With { items, body, .. } => {
if self.settings.rules.enabled(Rule::AssertRaisesException) {
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<Rule> {
(Pylint, "R2004") => Rule::MagicValueComparison,
(Pylint, "R5501") => Rule::CollapsibleElseIf,
(Pylint, "W0120") => Rule::UselessElseOnLoop,
(Pylint, "W0129") => Rule::AssertOnStringLiteral,
(Pylint, "W0602") => Rule::GlobalVariableNotAssigned,
(Pylint, "W0603") => Rule::GlobalStatement,
(Pylint, "W1508") => Rule::InvalidEnvvarDefault,
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ ruff_macros::register_rules!(
rules::pyflakes::rules::UnusedAnnotation,
rules::pyflakes::rules::RaiseNotImplemented,
// pylint
rules::pylint::rules::AssertOnStringLiteral,
rules::pylint::rules::UselessReturn,
rules::pylint::rules::YieldInInit,
rules::pylint::rules::InvalidAllObject,
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/rules/pylint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ mod tests {
use crate::test::test_path;

#[test_case(Rule::AwaitOutsideAsync, Path::new("await_outside_async.py"); "PLE1142")]
#[test_case(Rule::AssertOnStringLiteral, Path::new("assert_on_string_literal.py"); "PLW0129")]
#[test_case(Rule::BadStrStripCall, Path::new("bad_str_strip_call.py"); "PLE01310")]
#[test_case(Rule::BadStringFormatType, Path::new("bad_string_format_type.py"); "PLE1307")]
#[test_case(Rule::BidirectionalUnicode, Path::new("bidirectional_unicode.py"); "PLE2502")]
Expand Down
105 changes: 105 additions & 0 deletions crates/ruff/src/rules/pylint/rules/assert_on_string_literal.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
use rustpython_parser::ast::{Constant, Expr, ExprKind};

use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::types::Range;

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

#[derive(Debug, PartialEq, Eq, Copy, Clone)]
enum Kind {
Empty,
NonEmpty,
Unknown,
}

/// ## What it does
/// Checks for `assert` statements that use a string literal as the first
/// argument.
///
/// ## Why is this bad?
/// An `assert` on a non-empty string literal will always pass, while an
/// `assert` on an empty string literal will always fail.
///
/// ## Example
/// ```python
/// assert "always true"
/// ```
#[violation]
pub struct AssertOnStringLiteral {
kind: Kind,
}

impl Violation for AssertOnStringLiteral {
#[derive_message_formats]
fn message(&self) -> String {
let AssertOnStringLiteral { kind } = self;
match kind {
Kind::Empty => format!("Asserting on an empty string literal will never pass"),
Kind::NonEmpty => format!("Asserting on a non-empty string literal will always pass"),
Kind::Unknown => format!("Asserting on a string literal may have unintended results"),
}
}
}

/// PLW0129
pub fn assert_on_string_literal(checker: &mut Checker, test: &Expr) {
match &test.node {
ExprKind::Constant { value, .. } => match value {
Constant::Str(value, ..) => {
checker.diagnostics.push(Diagnostic::new(
AssertOnStringLiteral {
kind: if value.is_empty() {
Kind::Empty
} else {
Kind::NonEmpty
},
},
Range::from(test),
));
}
Constant::Bytes(value) => {
checker.diagnostics.push(Diagnostic::new(
AssertOnStringLiteral {
kind: if value.is_empty() {
Kind::Empty
} else {
Kind::NonEmpty
},
},
Range::from(test),
));
}
_ => {}
},
ExprKind::JoinedStr { values } => {
checker.diagnostics.push(Diagnostic::new(
AssertOnStringLiteral {
kind: if values.iter().all(|value| match &value.node {
ExprKind::Constant { value, .. } => match value {
Constant::Str(value, ..) => value.is_empty(),
Constant::Bytes(value) => value.is_empty(),
_ => false,
},
_ => false,
}) {
Kind::Empty
} else if values.iter().any(|value| match &value.node {
ExprKind::Constant { value, .. } => match value {
Constant::Str(value, ..) => !value.is_empty(),
Constant::Bytes(value) => !value.is_empty(),
_ => false,
},
_ => false,
}) {
Kind::NonEmpty
} else {
Kind::Unknown
},
},
Range::from(test),
));
}
_ => {}
}
}
2 changes: 2 additions & 0 deletions crates/ruff/src/rules/pylint/rules/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
pub use assert_on_string_literal::{assert_on_string_literal, AssertOnStringLiteral};
pub use await_outside_async::{await_outside_async, AwaitOutsideAsync};
pub use bad_str_strip_call::{bad_str_strip_call, BadStrStripCall};
pub use bad_string_format_type::{bad_string_format_type, BadStringFormatType};
Expand Down Expand Up @@ -40,6 +41,7 @@ pub use useless_import_alias::{useless_import_alias, UselessImportAlias};
pub use useless_return::{useless_return, UselessReturn};
pub use yield_in_init::{yield_in_init, YieldInInit};

mod assert_on_string_literal;
mod await_outside_async;
mod bad_str_strip_call;
mod bad_string_format_type;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
---
source: crates/ruff/src/rules/pylint/mod.rs
expression: diagnostics
---
- kind:
name: AssertOnStringLiteral
body: Asserting on a non-empty string literal will always pass
suggestion: ~
fixable: false
location:
row: 3
column: 11
end_location:
row: 3
column: 45
fix: ~
parent: ~
- kind:
name: AssertOnStringLiteral
body: Asserting on a non-empty string literal will always pass
suggestion: ~
fixable: false
location:
row: 12
column: 11
end_location:
row: 12
column: 16
fix: ~
parent: ~
- kind:
name: AssertOnStringLiteral
body: Asserting on a non-empty string literal will always pass
suggestion: ~
fixable: false
location:
row: 14
column: 11
end_location:
row: 14
column: 22
fix: ~
parent: ~
- kind:
name: AssertOnStringLiteral
body: Asserting on a non-empty string literal will always pass
suggestion: ~
fixable: false
location:
row: 17
column: 7
end_location:
row: 17
column: 19
fix: ~
parent: ~
- kind:
name: AssertOnStringLiteral
body: Asserting on a string literal may have unintended results
suggestion: ~
fixable: false
location:
row: 18
column: 7
end_location:
row: 18
column: 13
fix: ~
parent: ~
- kind:
name: AssertOnStringLiteral
body: Asserting on an empty string literal will never pass
suggestion: ~
fixable: false
location:
row: 19
column: 7
end_location:
row: 19
column: 10
fix: ~
parent: ~
- kind:
name: AssertOnStringLiteral
body: Asserting on an empty string literal will never pass
suggestion: ~
fixable: false
location:
row: 20
column: 7
end_location:
row: 20
column: 9
fix: ~
parent: ~
- kind:
name: AssertOnStringLiteral
body: Asserting on a non-empty string literal will always pass
suggestion: ~
fixable: false
location:
row: 21
column: 7
end_location:
row: 21
column: 15
fix: ~
parent: ~
- kind:
name: AssertOnStringLiteral
body: Asserting on an empty string literal will never pass
suggestion: ~
fixable: false
location:
row: 22
column: 7
end_location:
row: 22
column: 9
fix: ~
parent: ~
- kind:
name: AssertOnStringLiteral
body: Asserting on a non-empty string literal will always pass
suggestion: ~
fixable: false
location:
row: 23
column: 7
end_location:
row: 23
column: 20
fix: ~
parent: ~

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 a45753f

Please sign in to comment.