fix: disallow extra properties in rule options #410
Conversation
|
WalkthroughThe changes update the JSON schema definitions for rule options in several files to explicitly disallow additional, undeclared properties. In one file, a shared schema object is introduced for reuse and clarity, and a type definition is slightly broadened. No runtime logic or control flow is altered. Changes
Suggested labels
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
src/rules/dynamic-import-chunkname.tsOops! Something went wrong! :( ESLint: 9.31.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/lib/index.js' imported from /eslint.config.js src/rules/no-namespace.tsOops! Something went wrong! :( ESLint: 9.31.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/lib/index.js' imported from /eslint.config.js src/rules/extensions.tsOops! Something went wrong! :( ESLint: 9.31.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/lib/index.js' imported from /eslint.config.js
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🧰 Additional context used🧠 Learnings (4)📓 Common learningssrc/rules/dynamic-import-chunkname.ts (6)src/rules/no-namespace.ts (2)src/rules/no-unused-modules.ts (12)⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (7)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to 51af82f in 52 seconds. Click for details.
- Reviewed
169lines of code in4files - Skipped
0files when reviewing. - Skipped posting
5draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src/rules/dynamic-import-chunkname.ts:51
- Draft comment:
✅ AddedadditionalProperties: falsein the options schema to prevent unnoticed typos. Confirm that no valid extra options are required. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. src/rules/extensions.ts:61
- Draft comment:
✅ AddingadditionalProperties: falseensures that only defined schema keys are allowed. This increases the rule’s strictness. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. src/rules/no-namespace.ts:35
- Draft comment:
✅additionalProperties: falseis set for the options, ensuring that typos or unintended properties are caught. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
4. src/rules/no-unused-modules.ts:445
- Draft comment:
⚠️ Changed themissingExportsoption type from literaltruetoboolean. Verify that existing configurations are updated accordingly. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =20%<= threshold50%The comment is asking the author to verify that existing configurations are updated, which is against the rules. It does not provide a specific suggestion or ask for a specific test to be written. It is more of a general cautionary note.
5. src/rules/no-unused-modules.ts:498
- Draft comment:
✅ Reusing a shared schema (SHARED_OPTIONS_SCHEMA_PROPERTIES) and addingadditionalProperties: falsein each anyOf branch helps enforce strict option validation. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_IObtdHUzkcHY1Yrx
You can customize 
| @@ -444,13 +444,47 @@ const fileIsInPkg = (file: string) => { | |||
| export interface Options { | |||
| src?: string[] | |||
| ignoreExports?: string[] | |||
| missingExports?: true | |||
| missingExports?: boolean | |||
| unusedExports?: boolean | |||
There was a problem hiding this comment.
Maybe we should change it to be true to align the json schema instead?
There was a problem hiding this comment.
Technically, the first branch of anyOf allows missingExports to have any boolean value, so I thought I'd change the TypeScript type to reflect that.
There was a problem hiding this comment.
We should align all the schemas I think?
There was a problem hiding this comment.
As in, move the enum: [true] of unusedExports to SHARED_OPTIONS_SCHEMA_PROPERTIES? Should the same be done to missingExports? In general, constraining boolean properties to just a single value looks like an uncommon practice. I wonder if this is needed at all?
There was a problem hiding this comment.
To be honest I would probably set the type as boolean. So it should be possible for someone to set it to false even if it is the default
commit: |
Some rules, for example
dynamic-import-chunknamecurrently allow extra properties to be passed in options object, which should not be allowed. This makes it easier for typos in rule options to go unnoticed.This PR simply disallows extra properties in rules' schemas which currently allow them.
Important
Disallow extra properties in rule options for
dynamic-import-chunkname,extensions,no-namespace, andno-unused-modulesby settingadditionalProperties: falsein their schemas.additionalProperties: falsein the schema.dynamic-import-chunkname,extensions,no-namespace, andno-unused-modulesrules.dynamic-import-chunkname.ts: AddedadditionalProperties: falseto the options schema.extensions.ts: AddedadditionalProperties: falseto thepropertiesschema.no-namespace.ts: AddedadditionalProperties: falseto the options schema.no-unused-modules.ts: AddedadditionalProperties: falseto the options schema in two places.This description was created by
for 51af82f. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
Refactor
New Features
Chores