Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

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
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}");
Boshen marked this conversation as resolved.
Show resolved Hide resolved
ExitCode::FAILURE
}
| Self::LintMaxWarningsExceeded
| Self::InvalidOptionConfig
| Self::InvalidOptionTsConfig
| Self::InvalidOptionSeverityWithoutFilter
| Self::InvalidOptionSeverityWithoutPluginName
| Self::InvalidOptionSeverityWithoutRuleName => ExitCode::FAILURE,
}
}
}
8 changes: 8 additions & 0 deletions apps/oxlint/src/snapshots/_--tsconfig [email protected]
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
Loading