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

fix(lsp): include all diagnosable documents on initialize #17979

Merged
merged 37 commits into from
Mar 30, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
6125671
fix(lsp): add all documents to the lsp on initialize
dsherret Feb 28, 2023
f08a5bf
Fix
dsherret Feb 28, 2023
ef6475c
Take into account enabled_paths
dsherret Mar 1, 2023
17b1f0b
Another one.
dsherret Mar 1, 2023
67439dd
More work.
dsherret Mar 4, 2023
abedfff
Merge branch 'main' into fix_lsp_open_docs_root
dsherret Mar 15, 2023
459ce44
Fix inverted logic.
dsherret Mar 15, 2023
72bfa94
Compile
dsherret Mar 15, 2023
cdc4af0
Ignore minified files
dsherret Mar 15, 2023
0129cf9
Merge branch 'main' into fix_lsp_open_docs_root
dsherret Mar 29, 2023
8688718
Handle files in enabled paths
dsherret Mar 29, 2023
8b0ab70
Consolidate code
dsherret Mar 29, 2023
4c9e102
refactor(lsp): remove boolean parameters on `documents.documents(...)`
dsherret Mar 29, 2023
d9c5e91
Remove word "And"
dsherret Mar 29, 2023
fc4d027
Merge branch 'refactor_lsp_documents_bool_param' into fix_lsp_open_do…
dsherret Mar 29, 2023
08ef6af
Working.
dsherret Mar 29, 2023
2dc17b0
Failing test because our textDocument/references implementation is br…
dsherret Mar 29, 2023
bab99d7
fix(lsp): `textDocument/references` should respect `includeDeclaration`
dsherret Mar 29, 2023
b7dee99
Small refactor.
dsherret Mar 29, 2023
a8cb329
Fix codelens test
dsherret Mar 29, 2023
5894f85
Merge branch 'main' into fix_lsp_find_references_respect_include_decl…
dsherret Mar 29, 2023
c5cfb24
Merge branch 'main' into fix_lsp_find_references_respect_include_decl…
dsherret Mar 30, 2023
b7d7541
Add test and improve code
dsherret Mar 30, 2023
41b3004
Merge branch 'main' into fix_lsp_find_references_respect_include_decl…
dsherret Mar 30, 2023
8dec2d3
Oops... Fix test
dsherret Mar 30, 2023
5a55a7e
Reduce flakiness of package_json_uncached_no_error
dsherret Mar 30, 2023
74fb03f
Merge branch 'main' into fix_lsp_open_docs_root
dsherret Mar 30, 2023
818811e
Merge branch 'fix_lsp_find_references_respect_include_declaration' in…
dsherret Mar 30, 2023
7b96f00
Add enabled_root_urls tests
dsherret Mar 30, 2023
bc3c42e
Merge branch 'main' into fix_lsp_open_docs_root
dsherret Mar 30, 2023
697ace6
Refactor document finder to be more easily unit testable
dsherret Mar 30, 2023
2216402
Another test.
dsherret Mar 30, 2023
8fe2597
Merge branch 'main' into fix_lsp_open_docs_root
dsherret Mar 30, 2023
4d8a9b0
Fix tests
dsherret Mar 30, 2023
b416267
Merge branch 'main' into fix_lsp_open_docs_root
dsherret Mar 30, 2023
055f94d
Fix two failing tests.
dsherret Mar 30, 2023
6cc5a97
Update lsp tests to use a temp cwd
dsherret Mar 30, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Merge branch 'main' into fix_lsp_open_docs_root
  • Loading branch information
