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

[flake8-pytest-style] Fix defaults when lint.flake8-pytest-style config section is empty (PT001, PT023) #13292

Conversation

dizzy57
Copy link
Contributor

@dizzy57 dizzy57 commented Sep 9, 2024

Summary

The documentation states that lint.flake8-pytest-style.*-parentheses options default to false, but if the user provides an empty [lint.flake8-pytest-style] config section, those options are suddenly set to true.

This is a bug. After #12838 changed the defaults, Flake8PytestStyleOptions::try_into_settings wasn't updated and still relies on the preview mode to set the defaults.

Reproduction

% ruff --version
ruff 0.6.4

% cat example.py
import pytest


@pytest.fixture()
def parentheses(): ...


@pytest.fixture
def no_parentheses(): ...

% ruff check --select PT001 --output-format concise example.py
example.py:4:1: PT001 [*] Use `@pytest.fixture` over `@pytest.fixture()`
Found 1 error.
[*] 1 fixable with the `--fix` option.

% echo '[lint.flake8-pytest-style]' > ruff.toml

% ruff check --select PT001 --output-format concise example.py
example.py:8:1: PT001 [*] Use `@pytest.fixture()` over `@pytest.fixture`
Found 1 error.
[*] 1 fixable with the `--fix` option.

Test Plan

Manual testing, see reproduction section.

% ./target/debug/ruff check --select PT001 --output-format concise example.py
warning: Detected debug build without --no-cache.
example.py:4:1: PT001 [*] Use `@pytest.fixture` over `@pytest.fixture()`
Found 1 error.
[*] 1 fixable with the `--fix` option.

% echo '[lint.flake8-pytest-style]' > ruff.toml

% ./target/debug/ruff check --select PT001 --output-format concise example.py
warning: Detected debug build without --no-cache.
example.py:4:1: PT001 [*] Use `@pytest.fixture` over `@pytest.fixture()`
Found 1 error.
[*] 1 fixable with the `--fix` option.

Given the fact that this was the only reference to ruff_linter::settings::types::PreviewMode in crates/ruff_workspace/src/options.rs, looks like no other settings depend on the preview mode, and it doesn't make much sense to me to write any additional tests that verify there's no interaction between preview mode and other default values.

…yle.*-parentheses` config (`PT001`, `PT023`)
@dizzy57 dizzy57 changed the title [flake8-pytest-style] Fix defaults for missing lint.flake8-pytest-style.*-parentheses configs (PT001, PT023) [flake8-pytest-style] Fix defaults when lint.flake8-pytest-style config section is empty (PT001, PT023) Sep 9, 2024
Copy link
Contributor

github-actions bot commented Sep 9, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

LGTM! Sorry we didn't catch this, that's on us :-(

Unfortunately, however, this is will be a breaking change for some users, so we'll have to merge this as part of the 0.7 release. That should come at some point in the next few weeks, though; shouldn't be too long to wait :)

@AlexWaygood AlexWaygood added this to the v0.7 milestone Sep 9, 2024
@MichaReiser MichaReiser added breaking Breaking API change configuration Related to settings and configuration labels Sep 9, 2024
@AlexWaygood AlexWaygood changed the base branch from main to ruff-0.7 October 8, 2024 13:10
@AlexWaygood AlexWaygood merged commit 91161b4 into astral-sh:ruff-0.7 Oct 8, 2024
20 checks passed
@dizzy57 dizzy57 deleted the pytest-parentheses-style-options-default branch October 8, 2024 17:01
AlexWaygood pushed a commit that referenced this pull request Oct 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Breaking API change configuration Related to settings and configuration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants