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

Add node-execute-block-error-missing-item-index #86

Merged
merged 2 commits into from
Jul 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 README.md
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ If you specify both the plugin and a config, all the rules in the config (theme-
| [node-class-description-outputs-wrong](docs/rules/node-class-description-outputs-wrong.md) | The number of `outputs` in node class description for any node must be one, or two for If node, or four for Switch node. | Yes |
| [node-dirname-against-convention](docs/rules/node-dirname-against-convention.md) | Node dirname must match node filename, excluding the filename suffix. Example: `Test` node dirname matches `Test` section of `Test.node.ts` node filename. | No |
| [node-execute-block-double-assertion-for-items](docs/rules/node-execute-block-double-assertion-for-items.md) | In the `execute()` method there is no need to double assert the type of `items.length`. | Yes |
| [node-execute-block-error-missing-item-index](docs/rules/node-execute-block-error-missing-item-index.md) | In the `execute()` method in a node, `NodeApiError` and `NodeOperationError` must specify the `itemIndex` as the third argument. | No |
| [node-execute-block-missing-continue-on-fail](docs/rules/node-execute-block-missing-continue-on-fail.md) | The `execute()` method in a node must implement `continueOnFail` in a try-catch block. | No |
| [node-execute-block-operation-missing-plural-pairing](docs/rules/node-execute-block-operation-missing-plural-pairing.md) | Every `getAll` operation in the `execute()` method in a node must implement plural pairing. | Depends |
| [node-execute-block-operation-missing-singular-pairing](docs/rules/node-execute-block-operation-missing-singular-pairing.md) | Every non-`getAll` operation in the `execute()` method in a node must implement singular pairing. | No |
Expand Down
58 changes: 58 additions & 0 deletions docs/rules/node-execute-block-error-missing-item-index.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
[//]: # "File generated from a template. Do not edit this file directly."

# node-execute-block-error-missing-item-index

In the `execute()` method in a node, `NodeApiError` and `NodeOperationError` must specify the `itemIndex` as the third argument.

📋 This rule is part of the `plugin:n8n-nodes-base/nodes` config.

## Examples

❌ Example of **incorrect** code:

```js
class TestNode {
execute() {
throw new NodeApiError(this.getNode(), "An error occurred");
}
}

class TestNode {
execute() {
throw new NodeOperationError(this.getNode(), "An error occurred");
}
}

class TestNode {
execute() {
throw new NodeApiError(this.getNode(), "An error occurred", {
testIndex: 1,
});
}
}
```

✅ Example of **correct** code:

```js
class TestNode {
execute() {
throw new NodeApiError(this.getNode(), "An error occurred", {
itemIndex: i,
});
}
}

class TestNode {
execute() {
throw new NodeOperationError(this.getNode(), "An error occurred", {
itemIndex: i,
});
}
}
```

## Links

- [Rule source](../../lib/rules/node-execute-block-error-missing-item-index.ts)
- [Test source](../../tests/node-execute-block-error-missing-item-index.test.ts)
65 changes: 28 additions & 37 deletions lib/ast/getters/nodeExecuteBlock.getters.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
import { assert } from "console";
import { TSESTree, AST_NODE_TYPES } from "@typescript-eslint/utils";
import { id } from "../identifiers";

export function getOperationConsequents(
node: TSESTree.MethodDefinition,
{ type }: { type: "singular" | "plural" } // filter getAll or non-getAll
{ filter }: { filter: "singular" | "plural" | "all" } // filter getAll or non-getAll or do not filter
) {
const executeMethod = getExecuteMethod(node);
const executeMethod = getExecuteContent(node);

if (!executeMethod) return;

Expand Down Expand Up @@ -35,13 +34,17 @@ export function getOperationConsequents(
const operationConsequents = collectConsequents(resourcesRoot).reduce<
TSESTree.BlockStatement[]
>((acc, resourceConsequent) => {
assertSingleConditionalChain(resourceConsequent);
// TODO: Handle resource consequent with more than one `IfStatement`
if (resourceConsequent.body.length !== 1) return acc;

const [operationsRoot] = resourceConsequent.body;
const opConsequentsPerResource = collectConsequents(operationsRoot).filter(
(consequent) =>
type === "plural" ? isGetAll(consequent) : !isGetAll(consequent)
);

const opConsequentsPerResource =
filter === "all"
? collectConsequents(operationsRoot)
: collectConsequents(operationsRoot).filter((consequent) =>
filter === "plural" ? isGetAll(consequent) : !isGetAll(consequent)
);

return [...acc, ...opConsequentsPerResource];
}, []);
Expand All @@ -54,38 +57,26 @@ export function getOperationConsequents(
}

/**
* A conditional chain `if → else if → else` is reflected as
* an AST node with a consequent and increasingly nested alternates.
* Operation blocks are expected to contain a **single** conditional chain
* of indefinitely nested alternates.
*/
function assertSingleConditionalChain(
resourceConsequent: TSESTree.BlockStatement
) {
assert(
resourceConsequent.body.length === 1,
[
"Assertion failed: Found a resource consequent with more than one if-statement",
JSON.stringify(resourceConsequent.loc),
].join("\n")
);
}

/**
* Get the content of then `execute()` method.
* Get the block (i.e., content) of the `execute()` method.
*
* ```ts
* async execute(this: IExecuteFunctions): Promise<INodeExecutionData[][]> { _ }
* ```
*
* IMPORTANT! Do _not_ use the string `ock` in a typed function name, e.g. `getExecuteBlock`.
* `esbuild-jest` fails to transpile when searching for `ock` (`jest.mock`) in typed functions.
*
* https://github.com/aelbore/esbuild-jest/issues/57
* https://github.com/aelbore/esbuild-jest/blob/master/src/index.ts#L33-L40
*/
function getExecuteMethod(node: TSESTree.MethodDefinition) {
export function getExecuteContent({ key, value }: TSESTree.MethodDefinition) {
if (
node.key.type === AST_NODE_TYPES.Identifier &&
node.key.name === "execute" &&
node.value.type === AST_NODE_TYPES.FunctionExpression &&
node.value.body.type === AST_NODE_TYPES.BlockStatement
key.type === AST_NODE_TYPES.Identifier &&
key.name === "execute" &&
value.type === AST_NODE_TYPES.FunctionExpression &&
value.body.type === AST_NODE_TYPES.BlockStatement
) {
return node.value.body;
return value.body;
}
}

Expand Down Expand Up @@ -126,13 +117,13 @@ function getReturnDataArrayName(executeMethod: TSESTree.BlockStatement) {
}

/**
* Get the name of the `items` AST.
* Get the name of the index used to iterate through the input items.
*
* ```ts
* for (let _ = 0; _ < items.length; _++) {
* ```
*/
function getInputItemsIndexName(
export function getInputItemsIndexName(
forLoop: TSESTree.ForStatement & {
body: TSESTree.BlockStatement;
}
Expand All @@ -151,9 +142,9 @@ function getInputItemsIndexName(
}

/**
* Collect all the recursively nested consequents of if statements into an array.
* Recursively collect every alternate and consequent in an `IfStatement` and its children.
*/
function collectConsequents(
export function collectConsequents(
node: TSESTree.Node,
collection: TSESTree.BlockStatement[] = []
) {
Expand Down
2 changes: 2 additions & 0 deletions lib/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,8 @@ export const DOCUMENTATION = {
// misc
// ----------------------------------

export const NODE_ERROR_TYPES = ["NodeOperationError", "NodeApiError"];

/**
* Credentials exempted from these rules:
* - `cred-class-name-unsuffixed`
Expand Down
31 changes: 11 additions & 20 deletions lib/rules/node-execute-block-double-assertion-for-items.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { AST_NODE_TYPES, TSESTree } from "@typescript-eslint/utils";
import { utils } from "../ast/utils";
import { getters } from "../ast/getters";

export default utils.createRule({
name: utils.getRuleName(module),
Expand All @@ -22,38 +23,28 @@ export default utils.createRule({
MethodDefinition(node) {
if (!utils.isNodeFile(context.getFilename())) return;

const executeMethod = getExecuteMethod(node);
const executeContent = getters.nodeExecuteBlock.getExecuteContent(node);

if (!executeMethod) return;
if (!executeContent) return;

const declarationInit = getDoublyAssertedDeclarationInit(executeMethod);
const init = getDoublyAssertedDeclarationInit(executeContent);

if (declarationInit) {
if (init) {
context.report({
messageId: "removeDoubleAssertion",
node: declarationInit,
fix: (fixer) => fixer.replaceText(declarationInit, "items.length"),
node: init,
fix: (fixer) => fixer.replaceText(init, "items.length"),
});
}
},
};
},
});

function getExecuteMethod(node: TSESTree.MethodDefinition) {
if (
node.key.type === AST_NODE_TYPES.Identifier &&
node.key.name === "execute" &&
node.value.type === AST_NODE_TYPES.FunctionExpression &&
node.value.body.type === AST_NODE_TYPES.BlockStatement
) {
return node.value.body;
}

return null;
}

function getDoublyAssertedDeclarationInit(executeMethod: TSESTree.BlockStatement) {
// TODO: Refactor
function getDoublyAssertedDeclarationInit(
executeMethod: TSESTree.BlockStatement
) {
for (const node of executeMethod.body) {
if (node.type === AST_NODE_TYPES.VariableDeclaration) {
for (const declaration of node.declarations) {
Expand Down
Loading