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

feat(rome_cli): exit with error if there are warnings #4676

Merged
merged 3 commits into from
Jul 10, 2023
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
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,12 @@ multiple files:
- Fix the commands `rome check` and `rome lint`, they won't exit with an error code
if no error diagnostics are emitted.

- Add a new option `--error-on-warnings`, which instructs Rome to exit with an error code when warnings are emitted.

```shell
rome check --error-on-wanrings ./src
```

### Editors

#### Other changes
Expand Down
15 changes: 12 additions & 3 deletions crates/rome_cli/src/cli_options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,14 @@ pub struct CliOptions {
#[bpaf(long("config-path"), argument("PATH"), optional)]
pub config_path: Option<String>,

/// Cap the amount of diagnostics displayed (default: 20)
#[bpaf(long("max-diagnostics"), argument("NUMBER"), optional)]
pub max_diagnostics: Option<u16>,
/// Cap the amount of diagnostics displayed.
#[bpaf(
long("max-diagnostics"),
argument("NUMBER"),
fallback(20),
display_fallback
)]
pub max_diagnostics: u16,

/// Skip over files containing syntax errors instead of emitting an error diagnostic.
#[bpaf(long("skip-errors"), switch)]
Expand All @@ -32,6 +37,10 @@ pub struct CliOptions {
#[bpaf(long("no-errors-on-unmatched"), switch)]
pub no_errors_on_unmatched: bool,

/// Tell Rome to exit with an error code if some diagnostics emit warnings.
#[bpaf(long("error-on-warnings"), switch)]
pub error_on_warnings: bool,

/// Reports information using the JSON format
#[bpaf(long("json"), switch, hide_usage)]
pub json: bool,
Expand Down
86 changes: 40 additions & 46 deletions crates/rome_cli/src/diagnostics.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use rome_console::fmt::{Display, Formatter};
use rome_console::fmt::Formatter;
use rome_console::markup;
use rome_diagnostics::adapters::{BpafError, IoError};
use rome_diagnostics::{
Expand Down Expand Up @@ -153,54 +153,13 @@ pub struct IncompatibleArguments {
#[derive(Debug, Diagnostic)]
#[diagnostic(
severity = Error,
message(
description = "{action_kind}",
message({{self.action_kind}})
)
)]
pub struct CheckError {
action_kind: CheckActionKind,

#[category]
category: &'static Category,
}

#[derive(Clone, Copy)]
pub enum CheckActionKind {
Check,
Apply,
}

impl Debug for CheckActionKind {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
std::fmt::Display::fmt(self, f)
}
}

impl Display for CheckActionKind {
fn fmt(&self, fmt: &mut Formatter) -> std::io::Result<()> {
match self {
CheckActionKind::Check => fmt.write_markup(markup! {
"Some errors were emitted while "<Emphasis>"running checks"</Emphasis>
}),
CheckActionKind::Apply => fmt.write_markup(markup! {
"Some errors were emitted while "<Emphasis>"applying fixes"</Emphasis>
}),
}
}
}

impl std::fmt::Display for CheckActionKind {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
CheckActionKind::Check => {
write!(f, "Some errors were emitted while running checks")
}
CheckActionKind::Apply => {
write!(f, "Some errors were emitted while applying fixes")
}
}
}
#[message]
message: MessageAndDescription,
}

