From 902a1137ea6021af8c56b82fa38b8084a89a4bb1 Mon Sep 17 00:00:00 2001
From: David Sherret <dsherret@users.noreply.github.com>
Date: Mon, 4 Mar 2024 12:34:31 -0500
Subject: [PATCH] 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.
---
 cli/lsp/diagnostics.rs         | 38 ++++++++++++++++++++++++++--------
 tests/integration/lsp_tests.rs | 28 ++++++++++++++++++++++++-
 2 files changed, 56 insertions(+), 10 deletions(-)

diff --git a/cli/lsp/diagnostics.rs b/cli/lsp/diagnostics.rs
index 6deafde4c0..fe69048302 100644
--- a/cli/lsp/diagnostics.rs
+++ b/cli/lsp/diagnostics.rs
@@ -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) => {
@@ -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 {
diff --git a/tests/integration/lsp_tests.rs b/tests/integration/lsp_tests.rs
index 0c4c1544f2..9300413b53 100644
--- a/tests/integration/lsp_tests.rs
+++ b/tests/integration/lsp_tests.rs
@@ -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!(