From 29714d1ae0c63e4e213304d951efb86c9e898199 Mon Sep 17 00:00:00 2001 From: Johann150 Date: Mon, 2 Jan 2023 20:58:33 +0100 Subject: [PATCH] add custom eslint rule to prefer countBy over findBy --- packages/backend/.eslintrc.cjs | 4 + packages/backend/package.json | 3 +- packages/shared/custom-rules/index.js | 12 ++ .../custom-rules/typeorm-prefer-count.js | 120 ++++++++++++++++++ 4 files changed, 138 insertions(+), 1 deletion(-) create mode 100644 packages/shared/custom-rules/index.js create mode 100644 packages/shared/custom-rules/typeorm-prefer-count.js diff --git a/packages/backend/.eslintrc.cjs b/packages/backend/.eslintrc.cjs index 5a06889dc..4dad2ea08 100644 --- a/packages/backend/.eslintrc.cjs +++ b/packages/backend/.eslintrc.cjs @@ -6,7 +6,11 @@ module.exports = { extends: [ '../shared/.eslintrc.js', ], + plugins: [ + 'foundkey-custom-rules', + ] rules: { + 'foundkey-custom-rules/typeorm-prefer-count': 'error', 'import/order': ['warn', { 'groups': ['builtin', 'external', 'internal', 'parent', 'sibling', 'index', 'object', 'type'], 'pathGroups': [ diff --git a/packages/backend/package.json b/packages/backend/package.json index 261207528..7b710333c 100644 --- a/packages/backend/package.json +++ b/packages/backend/package.json @@ -174,6 +174,7 @@ "execa": "6.1.0", "form-data": "^4.0.0", "sinon": "^14.0.2", - "typescript": "^4.9.4" + "typescript": "^4.9.4", + "eslint-plugin-foundkey-custom-rules": "file:../shared/custom-rules" } } diff --git a/packages/shared/custom-rules/index.js b/packages/shared/custom-rules/index.js new file mode 100644 index 000000000..25330a2e8 --- /dev/null +++ b/packages/shared/custom-rules/index.js @@ -0,0 +1,12 @@ +const fs = require("fs"); +const path = require("path"); + +const ruleFiles = fs + .readdirSync(__dirname) + .filter((file) => file !== "index.js" && !file.endsWith("test.js")); + +const rules = Object.fromEntries( + ruleFiles.map((file) => [path.basename(file, ".js"), require("./" + file)]) +); + +module.exports = { rules }; diff --git a/packages/shared/custom-rules/typeorm-prefer-count.js b/packages/shared/custom-rules/typeorm-prefer-count.js new file mode 100644 index 000000000..fd6357e51 --- /dev/null +++ b/packages/shared/custom-rules/typeorm-prefer-count.js @@ -0,0 +1,120 @@ +const dbFunctions = ["find", "findBy", "findOne", "findOneBy", "findOneOrFail", "findOneByOrFail"]; + +module.exports = { + meta: { + type: "suggestion", + }, + create(context) { + return { + "Program:exit"(programNode) { + const isNull = (node) => node.type === "Literal" && node.value === null; + + const scopes = [context.getScope()]; + while (scopes.length > 0) { + const s = scopes.pop(); + s.childScopes.forEach(x => scopes.push(x)); + + const variables = s.variables + .filter(x => + x.references + .filter((ref) => ref.isRead()) + .length > 0 + && x.defs.length > 0 + ); + + for (const v of variables) { + let findValueAssign = null; + // if the find value was not read, there will already be an unused variable lint + // or maybe this was inside an if/else so we dont want to cause a false positive + let read = false; + for (const ref of v.references) { + if ( + ref.isWrite() + ) { + if (!ref.writeExpr) continue; + let writeExpr = ref.writeExpr; + // unwrap write expression, order matters to correctly unwrap + // something like "await Promise.all([a, b])". + // Basically a poor mans data flow analysis. + if (writeExpr.type === "AwaitExpression") + writeExpr = writeExpr.argument; + if ( + writeExpr.type === "CallExpression" + && writeExpr.callee.type === "MemberExpression" + && writeExpr.callee.object.type === "Identifier" + && writeExpr.callee.object.name === "Promise" + && writeExpr.callee.property.type === "Identifier" + && writeExpr.callee.property.name === "all" + ) + writeExpr = writeExpr.arguments[0]; + if ( + writeExpr.type === "ArrayExpression" + && ref.identifier.parent.type === "ArrayPattern" + ) { + // use same index as the index of the identifier in the array pattern + const index = ref.identifier.parent.elements.indexOf(ref.identifier); + if (index > -1) + writeExpr = writeExpr.elements[index]; + } + // check if this is a DB find thingy + if ( + writeExpr.type === "CallExpression" + && writeExpr.callee.type === "MemberExpression" + && writeExpr.callee.property.type === "Identifier" + && dbFunctions.includes(writeExpr.callee.property.name) + ) { + if (findValueAssign && read) { + context.report({ + node: findValueAssign, + message: "The returned object(s) are not read from, use `count` or `countBy` instead." + }); + } + findValueAssign = writeExpr; + read = false; + } + } + // must be a read + else if (findValueAssign) { + const node = ref.identifier; + if(!( + ( + // explicit null check + node.parent.type === "BinaryExpression" && + ["==", "==="].includes(node.parent.operator) && + ( + (isNull(node.parent.left) && node.parent.right === node) + || + (isNull(node.parent.right) && node.parent.left === node) + ) + ) || ( + // implicit null check + ["IfStatement", "ConditionalExpression"].includes(node.parent.type) + ) || ( + // different implicit null check + node.parent.type === "UnaryExpression" && + node.parent.operator === "!" + ) || ( + // length check + node.parent.type === "MemberExpression" && + node.parent.object === node && + node.parent.property.type === "Identifier" && + node.parent.property.name === "length" + ) + )) { + // other use, dont report + findValueAssign = null; + } + } + } + if (findValueAssign) { + context.report({ + node: findValueAssign, + message: "The returned object(s) are not read from, use `count` or `countBy` instead." + }); + } + } + } + } + }; + } +};