dsherret committed Mar 15, 2023
commit abedfff554180b2d454080ffcbfdb77ac2ddf7af
1 change: 0 additions & 1 deletion cli/lsp/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -592,7 +592,6 @@ impl Config {
workspace: ModuleSpecifier,
enabled_paths: Vec<String>,
) -> bool {
let workspace = ensure_directory_specifier(workspace);
let mut touched = false;
if !enabled_paths.is_empty() {
if let Ok(workspace_path) = specifier_to_file_path(&workspace) {
Expand Down
124 changes: 70 additions & 54 deletions cli/lsp/language_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ impl LanguageServer {
}
}

pub async fn refresh_specifiers_from_client(&self) {
pub async fn refresh_specifiers_from_client(&self) -> bool {
let (client, specifiers) =
{
let ls = self.0.read().await;
Expand Down Expand Up @@ -365,6 +365,7 @@ impl LanguageServer {
let mut touched = false;
if let Some(specifiers) = specifiers {
let configs_result = client
.when_outside_lsp_lock()
.specifier_configurations(
specifiers
.iter()
Expand All @@ -375,10 +376,11 @@ impl LanguageServer {

let mut ls = self.0.write().await;
if let Ok(configs) = configs_result {
for (i, value) in configs.into_iter().enumerate() {
for (value, internal_uri) in
configs.into_iter().zip(specifiers.into_iter().map(|s| s.1))
{
match value {
Ok(specifier_settings) => {
let internal_uri = specifiers[i].1.clone();
if ls
.config
.set_specifier_settings(internal_uri, specifier_settings)
Expand All @@ -403,6 +405,7 @@ impl LanguageServer {
ls.send_diagnostics_update();
}
}
touched
}
}

Expand Down Expand Up @@ -1130,55 +1133,14 @@ impl Inner {

fn refresh_documents_config(&mut self) {
self.documents.update_config(
self.config.root_dirs(),
self.maybe_import_map.clone(),
self.maybe_config_file.as_ref(),
self.maybe_package_json.as_ref(),
self.npm_resolver.api().clone(),
self.npm_resolver.resolution().clone(),
self.npm_api.clone(),
self.npm_resolution.clone(),
);
}

async fn initialized(&mut self, _: InitializedParams) {
if self
.config
.client_capabilities
.workspace_did_change_watched_files
{
// we are going to watch all the JSON files in the workspace, and the
// notification handler will pick up any of the changes of those files we
// are interested in.
let watch_registration_options =
DidChangeWatchedFilesRegistrationOptions {
watchers: vec![FileSystemWatcher {
glob_pattern: "**/*.{json,jsonc}".to_string(),
kind: Some(WatchKind::Change),
}],
};
let registration = Registration {
id: "workspace/didChangeWatchedFiles".to_string(),
method: "workspace/didChangeWatchedFiles".to_string(),
register_options: Some(
serde_json::to_value(watch_registration_options).unwrap(),
),
};
if let Err(err) =
self.client.register_capability(vec![registration]).await
{
warn!("Client errored on capabilities.\n{:#}", err);
}
}

if self.config.client_capabilities.testing_api {
let test_server = testing::TestServer::new(
self.client.clone(),
self.performance.clone(),
self.config.root_uri.clone(),
);
self.maybe_testing_server = Some(test_server);
}
}

async fn shutdown(&self) -> LspResult<()> {
Ok(())
}
Expand Down Expand Up @@ -2838,10 +2800,59 @@ impl tower_lsp::LanguageServer for LanguageServer {
language_server.initialize(params).await
}

async fn initialized(&self, params: InitializedParams) {
self.0.write().await.initialized(params).await;
async fn initialized(&self, _: InitializedParams) {
let mut maybe_registration = None;
let client = {
let mut ls = self.0.write().await;
if ls
.config
.client_capabilities
.workspace_did_change_watched_files
{
// we are going to watch all the JSON files in the workspace, and the
// notification handler will pick up any of the changes of those files we
// are interested in.
let watch_registration_options =
DidChangeWatchedFilesRegistrationOptions {
watchers: vec![FileSystemWatcher {
glob_pattern: "**/*.{json,jsonc}".to_string(),
kind: Some(WatchKind::Change),
}],
};
maybe_registration = Some(Registration {
id: "workspace/didChangeWatchedFiles".to_string(),
method: "workspace/didChangeWatchedFiles".to_string(),
register_options: Some(
serde_json::to_value(watch_registration_options).unwrap(),
),
});
}

self.refresh_specifiers_from_client().await;
if ls.config.client_capabilities.testing_api {
let test_server = testing::TestServer::new(
ls.client.clone(),
ls.performance.clone(),
ls.config.root_uri.clone(),
);
ls.maybe_testing_server = Some(test_server);
}
ls.client.clone()
};

if let Some(registration) = maybe_registration {
if let Err(err) = client
.when_outside_lsp_lock()
.register_capability(vec![registration])
.await
{
warn!("Client errored on capabilities.\n{:#}", err);
}
}

if !self.refresh_specifiers_from_client().await {
// force update config
self.0.write().await.refresh_documents_config();
}

lsp_log!("Server ready.");
}
Expand All @@ -2862,7 +2873,8 @@ impl tower_lsp::LanguageServer for LanguageServer {
let mut inner = self.0.write().await;
let client = inner.client.clone();
let client_uri = params.text_document.uri.clone();
let specifier = inner.url_map.normalize_url(&client_uri);
let specifier =
inner.url_map.normalize_url(&client_uri, LspUrlKind::File);
let document = inner.did_open(&specifier, params).await;
let has_specifier_settings =
inner.config.has_specifier_settings(&specifier);
Expand All @@ -2882,7 +2894,10 @@ impl tower_lsp::LanguageServer for LanguageServer {
// retrieve the specifier settings outside the lock if
// they haven't been asked for yet
if !had_specifier_settings {
let response = client.specifier_configuration(&client_uri).await;
let response = client
.when_outside_lsp_lock()
.specifier_configuration(&client_uri)
.await;
let mut ls = self.0.write().await;
match response {
Ok(specifier_settings) => {
Expand All @@ -2903,6 +2918,7 @@ impl tower_lsp::LanguageServer for LanguageServer {
{
ls.refresh_documents_config();
ls.send_diagnostics_update();
ls.send_testing_update();
}
}
}
Expand Down Expand Up @@ -2979,17 +2995,17 @@ impl tower_lsp::LanguageServer for LanguageServer {
&self,
params: DidChangeWorkspaceFoldersParams,
) {
let mark = {
let (performance, mark) = {
let mut ls = self.0.write().await;
let mark = ls
.performance
.mark("did_change_workspace_folders", Some(&params));
ls.did_change_workspace_folders(params);
mark
(ls.performance.clone(), mark)
};

self.refresh_specifiers_from_client().await;
self.0.read().await.performance.measure(mark);
performance.measure(mark);
}

async fn document_symbol(
Expand Down
5 changes: 3 additions & 2 deletions cli/lsp/tsc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2749,6 +2749,8 @@ fn op_script_names(state: &mut OpState) -> Vec<String> {
let state = state.borrow_mut::<State>();
let documents = &state.state_snapshot.documents;
let all_docs = documents.documents(false, true);
let mut result = Vec::new();
let mut seen = HashSet::new();

let mut result = Vec::with_capacity(all_docs.len() + 1);

Expand All @@ -2767,7 +2769,7 @@ fn op_script_names(state: &mut OpState) -> Vec<String> {
}

// finally include the documents and all their dependencies
for doc in &open_docs {
for doc in &all_docs {
let specifier = doc.specifier();
if seen.insert(specifier.as_str()) {
result.push(specifier.to_string());
Expand All @@ -2788,7 +2790,6 @@ fn op_script_names(state: &mut OpState) -> Vec<String> {
}
}

result.extend(all_docs.into_iter().map(|d| d.specifier().to_string()));
result
}

Expand Down
11 changes: 9 additions & 2 deletions cli/lsp/urls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,13 +148,20 @@ impl LspUrlMap {
/// converted into proper module specifiers, as well as handle situations
/// where the client encodes a file URL differently than Rust does by default
/// causing issues with string matching of URLs.
pub fn normalize_url(&self, url: &Url) -> ModuleSpecifier {
///
/// Note: Sometimes the url provided by the client may not have a trailing slash,
/// so we need to force it to in the mapping and nee to explicitly state whether
/// this is a file or directory url.
pub fn normalize_url(&self, url: &Url, kind: LspUrlKind) -> ModuleSpecifier {
let mut inner = self.0.lock();
if let Some(specifier) = inner.get_specifier(url).cloned() {
specifier
} else {
let specifier = if let Ok(path) = url.to_file_path() {
Url::from_file_path(path).unwrap()
match kind {
LspUrlKind::Folder => Url::from_directory_path(path).unwrap(),
LspUrlKind::File => Url::from_file_path(path).unwrap(),
}
} else {
url.clone()
};
Expand Down
You are viewing a condensed version of this merge commit. You can view the full changes here.