Skip to content

Conversation

@Newbie012
Copy link
Collaborator

@Newbie012 Newbie012 commented Sep 4, 2025

closes #402

Summary by CodeRabbit

  • New Features

    • Adds support for namespace types in type annotations (e.g., InlineNamespace.Type) within SQL type checking.
  • Refactor

    • Improves type resolution to more robustly handle primitives, arrays, unions/intersections, literals, and object shapes, with safer fallbacks.
  • Tests

    • Adds test coverage for namespace-import scenarios, validating correct type checking and autofix behavior.
  • Chores

    • Prepares a patch release entry for the ESLint plugin.

@changeset-bot
Copy link

changeset-bot bot commented Sep 4, 2025

🦋 Changeset detected

Latest commit: 0eb6d2f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 17 packages
Name Type
@ts-safeql/eslint-plugin Patch
@ts-safeql-demos/basic-flat-config Patch
@ts-safeql-demos/basic-migrations-raw Patch
@ts-safeql-demos/basic-transform-type Patch
@ts-safeql-demos/basic Patch
@ts-safeql-demos/big-project Patch
@ts-safeql-demos/config-file-flat-config Patch
@ts-safeql-demos/from-config-file Patch
@ts-safeql-demos/multi-connections Patch
@ts-safeql-demos/playground Patch
@ts-safeql-demos/postgresjs-custom-types Patch
@ts-safeql-demos/postgresjs-demo Patch
@ts-safeql-demos/vercel-postgres Patch
@ts-safeql/generate Patch
@ts-safeql/shared Patch
@ts-safeql/sql-ast Patch
@ts-safeql/test-utils Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@coderabbitai
Copy link

coderabbitai bot commented Sep 4, 2025

Walkthrough

Adds a changeset declaring a patch release for @ts-safeql/eslint-plugin and refactors type-resolution logic to handle namespace types in annotations. Introduces handlers for various TS nodes, safer fallbacks, and new tests covering inline namespace type usage in check-sql.

Changes

Cohort / File(s) Summary
Release metadata
.changeset/lovely-readers-find.md
Adds a patch changeset for @ts-safeql/eslint-plugin with description about namespace types in type annotations.
Rule tests for namespace types
packages/eslint-plugin/src/rules/check-sql.test.ts
Adds “namespace import” test suite covering valid and invalid cases of inline namespace interface usage in generic type parameters, including expected autofix output.
Type resolution refactor and namespace support
packages/eslint-plugin/src/utils/get-resolved-target-by-type-node.ts
Refactors resolution via kind-based dispatch, adds handlers for literals/objects/references/unions/intersections/arrays, centralizes primitive handling, enhances object property extraction, adds robust fallbacks, and explicitly supports namespaced types. Public API unchanged.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant ESLint as ESLint Rule (check-sql)
  participant TS as TypeScript TypeChecker
  participant Resolver as getResolvedTargetByTypeNode
  participant SQL as SQL Schema Comparator

  ESLint->>Resolver: resolve type for generic T (e.g., Namespace.Interface)
  Resolver->>TS: inspect node kind, symbol, type flags
  TS-->>Resolver: type info (literals/unions/objects/refs)
  alt Namespaced reference
    Resolver->>Resolver: extract object properties via symbol/valueDeclaration
  else Primitive/array/union/etc.
    Resolver->>Resolver: handle via dedicated kind handlers
  end
  Resolver-->>ESLint: ExpectedResolvedTarget (shape or typeName)
  ESLint->>SQL: compare query columns vs resolved shape
  alt Match
    ESLint-->>ESLint: no report
  else Mismatch
    ESLint-->>Developer: report incorrectTypeAnnotations + autofix
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Assessment against linked issues

Objective Addressed Explanation
Support comparing interfaces located in a namespace in type annotations [#402]

Assessment against linked issues: Out-of-scope changes

Code Change Explanation

Poem

I hop through types, from fields to names,
A namespace burrow—no longer games!
With unions, arrays, objects too,
I sniff their shapes and see them through.
Patch squeaks in, tests light the way—
Carrots for compile-time! Hippity hooray! 🥕

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat-namespaces

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore or @coderabbit ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@vercel
Copy link

vercel bot commented Sep 4, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Preview Comments Updated (UTC)
safeql Ignored Ignored Sep 4, 2025 7:21pm

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (6)
packages/eslint-plugin/src/rules/check-sql.test.ts (1)

2133-2167: Add cases for array and optional fields on namespaced types.

To guard against regressions in array handling and optional properties within namespaces, please add:

  • Caregiver.Name[] and Array<Caregiver.Name>
  • Optional field in the namespaced interface (e.g., middleName?: string | null) and the corresponding SELECT

Here’s a minimal addition:

@@
   ruleTester.run("namespace import", rules["check-sql"], {
     valid: [
       {
         name: "select statement with type imported from inline namespace",
@@
       },
+      {
+        name: "array of namespaced type (both syntaxes)",
+        options: withConnection(connections.base),
+        code: `
+          namespace Caregiver {
+            export interface Name { firstName: string; lastName: string; }
+          }
+          async function run() {
+            const r1 = conn.query<Caregiver.Name[]>(sql\`
+              select first_name as "firstName", last_name as "lastName" from caregiver
+            \`);
+            const r2 = conn.query<Array<Caregiver.Name>>(sql\`
+              select first_name as "firstName", last_name as "lastName" from caregiver
+            \`);
+          }
+        `,
+      },
+      {
+        name: "optional property in namespaced type",
+        options: withConnection(connections.base),
+        code: `
+          namespace Caregiver {
+            export interface Partial { middle_name?: string | null }
+          }
+          function run() {
+            const result = conn.query<Caregiver.Partial>(sql\`
+              select middle_name from caregiver
+            \`);
+          }
+        `,
+      },
     ],
packages/eslint-plugin/src/utils/get-resolved-target-by-type-node.ts (5)

279-304: Avoid expanding node_modules interface shapes by using declarations, not only valueDeclaration.

Interfaces from node_modules don’t have valueDeclaration, so they fall through to property expansion. That can be slow and noisy. Prefer returning a nominal { kind: "type", value: symbol.name } when all declarations are in node_modules.

-  if (type.symbol.valueDeclaration) {
-    const declaration = type.symbol.valueDeclaration;
-    const sourceFile = declaration.getSourceFile();
-    const filePath = sourceFile.fileName;
-
-    if (!filePath.includes("node_modules")) {
-      return extractObjectProperties(type, params);
-    }
-
-    return { kind: "type", value: type.symbol.name };
-  }
+  if (type.symbol.declarations && type.symbol.declarations.length > 0) {
+    const declFiles = type.symbol.declarations.map((d) => d.getSourceFile().fileName);
+    const allInNodeModules = declFiles.every((p) => p.includes("node_modules"));
+    if (allInNodeModules) {
+      return { kind: "type", value: type.symbol.name };
+    }
+    return extractObjectProperties(type, params);
+  }

208-225: Primitive detection: broaden coverage and simplify.

type.flags often combines bits (e.g., TypeFlags.BooleanLiteral), so the current direct map can miss cases. Prefer checker.typeToString for literals already handled; otherwise gate booleans via checker.typeToString.

-  const flagMap = {
-    [ts.TypeFlags.String]: "string",
-    [ts.TypeFlags.Number]: "number",
-    [ts.TypeFlags.Boolean]: "boolean",
-    [ts.TypeFlags.Null]: "null",
-    [ts.TypeFlags.Undefined]: "undefined",
-    [ts.TypeFlags.Any]: "any",
-  } as const;
-
-  return flagMap[type.flags as keyof typeof flagMap]
-    ? { kind: "type", value: flagMap[type.flags as keyof typeof flagMap] }
-    : null;
+  if (type.flags & ts.TypeFlags.String) return { kind: "type", value: "string" };
+  if (type.flags & ts.TypeFlags.Number) return { kind: "type", value: "number" };
+  if (type.flags & ts.TypeFlags.Boolean) return { kind: "type", value: "boolean" };
+  if (type.flags & ts.TypeFlags.Null) return { kind: "type", value: "null" };
+  if (type.flags & ts.TypeFlags.Undefined) return { kind: "type", value: "undefined" };
+  if (type.flags & ts.TypeFlags.Any) return { kind: "type", value: "any" };
+  return null;

85-90: Literal handler: support negative numeric literals.

TSLiteralType may wrap a UnaryExpression for negatives; current code returns "unknown".

-  return typeNode.literal.type === TSESTree.AST_NODE_TYPES.Literal
-    ? { kind: "type", value: `'${typeNode.literal.value}'` }
-    : { kind: "type", value: "unknown" };
+  if (typeNode.literal.type === TSESTree.AST_NODE_TYPES.Literal) {
+    return { kind: "type", value: `'${typeNode.literal.value}'` };
+  }
+  // Fallback to exact text for unary (e.g., -1)
+  return { kind: "type", value: (typeNode as unknown as { raw: string })?.raw ?? "unknown" };

11-17: ExpectedResolvedTarget['literal'] variant is never produced.

Either emit { kind: "literal", ... } for literals or remove the unused union member.

-  | { kind: "literal"; value: string; base: ExpectedResolvedTarget }
+  // Remove if not used, or start emitting it from literal handlers.

172-206: Array detection on resolveType: safer suffix strip.

replace("[]", "") strips only the first occurrence. Use a suffix-only regex to be correct for nested arrays.

-    return { kind: "array", value: { kind: "type", value: typeAsString.replace("[]", "") } };
+    return { kind: "array", value: { kind: "type", value: typeAsString.replace(/\[\]$/, "") } };
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 5763776 and 0eb6d2f.

📒 Files selected for processing (3)
  • .changeset/lovely-readers-find.md (1 hunks)
  • packages/eslint-plugin/src/rules/check-sql.test.ts (1 hunks)
  • packages/eslint-plugin/src/utils/get-resolved-target-by-type-node.ts (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: check (ubuntu-latest, 20)
🔇 Additional comments (2)
.changeset/lovely-readers-find.md (1)

2-5: Patch note is crisp and scoped correctly.

Good: patch release only for eslint-plugin; message matches the change.

packages/eslint-plugin/src/rules/check-sql.test.ts (1)

2111-2131: Great coverage for inline namespace happy-path.

This validates TSQualifiedName generics end-to-end.

@Newbie012 Newbie012 merged commit 0177905 into main Sep 7, 2025
4 checks passed
@Newbie012 Newbie012 deleted the feat-namespaces branch September 7, 2025 19:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

lib cannot compare interfaces if it is located in a namespace

2 participants