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

Change default for PT001 and PT023 #12838

Merged
merged 3 commits into from
Aug 13, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ linter.flake8_import_conventions.aliases = {
}
linter.flake8_import_conventions.banned_aliases = {}
linter.flake8_import_conventions.banned_from = []
linter.flake8_pytest_style.fixture_parentheses = true
linter.flake8_pytest_style.fixture_parentheses = false
linter.flake8_pytest_style.parametrize_names_type = tuple
linter.flake8_pytest_style.parametrize_values_type = list
linter.flake8_pytest_style.parametrize_values_row_type = tuple
Expand All @@ -278,7 +278,7 @@ linter.flake8_pytest_style.raises_require_match_for = [
socket.error,
]
linter.flake8_pytest_style.raises_extend_require_match_for = []
linter.flake8_pytest_style.mark_parentheses = true
linter.flake8_pytest_style.mark_parentheses = false
linter.flake8_quotes.inline_quotes = double
linter.flake8_quotes.multiline_quotes = double
linter.flake8_quotes.docstring_quotes = double
Expand Down
8 changes: 4 additions & 4 deletions crates/ruff_linter/src/rules/flake8_pytest_style/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ mod tests {
Rule::PytestFixtureIncorrectParenthesesStyle,
Path::new("PT001.py"),
Settings {
fixture_parentheses: false,
fixture_parentheses: true,
..Settings::default()
},
"PT001_no_parentheses"
"PT001_parentheses"
)]
#[test_case(
Rule::PytestFixturePositionalArgs,
Expand Down Expand Up @@ -252,10 +252,10 @@ mod tests {
Rule::PytestIncorrectMarkParenthesesStyle,
Path::new("PT023.py"),
Settings {
mark_parentheses: false,
mark_parentheses: true,
..Settings::default()
},
"PT023_no_parentheses"
"PT023_parentheses"
)]
#[test_case(
Rule::PytestUnnecessaryAsyncioMarkOnFixture,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,9 @@ use super::helpers::{
/// optional.
///
/// Either removing those unnecessary parentheses _or_ requiring them for all
/// fixtures is fine, but it's best to be consistent.
///
/// In [preview], this rule defaults to removing unnecessary parentheses, to match
/// the behavior of official pytest projects.
/// fixtures is fine, but it's best to be consistent. The rule defaults to
/// removing unnecessary parentheses, to match the documentation of the
/// official pytest projects.
///
/// ## Example
///
Expand All @@ -62,8 +61,6 @@ use super::helpers::{
///
/// ## References
/// - [`pytest` documentation: API Reference: Fixtures](https://docs.pytest.org/en/latest/reference/reference.html#fixtures-api)
///
/// [preview]: https://docs.astral.sh/ruff/preview/
#[violation]
pub struct PytestFixtureIncorrectParenthesesStyle {
expected: Parentheses,
Expand Down Expand Up @@ -930,9 +927,7 @@ pub(crate) fn fixture(
check_fixture_decorator(checker, name, decorator);
}

if checker.enabled(Rule::PytestDeprecatedYieldFixture)
&& checker.settings.flake8_pytest_style.fixture_parentheses
Copy link
Member Author

Choose a reason for hiding this comment

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

I think that was a bug. It's unclear to me why this rule would depend on enabling that setting.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, that's odd. I tried digging through blame but the best I can conclude without spending too much time on it is that it's been this way for a long time xD

{
if checker.enabled(Rule::PytestDeprecatedYieldFixture) {
check_fixture_decorator_name(checker, decorator);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ use super::helpers::get_mark_decorators;
/// without parentheses, depending on the [`lint.flake8-pytest-style.mark-parentheses`]
/// setting.
MichaReiser marked this conversation as resolved.
Show resolved Hide resolved
///
/// In [preview], this rule defaults to removing unnecessary parentheses, to match
/// the behavior of official pytest projects.
/// The rule defaults to removing unnecessary parentheses,
/// to match the documentation of the official pytest projects.
///
/// ## Why is this bad?
/// If a `@pytest.mark.<marker>()` doesn't take any arguments, the parentheses are
Expand Down Expand Up @@ -49,8 +49,6 @@ use super::helpers::get_mark_decorators;
///
/// ## References
/// - [`pytest` documentation: Marks](https://docs.pytest.org/en/latest/reference/reference.html#marks)
///
/// [preview]: https://docs.astral.sh/ruff/preview/
#[violation]
pub struct PytestIncorrectMarkParenthesesStyle {
mark_name: String,
Expand Down
20 changes: 3 additions & 17 deletions crates/ruff_linter/src/rules/flake8_pytest_style/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use std::fmt::Formatter;
use crate::display_settings;
use ruff_macros::CacheKey;

use crate::settings::types::{IdentifierPattern, PreviewMode};
use crate::settings::types::IdentifierPattern;

use super::types;

Expand Down Expand Up @@ -38,27 +38,13 @@ pub struct Settings {
impl Default for Settings {
fn default() -> Self {
Self {
fixture_parentheses: true,
fixture_parentheses: false,
parametrize_names_type: types::ParametrizeNameType::default(),
parametrize_values_type: types::ParametrizeValuesType::default(),
parametrize_values_row_type: types::ParametrizeValuesRowType::default(),
raises_require_match_for: default_broad_exceptions(),
raises_extend_require_match_for: vec![],
mark_parentheses: true,
}
}
}

impl Settings {
pub fn resolve_default(preview: PreviewMode) -> Self {
if preview.is_enabled() {
Self {
fixture_parentheses: false,
mark_parentheses: false,
..Default::default()
}
} else {
Self::default()
mark_parentheses: false,
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,61 +1,127 @@
---
source: crates/ruff_linter/src/rules/flake8_pytest_style/mod.rs
---
PT001.py:9:1: PT001 [*] Use `@pytest.fixture()` over `@pytest.fixture`
PT001.py:14:1: PT001 [*] Use `@pytest.fixture` over `@pytest.fixture()`
|
9 | @pytest.fixture
| ^^^^^^^^^^^^^^^ PT001
10 | def no_parentheses():
11 | return 42
14 | @pytest.fixture()
| ^^^^^^^^^^^^^^^^^ PT001
15 | def parentheses_no_params():
16 | return 42
|
= help: Add parentheses
= help: Remove parentheses

ℹ Safe fix
6 6 | # `import pytest`
7 7 |
8 8 |
9 |[email protected]
9 |[email protected]()
10 10 | def no_parentheses():
11 11 | return 42
12 12 |
13 13 |
14 |[email protected]()
14 |[email protected]
15 15 | def parentheses_no_params():
16 16 | return 42
17 17 |

PT001.py:34:1: PT001 [*] Use `@pytest.fixture()` over `@pytest.fixture`
PT001.py:24:1: PT001 [*] Use `@pytest.fixture` over `@pytest.fixture()`
|
34 | @fixture
| ^^^^^^^^ PT001
35 | def imported_from_no_parentheses():
36 | return 42
24 | / @pytest.fixture(
25 | |
26 | | )
| |_^ PT001
27 | def parentheses_no_params_multiline():
28 | return 42
|
= help: Add parentheses
= help: Remove parentheses

ℹ Safe fix
21 21 | return 42
22 22 |
23 23 |
24 |[email protected](
25 |-
26 |-)
24 |[email protected]
27 25 | def parentheses_no_params_multiline():
28 26 | return 42
29 27 |

PT001.py:39:1: PT001 [*] Use `@pytest.fixture` over `@pytest.fixture()`
|
39 | @fixture()
| ^^^^^^^^^^ PT001
40 | def imported_from_parentheses_no_params():
41 | return 42
|
= help: Remove parentheses

ℹ Safe fix
31 31 | # `from pytest import fixture`
32 32 |
33 33 |
34 |-@fixture
34 |+@fixture()
35 35 | def imported_from_no_parentheses():
36 36 | return 42
37 37 |
38 38 |
39 |-@fixture()
39 |+@fixture
40 40 | def imported_from_parentheses_no_params():
41 41 | return 42
42 42 |

PT001.py:59:1: PT001 [*] Use `@pytest.fixture()` over `@pytest.fixture`
PT001.py:49:1: PT001 [*] Use `@pytest.fixture` over `@pytest.fixture()`
|
59 | @aliased
| ^^^^^^^^ PT001
60 | def aliased_no_parentheses():
61 | return 42
49 | / @fixture(
50 | |
51 | | )
| |_^ PT001
52 | def imported_from_parentheses_no_params_multiline():
53 | return 42
|
= help: Add parentheses
= help: Remove parentheses

ℹ Safe fix
46 46 | return 42
47 47 |
48 48 |
49 |-@fixture(
50 |-
51 |-)
49 |+@fixture
52 50 | def imported_from_parentheses_no_params_multiline():
53 51 | return 42
54 52 |

PT001.py:64:1: PT001 [*] Use `@pytest.fixture` over `@pytest.fixture()`
|
64 | @aliased()
| ^^^^^^^^^^ PT001
65 | def aliased_parentheses_no_params():
66 | return 42
|
= help: Remove parentheses

ℹ Safe fix
56 56 | # `from pytest import fixture as aliased`
57 57 |
58 58 |
59 |-@aliased
59 |+@aliased()
60 60 | def aliased_no_parentheses():
61 61 | return 42
62 62 |
63 63 |
64 |-@aliased()
64 |+@aliased
65 65 | def aliased_parentheses_no_params():
66 66 | return 42
67 67 |

PT001.py:74:1: PT001 [*] Use `@pytest.fixture` over `@pytest.fixture()`
|
74 | / @aliased(
75 | |
76 | | )
| |_^ PT001
77 | def aliased_parentheses_no_params_multiline():
78 | return 42
|
= help: Remove parentheses

ℹ Safe fix
71 71 | return 42
72 72 |
73 73 |
74 |-@aliased(
75 |-
76 |-)
74 |+@aliased
77 75 | def aliased_parentheses_no_params_multiline():
78 76 | return 42
Loading
Loading