Skip to content

Conversation

@MichaReiser
Copy link
Member

@MichaReiser MichaReiser commented Apr 11, 2025

Summary

Given:

main.py:

from foo import Foo

foo.py

class Fo: ...

Red Knot reports an unresolved import diagnostic in main.py because foo doesn't export Foo.
Renaming the class Fo in foo.py to Foo should refresh the diagnsotics in main.py.

It turns out, that this is a fairly easy change. All that was necessary is to set inter_file_dependencies in the server registration options to true.

We'll need a different solution for publish diagnostics but Red Knot doesn't yet support publish diagnostics (astral-sh/ty#79)

Test Plan

Screen.Recording.2025-04-11.at.14.46.47.mov

@MichaReiser MichaReiser added server Related to the LSP server ty Multi-file analysis & type inference labels Apr 11, 2025
@MichaReiser MichaReiser marked this pull request as ready for review April 11, 2025 12:47
@MichaReiser MichaReiser requested review from dhruvmanila and removed request for AlexWaygood, carljm, dcreager and sharkdp April 11, 2025 12:48
@github-actions
Copy link
Contributor

mypy_primer results

No ecosystem changes detected ✅

@MichaReiser MichaReiser changed the title [red-knot] Enable inter-file-dependencies [red-knot] Refresh diagnostics when changing related files Apr 11, 2025
Copy link
Member

@dhruvmanila dhruvmanila left a comment

Choose a reason for hiding this comment

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

Nice!

@MichaReiser MichaReiser merged commit 7e57179 into main Apr 11, 2025
23 checks passed
@MichaReiser MichaReiser deleted the micha/inter-file-dependencies branch April 11, 2025 13:50
@dhruvmanila
Copy link
Member

We'll need a different solution for publish diagnostics but Red Knot doesn't yet support publish diagnostics

Yeah, I think we might need to keep track of the diagnostic dependencies ourselves in that case and if any of the dependent file changes, we'll need to refresh the diagnostics for the parent files. This might also become useful and required for caching diagnostics on the server side.

@MichaReiser
Copy link
Member Author

Yeah, I think we might need to keep track of the diagnostic dependencies ourselves in that case and if any of the dependent file changes, we'll need to refresh the diagnostics for the parent files. This might also become useful and required for caching diagnostics on the server side.

Yeah, I'm not sure how that would work because we have no way of asking: What files does file X depend on. Only including its direct dependencies isn't good enough. We might have to publish all diagnostics eagerly. I wanted to look into how r-a does that.

dcreager added a commit that referenced this pull request Apr 11, 2025
* main:
  [red-knot] Specialize `str.startswith` for string literals (#17351)
  [syntax-errors] `yield`, `yield from`, and `await` outside functions (#17298)
  [red-knot] Refresh diagnostics when changing related files (#17350)
  Add `Checker::import_from_typing` (#17340)
  Don't add chaperone space after escaped quote in triple quote (#17216)
  [red-knot] Silence errors in unreachable type annotations / class bases (#17342)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

server Related to the LSP server ty Multi-file analysis & type inference

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants