fix(jsii): incorrect symbol-id generated with typesVersions (#4037)

When using types sourced from redirected declarations files (using `typesVersions`), the `symbolId` generated for type lookups incorrectly refers to the redirected path instead of the canonical one. This adds code to reverse the `typesVersions` mapping to get back to the original and canonical path.



---

By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license].

[Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0

packages/@jsii/python-runtime/tests/test_invoke_bin.py

@@ -1,15 +1,40 @@
+from os import environ
 import platform
 import subprocess
 from datetime import datetime
 import pytest
 
 
+@pytest.fixture()
+def silence_node_deprecation_warnings():
+    """Ensures @jsii/check-node does not emit any warning that would cause the
+    test output assertions to be invalidated.
+    """
+
+    variables = [
+        "JSII_SILENCE_WARNING_KNOWN_BROKEN_NODE_VERSION",
+        "JSII_SILENCE_WARNING_UNTESTED_NODE_VERSION",
+        "JSII_SILENCE_WARNING_DEPRECATED_NODE_VERSION",
+    ]
+
+    store = {var: environ.get(var, "") for var in variables}
+
+    for var in variables:
+        environ[var] = "1"
+
+    # Execute the test
+    yield
+
+    for var in variables:
+        environ[var] = store[var]
+
+
 class TestInvokeBinScript:
     @pytest.mark.skipif(
         platform.system() == "Windows",
         reason="jsii-pacmak does not generate windows scripts",
     )
-    def test_invoke_script(self) -> None:
+    def test_invoke_script(self, silence_node_deprecation_warnings) -> None:
         script_path = f".env/bin/calc"
         result = subprocess.run([script_path], capture_output=True)
 
@@ -21,7 +46,7 @@ def test_invoke_script(self) -> None:
         platform.system() == "Windows",
         reason="jsii-pacmak does not generate windows scripts",
     )
-    def test_invoke_script_with_args(self) -> None:
+    def test_invoke_script_with_args(self, silence_node_deprecation_warnings) -> None:
         script_path = f".env/bin/calc"
         result = subprocess.run([script_path, "arg1", "arg2"], capture_output=True)
 
@@ -33,7 +58,9 @@ def test_invoke_script_with_args(self) -> None:
         platform.system() == "Windows",
         reason="jsii-pacmak does not generate windows scripts",
     )
-    def test_invoke_script_with_failure(self) -> None:
+    def test_invoke_script_with_failure(
+        self, silence_node_deprecation_warnings
+    ) -> None:
         script_path = f".env/bin/calc"
         result = subprocess.run([script_path, "arg1", "fail"], capture_output=True)
 
@@ -45,7 +72,9 @@ def test_invoke_script_with_failure(self) -> None:
         platform.system() == "Windows",
         reason="jsii-pacmak does not generate windows scripts",
     )
-    def test_invoke_script_with_line_flush(self) -> None:
+    def test_invoke_script_with_line_flush(
+        self, silence_node_deprecation_warnings
+    ) -> None:
         """Make sure lines are flushed immediately as they are generated, rather than
         buffered to the end
         """

packages/jsii/lib/symbol-id.ts

@@ -3,6 +3,7 @@ import * as fs from 'fs-extra';
 import * as path from 'path';
 import * as ts from 'typescript';
 
