-
Notifications
You must be signed in to change notification settings - Fork 0
Eslint Config #24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Eslint Config #24
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughA comprehensive ESLint configuration file is added for TypeScript projects, defining global ignores, parser settings, plugin integrations, and rule sets with file-specific overrides to enforce code quality, functional programming patterns, and import hygiene. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes Areas requiring attention:
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
packages/opencode/src/tool/bash.ts
Outdated
| // Validate and resolve cwd parameter | ||
| let workingDirectory = Instance.directory | ||
| if (params.cwd) { | ||
| const resolvedCwd = await $`realpath ${params.cwd}` | ||
| .quiet() | ||
| .nothrow() | ||
| .text() | ||
| .then((x) => x.trim()) | ||
|
|
||
| if (!resolvedCwd) { | ||
| throw new Error(`Invalid working directory: ${params.cwd}`) | ||
| } | ||
|
|
||
| if (!Filesystem.contains(Instance.directory, resolvedCwd)) { | ||
| throw new Error( | ||
| `Working directory ${resolvedCwd} is outside of project directory ${Instance.directory}`, | ||
| ) | ||
| } | ||
|
|
||
| workingDirectory = resolvedCwd | ||
| } | ||
|
|
||
| const tree = await parser().then((p) => p.parse(params.command)) | ||
| if (!tree) { | ||
| throw new Error("Failed to parse command") | ||
| } | ||
| const permissions = await Agent.get(ctx.agent).then((x) => x.permission.bash) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Restore missing dependencies in Bash tool
The bash tool now calls parser(), $\realpath`, Filesystem.contains, and Agent.get, but the commit commented out the parserinitializer and removed the imports for$, Filesystem, Agent, and Wildcard`. As written these identifiers are undefined, so TypeScript compilation (or runtime execution) will fail before any command runs. Re‑introduce the relevant imports/definition or remove the references.
Useful? React with 👍 / 👎.
f00cc65 to
9ef2ac1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| files: ['**/*.test.{ts,tsx,js}', '**/*.spec.{ts,tsx,js}', '**/tests/**/*.{ts,tsx,js}'], | ||
| plugins: { ava }, | ||
| rules: { | ||
| 'ava/no-only-test': 'error', | ||
| 'ava/no-identical-title': 'error', | ||
| 'ava/test-title': 'warn', | ||
| // your existing no-restricted-syntax for setTimeout | ||
| 'no-restricted-syntax': [ | ||
| 'error', | ||
| { | ||
| selector: "CallExpression[callee.name='setTimeout'][arguments.0.type='Identifier']", | ||
| message: | ||
| 'Use sleep from @promethean/test-utils instead of setTimeout for sleeps in tests.', | ||
| }, | ||
| ], | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Restore base no-restricted-syntax restrictions in test overrides
This override replaces the earlier no-restricted-syntax settings, so test files now permit require, module.exports, and class declarations even though the main config forbids them. Because flat configs merge top-to-bottom and the last matching rule wins, we need to carry over the original selectors when adding the test-specific restriction.(eslint.org)
'no-restricted-syntax': [
'error',
+ {
+ selector: "CallExpression[callee.name='require']",
+ message: 'ESM only',
+ },
+ {
+ selector: "MemberExpression[object.name='module'][property.name='exports']",
+ message: 'ESM only',
+ },
+ {
+ selector: 'ClassDeclaration',
+ message: 'Class declarations are not allowed.',
+ },
+ {
+ selector: 'ClassExpression',
+ message: 'Class expressions are not allowed.',
+ },
{
selector: "CallExpression[callee.name='setTimeout'][arguments.0.type='Identifier']",
message:
'Use sleep from @promethean/test-utils instead of setTimeout for sleeps in tests.',
},
@@
- ],
+ ],📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| files: ['**/*.test.{ts,tsx,js}', '**/*.spec.{ts,tsx,js}', '**/tests/**/*.{ts,tsx,js}'], | |
| plugins: { ava }, | |
| rules: { | |
| 'ava/no-only-test': 'error', | |
| 'ava/no-identical-title': 'error', | |
| 'ava/test-title': 'warn', | |
| // your existing no-restricted-syntax for setTimeout | |
| 'no-restricted-syntax': [ | |
| 'error', | |
| { | |
| selector: "CallExpression[callee.name='setTimeout'][arguments.0.type='Identifier']", | |
| message: | |
| 'Use sleep from @promethean/test-utils instead of setTimeout for sleeps in tests.', | |
| }, | |
| ], | |
| }, | |
| files: ['**/*.test.{ts,tsx,js}', '**/*.spec.{ts,tsx,js}', '**/tests/**/*.{ts,tsx,js}'], | |
| plugins: { ava }, | |
| rules: { | |
| 'ava/no-only-test': 'error', | |
| 'ava/no-identical-title': 'error', | |
| 'ava/test-title': 'warn', | |
| // your existing no-restricted-syntax for setTimeout | |
| 'no-restricted-syntax': [ | |
| 'error', | |
| { | |
| selector: "CallExpression[callee.name='require']", | |
| message: 'ESM only', | |
| }, | |
| { | |
| selector: "MemberExpression[object.name='module'][property.name='exports']", | |
| message: 'ESM only', | |
| }, | |
| { | |
| selector: 'ClassDeclaration', | |
| message: 'Class declarations are not allowed.', | |
| }, | |
| { | |
| selector: 'ClassExpression', | |
| message: 'Class expressions are not allowed.', | |
| }, | |
| { | |
| selector: "CallExpression[callee.name='setTimeout'][arguments.0.type='Identifier']", | |
| message: | |
| 'Use sleep from @promethean/test-utils instead of setTimeout for sleeps in tests.', | |
| }, | |
| ], | |
| }, |
🤖 Prompt for AI Agents
In eslint.config.mjs around lines 149 to 164, the test override replaces the
base no-restricted-syntax rule so tests now unintentionally allow constructs
(require, module.exports, class declarations) that the main config forbids;
restore the original selectors by merging them into this override: include the
base selectors for require, module.exports and class declarations alongside the
test-specific setTimeout selector so the override explicitly re‑applies those
restrictions, keeping the test-only setTimeout rule plus the original selectors
and messages so the last matching rule still enforces the base restrictions in
test files.
This PR contains changes for Eslint Config.
Summary by CodeRabbit