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-type-checking: Always recognise relative imports as first-party #12994

Merged
merged 1 commit into from
Aug 19, 2024

Conversation

AlexWaygood
Copy link
Member

A small refactor. The exact number of dots at the beginning of a relative import isnt relevant to the isort classification (and never will be). All that matters is whether there are any dots at the beginning or not. Using a boolean here improves readability and makes it clearer what the argument is for when the function is called from other rules elsewhere in Ruff.

In the process, this fixes what I believe is a small bug in the flake8-type-checking code, where we're always passing level=0 into the categorize() function. I think this means we may be miscategorising some imports there which would otherwise be categorised as first-party imports due to them being relative imports.

@AlexWaygood AlexWaygood added the isort Related to import sorting label Aug 19, 2024
Copy link
Contributor

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@AlexWaygood AlexWaygood marked this pull request as ready for review August 19, 2024 17:35
@AlexWaygood AlexWaygood added the internal An internal refactor or improvement label Aug 19, 2024
@MichaReiser
Copy link
Member

I guess we can't mark it as internal if it fixes a bug. It's important for users to be aware of this when looking at the changelog

@AlexWaygood
Copy link
Member Author

I guess we can't mark it as internal if it fixes a bug. It's important for users to be aware of this when looking at the changelog

It felt unlikely that it would actually come up in practice, and the ecosystem report strengthened my supposition there, so I wondered if it was really worth putting it in the changelog... but definitely don't feel strongly

@AlexWaygood AlexWaygood added rule Implementing or modifying a lint rule and removed internal An internal refactor or improvement isort Related to import sorting labels Aug 19, 2024
@AlexWaygood AlexWaygood changed the title Pass a boolean is_relative argument to isort::categorize, rather than a u32 level argument flake8-type-checking: Always recognise relative imports as first-party Aug 19, 2024
Copy link
Member

@MichaReiser MichaReiser left a comment

Choose a reason for hiding this comment

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

I like it.

I would be inclined to even use an enum with a Relative variant to improve readability on the call site but what you have here is good with me too.

@AlexWaygood AlexWaygood merged commit 049cda2 into main Aug 19, 2024
20 checks passed
@AlexWaygood AlexWaygood deleted the alex/cleanup-isort branch August 19, 2024 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants