Skip to content

Conversation

@connorshea
Copy link
Contributor

This fixes the no-inner-declarations docs by treating them as a tuple. Although, it also breaks the tests for no-inner-declarations. 🙃

I think there may be a bug in the tests/logic for our no-inner-declarations rule (I think oxc assumes strict mode by default maybe?). We previously returned None for block_scoped_expressions instead of allow, which the original rule does not do. It always just sets allow as the default value as far as I can tell.

I'm unsure if I should add an impl Default for NoInnerDeclarationsConfigObject and set it to return None, and retain that behavior, or if we should fix this behavior to match the original ESLint rule better? Up to cam, probably.

Generated docs:

## Configuration

### The 1st option

type: `"functions" | "both"`

#### `"functions"`

Disallows function declarations in nested blocks.

#### `"both"`

Disallows function and var declarations in nested blocks.

### The 2nd option

This option is an object with the following properties:

#### blockScopedFunctions

type: `"allow" | "disallow"`

##### `"allow"`

Allow function declarations in nested blocks in strict mode (ES6+ behavior).

##### `"disallow"`

Disallow function declarations in nested blocks regardless of strict mode.

…ex config shapes.

This will make it much easier to handle rules which accept tuples. I still need to fix a few things, though.
…mplify some things.

This will hopefully also ensure that the most common case (a default config object for a given rule) is fast. But I still need to simplify the rest of the code here.
… now that we can do tuples in DefaultRuleConfig.

This is a proof-of-concept to confirm that these updates to the DefaultRuleConfig tuple handling work as intended and simplify the code :)
…tions rule.

I'm pretty sure there's a bug in this rule. It doesn't seem like it should've been returning `None` for the `block_scoped_functions` value by default, the original ESLint rule returns "allow" by default. But I left the behavior alone here, we can address that later if we want to.
I'm pretty sure the no_inner_declarations rule has a bug in it right now, so it failing here isn't really indicative of any problem with the .

It's very odd to me that it was defaulting to None for the `block_scoped_functions` instead of `Allow`, I don't understand why that was done and it seems like there are behavioral differences vs. the original rule as a result.
I'd rather just leave it as-is for now so that we can focus on the other changes in this PR. I'll open another PR with the code cleanup on this rule.
@connorshea connorshea requested a review from camc314 as a code owner December 6, 2025 06:59
@github-actions github-actions bot added A-linter Area - Linter C-docs Category - Documentation. Related to user-facing or internal documentation labels Dec 6, 2025
@codspeed-hq
Copy link

codspeed-hq bot commented Dec 6, 2025

CodSpeed Performance Report

Merging #16556 will not alter performance

Comparing inner-decls (6f5894b) with handle-tuple-rules-better (3abce5e)

Summary

✅ 4 untouched
⏩ 41 skipped1

Footnotes

  1. 41 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.

@connorshea connorshea marked this pull request as draft December 6, 2025 20:37
@connorshea
Copy link
Contributor Author

Marking as a draft until we have a discussion on whether this current behavior is the desired one.

@camc314 camc314 self-assigned this Dec 6, 2025
@connorshea connorshea force-pushed the handle-tuple-rules-better branch from ca59885 to 962bfe2 Compare December 6, 2025 22:52
sort_order,
SortKeysOptions { case_sensitive, natural, min_keys, allow_line_separated_groups },
)))
serde_json::from_value::<DefaultRuleConfig<SortKeys>>(value)
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe we can use serde_json::from_value::<SortKeys>(value) here, so the change to DefaultRuleConfig is unnecessary?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-linter Area - Linter C-docs Category - Documentation. Related to user-facing or internal documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants