diff --git a/.changeset/eslint-plugin-add-promise-type.md b/.changeset/eslint-plugin-add-promise-type.md new file mode 100644 index 00000000000..78bf71e0c1e --- /dev/null +++ b/.changeset/eslint-plugin-add-promise-type.md @@ -0,0 +1,7 @@ +--- +'@clerk/eslint-plugin': patch +--- + +Handle non-promise return types when `require-auth-protection` rule fixer makes the function `async`. + +The eslint rule fixer will now wrap a non-promise return type with `Promise<>` to produce valid TypeScript. diff --git a/.changeset/eslint-plugin-fixer-quote-style.md b/.changeset/eslint-plugin-fixer-quote-style.md new file mode 100644 index 00000000000..49ce363508e --- /dev/null +++ b/.changeset/eslint-plugin-fixer-quote-style.md @@ -0,0 +1,7 @@ +--- +'@clerk/eslint-plugin': patch +--- + +The `require-auth-protection` fixer now matches the string quote style of existing imports when inserting a new `auth` import. + +Previously, new imports always used single quotes regardless of how other imports in the file were quoted. diff --git a/.changeset/eslint-plugin-or-auth-guards.md b/.changeset/eslint-plugin-or-auth-guards.md new file mode 100644 index 00000000000..178712f3b64 --- /dev/null +++ b/.changeset/eslint-plugin-or-auth-guards.md @@ -0,0 +1,7 @@ +--- +'@clerk/eslint-plugin': patch +--- + +The `require-auth-protection` rule now accepts OR-conditions like `if (!isAuthenticated || otherCondition)` when determining if a resource is protected. + +Previously, only bare auth checks such as `if (!isAuthenticated)` were recognized. Guards with only `||` are safe but were incorrectly reported as missing protection. diff --git a/packages/eslint-plugin/src/next/__tests__/fix-auth-protection.test.ts b/packages/eslint-plugin/src/next/__tests__/fix-auth-protection.test.ts index 5c4c75679d7..b2eeeaf5437 100644 --- a/packages/eslint-plugin/src/next/__tests__/fix-auth-protection.test.ts +++ b/packages/eslint-plugin/src/next/__tests__/fix-auth-protection.test.ts @@ -79,7 +79,7 @@ export default async function Page() { expect(result.unresolved).toEqual([]); expect(result.fixed).toEqual([{ filePath: page, protections: 1 }]); - expect(await readFile(page, 'utf8')).toBe(`import { auth } from '@clerk/nextjs/server'; + expect(await readFile(page, 'utf8')).toBe(`import { auth } from "@clerk/nextjs/server"; export default async function Page() { await auth.protect(); return
Hello
; @@ -109,7 +109,7 @@ export async function POST() { expect(result.fixed).toEqual([{ filePath: route, protections: 2 }]); const output = await readFile(route, 'utf8'); - expect(output).toBe(`import { auth } from '@clerk/nextjs/server'; + expect(output).toBe(`import { auth } from "@clerk/nextjs/server"; export async function GET() { await auth.protect(); return new Response('ok'); diff --git a/packages/eslint-plugin/src/next/__tests__/quote-style.test.ts b/packages/eslint-plugin/src/next/__tests__/quote-style.test.ts new file mode 100644 index 00000000000..8fc34c3e9c9 --- /dev/null +++ b/packages/eslint-plugin/src/next/__tests__/quote-style.test.ts @@ -0,0 +1,44 @@ +import * as tsParser from '@typescript-eslint/parser'; +import type { TSESLint, TSESTree } from '@typescript-eslint/utils'; +import { describe, expect, it } from 'vitest'; + +import { inferQuoteChar } from '../lib/quote-style'; + +function parse(code: string): { sourceCode: TSESLint.SourceCode; program: TSESTree.Program } { + const program = tsParser.parse(code, { + ecmaVersion: 'latest', + sourceType: 'module', + ecmaFeatures: { jsx: true }, + }) as TSESTree.Program; + const sourceCode = { + ast: program, + getText(node: TSESTree.Node) { + return code.slice(node.range[0], node.range[1]); + }, + } as TSESLint.SourceCode; + return { sourceCode, program }; +} + +describe('inferQuoteChar', () => { + it('returns double quotes from a double-quoted import', () => { + const { sourceCode, program } = parse('import { x } from "@pkg/foo";'); + expect(inferQuoteChar(sourceCode, program)).toBe('"'); + }); + + it('returns single quotes from a single-quoted import', () => { + const { sourceCode, program } = parse("import { x } from '@pkg/foo';"); + expect(inferQuoteChar(sourceCode, program)).toBe("'"); + }); + + it('returns double quotes from a double-quoted export source', () => { + const { sourceCode, program } = parse('export { GET } from "./route";'); + expect(inferQuoteChar(sourceCode, program)).toBe('"'); + }); + + it('falls back to double quotes when the file has no module sources to infer from', () => { + const { sourceCode, program } = parse(`export default function Page() { + return
; +}`); + expect(inferQuoteChar(sourceCode, program)).toBe('"'); + }); +}); diff --git a/packages/eslint-plugin/src/next/__tests__/require-auth-protection.suggestions.test.ts b/packages/eslint-plugin/src/next/__tests__/require-auth-protection.suggestions.test.ts index 32a50529b5d..99b5eaeb1f1 100644 --- a/packages/eslint-plugin/src/next/__tests__/require-auth-protection.suggestions.test.ts +++ b/packages/eslint-plugin/src/next/__tests__/require-auth-protection.suggestions.test.ts @@ -44,7 +44,7 @@ ruleTester.run('require-auth-protection (suggestions)', rule, { suggestions: [ { messageId: 'addAuthProtect', - output: `import { auth } from '@clerk/nextjs/server'; + output: `import { auth } from "@clerk/nextjs/server"; export default async function Page() { await auth.protect(); return
Hello
; @@ -65,7 +65,7 @@ export default async function Page() { suggestions: [ { messageId: 'addAuthProtect', - output: `import { auth } from '@clerk/nextjs/server'; + output: `import { auth } from "@clerk/nextjs/server"; export default async () => { await auth.protect(); return null; @@ -86,7 +86,7 @@ export default async () => { suggestions: [ { messageId: 'addAuthProtect', - output: `import { auth } from '@clerk/nextjs/server'; + output: `import { auth } from "@clerk/nextjs/server"; export const GET = async () => { await auth.protect(); return { ok: true }; @@ -107,7 +107,7 @@ export const GET = async () => { suggestions: [ { messageId: 'addAuthProtect', - output: `import { auth } from '@clerk/nextjs/server'; + output: `import { auth } from "@clerk/nextjs/server"; export default async () => { await auth.protect(); return
Hello
; @@ -132,7 +132,7 @@ export default Page;`, suggestions: [ { messageId: 'addAuthProtect', - output: `import { auth } from '@clerk/nextjs/server'; + output: `import { auth } from "@clerk/nextjs/server"; async function Page() { await auth.protect(); return
Hello
; @@ -159,7 +159,7 @@ export const POST = () => new Response('ok');`, suggestions: [ { messageId: 'addAuthProtect', - output: `import { auth } from '@clerk/nextjs/server'; + output: `import { auth } from "@clerk/nextjs/server"; export async function GET() { await auth.protect(); return new Response('ok'); @@ -174,7 +174,7 @@ export const POST = () => new Response('ok');`, suggestions: [ { messageId: 'addAuthProtect', - output: `import { auth } from '@clerk/nextjs/server'; + output: `import { auth } from "@clerk/nextjs/server"; export async function GET() { return new Response('ok'); } @@ -204,7 +204,7 @@ export async function loadData() { { messageId: 'addAuthProtect', output: `'use server'; -import { auth } from '@clerk/nextjs/server'; +import { auth } from "@clerk/nextjs/server"; export async function loadData() { await auth.protect(); @@ -229,7 +229,7 @@ export async function loadData() { suggestions: [ { messageId: 'addAuthProtect', - output: `import { auth } from '@clerk/nextjs/server'; + output: `import { auth } from "@clerk/nextjs/server"; export async function action() { 'use server'; await auth.protect(); @@ -254,7 +254,7 @@ export async function action() { suggestions: [ { messageId: 'addAuthProtect', - output: `import { auth } from '@clerk/nextjs/server'; + output: `import { auth } from "@clerk/nextjs/server"; const action = async function () { 'use server'; await auth.protect(); @@ -282,7 +282,7 @@ const action = async function () { suggestions: [ { messageId: 'addAuthProtect', - output: `import { auth } from '@clerk/nextjs/server'; + output: `import { auth } from "@clerk/nextjs/server"; export function getAction() { const create = async () => { 'use server'; @@ -339,6 +339,59 @@ export default function Page() { messageId: 'addAuthProtect', output: `import { currentUser, auth } from '@clerk/nextjs/server'; +export default async function Page() { + await auth.protect(); + return null; +}`, + }, + ], + }, + ], + }, + { + name: 'double-quoted clerk import without auth: merges the specifier without changing quote style', + code: `import { currentUser } from "@clerk/nextjs/server"; + +export default function Page() { + return null; +}`, + filename: abs('app/dashboard/page.tsx'), + options: [config], + errors: [ + { + messageId: 'missingProtect', + suggestions: [ + { + messageId: 'addAuthProtect', + output: `import { currentUser, auth } from "@clerk/nextjs/server"; + +export default async function Page() { + await auth.protect(); + return null; +}`, + }, + ], + }, + ], + }, + { + name: 'double-quoted import in file: new auth import matches file quote style', + code: `import { redirect } from "next/navigation"; + +export default function Page() { + return null; +}`, + filename: abs('app/dashboard/page.tsx'), + options: [config], + errors: [ + { + messageId: 'missingProtect', + suggestions: [ + { + messageId: 'addAuthProtect', + output: `import { auth } from "@clerk/nextjs/server"; +import { redirect } from "next/navigation"; + export default async function Page() { await auth.protect(); return null; @@ -378,7 +431,7 @@ export default async function Page() { }, { name: 'existing await auth() destructure: merges .protect() into the call', - code: `import { auth } from '@clerk/nextjs/server'; + code: `import { auth } from "@clerk/nextjs/server"; export default async function Page() { const { userId } = await auth(); @@ -392,7 +445,7 @@ export default async function Page() { suggestions: [ { messageId: 'addAuthProtect', - output: `import { auth } from '@clerk/nextjs/server'; + output: `import { auth } from "@clerk/nextjs/server"; export default async function Page() { const { userId } = await auth.protect(); @@ -405,7 +458,7 @@ export default async function Page() { }, { name: 'existing bare await auth(): merges .protect() into the call', - code: `import { auth } from '@clerk/nextjs/server'; + code: `import { auth } from "@clerk/nextjs/server"; export async function GET() { await auth(); @@ -419,7 +472,7 @@ export async function GET() { suggestions: [ { messageId: 'addAuthProtect', - output: `import { auth } from '@clerk/nextjs/server'; + output: `import { auth } from "@clerk/nextjs/server"; export async function GET() { await auth.protect(); @@ -432,7 +485,7 @@ export async function GET() { }, { name: 'concise arrow awaiting auth(): merges .protect() into the call', - code: `import { auth } from '@clerk/nextjs/server'; + code: `import { auth } from "@clerk/nextjs/server"; export const POST = async () => await auth();`, filename: abs('app/api/widgets/route.ts'), @@ -443,7 +496,7 @@ export const POST = async () => await auth();`, suggestions: [ { messageId: 'addAuthProtect', - output: `import { auth } from '@clerk/nextjs/server'; + output: `import { auth } from "@clerk/nextjs/server"; export const POST = async () => await auth.protect();`, }, @@ -472,6 +525,96 @@ export default async function Page() { export default async function Page() { const { userId } = await clerkAuth.protect(); return
{userId}
; +}`, + }, + ], + }, + ], + }, + { + name: 'page: sync default export with explicit return type — wraps in Promise', + code: `export default function Page(): JSX.Element { + return
Hello
; +}`, + filename: abs('app/dashboard/page.tsx'), + options: [config], + errors: [ + { + messageId: 'missingProtect', + suggestions: [ + { + messageId: 'addAuthProtect', + output: `import { auth } from "@clerk/nextjs/server"; +export default async function Page(): Promise { + await auth.protect(); + return
Hello
; +}`, + }, + ], + }, + ], + }, + { + name: 'route: sync arrow with explicit return type — wraps in Promise', + code: `export const GET = (): Response => new Response('ok');`, + filename: abs('app/api/widgets/route.ts'), + options: [config], + errors: [ + { + messageId: 'missingProtect', + suggestions: [ + { + messageId: 'addAuthProtect', + output: `import { auth } from "@clerk/nextjs/server"; +export const GET = async (): Promise => { + await auth.protect(); + return new Response('ok'); +};`, + }, + ], + }, + ], + }, + { + name: 'route: sync function with Promise return type — does not double-wrap', + code: `export function GET(): Promise { + return Promise.resolve(new Response('ok')); +}`, + filename: abs('app/api/widgets/route.ts'), + options: [config], + errors: [ + { + messageId: 'missingProtect', + suggestions: [ + { + messageId: 'addAuthProtect', + output: `import { auth } from "@clerk/nextjs/server"; +export async function GET(): Promise { + await auth.protect(); + return Promise.resolve(new Response('ok')); +}`, + }, + ], + }, + ], + }, + { + name: 'route: type-predicate return type — adds async but does not wrap return type', + code: `export function GET(value: unknown): value is boolean { + return typeof value === 'boolean'; +}`, + filename: abs('app/api/widgets/route.ts'), + options: [config], + errors: [ + { + messageId: 'missingProtect', + suggestions: [ + { + messageId: 'addAuthProtect', + output: `import { auth } from "@clerk/nextjs/server"; +export async function GET(value: unknown): value is boolean { + await auth.protect(); + return typeof value === 'boolean'; }`, }, ], diff --git a/packages/eslint-plugin/src/next/__tests__/require-auth-protection.test.ts b/packages/eslint-plugin/src/next/__tests__/require-auth-protection.test.ts index 8ef3be9c1b9..6ba3617338a 100644 --- a/packages/eslint-plugin/src/next/__tests__/require-auth-protection.test.ts +++ b/packages/eslint-plugin/src/next/__tests__/require-auth-protection.test.ts @@ -359,6 +359,59 @@ ruleTester.run('require-auth-protection', rule, { filename: abs('app/dashboard/page.tsx'), options: [config], }, + { + name: 'manual check: !isAuthenticated || otherCondition with return', + code: ` + import { auth } from '@clerk/nextjs/server'; + export default async function Page() { + const { isAuthenticated } = await auth(); + if (!isAuthenticated || someFlag) return null; + return
; + } + `, + filename: abs('app/dashboard/page.tsx'), + options: [config], + }, + { + name: 'manual check: otherCondition || !isAuthenticated with return', + code: ` + import { auth } from '@clerk/nextjs/server'; + export default async function Page() { + const { isAuthenticated } = await auth(); + if (someFlag || !isAuthenticated) return null; + return
; + } + `, + filename: abs('app/dashboard/page.tsx'), + options: [config], + }, + { + name: 'manual check: userId === null || otherCondition with redirect', + code: ` + import { auth } from '@clerk/nextjs/server'; + import { redirect } from 'next/navigation'; + export default async function Page() { + const { userId } = await auth(); + if (userId === null || someFlag) redirect('/sign-in'); + return
; + } + `, + filename: abs('app/dashboard/page.tsx'), + options: [config], + }, + { + name: 'manual check: !isAuthenticated || a || b with return', + code: ` + import { auth } from '@clerk/nextjs/server'; + export default async function Page() { + const { isAuthenticated } = await auth(); + if (!isAuthenticated || a || b) return null; + return
; + } + `, + filename: abs('app/dashboard/page.tsx'), + options: [config], + }, { name: 'leading directive is skipped (use cache before protect)', code: ` @@ -1581,6 +1634,34 @@ ruleTester.run('require-auth-protection', rule, { options: [config], errors: [missingProtectError()], }, + { + name: 'manual check: !isAuthenticated && otherCondition is NOT accepted', + code: ` + import { auth } from '@clerk/nextjs/server'; + export default async function Page() { + const { isAuthenticated } = await auth(); + if (!isAuthenticated && someFlag) return null; + return
; + } + `, + filename: abs('app/dashboard/page.tsx'), + options: [config], + errors: [missingProtectError()], + }, + { + name: 'manual check: (!isAuthenticated && x) || y is NOT accepted', + code: ` + import { auth } from '@clerk/nextjs/server'; + export default async function Page() { + const { isAuthenticated } = await auth(); + if ((!isAuthenticated && x) || y) return null; + return
; + } + `, + filename: abs('app/dashboard/page.tsx'), + options: [config], + errors: [missingProtectError()], + }, { name: 'manual check: orgId === null is NOT accepted (orgId can be null while signed in)', code: ` diff --git a/packages/eslint-plugin/src/next/lib/fixers.ts b/packages/eslint-plugin/src/next/lib/fixers.ts index 049fb2663a0..8d48bbba454 100644 --- a/packages/eslint-plugin/src/next/lib/fixers.ts +++ b/packages/eslint-plugin/src/next/lib/fixers.ts @@ -17,6 +17,7 @@ import type { TSESLint, TSESTree } from '@typescript-eslint/utils'; import type { FunctionNode } from './exports'; +import { inferQuoteChar } from './quote-style'; const CLERK_AUTH_SOURCE = '@clerk/nextjs/server'; @@ -62,10 +63,7 @@ export function buildAuthProtectFixes({ fixes.push(importFix); } - const asyncFix = ensureAsyncFix(fixer, sourceCode, fn); - if (asyncFix) { - fixes.push(asyncFix); - } + fixes.push(...ensureAsyncFixes(fixer, sourceCode, fn)); fixes.push(insertProtectCallFix(fixer, sourceCode, fn, authName, authNames)); @@ -120,19 +118,48 @@ function getLineIndent(sourceCode: TSESLint.SourceCode, node: TSESTree.Node): st return match ? match[0] : ''; } -function ensureAsyncFix( +function isPromiseTypeAnnotation(type: TSESTree.TypeNode): boolean { + return type.type === 'TSTypeReference' && type.typeName.type === 'Identifier' && type.typeName.name === 'Promise'; +} + +function wrapReturnTypeInPromiseFix( fixer: TSESLint.RuleFixer, sourceCode: TSESLint.SourceCode, fn: FunctionNode, ): TSESLint.RuleFix | null { - if (fn.async) { + const returnType = fn.returnType; + if (!returnType) { return null; } - const firstToken = sourceCode.getFirstToken(fn); - if (!firstToken) { + const typeAnnotation = returnType.typeAnnotation; + // If return type is a predicate like `value is boolean`, we still add `async`, + // but don't edit the return type. This will produce invalid TS so user can + // go fix it. Should be exceedingly rare. + if (typeAnnotation.type === 'TSTypePredicate' || isPromiseTypeAnnotation(typeAnnotation)) { return null; } - return fixer.insertTextBefore(firstToken, 'async '); + const typeText = sourceCode.getText(typeAnnotation); + return fixer.replaceText(typeAnnotation, `Promise<${typeText}>`); +} + +function ensureAsyncFixes( + fixer: TSESLint.RuleFixer, + sourceCode: TSESLint.SourceCode, + fn: FunctionNode, +): TSESLint.RuleFix[] { + if (fn.async) { + return []; + } + const fixes: TSESLint.RuleFix[] = []; + const firstToken = sourceCode.getFirstToken(fn); + if (firstToken) { + fixes.push(fixer.insertTextBefore(firstToken, 'async ')); + } + const returnTypeFix = wrapReturnTypeInPromiseFix(fixer, sourceCode, fn); + if (returnTypeFix) { + fixes.push(returnTypeFix); + } + return fixes; } function insertProtectCallFix( @@ -249,7 +276,8 @@ function ensureAuthImportFix( continue; } - const importText = `import { auth } from '${CLERK_AUTH_SOURCE}';`; + const quote = inferQuoteChar(sourceCode, program); + const importText = `import { auth } from ${quote}${CLERK_AUTH_SOURCE}${quote};`; const stmts = program.body; // Insert after a leading directive prologue (module-level `'use server'` / diff --git a/packages/eslint-plugin/src/next/lib/protection-checks.ts b/packages/eslint-plugin/src/next/lib/protection-checks.ts index 75077a1e1d1..05df5aeba7f 100644 --- a/packages/eslint-plugin/src/next/lib/protection-checks.ts +++ b/packages/eslint-plugin/src/next/lib/protection-checks.ts @@ -199,6 +199,19 @@ function isRecognizedAuthCheck(test: TSESTree.Expression, bindings: Set): boolean { + if (isRecognizedAuthCheck(test, bindings)) { + return true; + } + // Only `||` preserves the guarantee + if (test.type === 'LogicalExpression' && test.operator === '||') { + return guardFiresWhenUnauthenticated(test.left, bindings) || guardFiresWhenUnauthenticated(test.right, bindings); + } + return false; +} + function isExitCall(expr: TSESTree.Expression | null | undefined): boolean { if (!expr || expr.type !== 'CallExpression') { return false; @@ -247,7 +260,7 @@ function isAuthGuardWithExit(stmt: TSESTree.Statement, bindings: Set) if (stmt.type !== 'IfStatement') { return false; } - if (!isRecognizedAuthCheck(stmt.test, bindings)) { + if (!guardFiresWhenUnauthenticated(stmt.test, bindings)) { return false; } return consequentExits(stmt.consequent); diff --git a/packages/eslint-plugin/src/next/lib/quote-style.ts b/packages/eslint-plugin/src/next/lib/quote-style.ts new file mode 100644 index 00000000000..68f5e864d73 --- /dev/null +++ b/packages/eslint-plugin/src/next/lib/quote-style.ts @@ -0,0 +1,32 @@ +import type { TSESLint, TSESTree } from '@typescript-eslint/utils'; + +export type QuoteChar = "'" | '"'; + +function quoteFromModuleSource(sourceCode: TSESLint.SourceCode, source: TSESTree.StringLiteral): QuoteChar | null { + const text = sourceCode.getText(source); + const first = text[0]; + if (first === "'" || first === '"') { + return first; + } + return null; +} + +/** + * Infer the string quote style already used in `program`, so inserted import + * lines match the file. Only checks for imports and "export from" for simplicity, + * as those are almost always present. Falls back to double quotes (Prettier and + * ESLint defaults) when there is nothing to infer from. + */ +export function inferQuoteChar(sourceCode: TSESLint.SourceCode, program: TSESTree.Program): QuoteChar { + for (const stmt of program.body) { + // Only imports and "export from" has stmt.source + if ('source' in stmt && stmt.source) { + const quote = quoteFromModuleSource(sourceCode, stmt.source); + if (quote) { + return quote; + } + } + } + + return '"'; +}