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.

Sign up for GitHub

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

Jump to bottom

Change default for PT001 and PT023 #12838

Merged
merged 3 commits into from
Aug 13, 2024
Merged

Change default for PT001 and PT023 #12838

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ae8fe83
Change default for PT001 and PT023
MichaReiser Aug 12, 2024
2af4d3f
Update crates/ruff_linter/src/rules/flake8_pytest_style/rules/fixture.rs
MichaReiser Aug 12, 2024
fad87d4
Update crates/ruff_linter/src/rules/flake8_pytest_style/rules/marks.rs
MichaReiser Aug 12, 2024
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
4 changes: 2 additions & 2 deletions crates/ruff/tests/snapshots/show_settings__display_default_settings.snap
View file
Open in desktop
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
View file
Open in desktop
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
9 changes: 1 addition & 8 deletions crates/ruff_linter/src/rules/flake8_pytest_style/rules/fixture.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,6 @@ use super::helpers::{
/// Either removing those unnecessary parentheses _or_ requiring them for all
/// fixtures is fine, but it's best to be consistent.
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.
///
/// ## Example
///
/// ```python
Expand All @@ -62,8 +59,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 +925,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
5 changes: 0 additions & 5 deletions crates/ruff_linter/src/rules/flake8_pytest_style/rules/marks.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,6 @@ 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.
///
/// ## Why is this bad?
/// If a `@pytest.mark.<marker>()` doesn't take any arguments, the parentheses are
/// optional.
Expand Down Expand Up @@ -49,8 +46,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
View file
Open in desktop
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
138 changes: 102 additions & 36 deletions ...pytest_style/snapshots/ruff_linter__rules__flake8_pytest_style__tests__PT001_default.snap
View file
Open in desktop
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 |-@pytest.fixture
9 |+@pytest.fixture()
10 10 | def no_parentheses():
11 11 | return 42
12 12 |
13 13 |
14 |-@pytest.fixture()
14 |+@pytest.fixture
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 |-@pytest.fixture(
25 |-
26 |-)
24 |+@pytest.fixture
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