custom eslint plugin that fixes typedef imports

.github/workflows/test-all-packages.yml

@@ -370,6 +370,9 @@ jobs:
       - name: yarn test (eslint-config)
         if: (success() || failure())
         run: cd packages/eslint-config && yarn ${{ steps.vars.outputs.test }} | $TEST_COLLECT
+      - name: yarn test (eslint-plugin)
+        if: (success() || failure())
+        run: cd packages/eslint-plugin && yarn ${{ steps.vars.outputs.test }} | $TEST_COLLECT
       - name: yarn test (vat-data)
         if: (success() || failure())
         run: cd packages/vat-data && yarn ${{ steps.vars.outputs.test }} | $TEST_COLLECT

eslint.config.mjs

@@ -89,6 +89,7 @@ export default [
   ...fixupConfigRules(
     compat.extends(
       '@agoric',
+      'plugin:@agoric/recommended',
       'plugin:ava/recommended',
       'plugin:require-extensions/recommended',
     ),

package.json

@@ -25,7 +25,7 @@
     "ava": "^5.3.0",
     "c8": "^10.1.3",
     "conventional-changelog-conventionalcommits": "^9.1.0",
-    "eslint": "^9.32.0",
+    "eslint": "^9.39.1",
     "eslint-config-airbnb-base": "^15.0.0",
     "eslint-config-jessie": "^0.0.6",
     "eslint-config-prettier": "^10.0.2",

packages/SwingSet/package.json

@@ -102,6 +102,6 @@
     "access": "public"
   },
   "typeCoverage": {
-    "atLeast": 76.59
+    "atLeast": 76.71
   }
 }

packages/SwingSet/src/types-external.js

@@ -1,3 +1,4 @@
+/* eslint-disable @agoric/no-typedef-import -- it's the only way to re-export with JSDoc */
 export {};
 
 /**

packages/SwingSet/src/types-internal.js

@@ -38,12 +38,12 @@ export {};
  * @typedef { string } VatID
  * @typedef { string } MeterID
  * @typedef { { meterID?: MeterID } } OptMeterID
- * @typedef { import('./types-external.js').BaseVatOptions } BaseVatOptions
- * @typedef { import('@agoric/swingset-liveslots').VatDeliveryObject } VatDeliveryObject
- * @typedef { import('@agoric/swingset-liveslots').VatDeliveryResult } VatDeliveryResult
- * @typedef { import('@agoric/swingset-liveslots').VatSyscallObject } VatSyscallObject
- * @typedef { import('@agoric/swingset-liveslots').VatSyscallResult } VatSyscallResult
- * @typedef { import('@agoric/swingset-liveslots').VatSyscallHandler } VatSyscallHandler
+ * @import {BaseVatOptions} from './types-external.js';
+ * @import {VatDeliveryObject} from '@agoric/swingset-liveslots';
+ * @import {VatDeliveryResult} from '@agoric/swingset-liveslots';
+ * @import {VatSyscallObject} from '@agoric/swingset-liveslots';
+ * @import {VatSyscallResult} from '@agoric/swingset-liveslots';
+ * @import {VatSyscallHandler} from '@agoric/swingset-liveslots';
  *
  * // used by vatKeeper.setSourceAndOptions(source, RecordedVatOptions)
  *

packages/agoric-cli/src/commands/inter.js

@@ -570,7 +570,7 @@ $ inter bid list --from my-acct
           ? state.offerStatuses.entries()
           : current.liveOffers;
         for (const [id, spec] of entries) {
-          const offerStatus = state.offerStatuses.get(id) || spec;
+          const offerStatus = state.offerStatuses.get(String(id)) || spec;
           harden(offerStatus); // coalesceWalletState should do this
           // console.debug(offerStatus.invitationSpec);
           if (!matches(offerStatus.invitationSpec, bidInvitationShape))

packages/agoric-cli/src/sdk-package-names.js

@@ -24,6 +24,7 @@ export default [
   "@agoric/deploy-script-support",
   "@agoric/ertp",
   "@agoric/eslint-config",
+  "@agoric/eslint-plugin",
   "@agoric/fast-usdc",
   "@agoric/governance",
   "@agoric/import-manager",

packages/client-utils/src/wallet-utils.js

@@ -2,7 +2,7 @@
 
 import { makeSmartWalletKit } from './smart-wallet-kit.js';
 
-/** @typedef {import('./smart-wallet-kit.js').SmartWalletKit} WalletUtils  */
+/** @import {SmartWalletKit} from './smart-wallet-kit.js';  */
 
 /** @deprecated use `makeSmartWalletKit` */
 export const makeWalletUtils = makeSmartWalletKit;

packages/cosmic-swingset/package.json

@@ -82,6 +82,6 @@
     "timeout": "20m"
   },
   "typeCoverage": {
-    "atLeast": 86.54
+    "atLeast": 86.47
   }
 }

packages/eslint-config/package.json

@@ -25,6 +25,7 @@
     "eslint-config.*"
   ],
   "peerDependencies": {
+    "@agoric/eslint-plugin": "^0.1.0",
     "@endo/eslint-plugin": "^2.4.0",
     "@jessie.js/eslint-plugin": "^0.4.2",
     "eslint": "^9.0.0",

packages/eslint-plugin/eslint.config.js

@@ -0,0 +1,10 @@
+import eslintPluginEslintPlugin from 'eslint-plugin-eslint-plugin';
+
+export default [
+  {
+    plugins: {
+      'eslint-plugin': eslintPluginEslintPlugin,
+    },
+    extends: ['plugin:eslint-plugin/recommended'],
+  },
+];

packages/eslint-plugin/package.json

@@ -0,0 +1,33 @@
+{
+  "name": "@agoric/eslint-plugin",
+  "version": "0.1.0",
+  "description": "ESLint plugin for Agoric best practices",
+  "main": "src/index.js",
+  "type": "commonjs",
+  "files": [
+    "src"
+  ],
+  "keywords": [
+    "eslint",
+    "eslintplugin",
+    "eslint-plugin"
+  ],
+  "scripts": {
+    "build": "exit 0",
+    "lint": "yarn run -T run-s --continue-on-error 'lint:*'",
+    "lint:types": "yarn run -T tsc",
+    "test": "node --test **/*.test.*",
+    "test:xs": "exit 0"
+  },
+  "peerDependencies": {
+    "eslint": "^9.0.0"
+  },
+  "devDependencies": {
+    "eslint": "^9.0.0",
+    "eslint-plugin-eslint-plugin": "^7.2.0"
+  },
+  "license": "Apache-2.0",
+  "engines": {
+    "node": "^20.9 || ^22.11"
+  }
+}

packages/eslint-plugin/src/index.js

@@ -0,0 +1,31 @@
+const fs = require('node:fs');
+const path = require('path');
+
+const pkg = JSON.parse(
+  fs.readFileSync(path.join(__dirname, '../package.json'), 'utf8'),
+);
+
+// Import rules
+const noTypedefImport = require('./rules/no-typedef-import.js');
+
+module.exports = {
+  meta: {
+    name: pkg.name,
+    version: pkg.version,
+  },
+
+  // Rule definitions
+  rules: {
+    'no-typedef-import': noTypedefImport,
+  },
+
+  // Recommended config
+  configs: {
+    recommended: {
+      plugins: ['@agoric'],
+      rules: {
+        '@agoric/no-typedef-import': 'error',
+      },
+    },
+  },
+};

packages/eslint-plugin/src/rules/no-typedef-import.js

@@ -0,0 +1,147 @@
+/**
+ * ESLint rule: no-typedef-import
+ *
+ * Finds inline `import()` expressions inside `@typedef` declarations and replaces
+ * them with a corresponding `@import` directive. This prevents generating new type aliases
+ * and unwittingly exporting them from the module.
+ *
+ * If the module should be exporting the types, `@typedef` is still the only way in JSDoc
+ * so disable this rule in those files.
+ *
+ * @see https://github.com/microsoft/typescript/issues/60831
+ */
+
+const LINE_REGEX =
+  /^(\s*\*?\s*)@typedef\s+\{\s*import\((['"])([^'"]+)\2\)\.([A-Za-z0-9_$]+)([^}]*)\}\s+([A-Za-z0-9_$]+)(.*)$/;
+
+const getConversionPlan = value => {
+  const lines = value.split('\n');
+  const convertible = [];
+
+  for (let index = 0; index < lines.length; index += 1) {
+    const line = lines[index];
+    const match = line.match(LINE_REGEX);
+    if (!match) {
+      continue;
+    }
+
+    const [
+      ,
+      indent,
+      quote,
+      importPath,
+      importedType,
+      rawSuffix = '',
+      ,
+      // typedefName (unused),
+      trailing = '',
+    ] = match;
+
+    if (rawSuffix && rawSuffix.trim() !== '') {
+      continue; // skip typedefs that add generics or other suffixes
+    }
+
+    convertible.push({
+      index,
+      indent,
+      quote,
+      importPath,
+      importedType,
+      trailing,
+    });
+  }
+
+  return { lines, convertible };
+};
+
+const transformCommentValue = value => {
+  const { lines, convertible } = getConversionPlan(value);
+  if (!convertible.length) {
+    return null;
+  }
+
+  for (const {
+    index,
+    indent,
+    quote,
+    importPath,
+    importedType,
+    trailing,
+  } of convertible) {
+    const trailingText = trailing || '';
+    lines[index] =
+      `${indent}@import {${importedType}} from ${quote}${importPath}${quote};${trailingText}`;
+  }
+
+  return lines.join('\n');
+};
+
+/** @type {import('eslint').Rule.RuleModule} */
+module.exports = {
+  meta: {
+    type: 'problem',
+    docs: {
+      description:
+        'Disallow inline import() expressions inside @typedef; prefer @import directives.',
+      recommended: false,
+    },
+    fixable: 'code',
+    schema: [],
+    messages: {
+      noTypedefImport:
+        'Use an @import directive instead of an inline import() inside @typedef.',
+    },
+  },
+
+  create(context) {
+    const sourceCode = context.getSourceCode();
+
+    return {
+      Program() {
+        for (const comment of sourceCode.getAllComments()) {
+          if (comment.type !== 'Block') {
+            continue;
+          }
+          if (!comment.value.includes('@typedef')) {
+            continue;
+          }
+
+          const { convertible } = getConversionPlan(comment.value);
+          if (!convertible.length) {
+            continue;
+          }
+
+          const { range, loc } = comment;
+          if (!range) {
+            continue;
+          }
+
+          const originalText = sourceCode.text.slice(range[0], range[1]);
+          const reportLoc = loc ?? {
+            start: sourceCode.getLocFromIndex(range[0]),
+            end: sourceCode.getLocFromIndex(range[1]),
+          };
+
+          const descriptor = {
+            loc: reportLoc,
+            messageId: 'noTypedefImport',
+            fix(fixer) {
+              const newValue = transformCommentValue(comment.value);
+              if (!newValue) {
+                return null;
+              }
+
+              const newCommentText = originalText.replace(
+                comment.value,
+                newValue,
+              );
+              return fixer.replaceTextRange(range, newCommentText);
+            },
+          };
+
+          context.report(descriptor);
+        }
+      },
+    };
+  },
+};

packages/eslint-plugin/tests/rules/no-typedef-import.test.js

@@ -0,0 +1,52 @@
+const { RuleTester } = require('eslint');
+const rule = require('../../src/rules/no-typedef-import.js');
+
+const ruleTester = new RuleTester({
+  languageOptions: { ecmaVersion: 2022, sourceType: 'module' },
+});
+
+ruleTester.run('no-typedef-import', rule, {
+  valid: [
+    {
+      code: `/**
+ * @import {BundleCap} from './types.js';
+ */`,
+    },
+    {
+      code: `/** Just a doc block without typedef imports */`,
+    },
+    {
+      // Not just an import; it's qualifying the type.
+      code: `/**
+ * @typedef { import('@endo/marshal').CapData<string> } SwingSetCapData
+ */`,
+    },
+  ],
+  invalid: [
+    {
+      code: `/** @typedef { import('./types.js').BundleCap } BundleCap */`,
+      output: `/** @import {BundleCap} from './types.js'; */`,
+      errors: [{ messageId: 'noTypedefImport' }],
+    },
+    {
+      code: `/**
+ * @typedef { import('./kvStore.js').KVStore } KVStore
+ * @typedef { import('./kvStore.js').SnapshotResult } SnapshotResult
+ */`,
+      output: `/**
+ * @import {KVStore} from './kvStore.js';
+ * @import {SnapshotResult} from './kvStore.js';
+ */`,
+      errors: [{ messageId: 'noTypedefImport' }],
+    },
+    {
+      code: `/**
+ * @typedef { import('./smart-wallet-kit.js').SmartWalletKit } WalletUtils
+ */`,
+      output: `/**
+ * @import {SmartWalletKit} from './smart-wallet-kit.js';
+ */`,
+      errors: [{ messageId: 'noTypedefImport' }],
+    },
+  ],
+});

packages/eslint-plugin/tsconfig.json

@@ -0,0 +1,8 @@
+{
+  "extends": "../../tsconfig.json",
+  "compilerOptions": {},
+  "include": [
+    "src",
+    "test",
+  ],
+}

packages/governance/package.json

@@ -76,6 +76,6 @@
     "access": "public"
   },
   "typeCoverage": {
-    "atLeast": 89.47
+    "atLeast": 89.48
   }
 }