Add ESLint rule to validate translator key destructuring #13932

rtibbles · 2025-11-22T00:40:07Z

Summary

Implements no-undefined-translator-keys rule that prevents destructuring undefined keys from createTranslator objects. Validates both same-file and cross-file imports using AST parsing and Node.js module resolution.

References

No open issue, but observed as a problem during recent string updates for release

Reviewer guidance

🤖 This was created by Claude Code. @rtibbles then reviewed the generated output, and did iterative rounds of updates before making it ready for review 🤖

rtibbles · 2025-11-22T00:40:27Z

@coderabbitai full review

coderabbitai · 2025-11-22T00:40:33Z

✅ Actions performed

Full review triggered.

coderabbitai · 2025-11-22T00:44:23Z

Walkthrough

Introduces a new ESLint rule no-undefined-translator-keys that validates translator object destructuring. The implementation includes AST parsing for translator discovery, import resolution, and destructuring validation to ensure all accessed keys exist and use trailing '$' suffixes. Test fixtures demonstrate various export patterns and a comprehensive test suite covers valid and invalid scenarios.

Changes

Cohort / File(s)	Change Summary
Rule Implementation `packages/eslint-plugin-kolibri/lib/rules/no-undefined-translator-keys.js`	New ESLint rule with parser utilities, translator discovery via AST walking, import resolution (relative and external), destructuring validation, error reporting with suggested fixes, and ESLint rule definition with meta, messages, and create handler.
Test Fixtures `packages/eslint-plugin-kolibri/tests/lib/rules/fixtures/defaultExport.js`, `malformed.js`, `reExport.js`, `testStrings.js`	Four new test fixture files demonstrating translator patterns: default export, re-export, malformed/invalid syntax, and multiple translator instances with various message formats (primitive strings and object-format with metadata).
Test Suite `packages/eslint-plugin-kolibri/tests/lib/rules/no-undefined-translator-keys.spec.js`	Comprehensive ESLint RuleTester test suite with valid cases covering destructuring patterns, cross-file imports, and object-format messages; invalid cases asserting errors for undefined keys, missing '$' suffix, and incorrect key usage.
Configuration `packages/kolibri-format/.eslintrc.js`	New ESLint rule entry: `kolibri/no-undefined-translator-keys` set to ERROR in module.exports.rules.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Rule implementation complexity: The main rule file contains dense logic including AST parsing with caching, recursive translator discovery, multi-stage import resolution (relative imports, named/default imports, re-exports), and stateful validation across multiple AST event handlers.
Key validation logic: The destructuring validation logic that strips '$' suffix and compares against collected keys requires careful scrutiny for edge cases (aliasing, nested destructuring, cross-translator scenarios).
Import resolution: The logic for resolving relative imports and chained imports needs verification to prevent false positives/negatives, especially for re-export patterns.
Test coverage breadth: While test cases follow repetitive patterns, the sheer number of valid and invalid scenarios warrants careful verification that all edge cases are properly handled by the implementation.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title directly summarizes the main change: adding an ESLint rule to validate translator key destructuring, which aligns with the primary implementation of the no-undefined-translator-keys rule.
Description check	✅ Passed	The description is directly related to the changeset, explaining the rule's purpose, validation approach, and context for the implementation.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch claude/translator-destructuring-support-01LaLgvzVYK5N2GWsDHXyzPt

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

Provide your own instructions using the high_level_summary_instructions setting.
Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

📝 Description — Summarize the main change in 50–60 words, explaining what was done.

📓 References — List relevant issues, discussions, documentation, or related PRs.

📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.

📊 Contributor Summary — Include a Markdown table showing contributions:
| Contributor | Lines Added | Lines Removed | Files Changed |

✔️ Additional Notes — Add any extra reviewer context.
Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.

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

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

packages/eslint-plugin-kolibri/lib/rules/no-undefined-translator-keys.js (1)

11-13: Consider cache invalidation strategy.

The global cache improves performance but has no invalidation mechanism. In long-running processes (like IDE language servers), this could serve stale data if source files change, or grow unbounded over time. For typical CLI usage this is fine, but consider adding cache clearing on file changes or a size limit for language server scenarios.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 66a435a and f35c2db.

📒 Files selected for processing (7)

packages/eslint-plugin-kolibri/lib/rules/no-undefined-translator-keys.js (1 hunks)
packages/eslint-plugin-kolibri/tests/lib/rules/fixtures/defaultExport.js (1 hunks)
packages/eslint-plugin-kolibri/tests/lib/rules/fixtures/malformed.js (1 hunks)
packages/eslint-plugin-kolibri/tests/lib/rules/fixtures/reExport.js (1 hunks)
packages/eslint-plugin-kolibri/tests/lib/rules/fixtures/testStrings.js (1 hunks)
packages/eslint-plugin-kolibri/tests/lib/rules/no-undefined-translator-keys.spec.js (1 hunks)
packages/kolibri-format/.eslintrc.js (1 hunks)

🧰 Additional context used

🪛 Biome (2.1.2)

packages/eslint-plugin-kolibri/tests/lib/rules/fixtures/malformed.js

[error] 6-6: Expected a semicolon or an implicit semicolon after a statement, but found none