Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 1 addition & 31 deletions crates/ruff_linter/src/rules/flake8_pytest_style/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ mod tests {
use test_case::test_case;

use crate::registry::Rule;
use crate::settings::types::{IdentifierPattern, PreviewMode};
use crate::settings::types::IdentifierPattern;
use crate::test::test_path;
use crate::{assert_messages, settings};

Expand Down Expand Up @@ -358,36 +358,6 @@ mod tests {
Ok(())
}

#[test_case(
Rule::PytestRaisesWithMultipleStatements,
Path::new("PT012.py"),
Settings::default(),
"PT012_preview"
)]
#[test_case(
Rule::PytestWarnsWithMultipleStatements,
Path::new("PT031.py"),
Settings::default(),
"PT031_preview"
)]
fn test_pytest_style_preview(
rule_code: Rule,
path: &Path,
plugin_settings: Settings,
name: &str,
) -> Result<()> {
let diagnostics = test_path(
Path::new("flake8_pytest_style").join(path).as_path(),
&settings::LinterSettings {
preview: PreviewMode::Enabled,
flake8_pytest_style: plugin_settings,
..settings::LinterSettings::for_rule(rule_code)
},
)?;
assert_messages!(name, diagnostics);
Ok(())
}

/// This test ensure that PT006 and PT007 don't conflict when both of them suggest a fix that
/// edits `argvalues` for `pytest.mark.parametrize`.
#[test]
Expand Down
14 changes: 5 additions & 9 deletions crates/ruff_linter/src/rules/flake8_pytest_style/rules/raises.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,17 @@ use super::helpers::is_empty_or_null_string;
/// ## What it does
/// Checks for `pytest.raises` context managers with multiple statements.
///
/// This rule allows `pytest.raises` bodies to contain `for`
/// loops with empty bodies (e.g., `pass` or `...` statements), to test
/// iterator behavior.
///
/// ## Why is this bad?
/// When a `pytest.raises` is used as a context manager and contains multiple
/// statements, it can lead to the test passing when it actually should fail.
///
/// A `pytest.raises` context manager should only contain a single simple
/// statement that raises the expected exception.
///
/// In [preview], this rule allows `pytest.raises` bodies to contain `for`
/// loops with empty bodies (e.g., `pass` or `...` statements), to test
/// iterator behavior.
///
/// ## Example
/// ```python
/// import pytest
Expand All @@ -50,8 +50,6 @@ use super::helpers::is_empty_or_null_string;
///
/// ## References
/// - [`pytest` documentation: `pytest.raises`](https://docs.pytest.org/en/latest/reference/reference.html#pytest-raises)
///
/// [preview]: https://docs.astral.sh/ruff/preview/
#[derive(ViolationMetadata)]
pub(crate) struct PytestRaisesWithMultipleStatements;

