[`flake8-tidy-imports`] Add `TID253` #6378

durumu · 2023-08-06T21:42:08Z

Summary

Add a new rule TID253 (banned-module-level-imports), to ban a user-specified list of imports from appearing at module level. This rule doesn't exist in flake8-tidy-imports, so it's unique to Ruff. The implementation is pretty similar to TID251.

Briefly discussed here.

Test Plan

Added a new test case, checking that inline imports are allowed and that non-inline imports from the banned list are disallowed.

github-actions · 2023-08-06T21:52:21Z

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

Benchmark

Linux

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.00      8.2±0.05ms     4.9 MB/sec    1.02      8.4±0.22ms     4.8 MB/sec
formatter/numpy/ctypeslib.py               1.00  1636.2±13.97µs    10.2 MB/sec    1.01  1647.7±29.88µs    10.1 MB/sec
formatter/numpy/globals.py                 1.00    186.4±3.82µs    15.8 MB/sec    1.01    187.5±5.40µs    15.7 MB/sec
formatter/pydantic/types.py                1.00      3.5±0.08ms     7.4 MB/sec    1.01      3.5±0.09ms     7.3 MB/sec
linter/all-rules/large/dataset.py          1.01     10.3±0.06ms     3.9 MB/sec    1.00     10.2±0.05ms     4.0 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      2.8±0.02ms     6.0 MB/sec    1.00      2.8±0.02ms     6.0 MB/sec
linter/all-rules/numpy/globals.py          1.00    390.6±0.58µs     7.6 MB/sec    1.00    392.1±0.64µs     7.5 MB/sec
linter/all-rules/pydantic/types.py         1.00      5.3±0.08ms     4.8 MB/sec    1.00      5.3±0.03ms     4.8 MB/sec
linter/default-rules/large/dataset.py      1.00      5.4±0.01ms     7.5 MB/sec    1.00      5.4±0.01ms     7.5 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00   1195.9±5.08µs    13.9 MB/sec    1.00   1201.8±6.51µs    13.9 MB/sec
linter/default-rules/numpy/globals.py      1.00    140.1±1.48µs    21.1 MB/sec    1.00    140.1±0.32µs    21.1 MB/sec
linter/default-rules/pydantic/types.py     1.00      2.4±0.01ms    10.4 MB/sec    1.01      2.5±0.00ms    10.4 MB/sec

Windows

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.00     12.1±0.36ms     3.4 MB/sec    1.00     12.2±0.32ms     3.3 MB/sec
formatter/numpy/ctypeslib.py               1.00      2.3±0.07ms     7.2 MB/sec    1.02      2.4±0.08ms     7.1 MB/sec
formatter/numpy/globals.py                 1.00   259.4±12.62µs    11.4 MB/sec    1.00   260.6±12.72µs    11.3 MB/sec
formatter/pydantic/types.py                1.00      5.0±0.13ms     5.1 MB/sec    1.01      5.0±0.15ms     5.1 MB/sec
linter/all-rules/large/dataset.py          1.00     15.1±0.35ms     2.7 MB/sec    1.02     15.4±0.32ms     2.6 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      4.2±0.09ms     3.9 MB/sec    1.00      4.2±0.12ms     3.9 MB/sec
linter/all-rules/numpy/globals.py          1.00   537.6±14.16µs     5.5 MB/sec    1.00   536.1±32.20µs     5.5 MB/sec
linter/all-rules/pydantic/types.py         1.00      7.9±0.19ms     3.2 MB/sec    1.02      8.1±0.21ms     3.1 MB/sec
linter/default-rules/large/dataset.py      1.02      8.4±0.16ms     4.9 MB/sec    1.00      8.2±0.22ms     5.0 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.01  1774.1±30.39µs     9.4 MB/sec    1.00  1758.6±65.03µs     9.5 MB/sec
linter/default-rules/numpy/globals.py      1.00    214.5±6.54µs    13.8 MB/sec    1.00    215.0±9.99µs    13.7 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.8±0.07ms     6.8 MB/sec    1.00      3.8±0.09ms     6.8 MB/sec

konstin · 2023-08-11T10:41:37Z

crates/ruff/src/rules/flake8_tidy_imports/rules/banned_module_level_imports.rs

+/// Some modules take a long time to import. Library authors might want to ensure that you only pay
+/// the import cost for these modules if you directly use them, rather than if you import a module
+/// that happens to use an expensive module in one of its functions.
+///


this needs an example

crates/ruff/src/rules/flake8_tidy_imports/rules/banned_module_level_imports.rs

durumu · 2023-08-12T00:55:22Z

