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): don't use global cache paths for scope allocation #24353

Merged
merged 2 commits into from
Jun 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 9 additions & 0 deletions cli/lsp/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,4 +184,13 @@ impl LspCache {
.as_ref()?;
vendor.get_remote_url(&path)
}

pub fn is_valid_file_referrer(&self, specifier: &ModuleSpecifier) -> bool {
if let Ok(path) = specifier_to_file_path(specifier) {
if !path.starts_with(&self.deno_dir().root) {
return true;
}
}
false
}
}
47 changes: 28 additions & 19 deletions cli/lsp/documents.rs
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ impl Document {
file_referrer: Option<ModuleSpecifier>,
) -> Arc<Self> {
let file_referrer = Some(&specifier)
.filter(|s| s.scheme() == "file")
.filter(|s| cache.is_valid_file_referrer(s))
.cloned()
.or(file_referrer);
let media_type = resolve_media_type(
Expand Down Expand Up @@ -804,7 +804,7 @@ impl FileSystemDocuments {
file_referrer: Option<&ModuleSpecifier>,
) -> Option<Arc<Document>> {
let file_referrer = Some(specifier)
.filter(|s| s.scheme() == "file")
.filter(|s| cache.is_valid_file_referrer(s))
.or(file_referrer);
let new_fs_version = calculate_fs_version(cache, specifier, file_referrer);
let old_doc = self.docs.get(specifier).map(|v| v.value().clone());
Expand Down Expand Up @@ -1051,13 +1051,16 @@ impl Documents {
&self,
specifier: &'a ModuleSpecifier,
) -> Option<Cow<'a, ModuleSpecifier>> {
if specifier.scheme() == "file" {
Some(Cow::Borrowed(specifier))
} else {
self
.get(specifier)
.and_then(|d| d.file_referrer().cloned().map(Cow::Owned))
if self.is_valid_file_referrer(specifier) {
return Some(Cow::Borrowed(specifier));
}
self
.get(specifier)
.and_then(|d| d.file_referrer().cloned().map(Cow::Owned))
}

pub fn is_valid_file_referrer(&self, specifier: &ModuleSpecifier) -> bool {
self.cache.is_valid_file_referrer(specifier)
}

/// Return `true` if the provided specifier can be resolved to a document,
Expand Down Expand Up @@ -1447,20 +1450,25 @@ impl Documents {
return Some((specifier.clone(), MediaType::Dts));
}
}

if let Ok(npm_ref) = NpmPackageReqReference::from_specifier(specifier) {
return self
.resolver
.npm_to_file_url(&npm_ref, referrer, file_referrer);
let mut specifier = specifier.clone();
let mut media_type = None;
if let Ok(npm_ref) = NpmPackageReqReference::from_specifier(&specifier) {
let (s, mt) =
self
.resolver
.npm_to_file_url(&npm_ref, referrer, file_referrer)?;
specifier = s;
media_type = Some(mt);
}
let Some(doc) = self.get_or_load(specifier, referrer) else {
return Some((specifier.clone(), MediaType::from_specifier(specifier)));
let Some(doc) = self.get_or_load(&specifier, referrer) else {
let media_type =
media_type.unwrap_or_else(|| MediaType::from_specifier(&specifier));
return Some((specifier, media_type));
};
if let Some(types) = doc.maybe_types_dependency().maybe_specifier() {
self.resolve_dependency(types, specifier, file_referrer)
self.resolve_dependency(types, &specifier, file_referrer)
} else {
let media_type = doc.media_type();
Some((doc.specifier().clone(), media_type))
Some((doc.specifier().clone(), doc.media_type()))
}
}
}
Expand Down Expand Up @@ -1593,7 +1601,8 @@ mod tests {

async fn setup() -> (Documents, LspCache, TempDir) {
let temp_dir = TempDir::new();
let cache = LspCache::new(Some(temp_dir.uri()));
temp_dir.create_dir_all(".deno_dir");
let cache = LspCache::new(Some(temp_dir.uri().join(".deno_dir").unwrap()));
let config = Config::default();
let resolver =
Arc::new(LspResolver::from_config(&config, &cache, None).await);
Expand Down
12 changes: 8 additions & 4 deletions cli/lsp/language_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1058,8 +1058,10 @@ impl Inner {
params.text_document.uri
);
}
let file_referrer = (params.text_document.uri.scheme() == "file")
.then(|| params.text_document.uri.clone());
let file_referrer = (self
.documents
.is_valid_file_referrer(&params.text_document.uri))
.then(|| params.text_document.uri.clone());
let specifier = self
.url_map
.normalize_url(&params.text_document.uri, LspUrlKind::File);
Expand Down Expand Up @@ -1308,8 +1310,10 @@ impl Inner {
&self,
params: DocumentFormattingParams,
) -> LspResult<Option<Vec<TextEdit>>> {
let file_referrer = (params.text_document.uri.scheme() == "file")
.then(|| params.text_document.uri.clone());
let file_referrer = (self
.documents
.is_valid_file_referrer(&params.text_document.uri))
.then(|| params.text_document.uri.clone());
let mut specifier = self
.url_map
.normalize_url(&params.text_document.uri, LspUrlKind::File);
Expand Down
2 changes: 1 addition & 1 deletion cli/lsp/tsc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,7 @@ impl TsServer {
let mut diagnostics_map = IndexMap::with_capacity(specifiers.len());
let mut specifiers_by_scope = BTreeMap::new();
for specifier in specifiers {
let scope = if specifier.scheme() == "file" {
let scope = if snapshot.documents.is_valid_file_referrer(&specifier) {
snapshot
.config
.tree
Expand Down