diff --git a/eslint-factory/README.md b/eslint-factory/README.md index 64413851c51..a734ae0913b 100644 --- a/eslint-factory/README.md +++ b/eslint-factory/README.md @@ -32,6 +32,7 @@ Using an interpolated route bypasses Octokit's typed route dispatch, can silentl **Flagged forms:** - `` github.request(`GET /repos/${owner}/${repo}`, ...) `` — template literal with interpolations. - `github.request("GET /repos/" + owner + "/" + repo, ...)` — string concatenation. +- `` github.request(`POST ${endpoint}`, ...) `` — opaque whole-route helper; thread a typed route from the caller instead of interpolating the entire path. - `` context.github.request(`GET /repos/${owner}/${repo}`, ...) `` — `context.github` client. - `` const gh = github; gh.request(`GET /repos/${owner}/${repo}`, ...) `` — aliased client. - `` const client = getOctokit(token); client.request(`GET /repos/${owner}/${repo}`, ...) `` — `getOctokit` result alias. @@ -47,6 +48,8 @@ Using an interpolated route bypasses Octokit's typed route dispatch, can silentl github.request("GET /repos/{owner}/{repo}", { owner, repo }); ``` +For helpers that receive the entire route as a parameter, there is no mechanical `{owner}` / `{repo}` rewrite. Pass a typed route string from the caller instead of interpolating `POST ${endpoint}` or `"POST " + endpoint` at the helper call site. + ### `no-json-stringify-error` Disallow `JSON.stringify()` on caught error variables. `Error` properties (`message`, `stack`, etc.) are non-enumerable, so `JSON.stringify(err)` silently produces `{}`. diff --git a/eslint-factory/src/rules/no-github-request-interpolated-route.test.ts b/eslint-factory/src/rules/no-github-request-interpolated-route.test.ts index 52a5b35b59e..8cc10df7f1b 100644 --- a/eslint-factory/src/rules/no-github-request-interpolated-route.test.ts +++ b/eslint-factory/src/rules/no-github-request-interpolated-route.test.ts @@ -169,6 +169,32 @@ describe("no-github-request-interpolated-route", () => { }); }); + it("invalid: opaque whole-route helpers are flagged with tailored guidance", () => { + cjsRuleTester.run("no-github-request-interpolated-route", noGithubRequestInterpolatedRouteRule, { + valid: [], + invalid: [ + { + code: "function addReaction(endpoint) { github.request(`POST ${endpoint}`, { content: '+1' }); }", + errors: [ + { + messageId: "opaqueWholeRoute", + data: { kind: "template literal with interpolations", client: "github" }, + }, + ], + }, + { + code: 'function addComment(endpoint) { github.request("POST " + endpoint, { body }); }', + errors: [ + { + messageId: "opaqueWholeRoute", + data: { kind: "string concatenation expression", client: "github" }, + }, + ], + }, + ], + }); + }); + it("valid: simple aliases bound to non-Octokit sources are not flagged", () => { cjsRuleTester.run("no-github-request-interpolated-route", noGithubRequestInterpolatedRouteRule, { valid: [ diff --git a/eslint-factory/src/rules/no-github-request-interpolated-route.ts b/eslint-factory/src/rules/no-github-request-interpolated-route.ts index 00241923df4..25306410a83 100644 --- a/eslint-factory/src/rules/no-github-request-interpolated-route.ts +++ b/eslint-factory/src/rules/no-github-request-interpolated-route.ts @@ -4,6 +4,7 @@ const createRule = ESLintUtils.RuleCreator(name => `https://github.com/github/gh const OCTOKIT_CLIENT_NAMES = new Set(["github", "octokit", "githubClient", "octokitClient"]); const GET_OCTOKIT_MEMBER_OBJECT_NAMES = new Set(["github", "actions"]); +const HTTP_METHOD_PREFIXES = new Set(["GET ", "POST ", "PUT ", "PATCH ", "DELETE ", "HEAD ", "OPTIONS "]); /** * Returns true when the node is a template literal that contains at least one @@ -39,6 +40,54 @@ function isStringConcatenation(node: TSESTree.Node): boolean { return node.type === "BinaryExpression" && node.operator === "+" && !isStaticRouteExpression(node); } +/** + * Returns true when `text` is an exact HTTP-method route prefix including the + * required trailing space, such as `GET ` or `POST `. + */ +function isValidHttpMethodPrefix(text: string | null | undefined): boolean { + return typeof text === "string" && HTTP_METHOD_PREFIXES.has(text); +} + +/** + * Returns the human-readable route-expression kind string used in diagnostics, + * or null when `node` is not one of the interpolated route shapes this rule + * reports on. + */ +function getInterpolatedRouteKind(node: TSESTree.Node): string | null { + if (isInterpolatedTemplateLiteral(node)) return "template literal with interpolations"; + if (isStringConcatenation(node)) return "string concatenation expression"; + return null; +} + +/** + * Returns true when the route expression is exactly an HTTP method plus a + * single opaque route expression, such as: + * - `POST ${endpoint}` + * - `"POST " + endpoint` + * + * This shape differs from value-into-path interpolation because there is no + * known static path at the call site to rewrite with `{placeholder}` segments. + */ +function isOpaqueWholeRouteInterpolation(node: TSESTree.Node): boolean { + if (node.type === "TemplateLiteral") { + const hasTwoQuasis = node.quasis.length === 2; + const hasSingleExpression = node.expressions.length === 1; + const hasMethodPrefix = hasTwoQuasis && isValidHttpMethodPrefix(node.quasis[0].value.cooked); + const hasEmptyTrailingQuasi = hasTwoQuasis && node.quasis[1].value.cooked === ""; + const isDynamicWholeRoute = hasSingleExpression && !isStaticRouteExpression(node.expressions[0]); + return hasMethodPrefix && hasEmptyTrailingQuasi && isDynamicWholeRoute; + } + + if (node.type === "BinaryExpression" && node.operator === "+") { + if (node.left.type !== "Literal" || typeof node.left.value !== "string") return false; + const hasMethodPrefix = isValidHttpMethodPrefix(node.left.value); + const isRightDynamic = !isStaticRouteExpression(node.right); + return hasMethodPrefix && isRightDynamic; + } + + return false; +} + /** * Returns true when `node` is the `context.github` member expression. */ @@ -88,10 +137,13 @@ export const noGithubRequestInterpolatedRouteRule = createRule({ "Disallow template literals with interpolations or string concatenation as the route argument of Octokit.request() calls. " + "Octokit clients are detected by well-known names (github, octokit, githubClient, octokitClient), " + "identifiers initialized from getOctokit(...) call results, context.github, and simple const aliases of any of these. " + - 'Use the typed placeholder form instead: "GET /repos/{owner}/{repo}" with a separate params object.', + "Use the typed placeholder form for value interpolation, or thread a typed route string from the caller when the entire route is dynamic.", }, schema: [], messages: { + opaqueWholeRoute: + "Avoid using an opaque whole-route {{kind}} as the route argument of {{client}}.request(). " + + "When the entire route is dynamic, pass a typed route string from the caller instead of interpolating or concatenating a route variable.", interpolatedRoute: "Avoid using a {{kind}} as the route argument of {{client}}.request(). " + 'Use the typed placeholder form instead — e.g. github.request("GET /repos/{owner}/{repo}", { owner, repo }) — ' + @@ -169,22 +221,23 @@ export const noGithubRequestInterpolatedRouteRule = createRule({ const firstArg = node.arguments[0]; if (!firstArg) return; - if (isInterpolatedTemplateLiteral(firstArg)) { + const routeKind = getInterpolatedRouteKind(firstArg); + if (!routeKind) return; + + if (isOpaqueWholeRouteInterpolation(firstArg)) { context.report({ node: firstArg, - messageId: "interpolatedRoute", - data: { kind: "template literal with interpolations", client: clientName }, + messageId: "opaqueWholeRoute", + data: { kind: routeKind, client: clientName }, }); return; } - if (isStringConcatenation(firstArg)) { - context.report({ - node: firstArg, - messageId: "interpolatedRoute", - data: { kind: "string concatenation expression", client: clientName }, - }); - } + context.report({ + node: firstArg, + messageId: "interpolatedRoute", + data: { kind: routeKind, client: clientName }, + }); }, }; },