Skip to content

Commit

Permalink
feat: Add maxDepth option in connections.targets (#227)
Browse files Browse the repository at this point in the history
This commit introduces a new  option in  which allows for deeper sql tag lookup when using . This is particularly useful for handling nested queries. The documentation has been updated to reflect this change and tests have been added to ensure its functionality.
  • Loading branch information
Newbie012 authored May 30, 2024
1 parent 7587e2f commit 8162460
Show file tree
Hide file tree
Showing 8 changed files with 654 additions and 587 deletions.
20 changes: 20 additions & 0 deletions .changeset/fifty-cobras-wash.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
---
"@ts-safeql/eslint-plugin": minor
---

Support for `maxDepth` in `connections.targets` has been added, allowing deeper sql tag lookup when using `wrapper`:

```json
{
"connections": {
"targets": [
{
"wrapper": "conn.query",
"maxDepth": 2
}
]
}
}
```

This handles nested queries like: ``conn.query(...sql`SELECT id FROM users`)``
4 changes: 4 additions & 0 deletions docs/.vitepress/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,10 @@ export default defineConfig({
{ text: "targets", link: "/api/index.md#connections-targets" },
{ text: "targets.tag", link: "/api/index.md#connections-targets-tag" },
{ text: "targets.wrapper", link: "/api/index.md#connections-targets-wrapper" },
{
text: "targets.maxDepth",
link: "/api/index.md#connections-targets-maxdepth-optional",
},
{
text: "targets.transform",
link: "/api/index.md#connections-targets-transform-optional",
Expand Down
27 changes: 27 additions & 0 deletions docs/api/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,33 @@ conn.query(...);

:::

### `connections.targets.maxDepth` (Optional)

By default, SafeQL assumes that each wrapper has a direct descendant of an sql tag. In some cases, you may want to increase the depth of the sql tag lookup. For example:

```ts
conn.query(sql`SELECT id FROM users`); // depth = 0
conn.query([sql`SELECT id FROM users`]); // depth = 1
conn.query(...sql`SELECT id FROM users`); // depth = 1
conn.query(...[sql`SELECT id FROM users`]); // depth = 2
```

In this case, you can use the `maxDepth` option to specify the maximum depth of the sql tag. For example:

```json
{
"connections": {
// ...
"targets": [
{
"wrapper": "conn.query",
"maxDepth": 2 // [!code focus]
}
]
}
}
```

### `connections.targets.transform` (Optional)

Transform the end result of the query. For example, if you want to transform the result of the query
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,5 +32,5 @@
"turbo": "^1.13.3",
"typescript": "^5.4.5"
},
"packageManager": "[email protected].3"
"packageManager": "[email protected].4"
}
4 changes: 3 additions & 1 deletion packages/eslint-plugin/src/rules/RuleOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,9 @@ const zBaseTarget = z.object({
* ^^^^^^^^^^ wrapper
* ```
*/
const zWrapperTarget = z.object({ wrapper: zStringOrRegex }).merge(zBaseTarget);
const zWrapperTarget = z
.object({ wrapper: zStringOrRegex, maxDepth: z.number().optional() })
.merge(zBaseTarget);
export type WrapperTarget = z.infer<typeof zWrapperTarget>;
/**
* A target that is a tag expression. For example:
Expand Down
33 changes: 22 additions & 11 deletions packages/eslint-plugin/src/rules/check-sql.rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,18 @@ function checkConnectionByTagExpression(params: {
}
}

function getValidParentUntilDepth(node: TSESTree.Node, depth: number) {
if (node.type === "CallExpression" && node.callee.type === "MemberExpression") {
return node;
}

if (depth > 0 && node.parent) {
return getValidParentUntilDepth(node.parent, depth - 1);
}

return null;
}

function checkConnectionByWrapperExpression(params: {
context: RuleContext;
connection: RuleOptionConnection;
Expand All @@ -324,18 +336,17 @@ function checkConnectionByWrapperExpression(params: {
}) {
const { context, tag, projectDir, connection, target } = params;

if (
!isTagMemberValid(tag) ||
!ESTreeUtils.isCallExpression(tag.parent) ||
!ESTreeUtils.isMemberExpression(tag.parent.callee)
) {
if (!isTagMemberValid(tag)) {
return;
}

const calleeAsText = context
.getSourceCode()
.getText(tag.parent.callee)
.replace(/^this\./, "");
const wrapperNode = getValidParentUntilDepth(tag.parent, target.maxDepth ?? 0);

if (wrapperNode === null) {
return;
}

const calleeAsText = context.sourceCode.getText(wrapperNode.callee).replace(/^this\./, "");

if (doesMatchPattern({ pattern: target.wrapper, text: calleeAsText })) {
return reportCheck({
Expand All @@ -344,8 +355,8 @@ function checkConnectionByWrapperExpression(params: {
connection,
target,
projectDir,
baseNode: tag.parent.callee,
typeParameter: tag.parent.typeArguments,
baseNode: wrapperNode.callee,
typeParameter: wrapperNode.typeArguments,
});
}
}
Expand Down
29 changes: 28 additions & 1 deletion packages/eslint-plugin/src/rules/check-sql.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,11 @@ RuleTester.describe("check-sql", () => {
targets: [{ wrapper: { regex: "conn.(query|queryOne|queryOneOrNone)" } }],
keepAlive: false,
},
withMaxDepthOf: (maxDepth: number) => ({
databaseUrl: `postgres://postgres:postgres@localhost:5432/${databaseName}`,
targets: [{ wrapper: "conn.query", maxDepth }],
keepAlive: false,
}),
withTag: {
databaseUrl: `postgres://postgres:postgres@localhost:5432/${databaseName}`,
targets: [{ tag: "sql" }],
Expand All @@ -194,7 +199,7 @@ RuleTester.describe("check-sql", () => {
targets: [{ tag: { regex: "(conn1|conn2).sql" } }],
keepAlive: false,
},
} satisfies Record<string, RuleOptionConnection>;
} satisfies Record<string, RuleOptionConnection | ((...args: never[]) => RuleOptionConnection)>;

function withConnection(
connection: RuleOptionConnection,
Expand Down Expand Up @@ -1088,6 +1093,28 @@ RuleTester.describe("check-sql", () => {
`,
errors: [{ messageId: "missingTypeAnnotations" }, { messageId: "missingTypeAnnotations" }],
},
{
filename,
options: withConnection(connections.withMaxDepthOf(2)),
name: "regex pattern should be checked as well (wrapper regex)",
code: `
conn.query(...sql\`select 1 as num\`);
conn.query([sql\`select 1 as num\`]);
conn.query([...sql\`select 1 as num\`]);
conn.query(...[...sql\`select 1 as num\`]);
`,
output: `
conn.query<{ num: number }>(...sql\`select 1 as num\`);
conn.query<{ num: number }>([sql\`select 1 as num\`]);
conn.query<{ num: number }>([...sql\`select 1 as num\`]);
conn.query(...[...sql\`select 1 as num\`]);
`,
errors: [
{ messageId: "missingTypeAnnotations" },
{ messageId: "missingTypeAnnotations" },
{ messageId: "missingTypeAnnotations" },
],
},
{
filename,
options: withConnection(connections.withGlobTag),
Expand Down
Loading

0 comments on commit 8162460

Please sign in to comment.