From a3c34ebccb6fad5a2898679e5d0f98ab61c05bcf Mon Sep 17 00:00:00 2001 From: Micha Reiser Date: Tue, 8 Nov 2022 17:12:01 +0100 Subject: [PATCH] fix(rome_cli): Respect formatter/linter `enabled` from configuration Fixes an issue where `rome ci` ignored the `formatter.enabled` and `linter.disabled` configuration. The issue was that the CI defaulted to `true` when the `--formater/linter-enabled` options weren't specified. ## Test Plan The reason why the existing integration test didn't work is that Rome doesn't check the formatting for files that have lint/parse errors. I changed the integration test to a file that only has incorrect formatting. --- crates/rome_cli/src/commands/ci.rs | 8 +++- crates/rome_cli/tests/commands/ci.rs | 37 +++++++++++-------- .../ci_does_not_run_formatter.snap | 24 +----------- .../ci_does_not_run_formatter_via_cli.snap | 24 +----------- 4 files changed, 29 insertions(+), 64 deletions(-) diff --git a/crates/rome_cli/src/commands/ci.rs b/crates/rome_cli/src/commands/ci.rs index 1e34d52968d..93d6be235d7 100644 --- a/crates/rome_cli/src/commands/ci.rs +++ b/crates/rome_cli/src/commands/ci.rs @@ -31,13 +31,17 @@ pub(crate) fn ci(mut session: CliSession) -> Result<(), Termination> { .formatter .get_or_insert_with(FormatterConfiguration::default); - formatter.enabled = formatter_enabled.unwrap_or(true); + if let Some(formatter_enabled) = formatter_enabled { + formatter.enabled = formatter_enabled; + } let linter = configuration .linter .get_or_insert_with(LinterConfiguration::default); - linter.enabled = linter_enabled.unwrap_or(true); + if let Some(linter_enabled) = linter_enabled { + linter.enabled = linter_enabled; + } // no point in doing the traversal if all the checks have been disabled if configuration.is_formatter_disabled() && configuration.is_linter_disabled() { diff --git a/crates/rome_cli/tests/commands/ci.rs b/crates/rome_cli/tests/commands/ci.rs index 95c3c89dfa5..1f292481d60 100644 --- a/crates/rome_cli/tests/commands/ci.rs +++ b/crates/rome_cli/tests/commands/ci.rs @@ -160,29 +160,32 @@ fn ci_does_not_run_formatter() { let mut fs = MemoryFileSystem::default(); let mut console = BufferConsole::default(); - let file_path = Path::new("rome.json"); - fs.insert(file_path.into(), CONFIG_DISABLED_FORMATTER.as_bytes()); + fs.insert( + PathBuf::from("rome.json"), + CONFIG_DISABLED_FORMATTER.as_bytes(), + ); - let file_path = Path::new("file.js"); - fs.insert(file_path.into(), UNFORMATTED_AND_INCORRECT.as_bytes()); + let input_file = Path::new("file.js"); + + fs.insert(input_file.into(), UNFORMATTED.as_bytes()); let result = run_cli( DynRef::Borrowed(&mut fs), DynRef::Borrowed(&mut console), - Arguments::from_vec(vec![OsString::from("ci"), file_path.as_os_str().into()]), + Arguments::from_vec(vec![OsString::from("ci"), input_file.as_os_str().into()]), ); - assert!(result.is_err(), "run_cli returned {result:?}"); + assert!(result.is_ok(), "run_cli returned {result:?}"); let mut file = fs - .open(file_path) + .open(input_file) .expect("formatting target file was removed by the CLI"); let mut content = String::new(); file.read_to_string(&mut content) .expect("failed to read file from memory FS"); - assert_eq!(content, UNFORMATTED_AND_INCORRECT); + assert_eq!(content, UNFORMATTED); drop(file); assert_cli_snapshot(SnapshotPayload::new( @@ -199,8 +202,8 @@ fn ci_does_not_run_formatter_via_cli() { let mut fs = MemoryFileSystem::default(); let mut console = BufferConsole::default(); - let file_path = Path::new("file.js"); - fs.insert(file_path.into(), UNFORMATTED_AND_INCORRECT.as_bytes()); + let input_file = Path::new("file.js"); + fs.insert(input_file.into(), UNFORMATTED.as_bytes()); let result = run_cli( DynRef::Borrowed(&mut fs), @@ -208,21 +211,21 @@ fn ci_does_not_run_formatter_via_cli() { Arguments::from_vec(vec![ OsString::from("ci"), OsString::from("--formatter-enabled=false"), - file_path.as_os_str().into(), + input_file.as_os_str().into(), ]), ); - assert!(result.is_err(), "run_cli returned {result:?}"); + assert!(result.is_ok(), "run_cli returned {result:?}"); let mut file = fs - .open(file_path) + .open(input_file) .expect("formatting target file was removed by the CLI"); let mut content = String::new(); file.read_to_string(&mut content) .expect("failed to read file from memory FS"); - assert_eq!(content, UNFORMATTED_AND_INCORRECT); + assert_eq!(content, UNFORMATTED); drop(file); assert_cli_snapshot(SnapshotPayload::new( @@ -239,8 +242,10 @@ fn ci_does_not_run_linter() { let mut fs = MemoryFileSystem::default(); let mut console = BufferConsole::default(); - let file_path = Path::new("rome.json"); - fs.insert(file_path.into(), CONFIG_LINTER_DISABLED.as_bytes()); + fs.insert( + PathBuf::from("rome.json"), + CONFIG_LINTER_DISABLED.as_bytes(), + ); let file_path = Path::new("file.js"); fs.insert(file_path.into(), CUSTOM_FORMAT_BEFORE.as_bytes()); diff --git a/crates/rome_cli/tests/snapshots/main_commands_ci/ci_does_not_run_formatter.snap b/crates/rome_cli/tests/snapshots/main_commands_ci/ci_does_not_run_formatter.snap index 4439425e262..ad13a9742ad 100644 --- a/crates/rome_cli/tests/snapshots/main_commands_ci/ci_does_not_run_formatter.snap +++ b/crates/rome_cli/tests/snapshots/main_commands_ci/ci_does_not_run_formatter.snap @@ -16,31 +16,9 @@ expression: content ## `file.js` ```js -statement( ) ; let a = !b || !c; -``` - -# Termination Message - -```block -errors where emitted while running checks + statement( ) ``` # Emitted Messages -```block -file.js:1:27 lint/complexity/useSimplifiedLogicExpression FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - - × Logical expression contains unnecessary complexity. - - > 1 │ statement( ) ; let a = !b || !c; - │ ^^^^^^^^ - - i Suggested fix: Reduce the complexity of the logical expression. - - - statement(····)·;·let·a·=·!b·||·!c; - + statement(····)·;·let·a·=·!(b·&&·c); - - -``` - diff --git a/crates/rome_cli/tests/snapshots/main_commands_ci/ci_does_not_run_formatter_via_cli.snap b/crates/rome_cli/tests/snapshots/main_commands_ci/ci_does_not_run_formatter_via_cli.snap index b355bacab90..1d965d4d614 100644 --- a/crates/rome_cli/tests/snapshots/main_commands_ci/ci_does_not_run_formatter_via_cli.snap +++ b/crates/rome_cli/tests/snapshots/main_commands_ci/ci_does_not_run_formatter_via_cli.snap @@ -5,31 +5,9 @@ expression: content ## `file.js` ```js -statement( ) ; let a = !b || !c; -``` - -# Termination Message - -```block -errors where emitted while running checks + statement( ) ``` # Emitted Messages -```block -file.js:1:27 lint/complexity/useSimplifiedLogicExpression FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ - - × Logical expression contains unnecessary complexity. - - > 1 │ statement( ) ; let a = !b || !c; - │ ^^^^^^^^ - - i Suggested fix: Reduce the complexity of the logical expression. - - - statement(····)·;·let·a·=·!b·||·!c; - + statement(····)·;·let·a·=·!(b·&&·c); - - -``` -