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

feat(lsp): auto-import completions from byonm dependencies #26680

Merged
merged 9 commits into from
Nov 6, 2024
76 changes: 52 additions & 24 deletions cli/lsp/analysis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ use super::urls::url_to_uri;
use crate::args::jsr_url;
use crate::lsp::search::PackageSearchApi;
use crate::tools::lint::CliLinter;
use crate::util::path::relative_specifier;
use deno_config::workspace::MappedResolution;
use deno_graph::source::ResolutionMode;
use deno_lint::diagnostic::LintDiagnosticRange;

use deno_ast::SourceRange;
Expand Down Expand Up @@ -228,6 +230,7 @@ pub struct TsResponseImportMapper<'a> {
documents: &'a Documents,
maybe_import_map: Option<&'a ImportMap>,
resolver: &'a LspResolver,
tsc_specifier_map: &'a tsc::TscSpecifierMap,
file_referrer: ModuleSpecifier,
}

Expand All @@ -236,12 +239,14 @@ impl<'a> TsResponseImportMapper<'a> {
documents: &'a Documents,
maybe_import_map: Option<&'a ImportMap>,
resolver: &'a LspResolver,
tsc_specifier_map: &'a tsc::TscSpecifierMap,
file_referrer: &ModuleSpecifier,
) -> Self {
Self {
documents,
maybe_import_map,
resolver,
tsc_specifier_map,
file_referrer: file_referrer.clone(),
}
}
Expand Down Expand Up @@ -387,6 +392,11 @@ impl<'a> TsResponseImportMapper<'a> {
}
}
}
} else if let Some(dep_name) = self
.resolver
.file_url_to_package_json_dep(specifier, Some(&self.file_referrer))
{
return Some(dep_name);
}

