Content-Length: 304903 | pFad | https://redirect.github.com/astral-sh/ruff/pull/16050

2B [`flake8-datetime`] Ignore `.replace()` calls while looking for `.astimezone` by InSyncWithFoo · Pull Request #16050 · astral-sh/ruff · GitHub
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

[flake8-datetime] Ignore .replace() calls while looking for .astimezone #16050

Merged
merged 4 commits into from
Feb 9, 2025
Merged

[flake8-datetime] Ignore .replace() calls while looking for .astimezone #16050

merged 4 commits into from
Feb 9, 2025

Conversation

InSyncWithFoo
Copy link
Contributor

@InSyncWithFoo InSyncWithFoo commented Feb 9, 2025 •
edited
Loading

Summary

Resolves #15998.

Previously, this would be considered an error by DTZ005, as parent_expr_is_astimezone() would only traverse up one level:

datetime.now().replace(...).astimezone()

The function will now check for all expressions in the current stack, ignoring all such intermediate .replace() calls while looking for .astimezone.

Test Plan

cargo nextest run and cargo insta test.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Feb 9, 2025 •
edited
Loading

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@InSyncWithFoo
Copy link
Contributor Author

The second commit is just a rename. All substantial changes are isolated within the first.

AlexWaygood
AlexWaygood reviewed Feb 9, 2025
View reviewed 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.

Thank you, this overall looks great. Nicely done!

Thank you for adding comments to the code; it made it easier to understand what was going on. It still took me some time to figure out what was going on here, though. I almost wonder if it's worth adding a comment that links to the Ruff playground showing what the AST we're dealing with here looks like, and explaining that we start at the innermost node in the AST and have to work our way upwards https://play.ruff.rs/89b67ffb-c32a-437e-aa8c-8dcfc8b7068a

@AlexWaygood
Copy link
Member

OK, I added some documentation for you.

@AlexWaygood AlexWaygood added the bug Something isn't working label Feb 9, 2025
@AlexWaygood AlexWaygood enabled auto-merge (squash) February 9, 2025 15:47
@AlexWaygood AlexWaygood merged commit a46fbda into astral-sh:main Feb 9, 2025
20 checks passed
@InSyncWithFoo InSyncWithFoo deleted the DTZ005 branch February 9, 2025 15:52
@BrewTestBot BrewTestBot mentioned this pull request Feb 10, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AlexWaygood AlexWaygood AlexWaygood approved these changes

Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

DTZ005: false positive when replace(...) before astimezone()
2 participants
@InSyncWithFoo @AlexWaygood








ApplySandwichStrip

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier!      Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

Fetched URL: https://redirect.github.com/astral-sh/ruff/pull/16050

Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy