Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[red-knot] Diagnostics attached to wrong file #14334

Closed
sharkdp opened this issue Nov 14, 2024 · 0 comments · Fixed by #14337
Closed

[red-knot] Diagnostics attached to wrong file #14334

sharkdp opened this issue Nov 14, 2024 · 0 comments · Fixed by #14337
Labels
bug Something isn't working red-knot Multi-file analysis & type inference

Comments

@sharkdp
Copy link
Contributor

sharkdp commented Nov 14, 2024

There are cases where we add diagnostics with a source range from file A to the TypeCheckDiagnosticsBuilder of file B. A small example to trigger this bug is the following:

# test.py
from subprocess import Popen

We currently emit a (false positive?) invalid-base diagnostic here, probably because we don't understand the definition of Popen:

But the actual problem here is that we attach the diagnostic to test.py, not to typeshed/stdlib/subprocess.pyi (notice the large column offset, resulting from trying to translate the byte offset into line/col on the wrong file):

error[invalid-base] /home/shark/playground/test.py:3:61460 Invalid class base with type `object` (all bases must be a class, `Any`, `Unknown` or `Todo`)

I noticed this, because things can go wrong bad if the byte offset from file A does not translate to a valid-UTF-8 offset in file B. Or if there are out-of-bounds accesses (which are for some reason also only triggered with non-ASCII characters):

# Ä
from subprocess import Popen
thread 'main' panicked at crates/ruff_source_file/src/line_index.rs:117:28:
byte index 61498 is out of bounds of `# ä
from subprocess import Popen
`

What needs to be done here?

  • Add an assertion to TypeCheckDiagnosticsBuilder that makes sure we never attach diagnostics for nodes from a different file?
  • Prevent raising diagnostics from different files in the first place?
@sharkdp sharkdp added bug Something isn't working red-knot Multi-file analysis & type inference labels Nov 14, 2024
sharkdp added a commit that referenced this issue Nov 14, 2024
## Summary

Avoid attaching diagnostics to the wrong file. See related issue for
details.

Closes #14334

## Test Plan

New regression test.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working red-knot Multi-file analysis & type inference
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant