fix(lsp): don't apply preload limit to workspace walk (#23123)

This commit is contained in:
Nayeem Rahman 2024-03-31 21:39:23 +01:00 committed by GitHub
parent db89ce33f4
commit 0144594044
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 94 additions and 65 deletions

View File

@ -777,11 +777,12 @@ impl ConfigSnapshot {
pub struct Settings {
pub unscoped: WorkspaceSettings,
pub by_workspace_folder: BTreeMap<ModuleSpecifier, Option<WorkspaceSettings>>,
pub first_folder: Option<ModuleSpecifier>,
}
impl Settings {
pub fn first_root_uri(&self) -> Option<&ModuleSpecifier> {
self.by_workspace_folder.first_key_value().map(|e| e.0)
self.first_folder.as_ref()
}
/// Returns `None` if the value should be deferred to the presence of a
@ -836,13 +837,11 @@ impl Settings {
let Ok(path) = specifier_to_file_path(specifier) else {
return (&self.unscoped, None);
};
let mut is_first_folder = true;
for (folder_uri, settings) in self.by_workspace_folder.iter().rev() {
let mut settings = settings.as_ref();
if is_first_folder {
if self.first_folder.as_ref() == Some(folder_uri) {
settings = settings.or(Some(&self.unscoped));
}
is_first_folder = false;
if let Some(settings) = settings {
let Ok(folder_path) = specifier_to_file_path(folder_uri) else {
continue;
@ -870,6 +869,7 @@ impl Settings {
.map(|s| (&s.enable, &s.enable_paths, &s.disable_paths)),
);
}
hasher.write_hashable(&self.first_folder);
hasher.finish()
}
}
@ -908,6 +908,7 @@ impl Config {
) {
self.settings.by_workspace_folder =
folders.iter().map(|(s, _)| (s.clone(), None)).collect();
self.settings.first_folder = folders.first().map(|(s, _)| s.clone());
self.workspace_folders = folders;
}
@ -1125,6 +1126,7 @@ pub struct ConfigData {
pub lockfile: Option<Arc<Mutex<Lockfile>>>,
pub package_json: Option<Arc<PackageJson>>,
pub import_map: Option<Arc<ImportMap>>,
pub import_map_from_settings: bool,
watched_files: HashMap<ModuleSpecifier, ConfigWatchedFileType>,
}
@ -1317,6 +1319,7 @@ impl ConfigData {
let mut import_map = None;
let mut import_map_value = None;
let mut import_map_specifier = None;
let mut import_map_from_settings = false;
if let Some(config_file) = &config_file {
if config_file.is_an_import_map() {
import_map_value = Some(config_file.to_import_map_value_from_imports());
@ -1325,15 +1328,15 @@ impl ConfigData {
{
import_map_specifier = Some(specifier);
}
} else if let Some(import_map_str) = &settings.import_map {
if let Ok(specifier) = Url::parse(import_map_str) {
import_map_specifier = Some(specifier);
} else if let Some(folder_uri) = workspace_folder {
if let Ok(specifier) = folder_uri.join(import_map_str) {
import_map_specifier = Some(specifier);
}
}
}
import_map_specifier = import_map_specifier.or_else(|| {
let import_map_str = settings.import_map.as_ref()?;
let specifier = Url::parse(import_map_str)
.ok()
.or_else(|| workspace_folder?.join(import_map_str).ok())?;
import_map_from_settings = true;
Some(specifier)
});
if let Some(specifier) = &import_map_specifier {
if let Ok(path) = specifier_to_file_path(specifier) {
watched_files
@ -1416,6 +1419,7 @@ impl ConfigData {
lockfile: lockfile.map(Mutex::new).map(Arc::new),
package_json: package_json.map(Arc::new),
import_map: import_map.map(Arc::new),
import_map_from_settings,
watched_files,
}
}

View File

@ -1393,10 +1393,15 @@ impl Documents {
}
}
self.open_docs = open_docs;
let mut preload_count = 0;
for specifier in workspace_files {
if !config.specifier_enabled(specifier) {
continue;
}
if preload_count >= config.settings.unscoped.document_preload_limit {
break;
}
preload_count += 1;
if !self.open_docs.contains_key(specifier)
&& !fs_docs.docs.contains_key(specifier)
{

View File

@ -832,50 +832,6 @@ impl Inner {
file_fetcher
}
fn resolve_import_map_specifier(
&self,
referrer: &ModuleSpecifier,
) -> Result<Option<ModuleSpecifier>, AnyError> {
let Some(import_map_str) = self
.config
.settings
.get_for_specifier(referrer)
.0
.import_map
.clone()
.and_then(|s| if s.is_empty() { None } else { Some(s) })
else {
return Ok(None);
};
lsp_log!(
"Using import map from workspace settings: \"{}\"",
import_map_str
);
if let Some(config_file) =
self.config.tree.config_file_for_specifier(referrer)
{
if let Some(import_map_path) = &config_file.json.import_map {
lsp_log!("Warning: Import map \"{}\" configured in \"{}\" being ignored due to an import map being explicitly configured in workspace settings.", import_map_path, config_file.specifier);
}
}
if let Ok(url) = Url::parse(&import_map_str) {
Ok(Some(url))
} else if let Some(root_uri) = self.config.root_uri() {
let root_path = specifier_to_file_path(root_uri)?;
let import_map_path = root_path.join(&import_map_str);
let import_map_url =
Url::from_file_path(import_map_path).map_err(|_| {
anyhow!("Bad file path for import map: {}", import_map_str)
})?;
Ok(Some(import_map_url))
} else {
Err(anyhow!(
"The path to the import map (\"{}\") is not resolvable.",
import_map_str
))
}
}
pub fn update_debug_flag(&self) {
let internal_debug = self.config.workspace_settings().internal_debug;
super::logging::set_lsp_debug_flag(internal_debug)
@ -1069,8 +1025,7 @@ impl Inner {
fn walk_workspace(config: &Config) -> (BTreeSet<ModuleSpecifier>, bool) {
let mut workspace_files = Default::default();
let document_preload_limit =
config.workspace_settings().document_preload_limit;
let entry_limit = 1000;
let mut pending = VecDeque::new();
let mut entry_count = 0;
let mut roots = config
@ -1091,7 +1046,7 @@ impl Inner {
let Ok(entry) = entry else {
continue;
};
if entry_count >= document_preload_limit {
if entry_count >= entry_limit {
return (workspace_files, true);
}
entry_count += 1;
@ -3545,7 +3500,7 @@ impl Inner {
vec![referrer.clone()]
};
let workspace_settings = self.config.workspace_settings();
let mut cli_options = CliOptions::new(
let cli_options = CliOptions::new(
Flags {
cache_path: self.maybe_global_cache_path.clone(),
ca_stores: workspace_settings.certificate_stores.clone(),
@ -3553,6 +3508,12 @@ impl Inner {
unsafely_ignore_certificate_errors: workspace_settings
.unsafely_ignore_certificate_errors
.clone(),
import_map_path: config_data.as_ref().and_then(|d| {
if d.import_map_from_settings {
return Some(d.import_map.as_ref()?.base_url().to_string());
}
None
}),
node_modules_dir: Some(
config_data
.as_ref()
@ -3573,11 +3534,6 @@ impl Inner {
.and_then(|d| d.package_json.as_deref().cloned()),
force_global_cache,
)?;
// don't use the specifier in self.maybe_import_map because it's not
// necessarily an import map specifier (could be a deno.json)
if let Some(import_map) = self.resolve_import_map_specifier(&referrer)? {
cli_options.set_import_map_specifier(Some(import_map));
}
let open_docs = self.documents.documents(DocumentsFilter::OpenDiagnosable);
Ok(Some(PrepareCacheResult {

View File

@ -2568,6 +2568,70 @@ fn lsp_rename_synbol_file_scheme_edits_only() {
client.shutdown();
}
// Regression test for https://github.com/denoland/deno/issues/23121.
#[test]
fn lsp_document_preload_limit_zero_deno_json_detection() {
let context = TestContextBuilder::new()
.use_http_server()
.use_temp_cwd()
.build();
let temp_dir = context.temp_dir();
temp_dir.write("deno.json", json!({}).to_string());
let mut client = context.new_lsp_command().build();
client.initialize(|builder| {
builder.set_preload_limit(0);
});
let res = client
.read_notification_with_method::<Value>("deno/didChangeDenoConfiguration");
assert_eq!(
res,
Some(json!({
"changes": [{
"scopeUri": temp_dir.uri(),
"fileUri": temp_dir.uri().join("deno.json").unwrap(),
"type": "added",
"configurationType": "denoJson",
}],
}))
);
client.shutdown();
}
// Regression test for https://github.com/denoland/deno/issues/23141.
#[test]
fn lsp_import_map_setting_with_deno_json() {
let context = TestContextBuilder::new()
.use_http_server()
.use_temp_cwd()
.build();
let temp_dir = context.temp_dir();
temp_dir.write("deno.json", json!({}).to_string());
temp_dir.write(
"import_map.json",
json!({
"imports": {
"file2": "./file2.ts",
},
})
.to_string(),
);
temp_dir.write("file2.ts", "");
let mut client = context.new_lsp_command().build();
client.initialize(|builder| {
builder.set_import_map("import_map.json");
});
let diagnostics = client.did_open(json!({
"textDocument": {
"uri": "file:///a/file.ts",
"languageId": "typescript",
"version": 1,
"text": "import \"file2\";\n",
},
}));
assert_eq!(json!(diagnostics.all()), json!([]));
client.shutdown();
}
#[test]
fn lsp_hover_typescript_types() {
let context = TestContextBuilder::new()