-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Changes from 35 commits
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 f08a5bf
Fix
dsherret ef6475c
Take into account enabled_paths
dsherret 17b1f0b
Another one.
dsherret 67439dd
More work.
dsherret abedfff
Merge branch 'main' into fix_lsp_open_docs_root
dsherret 459ce44
Fix inverted logic.
dsherret 72bfa94
Compile
dsherret cdc4af0
Ignore minified files
dsherret 0129cf9
Merge branch 'main' into fix_lsp_open_docs_root
dsherret 8688718
Handle files in enabled paths
dsherret 8b0ab70
Consolidate code
dsherret 4c9e102
refactor(lsp): remove boolean parameters on `documents.documents(...)`
dsherret d9c5e91
Remove word "And"
dsherret fc4d027
Merge branch 'refactor_lsp_documents_bool_param' into fix_lsp_open_do…
dsherret 08ef6af
Working.
dsherret 2dc17b0
Failing test because our textDocument/references implementation is br…
dsherret bab99d7
fix(lsp): `textDocument/references` should respect `includeDeclaration`
dsherret b7dee99
Small refactor.
dsherret a8cb329
Fix codelens test
dsherret 5894f85
Merge branch 'main' into fix_lsp_find_references_respect_include_decl…
dsherret c5cfb24
Merge branch 'main' into fix_lsp_find_references_respect_include_decl…
dsherret b7d7541
Add test and improve code
dsherret 41b3004
Merge branch 'main' into fix_lsp_find_references_respect_include_decl…
dsherret 8dec2d3
Oops... Fix test
dsherret 5a55a7e
Reduce flakiness of package_json_uncached_no_error
dsherret 74fb03f
Merge branch 'main' into fix_lsp_open_docs_root
dsherret 818811e
Merge branch 'fix_lsp_find_references_respect_include_declaration' in…
dsherret 7b96f00
Add enabled_root_urls tests
dsherret bc3c42e
Merge branch 'main' into fix_lsp_open_docs_root
dsherret 697ace6
Refactor document finder to be more easily unit testable
dsherret 2216402
Another test.
dsherret 8fe2597
Merge branch 'main' into fix_lsp_open_docs_root
dsherret 4d8a9b0
Fix tests
dsherret b416267
Merge branch 'main' into fix_lsp_open_docs_root
dsherret 055f94d
Fix two failing tests.
dsherret 6cc5a97
Update lsp tests to use a temp cwd
dsherret File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -485,6 +485,31 @@ impl Config { | |
.unwrap_or_else(|| self.settings.workspace.enable) | ||
} | ||
|
||
/// Gets the root directories or file paths based on the workspace config. | ||
pub fn enabled_root_urls(&self) -> Vec<Url> { | ||
let mut urls: Vec<Url> = Vec::new(); | ||
|
||
if !self.settings.workspace.enable && self.enabled_paths.is_empty() { | ||
// do not return any urls when disabled | ||
return urls; | ||
} | ||
|
||
for (workspace, enabled_paths) in &self.enabled_paths { | ||
if !enabled_paths.is_empty() { | ||
urls.extend(enabled_paths.iter().cloned()); | ||
} else { | ||
urls.push(workspace.clone()); | ||
} | ||
} | ||
if urls.is_empty() { | ||
if let Some(root_dir) = &self.root_uri { | ||
urls.push(root_dir.clone()) | ||
} | ||
} | ||
sort_and_remove_non_leaf_urls(&mut urls); | ||
urls | ||
} | ||
|
||
pub fn specifier_code_lens_test(&self, specifier: &ModuleSpecifier) -> bool { | ||
let value = self | ||
.settings | ||
|
@@ -621,6 +646,21 @@ impl Config { | |
} | ||
} | ||
|
||
/// Removes any URLs that are a descendant of another URL in the collection. | ||
fn sort_and_remove_non_leaf_urls(dirs: &mut Vec<Url>) { | ||
if dirs.is_empty() { | ||
return; | ||
} | ||
|
||
dirs.sort(); | ||
for i in (0..dirs.len() - 1).rev() { | ||
let prev = &dirs[i + 1]; | ||
if prev.as_str().starts_with(dirs[i].as_str()) { | ||
dirs.remove(i + 1); | ||
} | ||
} | ||
} | ||
|
||
#[cfg(test)] | ||
mod tests { | ||
use super::*; | ||
|
@@ -785,4 +825,69 @@ mod tests { | |
WorkspaceSettings::default() | ||
); | ||
} | ||
|
||
#[test] | ||
fn test_sort_and_remove_non_leaf_urls() { | ||
fn run_test(dirs: Vec<&str>, expected_output: Vec<&str>) { | ||
let mut dirs = dirs | ||
.into_iter() | ||
.map(|dir| Url::parse(dir).unwrap()) | ||
.collect(); | ||
sort_and_remove_non_leaf_urls(&mut dirs); | ||
let dirs: Vec<_> = dirs.iter().map(|dir| dir.as_str()).collect(); | ||
assert_eq!(dirs, expected_output); | ||
} | ||
|
||
run_test( | ||
vec![ | ||
"file:///test/asdf/test/asdf/", | ||
"file:///test/asdf/", | ||
"file:///test/asdf/", | ||
"file:///testing/456/893/", | ||
"file:///testing/456/893/test/", | ||
], | ||
vec!["file:///test/asdf/", "file:///testing/456/893/"], | ||
); | ||
run_test(vec![], vec![]); | ||
} | ||
|
||
#[test] | ||
fn config_enabled_root_urls() { | ||
let mut config = Config::new(); | ||
let root_dir = Url::parse("file:///example/").unwrap(); | ||
config.root_uri = Some(root_dir.clone()); | ||
config.settings.workspace.enable = false; | ||
config.settings.workspace.enable_paths = Vec::new(); | ||
assert_eq!(config.enabled_root_urls(), vec![]); | ||
|
||
config.settings.workspace.enable = true; | ||
assert_eq!(config.enabled_root_urls(), vec![root_dir]); | ||
|
||
config.settings.workspace.enable = false; | ||
let root_dir1 = Url::parse("file:///root1/").unwrap(); | ||
let root_dir2 = Url::parse("file:///root2/").unwrap(); | ||
let root_dir3 = Url::parse("file:///root3/").unwrap(); | ||
config.enabled_paths = HashMap::from([ | ||
( | ||
root_dir1.clone(), | ||
vec![ | ||
root_dir1.join("sub_dir").unwrap(), | ||
root_dir1.join("sub_dir/other").unwrap(), | ||
root_dir1.join("test.ts").unwrap(), | ||
], | ||
), | ||
(root_dir2.clone(), vec![root_dir2.join("other.ts").unwrap()]), | ||
(root_dir3.clone(), vec![]), | ||
]); | ||
|
||
assert_eq!( | ||
config.enabled_root_urls(), | ||
vec![ | ||
root_dir1.join("sub_dir").unwrap(), | ||
root_dir1.join("test.ts").unwrap(), | ||
root_dir2.join("other.ts").unwrap(), | ||
root_dir3 | ||
] | ||
); | ||
Comment on lines
+883
to
+891
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice tests |
||
} | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now this error is more likely to happen because it might load in some random files that use a different scheme (ex. "ext:" scheme in our case). This could be especially annoying in the repl. I downgraded this to debug because it didn't seem that important to surface at an "error" level.