Skip to content

Commit

Permalink
refactor(oxlint): move output from CliRunResult::InvalidOption to o…
Browse files Browse the repository at this point in the history
…utside and use more Enums for different invalid options (#8778)

Now we can choose which exit code to use for a specific (unexpected) result
  • Loading branch information
Sysix committed Jan 30, 2025
1 parent fe45bee commit 9731c56
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 52 deletions.
71 changes: 41 additions & 30 deletions apps/oxlint/src/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,12 @@ impl Runner for LintRunner {

let filter = match Self::get_filters(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
Expand All @@ -99,7 +104,13 @@ impl Runner for LintRunner {
Self::find_oxlint_config(&self.cwd, basic_options.config.as_ref());

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();
Expand Down Expand Up @@ -178,9 +189,13 @@ impl Runner for LintRunner {
let handler = GraphicalReportHandler::new();
let mut err = String::new();
handler.render_report(&mut err, &diagnostic).unwrap();
return CliRunResult::InvalidOptions {
message: format!("Failed to parse configuration file.\n{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;
}
};

Expand All @@ -193,11 +208,12 @@ impl Runner for LintRunner {
options = options.with_tsconfig(path);
} else {
let path = if path.is_relative() { options.cwd().join(path) } else { path.clone() };
return CliRunResult::InvalidOptions {
message: format!(
"The tsconfig file {path:?} does not exist, Please provide a valid tsconfig file.",
),
};
stdout.write_all(format!(
"The tsconfig file {path:?} does not exist, Please provide a valid tsconfig file.\n",
).as_bytes()).or_else(Self::check_for_writer_error).unwrap();
stdout.flush().unwrap();

return CliRunResult::InvalidOptionTsConfig;
}
}

Expand Down Expand Up @@ -262,7 +278,7 @@ impl LintRunner {
// in one place.
fn get_filters(
filters_arg: Vec<(AllowWarnDeny, String)>,
) -> Result<Vec<LintFilter>, CliRunResult> {
) -> Result<Vec<LintFilter>, (CliRunResult, String)> {
let mut filters = Vec::with_capacity(filters_arg.len());

for (severity, filter_arg) in filters_arg {
Expand All @@ -271,23 +287,22 @@ impl LintRunner {
filters.push(filter);
}
Err(InvalidFilterKind::Empty) => {
return Err(CliRunResult::InvalidOptions {
message: format!("Cannot {severity} an empty filter."),
});
return Err((
CliRunResult::InvalidOptionSeverityWithoutFilter,
format!("Cannot {severity} an empty filter.\n"),
));
}
Err(InvalidFilterKind::PluginMissing(filter)) => {
return Err(CliRunResult::InvalidOptions {
message: format!(
"Failed to {severity} filter {filter}: Plugin name is missing. Expected <plugin>/<rule>"
return Err((CliRunResult::InvalidOptionSeverityWithoutPluginName, format!(
"Failed to {severity} filter {filter}: Plugin name is missing. Expected <plugin>/<rule>\n"
),
});
));
}
Err(InvalidFilterKind::RuleMissing(filter)) => {
return Err(CliRunResult::InvalidOptions {
message: format!(
"Failed to {severity} filter {filter}: Rule name is missing. Expected <plugin>/<rule>"
return Err((CliRunResult::InvalidOptionSeverityWithoutRuleName, format!(
"Failed to {severity} filter {filter}: Rule name is missing. Expected <plugin>/<rule>\n"
),
});
));
}
}
}
Expand All @@ -296,10 +311,10 @@ impl LintRunner {
}

// 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 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 {
let full_path = cwd.join(config_path);
return match Oxlintrc::from_file(&full_path) {
Expand All @@ -308,9 +323,7 @@ impl LintRunner {
let handler = GraphicalReportHandler::new();
let mut err = String::new();
handler.render_report(&mut err, &diagnostic).unwrap();
return Err(CliRunResult::InvalidOptions {
message: format!("Failed to parse configuration file.\n{err}"),
});
return Err(err);
}
};
}
Expand Down Expand Up @@ -568,9 +581,7 @@ mod test {
Tester::new().test(&["--tsconfig", "fixtures/tsconfig/tsconfig.json"]);

// failed
assert!(Tester::new()
.get_invalid_option_result(&["--tsconfig", "oxc/tsconfig.json"])
.contains("oxc/tsconfig.json\" does not exist, Please provide a valid tsconfig file."));
Tester::new().test_and_snapshot(&["--tsconfig", "oxc/tsconfig.json"]);
}

#[test]
Expand Down
17 changes: 11 additions & 6 deletions apps/oxlint/src/result.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ use std::process::{ExitCode, Termination};
#[derive(Debug)]
pub enum CliRunResult {
None,
InvalidOptions { message: String },
InvalidOptionConfig,
InvalidOptionTsConfig,
InvalidOptionSeverityWithoutFilter,
InvalidOptionSeverityWithoutPluginName,
InvalidOptionSeverityWithoutRuleName,
LintSucceeded,
LintFoundErrors,
LintMaxWarningsExceeded,
Expand All @@ -27,11 +31,12 @@ impl Termination for CliRunResult {
Self::ConfigFileInitFailed
| Self::LintFoundErrors
| Self::LintNoWarningsAllowed
| Self::LintMaxWarningsExceeded => ExitCode::FAILURE,
Self::InvalidOptions { message } => {
println!("Invalid Options: {message}");
ExitCode::FAILURE
}
| Self::LintMaxWarningsExceeded
| Self::InvalidOptionConfig
| Self::InvalidOptionTsConfig
| Self::InvalidOptionSeverityWithoutFilter
| Self::InvalidOptionSeverityWithoutPluginName
| Self::InvalidOptionSeverityWithoutRuleName => ExitCode::FAILURE,
}
}
}
Original file line number Diff line number Diff line change
@@ -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.
25 changes: 9 additions & 16 deletions apps/oxlint/src/tester.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#[cfg(test)]
use crate::cli::{lint_command, CliRunResult, LintRunner};
use crate::cli::{lint_command, LintRunner};
#[cfg(test)]
use crate::runner::Runner;
#[cfg(test)]
Expand Down Expand Up @@ -38,28 +38,14 @@ impl Tester {
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]) {
self.test_and_snapshot_multiple(&[args]);
}

pub fn test_and_snapshot_multiple(&self, multiple_args: &[&[&str]]) {
let mut output: Vec<u8> = Vec::new();
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 {
let options = lint_command().run_inner(*args).unwrap();
Expand All @@ -85,6 +71,13 @@ impl Tester {
let output_string = &String::from_utf8(output).unwrap();
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 =
multiple_args.iter().map(|args| args.join(" ")).collect::<Vec<String>>().join(" ");

Expand Down

0 comments on commit 9731c56

Please sign in to comment.