1
0
Fork 0
mirror of https://github.com/denoland/deno.git synced 2024-11-21 15:04:11 -05:00

fix(lsp): ignore code errors when type passes for non-@deno-types reolution (#22682)

This commit is contained in:
David Sherret 2024-03-04 11:10:39 -05:00 committed by GitHub
parent 625a9319f6
commit 6650019935
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 63 additions and 11 deletions

View file

@ -1456,12 +1456,28 @@ fn diagnose_dependency(
.iter() .iter()
.map(|i| documents::to_lsp_range(&i.range)) .map(|i| documents::to_lsp_range(&i.range))
.collect(); .collect();
// TODO(nayeemrmn): This is a crude way of detecting `@deno-types` which has
// a different specifier and therefore needs a separate call to
// `diagnose_resolution()`. It would be much cleaner if that were modelled as
// a separate dependency: https://github.com/denoland/deno_graph/issues/247.
let is_types_deno_types = !dependency.maybe_type.is_none()
&& !dependency
.imports
.iter()
.any(|i| dependency.maybe_type.includes(&i.range.start).is_some());
diagnostics.extend( diagnostics.extend(
diagnose_resolution( diagnose_resolution(
snapshot, snapshot,
dependency_key, dependency_key,
if dependency.maybe_code.is_none() { if dependency.maybe_code.is_none()
// If not @deno-types, diagnose the types if the code errored because
// it's likely resolving into the node_modules folder, which might be
// erroring correctly due to resolution only being for bundlers. Let this
// fail at runtime if necesarry, but don't bother erroring in the editor
|| !is_types_deno_types && matches!(dependency.maybe_type, Resolution::Ok(_))
&& matches!(dependency.maybe_code, Resolution::Err(_))
{
&dependency.maybe_type &dependency.maybe_type
} else { } else {
&dependency.maybe_code &dependency.maybe_code
@ -1476,16 +1492,8 @@ fn diagnose_dependency(
.map(|range| diag.to_lsp_diagnostic(range)) .map(|range| diag.to_lsp_diagnostic(range))
}), }),
); );
// TODO(nayeemrmn): This is a crude way of detecting `@deno-types` which has
// a different specifier and therefore needs a separate call to if is_types_deno_types {
// `diagnose_resolution()`. It would be much cleaner if that were modelled as
// a separate dependency: https://github.com/denoland/deno_graph/issues/247.
if !dependency.maybe_type.is_none()
&& !dependency
.imports
.iter()
.any(|i| dependency.maybe_type.includes(&i.range.start).is_some())
{
let range = match &dependency.maybe_type { let range = match &dependency.maybe_type {
Resolution::Ok(resolved) => documents::to_lsp_range(&resolved.range), Resolution::Ok(resolved) => documents::to_lsp_range(&resolved.range),
Resolution::Err(error) => documents::to_lsp_range(error.range()), Resolution::Err(error) => documents::to_lsp_range(error.range()),

View file

@ -8659,6 +8659,39 @@ fn lsp_ts_diagnostics_refresh_on_lsp_version_reset() {
client.shutdown(); client.shutdown();
} }
#[test]
fn lsp_diagnostics_none_for_resolving_types() {
let context = TestContextBuilder::for_npm().use_temp_cwd().build();
context
.temp_dir()
.write("deno.json", r#"{ "unstable": ["byonm"] }"#);
context.temp_dir().write(
"package.json",
r#"{ "dependencies": { "@denotest/monaco-editor": "*" } }"#,
);
context.run_npm("install");
let mut client = context.new_lsp_command().build();
client.initialize_default();
// The types for this package will succeed, but the code will fail
// because the package is only made for bundling and not meant to
// run in Deno or Node.
let diagnostics = client.did_open(json!({
"textDocument": {
"uri": context.temp_dir().path().join("file.ts").uri_file(),
"languageId": "typescript",
"version": 1,
"text": concat!(
"import * as a from \"@denotest/monaco-editor\";\n",
"console.log(new a.Editor())\n",
)
},
}));
let diagnostics = diagnostics.all();
assert!(diagnostics.is_empty(), "{:?}", diagnostics);
client.shutdown();
}
#[test] #[test]
fn lsp_jupyter_diagnostics() { fn lsp_jupyter_diagnostics() {
let context = TestContextBuilder::new().use_temp_cwd().build(); let context = TestContextBuilder::new().use_temp_cwd().build();

View file

@ -0,0 +1,4 @@
// The monaco-editor package uses an entry in the package.json
// where it has no "type": "module" and then only specifies a
// "module": "./main.js"-like entry that points at an ESM file.
export class Editor {}

View file

@ -0,0 +1 @@
export class Editor {}

View file

@ -0,0 +1,6 @@
{
"name": "@denotest/monaco-editor",
"version": "1.0.0",
"module": "./main.js",
"types": "./main.types.d.ts"
}