Expand Down Expand Up @@ -206,14 +204,12 @@ pub(crate) fn complex_raises(checker: &Checker, stmt: &Stmt, items: &[WithItem],
// Check body for `pytest.raises` context manager
if raises_called {
let is_too_complex = if let [stmt] = body {
let in_preview = checker.settings.preview.is_enabled();

match stmt {
Stmt::With(ast::StmtWith { body, .. }) => is_non_trivial_with_body(body),
// Allow function and class definitions to test decorators.
Stmt::ClassDef(_) | Stmt::FunctionDef(_) => false,
// Allow empty `for` loops to test iterators.
Stmt::For(ast::StmtFor { body, .. }) if in_preview => match &body[..] {
Stmt::For(ast::StmtFor { body, .. }) => match &body[..] {
[Stmt::Pass(_)] => false,
[Stmt::Expr(ast::StmtExpr { value, .. })] => !value.is_ellipsis_literal_expr(),
_ => true,
Expand Down
13 changes: 5 additions & 8 deletions crates/ruff_linter/src/rules/flake8_pytest_style/rules/warns.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,17 @@ use super::helpers::is_empty_or_null_string;
/// ## What it does
/// Checks for `pytest.warns` context managers with multiple statements.
///
/// This rule allows `pytest.warns` bodies to contain `for`
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, I think it made sense to move these up to ## What it does too.

/// loops with empty bodies (e.g., `pass` or `...` statements), to test
/// iterator behavior.
///
/// ## Why is this bad?
/// When `pytest.warns` is used as a context manager and contains multiple
/// statements, it can lead to the test passing when it should instead fail.
///
/// A `pytest.warns` context manager should only contain a single
/// simple statement that triggers the expected warning.
///
/// In [preview], this rule allows `pytest.warns` bodies to contain `for`
/// loops with empty bodies (e.g., `pass` or `...` statements), to test
/// iterator behavior.
///
/// ## Example
/// ```python
Expand All @@ -48,8 +49,6 @@ use super::helpers::is_empty_or_null_string;
///
/// ## References
/// - [`pytest` documentation: `pytest.warns`](https://docs.pytest.org/en/latest/reference/reference.html#pytest-warns)
///
/// [preview]: https://docs.astral.sh/ruff/preview/
#[derive(ViolationMetadata)]
pub(crate) struct PytestWarnsWithMultipleStatements;

Expand Down Expand Up @@ -206,14 +205,12 @@ pub(crate) fn complex_warns(checker: &Checker, stmt: &Stmt, items: &[WithItem],
// Check body for `pytest.warns` context manager
if warns_called {
let is_too_complex = if let [stmt] = body {
let in_preview = checker.settings.preview.is_enabled();

match stmt {
Stmt::With(ast::StmtWith { body, .. }) => is_non_trivial_with_body(body),
// Allow function and class definitions to test decorators.
Stmt::ClassDef(_) | Stmt::FunctionDef(_) => false,
// Allow empty `for` loops to test iterators.
Stmt::For(ast::StmtFor { body, .. }) if in_preview => match &body[..] {
Stmt::For(ast::StmtFor { body, .. }) => match &body[..] {
[Stmt::Pass(_)] => false,
[Stmt::Expr(ast::StmtExpr { value, .. })] => !value.is_ellipsis_literal_expr(),
_ => true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,49 +124,3 @@ PT012.py:95:5: PT012 `pytest.raises()` block should contain a single simple stat
97 | | assert foo
| |______________________^ PT012
|

PT012.py:102:5: PT012 `pytest.raises()` block should contain a single simple statement
|
100 | ## No errors in preview
101 |
102 | / with pytest.raises(RuntimeError):
103 | | for a in b:
104 | | pass
| |________________^ PT012
105 |
106 | with pytest.raises(RuntimeError):
|

PT012.py:106:5: PT012 `pytest.raises()` block should contain a single simple statement
|
104 | pass
105 |
106 | / with pytest.raises(RuntimeError):
107 | | for a in b:
108 | | ...
| |_______________^ PT012
109 |
110 | with pytest.raises(RuntimeError):
|

PT012.py:110:5: PT012 `pytest.raises()` block should contain a single simple statement
|
108 | ...
109 |
110 | / with pytest.raises(RuntimeError):
111 | | async for a in b:
112 | | pass
| |________________^ PT012
113 |
114 | with pytest.raises(RuntimeError):
|

PT012.py:114:5: PT012 `pytest.raises()` block should contain a single simple statement
|
112 | pass
113 |
114 | / with pytest.raises(RuntimeError):
115 | | async for a in b:
116 | | ...
| |_______________^ PT012
|

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -124,49 +124,3 @@ PT031.py:95:5: PT031 `pytest.warns()` block should contain a single simple state
97 | | assert foo
| |______________________^ PT031
|

PT031.py:102:5: PT031 `pytest.warns()` block should contain a single simple statement
|
100 | ## No errors in preview
101 |
102 | / with pytest.warns(RuntimeError):
103 | | for a in b:
104 | | pass
| |________________^ PT031
105 |
106 | with pytest.warns(RuntimeError):
|

PT031.py:106:5: PT031 `pytest.warns()` block should contain a single simple statement
|
104 | pass
105 |
106 | / with pytest.warns(RuntimeError):
107 | | for a in b:
108 | | ...
| |_______________^ PT031
109 |
110 | with pytest.warns(RuntimeError):
|

PT031.py:110:5: PT031 `pytest.warns()` block should contain a single simple statement
|
108 | ...
109 |
110 | / with pytest.warns(RuntimeError):
111 | | async for a in b:
112 | | pass
| |________________^ PT031
113 |
114 | with pytest.warns(RuntimeError):
|

PT031.py:114:5: PT031 `pytest.warns()` block should contain a single simple statement
|
112 | pass
113 |
114 | / with pytest.warns(RuntimeError):
115 | | async for a in b:
116 | | ...
| |_______________^ PT031
|
Loading
Loading