Skip to content

Commit

Permalink
[ruff] Ambiguous pattern passed to pytest.raises() (RUF043) (#1…
Browse files Browse the repository at this point in the history
  • Loading branch information
InSyncWithFoo authored Dec 18, 2024
1 parent c0b7c36 commit ac81c72
Show file tree
Hide file tree
Showing 11 changed files with 575 additions and 3 deletions.
91 changes: 91 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/ruff/RUF043.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
import re
import pytest

def test_foo():

### Errors

with pytest.raises(FooAtTheEnd, match="foo."): ...
with pytest.raises(PackageExtraSpecifier, match="Install `foo[bar]` to enjoy all features"): ...
with pytest.raises(InnocentQuestion, match="Did you mean to use `Literal` instead?"): ...

with pytest.raises(StringConcatenation, match="Huh"
"?"): ...
with pytest.raises(ManuallyEscapedWindowsPathToDotFile, match="C:\\\\Users\\\\Foo\\\\.config"): ...

with pytest.raises(MiddleDot, match="foo.bar"): ...
with pytest.raises(EndDot, match="foobar."): ...
with pytest.raises(EscapedFollowedByUnescaped, match="foo\\.*bar"): ...
with pytest.raises(UnescapedFollowedByEscaped, match="foo.\\*bar"): ...


## Metasequences

with pytest.raises(StartOfInput, match="foo\\Abar"): ...
with pytest.raises(WordBoundary, match="foo\\bbar"): ...
with pytest.raises(NonWordBoundary, match="foo\\Bbar"): ...
with pytest.raises(Digit, match="foo\\dbar"): ...
with pytest.raises(NonDigit, match="foo\\Dbar"): ...
with pytest.raises(Whitespace, match="foo\\sbar"): ...
with pytest.raises(NonWhitespace, match="foo\\Sbar"): ...
with pytest.raises(WordCharacter, match="foo\\wbar"): ...
with pytest.raises(NonWordCharacter, match="foo\\Wbar"): ...
with pytest.raises(EndOfInput, match="foo\\zbar"): ...

with pytest.raises(StartOfInput2, match="foobar\\A"): ...
with pytest.raises(WordBoundary2, match="foobar\\b"): ...
with pytest.raises(NonWordBoundary2, match="foobar\\B"): ...
with pytest.raises(Digit2, match="foobar\\d"): ...
with pytest.raises(NonDigit2, match="foobar\\D"): ...
with pytest.raises(Whitespace2, match="foobar\\s"): ...
with pytest.raises(NonWhitespace2, match="foobar\\S"): ...
with pytest.raises(WordCharacter2, match="foobar\\w"): ...
with pytest.raises(NonWordCharacter2, match="foobar\\W"): ...
with pytest.raises(EndOfInput2, match="foobar\\z"): ...


### Acceptable false positives

with pytest.raises(NameEscape, match="\\N{EN DASH}"): ...


### No errors

with pytest.raises(NoMatch): ...
with pytest.raises(NonLiteral, match=pattern): ...
with pytest.raises(FunctionCall, match=frobnicate("qux")): ...
with pytest.raises(ReEscaped, match=re.escape("foobar")): ...
with pytest.raises(RawString, match=r"fo()bar"): ...
with pytest.raises(RawStringPart, match=r"foo" '\bar'): ...
with pytest.raises(NoMetacharacters, match="foobar"): ...
with pytest.raises(EndBackslash, match="foobar\\"): ...

with pytest.raises(ManuallyEscaped, match="some\\.fully\\.qualified\\.name"): ...
with pytest.raises(ManuallyEscapedWindowsPath, match="C:\\\\Users\\\\Foo\\\\file\\.py"): ...

with pytest.raises(MiddleEscapedDot, match="foo\\.bar"): ...
with pytest.raises(MiddleEscapedBackslash, match="foo\\\\bar"): ...
with pytest.raises(EndEscapedDot, match="foobar\\."): ...
with pytest.raises(EndEscapedBackslash, match="foobar\\\\"): ...


## Not-so-special metasequences

with pytest.raises(Alert, match="\\f"): ...
with pytest.raises(FormFeed, match="\\f"): ...
with pytest.raises(Newline, match="\\n"): ...
with pytest.raises(CarriageReturn, match="\\r"): ...
with pytest.raises(Tab, match="\\t"): ...
with pytest.raises(VerticalTab, match="\\v"): ...
with pytest.raises(HexEscape, match="\\xFF"): ...
with pytest.raises(_16BitUnicodeEscape, match="\\FFFF"): ...
with pytest.raises(_32BitUnicodeEscape, match="\\0010FFFF"): ...

## Escaped metasequences

with pytest.raises(Whitespace, match="foo\\\\sbar"): ...
with pytest.raises(NonWhitespace, match="foo\\\\Sbar"): ...

## Work by accident

with pytest.raises(OctalEscape, match="\\042"): ...
3 changes: 3 additions & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1105,6 +1105,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
if checker.enabled(Rule::BatchedWithoutExplicitStrict) {
flake8_bugbear::rules::batched_without_explicit_strict(checker, call);
}
if checker.enabled(Rule::PytestRaisesAmbiguousPattern) {
ruff::rules::pytest_raises_ambiguous_pattern(checker, call);
}
}
Expr::Dict(dict) => {
if checker.any_enabled(&[
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 @@ -985,6 +985,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Ruff, "039") => (RuleGroup::Preview, rules::ruff::rules::UnrawRePattern),
(Ruff, "040") => (RuleGroup::Preview, rules::ruff::rules::InvalidAssertMessageLiteralArgument),
(Ruff, "041") => (RuleGroup::Preview, rules::ruff::rules::UnnecessaryNestedLiteral),
(Ruff, "043") => (RuleGroup::Preview, rules::ruff::rules::PytestRaisesAmbiguousPattern),
(Ruff, "046") => (RuleGroup::Preview, rules::ruff::rules::UnnecessaryCastToInt),
(Ruff, "048") => (RuleGroup::Preview, rules::ruff::rules::MapIntVersionParsing),
(Ruff, "051") => (RuleGroup::Preview, rules::ruff::rules::IfKeyInDictDel),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ impl Violation for PytestAssertInExcept {
/// ...
/// ```
///
/// References
/// ## References
/// - [`pytest` documentation: `pytest.fail`](https://docs.pytest.org/en/latest/reference/reference.html#pytest-fail)
#[derive(ViolationMetadata)]
pub(crate) struct PytestAssertAlwaysFalse;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,6 @@ impl AlwaysFixableViolation for PytestIncorrectMarkParenthesesStyle {
///
/// ## References
/// - [`pytest` documentation: `pytest.mark.usefixtures`](https://docs.pytest.org/en/latest/reference/reference.html#pytest-mark-usefixtures)
#[derive(ViolationMetadata)]
pub(crate) struct PytestUseFixturesWithoutParameters;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ impl Violation for PytestRaisesWithoutException {
}
}

fn is_pytest_raises(func: &Expr, semantic: &SemanticModel) -> bool {
pub(crate) fn is_pytest_raises(func: &Expr, semantic: &SemanticModel) -> bool {
semantic
.resolve_qualified_name(func)
.is_some_and(|qualified_name| matches!(qualified_name.segments(), ["pytest", "raises"]))
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 @@ -415,6 +415,7 @@ mod tests {
#[test_case(Rule::UnnecessaryRegularExpression, Path::new("RUF055_0.py"))]
#[test_case(Rule::UnnecessaryRegularExpression, Path::new("RUF055_1.py"))]
#[test_case(Rule::UnnecessaryCastToInt, Path::new("RUF046.py"))]
#[test_case(Rule::PytestRaisesAmbiguousPattern, Path::new("RUF043.py"))]
fn preview_rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!(
"preview__{}_{}",
Expand Down
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 @@ -23,6 +23,7 @@ pub(crate) use never_union::*;
pub(crate) use none_not_at_end_of_union::*;
pub(crate) use parenthesize_chained_operators::*;
pub(crate) use post_init_default::*;
pub(crate) use pytest_raises_ambiguous_pattern::*;
pub(crate) use quadratic_list_summation::*;
pub(crate) use redirected_noqa::*;
pub(crate) use redundant_bool_literal::*;
Expand Down Expand Up @@ -71,6 +72,7 @@ mod never_union;
mod none_not_at_end_of_union;
mod parenthesize_chained_operators;
mod post_init_default;
mod pytest_raises_ambiguous_pattern;
mod quadratic_list_summation;
mod redirected_noqa;
mod redundant_bool_literal;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
use crate::checkers::ast::Checker;
use crate::rules::flake8_pytest_style::rules::is_pytest_raises;
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, ViolationMetadata};
use ruff_python_ast as ast;

/// ## What it does
/// Checks for non-raw literal string arguments passed to the `match` parameter
/// of `pytest.raises()` where the string contains at least one unescaped
/// regex metacharacter.
///
/// ## Why is this bad?
/// The `match` argument is implicitly converted to a regex under the hood.
/// It should be made explicit whether the string is meant to be a regex or a "plain" pattern
/// by prefixing the string with the `r` suffix, escaping the metacharacter(s)
/// in the string using backslashes, or wrapping the entire string in a call to
/// `re.escape()`.
///
/// ## Example
///
/// ```python
/// import pytest
///
///
/// with pytest.raises(Exception, match="A full sentence."):
/// do_thing_that_raises()
/// ```
///
/// Use instead:
///
/// ```python
/// import pytest
///
///
/// with pytest.raises(Exception, match=r"A full sentence."):
/// do_thing_that_raises()
/// ```
///
/// Alternatively:
///
/// ```python
/// import pytest
/// import re
///
///
/// with pytest.raises(Exception, match=re.escape("A full sentence.")):
/// do_thing_that_raises()
/// ```
///
/// or:
///
/// ```python
/// import pytest
/// import re
///
///
/// with pytest.raises(Exception, "A full sentence\\."):
/// do_thing_that_raises()
/// ```
///
/// ## References
/// - [Python documentation: `re.escape`](https://docs.python.org/3/library/re.html#re.escape)
/// - [`pytest` documentation: `pytest.raises`](https://docs.pytest.org/en/latest/reference/reference.html#pytest-raises)
#[derive(ViolationMetadata)]
pub(crate) struct PytestRaisesAmbiguousPattern;

impl Violation for PytestRaisesAmbiguousPattern {
#[derive_message_formats]
fn message(&self) -> String {
"Pattern passed to `match=` contains metacharacters but is neither escaped nor raw"
.to_string()
}

fn fix_title(&self) -> Option<String> {
Some("Use a raw string or `re.escape()` to make the intention explicit".to_string())
}
}

/// RUF043
pub(crate) fn pytest_raises_ambiguous_pattern(checker: &mut Checker, call: &ast::ExprCall) {
if !is_pytest_raises(&call.func, checker.semantic()) {
return;
}

// It *can* be passed as a positional argument if you try very hard,
// but pytest only documents it as a keyword argument, and it's quite hard pass it positionally
let Some(ast::Keyword { value, .. }) = call.arguments.find_keyword("match") else {
return;
};

let ast::Expr::StringLiteral(string) = value else {
return;
};

let any_part_is_raw = string.value.iter().any(|part| part.flags.prefix().is_raw());

if any_part_is_raw || !string_has_unescaped_metacharacters(&string.value) {
return;
}

let diagnostic = Diagnostic::new(PytestRaisesAmbiguousPattern, string.range);

checker.diagnostics.push(diagnostic);
}

fn string_has_unescaped_metacharacters(value: &ast::StringLiteralValue) -> bool {
let mut escaped = false;

for character in value.chars() {
if escaped {
if escaped_char_is_regex_metasequence(character) {
return true;
}

escaped = false;
continue;
}

if character == '\\' {
escaped = true;
continue;
}

if char_is_regex_metacharacter(character) {
return true;
}
}

false
}

/// Whether the sequence `\<c>` means anything special:
///
/// * `\A`: Start of input
/// * `\b`, `\B`: Word boundary and non-word-boundary
/// * `\d`, `\D`: Digit and non-digit
/// * `\s`, `\S`: Whitespace and non-whitespace
/// * `\w`, `\W`: Word and non-word character
/// * `\z`: End of input
///
/// `\u`, `\U`, `\N`, `\x`, `\a`, `\f`, `\n`, `\r`, `\t`, `\v`
/// are also valid in normal strings and thus do not count.
/// `\b` means backspace only in character sets,
/// while backreferences (e.g., `\1`) are not valid without groups,
/// both of which should be caught in [`string_has_unescaped_metacharacters`].
const fn escaped_char_is_regex_metasequence(c: char) -> bool {
matches!(c, 'A' | 'b' | 'B' | 'd' | 'D' | 's' | 'S' | 'w' | 'W' | 'z')
}

const fn char_is_regex_metacharacter(c: char) -> bool {
matches!(
c,
'.' | '^' | '$' | '*' | '+' | '?' | '{' | '[' | '\\' | '|' | '('
)
}
Loading

0 comments on commit ac81c72

Please sign in to comment.