Skip to content

Commit

Permalink
refactor(lsp): unify resolver types into LspResolver (#23514)
Browse files Browse the repository at this point in the history
  • Loading branch information
nayeemrmn committed Apr 26, 2024
1 parent c519355 commit f8ddcc4
Show file tree
Hide file tree
Showing 8 changed files with 451 additions and 376 deletions.
23 changes: 8 additions & 15 deletions cli/lsp/analysis.rs
Expand Up @@ -4,11 +4,10 @@ use super::diagnostics::DenoDiagnostic;
use super::diagnostics::DiagnosticSource;
use super::documents::Documents;
use super::language_server;
use super::resolver::LspResolver;
use super::tsc;

use crate::args::jsr_url;
use crate::npm::CliNpmResolver;
use crate::resolver::CliNodeResolver;
use crate::tools::lint::create_linter;
use deno_runtime::fs_util::specifier_to_file_path;

Expand All @@ -27,7 +26,6 @@ use deno_lint::diagnostic::LintDiagnostic;
use deno_lint::rules::LintRule;
use deno_runtime::deno_node::NpmResolver;
use deno_runtime::deno_node::PathClean;
use deno_runtime::permissions::PermissionsContainer;
use deno_semver::jsr::JsrPackageNvReference;
use deno_semver::jsr::JsrPackageReqReference;
use deno_semver::npm::NpmPackageReqReference;
Expand Down Expand Up @@ -217,22 +215,19 @@ fn code_as_string(code: &Option<lsp::NumberOrString>) -> String {
pub struct TsResponseImportMapper<'a> {
documents: &'a Documents,
maybe_import_map: Option<&'a ImportMap>,
node_resolver: Option<&'a CliNodeResolver>,
npm_resolver: Option<&'a dyn CliNpmResolver>,
resolver: &'a LspResolver,
}

impl<'a> TsResponseImportMapper<'a> {
pub fn new(
documents: &'a Documents,
maybe_import_map: Option<&'a ImportMap>,
node_resolver: Option<&'a CliNodeResolver>,
npm_resolver: Option<&'a dyn CliNpmResolver>,
resolver: &'a LspResolver,
) -> Self {
Self {
documents,
maybe_import_map,
node_resolver,
npm_resolver,
resolver,
}
}

Expand Down Expand Up @@ -304,9 +299,7 @@ impl<'a> TsResponseImportMapper<'a> {
return Some(spec_str);
}

if let Some(npm_resolver) =
self.npm_resolver.as_ref().and_then(|r| r.as_managed())
{
if let Some(npm_resolver) = self.resolver.maybe_managed_npm_resolver() {
if npm_resolver.in_npm_package(specifier) {
if let Ok(Some(pkg_id)) =
npm_resolver.resolve_pkg_id_from_specifier(specifier)
Expand Down Expand Up @@ -370,9 +363,9 @@ impl<'a> TsResponseImportMapper<'a> {
&self,
specifier: &ModuleSpecifier,
) -> Option<String> {
let node_resolver = self.node_resolver?;
let package_json = node_resolver
.get_closest_package_json(specifier, &PermissionsContainer::allow_all())
let package_json = self
.resolver
.get_closest_package_json(specifier)
.ok()
.flatten()?;
let root_folder = package_json.path.parent()?;
Expand Down
2 changes: 2 additions & 0 deletions cli/lsp/config.rs
Expand Up @@ -1150,6 +1150,7 @@ pub enum ConfigWatchedFileType {
/// Contains the config file and dependent information.
#[derive(Debug, Clone)]
pub struct ConfigData {
pub scope: ModuleSpecifier,
pub config_file: Option<Arc<ConfigFile>>,
pub fmt_options: Arc<FmtOptions>,
pub lint_options: Arc<LintOptions>,
Expand Down Expand Up @@ -1487,6 +1488,7 @@ impl ConfigData {
let ts_config = LspTsConfig::new(config_file.as_ref(), import_map.as_ref());

ConfigData {
scope: scope.clone(),
config_file: config_file.map(Arc::new),
fmt_options,
lint_options,
Expand Down
31 changes: 12 additions & 19 deletions cli/lsp/diagnostics.rs
Expand Up @@ -809,10 +809,8 @@ fn generate_lint_diagnostics(
break;
}
// ignore any npm package files
if let Some(npm) = &snapshot.npm {
if npm.node_resolver.in_npm_package(specifier) {
continue;
}
if snapshot.resolver.in_npm_package(specifier) {
continue;
}
let version = document.maybe_lsp_version();
let (lint_options, lint_rules) = config
Expand Down Expand Up @@ -1347,6 +1345,7 @@ fn diagnose_resolution(
diagnostics.push(DenoDiagnostic::DenoWarn(message));
}
}
let managed_npm_resolver = snapshot.resolver.maybe_managed_npm_resolver();
if let Some(doc) = snapshot.documents.get(specifier) {
if let Some(diagnostic) = check_redirect_diagnostic(specifier, &doc) {
diagnostics.push(diagnostic);
Expand Down Expand Up @@ -1375,11 +1374,7 @@ fn diagnose_resolution(
} else if let Ok(pkg_ref) =
NpmPackageReqReference::from_specifier(specifier)
{
if let Some(npm_resolver) = snapshot
.npm
.as_ref()
.and_then(|n| n.npm_resolver.as_managed())
{
if let Some(npm_resolver) = managed_npm_resolver {
// show diagnostics for npm package references that aren't cached
let req = pkg_ref.into_inner().req;
if !npm_resolver.is_pkg_req_folder_cached(&req) {
Expand All @@ -1406,11 +1401,7 @@ fn diagnose_resolution(
diagnostics
.push(DenoDiagnostic::BareNodeSpecifier(module_name.to_string()));
}
} else if let Some(npm_resolver) = snapshot
.npm
.as_ref()
.and_then(|n| n.npm_resolver.as_managed())
{
} else if let Some(npm_resolver) = managed_npm_resolver {
// check that a @types/node package exists in the resolver
let types_node_req = PackageReq::from_str("@types/node").unwrap();
if !npm_resolver.is_pkg_req_folder_cached(&types_node_req) {
Expand Down Expand Up @@ -1451,10 +1442,8 @@ fn diagnose_dependency(
dependency_key: &str,
dependency: &deno_graph::Dependency,
) {
if let Some(npm) = &snapshot.npm {
if npm.npm_resolver.in_npm_package(referrer) {
return; // ignore, surface typescript errors instead
}
if snapshot.resolver.in_npm_package(referrer) {
return; // ignore, surface typescript errors instead
}

let import_map = snapshot.config.tree.root_import_map();
Expand Down Expand Up @@ -1592,6 +1581,7 @@ mod tests {
use crate::lsp::documents::Documents;
use crate::lsp::documents::LanguageId;
use crate::lsp::language_server::StateSnapshot;
use crate::lsp::resolver::LspResolver;
use deno_config::ConfigFile;
use pretty_assertions::assert_eq;
use std::path::Path;
Expand Down Expand Up @@ -1630,6 +1620,9 @@ mod tests {
.unwrap();
config.tree.inject_config_file(config_file).await;
}
let resolver = LspResolver::default()
.with_new_config(&config, None, None)
.await;
StateSnapshot {
project_version: 0,
documents,
Expand All @@ -1638,7 +1631,7 @@ mod tests {
GlobalHttpCache::new(location.to_path_buf(), RealDenoCacheEnv),
)),
config: config.snapshot(),
npm: None,
resolver,
}
}

Expand Down

0 comments on commit f8ddcc4

Please sign in to comment.