Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

eslint-plugin for Agoric SDK #10665

Draft
wants to merge 27 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
3337a71
feat: scaffold @agoric/eslint-plugin
turadg Dec 10, 2024
580780a
feat: Add eslint rule to enforce start function synchronous prelude
turadg Dec 10, 2024
5d8e1d5
fixup prelude rule
turadg Dec 10, 2024
83ad2ab
fix: Resolve TypeScript errors in ESLint plugin with AST traversal im…
turadg Dec 10, 2024
0db395e
fix RuleTester import
turadg Dec 10, 2024
623ecb8
refactor: Fix stack overflow in start-function-prelude AST traversal
turadg Dec 10, 2024
caab043
fix: Update error reporting in start-function-prelude ESLint rule
turadg Dec 10, 2024
4f16078
fix: Improve await expression detection in start-function-prelude rule
turadg Dec 10, 2024
a474c2d
fix: Improve await expression detection in ESLint rule using AST trav…
turadg Dec 10, 2024
0c0ee61
refactor: Improve await expression detection in start function prelud…
turadg Dec 10, 2024
176d74d
fix: Refactor await expression detection in ESLint rule
turadg Dec 10, 2024
531442b
fix: Refactor AST traversal to prevent stack overflow in ESLint rule
turadg Dec 10, 2024
f2667f2
feat: rule to group JSDoc imports
turadg Dec 11, 2024
6e93fe3
chore: remove dollar-sign scaffold
turadg Dec 29, 2024
2cfbbb6
lint: adopt @agoric/eslint-plugin/recommended
turadg Dec 29, 2024
be8fa4d
refactor: Convert ESM modules to CJS in eslint-plugin rules
turadg Dec 29, 2024
0376add
refactor: Remove unused fileURLToPath import
turadg Dec 29, 2024
201fbce
refactor: Convert index.js from ESM to CommonJS module
turadg Dec 29, 2024
b504981
fixup
turadg Dec 29, 2024
ea588b5
fixup! feat: rule to group JSDoc imports
turadg Dec 29, 2024
7a593f2
fix: Correct ESLint plugin configuration in index.js
turadg Dec 29, 2024
50d16bd
fixup! feat: rule to group JSDoc imports
turadg Dec 29, 2024
a4cd9cd
fixup CJS conversion
turadg Dec 29, 2024
16a518f
hack for /**
turadg Dec 29, 2024
aab354b
lint: fix group-jsdoc-imports
turadg Dec 29, 2024
4ba1c6c
lint: fix group-jsdoc-imports
turadg Dec 29, 2024
f041d17
lint: fix group-jsdoc-imports
turadg Dec 29, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .eslintrc.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ module.exports = {
plugins: ['@typescript-eslint', 'prettier', 'require-extensions'],
extends: [
'@agoric',
'plugin:@agoric/recommended',
'plugin:ava/recommended',
'plugin:require-extensions/recommended',
],
Expand Down
1 change: 1 addition & 0 deletions packages/agoric-cli/src/sdk-package-names.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export default [
"@agoric/deployment",
"@agoric/ertp",
"@agoric/eslint-config",
"@agoric/eslint-plugin",
"@agoric/fast-usdc",
"@agoric/governance",
"@agoric/import-manager",
Expand Down
1 change: 1 addition & 0 deletions packages/eslint-config/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
"eslint-config.*"
],
"peerDependencies": {
"@agoric/eslint-plugin": "^0.1.0",
"@endo/eslint-plugin": "^2.2.3",
"@jessie.js/eslint-plugin": "^0.4.1",
"typescript-eslint": "^8.17.0",
Expand Down
15 changes: 15 additions & 0 deletions packages/eslint-plugin/eslint.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import eslintPluginEslintPlugin from 'eslint-plugin-eslint-plugin';
import eslintPluginN from 'eslint-plugin-n';

export default [
{
plugins: {
'eslint-plugin': eslintPluginEslintPlugin,
n: eslintPluginN,
},
extends: [
'plugin:eslint-plugin/recommended',
'plugin:n/recommended',
],
},
];
29 changes: 29 additions & 0 deletions packages/eslint-plugin/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
{
"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",
"test": "node --test tests/"
},
"peerDependencies": {
"eslint": "^8.57.1"
},
"devDependencies": {
"@types/eslint": "^8.56.5",
"eslint": "^8.57.1",
"eslint-plugin-eslint-plugin": "^6.3.2",
"eslint-plugin-n": "^17.15.0"
},
"license": "Apache-2.0"
}
34 changes: 34 additions & 0 deletions packages/eslint-plugin/src/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
const fs = require('node:fs');
const path = require('path');

const pkg = JSON.parse(
fs.readFileSync(path.join(__dirname, '../package.json'), 'utf8'),
);

// Import rules
const startFunctionPrelude = require('./rules/start-function-prelude.js');
const groupJsdocImports = require('./rules/group-jsdoc-imports.js');

module.exports = {
meta: {
name: pkg.name,
version: pkg.version,
},

// Rule definitions
rules: {
'start-function-prelude': startFunctionPrelude,
'group-jsdoc-imports': groupJsdocImports,
},

// Recommended config
configs: {
recommended: {
plugins: ['@agoric'],
rules: {
'@agoric/start-function-prelude': 'error',
'@agoric/group-jsdoc-imports': 'warn',
},
},
},
};
188 changes: 188 additions & 0 deletions packages/eslint-plugin/src/rules/group-jsdoc-imports.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,188 @@
/**
* ESLint rule: group-jsdoc-imports
*
* Moves inline JSDoc type imports (e.g. \@type {import('foo').Bar})
* into a top-level JSDoc block with \@import lines, then references the
* import by name (e.g. \@type {Bar}).
*
* Usage example in your .eslintrc.js:
*
* {
* "plugins": ["@agoric"],
* "rules": {
* "@agoric/group-jsdoc-imports": "warning"
* }
* }
*
* @type {import('eslint').Rule.RuleModule}
*/
module.exports = {
meta: {
type: 'suggestion',
docs: {
description: 'Move inline type import to top-level JSDoc import block',
category: 'Stylistic Issues',
recommended: false,
},
fixable: 'code',
schema: [
{
type: 'object',
properties: {
paths: {
type: 'array',
items: { type: 'string' },
},
},
additionalProperties: false,
},
],
messages: {
moveInline: 'Move inline type import to top-level JSDoc import block',
},
},

create(context) {
const sourceCode = context.getSourceCode();
const { paths: allowedPaths = [] } = context.options[0] || {};

// Use the `g` flag so we can match multiple inline imports in one comment.
const INLINE_IMPORT_REGEX =
/import\(['"]([^'"]+)['"]\)\.([A-Za-z0-9_$]+(?:<[A-Za-z0-9_<>{},\s]+>)?)/g;

function isAllowedPath(importPath) {
if (!allowedPaths.length) {
return true; // no filtering if no paths configured
}
return allowedPaths.some(prefix => importPath.startsWith(prefix));
}

/**
* Finds an existing top-level @import block comment (one that contains "@import")
* or returns `null` if none exists.
*/
function findTopBlockComment(allComments) {
for (const comment of allComments) {
if (comment.type === 'Block') {
// Rebuild with the /* ... */ so we can search for "@import"
const text = `/*${comment.value}*/`;
if (text.includes('@import ')) {
return comment;
}
}
}
return null;
}

/**
* Checks if the doc block already imports a specific type from a path,
* returning `true` if found.
*/
function alreadyHasImport(blockValue, typeName, importPath) {
// blockValue is the text inside /* ... */
// Look for lines like: @import {Type} from 'xyz'
// Possibly could handle multiple types in one line, but let's keep it simple:
const lines = blockValue.split('\n');
for (const line of lines) {
const m = line.match(/@import\s+{([^}]+)}\s+from\s+['"]([^'"]+)['"]/);
if (m) {
const importedTypes = m[1].split(',').map(s => s.trim());
const fromPath = m[2];
if (fromPath === importPath && importedTypes.includes(typeName)) {
return true; // Found a matching import already
}
}
}
return false;
}

const allComments = sourceCode.getAllComments();

// Walk all block comments and look for inline imports.
for (const comment of allComments) {
if (comment.type !== 'Block') {
continue;
}
const { value: commentText } = comment;

// We collect all matches in this comment for `import('...').SomeType`
const matches = [...commentText.matchAll(INLINE_IMPORT_REGEX)];
for (const match of matches) {
const [fullMatch, importPath, typeName] = match;
if (!isAllowedPath(importPath)) {
continue; // skip if path is not in allowedPaths
}

// We have an inline import. We'll report exactly one error
// for *this* inline import. The fix will:
// 1) Insert/update the doc block
// 2) Remove inline usage from this comment
context.report({
loc: comment.loc,
messageId: 'moveInline',
fix: fixer => {
// We'll build a set of fix operations for both the doc block
// (somewhere in the file) and this inline usage.
const fixOps = [];

// (a) We locate or create a top-level block
const topBlockComment = findTopBlockComment(allComments);

if (!topBlockComment) {
// If no block with @import found, create a new block at top
const lines = [
'/**',
` * @import {${typeName}} from '${importPath}';`,
' */\n',
];
// Insert at the very start of the file:
fixOps.push(
fixer.insertTextBeforeRange([0, 0], lines.join('\n')),
);
} else {
// If we do have a block, ensure it includes `@import {typeName} from 'importPath'`
const originalLines = topBlockComment.value.split('\n');

// Only add if it's not already imported
if (
!alreadyHasImport(topBlockComment.value, typeName, importPath)
) {
// Insert a new line with that import just before final '*/'
const newLines = [...originalLines];
const lastLineIndex = newLines.length - 1;
newLines.splice(
lastLineIndex,
0,
`* @import {${typeName}} from '${importPath}';`,
);
const newCommentValue = newLines.join('\n');
fixOps.push(
fixer.replaceTextRange(
[topBlockComment.range[0], topBlockComment.range[1]],
`/*${newCommentValue}*/`,
),
);
}
}

// (b) Remove the inline usage from *this* comment
// Replace `import('...').Foo` → `Foo`
const updatedCommentText = commentText.replace(fullMatch, typeName);
fixOps.push(
fixer.replaceTextRange(
[comment.range[0], comment.range[1]],
`/*${updatedCommentText}*/`,
),
);

return fixOps;
},
});
}
}

// We do NOT do anything in Program:exit, because each inline usage
// is already handled in a single fix above.
return {};
},
};
92 changes: 92 additions & 0 deletions packages/eslint-plugin/src/rules/start-function-prelude.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
/** @type {import('eslint').Rule.RuleModule} */
module.exports = {
meta: {
type: 'problem',
docs: {
description:
'Enforce start function to have proper synchronous prelude region',
recommended: true,
},
schema: [], // no options
},
create(context) {
return {
ExportNamedDeclaration(node) {
if (
!node.declaration ||
node.declaration.type !== 'FunctionDeclaration'
) {
return;
}

const functionNode = node.declaration;
if (functionNode.id.name !== 'start') {
return;
}

const sourceCode = context.getSourceCode();
const functionBody = sourceCode.getText(functionNode.body);

// Check for #region and #endregion markers
const hasRegionMarker = functionBody.includes(
'#region synchronous prelude',
);
const hasEndRegionMarker = functionBody.includes('#endregion');

if (!hasRegionMarker || !hasEndRegionMarker) {
context.report({
node: functionNode,
message:
'start function must contain #region synchronous prelude and #endregion markers',
});
return;
}

// Get the lines between #region and #endregion
const lines = functionBody.split('\n');
let inRegion = false;
let regionStartLine = -1;
let regionEndLine = -1;

for (let i = 0; i < lines.length; i++) {
if (lines[i].includes('#region synchronous prelude')) {
inRegion = true;
regionStartLine = i;
} else if (lines[i].includes('#endregion')) {
regionEndLine = i;
break;
}
}

// Check for await usage before and within the region
let foundAwait = false;

// Use ESLint's built-in traversal
const awaitExpressions = [];
context.getSourceCode().visitorKeys.BlockStatement.forEach(key => {
const bodyContent = functionNode.body[key];
if (Array.isArray(bodyContent)) {
bodyContent.forEach(statement => {
if (statement.type === 'ExpressionStatement' &&
statement.expression.type === 'AwaitExpression') {
const awaitLine = statement.loc.start.line;
const relativeLine = awaitLine - functionNode.loc.start.line;
if (relativeLine <= regionEndLine) {
foundAwait = true;
}
}
});
}
});

if (foundAwait) {
context.report({
node: functionNode,
message:
'await expressions are not allowed before or within the synchronous prelude region',
});
}
},
};
},
};
Loading