[ty] avoid zero-project panics after removing the last workspace#25367
[ty] avoid zero-project panics after removing the last workspace#25367fallintoplace wants to merge 2 commits into
Conversation
Typing conformance resultsNo changes detected ✅Current numbersThe percentage of diagnostics emitted that were expected errors held steady at 89.52%. The percentage of expected errors that received a diagnostic held steady at 86.99%. The number of fully passing files held steady at 90/134. |
Memory usage reportMemory usage unchanged ✅ |
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
not-iterable |
0 | 0 | 1 |
| Total | 0 | 0 | 1 |
Raw diff:
dd-trace-py (https://github.com/DataDog/dd-trace-py)
- ddtrace/contrib/internal/subprocess/patch.py:321:44 error[not-iterable] Object of type `list[Unknown] | None | list[str] | ... omitted 3 union elements` may not be iterable
+ ddtrace/contrib/internal/subprocess/patch.py:321:44 error[not-iterable] Object of type `list[Unknown] | None | list[str] | Unknown | list[_T@deque | str]` may not be iterable|
Thank you. Can you say more why you decided to make the db methods fallible over creating a default database when the last workspace folder got removed? I also ask you to read and conform to our AI use policy (https://github.com/astral-sh/ruff/blob/main/CONTRIBUTING.md#use-of-ai) |
|
My thinking was that once the client removes the last workspace folder, we really do have zero workspaces, so I treated that as zero projects too instead of immediately creating a fallback DB again. The spec seemed compatible with that: workspace/workspaceFolders can be empty, and workspace/didChangeWorkspaceFolders doesn’t seem to treat the last-folder case specially. That said, I agree the fix got broader than I’d like. If you’d rather recreate a default DB there, I’m happy to rework it. |
|
I think I'd go with creating a default database (as suggested by Micha) here instead. My main worry is what to do when a workspace folder is then added back to the session which contains the opened file? Because these opened files were added to the default database, should these be removed from the default database and added to the project specific database now? Maybe, an easier thing to do here would just be to go through the open files in this scenario and move them to the project specific database if they belong in that workspace and remove them from the default database. I'd also be fine skipping this in this PR if it's not an actual issue and could be done as a follow-up. |
Closes astral-sh/ty#3534.
Summary
Handle the zero-project state that appears after the last workspace folder is removed.
When the final workspace was removed, ty dropped the last project database but later document flows still assumed that every open document could resolve to a backing project. That made
didOpenand background document requests panic on the zero-project path.This change keeps that state behavior-preserving and non-panicking by:
remove last workspace -> didOpen -> diagnostic/hoverTesting
cargo test -p ty_server open_document_after_removing_only_workspace --test e2ecargo test -p ty_server workspace_folders --test e2euvx prek run --files crates/ty_project/src/db/changes.rs crates/ty_server/src/server/api/traits.rs crates/ty_server/src/server/api.rs crates/ty_server/src/server/api/diagnostics.rs crates/ty_server/src/session.rs crates/ty_server/tests/e2e/workspace_folders.rs