feat: add no-redirect-to-route-group and require-auth-initiate-call rules#62
feat: add no-redirect-to-route-group and require-auth-initiate-call rules#62danielchen0 wants to merge 5 commits intomainfrom
Conversation
|
Auto-fix: ran — danielchen0-pr-monitor |
| JSXOpeningElement(path) { | ||
| const { name, attributes } = path.node; | ||
|
|
||
| if (name.type !== 'JSXIdentifier' || name.name !== 'Redirect') { |
There was a problem hiding this comment.
Same bug class via imperative API isn't caught (verified):
router.replace('/(tabs)'); router.push('/(tabs)'); router.navigate('/(tabs)');Add a CallExpression visitor for <id>.replace/.push/.navigate with a string-literal first arg, reusing hrefIsOnlyRouteGroups. If out of scope here, document the gap in the rule's JSDoc.
| IfStatement(path) { | ||
| const { test, consequent } = path.node; | ||
| const matchedName = matchesNegatedIdentifier( | ||
| test, | ||
| useAuthDestructures.map((d) => d.isReadyName).filter((n): n is string => n !== null), | ||
| ); | ||
| if (matchedName === null) return; | ||
| if (containsReturn(consequent)) { | ||
| usedAsRenderGate = true; | ||
| } | ||
| }, | ||
| }); | ||
|
|
||
| if (!usedAsRenderGate) { | ||
| return results; |
There was a problem hiding this comment.
Render-gate detection misses two common forms (verified []):
return isReady ? <Stack /> : null; // ternary
if (isReady) return <Stack />; return null; // positive early-returnSame bug from a codegen perspective. Extend usedAsRenderGate to recognize:
ReturnStatementwhose argument is aConditionalExpressiontesting the destructured name;IfStatementwith a positive identifiertest(no!) whose consequent returns.
| return results; | ||
| } | ||
|
|
||
| function matchesNegatedIdentifier(node: unknown, names: string[]): string | null { |
There was a problem hiding this comment.
unknown + ad-hoc casts in matchesNegatedIdentifier/containsReturn (and extractStringLiteral in the sibling rule) fight CLAUDE.md's "use Babel's type system directly". Use t.isUnaryExpression/t.isIdentifier/t.isReturnStatement/t.isBlockStatement to narrow.
|
@danielchen0 dx5v just approved this PR (3:36Z) with 3 non-blocking inline review comments worth a look before merging:
All three are substantive logic/typing changes I won't apply autonomously. Also reminder: branch is still dirty (merge conflict, rebaseable=false) since 01:21Z May 1 — needs your rebase before merge. — danielchen0-pr-monitor |
|
Auto-fix: Lint & Format CI was failing on this rebase because README.md needed prettier formatting. Pushed prettier --write README.md as a follow-up commit. CI should go green on the next run. |
…ules Rebased onto main to resolve conflicts after #51 merge.
f560084 to
ee04f11
Compare
Auto-resolved trivial conflicts in shared files. Resolved by Lainter (steward).
|
Auto-fix: lainterr's main-merge bumped this branch's head and the README rule-table alignment drifted (prettier flagged it on the new head). Pushed dx5v's three inline review comments on this PR (router.replace/push/navigate variants, ternary + positive-if render gates, and Babel-typed narrowing) are still open — flagging since you also asked me to address review comments on #57. Want me to apply the same pattern here? They're real false-negative gaps but the fixes are larger than a one-line auto-fix, so I'd rather get a thumbs-up before touching the rule logic on this one. |
Auto-resolved trivial conflicts in shared files. Resolved by Lainter (steward).
|
Auto-fix: lainterr's 18:30 main-merge (head 2a9e06d) again drifted the General Rules table column alignment in README.md and left a stray blank line above laint/55 README came back clean from this main-merge — no fix needed there. dx5v's three inline review comments on this PR are still unaddressed; let me know if you want me to take them on or if you'd rather handle them yourself. |
Summary
Two new
expo-tagged rules driven by recurring V2 mobile preview regressions where the agent generated code that compiled clean but rendered a blank screen. Both bugs were traced live in escher#fix_mobile_v2_previews before this PR.no-redirect-to-route-group(error)In Expo Router, segments wrapped in parens like
(tabs)are route groups — they're stripped from URL resolution. So<Redirect href="/(tabs)" />targets a path that doesn't resolve to any concrete route:app/(tabs)/index.tsxmaps to/, not/(tabs). When bothapp/index.tsx(with the redirect) andapp/(tabs)/index.tsxexist, you get a route conflict or a silent no-op redirect, and the screen renders blank.The bug is encouraged by V2's own system prompt at
stacks/v2/prompts/system.ts:460-472which literally shows<Redirect href="/(tabs)" />as the canonical example. The companion fix is to either correct or delete that example, but until then this lint rule fails the commit so the agent self-corrects.Flag any href whose segments are all route-group segments with no concrete path beyond them.
/(tabs)/exploreis fine (strips to/explore);/(tabs)alone is the bug.require-auth-initiate-call(error)The shipped V2 mobile
useAuth()exposesisReady(gates render until the persisted JWT loads from SecureStore) andinitiate()(the function that does the load). The gate doesn't flip on its own. If a layout gates render onisReadybut never callsinitiate(), the gate stays closed forever and the app rendersnull/blank — the exact regression we hit in production project groupc607f7e2-c087-4c3f-8015-5a26b522da58, where rev 5 of_layout.tsxstrippeduseAuth().initiate()while keeping theisReadygate.Fires when:
useAuth()is destructured to pullisReady(or aliased:{ isReady: authReady })if (!isReady) return ...)initiateis neither destructured-and-invoked nor called via member access on an auth returnIncludes a "non-gate" exception: a file that uses
isReadyonly inside JSX (e.g.{isReady ? <Profile /> : null}) doesn't fire — that's a sibling of the layout that's allowed to consume the global auth state without bootstrapping it.Tests
no-redirect-to-route-group: literal +JSXExpressionContainerhref forms, leading/trailing slashes, multiple group segments, dynamic hrefs, non-Redirect components.require-auth-initiate-call: happy path, aliasedisReady, member-accessinitiate, non-gateisReadyreads (negative case), and the literal agent-stripped layout that triggered this PR.Risks
require-auth-initiate-calllooks at anyuseAuth()symbol. If a different module also exportsuseAuthwith different semantics, the rule still fires. In practice the V2 stack only has the one shippeduseAuth, and the rule has a strict "must be used as a render gate" filter that should keep false positives close to zero. If it surfaces noise, narrowing touseAuthfrom a specific import path is a one-line follow-up.no-redirect-to-route-grouponly checks string-literalhrefs. A redirect built from a template literal or variable bypasses the rule — but those are rare in agent-generated code, and adding template-literal support is straightforward if needed.npm testpasses 422/422.Test plan
npm test— 422/422npm run lint— 0 errors (20 pre-existing fs warnings unchanged)npm run build— greennpx prettier --check .— cleannpx knip— cleanlaintin escher and confirm the agent receives lint feedback for the two patterns end-to-end