Repository: denoland/deno
Pull request: fix(lsp): don't show no-export diagnostics for type-only npm imports

Fixes #29548.

The previous behaviour is justifiable because the reproductive code will error at runtime. But this aligns better with tsc.

Changed files:
filename: cli/lsp/diagnostics.rs
patch: @@ -25,7 +25,6 @@ use deno_core::unsync::spawn_blocking;
 use deno_core::unsync::JoinHandle;
 use deno_core::url::Url;
 use deno_core::ModuleSpecifier;
-use deno_graph::source::ResolutionKind;
 use deno_graph::source::ResolveError;
 use deno_graph::Resolution;
 use deno_graph::ResolutionError;
@@ -1568,7 +1567,6 @@ fn diagnose_resolution(
   snapshot: &language_server::StateSnapshot,
   dependency_key: &str,
   resolution: &Resolution,
-  resolution_kind: ResolutionKind,
   is_dynamic: bool,
   maybe_assert_type: Option<&str>,
   referrer_module: &DocumentModule,
@@ -1631,16 +1629,21 @@ fn diagnose_resolution(
               req.clone(),
               specifier.clone(),
             ));
-          } else if scoped_resolver
-            .npm_to_file_url(
-              &pkg_ref,
-              &referrer_module.specifier,
-              NodeResolutionKind::from_deno_graph(resolution_kind),
-              referrer_module.resolution_mode,
-            )
-            .is_none()
-          {
-            diagnostics.push(DenoDiagnostic::NoExportNpm(pkg_ref.clone()));
+          } else {
+            let resolution_kinds =
+              [NodeResolutionKind::Types, NodeResolutionKind::Execution];
+            if resolution_kinds.into_iter().all(|k| {
+              scoped_resolver
+                .npm_to_file_url(
+                  &pkg_ref,
+                  &referrer_module.specifier,
+                  k,
+                  referrer_module.resolution_mode,
+                )
+                .is_none()
+            }) {
+              diagnostics.push(DenoDiagnostic::NoExportNpm(pkg_ref.clone()));
+            }
           }
         }
       } else if let Some(module_name) = specifier.as_str().strip_prefix("node:")
@@ -1763,27 +1766,22 @@ fn diagnose_dependency(
         .includes(i.specifier_range.range.start)
         .is_some()
     });
-  let resolution;
-  let resolution_kind;
-  if dependency.maybe_code.is_none()
+  let resolution = if dependency.maybe_code.is_none()
   // If not @ts-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 necessary, but don't bother erroring in the editor
   || !is_types_deno_types && matches!(dependency.maybe_type, Resolution::Ok(_))
     && matches!(dependency.maybe_code, Resolution::Err(_))
   {
-    resolution = &dependency.maybe_type;
-    resolution_kind = ResolutionKind::Types;
+    &dependency.maybe_type
   } else {
-    resolution = &dependency.maybe_code;
-    resolution_kind = ResolutionKind::Execution;
+    &dependency.maybe_code
   };
   let (resolution_diagnostics, deferred) = diagnose_resolution(
     snapshot,
     dependency_key,
     resolution,
-    resolution_kind,
     dependency.is_dynamic,
     dependency.maybe_attribute_type.as_deref(),
     referrer_module,
@@ -1819,7 +1817,6 @@ fn diagnose_dependency(
       snapshot,
       dependency_key,
       &dependency.maybe_type,
-      ResolutionKind::Types,
       dependency.is_dynamic,
       dependency.maybe_attribute_type.as_deref(),
       referrer_module,

filename: tests/integration/lsp_tests.rs
patch: @@ -6579,6 +6579,36 @@ fn lsp_npm_managed_no_export_diagnostic() {
   client.shutdown();
 }
 
+// Regression test for https://github.com/denoland/deno/issues/29548.
+#[test]
+#[timeout(300_000)]
+fn lsp_npm_managed_type_only_export_no_diagnostic() {
+  let context = TestContextBuilder::new()
+    .use_http_server()
+    .use_temp_cwd()
+    .build();
+  let mut client = context.new_lsp_command().build();
+  client.initialize_default();
+  client.did_open(json!({
+    "textDocument": {
+      "uri": "file:///a/file.ts",
+      "languageId": "typescript",
+      "version": 1,
+      "text": "import \"npm:@minecraft/common\";\n",
+    },
+  }));
+  client.write_request(
+    "workspace/executeCommand",
+    json!({
+      "command": "deno.cache",
+      "arguments": [[], "file:///a/file.ts"],
+    }),
+  );
+  let diagnostics = client.read_diagnostics();
+  assert_eq!(json!(diagnostics.all()), json!([]));
+  client.shutdown();
+}
+
 // Regression test for https://github.com/denoland/deno/issues/29177.
 #[test]
 #[timeout(300_000)]

filename: tests/registry/npm/@minecraft/common/registry.json
patch: @@ -0,0 +1,39 @@
+{
+  "name": "@minecraft/common",
+  "description": "See full documentation for this module here:",
+  "dist-tags": {
+    "latest": "1.2.0"
+  },
+  "versions": {
+    "1.2.0": {
+      "name": "@minecraft/common",
+      "version": "1.2.0",
+      "license": "MIT",
+      "_id": "@minecraft/common@1.2.0",
+      "dist": {
+        "shasum": "a00d4130dcd5084dfd30e85e108c6d58eae8358b",
+        "tarball": "http://localhost:4260/@minecraft/common/common-1.2.0.tgz",
+        "fileCount": 5,
+        "integrity": "sha512-JdmEq4P3Z/FtoBzhLijFgMSVFnFRrUoLwY8DHHrgtFo0mfLTOLTB1RErYjLMsA6b7BGVNxkX/pfFRiH7QZ0XwQ==",
+        "signatures": [
+          {
+            "sig": "MEYCIQCajw5bMsAwRHAZ5idQRHb+vxluFUKU5G/avR3YuwZ70QIhAK3AWB8eGTFL0wKLkHjtTc5A9qwrs1/5isK5MI2eDXp/",
+            "keyid": "SHA256:jl3bwswu80PjjokCgh0o2w5c2U4LhQAE57gj9cz1kzA"
+          }
+        ],
+        "unpackedSize": 3752
+      },
+      "types": "./index.d.ts",
+      "_npmVersion": "10.2.3",
+      "description": "See full documentation for this module here:",
+      "directories": {},
+      "_nodeVersion": "18.19.0",
+      "_hasShrinkwrap": false,
+      "_npmOperationalInternal": {
+        "tmp": "tmp/common_1.2.0_1710267246154_0.9556939208397457",
+        "host": "s3://npm-registry-packages"
+      }
+    }
+  },
+  "license": "MIT"
+}