Skip to content

Commit

Permalink
chore(linter): remove commented-out code
Browse files Browse the repository at this point in the history
  • Loading branch information
DonIsaac committed Aug 29, 2024
1 parent c2f27a0 commit 1c556df
Show file tree
Hide file tree
Showing 8 changed files with 452 additions and 85 deletions.
35 changes: 32 additions & 3 deletions apps/oxlint/src/lint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ use std::{env, io::BufWriter, time::Instant};
use ignore::gitignore::Gitignore;
use oxc_diagnostics::{DiagnosticService, GraphicalReportHandler};
use oxc_linter::{
partial_loader::LINT_PARTIAL_LOADER_EXT, LintService, LintServiceOptions, Linter, OxlintOptions,
partial_loader::LINT_PARTIAL_LOADER_EXT, InvalidFilterKind, LintFilter, LintService,
LintServiceOptions, Linter, OxlintOptions,
};
use oxc_span::VALID_EXTENSIONS;

Expand Down Expand Up @@ -35,7 +36,7 @@ impl Runner for LintRunner {

let LintCommand {
paths,
filter,
filter: filter_args,
basic_options,
warning_options,
ignore_options,
Expand All @@ -46,6 +47,34 @@ impl Runner for LintRunner {
..
} = self.options;

let mut filters = Vec::with_capacity(filter_args.len());
for (severity, filter_arg) in filter_args {
match LintFilter::new(severity, filter_arg) {
Ok(filter) => {
filters.push(filter);
}
Err(InvalidFilterKind::Empty) => {
return CliRunResult::InvalidOptions {
message: format!("Cannot {severity} an empty filter."),
};
}
Err(InvalidFilterKind::PluginMissing(filter)) => {
return CliRunResult::InvalidOptions {
message: format!(
"Failed to {severity} filter {filter}: Plugin name is missing. Expected <plugin>/<rule>"
),
};
}
Err(InvalidFilterKind::RuleMissing(filter)) => {
return CliRunResult::InvalidOptions {
message: format!(
"Failed to {severity} filter {filter}: Rule name is missing. Expected <plugin>/<rule>"
),
};
}
}
}

let mut paths = paths;
let provided_path_count = paths.len();
let now = Instant::now();
Expand Down Expand Up @@ -93,7 +122,7 @@ impl Runner for LintRunner {

let cwd = std::env::current_dir().unwrap().into_boxed_path();
let lint_options = OxlintOptions::default()
.with_filter(filter)
.with_filter(filters)
.with_config_path(basic_options.config)
.with_fix(fix_options.fix_kind())
.with_react_plugin(enable_plugins.react_plugin)
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ pub use crate::{
context::LintContext,
fixer::FixKind,
frameworks::FrameworkFlags,
options::{AllowWarnDeny, OxlintOptions},
options::{AllowWarnDeny, InvalidFilterKind, LintFilter, LintFilterKind, OxlintOptions},
rule::{RuleCategory, RuleFixMeta, RuleMeta, RuleWithSeverity},
service::{LintService, LintServiceOptions},
};
Expand Down
15 changes: 14 additions & 1 deletion crates/oxc_linter/src/options/allow_warn_deny.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::convert::From;
use std::{convert::From, fmt};

use oxc_diagnostics::{OxcDiagnostic, Severity};
use schemars::{schema::SchemaObject, JsonSchema};
Expand All @@ -19,6 +19,14 @@ impl AllowWarnDeny {
pub fn is_allow(self) -> bool {
self == Self::Allow
}

pub fn as_str(self) -> &'static str {
match self {
Self::Allow => "allow",
Self::Warn => "warn",
Self::Deny => "deny",
}
}
}

impl TryFrom<&str> for AllowWarnDeny {
Expand Down Expand Up @@ -64,6 +72,11 @@ impl TryFrom<&Number> for AllowWarnDeny {
}
}
}
impl fmt::Display for AllowWarnDeny {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
self.as_str().fmt(f)
}
}