Sorry for the ping @dhruvmanila, I don't think you need to review this one -- I accidentally included some extra commits when I pushed 😅 (first github PR!)

charliermarsh · 2023-08-12T04:18:36Z

Generally looks good, will try to get to this soon.

charliermarsh · 2023-08-12T04:20:00Z

crates/ruff/src/rules/flake8_tidy_imports/rules/banned_module_level_imports.rs

+) {
+    if !locator.is_at_start_of_line(stmt.start()) {
+        return;
+    }


I think more robust here would be to use checker.semantic().at_top_level(), which will base its decision on scoping and parent statements. It's possible to have statements that aren't at the start of the line, but are at the top level, like:

x = 1; import tensorflow

Thanks! I shamelessly copied this from the module_import_not_at_top_of_file rule (E402), so I think that will need to be fixed too.

Thanks for pointing that out, good call! Fixed in https://github.com/astral-sh/ruff/pull/6526/files.

## Summary Noticed in #6378. Given `import h; import i`, we don't consider `import i` to be a "top-level" import for E402 purposes, which is wrong. Similarly, we _do_ consider `import k` to be a "top-level" import in: ```python if __name__ == "__main__": import j; \ import k ``` Using the semantic detection, rather than relying on newline position, fixes both cases. ## Test Plan `cargo test`

…6527) ## Summary Generalizes the abstractions for name matching introduced in #6378 and applies them to the existing `banned_api` rule, such that both rules have a uniform API and implementation. ## Test Plan `cargo test`

add TID253: ban module level imports~

48b134c

konstin approved these changes Aug 11, 2023

View reviewed changes

konstin requested a review from charliermarsh August 11, 2023 10:45

durumu requested a review from dhruvmanila as a code owner August 12, 2023 00:38

Implement konstin's suggested improvements

04fa4e8

durumu force-pushed the durumu/bannedmodulelevel branch from d32b655 to 04fa4e8 Compare August 12, 2023 00:47

appease clippy

51f2af3

charliermarsh self-assigned this Aug 12, 2023

charliermarsh added the rule Implementing or modifying a lint rule label Aug 12, 2023

charliermarsh reviewed Aug 12, 2023

View reviewed changes

durumu and others added 2 commits August 12, 2023 09:00

handle module level imports that aren't at the start of the line

07a26e3

Tweak docs

17d5c9a

charliermarsh force-pushed the durumu/bannedmodulelevel branch 2 times, most recently from cc52d29 to 64a61ed Compare August 12, 2023 18:31

Merge branch 'main' into durumu/bannedmodulelevel

3725ca8

charliermarsh force-pushed the durumu/bannedmodulelevel branch from 64a61ed to 3725ca8 Compare August 12, 2023 18:36

charliermarsh enabled auto-merge (squash) August 12, 2023 18:36

charliermarsh mentioned this pull request Aug 12, 2023

Use top-level semantic detection for E402 #6526

Merged

charliermarsh merged commit c03e2ac into astral-sh:main Aug 12, 2023

charliermarsh mentioned this pull request Aug 12, 2023

Use a unified policy abstraction for the flake8-tidy-imports rules #6527

Merged

chenrui333 mentioned this pull request Aug 17, 2023

ruff 0.0.285 Homebrew/homebrew-core#139823

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[`flake8-tidy-imports`] Add `TID253` #6378

[`flake8-tidy-imports`] Add `TID253` #6378

durumu commented Aug 6, 2023

github-actions bot commented Aug 6, 2023 •

edited

Loading

konstin Aug 11, 2023

durumu Aug 12, 2023

durumu commented Aug 12, 2023

charliermarsh commented Aug 12, 2023

charliermarsh Aug 12, 2023

durumu Aug 12, 2023

charliermarsh Aug 12, 2023

[flake8-tidy-imports] Add TID253 #6378

[flake8-tidy-imports] Add TID253 #6378

Conversation

durumu commented Aug 6, 2023

Summary

Test Plan

github-actions bot commented Aug 6, 2023 • edited Loading

PR Check Results

Ecosystem

Benchmark

Linux

Windows

konstin Aug 11, 2023

Choose a reason for hiding this comment

durumu Aug 12, 2023

Choose a reason for hiding this comment

durumu commented Aug 12, 2023

charliermarsh commented Aug 12, 2023

charliermarsh Aug 12, 2023

Choose a reason for hiding this comment

durumu Aug 12, 2023

Choose a reason for hiding this comment

charliermarsh Aug 12, 2023

Choose a reason for hiding this comment

[`flake8-tidy-imports`] Add `TID253` #6378

[`flake8-tidy-imports`] Add `TID253` #6378

github-actions bot commented Aug 6, 2023 •

edited

Loading