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

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

38 changes: 29 additions & 9 deletions cli/lsp/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1305,6 +1305,33 @@ fn diagnose_resolution(
is_dynamic: bool,
maybe_assert_type: Option<&str>,
) -> 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![];
match resolution {
Resolution::Ok(resolved) => {
Expand All @@ -1319,15 +1346,8 @@ fn diagnose_resolution(
}
}
if let Some(doc) = snapshot.documents.get(specifier) {
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 {
diagnostics.push(DenoDiagnostic::Redirect {
from: specifier.clone(),
to: doc_specifier.clone(),
});
if let Some(diagnostic) = check_redirect_diagnostic(specifier, &doc) {
diagnostics.push(diagnostic);
}
if doc.media_type() == MediaType::Json {
match maybe_assert_type {
Expand Down
28 changes: 27 additions & 1 deletion tests/integration/lsp_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11318,12 +11318,38 @@ fn lsp_sloppy_imports_warn() {
"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!({
"textDocument": {
"uri": temp_dir.join("file.ts").uri_file(),
"languageId": "typescript",
"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!(
Expand Down