refactor(oxlint): move output from CliRunResult::InvalidOption to outside and use more Enums for different invalid options (#8778)

Now we can choose which exit code to use for a specific (unexpected) result
This commit is contained in:
Sysix 2025-01-30 01:43:42 +00:00
parent fe45bee0c2
commit 9731c56b6a
4 changed files with 69 additions and 52 deletions

View file

@ -86,7 +86,12 @@ impl Runner for LintRunner {
let filter = match Self::get_filters(filter) { let filter = match Self::get_filters(filter) {
Ok(filter) => filter, Ok(filter) => filter,
Err(e) => return e, Err((result, message)) => {
stdout.write_all(message.as_bytes()).or_else(Self::check_for_writer_error).unwrap();
stdout.flush().unwrap();
return result;
}
}; };
let extensions = VALID_EXTENSIONS let extensions = VALID_EXTENSIONS
@ -99,7 +104,13 @@ impl Runner for LintRunner {
Self::find_oxlint_config(&self.cwd, basic_options.config.as_ref()); Self::find_oxlint_config(&self.cwd, basic_options.config.as_ref());
if let Err(err) = config_search_result { if let Err(err) = config_search_result {
return err; stdout
.write_all(format!("Failed to parse configuration file.\n{err}\n").as_bytes())
.or_else(Self::check_for_writer_error)
.unwrap();
stdout.flush().unwrap();
return CliRunResult::InvalidOptionConfig;
} }
let mut oxlintrc = config_search_result.unwrap(); let mut oxlintrc = config_search_result.unwrap();
@ -178,9 +189,13 @@ impl Runner for LintRunner {
let handler = GraphicalReportHandler::new(); let handler = GraphicalReportHandler::new();
let mut err = String::new(); let mut err = String::new();
handler.render_report(&mut err, &diagnostic).unwrap(); handler.render_report(&mut err, &diagnostic).unwrap();
return CliRunResult::InvalidOptions { stdout
message: format!("Failed to parse configuration file.\n{err}"), .write_all(format!("Failed to parse configuration file.\n{err}\n").as_bytes())
}; .or_else(Self::check_for_writer_error)
.unwrap();
stdout.flush().unwrap();
return CliRunResult::InvalidOptionConfig;
} }
}; };
@ -193,11 +208,12 @@ impl Runner for LintRunner {
options = options.with_tsconfig(path); options = options.with_tsconfig(path);
} else { } else {
let path = if path.is_relative() { options.cwd().join(path) } else { path.clone() }; let path = if path.is_relative() { options.cwd().join(path) } else { path.clone() };
return CliRunResult::InvalidOptions { stdout.write_all(format!(
message: format!( "The tsconfig file {path:?} does not exist, Please provide a valid tsconfig file.\n",
"The tsconfig file {path:?} does not exist, Please provide a valid tsconfig file.", ).as_bytes()).or_else(Self::check_for_writer_error).unwrap();
), stdout.flush().unwrap();
};
return CliRunResult::InvalidOptionTsConfig;
} }
} }
@ -262,7 +278,7 @@ impl LintRunner {
// in one place. // in one place.
fn get_filters( fn get_filters(
filters_arg: Vec<(AllowWarnDeny, String)>, filters_arg: Vec<(AllowWarnDeny, String)>,
) -> Result<Vec<LintFilter>, CliRunResult> { ) -> Result<Vec<LintFilter>, (CliRunResult, String)> {
let mut filters = Vec::with_capacity(filters_arg.len()); let mut filters = Vec::with_capacity(filters_arg.len());
for (severity, filter_arg) in filters_arg { for (severity, filter_arg) in filters_arg {
@ -271,23 +287,22 @@ impl LintRunner {
filters.push(filter); filters.push(filter);
} }
Err(InvalidFilterKind::Empty) => { Err(InvalidFilterKind::Empty) => {
return Err(CliRunResult::InvalidOptions { return Err((
message: format!("Cannot {severity} an empty filter."), CliRunResult::InvalidOptionSeverityWithoutFilter,
}); format!("Cannot {severity} an empty filter.\n"),
));
} }
Err(InvalidFilterKind::PluginMissing(filter)) => { Err(InvalidFilterKind::PluginMissing(filter)) => {
return Err(CliRunResult::InvalidOptions { return Err((CliRunResult::InvalidOptionSeverityWithoutPluginName, format!(
message: format!( "Failed to {severity} filter {filter}: Plugin name is missing. Expected <plugin>/<rule>\n"
"Failed to {severity} filter {filter}: Plugin name is missing. Expected <plugin>/<rule>"
), ),
}); ));
} }
Err(InvalidFilterKind::RuleMissing(filter)) => { Err(InvalidFilterKind::RuleMissing(filter)) => {
return Err(CliRunResult::InvalidOptions { return Err((CliRunResult::InvalidOptionSeverityWithoutRuleName, format!(
message: format!( "Failed to {severity} filter {filter}: Rule name is missing. Expected <plugin>/<rule>\n"
"Failed to {severity} filter {filter}: Rule name is missing. Expected <plugin>/<rule>"
), ),
}); ));
} }
} }
} }
@ -296,10 +311,10 @@ impl LintRunner {
} }
// finds the oxlint config // finds the oxlint config
// when config is provided, but not found, an CliRunResult is returned, else the oxlintrc config file is returned // when config is provided, but not found, an String with the formatted error is returned, else the oxlintrc config file is returned
// when no config is provided, it will search for the default file names in the current working directory // when no config is provided, it will search for the default file names in the current working directory
// when no file is found, the default configuration is returned // when no file is found, the default configuration is returned
fn find_oxlint_config(cwd: &Path, config: Option<&PathBuf>) -> Result<Oxlintrc, CliRunResult> { fn find_oxlint_config(cwd: &Path, config: Option<&PathBuf>) -> Result<Oxlintrc, String> {
if let Some(config_path) = config { if let Some(config_path) = config {
let full_path = cwd.join(config_path); let full_path = cwd.join(config_path);
return match Oxlintrc::from_file(&full_path) { return match Oxlintrc::from_file(&full_path) {
@ -308,9 +323,7 @@ impl LintRunner {
let handler = GraphicalReportHandler::new(); let handler = GraphicalReportHandler::new();
let mut err = String::new(); let mut err = String::new();
handler.render_report(&mut err, &diagnostic).unwrap(); handler.render_report(&mut err, &diagnostic).unwrap();
return Err(CliRunResult::InvalidOptions { return Err(err);
message: format!("Failed to parse configuration file.\n{err}"),
});
} }
}; };
} }
@ -568,9 +581,7 @@ mod test {
Tester::new().test(&["--tsconfig", "fixtures/tsconfig/tsconfig.json"]); Tester::new().test(&["--tsconfig", "fixtures/tsconfig/tsconfig.json"]);
// failed // failed
assert!(Tester::new() Tester::new().test_and_snapshot(&["--tsconfig", "oxc/tsconfig.json"]);
.get_invalid_option_result(&["--tsconfig", "oxc/tsconfig.json"])
.contains("oxc/tsconfig.json\" does not exist, Please provide a valid tsconfig file."));
} }
#[test] #[test]