#[derive(Debug, Diagnostic)]
Expand Down Expand Up @@ -406,16 +365,51 @@ impl CliDiagnostic {
/// Emitted when errors were emitted while running `check` command
pub fn check_error(category: &'static Category) -> Self {
Self::CheckError(CheckError {
action_kind: CheckActionKind::Check,
category,
message: MessageAndDescription::from(
markup! {
"Some "<Emphasis>"errors"</Emphasis>" were emitted while "<Emphasis>"running checks"</Emphasis>"."
}
.to_owned(),
),
})
}

/// Emitted when warnings were emitted while running `check` command
pub fn check_warnings(category: &'static Category) -> Self {
Self::CheckError(CheckError {
category,
message: MessageAndDescription::from(
markup! {
"Some "<Emphasis>"warnings"</Emphasis>" were emitted while "<Emphasis>"running checks"</Emphasis>"."
}
.to_owned(),
),
})
}

/// Emitted when errors were emitted while apply code fixes
pub fn apply_error(category: &'static Category) -> Self {
Self::CheckError(CheckError {
action_kind: CheckActionKind::Apply,
category,
message: MessageAndDescription::from(
markup! {
"Some "<Emphasis>"errors"</Emphasis>" were emitted while "<Emphasis>"applying fixes"</Emphasis>"."
}
.to_owned(),
),
})
}
/// Emitted when warnings were emitted while apply code fixes
pub fn apply_warnings(category: &'static Category) -> Self {
Self::CheckError(CheckError {
category,
message: MessageAndDescription::from(
markup! {
"Some "<Emphasis>"warnings"</Emphasis>" were emitted while "<Emphasis>"running checks"</Emphasis>"."
}
.to_owned(),
),
})
}

Expand Down
37 changes: 19 additions & 18 deletions crates/rome_cli/src/execute/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ mod traverse;
use crate::cli_options::CliOptions;
use crate::execute::traverse::traverse;
use crate::{CliDiagnostic, CliSession};
use rome_diagnostics::MAXIMUM_DISPLAYABLE_DIAGNOSTICS;
use rome_diagnostics::{category, Category, MAXIMUM_DISPLAYABLE_DIAGNOSTICS};
use rome_fs::RomePath;
use rome_service::workspace::{FeatureName, FixFileMode};
use std::ffi::OsString;
Expand Down Expand Up @@ -145,6 +145,16 @@ impl Execution {
}
}

pub(crate) fn as_diagnostic_category(&self) -> &'static Category {
match self.traversal_mode {
TraversalMode::Check { .. } => category!("check"),
TraversalMode::Lint { .. } => category!("lint"),
TraversalMode::CI => category!("ci"),
TraversalMode::Format { .. } => category!("format"),
TraversalMode::Migrate { .. } => category!("migrate"),
}
}

pub(crate) const fn is_ci(&self) -> bool {
matches!(self.traversal_mode, TraversalMode::CI { .. })
}
Expand Down Expand Up @@ -210,28 +220,19 @@ pub(crate) fn execute_mode(
cli_options: &CliOptions,
paths: Vec<OsString>,
) -> Result<(), CliDiagnostic> {
mode.max_diagnostics = if let Some(max_diagnostics) = cli_options.max_diagnostics {
if max_diagnostics > MAXIMUM_DISPLAYABLE_DIAGNOSTICS {
return Err(CliDiagnostic::overflown_argument(
"--max-diagnostics",
MAXIMUM_DISPLAYABLE_DIAGNOSTICS,
));
}
if cli_options.max_diagnostics > MAXIMUM_DISPLAYABLE_DIAGNOSTICS {
return Err(CliDiagnostic::overflown_argument(
"--max-diagnostics",
MAXIMUM_DISPLAYABLE_DIAGNOSTICS,
));
}

max_diagnostics
} else {
// The commands `rome check` and `rome lint` give a default value of 20.
// In case of other commands that pass here, we limit to 50 to avoid to delay the terminal.
match &mode.traversal_mode {
TraversalMode::Check { .. } | TraversalMode::Lint { .. } => 20,
TraversalMode::CI | TraversalMode::Format { .. } | TraversalMode::Migrate { .. } => 50,
}
};
mode.max_diagnostics = cli_options.max_diagnostics;

// don't do any traversal if there's some content coming from stdin
if let Some((path, content)) = mode.as_stdin_file() {
let rome_path = RomePath::new(path);
std_in::run(session, &mode, rome_path, content.as_str(), cli_options)
std_in::run(session, &mode, rome_path, content.as_str())
} else if let TraversalMode::Migrate {
write,
configuration_path,
Expand Down
12 changes: 3 additions & 9 deletions crates/rome_cli/src/execute/std_in.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
//! In here, there are the operations that run via standard input
//!
use crate::execute::diagnostics::{ContentDiffAdvice, FormatDiffDiagnostic};
use crate::execute::Execution;
use crate::{CliDiagnostic, CliSession};
use rome_console::{markup, ConsoleExt};
use rome_diagnostics::PrintDiagnostic;
use rome_fs::RomePath;

use crate::cli_options::CliOptions;
use crate::execute::diagnostics::{ContentDiffAdvice, FormatDiffDiagnostic};
use rome_diagnostics::{PrintDiagnostic, MAXIMUM_DISPLAYABLE_DIAGNOSTICS};
use rome_service::workspace::{
ChangeFileParams, FeatureName, FeaturesBuilder, FixFileParams, FormatFileParams, Language,
OpenFileParams, OrganizeImportsParams, PullDiagnosticsParams, RuleCategories,
Expand All @@ -20,7 +18,6 @@ pub(crate) fn run<'a>(
mode: &'a Execution,
rome_path: RomePath,
content: &'a str,
cli_options: &CliOptions,
) -> Result<(), CliDiagnostic> {
let workspace = &*session.app.workspace;
let console = &mut *session.app.console;
Expand Down Expand Up @@ -109,10 +106,7 @@ pub(crate) fn run<'a>(
let result = workspace.pull_diagnostics(PullDiagnosticsParams {
categories: RuleCategories::LINT | RuleCategories::SYNTAX,
path: rome_path.clone(),
max_diagnostics: cli_options
.max_diagnostics
.unwrap_or(MAXIMUM_DISPLAYABLE_DIAGNOSTICS)
as u64,
max_diagnostics: mode.max_diagnostics.into(),
})?;
diagnostics.extend(result.diagnostics);
}
Expand Down
29 changes: 22 additions & 7 deletions crates/rome_cli/src/execute/traverse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ pub(crate) fn traverse(
let remaining_diagnostics = AtomicU16::new(max_diagnostics);

let mut errors: usize = 0;
let mut warnings: usize = 0;
let mut report = Report::default();

let duration = thread::scope(|s| {
Expand All @@ -102,6 +103,7 @@ pub(crate) fn traverse(
errors: &mut errors,
report: &mut report,
verbose: cli_options.verbose,
warnings: &mut warnings,
});
})
.expect("failed to spawn console thread");
Expand Down Expand Up @@ -201,16 +203,19 @@ pub(crate) fn traverse(
});
}

