refactor(rome_js_analyze): enable the recommended nursery rules on unstable builds

[leops]

Summary

This PR refactors the rome_flags crate to replace the previous dynamic approach to feature flags (that was unused as it was hard to test correctly) with a static approach where the value of flags like is_unstable is determined at compilation time.

This unstable flag is now used in rome_service to enabled recommended rules from the nursery group by default on unstable builds. In order to test this change I've marked all the existing nursery rules as recommended, but that may not be appropriate for all rules so we could decide to downgrade some of them before they get stabilized.

Additionally I've also adjusted the logic used to determine which rules should be enabled to allow users to enable the recommended rules from a group without enabling all the recommended rules (eg. { "rules": { "recommended": false, "correctness": { "recommended": true } } }

Test Plan

I've added two new test to check that nursery rules are enabled in unstable builds (cargo tests should always be built as unstable), and that nesting the recommended setting works correctly

[netlify[bot]]

✅ Deploy Preview for docs-rometools ready!

Name	Link
🔨 Latest commit	36ca154c25d6997bbebed0335e27fe43d00c9ad5
🔍 Latest deploy log	https://app.netlify.com/sites/docs-rometools/deploys/638a2e75c87e9b0008a81a94
😎 Deploy Preview	https://deploy-preview-3880--docs-rometools.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site settings.

[github-actions[bot]]

Parser conformance results on ubuntu-latest

js/262

Test result	main count	This PR count	Difference
Total	45879	45879	0
Passed	44936	44936	0
Failed	943	943	0
Panics	0	0	0
Coverage	97.94%	97.94%	0.00%

jsx/babel

Test result	main count	This PR count	Difference
Total	39	39	0
Passed	36	36	0
Failed	3	3	0
Panics	0	0	0
Coverage	92.31%	92.31%	0.00%

symbols/microsoft

Test result	main count	This PR count	Difference
Total	5946	5946	0
Passed	1757	1757	0
Failed	4189	4189	0
Panics	0	0	0
Coverage	29.55%	29.55%	0.00%

ts/babel

Test result	main count	This PR count	Difference
Total	588	588	0
Passed	519	519	0
Failed	69	69	0
Panics	0	0	0
Coverage	88.27%	88.27%	0.00%

ts/microsoft

Test result	main count	This PR count	Difference
Total	16257	16257	0
Passed	12397	12397	0
Failed	3860	3860	0
Panics	0	0	0
Coverage	76.26%	76.26%	0.00%

[leops]

Does the lint rule dropdown on the playground become obsolete because of this change? If so, can you remove it?

I didn't necessarily intend to include the playground in this change, but it makes sense that it ended up being included anyway as the playground is always using the latest main so it's technically an unstable build. Ideally I think we should retain the existing behavior for the playground though, where only stable rules are enabled by default but the user can opt-in to enable everything.

[Conaclos]

noConstEnum should probably be marked as recommended. The merge was done just after this PR was opened.

[leops]

Although we are enabling ALL rules, which goes against what we said before. I think I am missing something... When do we deem a rule to be recommended? When do we decide if a rule catches bugs?

We're not enabling all rules, only those that are recommended, so I specifically left out mentioning "when" determining whether a rule is recommended should be done from the documentation precisely because it should be done from the start when the rule is initially created. What this new approach does is bring us more usage information from the unstable builds, so we could decide to remove a rule from the recommended set before stabilizing it, if it turns out that it gets in the way of writing code more than help in catching actual errors.

Isn't it more natural to have nursery have rules recommended: false and then make them recommended only in debug builds? I fear that how things are worded and how's the code, could confuse people (I am confused).

We can't really have the nursery rules marked as recommended: false, since we wouldn't know which rules should be enabled on unstable builds. We'd have to maintain a separate list of recommended nursery rules, and remove entries from that list and mark the rule as actually recommended when it gets stabilized which seemed unnecessarily complex when we could have marked the rule as recommended from the start.

For example now all the diagnostic of nursery rules in the website show errors, which feels wrong. They should show warnings unless told otherwise (change the severity via config).

I'm not really sure why it would feel wrong in general, if a rule is recommended according to the documentation that means it's supposed to catch actual programming error, so Error is the correct severity for those. Now if emitting error diagnostics feels wrong for a specific rule, that's an important feedback, it means the rule probably shouldn't be recommended as it's not actually catching an error.

[ematipico]

@leops I read your reply, and it's now clear what's missing, from my prospective, with this PR. Bear with me while I explain what's going on, I try to make this clear:

1. The PR is meant to change how nursery rules are enabled for nightly builds; that's what the title says. Nightly builds don't reflect production builds.
2. The description of the PR highlights the technical details to reach that.
3. Although, it seems that there are also changes to the contribution guidelines and the website, which is the mirror of our production releases, not nightly builds.

From our unwritten practices, we agreed that:

5. Recommended rules should emit errors
6. Rules that are not recommended, should emit warnings

From our documentation on the website, we state that nursery rules are unstable, hence they should be recommended.

---

Now, from our discussion and the changes of the PR, it seems we are changing things in production too, we are actually changing things on the website. Although, these changes aren't actually reflected anywhere on our website. Here's what:

• we don't explain why some nursery rules show Error instead of a warning, while we explained them otherwise

New rules that are still under development.

• now some nursery rules are shown as not recommended but still they emit errors, and we don't explain this "expectation", which goes against with

5. Recommended rules should emit errors
6. Rules that are not recommended, should emit warnings

I you meant to also change how nursery rules are created and their process, then please make sure to state these changes on the website. My concerns are not around the changes of the PR, but the fact that changes are "mixed" (technical and not) and the fact that the non-technical changes of this PR aren't reflected on the website (apart form the auto-generated parts).

[leops]

@ematipico Just to make sure I understand correctly, the changes you're proposing to the website would be:

• Explicitly document the current behavior that recommended rules emit errors by default while non-recommended rules emit warnings
• Document a specific exception to this for nursery rules on nightly builds, as they are unstable and may emit diagnostics with any severity by default depending on whether we intend for them to be recommended or not on stabilization

[ematipico]

@ematipico Just to make sure I understand correctly, the changes you're proposing to the website would be:

* Explicitly document the current behavior that recommended rules emit errors by default while non-recommended rules emit warnings

* Document a specific exception to this for nursery rules on nightly builds, as they are unstable and may emit diagnostics with any severity by default depending on whether we intend for them to be recommended or not on stabilization

And explain why nursery rules that are shown as "not recommended" have Error in their diagnostics/examples pages.