View file

@ -3,7 +3,11 @@ use std::process::{ExitCode, Termination};
#[derive(Debug)] #[derive(Debug)]
pub enum CliRunResult { pub enum CliRunResult {
None, None,
InvalidOptions { message: String }, InvalidOptionConfig,
InvalidOptionTsConfig,
InvalidOptionSeverityWithoutFilter,
InvalidOptionSeverityWithoutPluginName,
InvalidOptionSeverityWithoutRuleName,
LintSucceeded, LintSucceeded,
LintFoundErrors, LintFoundErrors,
LintMaxWarningsExceeded, LintMaxWarningsExceeded,
@ -27,11 +31,12 @@ impl Termination for CliRunResult {
Self::ConfigFileInitFailed Self::ConfigFileInitFailed
| Self::LintFoundErrors | Self::LintFoundErrors
| Self::LintNoWarningsAllowed | Self::LintNoWarningsAllowed
| Self::LintMaxWarningsExceeded => ExitCode::FAILURE, | Self::LintMaxWarningsExceeded
Self::InvalidOptions { message } => { | Self::InvalidOptionConfig
println!("Invalid Options: {message}"); | Self::InvalidOptionTsConfig
ExitCode::FAILURE | Self::InvalidOptionSeverityWithoutFilter
} | Self::InvalidOptionSeverityWithoutPluginName
| Self::InvalidOptionSeverityWithoutRuleName => ExitCode::FAILURE,
} }
} }
} }

