[red-knot] Add failing tests for * imports
#16873
Conversation
| `c.py`: | ||
|
|
||
| ```py | ||
| from b import * |
There was a problem hiding this comment.
the reason why we do not emit an error here is somewhat horrifying... we're currently inserting a global-scope symbol with the name "*" in the symbol table for b.py
AlexWaygood
force-pushed
the
alex/star-imports-tests
branch
from
697ac20 to
66aff0a
Compare
|
| reveal_type(X) # revealed: Unknown | ||
| ``` | ||
|
|
||
| ## Star imports with `__all__` |
There was a problem hiding this comment.
I think this could be done separately from the initial PR, but I'm guessing that's already your plan; having it in the initial set of tests doesn't preclude that, this test can just keep its TODOs longer.
| 1. Emit a diagnostic at the invalid definition of `__all__` (which will not fail at runtime)? | ||
| 1. Emit a diagnostic at the star-import from the module with the invalid `__all__` (which _will_ | ||
| fail at runtime)? | ||
| 1. Emit a diagnostic on both? |
There was a problem hiding this comment.
I would go for the first. If code is definitely wrong, even if it won't actually error at runtime until "used", I think it's generally better if we issue the error closer to the source, and not further from the source (which might be code with a different author who can't do anything about the problem.)
There was a problem hiding this comment.
I'm sort of inclined to go for the third: emit diagnostics at both locations?
- Having an invalid
__all__and having an import statement that we can say for sure will fail both seem like distinct errors that I'd want to know about if writing a Python module (even if they both have the same root cause) - If the module you're importing from is third-party and has an invalid
__all__but we only emit an error where__all__is defined, the user won't get any warning that their*import from the third-party module will fail, since we don't display diagnostics relating to type errors in third-party code. This isn't that far-fetched: we've had several situations at typeshed where the library we're stubbing has an invalid__all__and this was never detected by the library's tests (though it was caught by typeshed's tests!)
crates/red_knot_python_semantic/resources/mdtest/import/star.md
Outdated
Show resolved
Hide resolved
|
Added some more tests in 7cf8eab ... I don't think they're contentious, but happy to address any comments in a followup! Merging for now. |
Summary
This PR adds a suite of tests for wildcard (
*) imports. The tests nearly all fail for now, and those that don't, ahem, pass for the wrong reasons...I've tried to add TODO comments in all instances for places where we are currently inferring the incorrect thing, incorrectly emitting a diagnostic, or emitting a diagnostic with a bad error message.
Test Plan
cargo test -p red_knot_python_semantic