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

[ruff] Implement post-init-default (RUF033) #13192

Merged
merged 10 commits into from
Sep 2, 2024

Conversation

tjkuson
Copy link
Contributor

@tjkuson tjkuson commented Sep 1, 2024

Summary

Implements the post-init-default (RUF033) rule, which reports a diagnostic upon any __post_init__ method with argument default. Has a sometimes available unsafe autofix. For example,

 from dataclasses import InitVar, dataclass


 @dataclass
 class Foo:
     """A very helpful docstring."""

+    baz: InitVar[str] = "something"
     bar: InitVar[int] = 1

-    def __post_init__(self, bar: int, baz: str = "something") -> None: ...
+    def __post_init__(self, bar: int, baz: str) -> None: ...

Closes #13128

Test Plan

cargo nextest run

Copy link
Contributor

github-actions bot commented Sep 1, 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.

Sorry for reviewing a draft PR... couldn't resist! Looks great.

Copy link

codspeed-hq bot commented Sep 1, 2024

CodSpeed Performance Report

Merging #13192 will improve performances by 5.89%

Comparing tjkuson:post-init-defaults (7b21ade) with main (0f85769)

Summary

⚡ 1 improvements
✅ 31 untouched benchmarks

Benchmarks breakdown

Benchmark main tjkuson:post-init-defaults Change
linter/all-with-preview-rules[numpy/globals.py] 866.2 µs 818 µs +5.89%

@tjkuson tjkuson marked this pull request as ready for review September 1, 2024 18:23
@tjkuson tjkuson changed the title Implement post-init-default (RUF033) [ruff] Implement post-init-default (RUF033) Sep 1, 2024
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.

Thanks! I pushed a couple of simplifications, but this was overall an excellent PR

}

/// RUF033
pub(crate) fn post_init_default(checker: &mut Checker, function_def: &ast::StmtFunctionDef) {
Copy link
Member

Choose a reason for hiding this comment

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

I made this run on StmtFunctionDef nodes rather than StmtClassDef nodes, as this allows us to access checker.semantic().current_scope() to see what other symbols have been bound in the class. This enables us to see much more simply and more accurately whether it's safe to add a new InitVar field to the class body in the fix

Comment on lines +182 to +185
let initvar_edit = Edit::insertion(
content.into_owned(),
locator.line_start(post_init_def.start()),
);
Copy link
Member

Choose a reason for hiding this comment

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

Rather than iterate through all statements in the class body until we get the first non-docstring statement, I changed it so that we just insert the InitVar symbol in the line before the __post_init__ definition. It seems much simpler and less error-prone as we already know that we're past the docstring in this case

@MichaReiser MichaReiser added rule Implementing or modifying a lint rule preview Related to preview mode features labels Sep 2, 2024
@AlexWaygood AlexWaygood merged commit ea0246c into astral-sh:main Sep 2, 2024
20 checks passed
@tjkuson tjkuson deleted the post-init-defaults branch September 2, 2024 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
preview Related to preview mode features rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rule request: Warn whenever any default value is provided in __post_init__
3 participants