View file

@ -0,0 +1,8 @@
---
source: apps/oxlint/src/tester.rs
---
##########
arguments: --tsconfig oxc/tsconfig.json
working directory:
----------
The tsconfig file "<cwd>/oxc/tsconfig.json" does not exist, Please provide a valid tsconfig file.

View file

@ -1,5 +1,5 @@
#[cfg(test)] #[cfg(test)]
use crate::cli::{lint_command, CliRunResult, LintRunner}; use crate::cli::{lint_command, LintRunner};
#[cfg(test)] #[cfg(test)]
use crate::runner::Runner; use crate::runner::Runner;
#[cfg(test)] #[cfg(test)]
@ -38,20 +38,6 @@ impl Tester {
let _ = LintRunner::new(options).with_cwd(self.cwd.clone()).run(&mut output); let _ = LintRunner::new(options).with_cwd(self.cwd.clone()).run(&mut output);
} }
pub fn get_invalid_option_result(&self, args: &[&str]) -> String {
let mut new_args = vec!["--silent"];
new_args.extend(args);
let options = lint_command().run_inner(new_args.as_slice()).unwrap();
let mut output = Vec::new();
match LintRunner::new(options).with_cwd(self.cwd.clone()).run(&mut output) {
CliRunResult::InvalidOptions { message } => message,
other => {
panic!("Expected InvalidOptions, got {other:?}");
}
}
}
pub fn test_and_snapshot(&self, args: &[&str]) { pub fn test_and_snapshot(&self, args: &[&str]) {
self.test_and_snapshot_multiple(&[args]); self.test_and_snapshot_multiple(&[args]);
} }
@ -59,7 +45,7 @@ impl Tester {
pub fn test_and_snapshot_multiple(&self, multiple_args: &[&[&str]]) { pub fn test_and_snapshot_multiple(&self, multiple_args: &[&[&str]]) {
let mut output: Vec<u8> = Vec::new(); let mut output: Vec<u8> = Vec::new();
let current_cwd = std::env::current_dir().unwrap(); let current_cwd = std::env::current_dir().unwrap();
let relative_dir = self.cwd.strip_prefix(current_cwd).unwrap_or(&self.cwd); let relative_dir = self.cwd.strip_prefix(&current_cwd).unwrap_or(&self.cwd);
for args in multiple_args { for args in multiple_args {
let options = lint_command().run_inner(*args).unwrap(); let options = lint_command().run_inner(*args).unwrap();
@ -85,6 +71,13 @@ impl Tester {
let output_string = &String::from_utf8(output).unwrap(); let output_string = &String::from_utf8(output).unwrap();
let output_string = regex.replace_all(output_string, "<variable>ms"); let output_string = regex.replace_all(output_string, "<variable>ms");
// do not output the current working directory, each machine has a different one
let cwd_string = current_cwd.to_str().unwrap();
#[allow(clippy::disallowed_methods)]
let cwd_string = cwd_string.replace('\\', "/");
#[allow(clippy::disallowed_methods)]
let output_string = output_string.replace(&cwd_string, "<cwd>");
let full_args_list = let full_args_list =
multiple_args.iter().map(|args| args.join(" ")).collect::<Vec<String>>().join(" "); multiple_args.iter().map(|args| args.join(" ")).collect::<Vec<String>>().join(" ");