Skip to content

Commit c0be144

Browse files
authored
[ty] Fix notifications about watched changes for entities outside any workspace (astral-sh#24775)
## Summary Distribute watched file change events generated by the LSP client to all dbs. This simplifies the filtering logic to be handled by each db, and also fixes venvs which are outside the workspace (which can happen for symlinked environments). ## Test Plan Neovim, and a custom test harness (let me know if there is a way to test this in ty itself, Micha made it sound like it wasn't so easy.
1 parent 10e03a6 commit c0be144

7 files changed

Lines changed: 71 additions & 85 deletions

File tree

crates/ruff_benchmark/benches/ty.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ fn benchmark_incremental(criterion: &mut Criterion) {
162162
let Case { db, .. } = case;
163163

164164
db.apply_changes(
165-
vec![ChangeEvent::Changed {
165+
&[ChangeEvent::Changed {
166166
path: case.file_path.clone(),
167167
kind: ChangedKind::FileContent,
168168
}],

crates/ty/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -456,7 +456,7 @@ impl MainLoop {
456456

457457
revision += 1;
458458
// Automatically cancels any pending queries and waits for them to complete.
459-
db.apply_changes(changes, Some(&self.project_options_overrides));
459+
db.apply_changes(&changes, Some(&self.project_options_overrides));
460460
if let Some(watcher) = self.watcher.as_mut() {
461461
watcher.update(db);
462462
}

crates/ty/tests/file_watching.rs

Lines changed: 35 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ impl TestCase {
175175

176176
fn apply_changes(
177177
&mut self,
178-
changes: Vec<ChangeEvent>,
178+
changes: &[ChangeEvent],
179179
project_options_overrides: Option<&ProjectOptionsOverrides>,
180180
) {
181181
self.db.apply_changes(changes, project_options_overrides);
@@ -193,7 +193,7 @@ impl TestCase {
193193
.context("Failed to write configuration")?;
194194

195195
let changes = self.take_watch_changes(event_for_file("pyproject.toml"));
196-
self.apply_changes(changes, None);
196+
self.apply_changes(&changes, None);
197197

198198
if let Some(watcher) = &mut self.watcher {
199199
watcher.update(&self.db);
@@ -521,7 +521,7 @@ fn new_file() -> anyhow::Result<()> {
521521

522522
let changes = case.stop_watch(event_for_file("foo.py"));
523523

524-
case.apply_changes(changes, None);
524+
case.apply_changes(&changes, None);
525525

526526
let foo = case.system_file(&foo_path).expect("foo.py to exist.");
527527

@@ -544,7 +544,7 @@ fn new_ignored_file() -> anyhow::Result<()> {
544544

545545
let changes = case.stop_watch(event_for_file("foo.py"));
546546

547-
case.apply_changes(changes, None);
547+
case.apply_changes(&changes, None);
548548

549549
assert!(case.system_file(&foo_path).is_ok());
550550
case.assert_indexed_project_files([bar_file]);
@@ -580,7 +580,7 @@ fn new_non_project_file() -> anyhow::Result<()> {
580580

581581
let changes = case.stop_watch(event_for_file("black.py"));
582582

583-
case.apply_changes(changes, None);
583+
case.apply_changes(&changes, None);
584584

585585
assert!(case.system_file(&black_path).is_ok());
586586

@@ -621,7 +621,7 @@ fn new_files_with_explicit_included_paths() -> anyhow::Result<()> {
621621

622622
let changes = case.stop_watch(event_for_file("test2.py"));
623623

624-
case.apply_changes(changes, None);
624+
case.apply_changes(&changes, None);
625625

626626
let sub_a_file = case.system_file(&sub_a_path).expect("sub/a.py to exist");
627627

@@ -666,7 +666,7 @@ fn new_file_in_included_out_of_project_directory() -> anyhow::Result<()> {
666666

667667
let changes = case.stop_watch(event_for_file("script2.py"));
668668

669-
case.apply_changes(changes, None);
669+
case.apply_changes(&changes, None);
670670

671671
let src_a_file = case.system_file(&src_a).unwrap();
672672
let outside_b_file = case.system_file(&outside_b_path).unwrap();
@@ -693,7 +693,7 @@ fn changed_file() -> anyhow::Result<()> {
693693

694694
assert!(!changes.is_empty());
695695

696-
case.apply_changes(changes, None);
696+
case.apply_changes(&changes, None);
697697

698698
assert_eq!(source_text(case.db(), foo).as_str(), "print('Version 2')");
699699
case.assert_indexed_project_files([foo]);
@@ -716,7 +716,7 @@ fn deleted_file() -> anyhow::Result<()> {
716716

717717
let changes = case.stop_watch(event_for_file("foo.py"));
718718

719-
case.apply_changes(changes, None);
719+
case.apply_changes(&changes, None);
720720

721721
assert!(!foo.exists(case.db()));
722722
case.assert_indexed_project_files([]);
@@ -748,7 +748,7 @@ fn move_file_to_trash() -> anyhow::Result<()> {
748748

749749
let changes = case.stop_watch(event_for_file("foo.py"));
750750

751-
case.apply_changes(changes, None);
751+
case.apply_changes(&changes, None);
752752

753753
assert!(!foo.exists(case.db()));
754754
case.assert_indexed_project_files([]);
@@ -775,7 +775,7 @@ fn move_file_to_project() -> anyhow::Result<()> {
775775

776776
let changes = case.stop_watch(event_for_file("foo.py"));
777777

778-
case.apply_changes(changes, None);
778+
case.apply_changes(&changes, None);
779779

780780
let foo_in_project = case.system_file(&foo_in_project)?;
781781

@@ -800,7 +800,7 @@ fn rename_file() -> anyhow::Result<()> {
800800

801801
let changes = case.stop_watch(event_for_file("bar.py"));
802802

803-
case.apply_changes(changes, None);
803+
case.apply_changes(&changes, None);
804804

805805
assert!(!foo.exists(case.db()));
806806

@@ -839,7 +839,7 @@ fn directory_moved_to_project() -> anyhow::Result<()> {
839839

840840
let changes = case.stop_watch(event_for_file("sub"));
841841

842-
case.apply_changes(changes, None);
842+
case.apply_changes(&changes, None);
843843

844844
let init_file = case
845845
.system_file(sub_new_path.join("__init__.py"))
@@ -888,7 +888,7 @@ fn directory_moved_to_trash() -> anyhow::Result<()> {
888888

889889
let changes = case.stop_watch(event_for_file("sub"));
890890

891-
case.apply_changes(changes, None);
891+
case.apply_changes(&changes, None);
892892

893893
// `import sub.a` should no longer resolve
894894
assert!(
@@ -939,7 +939,7 @@ fn directory_renamed() -> anyhow::Result<()> {
939939
// Linux and windows only emit an event for the newly created root directory, but not for every new component.
940940
let changes = case.stop_watch(event_for_file("sub"));
941941

942-
case.apply_changes(changes, None);
942+
case.apply_changes(&changes, None);
943943

944944
// `import sub.a` should no longer resolve
945945
assert!(
@@ -1000,7 +1000,7 @@ fn directory_deleted() -> anyhow::Result<()> {
10001000

10011001
let changes = case.stop_watch(event_for_file("sub"));
10021002

1003-
case.apply_changes(changes, None);
1003+
case.apply_changes(&changes, None);
10041004

10051005
// `import sub.a` should no longer resolve
10061006
assert!(
@@ -1042,7 +1042,7 @@ fn search_path() -> anyhow::Result<()> {
10421042

10431043
let changes = case.stop_watch(event_for_file("a.py"));
10441044

1045-
case.apply_changes(changes, None);
1045+
case.apply_changes(&changes, None);
10461046

10471047
assert!(resolve_module_confident(case.db(), &ModuleName::new_static("a").unwrap()).is_some());
10481048
case.assert_indexed_project_files([case.system_file(case.project_path("bar.py")).unwrap()]);
@@ -1073,7 +1073,7 @@ fn add_search_path() -> anyhow::Result<()> {
10731073

10741074
let changes = case.stop_watch(event_for_file("a.py"));
10751075

1076-
case.apply_changes(changes, None);
1076+
case.apply_changes(&changes, None);
10771077

10781078
assert!(resolve_module_confident(case.db(), &ModuleName::new_static("a").unwrap()).is_some());
10791079

@@ -1220,7 +1220,7 @@ fn changed_versions_file() -> anyhow::Result<()> {
12201220

12211221
let changes = case.stop_watch(event_for_file("VERSIONS"));
12221222

1223-
case.apply_changes(changes, None);
1223+
case.apply_changes(&changes, None);
12241224

12251225
assert!(resolve_module_confident(case.db(), &ModuleName::new_static("os").unwrap()).is_some());
12261226

@@ -1274,7 +1274,7 @@ fn hard_links_in_project() -> anyhow::Result<()> {
12741274

12751275
let changes = case.stop_watch(event_for_file("foo.py"));
12761276

1277-
case.apply_changes(changes, None);
1277+
case.apply_changes(&changes, None);
12781278

12791279
assert_eq!(source_text(case.db(), foo).as_str(), "print('Version 2')");
12801280

@@ -1345,7 +1345,7 @@ fn hard_links_to_target_outside_project() -> anyhow::Result<()> {
13451345

13461346
let changes = case.stop_watch(ChangeEvent::is_changed);
13471347

1348-
case.apply_changes(changes, None);
1348+
case.apply_changes(&changes, None);
13491349

13501350
assert_eq!(source_text(case.db(), bar).as_str(), "print('Version 2')");
13511351

@@ -1384,7 +1384,7 @@ mod unix {
13841384

13851385
let changes = case.stop_watch(event_for_file("foo.py"));
13861386

1387-
case.apply_changes(changes, None);
1387+
case.apply_changes(&changes, None);
13881388

13891389
assert_eq!(
13901390
foo.permissions(case.db()),
@@ -1464,7 +1464,7 @@ mod unix {
14641464

14651465
let changes = case.take_watch_changes(event_for_file("baz.py"));
14661466

1467-
case.apply_changes(changes, None);
1467+
case.apply_changes(&changes, None);
14681468

14691469
assert_eq!(
14701470
source_text(case.db(), baz_file).as_str(),
@@ -1477,7 +1477,7 @@ mod unix {
14771477

14781478
let changes = case.stop_watch(event_for_file("baz.py"));
14791479

1480-
case.apply_changes(changes, None);
1480+
case.apply_changes(&changes, None);
14811481

14821482
assert_eq!(
14831483
source_text(case.db(), baz_file).as_str(),
@@ -1545,7 +1545,7 @@ mod unix {
15451545

15461546
let changes = case.stop_watch(event_for_file("baz.py"));
15471547

1548-
case.apply_changes(changes, None);
1548+
case.apply_changes(&changes, None);
15491549

15501550
// The file watcher is guaranteed to emit one event for the changed file, but it isn't specified
15511551
// if the event is emitted for the "original" or linked path because both paths are watched.
@@ -1659,7 +1659,7 @@ mod unix {
16591659

16601660
let changes = case.stop_watch(event_for_file("baz.py"));
16611661

1662-
case.apply_changes(changes, None);
1662+
case.apply_changes(&changes, None);
16631663

16641664
assert_eq!(
16651665
source_text(case.db(), baz_original_file).as_str(),
@@ -1716,7 +1716,7 @@ fn nested_projects_delete_root() -> anyhow::Result<()> {
17161716

17171717
let changes = case.stop_watch(ChangeEvent::is_deleted);
17181718

1719-
case.apply_changes(changes, None);
1719+
case.apply_changes(&changes, None);
17201720

17211721
// It should now pick up the outer project.
17221722
assert_eq!(case.db().project().root(case.db()), case.root_path());
@@ -1782,7 +1782,7 @@ fn changes_to_user_configuration() -> anyhow::Result<()> {
17821782

17831783
let changes = case.stop_watch(event_for_file("ty.toml"));
17841784

1785-
case.apply_changes(changes, None);
1785+
case.apply_changes(&changes, None);
17861786

17871787
let diagnostics = case.db().check_file(foo);
17881788

@@ -1843,7 +1843,7 @@ fn changes_to_config_file_override() -> anyhow::Result<()> {
18431843
let changes = case.stop_watch(event_for_file("ty-override.toml"));
18441844

18451845
case.apply_changes(
1846-
changes,
1846+
&changes,
18471847
Some(&ProjectOptionsOverrides::new(
18481848
Some(case.project_path("ty-override.toml")),
18491849
Options::default(),
@@ -1922,7 +1922,7 @@ fn rename_files_casing_only() -> anyhow::Result<()> {
19221922
}
19231923

19241924
let changes = case.stop_watch(event_for_file("Lib.py"));
1925-
case.apply_changes(changes, None);
1925+
case.apply_changes(&changes, None);
19261926

19271927
// Resolving `lib` should now fail but `Lib` should now succeed
19281928
assert_eq!(
@@ -1952,7 +1952,7 @@ fn submodule_cache_invalidation_created() -> anyhow::Result<()> {
19521952

19531953
std::fs::write(case.project_path("bar/wazoo.py").as_std_path(), "")?;
19541954
let changes = case.stop_watch(event_for_file("wazoo.py"));
1955-
case.apply_changes(changes, None);
1955+
case.apply_changes(&changes, None);
19561956

19571957
insta::assert_snapshot!(
19581958
case.sorted_submodule_names("bar").join("\n"),
@@ -1986,7 +1986,7 @@ fn submodule_cache_invalidation_deleted() -> anyhow::Result<()> {
19861986

19871987
std::fs::remove_file(case.project_path("bar/wazoo.py").as_std_path())?;
19881988
let changes = case.stop_watch(event_for_file("wazoo.py"));
1989-
case.apply_changes(changes, None);
1989+
case.apply_changes(&changes, None);
19901990

19911991
insta::assert_snapshot!(
19921992
case.sorted_submodule_names("bar").join("\n"),
@@ -2009,11 +2009,11 @@ fn submodule_cache_invalidation_created_then_deleted() -> anyhow::Result<()> {
20092009

20102010
std::fs::write(case.project_path("bar/wazoo.py").as_std_path(), "")?;
20112011
let changes = case.take_watch_changes(event_for_file("wazoo.py"));
2012-
case.apply_changes(changes, None);
2012+
case.apply_changes(&changes, None);
20132013

20142014
std::fs::remove_file(case.project_path("bar/wazoo.py").as_std_path())?;
20152015
let changes = case.stop_watch(event_for_file("wazoo.py"));
2016-
case.apply_changes(changes, None);
2016+
case.apply_changes(&changes, None);
20172017

20182018
insta::assert_snapshot!(
20192019
case.sorted_submodule_names("bar").join("\n"),
@@ -2039,7 +2039,7 @@ fn submodule_cache_invalidation_after_pyproject_created() -> anyhow::Result<()>
20392039

20402040
std::fs::write(case.project_path("bar/wazoo.py").as_std_path(), "")?;
20412041
let changes = case.take_watch_changes(event_for_file("wazoo.py"));
2042-
case.apply_changes(changes, None);
2042+
case.apply_changes(&changes, None);
20432043

20442044
insta::assert_snapshot!(
20452045
case.sorted_submodule_names("bar").join("\n"),

0 commit comments

Comments
 (0)