fix(lsp): do not warn about local file "redirects" from .js to .d.ts files (#22670)

The diagnostic was incorrect when importing a `.js` file with a
corresponding `.d.ts` file with sloppy imports because it would say to
change the `.js` extension to `.d.ts`, which is incorrect. We might as
well just hide this diagnostic.
This commit is contained in:
David Sherret 2024-03-04 12:34:31 -05:00 committed by GitHub
parent 5a28d70e05
commit 902a1137ea
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 56 additions and 10 deletions

View File

@ -1305,6 +1305,33 @@ fn diagnose_resolution(
is_dynamic: bool, is_dynamic: bool,
maybe_assert_type: Option<&str>, maybe_assert_type: Option<&str>,
) -> Vec<DenoDiagnostic> { ) -> Vec<DenoDiagnostic> {
fn check_redirect_diagnostic(
specifier: &ModuleSpecifier,
doc: &Document,
) -> Option<DenoDiagnostic> {
let doc_specifier = doc.specifier();
// If the module was redirected, we want to issue an informational
// diagnostic that indicates this. This then allows us to issue a code
// action to replace the specifier with the final redirected one.
if specifier.scheme() == "jsr" || doc_specifier == specifier {
return None;
}
// don't bother warning about sloppy import redirects from .js to .d.ts
// because explaining how to fix this via a diagnostic involves using
// @deno-types and that's a bit complicated to explain
let is_sloppy_import_dts_redirect = doc_specifier.scheme() == "file"
&& doc.media_type().is_declaration()
&& !MediaType::from_specifier(specifier).is_declaration();
if is_sloppy_import_dts_redirect {
return None;
}
Some(DenoDiagnostic::Redirect {
from: specifier.clone(),
to: doc_specifier.clone(),
})
}
let mut diagnostics = vec![]; let mut diagnostics = vec![];
match resolution { match resolution {
Resolution::Ok(resolved) => { Resolution::Ok(resolved) => {
@ -1319,15 +1346,8 @@ fn diagnose_resolution(
} }
} }
if let Some(doc) = snapshot.documents.get(specifier) { if let Some(doc) = snapshot.documents.get(specifier) {
let doc_specifier = doc.specifier(); if let Some(diagnostic) = check_redirect_diagnostic(specifier, &doc) {
// If the module was redirected, we want to issue an informational diagnostics.push(diagnostic);
// diagnostic that indicates this. This then allows us to issue a code
// action to replace the specifier with the final redirected one.
if specifier.scheme() != "jsr" && doc_specifier != specifier {
diagnostics.push(DenoDiagnostic::Redirect {
from: specifier.clone(),
to: doc_specifier.clone(),
});
} }
if doc.media_type() == MediaType::Json { if doc.media_type() == MediaType::Json {
match maybe_assert_type { match maybe_assert_type {

View File

@ -11318,12 +11318,38 @@ fn lsp_sloppy_imports_warn() {
"text": "export class B {}", "text": "export class B {}",
}, },
})); }));
client.did_open(json!({
"textDocument": {
"uri": temp_dir.join("c.js").uri_file(),
"languageId": "typescript",
"version": 1,
"text": "export class C {}",
},
}));
client.did_open(json!({
"textDocument": {
"uri": temp_dir.join("c.d.ts").uri_file(),
"languageId": "typescript",
"version": 1,
"text": "export class C {}",
},
}));
let diagnostics = client.did_open(json!({ let diagnostics = client.did_open(json!({
"textDocument": { "textDocument": {
"uri": temp_dir.join("file.ts").uri_file(), "uri": temp_dir.join("file.ts").uri_file(),
"languageId": "typescript", "languageId": "typescript",
"version": 1, "version": 1,
"text": "import * as a from './a';\nimport * as b from './b.js';\nconsole.log(a)\nconsole.log(b);\n", "text": concat!(
"import * as a from './a';\n",
"import * as b from './b.js';\n",
// this one's types resolve to a .d.ts file and we don't
// bother warning about it because it's a bit complicated
// to explain to use @deno-types in a diagnostic
"import * as c from './c.js';\n",
"console.log(a)\n",
"console.log(b);\n",
"console.log(c);\n",
),
}, },
})); }));
assert_eq!( assert_eq!(