Skip to content

fix(dx): deny type Options = () in rules#9281

Merged
dyc3 merged 1 commit intomainfrom
dyc3/forbid-empty-options
Feb 28, 2026
Merged

fix(dx): deny type Options = () in rules#9281
dyc3 merged 1 commit intomainfrom
dyc3/forbid-empty-options

Conversation

@dyc3
Copy link
Contributor

@dyc3 dyc3 commented Feb 28, 2026

Summary

fixes #9263

Test Plan

CI should be green

Docs

@changeset-bot
Copy link

changeset-bot bot commented Feb 28, 2026

⚠️ No Changeset found

Latest commit: 7b35275

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions github-actions bot added A-Linter Area: linter A-Tooling Area: internal tools L-JavaScript Language: JavaScript and super languages labels Feb 28, 2026
@github-actions
Copy link
Contributor

Parser conformance results on

js/262

Test result main count This PR count Difference
Total 52934 52934 0
Passed 51714 51714 0
Failed 1178 1178 0
Panics 42 42 0
Coverage 97.70% 97.70% 0.00%

jsx/babel

Test result main count This PR count Difference
Total 38 38 0
Passed 37 37 0
Failed 1 1 0
Panics 0 0 0
Coverage 97.37% 97.37% 0.00%

markdown/commonmark

Test result main count This PR count Difference
Total 652 652 0
Passed 652 652 0
Failed 0 0 0
Panics 0 0 0
Coverage 100.00% 100.00% 0.00%

symbols/microsoft

Test result main count This PR count Difference
Total 5464 5464 0
Passed 1915 1915 0
Failed 3549 3549 0
Panics 0 0 0
Coverage 35.05% 35.05% 0.00%

ts/babel

Test result main count This PR count Difference
Total 635 635 0
Passed 567 567 0
Failed 68 68 0
Panics 0 0 0
Coverage 89.29% 89.29% 0.00%

ts/microsoft

Test result main count This PR count Difference
Total 18872 18872 0
Passed 13012 13012 0
Failed 5859 5859 0
Panics 1 1 0
Coverage 68.95% 68.95% 0.00%

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 28, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0fc9bbe and 7b35275.

📒 Files selected for processing (1)
  • xtask/rules_check/src/lib.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • xtask/rules_check/src/lib.rs

Walkthrough

The rules check harness now validates that lint rules declare proper options types. Using TypeId at runtime, the harness detects when a rule declares type Options = () and records an error, directing developers to use a generated options struct instead (such as RuleNameOptions). This prevents the unit type from reaching downstream code that expects a concrete options structure.

Possibly related PRs

Suggested reviewers

  • ematipico
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: forbidding empty unit type options in rules. It's specific and directly reflects the changeset.
Description check ✅ Passed The description references issue #9263 and explains the context (panic prevention), aligning with the changeset's purpose to enforce proper options types.
Linked Issues check ✅ Passed The PR fully implements the proposed solution from issue #9263: adding a runtime check to forbid type Options = () and updating four rules to use proper options structs.
Out of Scope Changes check ✅ Passed All changes directly support the objective of enforcing proper options types—no unrelated modifications are present.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch dyc3/forbid-empty-options

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@crates/biome_rule_options/src/no_initializer_with_definite.rs`:
- Line 6: Add an inline rustdoc comment for the public options struct
NoInitializerWithDefiniteOptions describing its purpose and usage so it appears
in generated docs; update the declaration of NoInitializerWithDefiniteOptions to
include a concise triple-slash doc comment (one-line summary, optional second
line for brief details) above the struct definition to follow the guideline "Use
inline rustdoc documentation for rules, assists, and their options".

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a2b4494 and 0fc9bbe.

📒 Files selected for processing (10)
  • crates/biome_js_analyze/src/syntax/correctness/no_duplicate_private_class_members.rs
  • crates/biome_js_analyze/src/syntax/correctness/no_initializer_with_definite.rs
  • crates/biome_js_analyze/src/syntax/correctness/no_super_without_extends.rs
  • crates/biome_js_analyze/src/syntax/correctness/no_type_only_import_attributes.rs
  • crates/biome_rule_options/src/lib.rs
  • crates/biome_rule_options/src/no_duplicate_private_class_members.rs
  • crates/biome_rule_options/src/no_initializer_with_definite.rs
  • crates/biome_rule_options/src/no_super_without_extends.rs
  • crates/biome_rule_options/src/no_type_only_import_attributes.rs
  • xtask/rules_check/src/lib.rs

#[derive(Default, Clone, Debug, Deserialize, Deserializable, Merge, Eq, PartialEq, Serialize)]
#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))]
#[serde(rename_all = "camelCase", deny_unknown_fields, default)]
pub struct NoInitializerWithDefiniteOptions {}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Add inline rustdoc for the new options type.

Line 6 introduces a public options struct without a /// doc comment. Please add a short rustdoc summary so this stays discoverable in generated docs (future-you will thank present-you).

As per coding guidelines: “Use inline rustdoc documentation for rules, assists, and their options”.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@crates/biome_rule_options/src/no_initializer_with_definite.rs` at line 6, Add
an inline rustdoc comment for the public options struct
NoInitializerWithDefiniteOptions describing its purpose and usage so it appears
in generated docs; update the declaration of NoInitializerWithDefiniteOptions to
include a concise triple-slash doc comment (one-line summary, optional second
line for brief details) above the struct definition to follow the guideline "Use
inline rustdoc documentation for rules, assists, and their options".

@codspeed-hq
Copy link

codspeed-hq bot commented Feb 28, 2026

Merging this PR will not alter performance

✅ 58 untouched benchmarks
⏩ 156 skipped benchmarks1


Comparing dyc3/forbid-empty-options (0fc9bbe) with main (a2b4494)

Open in CodSpeed

Footnotes

  1. 156 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@dyc3
Copy link
Contributor Author

dyc3 commented Feb 28, 2026

Actually at a bit of a loss for why the CI is failing here. Everything passes for me locally. But seeing as its all related to syntax rules, it would make sense for those to not have options, so I'm just going to exclude them.

@dyc3 dyc3 force-pushed the dyc3/forbid-empty-options branch from 0fc9bbe to 7b35275 Compare February 28, 2026 22:40
@github-actions github-actions bot removed the A-Linter Area: linter label Feb 28, 2026
@dyc3 dyc3 merged commit e49529b into main Feb 28, 2026
12 checks passed
@dyc3 dyc3 deleted the dyc3/forbid-empty-options branch February 28, 2026 23:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Tooling Area: internal tools L-JavaScript Language: JavaScript and super languages

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve rules check harness to forbid type Options = ()

1 participant