let should_exit_on_warnings = warnings > 0 && cli_options.error_on_warnings;
// Processing emitted error diagnostics, exit with a non-zero code
if count.saturating_sub(skipped) == 0 && !cli_options.no_errors_on_unmatched {
Err(CliDiagnostic::no_files_processed())
} else if errors > 0 {
let category = if execution.is_ci() {
category!("ci")
} else {
category!("check")
};
if execution.is_check_apply() {
} else if errors > 0 || should_exit_on_warnings {
let category = execution.as_diagnostic_category();
if should_exit_on_warnings {
if execution.is_check_apply() {
Err(CliDiagnostic::apply_warnings(category))
} else {
Err(CliDiagnostic::check_warnings(category))
}
} else if execution.is_check_apply() {
Err(CliDiagnostic::apply_error(category))
} else {
Err(CliDiagnostic::check_error(category))
Expand Down Expand Up @@ -266,6 +271,9 @@ struct ProcessMessagesOptions<'ctx> {
/// Mutable reference to a boolean flag tracking whether the console thread
/// printed any error-level message
errors: &'ctx mut usize,
/// Mutable reference to a boolean flag tracking whether the console thread
/// printed any warnings-level message
warnings: &'ctx mut usize,
/// Mutable handle to a [Report] instance the console thread should write
/// stats into
report: &'ctx mut Report,
Expand All @@ -287,6 +295,7 @@ fn process_messages(options: ProcessMessagesOptions) {
errors,
report,
verbose,
warnings,
} = options;

let mut paths: HashSet<String> = HashSet::new();
Expand Down Expand Up @@ -345,6 +354,9 @@ fn process_messages(options: ProcessMessagesOptions) {

Message::Error(mut err) => {
let location = err.location();
if err.severity() == Severity::Warning {
*warnings += 1;
}
if let Some(Resource::File(file_path)) = location.resource.as_ref() {
// Retrieves the file name from the file ID cache, if it's a miss
// flush entries from the interner channel until it's found
Expand Down Expand Up @@ -432,6 +444,9 @@ fn process_messages(options: ProcessMessagesOptions) {
if severity == Severity::Error {
*errors += 1;
}
if severity == Severity::Warning {
*warnings += 1;
}

let should_print = printed_diagnostics < max_diagnostics;
if should_print {
Expand Down
Loading