refactor(rome_js_analyze): enable the recommended nursery rules on un…

…stable builds (#3880)

* add test for nested recommended settings

* refactor `rome_flags` to expose a single `is_unstable` function

* mark all nursery rules as recommended

* add a test to ensure nursery rules are enabled in unstable builds

* add contributor documentation for the recommended flag

* mark additional rules as recommended and update playground

* update documentation

* update documentation

* Update website/src/pages/linter/index.mdx

Co-authored-by: Emanuele Stoppa <my.burning@gmail.com>

* fix or suppress errors from nursery rules

* additional rule fixes

Co-authored-by: Emanuele Stoppa <my.burning@gmail.com>

crates/rome_analyze/CONTRIBUTING.md

@@ -23,6 +23,17 @@ Inside the folders, we will have folders for each group that Rome supports.
 When implementing **new rules**, they have to be implemented under the group `nursery`. New rules should
 always be considered unstable/not exhaustive.
 
+In addition to selecting a group, rules may be flagged as `recommended` if they
+should be part of the set of rules that are run in the default configuration of the
+Rome linter. As a general principle, rules should be recommended if they catch actual
+programming errors (for instance detecting a coding pattern that will throw an
+exception at runtime), while the more pedantic rules that check for certain unwanted
+patterns but may have high false positive rates (for instance style-related rules)
+are left off from the recommended set, and the final user should enable them
+explicitly in their configuration. Rules intended to be recommended should be
+flagged as such even if they are still part of the `nursery` group, as unstable rules
+are only enabled by default on unstable builds.
+
 ## Lint rules
 
 This gives to the project time to test the rule, find edge cases, etc.

crates/rome_cli/src/lib.rs

@@ -10,7 +10,6 @@ use std::str::FromStr;
 
 pub use pico_args::Arguments;
 use rome_console::{ColorMode, EnvConsole};
-use rome_flags::FeatureFlags;
 use rome_fs::OsFileSystem;
 use rome_service::{App, DynRef, Workspace, WorkspaceRef};
 
@@ -97,10 +96,6 @@ impl<'app> CliSession<'app> {
             crate::metrics::init_metrics();
         }
 
-        if self.args.contains("--unstable") {
-            rome_flags::set_unstable_flags(FeatureFlags::ALL);
-        }
-
         let has_help = self.args.contains("--help");
         let subcommand = self
             .args

crates/rome_cli/tests/commands/check.rs

@@ -13,7 +13,7 @@ use crate::configs::{
     CONFIG_FILE_SIZE_LIMIT, CONFIG_LINTER_AND_FILES_IGNORE, CONFIG_LINTER_DISABLED,
     CONFIG_LINTER_DOWNGRADE_DIAGNOSTIC, CONFIG_LINTER_IGNORED_FILES,
     CONFIG_LINTER_SUPPRESSED_GROUP, CONFIG_LINTER_SUPPRESSED_RULE,
-    CONFIG_LINTER_UPGRADE_DIAGNOSTIC,
+    CONFIG_LINTER_UPGRADE_DIAGNOSTIC, CONFIG_RECOMMENDED_GROUP,
 };
 use crate::snap_test::SnapshotPayload;
 use crate::{assert_cli_snapshot, run_cli, FORMATTED, LINT_ERROR, PARSE_ERROR};
@@ -46,7 +46,7 @@ debugger;
 console.log(a);
 ";
 
-const APPLY_SUGGESTED_AFTER: &str = "let a = 4;\nconsole.log(a);\n";
+const APPLY_SUGGESTED_AFTER: &str = "const a = 4;\nconsole.log(a);\n";
 
 const NO_DEBUGGER_BEFORE: &str = "debugger;";
 const NO_DEBUGGER_AFTER: &str = "debugger;";
@@ -68,6 +68,9 @@ const UPGRADE_SEVERITY_CODE: &str = r#"class A extends B {
     constructor() {}
 }"#;
 
+const NURSERY_UNSTABLE: &str = r#"const array = ["split", "the text", "into words"];
+array.map(sentence => sentence.split(' ')).flat();"#;
+
 #[test]
 fn ok() {
     let mut fs = MemoryFileSystem::default();
@@ -1231,3 +1234,64 @@ fn suppression_syntax_error() {
         result,
     ));
 }
