Skip to content

Commit 4593d35

Browse files
thomasballingerConvex, Inc.
authored and
Convex, Inc.
committed
Improve eslint plugin (#36486)
A little less stringent: in the default configuration allow no args validator when a handler uses no args. GitOrigin-RevId: d768c99fe447b39b16f68a89a421d90b5546ca38
1 parent fc886ed commit 4593d35

File tree

7 files changed

+233
-132
lines changed

7 files changed

+233
-132
lines changed

npm-packages/@convex-dev/eslint-plugin/README.md

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,11 @@ files in the `convex/` directory.
77

88
### ESLint 8 (`.eslintrc.js`)
99

10-
In `.eslintrc.js`:
11-
1210
```bash
1311
npm i @typescript-eslint/eslint-plugin @convex-dev/eslint-plugin
1412
```
1513

16-
and add these two
14+
and add these two in `.eslintrc.js`:
1715

1816
```js
1917
module.exports = {
@@ -27,6 +25,10 @@ module.exports = {
2725

2826
### ESLint 9 (`eslint.config.js`)
2927

28+
```bash
29+
npm i @convex-dev/eslint-plugin
30+
```
31+
3032
In `eslint.config.js`:
3133

3234
```js
Lines changed: 21 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@convex-dev/eslint-plugin",
3-
"version": "0.0.0-alpha.0",
3+
"version": "0.0.1-alpha.4",
44
"license": "Apache-2.0",
55
"type": "module",
66
"scripts": {
@@ -14,11 +14,20 @@
1414
"test:watch": "vitest",
1515
"prepare": "tshy"
1616
},
17-
"files": [
18-
"dist",
19-
"src/index.ts",
20-
"LICENSE"
21-
],
17+
"dependencies": {
18+
"@typescript-eslint/utils": "~8.4.0"
19+
},
20+
"devDependencies": {
21+
"@types/eslint": "~9.6.1",
22+
"@types/node": "^18.17.0",
23+
"@typescript-eslint/rule-tester": "~8.4.0",
24+
"@typescript-eslint/types": "~8.4.0",
25+
"eslint": "9.21.0",
26+
"eslint-doc-generator": "^1.7.1",
27+
"tshy": "^3.0.0",
28+
"typescript": "^5.7.2",
29+
"vitest": "^1.6.1"
30+
},
2231
"tshy": {
2332
"dialects": [
2433
"esm",
@@ -33,18 +42,11 @@
3342
],
3443
"selfLink": false
3544
},
36-
"peerDependencies": {
37-
"eslint": "9",
38-
"@types/eslint": "~9.6.1",
39-
"@typescript-eslint/rule-tester": "~8.4.0",
40-
"@typescript-eslint/types": "~8.4.0",
41-
"@typescript-eslint/utils": "~8.4.0",
42-
"eslint-doc-generator": "~1.7.1"
43-
},
44-
"devDependencies": {
45-
"tshy": "^3.0.0",
46-
"typescript": "~5.0.3"
47-
},
45+
"files": [
46+
"dist",
47+
"src/index.ts",
48+
"LICENSE"
49+
],
4850
"exports": {
4951
".": {
5052
"import": {
@@ -60,9 +62,5 @@
6062
},
6163
"module": "./dist/esm/index.js",
6264
"main": "./dist/commonjs/index.js",
63-
"types": "./dist/commonjs/index.d.ts",
64-
"dependencies": {
65-
"@types/node": "^18.17.0",
66-
"vitest": "^1.6.1"
67-
}
65+
"types": "./dist/commonjs/index.d.ts"
6866
}

npm-packages/@convex-dev/eslint-plugin/src/index.ts

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,27 @@
11
import { noImportUseNode } from "./lib/noImportUseNode.js";
22
import noOldRegisteredFunctionSyntax from "./lib/noOldRegisteredFunctionSyntax.js";
3-
import noMissingArgs from "./lib/noMissingArgs.js";
3+
import { noMissingArgs, noArgsWithoutValidator } from "./lib/noMissingArgs.js";
44
import { RuleModule } from "@typescript-eslint/utils/ts-eslint";
55

66
const rules = {
77
"no-old-registered-function-syntax": noOldRegisteredFunctionSyntax,
8+
"no-args-without-validator": noArgsWithoutValidator,
89
"no-missing-args-validator": noMissingArgs,
910
"import-wrong-runtime": noImportUseNode,
1011
} satisfies Record<string, RuleModule<any>>;
1112

1213
const recommendedRules = {
13-
// This rule is a good idea but hard to convert projects to later.
14+
// This rule is a good idea but bothersome to convert projects to later:
15+
// it's possible to safely import specific exports from a "use node"
16+
// file if all Node.js-specific imports are side-effect free.
1417
"@convex-dev/import-wrong-runtime": "off",
1518
"@convex-dev/no-old-registered-function-syntax": "error",
16-
"@convex-dev/no-missing-args-validator": "error",
19+
// This is a reasonable idea in large projects: throw at runtime
20+
// when API endpoints that don't expect arguments receive them.
21+
// But it lacks the typical benefit of a validator providing
22+
// types so it feels more pedantic.
23+
"@convex-dev/no-missing-args-validator": "off",
24+
"@convex-dev/no-args-without-validator": "error",
1725
} satisfies {
1826
[key: `@convex-dev/${string}`]: "error" | "warn" | "off";
1927
};
@@ -22,7 +30,12 @@ const isESM = typeof require === "undefined";
2230

2331
// Base plugin structure, common across ESLint 8 and 9
2432
const plugin = {
25-
configs: {},
33+
// loose types so this can work with ESlint 8 and 9
34+
configs: {} as {
35+
recommended: any;
36+
/** Only available in ESlint 8 */
37+
recommendedRulesCustomConvexDirectoryLocation: any;
38+
},
2639
meta: {
2740
name: "@convex-dev/eslint-plugin",
2841
version: "0.0.0-alpha.0",
Lines changed: 127 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,67 @@
11
import type { TSESTree } from "@typescript-eslint/utils";
22
import { CONVEX_REGISTRARS, createRule } from "../util.js";
33

4+
/**
5+
* Helper function to check if an object expression has an args property
6+
*/
7+
function hasArgsProperty(objectExpr: TSESTree.ObjectExpression): boolean {
8+
return objectExpr.properties.some(
9+
(prop) =>
10+
prop.type === "Property" &&
11+
prop.key.type === "Identifier" &&
12+
prop.key.name === "args",
13+
);
14+
}
15+
16+
/**
17+
* Helper function to check if a handler function has a second parameter (args parameter)
18+
*/
19+
function handlerHasArgsParameter(handler: TSESTree.Property): boolean {
20+
if (
21+
handler.value.type === "ArrowFunctionExpression" ||
22+
handler.value.type === "FunctionExpression"
23+
) {
24+
return handler.value.params.length >= 2;
25+
}
26+
return false;
27+
}
28+
29+
/**
30+
* Helper function to get the handler property from an object expression
31+
*/
32+
function getHandlerProperty(
33+
objectExpr: TSESTree.ObjectExpression,
34+
): TSESTree.Property | undefined {
35+
return objectExpr.properties.find(
36+
(prop) =>
37+
prop.type === "Property" &&
38+
prop.key.type === "Identifier" &&
39+
prop.key.name === "handler",
40+
) as TSESTree.Property | undefined;
41+
}
42+
43+
/**
44+
* Helper function to create a fix for missing args property
45+
*/
46+
function createArgsFix(
47+
context: any,
48+
objectArg: TSESTree.ObjectExpression,
49+
): ((fixer: any) => any) | undefined {
50+
return (fixer) => {
51+
const sourceCode = context.getSourceCode();
52+
const objectText = sourceCode.getText(objectArg);
53+
const firstBracePos = objectText.indexOf("{");
54+
55+
if (firstBracePos === -1) return null;
56+
57+
const insertPos = objectArg.range[0] + firstBracePos + 1;
58+
return fixer.insertTextAfterRange(
59+
[insertPos, insertPos],
60+
"\n args: {},\n",
61+
);
62+
};
63+
}
64+
465
/**
566
* Rule to enforce that every registered Convex function has an args property
667
*/
@@ -17,61 +78,19 @@ export const noMissingArgs = createRule({
1778
"Convex function is missing args validator but has parameter. Add appropriate args validator.",
1879
},
1980
schema: [],
20-
fixable: "code", // Now fixable when handler doesn't have second parameter
81+
fixable: "code",
2182
hasSuggestions: true,
2283
},
2384
defaultOptions: [],
2485
create: (context) => {
25-
// Skip generated files
2686
const filename = context.getFilename();
2787
const isGenerated = filename.includes("_generated");
2888
if (isGenerated) {
2989
return {};
3090
}
3191

32-
/**
33-
* Checks if an object expression has an args property
34-
*/
35-
function hasArgsProperty(objectExpr: TSESTree.ObjectExpression): boolean {
36-
return objectExpr.properties.some(
37-
(prop) =>
38-
prop.type === "Property" &&
39-
prop.key.type === "Identifier" &&
40-
prop.key.name === "args",
41-
);
42-
}
43-
44-
/**
45-
* Checks if a handler function has a second parameter (args parameter)
46-
*/
47-
function handlerHasArgsParameter(handler: TSESTree.Property): boolean {
48-
if (
49-
handler.value.type === "ArrowFunctionExpression" ||
50-
handler.value.type === "FunctionExpression"
51-
) {
52-
return handler.value.params.length >= 2;
53-
}
54-
return false;
55-
}
56-
57-
/**
58-
* Gets the handler property from an object expression
59-
*/
60-
function getHandlerProperty(
61-
objectExpr: TSESTree.ObjectExpression,
62-
): TSESTree.Property | undefined {
63-
return objectExpr.properties.find(
64-
(prop) =>
65-
prop.type === "Property" &&
66-
prop.key.type === "Identifier" &&
67-
prop.key.name === "handler",
68-
) as TSESTree.Property | undefined;
69-
}
70-
7192
return {
72-
// Check variable declarations for exports that use object syntax
7393
VariableDeclarator(node) {
74-
// Only interested in export declarations
7594
const parentDecl = node.parent;
7695
if (!parentDecl) return;
7796

@@ -83,7 +102,6 @@ export const noMissingArgs = createRule({
83102
return;
84103
}
85104

86-
// Check if it's a call to a registrar with an object argument
87105
if (
88106
node.init?.type === "CallExpression" &&
89107
node.init.callee.type === "Identifier" &&
@@ -93,38 +111,17 @@ export const noMissingArgs = createRule({
93111
) {
94112
const objectArg = node.init.arguments[0] as TSESTree.ObjectExpression;
95113

96-
// Check if the object has an args property
97114
if (!hasArgsProperty(objectArg)) {
98115
const handlerProp = getHandlerProperty(objectArg);
99-
100-
// Determine if the handler has a second parameter
101116
const handlerHasArgs =
102117
handlerProp && handlerHasArgsParameter(handlerProp);
103118

104119
context.report({
105120
node: objectArg,
106121
messageId: handlerHasArgs ? "missing-args" : "missing-empty-args",
107-
// Only provide a fix if the handler doesn't have a second parameter
108122
fix: handlerHasArgs
109123
? undefined
110-
: (fixer) => {
111-
// Find the position to insert args property
112-
const sourceCode = context.getSourceCode();
113-
const objectText = sourceCode.getText(objectArg);
114-
const firstBracePos = objectText.indexOf("{");
115-
116-
// Make sure we found the opening brace
117-
if (firstBracePos === -1) return null;
118-
119-
// Insert args property after the opening brace
120-
const insertPos = objectArg.range[0] + firstBracePos + 1;
121-
122-
// Add args: {} at the beginning of the object
123-
return fixer.insertTextAfterRange(
124-
[insertPos, insertPos],
125-
"\n args: {},\n",
126-
);
127-
},
124+
: createArgsFix(context, objectArg),
128125
});
129126
}
130127
}
@@ -133,4 +130,67 @@ export const noMissingArgs = createRule({
133130
},
134131
});
135132

136-
export default noMissingArgs;
133+
/**
134+
* Rule to enforce that Convex functions with args parameters have args validators
135+
*/
136+
export const noArgsWithoutValidator = createRule({
137+
name: "no-args-without-validator",
138+
meta: {
139+
type: "suggestion",
140+
docs: {
141+
description:
142+
"Convex functions with args parameters should validate their arguments.",
143+
},
144+
messages: {
145+
"missing-args":
146+
"Convex function is missing args validator but has parameter. Add appropriate args validator.",
147+
},
148+
schema: [],
149+
fixable: "code",
150+
hasSuggestions: true,
151+
},
152+
defaultOptions: [],
153+
create: (context) => {
154+
const filename = context.getFilename();
155+
const isGenerated = filename.includes("_generated");
156+
if (isGenerated) {
157+
return {};
158+
}
159+
160+
return {
161+
VariableDeclarator(node) {
162+
const parentDecl = node.parent;
163+
if (!parentDecl) return;
164+
165+
const exportDecl = parentDecl.parent;
166+
if (
167+
exportDecl?.type !== "ExportNamedDeclaration" &&
168+
parentDecl.parent?.parent?.type !== "ExportNamedDeclaration"
169+
) {
170+
return;
171+
}
172+
173+
if (
174+
node.init?.type === "CallExpression" &&
175+
node.init.callee.type === "Identifier" &&
176+
CONVEX_REGISTRARS.includes(node.init.callee.name) &&
177+
node.init.arguments.length === 1 &&
178+
node.init.arguments[0].type === "ObjectExpression"
179+
) {
180+
const objectArg = node.init.arguments[0] as TSESTree.ObjectExpression;
181+
const handlerProp = getHandlerProperty(objectArg);
182+
const handlerHasArgs =
183+
handlerProp && handlerHasArgsParameter(handlerProp);
184+
185+
if (handlerHasArgs && !hasArgsProperty(objectArg)) {
186+
context.report({
187+
node: objectArg,
188+
messageId: "missing-args",
189+
fix: createArgsFix(context, objectArg),
190+
});
191+
}
192+
}
193+
},
194+
};
195+
},
196+
});

0 commit comments

Comments
 (0)