+import { undoTypesVersionsRedirect } from './types-versions';
 import { findUp } from './utils';
 
 /**
@@ -125,6 +126,11 @@ class Helper {
       sourcePath = normalizePath(sourcePath, tscRootDir, tscOutDir);
     }
 
+    sourcePath = undoTypesVersionsRedirect(
+      sourcePath,
+      packageInfo.typesVersions,
+    );
+
     return sourcePath.replace(/(\.d)?\.ts$/, '');
 
     function removePrefix(prefix: string, filePath: string) {
@@ -156,13 +162,16 @@ class Helper {
       return this.packageInfo.get(packageJsonDir);
     }
 
-    const { jsii } = fs.readJsonSync(path.join(packageJsonDir, 'package.json'));
+    const { jsii, typesVersions } = fs.readJsonSync(
+      path.join(packageJsonDir, 'package.json'),
+    );
 
     const result = {
       packageJsonDir,
       outdir: jsii?.outdir,
       tscRootDir: jsii?.tsc?.rootDir,
       tscOutDir: jsii?.tsc?.outDir,
+      typesVersions,
     };
     this.packageInfo.set(from, result);
     this.packageInfo.set(packageJsonDir, result);
@@ -175,6 +184,13 @@ interface PackageInfo {
   readonly outdir: string | undefined;
   readonly tscRootDir: string | undefined;
   readonly tscOutDir: string | undefined;
+  readonly typesVersions:
+    | {
+        readonly [range: string]: {
+          readonly [glob: string]: readonly string[];
+        };
+      }
+    | undefined;
 }
 
 /**

packages/jsii/lib/types-versions.ts

@@ -0,0 +1,68 @@
+import * as semver from 'semver';
+import * as ts from 'typescript';
+
+export interface TypesVersions {
+  readonly [range: string]: {
+    readonly [glob: string]: readonly string[];
+  };
+}
+
+export function undoTypesVersionsRedirect(
+  sourcePath: string,
+  typesVersionsMap: TypesVersions | undefined,
+): string {
+  if (typesVersionsMap == null) {
+    return sourcePath;
+  }
+
+  const [_, typesVersions] =
+    Object.entries(typesVersionsMap).find(([range, _]) =>
+      semver.satisfies(ts.version, range),
+    ) ?? [];
+
+  if (typesVersions == null) {
+    return sourcePath;
+  }
+
+  for (const [orig, candidates] of Object.entries(typesVersions)) {
+    const [found] = candidates.flatMap((candidate) => {
+      const [before, after, ...rest] = candidate.split('*');
+      // There can be only 1 "*" in a typesVersions pattern.
+      if (rest.length > 0) {
+        return [];
+      }
+      if (sourcePath.startsWith(before) && sourcePath.endsWith(after)) {
+        return [
+          {
+            before,
+            after,
+            length: before.length + after.length,
+          },
+        ];
+      }
+
+      return [];
+    });
+
+    if (found == null) {
+      continue;
+    }
+
+    const [before, after, ...rest] = orig.split('*');
+    // There can be only 1 "*" in a typesVersions pattern.
+    if (rest.length > 0) {
+      continue;
+    }
+
+    // Remove the typesVersion prefix & suffix
+    const globbed = sourcePath.slice(
+      found.before.length,
+      sourcePath.length - found.after.length,
+    );
+    // Add the original prefix & suffix
+    return `${before}${globbed}${after}`;
+  }
+
+  // No match found...
+  return sourcePath;
+}

packages/jsii/test/types-versions.test.ts

@@ -0,0 +1,68 @@
+import { undoTypesVersionsRedirect } from '../lib/types-versions';
+
+test('no configuration', () => {
+  expect(undoTypesVersionsRedirect('test/path/index.ts', undefined)).toBe(
+    'test/path/index.ts',
+  );
+});
+
+test('no candidate matches', () => {
+  expect(
+    undoTypesVersionsRedirect('test/path/index.ts', {
+      '*': { '*': ['.types-versions/ts3.9/*'] },
+    }),
+  ).toBe('test/path/index.ts');
+});
+
+test('incompatible-typescript-version', () => {
+  expect(
+    undoTypesVersionsRedirect('test/path/index.ts', {
+      '>99.99.99': {
+        '*': ['test/*'],
+      },
+    }),
+  ).toBe('test/path/index.ts');
+});
+
+test('simple match', () => {
+  expect(
+    undoTypesVersionsRedirect('test/path/index.ts', {
+      '*': {
+        '*': ['test/*/index.ts'],
+      },
+    }),
+  ).toBe('path');
+});
+
+test('multiple typeScript versions (first one wins)', () => {
+  expect(
+    undoTypesVersionsRedirect('test/path/index.ts', {
+      '>=0': {
+        '*': ['test/*/index.ts'],
+      },
+      '*': {
+        'before/*/after.ts': ['test/*/index.ts'],
+      },
+    }),
+  ).toBe('path');
+});
+
+test('prefix+suffix match', () => {
+  expect(
+    undoTypesVersionsRedirect('test/path/index.ts', {
+      '*': {
+        'before/*/after.ts': ['test/*/index.ts'],
+      },
+    }),
+  ).toBe('before/path/after.ts');
+});
+
+test('multiple candidates (first match wins)', () => {
+  expect(
+    undoTypesVersionsRedirect('test/path/index.ts', {
+      '*': {
+        'before/*/after.ts': ['*/path/index.ts', 'test/*/index.ts'],
+      },
+    }),
+  ).toBe('before/test/after.ts');
+});