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

Commit

Permalink
feat(rome_cli): cli exit as error when no files are found (#3722)
Browse files Browse the repository at this point in the history
* cli exit as error when no files are found
  • Loading branch information
xunilrj authored Nov 17, 2022
1 parent 41f1764 commit 062b100
Show file tree
Hide file tree
Showing 29 changed files with 256 additions and 35 deletions.
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

0 comments on commit 062b100

Please sign in to comment.