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

[pylint]: Implement assert-on-string-literal (W0129) #3610

Merged
merged 15 commits into from
Mar 20, 2023
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.