// check if the import map has this specifier
Expand Down Expand Up @@ -457,19 +467,36 @@ impl<'a> TsResponseImportMapper<'a> {
specifier: &str,
referrer: &ModuleSpecifier,
) -> Option<String> {
if let Ok(specifier) = referrer.join(specifier) {
if let Some(specifier) = self.check_specifier(&specifier, referrer) {
return Some(specifier);
}
}
let specifier = specifier.strip_suffix(".js").unwrap_or(specifier);
for ext in SUPPORTED_EXTENSIONS {
let specifier_with_ext = format!("{specifier}{ext}");
if self
.documents
.contains_import(&specifier_with_ext, referrer)
let specifier_stem = specifier.strip_suffix(".js").unwrap_or(specifier);
let specifiers = std::iter::once(Cow::Borrowed(specifier)).chain(
SUPPORTED_EXTENSIONS
.iter()
.map(|ext| Cow::Owned(format!("{specifier_stem}{ext}"))),
);
for specifier in specifiers {
if let Some(specifier) = self
.resolver
.as_graph_resolver(Some(&self.file_referrer))
.resolve(
&specifier,
&deno_graph::Range {
specifier: referrer.clone(),
start: deno_graph::Position::zeroed(),
end: deno_graph::Position::zeroed(),
},
ResolutionMode::Types,
)
.ok()
.and_then(|s| self.tsc_specifier_map.normalize(s.as_str()).ok())
.filter(|s| self.documents.exists(s, Some(&self.file_referrer)))
{
return Some(specifier_with_ext);
if let Some(specifier) = self
.check_specifier(&specifier, referrer)
.or_else(|| relative_specifier(referrer, &specifier))
.filter(|s| !s.contains("/node_modules/"))
{
return Some(specifier);
}
}
}
None
Expand Down Expand Up @@ -559,8 +586,9 @@ fn try_reverse_map_package_json_exports(
pub fn fix_ts_import_changes(
referrer: &ModuleSpecifier,
changes: &[tsc::FileTextChanges],
import_mapper: &TsResponseImportMapper,
language_server: &language_server::Inner,
) -> Result<Vec<tsc::FileTextChanges>, AnyError> {
let import_mapper = language_server.get_ts_response_import_mapper(referrer);
let mut r = Vec::new();
for change in changes {
let mut text_changes = Vec::new();
Expand Down Expand Up @@ -605,7 +633,7 @@ pub fn fix_ts_import_changes(
fn fix_ts_import_action<'a>(
referrer: &ModuleSpecifier,
action: &'a tsc::CodeFixAction,
import_mapper: &TsResponseImportMapper,
language_server: &language_server::Inner,
) -> Option<Cow<'a, tsc::CodeFixAction>> {
if !matches!(
action.fix_name.as_str(),
Expand All @@ -621,6 +649,7 @@ fn fix_ts_import_action<'a>(
let Some(specifier) = specifier else {
return Some(Cow::Borrowed(action));
};
let import_mapper = language_server.get_ts_response_import_mapper(referrer);
if let Some(new_specifier) =
import_mapper.check_unresolved_specifier(specifier, referrer)
{
Expand Down Expand Up @@ -728,7 +757,7 @@ pub fn ts_changes_to_edit(
}))
}

#[derive(Debug, Deserialize)]
#[derive(Debug, Deserialize, Serialize)]
#[serde(rename_all = "camelCase")]
pub struct CodeActionData {
pub specifier: ModuleSpecifier,
Expand Down Expand Up @@ -998,11 +1027,8 @@ impl CodeActionCollection {
"The action returned from TypeScript is unsupported.",
));
}
let Some(action) = fix_ts_import_action(
specifier,
action,
&language_server.get_ts_response_import_mapper(specifier),
) else {
let Some(action) = fix_ts_import_action(specifier, action, language_server)
else {
return Ok(());
};
let edit = ts_changes_to_edit(&action.changes, language_server)?;
Expand Down Expand Up @@ -1051,10 +1077,12 @@ impl CodeActionCollection {
specifier: &ModuleSpecifier,
diagnostic: &lsp::Diagnostic,
) {
let data = Some(json!({
"specifier": specifier,
"fixId": action.fix_id,
}));
let data = action.fix_id.as_ref().map(|fix_id| {
json!(CodeActionData {
specifier: specifier.clone(),
fix_id: fix_id.clone(),
})
});
let title = if let Some(description) = &action.fix_all_description {
description.clone()
} else {
Expand Down
28 changes: 0 additions & 28 deletions cli/lsp/documents.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1059,34 +1059,6 @@ impl Documents {
self.cache.is_valid_file_referrer(specifier)
}

/// Return `true` if the provided specifier can be resolved to a document,
/// otherwise `false`.
pub fn contains_import(
&self,
specifier: &str,
referrer: &ModuleSpecifier,
) -> bool {
let file_referrer = self.get_file_referrer(referrer);
let maybe_specifier = self
.resolver
.as_graph_resolver(file_referrer.as_deref())
.resolve(
specifier,
&deno_graph::Range {
specifier: referrer.clone(),
start: deno_graph::Position::zeroed(),
end: deno_graph::Position::zeroed(),
},
ResolutionMode::Types,
)
.ok();
if let Some(import_specifier) = maybe_specifier {
self.exists(&import_specifier, file_referrer.as_deref())
} else {
false
}
}

pub fn resolve_document_specifier(
&self,
specifier: &ModuleSpecifier,
Expand Down
13 changes: 9 additions & 4 deletions cli/lsp/language_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1837,7 +1837,7 @@ impl Inner {
fix_ts_import_changes(
&code_action_data.specifier,
&combined_code_actions.changes,
&self.get_ts_response_import_mapper(&code_action_data.specifier),
self,
)
.map_err(|err| {
error!("Unable to remap changes: {:#}", err);
Expand Down Expand Up @@ -1890,7 +1890,7 @@ impl Inner {
refactor_edit_info.edits = fix_ts_import_changes(
&action_data.specifier,
&refactor_edit_info.edits,
&self.get_ts_response_import_mapper(&action_data.specifier),
self,
)
.map_err(|err| {
error!("Unable to remap changes: {:#}", err);
Expand Down Expand Up @@ -1921,7 +1921,8 @@ impl Inner {
// todo(dsherret): this should probably just take the resolver itself
// as the import map is an implementation detail
.and_then(|d| d.resolver.maybe_import_map()),
self.resolver.as_ref(),
&self.resolver,
&self.ts_server.specifier_map,
file_referrer,
)
}
Expand Down Expand Up @@ -2284,7 +2285,11 @@ impl Inner {
.into(),
scope.cloned(),
)
.await;
.await
.unwrap_or_else(|err| {
error!("Unable to get completion info from TypeScript: {:#}", err);
None
});

if let Some(completions) = maybe_completion_info {
response = Some(
Expand Down
45 changes: 45 additions & 0 deletions cli/lsp/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ struct LspScopeResolver {
pkg_json_resolver: Option<Arc<PackageJsonResolver>>,
redirect_resolver: Option<Arc<RedirectResolver>>,
graph_imports: Arc<IndexMap<ModuleSpecifier, GraphImport>>,
package_json_deps_by_resolution: Arc<IndexMap<ModuleSpecifier, String>>,
config_data: Option<Arc<ConfigData>>,
}

Expand All @@ -88,6 +89,7 @@ impl Default for LspScopeResolver {
pkg_json_resolver: None,
redirect_resolver: None,
graph_imports: Default::default(),
package_json_deps_by_resolution: Default::default(),
config_data: None,
}
}
Expand Down Expand Up @@ -165,6 +167,33 @@ impl LspScopeResolver {
)
})
.unwrap_or_default();
let package_json_deps_by_resolution = (|| {
let node_resolver = node_resolver.as_ref()?;
let package_json = config_data?.maybe_pkg_json()?;
let referrer = package_json.specifier();
let dependencies = package_json.dependencies.as_ref()?;
let result = dependencies
.iter()
.flat_map(|(name, _)| {
let req_ref =
NpmPackageReqReference::from_str(&format!("npm:{name}")).ok()?;
let specifier = into_specifier_and_media_type(Some(
node_resolver
.resolve_req_reference(
&req_ref,
&referrer,
NodeResolutionMode::Types,
)
.ok()?,
))
.0;
Some((specifier, name.clone()))
})
.collect();
Some(result)
})();
let package_json_deps_by_resolution =
Arc::new(package_json_deps_by_resolution.unwrap_or_default());
Self {
cjs_tracker: lsp_cjs_tracker,
graph_resolver,
Expand All @@ -174,6 +203,7 @@ impl LspScopeResolver {
pkg_json_resolver: Some(pkg_json_resolver),
redirect_resolver,
graph_imports,
package_json_deps_by_resolution,
config_data: config_data.cloned(),
}
}
Expand Down Expand Up @@ -216,6 +246,9 @@ impl LspScopeResolver {
redirect_resolver: self.redirect_resolver.clone(),
pkg_json_resolver: Some(pkg_json_resolver),
graph_imports: self.graph_imports.clone(),
package_json_deps_by_resolution: self
.package_json_deps_by_resolution
.clone(),
config_data: self.config_data.clone(),
})
}
Expand Down Expand Up @@ -407,6 +440,18 @@ impl LspResolver {
)))
}

pub fn file_url_to_package_json_dep(
&self,
specifier: &ModuleSpecifier,
file_referrer: Option<&ModuleSpecifier>,
) -> Option<String> {
let resolver = self.get_scope_resolver(file_referrer);
resolver
.package_json_deps_by_resolution
.get(specifier)
.cloned()
}

pub fn in_node_modules(&self, specifier: &ModuleSpecifier) -> bool {
fn has_node_modules_dir(specifier: &ModuleSpecifier) -> bool {
// consider any /node_modules/ directory as being in the node_modules
Expand Down
Loading
Loading