Skip to content

Commit a7aada5

Browse files
dhruvmaniladhruvmanila
committed
Address review feedback
1 parent c9be67b commit a7aada5

File tree

5 files changed

+250
-243
lines changed

5 files changed

+250
-243
lines changed

‎crates/ty_server/src/server.rs‎

Lines changed: 19 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ impl Server {
3939
pub(crate) fn new(
4040
worker_threads: NonZeroUsize,
4141
connection: Connection,
42-
fallback_system: Arc<dyn System + 'static + Send + Sync + RefUnwindSafe>,
42+
native_system: Arc<dyn System + 'static + Send + Sync + RefUnwindSafe>,
4343
) -> crate::Result<Self> {
4444
let (id, init_value) = connection.initialize_start()?;
4545
let init_params: InitializeParams = serde_json::from_value(init_value)?;
@@ -107,7 +107,7 @@ impl Server {
107107
.collect()
108108
})
109109
.or_else(|| {
110-
let current_dir = fallback_system
110+
let current_dir = native_system
111111
.current_directory()
112112
.as_std_path()
113113
.to_path_buf();
@@ -152,7 +152,7 @@ impl Server {
152152
position_encoding,
153153
global_options,
154154
workspaces,
155-
fallback_system,
155+
native_system,
156156
)?,
157157
client_capabilities,
158158
})
@@ -309,35 +309,33 @@ mod tests {
309309
use crate::test::TestServerBuilder;
310310

311311
#[test]
312-
fn initialization_sequence() {
313-
let system = InMemorySystem::default();
312+
fn initialization() -> Result<()> {
314313
let test_server = TestServerBuilder::new()
315-
.with_memory_system(system)
316-
.build()
317-
.unwrap()
318-
.wait_until_workspaces_are_initialized()
319-
.unwrap();
314+
.build()?
315+
.wait_until_workspaces_are_initialized()?;
320316

321317
let initialization_result = test_server.initialization_result().unwrap();
322318

323-
insta::assert_json_snapshot!("initialization_capabilities", initialization_result);
319+
insta::assert_json_snapshot!("initialization", initialization_result);
320+
321+
Ok(())
324322
}
325323

326324
#[test]
327-
fn initialization_with_workspace() {
325+
fn initialization_with_workspace() -> Result<()> {
328326
let workspace_root = SystemPathBuf::from("/foo");
329327
let system = InMemorySystem::new(workspace_root.clone());
330328
let test_server = TestServerBuilder::new()
331329
.with_memory_system(system)
332330
.with_workspace(&workspace_root, ClientOptions::default())
333-
.build()
334-
.unwrap()
335-
.wait_until_workspaces_are_initialized()
336-
.unwrap();
331+
.build()?
332+
.wait_until_workspaces_are_initialized()?;
337333

338334
let initialization_result = test_server.initialization_result().unwrap();
339335

340336
insta::assert_json_snapshot!("initialization_with_workspace", initialization_result);
337+
338+
Ok(())
341339
}
342340

343341
#[test]
@@ -355,12 +353,12 @@ def foo() -> str:
355353
let mut server = TestServerBuilder::new()
356354
.with_memory_system(InMemorySystem::from_memory_fs(fs))
357355
.with_workspace(&workspace_root, ClientOptions::default())
358-
.with_pull_diagnostics(false)
356+
.enable_pull_diagnostics(false)
359357
.build()?
360358
.wait_until_workspaces_are_initialized()?;
361359

362-
server.open_text_document(&foo, &foo_content)?;
363-
let diagnostics = server.get_notification::<PublishDiagnostics>()?;
360+
server.open_text_document(&foo, &foo_content);
361+
let diagnostics = server.await_notification::<PublishDiagnostics>()?;
364362

365363
insta::assert_debug_snapshot!(diagnostics);
366364

@@ -382,11 +380,11 @@ def foo() -> str:
382380
let mut server = TestServerBuilder::new()
383381
.with_memory_system(InMemorySystem::from_memory_fs(fs))
384382
.with_workspace(&workspace_root, ClientOptions::default())
385-
.with_pull_diagnostics(true)
383+
.enable_pull_diagnostics(true)
386384
.build()?
387385
.wait_until_workspaces_are_initialized()?;
388386

389-
server.open_text_document(&foo, &foo_content)?;
387+
server.open_text_document(&foo, &foo_content);
390388
let diagnostics = server.document_diagnostic_request(&foo)?;
391389

392390
insta::assert_debug_snapshot!(diagnostics);

‎crates/ty_server/src/session.rs‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,8 @@ mod settings;
4040

4141
/// The global state for the LSP
4242
pub(crate) struct Session {
43-
/// A fallback system to use with the [`LSPSystem`].
44-
fallback_system: Arc<dyn System + 'static + Send + Sync + RefUnwindSafe>,
43+
/// A native system to use with the [`LSPSystem`].
44+
native_system: Arc<dyn System + 'static + Send + Sync + RefUnwindSafe>,
4545

4646
/// Used to retrieve information about open documents and settings.
4747
///
@@ -105,7 +105,7 @@ impl Session {
105105
position_encoding: PositionEncoding,
106106
global_options: GlobalOptions,
107107
workspace_folders: Vec<(Url, ClientOptions)>,
108-
fallback_system: Arc<dyn System + 'static + Send + Sync + RefUnwindSafe>,
108+
native_system: Arc<dyn System + 'static + Send + Sync + RefUnwindSafe>,
109109
) -> crate::Result<Self> {
110110
let index = Arc::new(Index::new(global_options.into_settings()));
111111

@@ -115,7 +115,7 @@ impl Session {
115115
}
116116

117117
Ok(Self {
118-
fallback_system,
118+
native_system,
119119
position_encoding,
120120
workspaces,
121121
deferred_messages: VecDeque::new(),
@@ -235,7 +235,7 @@ impl Session {
235235
AnySystemPath::System(system_path) => {
236236
self.project_state_for_path(system_path).unwrap_or_else(|| {
237237
self.default_project
238-
.get(self.index.as_ref(), &self.fallback_system)
238+
.get(self.index.as_ref(), &self.native_system)
239239
})
240240
}
241241
AnySystemPath::SystemVirtual(_virtual_path) => {
@@ -266,7 +266,7 @@ impl Session {
266266
.map(|(_, project)| project)
267267
.unwrap_or_else(|| {
268268
self.default_project
269-
.get_mut(self.index.as_ref(), &self.fallback_system)
269+
.get_mut(self.index.as_ref(), &self.native_system)
270270
}),
271271
AnySystemPath::SystemVirtual(_virtual_path) => {
272272
// TODO: Currently, ty only supports single workspace but we need to figure out
@@ -352,7 +352,7 @@ impl Session {
352352
// and create a project database for each.
353353
let system = LSPSystem::new(
354354
self.index.as_ref().unwrap().clone(),
355-
self.fallback_system.clone(),
355+
self.native_system.clone(),
356356
);
357357

358358
let project = ProjectMetadata::discover(&root, &system)

‎crates/ty_server/src/snapshots/ty_server__server__tests__initialization_capabilities.snap‎ renamed to ‎crates/ty_server/src/snapshots/ty_server__server__tests__initialization.snap‎

File renamed without changes.

‎crates/ty_server/src/system.rs‎

Lines changed: 21 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -119,18 +119,21 @@ pub(crate) struct LSPSystem {
119119
/// [`index_mut`]: crate::Session::index_mut
120120
index: Option<Arc<Index>>,
121121

122-
/// A fallback system implementation used when documents are not found in the index.
123-
fallback_system: Arc<dyn System + 'static + Send + Sync + RefUnwindSafe>,
122+
/// A native system implementation.
123+
///
124+
/// This is used to delegate method calls that are not handled by the LSP system. It is also
125+
/// used as a fallback when the documents are not found in the LSP index.
126+
native_system: Arc<dyn System + 'static + Send + Sync + RefUnwindSafe>,
124127
}
125128

126129
impl LSPSystem {
127130
pub(crate) fn new(
128131
index: Arc<Index>,
129-
fallback_system: Arc<dyn System + 'static + Send + Sync + RefUnwindSafe>,
132+
native_system: Arc<dyn System + 'static + Send + Sync + RefUnwindSafe>,
130133
) -> Self {
131134
Self {
132135
index: Some(index),
133-
fallback_system,
136+
native_system,
134137
}
135138
}
136139

@@ -184,25 +187,24 @@ impl System for LSPSystem {
184187
FileType::File,
185188
))
186189
} else {
187-
self.fallback_system.path_metadata(path)
190+
self.native_system.path_metadata(path)
188191
}
189192
}
190193

191194
fn canonicalize_path(&self, path: &SystemPath) -> Result<SystemPathBuf> {
192-
self.fallback_system.canonicalize_path(path)
195+
self.native_system.canonicalize_path(path)
193196
}
194197

195198
fn path_exists_case_sensitive(&self, path: &SystemPath, prefix: &SystemPath) -> bool {
196-
self.fallback_system
197-
.path_exists_case_sensitive(path, prefix)
199+
self.native_system.path_exists_case_sensitive(path, prefix)
198200
}
199201

200202
fn read_to_string(&self, path: &SystemPath) -> Result<String> {
201203
let document = self.system_path_to_document_ref(path);
202204

203205
match document {
204206
Some(DocumentQuery::Text { document, .. }) => Ok(document.contents().to_string()),
205-
_ => self.fallback_system.read_to_string(path),
207+
_ => self.native_system.read_to_string(path),
206208
}
207209
}
208210

@@ -214,7 +216,7 @@ impl System for LSPSystem {
214216
Notebook::from_source_code(document.contents())
215217
}
216218
Some(DocumentQuery::Notebook { notebook, .. }) => Ok(notebook.make_ruff_notebook()),
217-
None => self.fallback_system.read_to_notebook(path),
219+
None => self.native_system.read_to_notebook(path),
218220
}
219221
}
220222

@@ -245,26 +247,26 @@ impl System for LSPSystem {
245247
}
246248

247249
fn current_directory(&self) -> &SystemPath {
248-
self.fallback_system.current_directory()
250+
self.native_system.current_directory()
249251
}
250252

251253
fn user_config_directory(&self) -> Option<SystemPathBuf> {
252-
self.fallback_system.user_config_directory()
254+
self.native_system.user_config_directory()
253255
}
254256

255257
fn cache_dir(&self) -> Option<SystemPathBuf> {
256-
self.fallback_system.cache_dir()
258+
self.native_system.cache_dir()
257259
}
258260

259261
fn read_directory<'a>(
260262
&'a self,
261263
path: &SystemPath,
262264
) -> Result<Box<dyn Iterator<Item = Result<DirectoryEntry>> + 'a>> {
263-
self.fallback_system.read_directory(path)
265+
self.native_system.read_directory(path)
264266
}
265267

266268
fn walk_directory(&self, path: &SystemPath) -> WalkDirectoryBuilder {
267-
self.fallback_system.walk_directory(path)
269+
self.native_system.walk_directory(path)
268270
}
269271

270272
fn glob(
@@ -274,11 +276,11 @@ impl System for LSPSystem {
274276
Box<dyn Iterator<Item = std::result::Result<SystemPathBuf, GlobError>> + '_>,
275277
PatternError,
276278
> {
277-
self.fallback_system.glob(pattern)
279+
self.native_system.glob(pattern)
278280
}
279281

280282
fn as_writable(&self) -> Option<&dyn WritableSystem> {
281-
self.fallback_system.as_writable()
283+
self.native_system.as_writable()
282284
}
283285

284286
fn as_any(&self) -> &dyn Any {
@@ -290,11 +292,11 @@ impl System for LSPSystem {
290292
}
291293

292294
fn case_sensitivity(&self) -> CaseSensitivity {
293-
self.fallback_system.case_sensitivity()
295+
self.native_system.case_sensitivity()
294296
}
295297

296298
fn env_var(&self, name: &str) -> std::result::Result<String, std::env::VarError> {
297-
self.fallback_system.env_var(name)
299+
self.native_system.env_var(name)
298300
}
299301
}
300302

0 commit comments

Comments
 (0)