Skip to content

Conversation

@TaKO8Ki
Copy link
Contributor

@TaKO8Ki TaKO8Ki commented Aug 27, 2025

Summary

Fixes #19664

Fix allowed unused imports matching for top-level modules.

I've simply replaced from_dotted_name with user_defined. Since QualifiedName for imports is created in crates/ruff_python_semantic/src/imports.rs, I guess it's acceptable to use user_defined here. Please tell me if there is better way.

NameImport::Import(import) => QualifiedName::user_defined(&import.name.name),

Test Plan

I've added a snapshot test f401_allowed_unused_imports_top_level_module.

@ntBre ntBre added the bug Something isn't working label Aug 27, 2025
Copy link
Contributor

@ntBre ntBre 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! I just had a couple of suggestions about the tests, but this looks great to me.

Do you want to test if this fixes #15705 as well? There's no need to extend the PR if not, I just thought it sounded related and had a chance of fixing it already.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 27, 2025

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@TaKO8Ki
Copy link
Contributor Author

TaKO8Ki commented Aug 28, 2025

@ntBre

Thank you for the review. I have added a simple test case import hvplot and wrapped all test cases in a function.

Do you want to test if this fixes #15705 as well? There's no need to extend the PR if not, I just thought it sounded related and had a chance of fixing it already.

I want to try it and I guess it takes some time to fix it, so can I work on it in a different pull request?

Copy link
Contributor

@ntBre ntBre left a comment

Choose a reason for hiding this comment

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

Looks great, just one clarification on the test.

I want to try it and I guess it takes some time to fix it, so can I work on it in a different pull request?

That sounds good! I think your fix here might resolve that issue too without any other code changes, but it makes sense to add a test in a separate PR even so. No pressure to work on this, just an idea :)

Copy link
Contributor

@ntBre ntBre 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!

@ntBre ntBre changed the title [pyflakes] Fix allowed unused imports matching for top-level modules [pyflakes] Fix allowed-unused-imports matching for top-level modules (F401) Aug 28, 2025
@ntBre ntBre enabled auto-merge (squash) August 28, 2025 13:02
@ntBre ntBre merged commit 76a6b7e into astral-sh:main Aug 28, 2025
34 checks passed
dcreager added a commit that referenced this pull request Aug 28, 2025
* main:
  Fix mdtest ignore python code blocks (#20139)
  [ty] add support for cyclic legacy generic protocols (#20125)
  [ty] add cycle detection for find_legacy_typevars (#20124)
  Use new diff rendering format in tests (#20101)
  [ty] Fix 'too many cycle iterations' for unions of literals (#20137)
  [ty] No boundness analysis for implicit instance attributes (#20128)
  Bump 0.12.11 (#20136)
  [ty] Benchmarks for problematic implicit instance attributes cases (#20133)
  [`pyflakes`] Fix `allowed-unused-imports` matching for top-level modules (`F401`) (#20115)
  Move GitLab output rendering to `ruff_db` (#20117)
  [ty] Evaluate reachability of non-definitely-bound to Ambiguous (#19579)
  [ty] Introduce a representation for the top/bottom materialization of an invariant generic (#20076)
  [`flake8-async`] Implement `blocking-http-call-httpx` (`ASYNC212`) (#20091)
  [ty] print diagnostics with fully qualified name to disambiguate some cases (#19850)
  [`ruff`] Preserve relative whitespace in multi-line expressions (`RUF033`) (#19647)
second-ed pushed a commit to second-ed/ruff that referenced this pull request Sep 9, 2025
…les (`F401`) (astral-sh#20115)

<!--
Thank you for contributing to Ruff/ty! To help us out with reviewing,
please consider the following:

- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title? (Please prefix
with `[ty]` for ty pull
  requests.)
- Does this pull request include references to any relevant issues?
-->

## Summary

<!-- What's the purpose of the change? What does it do, and why? -->

Fixes astral-sh#19664

Fix allowed unused imports matching for top-level modules.

I've simply replaced `from_dotted_name` with `user_defined`. Since
QualifiedName for imports is created in
crates/ruff_python_semantic/src/imports.rs, I guess it's acceptable to
use `user_defined` here. Please tell me if there is better way.


https://github.com/astral-sh/ruff/blob/0c5089ed9e180da7bc76733ffe1a1d132e9142b0/crates/ruff_python_semantic/src/imports.rs#L62

## Test Plan

<!-- How was it tested? -->

I've added a snapshot test
`f401_allowed_unused_imports_top_level_module`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

allowed-unused-imports doesn't work for top-level modules

2 participants