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.

Sign up for GitHub

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

fix(linter): fix file name checking behavior of unicorn/filename-case #6463

Merged
merged 1 commit into from
Oct 13, 2024
Merged

fix(linter): fix file name checking behavior of unicorn/filename-case #6463

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
262 changes: 205 additions & 57 deletions crates/oxc_linter/src/rules/unicorn/filename_case.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,46 +1,86 @@
use convert_case::{Case, Casing};
use convert_case::{Boundary, Case, Converter};
use oxc_diagnostics::OxcDiagnostic;
use oxc_macros::declare_oxc_lint;
use oxc_span::Span;
use serde_json::Value;

use crate::{context::LintContext, rule::Rule};

fn filename_case_diagnostic(span: Span, case_name: &str) -> OxcDiagnostic {
OxcDiagnostic::warn(format!("Filename should not be in {case_name} case")).with_label(span)
fn join_strings_disjunction(strings: &[String]) -> String {
let mut list = String::new();
for (i, s) in strings.iter().enumerate() {
if i == 0 {
list.push_str(s);
} else if i == strings.len() - 1 {
list.push_str(&format!(", or {s}"));
} else {
list.push_str(&format!(", {s}"));
}
}
list
}

fn filename_case_diagnostic(valid_cases: &[&str]) -> OxcDiagnostic {
let case_names = valid_cases.iter().map(|s| format!("{s} case")).collect::<Vec<_>>();
let message = format!("Filename should be in {}", join_strings_disjunction(&case_names));
OxcDiagnostic::warn(message).with_label(Span::default())
}

#[derive(Debug, Clone)]
#[allow(clippy::struct_field_names)]
pub struct FilenameCase {
/// Whether kebab case is allowed.
kebab_case: bool,
/// Whether camel case is allowed.
camel_case: bool,
/// Whether snake case is allowed.
snake_case: bool,
/// Whether pascal case is allowed.
pascal_case: bool,
underscore_case: bool,
}

impl Default for FilenameCase {
fn default() -> Self {
Self {
kebab_case: false,
camel_case: true,
snake_case: false,
pascal_case: true,
underscore_case: false,
}
Self { kebab_case: true, camel_case: false, snake_case: false, pascal_case: false }
}
}

declare_oxc_lint!(
/// ### What it does
///
/// Enforce a case style for filenames.
/// Enforces specific case styles for filenames. By default, kebab case is enforced.
///
/// ### Why is this bad?
///
/// ### Example
/// ```
/// Inconsistent file naming conventions can make it harder to locate files or to create new ones.
///
/// ### Cases
///
/// Examples of **correct** filenames for each case:
///
/// #### `kebabCase`
///
/// - `some-file-name.js`
/// - `some-file-name.test.js`
/// - `some-file-name.test-utils.js`
///
/// #### `camelCase`
///
/// - `someFileName.js`
/// - `someFileName.test.js`
/// - `someFileName.testUtils.js`
///
/// #### `snakeCase`
///
/// - `some_file_name.js`
/// - `some_file_name.test.js`
/// - `some_file_name.test_utils.js`
///
/// #### `pascalCase`
///
/// - `SomeFileName.js`
/// - `SomeFileName.Test.js`
/// - `SomeFileName.TestUtils.js`
FilenameCase,
style
);
Expand All @@ -51,37 +91,27 @@ impl Rule for FilenameCase {
return Self::default();
};

if let Some(Value::String(s)) = case_type.get("case") {
return match s.as_str() {
"kebabCase" => Self { kebab_case: true, ..Self::default() },
"camelCase" => Self { camel_case: true, ..Self::default() },
"snakeCase" => Self { snake_case: true, ..Self::default() },
"pascalCase" => Self { pascal_case: true, ..Self::default() },
"underscoreCase" => Self { underscore_case: true, ..Self::default() },
_ => Self::default(),
};
}
let off =
Self { kebab_case: false, camel_case: false, snake_case: false, pascal_case: false };

if let Some(Value::String(s)) = case_type.get("case") {
return match s.as_str() {
"kebabCase" => Self { kebab_case: true, ..Self::default() },
"camelCase" => Self { camel_case: true, ..Self::default() },
"snakeCase" => Self { snake_case: true, ..Self::default() },
"pascalCase" => Self { pascal_case: true, ..Self::default() },
"underscoreCase" => Self { underscore_case: true, ..Self::default() },
"kebabCase" => Self { kebab_case: true, ..off },
"camelCase" => Self { camel_case: true, ..off },
"snakeCase" => Self { snake_case: true, ..off },
"pascalCase" => Self { pascal_case: true, ..off },
_ => Self::default(),
};
}

if let Some(Value::Object(map)) = case_type.get("cases") {
let mut filename_case = Self::default();
let mut filename_case = off;
for (key, value) in map {
match (key.as_str(), value) {
("kebabCase", Value::Bool(b)) => filename_case.kebab_case = *b,
("camelCase", Value::Bool(b)) => filename_case.camel_case = *b,
("snakeCase", Value::Bool(b)) => filename_case.snake_case = *b,
("pascalCase", Value::Bool(b)) => filename_case.pascal_case = *b,
("underscoreCase", Value::Bool(b)) => filename_case.underscore_case = *b,
_ => (),
}
}
Expand All @@ -96,27 +126,29 @@ impl Rule for FilenameCase {
return;
};

let mut case_name = None;
// get filename up to the first dot, or the whole filename if there is no dot
let filename = filename.split('.').next().unwrap_or(filename);
// ignore all leading and trailing underscores
let filename = filename.trim_matches('_');

let cases = [
(Case::Kebab, "kebab", self.kebab_case),
(Case::Camel, "camel", self.camel_case),
(Case::Snake, "snake", self.snake_case),
(Case::Pascal, "pascal", self.pascal_case),
(Case::Pascal, "underscore", self.underscore_case),
(self.camel_case, Case::Camel, "camel"),
(self.kebab_case, Case::Kebab, "kebab"),
(self.snake_case, Case::Snake, "snake"),
(self.pascal_case, Case::Pascal, "pascal"),
];
let mut enabled_cases = cases.iter().filter(|(enabled, _, _)| *enabled);

for (case, name, condition) in cases {
if filename.to_case(case) == filename {
if condition {
return;
}
case_name.replace(name);
}
}

if let Some(case_name) = case_name {
ctx.diagnostic(filename_case_diagnostic(Span::default(), case_name));
if !enabled_cases.any(|(_, case, _)| {
let converter =
Converter::new().remove_boundaries(&[Boundary::LowerDigit, Boundary::DigitLower]);
converter.to_case(*case).convert(filename) == filename
}) {
let case_names = cases
.iter()
.filter_map(|(enabled, _, name)| if *enabled { Some(*name) } else { None })
.collect::<Vec<_>>();
ctx.diagnostic(filename_case_diagnostic(&case_names));
}
}
}
Expand All @@ -127,18 +159,134 @@ fn test() {

use crate::tester::Tester;

fn test_case(
path: &'static str,
casing: &'static str,
) -> (&'static str, Option<Value>, Option<Value>, Option<PathBuf>) {
(
"",
if casing.is_empty() {
None
} else {
Some(serde_json::json!(
[{ "case": casing }]
))
},
None,
Some(PathBuf::from(path)),
)
}

fn test_cases<const N: usize>(
path: &'static str,
casings: [&'static str; N],
) -> (&'static str, Option<Value>, Option<Value>, Option<PathBuf>) {
(
"",
if casings.is_empty() {
None
} else {
let mut map = serde_json::Map::new();
// turn ["camelCase", "snakeCase"] into [{ "cases": { "camelCase": true, "snakeCase": true } }]
for casing in casings {
map.insert(casing.to_string(), Value::Bool(true));
}
Some(serde_json::json!([{ "cases": map }]))
},
None,
Some(PathBuf::from(path)),
)
}

let pass = vec![
// should pass - camel_case, pascal_case both allowed
("", None, None, Some(PathBuf::from("foo/bar/baz/Que.tsx"))),
// should pass - camel_case, pascal_case both allowed
("", None, None, Some(PathBuf::from("foo/bar/baz/QueAbc.tsx"))),
("", None, None, Some(PathBuf::from("ansiHTML.tsx"))),
test_cases("src/foo/fooBar.js", ["camelCase"]),
test_case("src/foo/bar.js", "camelCase"),
test_case("src/foo/fooBar.js", "camelCase"),
test_case("src/foo/bar.test.js", "camelCase"),
test_case("src/foo/fooBar.test.js", "camelCase"),
test_case("src/foo/fooBar.test-utils.js", "camelCase"),
test_case("src/foo/fooBar.test_utils.js", "camelCase"),
test_case("src/foo/.test_utils.js", "camelCase"),
test_case("src/foo/foo.js", "snakeCase"),
test_case("src/foo/foo_bar.js", "snakeCase"),
test_case("src/foo/foo.test.js", "snakeCase"),
test_case("src/foo/foo_bar.test.js", "snakeCase"),
test_case("src/foo/foo_bar.test_utils.js", "snakeCase"),
test_case("src/foo/foo_bar.test-utils.js", "snakeCase"),
test_case("src/foo/.test-utils.js", "snakeCase"),
test_case("src/foo/foo.js", "kebabCase"),
test_case("src/foo/foo-bar.js", "kebabCase"),
test_case("src/foo/foo.test.js", "kebabCase"),
test_case("src/foo/foo-bar.test.js", "kebabCase"),
test_case("src/foo/foo-bar.test-utils.js", "kebabCase"),
test_case("src/foo/foo-bar.test_utils.js", "kebabCase"),
test_case("src/foo/.test_utils.js", "kebabCase"),
test_case("src/foo/Foo.js", "pascalCase"),
test_case("src/foo/FooBar.js", "pascalCase"),
test_case("src/foo/Foo.test.js", "pascalCase"),
test_case("src/foo/FooBar.test.js", "pascalCase"),
test_case("src/foo/FooBar.test-utils.js", "pascalCase"),
test_case("src/foo/FooBar.test_utils.js", "pascalCase"),
test_case("src/foo/.test_utils.js", "pascalCase"),
test_case("spec/iss47Spec.js", "camelCase"),
test_case("spec/iss47Spec100.js", "camelCase"),
test_case("spec/i18n.js", "camelCase"),
test_case("spec/iss47-spec.js", "kebabCase"),
test_case("spec/iss-47-spec.js", "kebabCase"),
test_case("spec/iss47-100spec.js", "kebabCase"),
test_case("spec/i18n.js", "kebabCase"),
test_case("spec/iss47_spec.js", "snakeCase"),
test_case("spec/iss_47_spec.js", "snakeCase"),
test_case("spec/iss47_100spec.js", "snakeCase"),
test_case("spec/i18n.js", "snakeCase"),
test_case("spec/Iss47Spec.js", "pascalCase"),
test_case("spec/Iss47.100spec.js", "pascalCase"),
test_case("spec/I18n.js", "pascalCase"),
test_case("src/foo/_fooBar.js", "camelCase"),
test_case("src/foo/___fooBar.js", "camelCase"),
test_case("src/foo/_foo_bar.js", "snakeCase"),
test_case("src/foo/___foo_bar.js", "snakeCase"),
test_case("src/foo/_foo-bar.js", "kebabCase"),
test_case("src/foo/___foo-bar.js", "kebabCase"),
test_case("src/foo/_FooBar.js", "pascalCase"),
test_case("src/foo/___FooBar.js", "pascalCase"),
test_case("src/foo/$foo.js", "pascalCase"),
test_cases("src/foo/foo-bar.js", []),
test_cases("src/foo/fooBar.js", ["camelCase"]),
test_cases("src/foo/FooBar.js", ["kebabCase", "pascalCase"]),
test_cases("src/foo/___foo_bar.js", ["snakeCase", "pascalCase"]),
];
let fail = vec![
// should pass - by default kebab_case is not allowed
("import foo from 'bar'", None, None, Some(PathBuf::from("foo/bar/baz/aaa-bbb.tsx"))),
// should pass - by default snake_case is not allowed
("baz;", None, None, Some(PathBuf::from("foo/bar/baz/foo_bar.tsx"))),
test_case("src/foo/foo_bar.js", ""),
// todo: linter does not support uppercase JS files
// test_case("src/foo/foo_bar.JS", "camelCase"),
test_case("src/foo/foo_bar.test.js", "camelCase"),
test_case("test/foo/foo_bar.test_utils.js", "camelCase"),
test_case("test/foo/fooBar.js", "snakeCase"),
test_case("test/foo/fooBar.test.js", "snakeCase"),
test_case("test/foo/fooBar.testUtils.js", "snakeCase"),
test_case("test/foo/fooBar.js", "kebabCase"),
test_case("test/foo/fooBar.test.js", "kebabCase"),
test_case("test/foo/fooBar.testUtils.js", "kebabCase"),
test_case("test/foo/fooBar.js", "pascalCase"),
test_case("test/foo/foo_bar.test.js", "pascalCase"),
test_case("test/foo/foo-bar.test-utils.js", "pascalCase"),
test_case("src/foo/_FOO-BAR.js", "camelCase"),
test_case("src/foo/___FOO-BAR.js", "camelCase"),
test_case("src/foo/_FOO-BAR.js", "snakeCase"),
test_case("src/foo/___FOO-BAR.js", "snakeCase"),
test_case("src/foo/_FOO-BAR.js", "kebabCase"),
test_case("src/foo/___FOO-BAR.js", "kebabCase"),
test_case("src/foo/_FOO-BAR.js", "pascalCase"),
test_case("src/foo/___FOO-BAR.js", "pascalCase"),
test_cases("src/foo/foo_bar.js", []),
test_cases("src/foo/foo-bar.js", ["camelCase", "pascalCase"]),
test_cases("src/foo/_foo_bar.js", ["camelCase", "pascalCase", "kebabCase"]),
test_cases("src/foo/_FOO-BAR.js", ["snakeCase"]),
test_case("src/foo/[foo_bar].js", ""),
test_case("src/foo/$foo_bar.js", ""),
test_case("src/foo/$fooBar.js", ""),
test_cases("src/foo/{foo_bar}.js", ["camelCase", "pascalCase", "kebabCase"]),
];

Tester::new(FilenameCase::NAME, pass, fail).test_and_snapshot();
Expand Down
Loading