impl JsonSchema for AllowWarnDeny {
fn schema_name() -> String {
Expand Down
265 changes: 265 additions & 0 deletions crates/oxc_linter/src/options/filter.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,265 @@
use crate::RuleCategory;

use super::{plugins::LintPlugins, AllowWarnDeny};
use std::{borrow::Cow, fmt};

#[derive(Debug, Clone)]
#[cfg_attr(test, derive(PartialEq))]
pub struct LintFilter(AllowWarnDeny, LintFilterKind);
impl LintFilter {
/// # Errors
///
/// If `kind` is an empty string, or is a `<plugin>/<rule>` filter but is missing either the
/// plugin or the rule.
pub fn new<F: TryInto<LintFilterKind>>(
severity: AllowWarnDeny,
kind: F,
) -> Result<Self, <F as TryInto<LintFilterKind>>::Error> {
Ok(Self(severity, kind.try_into()?))
}

#[must_use]
pub fn allow<F: Into<LintFilterKind>>(kind: F) -> Self {
Self(AllowWarnDeny::Allow, kind.into())
}

#[must_use]
pub fn warn<F: Into<LintFilterKind>>(kind: F) -> Self {
Self(AllowWarnDeny::Warn, kind.into())
}

#[must_use]
pub fn deny<F: Into<LintFilterKind>>(kind: F) -> Self {
Self(AllowWarnDeny::Deny, kind.into())
}

#[inline]
pub fn severity(&self) -> AllowWarnDeny {
self.0
}

#[inline]
pub fn kind(&self) -> &LintFilterKind {
&self.1
}
}

impl Default for LintFilter {
fn default() -> Self {
Self(AllowWarnDeny::Warn, LintFilterKind::Category(RuleCategory::Correctness))
}
}

impl From<LintFilter> for (AllowWarnDeny, LintFilterKind) {
fn from(val: LintFilter) -> Self {
(val.0, val.1)
}
}
impl<'a> From<&'a LintFilter> for (AllowWarnDeny, &'a LintFilterKind) {
fn from(val: &'a LintFilter) -> Self {
(val.0, &val.1)
}
}

#[derive(Debug, Clone)]
#[cfg_attr(test, derive(PartialEq))]
pub enum LintFilterKind {
Generic(Cow<'static, str>),
/// e.g. `no-const-assign` or `eslint/no-const-assign`
Rule(LintPlugins, Cow<'static, str>),
/// e.g. `correctness`
Category(RuleCategory),
// TODO: plugin + category? e.g `-A react:correctness`
}

impl LintFilterKind {
/// # Errors
///
/// If `filter` is an empty string, or is a `<plugin>/<rule>` filter but is missing either the
/// plugin or the rule.
pub fn parse(filter: Cow<'static, str>) -> Result<Self, InvalidFilterKind> {
if filter.is_empty() {
return Err(InvalidFilterKind::Empty);
}

if filter.contains('/') {
// this is an unfortunate amount of code duplication, but it needs to be done for
// `filter` to live long enough to avoid a String allocation for &'static str
let (plugin, rule) = match filter {
Cow::Borrowed(filter) => {
let mut parts = filter.splitn(2, '/');

let plugin = parts
.next()
.ok_or(InvalidFilterKind::PluginMissing(Cow::Borrowed(filter)))?;
if plugin.is_empty() {
return Err(InvalidFilterKind::PluginMissing(Cow::Borrowed(filter)));
}

let rule = parts
.next()
.ok_or(InvalidFilterKind::RuleMissing(Cow::Borrowed(filter)))?;
if rule.is_empty() {
return Err(InvalidFilterKind::RuleMissing(Cow::Borrowed(filter)));
}

(LintPlugins::from(plugin), Cow::Borrowed(rule))
}
Cow::Owned(filter) => {
let mut parts = filter.splitn(2, '/');

let plugin = parts
.next()
.ok_or_else(|| InvalidFilterKind::PluginMissing(filter.clone().into()))?;
if plugin.is_empty() {
return Err(InvalidFilterKind::PluginMissing(filter.into()));
}

let rule = parts
.next()
.ok_or_else(|| InvalidFilterKind::RuleMissing(filter.clone().into()))?;
if rule.is_empty() {
return Err(InvalidFilterKind::RuleMissing(filter.into()));
}

(LintPlugins::from(plugin), Cow::Owned(rule.to_string()))
}
};
Ok(LintFilterKind::Rule(plugin, rule))
} else {
match RuleCategory::try_from(filter.as_ref()) {
Ok(category) => Ok(LintFilterKind::Category(category)),
Err(()) => Ok(LintFilterKind::Generic(filter)),
}
}
}
}

impl TryFrom<String> for LintFilterKind {
type Error = InvalidFilterKind;

#[inline]
fn try_from(filter: String) -> Result<Self, Self::Error> {
Self::parse(Cow::Owned(filter))
}
}

impl TryFrom<&'static str> for LintFilterKind {
type Error = InvalidFilterKind;

#[inline]
fn try_from(filter: &'static str) -> Result<Self, Self::Error> {
Self::parse(Cow::Borrowed(filter))
}
}

impl TryFrom<Cow<'static, str>> for LintFilterKind {
type Error = InvalidFilterKind;

#[inline]
fn try_from(filter: Cow<'static, str>) -> Result<Self, Self::Error> {
Self::parse(filter)
}
}

impl From<RuleCategory> for LintFilterKind {
#[inline]
fn from(category: RuleCategory) -> Self {
LintFilterKind::Category(category)
}
}

#[derive(Debug, Clone, PartialEq, Eq)]
pub enum InvalidFilterKind {
Empty,
PluginMissing(Cow<'static, str>),
RuleMissing(Cow<'static, str>),
}

impl fmt::Display for InvalidFilterKind {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self {
Self::Empty => "Filter cannot be empty.".fmt(f),
Self::PluginMissing(filter) => {
write!(
f,
"Filter '{filter}' must match <plugin>/<rule> but is missing a plugin name."
)
}
Self::RuleMissing(filter) => {
write!(
f,
"Filter '{filter}' must match <plugin>/<rule> but is missing a rule name."
)
}
}
}
}

impl std::error::Error for InvalidFilterKind {}

#[cfg(test)]
mod test {
use super::*;

#[test]
fn test_from_category() {
let correctness: LintFilter = LintFilter::new(AllowWarnDeny::Warn, "correctness").unwrap();
assert_eq!(correctness.severity(), AllowWarnDeny::Warn);
assert!(
matches!(correctness.kind(), LintFilterKind::Category(RuleCategory::Correctness)),
"{:?}",
correctness.kind()
);
}

#[test]
fn test_eslint_deny() {
let filter = LintFilter::deny(LintFilterKind::try_from("no-const-assign").unwrap());
assert_eq!(filter.severity(), AllowWarnDeny::Deny);
assert_eq!(filter.kind(), &LintFilterKind::Generic("no-const-assign".into()));

let filter = LintFilter::deny(LintFilterKind::try_from("eslint/no-const-assign").unwrap());
assert_eq!(filter.severity(), AllowWarnDeny::Deny);
assert_eq!(
filter.kind(),
&LintFilterKind::Rule(LintPlugins::from("eslint"), "no-const-assign".into())
);
assert!(matches!(filter.kind(), LintFilterKind::Rule(_, _)));
}

#[test]
fn test_parse() {
let test_cases: Vec<(&'static str, LintFilterKind)> = vec![
("import/namespace", LintFilterKind::Rule(LintPlugins::IMPORT, "namespace".into())),
(
"react-hooks/exhaustive-deps",
LintFilterKind::Rule(LintPlugins::REACT, "exhaustive-deps".into()),
),
// categories
("nursery", LintFilterKind::Category("nursery".try_into().unwrap())),
("perf", LintFilterKind::Category("perf".try_into().unwrap())),
// misc
("not-a-valid-filter", LintFilterKind::Generic("not-a-valid-filter".into())),
];

for (input, expected) in test_cases {
let actual = LintFilterKind::try_from(input).unwrap();
assert_eq!(actual, expected, "input: {input}");
}
}

#[test]
fn test_parse_invalid() {
let test_cases = vec!["/rules-of-hooks", "import/", "", "/", "//"];

for input in test_cases {
let actual = LintFilterKind::parse(Cow::Borrowed(input));
assert!(
actual.is_err(),
"input '{input}' produced filter '{:?}' but it should have errored",
actual.unwrap()
);
}
}
}
Loading

0 comments on commit 1c556df

Please sign in to comment.