ruff
ruff copied to clipboard
`ruff analyze graph` does not understand submodule imports
ruff 0.6.8
i see something like this:
{
"src/view.py": [
"src/models/User.py",
],
"src/code.py": [
"src/models/user.py",
],
}
content of src/models/__init__.py:
from models.user import User
__all__ = ['User']
Are you able to put together a minimal reproduction, like a set of files I can run ruff over to reproduce? Happy to help.
I was sure it wouldn't work, but fortunately it was easily reproduced.
uvx ruff analyze graph . on https://github.com/minmax/ruff-analyze-tree
"src/ruff_analyze_tree/colors.py": [
"src/ruff_analyze_tree/models/User.py"
],
"src/ruff_analyze_tree/models/__init__.py": [
"src/ruff_analyze_tree/models/user.py"
],
Could you explain what result you would expect or why the actual result is incorrect?
The repo don't contains User.py, only user.py, so "src/ruff_analyze_tree/models/User.py" - is invalid.
Maybe the resolver is case insensitive? I mean, that's arguably correct on case-insensitive filesystems which are common.
I can look into it!
Maybe the resolver is case insensitive?
no, im on mac (so case sensitive) and user.py was manually created 30 min ago.
My guess is that it's a Camel case import broke something from models.user import User.
(Just to clarify, it may not matter what filesystem you're on.)
oh my god, "By default, Mac OS uses a case-insensitive file system", im not sure now.
Let's proceed under the assumption that there's a bug here, I will debug.
I do think the issue here is that ruff-analyze-tree/src/ruff_analyze_tree/models/User.py exists from the perspective of the filesystem. \cc @AlexWaygood for insight on whether it's intended in the resolver.
That makes sense. What's the reason for querying the module resolver with User instead of user?
Because the import is from models import User. That could mean that there's a file at models/User.py, so we look for that first. But User is re-exported from models/__init__.py.
(It's incorrect to resolve from models import User to models/user.py; it should map to models/__init__.py. So it's at least incorrect from the perspective of mapping to Python's semantics.)
That's correct, but I think that's an issue in the analyze graph because it first tries to resolve models.User before testing for models
https://github.com/astral-sh/ruff/blob/3018303c8759b3e96d075c62eeb8b8ef24b4d0c3/crates/ruff_graph/src/resolver.rs#L27-L34
Red Knot does the resolution in the opposite order:
- First resolve
models - Then resolve
models.userby looking upUserinmodels
I don't know if I agree... but it depends on how we define the API surface? Like, on the face of it, it's incorrect that the module resolver returns models/User.py given modules.User -- that's not the correct file for that symbol!
As a different example: imagine that models/User.py exists, but models/user.py does not, and we try to resolve import models.user. I think today that would resolve to models/User.py? That seems wrong and has nothing to do with member vs. module imports.
The input of the module resolver isn't the name of a symbol. It's a module path. You have to use red knot's type checker if you want resolution by symbol.
What is a "module path"? Can you explain in terms of the second example I listed?
A module path is a name of a module. E.g. models or models.user
However, the module models.user is not the same as the symbol models.user
- Module
models.user:modelsis a package (namespace or normal).usercan resolve to either auser.pyoruser/__init__.py - Symbol
models.user: Import the modulemodels, then resolve the symboluserin its global scope, import the submodule if no such symbol is defined
From the python spec:
The module resolver only implements resolving the module name models.user to a file. It doesn't implement the logic for resolving the symbol user in the scope of models.
For now, what you could do is to use Ruff's existing semantic model to determine if the symbol User exists in models and, if so, don't resolve models.User
@AlexWaygood Is my understanding correct that red knot's inference for from import misses the automatic sub-module fallback?
@AlexWaygood Is my understanding correct that red knot's inference for
from importmisses the automatic sub-module fallback?
Yeah, I think that's a known limitation of the red-knot semantic model right now. There's a test case with a TODO for it IIRC. I can look at doing the todo next week!
The relevant TODOs in red-knot are this comment and these currently-skipped tests. I think we'd fix this in the red-knot semantic model rather than the red-knot module resolver, however, so you might need to implement an independent fix in ruff_graph @charliermarsh
https://github.com/astral-sh/ruff/blob/bee498d6359dd7ce9aa8bff685ca821ab1bbfe31/crates/red_knot_python_semantic/src/types/infer.rs#L1416-L1424
https://github.com/astral-sh/ruff/blob/bee498d6359dd7ce9aa8bff685ca821ab1bbfe31/crates/red_knot_python_semantic/src/types/infer.rs#L2986-L3015