Merge pull request swiftwasm#660 from swiftwasm/katei/b8fc-ts2swift-should

TS2Swift: warn on skipped exports

Author: krodak

Plugins/BridgeJS/Sources/TS2Swift/JavaScript/src/cli.js

@@ -97,13 +97,13 @@ Examples:
 /**
  * Run ts2swift for a single input file (programmatic API, no process I/O).
  * @param {string[]} filePaths - Paths to the .d.ts files
- * @param {{ tsconfigPath: string, logLevel?: keyof typeof DiagnosticEngine.LEVELS, globalFiles?: string[] }} options
+ * @param {{ tsconfigPath: string, logLevel?: keyof typeof DiagnosticEngine.LEVELS, globalFiles?: string[], diagnosticEngine?: DiagnosticEngine }} options
  * @returns {string} Generated Swift source
  * @throws {Error} on parse/type-check errors (diagnostics are included in the message)
  */
 export function run(filePaths, options) {
-    const { tsconfigPath, logLevel = 'info', globalFiles = [] } = options;
-    const diagnosticEngine = new DiagnosticEngine(logLevel);
+    const { tsconfigPath, logLevel = 'info', globalFiles = [], diagnosticEngine } = options;
+    const engine = diagnosticEngine ?? new DiagnosticEngine(logLevel);
 
     const configFile = ts.readConfigFile(tsconfigPath, ts.sys.readFile);
     const configParseResult = ts.parseJsonConfigFileContent(
@@ -164,7 +164,7 @@ export function run(filePaths, options) {
     const bodies = [];
     const globalFileSet = new Set(globalFiles);
     for (const inputPath of [...filePaths, ...globalFiles]) {
-        const processor = new TypeProcessor(program.getTypeChecker(), diagnosticEngine, {
+        const processor = new TypeProcessor(program.getTypeChecker(), engine, {
             defaultImportFromGlobal: globalFileSet.has(inputPath),
         });
         const result = processor.processTypeDeclarations(program, inputPath);
@@ -247,7 +247,7 @@ export function main(args) {
 
     let swiftOutput;
     try {
-        swiftOutput = run(filePaths, { tsconfigPath, logLevel, globalFiles });
+        swiftOutput = run(filePaths, { tsconfigPath, logLevel, globalFiles, diagnosticEngine });
     } catch (/** @type {unknown} */ err) {
         if (err instanceof Error) {
             diagnosticEngine.print("error", err.message);

Plugins/BridgeJS/Sources/TS2Swift/JavaScript/src/processor.js

@@ -57,6 +57,9 @@ export class TypeProcessor {
         /** @type {Set<string>} */
         this.emittedStringLiteralUnionNames = new Set();
 
+        /** @type {Set<ts.Node>} */
+        this.warnedExportNodes = new Set();
+
         /** @type {Set<string>} */
         this.visitedDeclarationKeys = new Set();
 
@@ -192,6 +195,8 @@ export class TypeProcessor {
             this.visitEnumDeclaration(node);
         } else if (ts.isExportDeclaration(node)) {
             this.visitExportDeclaration(node);
+        } else if (ts.isExportAssignment(node)) {
+            this.visitExportAssignment(node);
         }
     }
 
@@ -239,6 +244,7 @@ export class TypeProcessor {
             }
         } else {
             // export * as ns from "..." is not currently supported by BridgeJS imports.
+            this.warnExportSkip(node, "Skipping namespace re-export (export * as ns) which is not supported");
             return;
         }
 
@@ -254,6 +260,19 @@ export class TypeProcessor {
                 this.visitNode(declaration);
             }
         }
+
+        if (targetSymbols.length === 0) {
+            this.warnExportSkip(node, "Export declaration resolved to no symbols; nothing was generated");
+        }
+    }
+
+    /**
+     * Handle `export default foo;` style assignments.
+     * @param {ts.ExportAssignment} node
+     */
+    visitExportAssignment(node) {
+        // BridgeJS does not currently model default export assignments (they may point to expressions).
+        this.warnExportSkip(node, "Skipping export assignment (export default ...) which is not supported");
     }
 
     /**
@@ -271,7 +290,10 @@ export class TypeProcessor {
         const isConst = (node.declarationList.flags & ts.NodeFlags.Const) !== 0;
 
         for (const decl of node.declarationList.declarations) {
-            if (!ts.isIdentifier(decl.name)) continue;
+            if (!ts.isIdentifier(decl.name)) {
+                this.warnExportSkip(decl, "Skipping exported variable with a non-identifier name");
+                continue;
+            }
 
             const jsName = decl.name.text;
             const swiftName = this.swiftTypeName(jsName);
@@ -399,7 +421,12 @@ export class TypeProcessor {
      */
     visitEnumDeclaration(node) {
         const name = node.name?.text;
-        if (!name) return;
+        if (!name) {
+            if (this.isExported(node)) {
+                this.warnExportSkip(node, "Skipping exported enum without a name");
+            }
+            return;
+        }
         this.emitEnumFromDeclaration(name, node, node);
     }
 
@@ -532,7 +559,12 @@ export class TypeProcessor {
      * @private
      */
     visitFunctionDeclaration(node) {
-        if (!node.name) return;
+        if (!node.name) {
+            if (this.isExported(node)) {
+                this.warnExportSkip(node, "Skipping exported function without a name");
+            }
+            return;
+        }
         const jsName = node.name.text;
         const swiftName = this.swiftTypeName(jsName);
         const fromArg = this.renderDefaultJSImportFromArgument();
@@ -774,7 +806,12 @@ export class TypeProcessor {
      * @private
      */
     visitClassDecl(node) {
-        if (!node.name) return;
+        if (!node.name) {
+            if (this.isExported(node)) {
+                this.warnExportSkip(node, "Skipping exported class without a name");
+            }
+            return;
+        }
 
         const jsName = node.name.text;
         if (this.emittedStructuredTypeNames.has(jsName)) return;
@@ -1244,6 +1281,28 @@ export class TypeProcessor {
         return parts.join(" ");
     }
 
+    /**
+     * @param {ts.Node} node
+     * @returns {boolean}
+     */
+    isExported(node) {
+        const hasExportModifier = /** @type {ts.ModifierLike[] | undefined} */ (node.modifiers)?.some(
+            (m) => m.kind === ts.SyntaxKind.ExportKeyword
+        ) ?? false;
+        return hasExportModifier || ts.isExportAssignment(node);
+    }
+
+    /**
+     * Emit a single warning per node when an exported declaration cannot be generated.
+     * @param {ts.Node} node
+     * @param {string} reason
+     */
+    warnExportSkip(node, reason) {
+        if (this.warnedExportNodes.has(node)) return;
+        this.warnedExportNodes.add(node);
+        this.diagnosticEngine.print("warning", `${reason}. Swift binding not generated`, node);
+    }
+
     /**
      * Render identifier with backticks if needed
      * @param {string} name

Plugins/BridgeJS/Sources/TS2Swift/JavaScript/test/__snapshots__/ts2swift.test.js.snap

@@ -121,6 +121,19 @@ exports[`ts2swift > snapshots Swift output for Documentation.d.ts > Documentatio
 "
 `;
 
+exports[`ts2swift > snapshots Swift output for ExportAssignment.d.ts > ExportAssignment 1`] = `
+"// NOTICE: This is auto-generated code by BridgeJS from JavaScriptKit,
+// DO NOT EDIT.
+//
+// To update this file, just rebuild your project or run
+// \`swift package bridge-js\`.
+
+@_spi(BridgeJS) import JavaScriptKit
+
+@JSGetter var foo: Double
+"
+`;
+
 exports[`ts2swift > snapshots Swift output for Interface.d.ts > Interface 1`] = `
 "// NOTICE: This is auto-generated code by BridgeJS from JavaScriptKit,
 // DO NOT EDIT.

Plugins/BridgeJS/Sources/TS2Swift/JavaScript/test/fixtures/ExportAssignment.d.ts

@@ -0,0 +1,2 @@
+export const foo: number;
+export default foo;

Plugins/BridgeJS/Sources/TS2Swift/JavaScript/test/ts2swift.test.js

@@ -50,4 +50,18 @@ describe('ts2swift', () => {
             rmSync(tmpDir, { recursive: true, force: true });
         }
     });
+
+    it('emits a warning when export assignments cannot be generated', () => {
+        const dtsPath = path.join(inputsDir, 'ExportAssignment.d.ts');
+        /** @type {{ level: string, message: string }[]} */
+        const diagnostics = [];
+        const diagnosticEngine = {
+            print: (level, message) => diagnostics.push({ level, message }),
+        };
+        run([dtsPath], { tsconfigPath, logLevel: 'warning', diagnosticEngine });
+        const messages = diagnostics.map((d) => d.message).join('\n');
+        expect(messages).toMatch(/Skipping export assignment/);
+        const occurrences = (messages.match(/Skipping export assignment/g) || []).length;
+        expect(occurrences).toBe(1);
+    });
 });