chore(rome_js_analyze): noSwitchDeclarations

[Conaclos]

Summary

This implements ESLint's no-case-declarations under the name noSwitchDeclarations.

In contrast to ESLint, this rule handles TypeScript declarations (enum, ...).

Test Plan

Uni test and doc-tests included.

[netlify]

✅ Deploy Preview for docs-rometools canceled.

Built without sensitive environment variables

Name	Link
🔨 Latest commit	2c49a0a
🔍 Latest deploy log	https://app.netlify.com/sites/docs-rometools/deploys/63ea9e24bcfa930008c18c8f

[MichaReiser]

This rule has overlaps with the UseSingleCaseStatement. What's your recommendation on how these rules should (or should not) co-exist?

[MichaReiser]

What's your reasoning for using the name noSwitchDeclaration over ESLint's name?

[Conaclos]

1. the rule applies to both any switch clauses (including default one)
2. It is not really declarations at the scope of case/default that are prohibited: it is at the scope of the switch

I first renamed to noSwitchScopedDeclarations (alternative could be noSwitchHoistedDeclarations). And then simplified it to noSwitchDeclarations.

By the way, I am not sure about the plural form.

[ematipico]

By the way, I am not sure about the plural form.

It should be singular, in my opinion. The rule is triggered for discriminant inside the switch, not for the whole switch. I would suggest noDeclarationInsideSwitch, which sounds better to me.

[Conaclos]

noDeclarationInSwitch could be more consistent with existing rules? (noAssignInExpressions, ...)

Or noSwitchClauseDeclarations?

I personally prefer noSwitchDeclarations because these declarations are hoisted to the switch scope.

[Conaclos]

@MichaReiser

This rule has overlaps with the UseSingleCaseStatement. What's your recommendation on how these rules should (or should not) co-exist?

I could suggest removing useSingleCaseStatement of the recommended rule set, moving the rule to style. The rule has already some negative user feedbacks.

I could also propose to eventually make useSingleCaseStatement obsolete, and adding an option (disabled by default) to useBlock for requiring a block for every non-empty case/default clause.

[ematipico]

I could also propose to eventually make useSingleCaseStatement obsolete

We could deprecate useSingleCaseStatement in this release cycle and suggest this new rule. This won't create breaking changes.

[leops]

I could also propose to eventually make useSingleCaseStatement obsolete

We could deprecate useSingleCaseStatement in this release cycle and suggest this new rule. This won't create breaking changes.

The useSingleCaseStatement rule has already been moved to the style group and removed from the recommended set in 11.0.0. I don't think we should go as far as deprecating it since its use case (enforce that case clause are wrapped in a block) is somewhat different from what noSwitchDeclaration is enforcing (disallow declarations in case clause), and some users may want to keep both rules enabled.

[ematipico]

While it's true that we render diagnostics as part of the documentation, we should also test in our test suite for many reasons:

• diagnostics can be seen as plain text, helping the reviewers;
• they help to catch regression tools - in fact if a rule changes group and we forget to change its group in the testing suite, a snapshot test change is triggered;