Skip to content

Commit 08f5e79

Browse files
authored
fix(check/lsp): fall back to @types/* packages if npm package doesn't have types (#28185)
Fixes #27569. Fixes #27215. This PR makes it so type resolution falls back to looking for definitely typed packages (`@types/foo`) if a given NPM package does not contain type declarations. One complication is choosing _which_ version of the `@types/*` package to use, if the project depends on multiple versions. The heuristic here is to try to match the major and minor versions, falling back to the latest version. So if you have ``` @types/foo: 0.1.0, 0.2.0, 3.1.0, 3.1.2, 4.0.0 foo: 3.1.0 ``` we would choose `@types/[email protected]` when resolving types for `foo`. --- Note that this only uses `@types/` packages if you _already_ depend on them. So a follow up to this PR could be to add a diagnostic and quickfix to install `@types/foo` if we don't find types for `foo`.
1 parent 3da3fe8 commit 08f5e79

File tree

23 files changed

+567
-389
lines changed

23 files changed

+567
-389
lines changed

cli/lsp/analysis.rs

Lines changed: 32 additions & 206 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,9 @@ use deno_core::serde_json::json;
1818
use deno_core::ModuleSpecifier;
1919
use deno_error::JsErrorBox;
2020
use deno_lint::diagnostic::LintDiagnosticRange;
21+
use deno_npm::NpmPackageId;
2122
use deno_path_util::url_to_file_path;
2223
use deno_resolver::npm::managed::NpmResolutionCell;
23-
use deno_resolver::workspace::MappedResolution;
2424
use deno_runtime::deno_node::PathClean;
2525
use deno_semver::jsr::JsrPackageNvReference;
2626
use deno_semver::jsr::JsrPackageReqReference;
@@ -38,23 +38,20 @@ use node_resolver::NodeResolutionKind;
3838
use node_resolver::ResolutionMode;
3939
use once_cell::sync::Lazy;
4040
use regex::Regex;
41-
use text_lines::LineAndColumnIndex;
4241
use tokio_util::sync::CancellationToken;
4342
use tower_lsp::lsp_types as lsp;
4443
use tower_lsp::lsp_types::Position;
4544
use tower_lsp::lsp_types::Range;
4645

4746
use super::diagnostics::DenoDiagnostic;
4847
use super::diagnostics::DiagnosticSource;
49-
use super::documents::Document;
5048
use super::documents::Documents;
5149
use super::language_server;
5250
use super::resolver::LspResolver;
5351
use super::tsc;
5452
use super::urls::url_to_uri;
5553
use crate::args::jsr_url;
5654
use crate::lsp::logging::lsp_warn;
57-
use crate::lsp::search::PackageSearchApi;
5855
use crate::tools::lint::CliLinter;
5956
use crate::util::path::relative_specifier;
6057

@@ -370,9 +367,13 @@ impl<'a> TsResponseImportMapper<'a> {
370367
if let Ok(Some(pkg_id)) =
371368
npm_resolver.resolve_pkg_id_from_specifier(specifier)
372369
{
373-
let pkg_reqs = npm_resolver
374-
.resolution()
375-
.resolve_pkg_reqs_from_pkg_id(&pkg_id);
370+
let pkg_reqs =
371+
maybe_reverse_definitely_typed(&pkg_id, npm_resolver.resolution())
372+
.unwrap_or_else(|| {
373+
npm_resolver
374+
.resolution()
375+
.resolve_pkg_reqs_from_pkg_id(&pkg_id)
376+
});
376377
// check if any pkg reqs match what is found in an import map
377378
if !pkg_reqs.is_empty() {
378379
let sub_path = npm_resolver
@@ -558,6 +559,30 @@ impl<'a> TsResponseImportMapper<'a> {
558559
}
559560
}
560561

562+
fn maybe_reverse_definitely_typed(
563+
pkg_id: &NpmPackageId,
564+
resolution: &NpmResolutionCell,
565+
) -> Option<Vec<PackageReq>> {
566+
let rest = pkg_id.nv.name.strip_prefix("@types/")?;
567+
let package_name = if rest.contains("__") {
568+
Cow::Owned(format!("@{}", rest.replace("__", "/")))
569+
} else {
570+
Cow::Borrowed(rest)
571+
};
572+
573+
let reqs = resolution
574+
.package_reqs()
575+
.into_iter()
576+
.filter_map(|(req, nv)| (*nv.name == package_name).then_some(req))
577+
.collect::<Vec<_>>();
578+
579+
if reqs.is_empty() {
580+
None
581+
} else {
582+
Some(reqs)
583+
}
584+
}
585+
561586
fn try_reverse_map_package_json_exports(
562587
root_path: &Path,
563588
target_path: &Path,
@@ -1267,205 +1292,6 @@ impl CodeActionCollection {
12671292
..Default::default()
12681293
}));
12691294
}
1270-
1271-
pub async fn add_source_actions(
1272-
&mut self,
1273-
document: &Document,
1274-
range: &lsp::Range,
1275-
language_server: &language_server::Inner,
1276-
) {
1277-
fn import_start_from_specifier(
1278-
document: &Document,
1279-
import: &deno_graph::Import,
1280-
) -> Option<LineAndColumnIndex> {
1281-
// find the top level statement that contains the specifier
1282-
let parsed_source = document.maybe_parsed_source()?.as_ref().ok()?;
1283-
let text_info = parsed_source.text_info_lazy();
1284-
let specifier_range = SourceRange::new(
1285-
text_info.loc_to_source_pos(LineAndColumnIndex {
1286-
line_index: import.specifier_range.range.start.line,
1287-
column_index: import.specifier_range.range.start.character,
1288-
}),
1289-
text_info.loc_to_source_pos(LineAndColumnIndex {
1290-
line_index: import.specifier_range.range.end.line,
1291-
column_index: import.specifier_range.range.end.character,
1292-
}),
1293-
);
1294-
1295-
parsed_source
1296-
.program_ref()
1297-
.body()
1298-
.find(|i| i.range().contains(&specifier_range))
1299-
.map(|i| text_info.line_and_column_index(i.range().start))
1300-
}
1301-
1302-
async fn deno_types_for_npm_action(
1303-
document: &Document,
1304-
range: &lsp::Range,
1305-
language_server: &language_server::Inner,
1306-
) -> Option<lsp::CodeAction> {
1307-
fn top_package_req_for_name(
1308-
resolution: &NpmResolutionCell,
1309-
name: &str,
1310-
) -> Option<PackageReq> {
1311-
let package_reqs = resolution.package_reqs();
1312-
let mut entries = package_reqs
1313-
.into_iter()
1314-
.filter(|(_, nv)| nv.name == name)
1315-
.collect::<Vec<_>>();
1316-
entries.sort_by(|a, b| a.1.version.cmp(&b.1.version));
1317-
Some(entries.pop()?.0)
1318-
}
1319-
1320-
let (dep_key, dependency, _) =
1321-
document.get_maybe_dependency(&range.end)?;
1322-
if dependency.maybe_deno_types_specifier.is_some() {
1323-
return None;
1324-
}
1325-
if dependency.maybe_code.maybe_specifier().is_none()
1326-
&& dependency.maybe_type.maybe_specifier().is_none()
1327-
{
1328-
// We're using byonm and the package is not cached.
1329-
return None;
1330-
}
1331-
let position = deno_graph::Position::new(
1332-
range.end.line as usize,
1333-
range.end.character as usize,
1334-
);
1335-
let import_start = dependency.imports.iter().find_map(|i| {
1336-
if json!(i.kind) != json!("es") && json!(i.kind) != json!("tsType") {
1337-
return None;
1338-
}
1339-
if !i.specifier_range.includes(position) {
1340-
return None;
1341-
}
1342-
1343-
import_start_from_specifier(document, i)
1344-
})?;
1345-
let referrer = document.specifier();
1346-
let resolution_mode = document.resolution_mode();
1347-
let file_referrer = document.file_referrer();
1348-
let config_data = language_server
1349-
.config
1350-
.tree
1351-
.data_for_specifier(file_referrer?)?;
1352-
let workspace_resolver = config_data.resolver.clone();
1353-
let npm_ref = if let Ok(resolution) = workspace_resolver.resolve(
1354-
&dep_key,
1355-
document.specifier(),
1356-
deno_resolver::workspace::ResolutionKind::Execution,
1357-
) {
1358-
let specifier = match resolution {
1359-
MappedResolution::Normal { specifier, .. } => specifier,
1360-
_ => {
1361-
return None;
1362-
}
1363-
};
1364-
NpmPackageReqReference::from_specifier(&specifier).ok()?
1365-
} else {
1366-
// Only resolve bare package.json deps for byonm.
1367-
if !config_data.byonm {
1368-
return None;
1369-
}
1370-
if !language_server.resolver.is_bare_package_json_dep(
1371-
&dep_key,
1372-
referrer,
1373-
resolution_mode,
1374-
) {
1375-
return None;
1376-
}
1377-
NpmPackageReqReference::from_str(&format!("npm:{}", &dep_key)).ok()?
1378-
};
1379-
let package_name = &npm_ref.req().name;
1380-
if package_name.starts_with("@types/") {
1381-
return None;
1382-
}
1383-
let managed_npm_resolver = language_server
1384-
.resolver
1385-
.maybe_managed_npm_resolver(file_referrer);
1386-
if let Some(npm_resolver) = managed_npm_resolver {
1387-
if !npm_resolver.is_pkg_req_folder_cached(npm_ref.req()) {
1388-
return None;
1389-
}
1390-
}
1391-
if language_server
1392-
.resolver
1393-
.npm_to_file_url(&npm_ref, referrer, resolution_mode, file_referrer)
1394-
.is_some()
1395-
{
1396-
// The package import has types.
1397-
return None;
1398-
}
1399-
let types_package_name = format!("@types/{package_name}");
1400-
let types_package_version = language_server
1401-
.npm_search_api
1402-
.versions(&types_package_name)
1403-
.await
1404-
.ok()
1405-
.and_then(|versions| versions.first().cloned())?;
1406-
let types_specifier_text =
1407-
if let Some(npm_resolver) = managed_npm_resolver {
1408-
let mut specifier_text = if let Some(req) = top_package_req_for_name(
1409-
npm_resolver.resolution(),
1410-
&types_package_name,
1411-
) {
1412-
format!("npm:{req}")
1413-
} else {
1414-
format!("npm:{}@^{}", &types_package_name, types_package_version)
1415-
};
1416-
let specifier = ModuleSpecifier::parse(&specifier_text).ok()?;
1417-
if let Some(file_referrer) = file_referrer {
1418-
if let Some(text) = language_server
1419-
.get_ts_response_import_mapper(file_referrer)
1420-
.check_specifier(&specifier, referrer)
1421-
{
1422-
specifier_text = text;
1423-
}
1424-
}
1425-
specifier_text
1426-
} else {
1427-
types_package_name.clone()
1428-
};
1429-
let uri = language_server
1430-
.url_map
1431-
.specifier_to_uri(referrer, file_referrer)
1432-
.ok()?;
1433-
let position = lsp::Position {
1434-
line: import_start.line_index as u32,
1435-
character: import_start.column_index as u32,
1436-
};
1437-
let new_text = format!(
1438-
"{}// @ts-types=\"{}\"\n",
1439-
if position.character == 0 { "" } else { "\n" },
1440-
&types_specifier_text
1441-
);
1442-
let text_edit = lsp::TextEdit {
1443-
range: lsp::Range {
1444-
start: position,
1445-
end: position,
1446-
},
1447-
new_text,
1448-
};
1449-
Some(lsp::CodeAction {
1450-
title: format!(
1451-
"Add @ts-types directive for \"{}\"",
1452-
&types_specifier_text
1453-
),
1454-
kind: Some(lsp::CodeActionKind::QUICKFIX),
1455-
diagnostics: None,
1456-
edit: Some(lsp::WorkspaceEdit {
1457-
changes: Some([(uri, vec![text_edit])].into_iter().collect()),
1458-
..Default::default()
1459-
}),
1460-
..Default::default()
1461-
})
1462-
}
1463-
if let Some(action) =
1464-
deno_types_for_npm_action(document, range, language_server).await
1465-
{
1466-
self.actions.push(CodeActionKind::Deno(action));
1467-
}
1468-
}
14691295
}
14701296

14711297
/// Prepend the whitespace characters found at the start of line_content to content.

cli/lsp/language_server.rs

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1847,11 +1847,7 @@ impl Inner {
18471847
}
18481848
}
18491849
}
1850-
if let Some(document) = asset_or_doc.document() {
1851-
code_actions
1852-
.add_source_actions(document, &params.range, self)
1853-
.await;
1854-
}
1850+
18551851
code_actions.set_preferred_fixes();
18561852
all_actions.extend(code_actions.get_response());
18571853

cli/lsp/resolver.rs

Lines changed: 22 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -189,32 +189,34 @@ impl LspScopeResolver {
189189
let result = dependencies
190190
.iter()
191191
.flat_map(|(name, _)| {
192-
let req_ref =
193-
NpmPackageReqReference::from_str(&format!("npm:{name}")).ok()?;
194-
let specifier = into_specifier_and_media_type(Some(
195-
npm_pkg_req_resolver
192+
let mut deps = Vec::with_capacity(2);
193+
let Some(req_ref) =
194+
NpmPackageReqReference::from_str(&format!("npm:{name}")).ok()
195+
else {
196+
return vec![];
197+
};
198+
for kind in [NodeResolutionKind::Types, NodeResolutionKind::Execution]
199+
{
200+
let Some(req) = npm_pkg_req_resolver
196201
.resolve_req_reference(
197202
&req_ref,
198203
&referrer,
199204
// todo(dsherret): this is wrong because it doesn't consider CJS referrers
200205
ResolutionMode::Import,
201-
NodeResolutionKind::Types,
206+
kind,
202207
)
203-
.or_else(|_| {
204-
npm_pkg_req_resolver.resolve_req_reference(
205-
&req_ref,
206-
&referrer,
207-
// todo(dsherret): this is wrong because it doesn't consider CJS referrers
208-
ResolutionMode::Import,
209-
NodeResolutionKind::Execution,
210-
)
211-
})
212-
.ok()?
213-
.into_url()
214-
.ok()?,
215-
))
216-
.0;
217-
Some((specifier, name.clone()))
208+
.ok()
209+
else {
210+
continue;
211+
};
212+
213+
let Some(url) = req.into_url().ok() else {
214+
continue;
215+
};
216+
let specifier = into_specifier_and_media_type(Some(url)).0;
217+
deps.push((specifier, name.clone()))
218+
}
219+
deps
218220
})
219221
.collect();
220222
Some(result)
@@ -579,29 +581,6 @@ impl LspResolver {
579581
has_node_modules_dir(specifier)
580582
}
581583

582-
pub fn is_bare_package_json_dep(
583-
&self,
584-
specifier_text: &str,
585-
referrer: &ModuleSpecifier,
586-
resolution_mode: ResolutionMode,
587-
) -> bool {
588-
let resolver = self.get_scope_resolver(Some(referrer));
589-
let Some(npm_pkg_req_resolver) = resolver.npm_pkg_req_resolver.as_ref()
590-
else {
591-
return false;
592-
};
593-
npm_pkg_req_resolver
594-
.resolve_if_for_npm_pkg(
595-
specifier_text,
596-
referrer,
597-
resolution_mode,
598-
NodeResolutionKind::Types,
599-
)
600-
.ok()
601-
.flatten()
602-
.is_some()
603-
}
604-
605584
pub fn resolve_redirects(
606585
&self,
607586
specifier: &ModuleSpecifier,

0 commit comments

Comments
 (0)