+
+#[test]
+fn config_recommended_group() {
+    let mut fs = MemoryFileSystem::default();
+    let mut console = BufferConsole::default();
+
+    let file_path = Path::new("rome.json");
+    fs.insert(file_path.into(), CONFIG_RECOMMENDED_GROUP.as_bytes());
+
+    let file_path = Path::new("check.js");
+    fs.insert(file_path.into(), NO_DEBUGGER.as_bytes());
+
+    let result = run_cli(
+        DynRef::Borrowed(&mut fs),
+        DynRef::Borrowed(&mut console),
+        Arguments::from_vec(vec![OsString::from("check"), file_path.as_os_str().into()]),
+    );
+
+    match result {
+        Ok(()) => {}
+        Err(Termination::CheckError) => {}
+        _ => panic!("run_cli returned {result:?} for a failed CI check, expected an error"),
+    }
+
+    assert_cli_snapshot(SnapshotPayload::new(
+        module_path!(),
+        "config_recommended_group",
+        fs,
+        console,
+        result,
+    ));
+}
+
+#[test]
+fn nursery_unstable() {
+    let mut fs = MemoryFileSystem::default();
+    let mut console = BufferConsole::default();
+
+    let file_path = Path::new("check.js");
+    fs.insert(file_path.into(), NURSERY_UNSTABLE.as_bytes());
+
+    let result = run_cli(
+        DynRef::Borrowed(&mut fs),
+        DynRef::Borrowed(&mut console),
+        Arguments::from_vec(vec![OsString::from("check"), file_path.as_os_str().into()]),
+    );
+
+    match result {
+        Ok(()) => {}
+        Err(Termination::CheckError) => {}
+        _ => panic!("run_cli returned {result:?} for a failed CI check, expected an error"),
+    }
+
+    assert_cli_snapshot(SnapshotPayload::new(
+        module_path!(),
+        "nursery_unstable",
+        fs,
+        console,
+        result,
+    ));
+}

crates/rome_cli/tests/configs.rs

@@ -150,6 +150,17 @@ pub const CONFIG_LINTER_UPGRADE_DIAGNOSTIC: &str = r#"{
   }
 }"#;
 
+pub const CONFIG_RECOMMENDED_GROUP: &str = r#"{
+  "linter": {
+    "rules": {
+        "recommended": false,
+        "correctness": {
+            "recommended": true
+        }
+    }
+  }
+}"#;
+
 pub const CONFIG_INCORRECT_GLOBALS_V2: &str = r#"{
     "javascript": {
       "formatter": {

crates/rome_cli/tests/snapshots/main_commands_check/apply_suggested.snap

@@ -5,7 +5,7 @@ expression: content
 ## `fix.js`
 
 ```js
-let a = 4;
+const a = 4;
 console.log(a);
 
 ```

crates/rome_cli/tests/snapshots/main_commands_check/config_recommended_group.snap

@@ -0,0 +1,53 @@
+---
+source: crates/rome_cli/tests/snap_test.rs
+expression: content
+---
+## `rome.json`
+
+```json
+{
+  "linter": {
+    "rules": {
+        "recommended": false,
+        "correctness": {
+            "recommended": true
+        }
+    }
+  }
+}
+```
+
+## `check.js`
+
+```js
+debugger;
+```
+
+# Termination Message
+
+```block
+some errors were emitted while running checks
+```
+
+# Emitted Messages
+
+```block
+check.js:1:1 lint/correctness/noDebugger  FIXABLE  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  × This is an unexpected use of the debugger statement.
+  
+  > 1 │ debugger;
+      │ ^^^^^^^^^
+  
+  i Suggested fix: Remove debugger statement
+  
+    1 │ debugger;
+      │ ---------
+
+```
+
+```block
+Checked 1 file(s) in <TIME>
+```
+
+

crates/rome_cli/tests/snapshots/main_commands_check/nursery_unstable.snap

@@ -0,0 +1,42 @@
+---
+source: crates/rome_cli/tests/snap_test.rs
+expression: content
+---
+## `check.js`
+
+```js
+const array = ["split", "the text", "into words"];
+array.map(sentence => sentence.split(' ')).flat();
+```
+
+# Termination Message
+
+```block
+some errors were emitted while running checks
+```
+
+# Emitted Messages
+
+```block
+check.js:2:1 lint/nursery/useFlatMap  FIXABLE  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  × The call chain .map().flat() can be replaced with a single .flatMap() call.
+  
+    1 │ const array = ["split", "the text", "into words"];
+  > 2 │ array.map(sentence => sentence.split(' ')).flat();
+      │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+  
+  i Safe fix: Replace the chain with .flatMap().
+  
+    1 1 │   const array = ["split", "the text", "into words"];
+    2   │ - array.map(sentence·=>·sentence.split('·')).flat();
+      2 │ + array.flatMap(sentence·=>·sentence.split('·'));
+  
+
+```
+
+```block
+Checked 1 file(s) in <TIME>
+```
+
+

crates/rome_cli/tests/snapshots/main_commands_ci/ci_runs_linter_not_formatter_issue_3495.snap

@@ -41,6 +41,27 @@ file.js:1:9 lint/complexity/useSimplifiedLogicExpression  FIXABLE  ━━━━
   + let·a·=·!(b·&&·c)
   
 
+```
+
+```block
+file.js:1:1 lint/nursery/useConst  FIXABLE  ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  × This 'let' declares a variable which is never re-assigned.
+  
+  > 1 │ let a = !b || !c
+      │ ^^^
+  
+  i 'a' is never re-assigned.
+  
+  > 1 │ let a = !b || !c
+      │     ^
+  
+  i Suggested fix: Use 'const' instead.
+  
+  - let·a·=·!b·||·!c
+  + const·a·=·!b·||·!c
+  
+
 ```
 
 ```block

crates/rome_flags/Cargo.toml

@@ -9,4 +9,3 @@ license = "MIT"
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [dependencies]
-once_cell = "1.10"