Skip to content

Document and pin no-github-request-interpolated-route boundary behavior#44550

Merged
pelikhan merged 2 commits into
mainfrom
copilot/add-tests-docs-for-no-github-request-rule
Jul 9, 2026
Merged

Document and pin no-github-request-interpolated-route boundary behavior#44550
pelikhan merged 2 commits into
mainfrom
copilot/add-tests-docs-for-no-github-request-rule

Conversation

Copilot AI commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

no-github-request-interpolated-route had several intentional syntactic limits that were neither tested nor documented, making it hard to distinguish deliberate scope from missing coverage. This change pins those boundaries in tests, documents them in the eslint-factory README, and narrows one false positive in constant-only route concatenation.

  • Rule behavior

    • Stop flagging compile-time constant + route expressions such as "GET /repos" + "/{owner}/{repo}".
    • Keep flagging non-constant + concatenation with dynamic values.
  • Boundary coverage

    • Add valid tests for currently out-of-scope forms:
      • member-object clients: this.github.request(...), context.github.request(...)
      • variable indirection: github.request(route, ...)
      • .concat() route building
      • constant-only + concatenation
  • README coverage

    • Add an explicit Out of scope note for the route rule covering member-object clients, variable routes, and .concat().
    • Expand the eslint-factory README to document all registered rules, not just the previously documented subset.
  • Example

    // still flagged
    github.request("GET /repos/" + owner + "/" + repo, { owner, repo });
    
    // now accepted
    github.request("GET /repos" + "/{owner}/{repo}", { owner, repo });

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Hey @copilot-swe-agent 👋 — thanks for picking up issue #44516 and working on the boundary-case tests and documentation for no-github-request-interpolated-route! The scope is well-defined and the plan embedded in the PR body covers all four boundary cases cleanly.

This PR is currently a draft with no file changes yet, so a few things to land before it's ready for review:

  • Add tests — the four valid/invalid test cases described in the issue need to be written:
    1. Member-object client (this.github.request(...)) → valid
    2. Variable-route indirection (github.request(route)) → valid
    3. .concat() string building → valid (or extend isStringConcatenation)
    4. Constant-only concatenation ("GET /x" + "/y") → fix the false positive or pin with a test
  • Update README — backfill rule documentation for the remaining entries registered in src/index.ts (or at minimum add an "Out of scope" note for the boundary behaviors).

If you'd like a hand, you can assign this prompt to your coding agent:

In the eslint-factory package, add tests and documentation for the `no-github-request-interpolated-route` rule. Specifically:

1. Add a `valid` test case for member-object clients: `this.github.request(GET /repos/${id})` should NOT be flagged (callee.object is not a plain Identifier).
2. Add a `valid` test case for variable-route indirection: `github.request(route)` should NOT be flagged.
3. Add a `valid` test case for `.concat()` string building: `github.request("GET /repos/".concat(id))` should NOT be flagged (or extend `isStringConcatenation` to cover `CallExpression` `.concat()` and add an `invalid` test).
4. Fix the constant-only concatenation false positive: `github.request("GET /x" + "/y")` (no variables) should NOT be reported. Update `isStringConcatenation` to only flag when at least one operand is non-literal, and add a `valid` test.
5. In the eslint-factory README, add an "Out of scope" section under the `no-github-request-interpolated-route` rule documenting the member-object, variable-route, and `.concat()` limitations.

Generated by ✅ Contribution Check · 138 AIC · ⌖ 20.1 AIC · ⊞ 6.2K ·

Copilot AI changed the title [WIP] Add tests and documentation for no-github-request-interpolated-route Document and pin no-github-request-interpolated-route boundary behavior Jul 9, 2026
Copilot AI requested a review from pelikhan July 9, 2026 14:00
@pelikhan pelikhan marked this pull request as ready for review July 9, 2026 14:05
Copilot AI review requested due to automatic review settings July 9, 2026 14:05
@pelikhan pelikhan merged commit 4ce5526 into main Jul 9, 2026
16 checks passed
@pelikhan pelikhan deleted the copilot/add-tests-docs-for-no-github-request-rule branch July 9, 2026 14:06

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This pull request clarifies and “pins” the intended scope boundaries of the no-github-request-interpolated-route ESLint rule by documenting out-of-scope syntaxes, adding tests to lock in that behavior, and narrowing a false positive so compile-time constant + route expressions are no longer flagged.

Changes:

  • Update the rule to stop flagging compile-time-constant + route concatenations composed only of literals.
  • Add tests that explicitly assert several intentionally out-of-scope route-building forms are accepted.
  • Expand eslint-factory README rule documentation, including an explicit “Out of scope” section for the route rule.
Show a summary per file
File Description
eslint-factory/src/rules/no-github-request-interpolated-route.ts Adds static-expression detection to exclude constant-only + concatenations from being reported.
eslint-factory/src/rules/no-github-request-interpolated-route.test.ts Adds “valid” cases covering explicitly out-of-scope forms and constant-only concatenation acceptance.
eslint-factory/README.md Documents out-of-scope boundaries for the route rule and expands rule documentation coverage.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 3/3 changed files
  • Comments generated: 1
  • Review effort level: Low

Comment on lines +20 to +27
function isStaticRouteExpression(node: TSESTree.Node): boolean {
if (node.type === "Literal") return true;
if (node.type === "TemplateLiteral") return node.expressions.length === 0;
if (node.type === "BinaryExpression" && node.operator === "+") {
return isStaticRouteExpression(node.left) && isStaticRouteExpression(node.right);
}
return false;
}
@github-actions

Copy link
Copy Markdown
Contributor

🎉 This pull request is included in a new release.

Release: v0.82.8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants