ruff icon indicating copy to clipboard operation
ruff copied to clipboard

[relative-imports] Add setting to require relative imports

Open genevieve-me opened this issue 5 months ago • 3 comments

Summary

Adds an 'always relative' import style setting to flake8-tidy-imports, closing #4188 .

The PR changes the flake8-tidy-imports TID252 rule and its ban-relative-imports setting to allow forcing imports to be always absolute, relative, or absolute for parents.

Since the rule used to just configure the maximum level of relative import (either never allowing relative or allowing at the same level), I renamed the rule from ban-relative-imports to relative-import-style. I also made some internal name changes, e.g., relative import Strictness became relative import ImportStyle (perhaps a slightly redundant name), and diagnostics were updated.

The implementation relies on the categorize function used for the isort rule to determine if absolute imports are first party and therefore could be relative.

NOTE: I realize now that this implementation is wrong, which is why this PR is marked as draft. First party doesn't mean 'can be relative,' as users can configure separate packages to be first party with isort.known-first-party but they can't be imported relatively. I'll have to take a closer look at the ImportType options and see which one applies to same-package imports that could become from .bar import baz instead of from foo.bar import baz.

Settings

I also presume that the way this PR naively renames the setting is a big no-no for breaking configs, especially since TID252 is not in preview. I actually couldn't find any examples of renaming settings in the changelogs: presumably there should be some sort of deprecation period.

Getting feedback on the settings is the main reason I'm opening the PR even in its incomplete state. The renaming seems necessary, because:

  • The initial feedback on the feature request said "We could probably add support for this as a configurable setting in TID252"
  • The existing setting can cleanly cover all three cases, so adding a second one and having two overlapping settings for this rule would seem odd
  • However, setting ban-relative-imports to force relative imports would be confusing. Even something like ban-relative-imports = 'none', while in keeping with the current rules, doesn't make it clear that absolute imports are actually being banned.

Test Plan

Right now, I updated the existing tests, but I still need to add new tests where absolute imports trigger diagnostics if appropriate (they are in the same package namespace and could be relative).

I also want to look into the performance. It feels redundant to re-categorize all imports when they're already being categorized elsewhere for the isort rule, but we can't assume that the isort rule is enabled and we can get the information from there. Maybe we could categorize them all at a higher level and pass that information to the relevant places, but that could also add unnecessary complexity?

genevieve-me avatar Aug 18 '25 16:08 genevieve-me

Apologies for the delay here. I just wanted to say thanks for your work on this, and that I do intend to take a closer look at some point!

ntBre avatar Sep 03 '25 14:09 ntBre

I would personally love for something to enforce/convert modules using relative imports, because nothing bugs me more than using relative imports everywhere, and then VSCode's auto-import code actions creating absolute imports, even from modules that are already using relative imports, and only noticing once tests fail or a release gets flagged for breaking something. 😅

amyreese avatar Dec 16 '25 18:12 amyreese

Hey Brent, thanks for taking a look and for the guidance on the preview approach. I'll be able to take a closer look over the weekend.

genevieve-me avatar Dec 17 '25 00:12 genevieve-me