Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

feat(rome_cli): cli exit as error when no files are found #3722

Merged
merged 7 commits into from
Nov 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions crates/rome_cli/src/termination.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,9 @@ pub enum Termination {

#[error("The combination of configuration and arguments is invalid: \n {0}")]
IncompatibleEndConfiguration(&'static str),

#[error("no files were processed in the specified paths.")]
NoFilesWereProcessed,
}

fn command_name() -> String {
Expand Down
26 changes: 15 additions & 11 deletions crates/rome_cli/src/traversal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,9 @@ pub(crate) fn traverse(execution: Execution, mut session: CliSession) -> Result<
}

// Processing emitted error diagnostics, exit with a non-zero code
if errors > 0 {
if (count - skipped) == 0 {
Err(Termination::NoFilesWereProcessed)
} else if errors > 0 {
Err(Termination::CheckError)
} else {
Ok(())
Expand Down Expand Up @@ -605,6 +607,10 @@ struct TraversalOptions<'ctx, 'app> {
}

impl<'ctx, 'app> TraversalOptions<'ctx, 'app> {
fn increment_processed(&self) {
self.processed.fetch_add(1, Ordering::Relaxed);
}

/// Send a message to the display thread
fn push_message(&self, msg: impl Into<Message>) {
self.messages.send(msg.into()).ok();
Expand Down Expand Up @@ -688,11 +694,8 @@ impl<'ctx, 'app> TraversalContext for TraversalOptions<'ctx, 'app> {
/// traversal function returns Err or panics)
fn handle_file(ctx: &TraversalOptions, path: &Path, file_id: FileId) {
match catch_unwind(move || process_file(ctx, path, file_id)) {
Ok(Ok(FileStatus::Success)) => {
ctx.processed.fetch_add(1, Ordering::Relaxed);
}
Ok(Ok(FileStatus::Success)) => {}
Ok(Ok(FileStatus::Message(msg))) => {
ctx.processed.fetch_add(1, Ordering::Relaxed);
ctx.push_message(msg);
}
Ok(Ok(FileStatus::Ignored)) => {}
Expand Down Expand Up @@ -739,13 +742,16 @@ type FileResult = Result<FileStatus, Message>;
fn process_file(ctx: &TraversalOptions, path: &Path, file_id: FileId) -> FileResult {
tracing::trace_span!("process_file", path = ?path).in_scope(move || {
let rome_path = RomePath::new(path, file_id);

let supported_format = ctx
.can_format(&rome_path)
.with_file_id_and_code(file_id, category!("files/missingHandler"))?;

let supported_lint = ctx
.can_lint(&rome_path)
.with_file_id_and_code(file_id, category!("files/missingHandler"))?;
let supported_file = match ctx.execution.traversal_mode() {

let unsupported_reason = match ctx.execution.traversal_mode() {
TraversalMode::Check { .. } => supported_lint.reason.as_ref(),
TraversalMode::CI { .. } => supported_lint
.reason
Expand All @@ -754,7 +760,7 @@ fn process_file(ctx: &TraversalOptions, path: &Path, file_id: FileId) -> FileRes
TraversalMode::Format { .. } => supported_format.reason.as_ref(),
};

if let Some(reason) = supported_file {
if let Some(reason) = unsupported_reason {
return match reason {
UnsupportedReason::FileNotSupported => {
Err(Message::from(UnhandledDiagnostic.with_file_path(file_id)))
Expand All @@ -773,6 +779,7 @@ fn process_file(ctx: &TraversalOptions, path: &Path, file_id: FileId) -> FileRes

let mut input = String::new();
file.read_to_string(&mut input).with_file_id(file_id)?;
ctx.increment_processed();

let file_guard = FileGuard::open(
ctx.workspace,
Expand All @@ -797,12 +804,9 @@ fn process_file(ctx: &TraversalOptions, path: &Path, file_id: FileId) -> FileRes
if fixed.code != input {
file.set_content(fixed.code.as_bytes())
.with_file_id(file_id)?;

return Ok(FileStatus::Success);
}

// If the file isn't changed, do not increment the "fixed files" counter
return Ok(FileStatus::Ignored);
return Ok(FileStatus::Success);
}

let categories = if ctx.execution.is_format() || supported_lint.reason.is_some() {
Expand Down
42 changes: 32 additions & 10 deletions crates/rome_cli/tests/commands/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ fn no_lint_if_linter_is_disabled_when_run_apply() {
]),
);

assert!(result.is_ok(), "run_cli returned {result:?}");
assert!(result.is_err(), "run_cli returned {result:?}");

let mut buffer = String::new();
fs.open(file_path)
Expand Down Expand Up @@ -386,7 +386,7 @@ fn no_lint_if_linter_is_disabled() {
Arguments::from_vec(vec![OsString::from("check"), file_path.as_os_str().into()]),
);

assert!(result.is_ok(), "run_cli returned {result:?}");
assert!(result.is_err(), "run_cli returned {result:?}");

let mut buffer = String::new();
fs.open(file_path)
Expand Down Expand Up @@ -600,7 +600,7 @@ fn no_lint_when_file_is_ignored() {
]),
);

assert!(result.is_ok(), "run_cli returned {result:?}");
assert!(result.is_err(), "run_cli returned {result:?}");

let mut buffer = String::new();
fs.open(file_path)
Expand Down Expand Up @@ -644,7 +644,7 @@ fn no_lint_if_files_are_listed_in_ignore_option() {
]),
);

assert!(result.is_ok(), "run_cli returned {result:?}");
assert!(result.is_err(), "run_cli returned {result:?}");

let mut buffer = String::new();
fs.open(file_path_test1)
Expand Down Expand Up @@ -707,7 +707,7 @@ fn fs_error_dereferenced_symlink() {

remove_dir_all(root_path).unwrap();

assert!(result.is_ok(), "run_cli returned {result:?}");
assert!(result.is_err(), "run_cli returned {result:?}");

assert_cli_snapshot(SnapshotPayload::new(
module_path!(),
Expand Down Expand Up @@ -762,7 +762,7 @@ fn fs_error_infinite_symlink_exapansion() {

remove_dir_all(root_path).unwrap();

assert!(result.is_ok(), "run_cli returned {result:?}");
assert!(result.is_err(), "run_cli returned {result:?}");

assert_cli_snapshot(SnapshotPayload::new(
module_path!(),
Expand All @@ -789,7 +789,7 @@ fn fs_error_unknown() {
Arguments::from_vec(vec![OsString::from("check"), OsString::from("prefix")]),
);

assert!(result.is_ok(), "run_cli returned {result:?}");
assert!(result.is_err(), "run_cli returned {result:?}");

assert_cli_snapshot(SnapshotPayload::new(
module_path!(),
Expand All @@ -814,7 +814,7 @@ fn file_too_large() {
Arguments::from_vec(vec![OsString::from("check"), file_path.as_os_str().into()]),
);

assert!(result.is_ok(), "run_cli returned {result:?}");
assert!(result.is_err(), "run_cli returned {result:?}");

// Do not store the content of the file in the snapshot
fs.remove(file_path);
Expand Down Expand Up @@ -844,7 +844,7 @@ fn file_too_large_config_limit() {
Arguments::from_vec(vec![OsString::from("check"), file_path.as_os_str().into()]),
);

assert!(result.is_ok(), "run_cli returned {result:?}");
assert!(result.is_err(), "run_cli returned {result:?}");

assert_cli_snapshot(SnapshotPayload::new(
module_path!(),
Expand Down Expand Up @@ -874,7 +874,7 @@ fn file_too_large_cli_limit() {
]),
);

assert!(result.is_ok(), "run_cli returned {result:?}");
assert!(result.is_err(), "run_cli returned {result:?}");

assert_cli_snapshot(SnapshotPayload::new(
module_path!(),
Expand Down Expand Up @@ -970,3 +970,25 @@ fn max_diagnostics() {

assert_eq!(console.out_buffer.len(), 11);
}

#[test]
fn no_supported_file_found() {
let mut fs = MemoryFileSystem::default();
let mut console = BufferConsole::default();

let result = run_cli(
DynRef::Borrowed(&mut fs),
DynRef::Borrowed(&mut console),
Arguments::from_vec(vec![std::ffi::OsString::from("check"), ".".into()]),
);

eprintln!("{:?}", console.out_buffer);

assert_cli_snapshot(SnapshotPayload::new(
module_path!(),
"no_supported_file_found",
fs,
console,
result,
));
}
6 changes: 3 additions & 3 deletions crates/rome_cli/tests/commands/ci.rs
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ fn file_too_large() {
Arguments::from_vec(vec![OsString::from("ci"), file_path.as_os_str().into()]),
);

assert!(result.is_ok(), "run_cli returned {result:?}");
assert!(result.is_err(), "run_cli returned {result:?}");

// Do not store the content of the file in the snapshot
fs.remove(file_path);
Expand Down Expand Up @@ -406,7 +406,7 @@ fn file_too_large_config_limit() {
Arguments::from_vec(vec![OsString::from("ci"), file_path.as_os_str().into()]),
);

assert!(result.is_ok(), "run_cli returned {result:?}");
assert!(result.is_err(), "run_cli returned {result:?}");

assert_cli_snapshot(SnapshotPayload::new(
module_path!(),
Expand Down Expand Up @@ -436,7 +436,7 @@ fn file_too_large_cli_limit() {
]),
);

assert!(result.is_ok(), "run_cli returned {result:?}");
assert!(result.is_err(), "run_cli returned {result:?}");

assert_cli_snapshot(SnapshotPayload::new(
module_path!(),
Expand Down
34 changes: 28 additions & 6 deletions crates/rome_cli/tests/commands/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -859,7 +859,7 @@ fn format_is_disabled() {
]),
);

assert!(result.is_ok(), "run_cli returned {result:?}");
assert!(result.is_err(), "run_cli returned {result:?}");

let mut file = fs
.open(file_path)
Expand Down Expand Up @@ -1025,7 +1025,7 @@ fn does_not_format_ignored_files() {
]),
);

assert!(result.is_ok(), "run_cli returned {result:?}");
assert!(result.is_err(), "run_cli returned {result:?}");

let mut file = fs
.open(file_path)
Expand Down Expand Up @@ -1075,7 +1075,7 @@ fn does_not_format_if_files_are_listed_in_ignore_option() {
]),
);

assert!(result.is_ok(), "run_cli returned {result:?}");
assert!(result.is_err(), "run_cli returned {result:?}");

let mut buffer = String::new();
fs.open(file_path_test1)
Expand Down Expand Up @@ -1178,7 +1178,7 @@ fn file_too_large() {
]),
);

assert!(result.is_ok(), "run_cli returned {result:?}");
assert!(result.is_err(), "run_cli returned {result:?}");

// Do not store the content of the file in the snapshot
fs.remove(file_path);
Expand Down Expand Up @@ -1208,7 +1208,7 @@ fn file_too_large_config_limit() {
Arguments::from_vec(vec![OsString::from("format"), file_path.as_os_str().into()]),
);

assert!(result.is_ok(), "run_cli returned {result:?}");
assert!(result.is_err(), "run_cli returned {result:?}");

assert_cli_snapshot(SnapshotPayload::new(
module_path!(),
Expand Down Expand Up @@ -1238,7 +1238,7 @@ fn file_too_large_cli_limit() {
]),
);

assert!(result.is_ok(), "run_cli returned {result:?}");
assert!(result.is_err(), "run_cli returned {result:?}");

assert_cli_snapshot(SnapshotPayload::new(
module_path!(),
Expand Down Expand Up @@ -1328,3 +1328,25 @@ fn max_diagnostics() {

assert_eq!(console.out_buffer.len(), 12);
}

#[test]
fn no_supported_file_found() {
let mut fs = MemoryFileSystem::default();
let mut console = BufferConsole::default();

let result = run_cli(
DynRef::Borrowed(&mut fs),
DynRef::Borrowed(&mut console),
Arguments::from_vec(vec![std::ffi::OsString::from("check"), ".".into()]),
);

eprintln!("{:?}", console.out_buffer);

assert_cli_snapshot(SnapshotPayload::new(
module_path!(),
"no_supported_file_found",
fs,
console,
result,
));
}
9 changes: 6 additions & 3 deletions crates/rome_cli/tests/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ mod configuration {
Arguments::from_vec(vec![OsString::from("format"), OsString::from("file.js")]),
);

assert!(result.is_ok(), "run_cli returned {result:?}");
assert!(result.is_err(), "run_cli returned {result:?}");

assert_cli_snapshot(SnapshotPayload::new(
module_path!(),
Expand Down Expand Up @@ -381,8 +381,11 @@ mod configuration {
let mut fs = MemoryFileSystem::default();
let mut console = BufferConsole::default();

let file_path = Path::new("rome.json");
fs.insert(file_path.into(), CONFIG_INCORRECT_GLOBALS_V2.as_bytes());
fs.insert(
Path::new("rome.json").into(),
CONFIG_INCORRECT_GLOBALS_V2.as_bytes(),
);
fs.insert(Path::new("file.js").into(), UNFORMATTED.as_bytes());

let result = run_cli(
DynRef::Borrowed(&mut fs),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@
source: crates/rome_cli/tests/snap_test.rs
expression: content
---
# Termination Message

```block
no files were processed in the specified paths.
```

# Emitted Messages

```block
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@ statement1();
statement2();
```

# Termination Message

```block
no files were processed in the specified paths.
```

# Emitted Messages

```block
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@ statement1();
statement2();
```

# Termination Message

```block
no files were processed in the specified paths.
```

# Emitted Messages

```block
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
---
source: crates/rome_cli/tests/snap_test.rs
assertion_line: 223
expression: content
---
# Termination Message

```block
no files were processed in the specified paths.
```

# Emitted Messages

```block
Expand Down
Loading