From bd3768b8fa190921fc0ec045ca7c2c7b4899221c Mon Sep 17 00:00:00 2001 From: Peter Wagenet Date: Sat, 25 Oct 2025 09:20:17 -0700 Subject: [PATCH 1/2] Improve import/extensions functionality --- .../oxc_linter/src/rules/import/extensions.rs | 826 ++++++++++++------ .../src/snapshots/import_extensions.snap | 488 +++++------ 2 files changed, 774 insertions(+), 540 deletions(-) diff --git a/crates/oxc_linter/src/rules/import/extensions.rs b/crates/oxc_linter/src/rules/import/extensions.rs index bfc0ac9fe63a0..29e0eac632ba8 100644 --- a/crates/oxc_linter/src/rules/import/extensions.rs +++ b/crates/oxc_linter/src/rules/import/extensions.rs @@ -1,3 +1,6 @@ +use std::collections::BTreeMap; + +use fast_glob::glob_match; use oxc_ast::{ AstKind, ast::{Argument, CallExpression, Expression}, @@ -55,6 +58,21 @@ impl FileExtensionConfig { } } +#[derive(Debug, Clone, Default, JsonSchema, Serialize)] +#[serde(rename_all = "camelCase")] +enum OverrideAction { + #[default] + Enforce, + Ignore, +} + +#[derive(Debug, Clone, Default, JsonSchema, Serialize)] +#[serde(rename_all = "camelCase")] +struct PathGroupOverride { + pattern: String, + action: OverrideAction, +} + #[derive(Debug, Clone, JsonSchema, Serialize)] #[serde(rename_all = "camelCase", default)] pub struct ExtensionsConfig { @@ -64,53 +82,39 @@ pub struct ExtensionsConfig { require_extension: Option, /// Whether to check type imports when enforcing extension rules. check_type_imports: bool, - /// Configuration for `.js` file extensions. - js: FileExtensionConfig, - /// Configuration for `.jsx` file extensions. - jsx: FileExtensionConfig, - /// Configuration for `.ts` file extensions. - ts: FileExtensionConfig, - /// Configuration for `.tsx` file extensions. - tsx: FileExtensionConfig, - /// Configuration for `.json` file extensions. - json: FileExtensionConfig, + extensions: BTreeMap, + default_config: FileExtensionConfig, + path_group_overrides: Vec, + has_wildcard: bool, // Whether "*" pattern was explicitly set } impl ExtensionsConfig { - pub fn is_always(&self, ext: &str) -> bool { - match ext { - "js" => matches!(self.js, FileExtensionConfig::Always), - "jsx" => matches!(self.jsx, FileExtensionConfig::Always), - "ts" => matches!(self.ts, FileExtensionConfig::Always), - "tsx" => matches!(self.tsx, FileExtensionConfig::Always), - "json" => matches!(self.json, FileExtensionConfig::Always), - _ => false, - } + fn is_never(&self, ext: &str) -> bool { + self.extensions.get(ext).is_some_and(|config| matches!(config, FileExtensionConfig::Never)) } - pub fn is_never(&self, ext: &str) -> bool { - match ext { - "js" => matches!(self.js, FileExtensionConfig::Never), - "jsx" => matches!(self.jsx, FileExtensionConfig::Never), - "ts" => matches!(self.ts, FileExtensionConfig::Never), - "tsx" => matches!(self.tsx, FileExtensionConfig::Never), - "json" => matches!(self.json, FileExtensionConfig::Never), - _ => false, - } + fn get_modifier(&self, ext: &str) -> &FileExtensionConfig { + self.extensions.get(ext).unwrap_or(&self.default_config) } } impl Default for ExtensionsConfig { fn default() -> Self { + // Pre-populate standard extensions with "Never" to match the original behavior + // when no configuration is provided + let mut extensions = BTreeMap::new(); + for ext in ["js", "jsx", "ts", "tsx", "json"] { + extensions.insert(ext.to_string(), FileExtensionConfig::Never); + } + Self { - ignore_packages: true, + ignore_packages: false, // ESLint default is false require_extension: None, check_type_imports: false, - js: FileExtensionConfig::Never, - jsx: FileExtensionConfig::Never, - ts: FileExtensionConfig::Never, - tsx: FileExtensionConfig::Never, - json: FileExtensionConfig::Never, + extensions, + default_config: FileExtensionConfig::Never, + path_group_overrides: Vec::new(), + has_wildcard: false, } } } @@ -236,44 +240,102 @@ fn build_config( value: &serde_json::Value, default: Option<&FileExtensionConfig>, ) -> ExtensionsConfig { - let config: ExtensionsConfig = ExtensionsConfig { - ignore_packages: value.get("ignorePackages").and_then(Value::as_bool).unwrap_or(true), - require_extension: default.cloned(), + let mut extensions = BTreeMap::new(); + let default_config = default.cloned().unwrap_or_default(); + + // If no explicit mode provided, pre-populate standard extensions with default + // This preserves backward compatibility with the original behavior + if default.is_none() { + for ext in ["js", "jsx", "ts", "tsx", "json"] { + extensions.insert(ext.to_string(), default_config.clone()); + } + } + + // Parse extension-specific configurations + if let Some(obj) = value.as_object() { + for (key, val) in obj { + // Skip known non-extension keys + if matches!( + key.as_str(), + "ignorePackages" | "checkTypeImports" | "pattern" | "pathGroupOverrides" + ) { + continue; + } + + // Handle wildcard "*" as default config + if key == "*" { + // Wildcard will be handled separately + continue; + } + + // Parse extension config + if let Some(config_str) = val.as_str() { + extensions.insert(key.clone(), FileExtensionConfig::from(config_str)); + } + } + } + + // Check for wildcard "*" pattern to set default + let has_wildcard = value.get("*").is_some(); + let default_config = value + .get("*") + .and_then(Value::as_str) + .map(FileExtensionConfig::from) + .unwrap_or(default_config); + + // Parse pathGroupOverrides + let mut path_group_overrides = Vec::new(); + if let Some(overrides_array) = value.get("pathGroupOverrides").and_then(Value::as_array) { + for override_obj in overrides_array { + let Some(obj) = override_obj.as_object() else { + continue; + }; + let Some(pattern) = obj.get("pattern").and_then(Value::as_str) else { + continue; + }; + let Some(action) = obj.get("action").and_then(Value::as_str) else { + continue; + }; + let action_enum = match action { + "enforce" => OverrideAction::Enforce, + "ignore" => OverrideAction::Ignore, + _ => continue, + }; + path_group_overrides + .push(PathGroupOverride { pattern: pattern.to_string(), action: action_enum }); + } + } + + // Handle ignorePackages flag + // When first arg is "ignorePackages", it means: require_extension="always" + ignore_packages=true + // When it's a boolean in config object, use that value + let ignore_packages = if matches!(default, Some(FileExtensionConfig::IgnorePackages)) { + // First arg was "ignorePackages" string - this means ignore_packages=true + true + } else { + // Check for explicit boolean value in config object + value.get("ignorePackages").and_then(Value::as_bool).unwrap_or(false) + }; + + // Transform "ignorePackages" mode to "always" (matching ESLint behavior) + let require_extension = if matches!(default, Some(FileExtensionConfig::IgnorePackages)) { + Some(FileExtensionConfig::Always) + } else { + default.cloned() + }; + + ExtensionsConfig { + ignore_packages, + require_extension, check_type_imports: value .get("checkTypeImports") .and_then(Value::as_bool) .unwrap_or_default(), - js: value - .get("js") - .and_then(Value::as_str) - .map(FileExtensionConfig::from) - .unwrap_or(default.cloned().unwrap_or_default()), - jsx: value - .get("jsx") - .and_then(Value::as_str) - .map(FileExtensionConfig::from) - .unwrap_or(default.cloned().unwrap_or_default()), - - ts: value - .get("ts") - .and_then(Value::as_str) - .map(FileExtensionConfig::from) - .unwrap_or(default.cloned().unwrap_or_default()), - - tsx: value - .get("tsx") - .and_then(Value::as_str) - .map(FileExtensionConfig::from) - .unwrap_or(default.cloned().unwrap_or_default()), - - json: value - .get("json") - .and_then(Value::as_str) - .map(FileExtensionConfig::from) - .unwrap_or(default.cloned().unwrap_or_default()), - }; - - config + extensions, + default_config, + path_group_overrides, + has_wildcard, + } } impl Extensions { @@ -296,26 +358,126 @@ impl Extensions { let is_builtin_node_module = NODEJS_BUILTINS.binary_search(&module_name.as_str()).is_ok() || ctx.globals().is_enabled(module_name.as_str()); - let is_package = module_name.as_str().starts_with('@') - || (!module_name.as_str().starts_with('.') - && !module_name.as_str().get(1..).is_some_and(|v| v.contains('/'))); + // Determine if this is a package import (external module) + // Since oxc doesn't do file resolution, we use heuristics: + // - Scoped packages: "@scope/package" or "@scope/package/subpath" (scope is alphanumeric) + // - Regular packages: "package" or "package/subpath" + // - Relative paths: start with "." or ".." + // - Path aliases like "@/" are NOT packages (@ must be followed by alphanumeric) + + let is_relative = module_name.as_str().starts_with('.'); + let starts_with_word_char = + module_name.chars().next().is_some_and(|c| c.is_alphanumeric() || c == '_'); + + // Check if this is a scoped package (must be @scope/... where scope is alphanumeric) + let is_scoped = if module_name.as_str().starts_with('@') { + // Must have a scope name after @, not just @/ or @. + module_name.as_str().get(1..).and_then(|s| s.chars().next()).is_some_and(|c| c.is_alphanumeric() || c == '_') + } else { + false + }; + + // This is a package if it's scoped or starts with a word character (not relative) + let is_package_root = is_scoped || (!is_relative && starts_with_word_char); + + if is_builtin_node_module { + return; + } + + // For root package imports without subpaths, skip extension checking entirely + // because package names can contain dots (e.g., "decimal.js", "pkg.config.js") + let has_subpath = if is_scoped { + // For scoped packages like "@scope/pkg", the first '/' is part of the package name + // A subpath would be "@scope/pkg/subpath" (more than one '/') + module_name.matches('/').count() > 1 + } else { + // For regular packages, any '/' indicates a subpath + module_name.contains('/') + }; + + // Check pathGroupOverrides FIRST, before any package ignoring logic + // This allows "enforce" to override ignorePackages for specific patterns + let mut should_enforce = false; + for override_item in &config.path_group_overrides { + if glob_match(&override_item.pattern, module_name.as_str()) { + match override_item.action { + OverrideAction::Ignore => { + // Skip validation for this import + return; + } + OverrideAction::Enforce => { + // Mark that we should enforce validation even if ignorePackages=true + should_enforce = true; + break; + } + } + } + } + + if is_package_root && !has_subpath { + // Root package name only, no subpath + if ignore_packages && !should_enforce { + return; + } + // Even without ignore_packages, we can't reliably extract extensions from package names + return; + } - if is_builtin_node_module || (is_package && ignore_packages) { + // For package subpaths (e.g., "lodash/map", "@babel/core/lib/index"), + // handle based on ignore_packages flag (unless overridden by enforce) + if is_package_root && ignore_packages && !should_enforce { return; } + // At this point, it's either: + // - A relative path (always validate) + // - A package subpath with ignore_packages=false (validate) + // - A package subpath with should_enforce=true (validate) + // Continue to extension checking... + let file_extension = get_file_extension_from_module_name(&module_name); let span = module.statement_span; if let Some(file_extension) = file_extension { let ext_str = file_extension.as_str(); + let modifier = config.get_modifier(ext_str); + let should_flag = match require_extension { Some(FileExtensionConfig::Always) => { - config.is_never(ext_str) || !config.is_always(ext_str) + // In "always" mode, flag only if modifier says "never" + // Unknown extensions inherit the "always" default + matches!(modifier, FileExtensionConfig::Never) + } + Some(FileExtensionConfig::Never) => { + // In "never" mode, we can't use file resolution like ESLint does + // to determine which extensions are resolvable. To avoid false positives, + // we only flag explicitly configured extensions or standard JS/TS extensions + // that are likely to be resolvable. + // + // However, if wildcard "*" is set to "never", we should check ALL extensions + // + // ESLint behavior: only flags resolvable extensions (determined by resolver config) + // oxc behavior: flag standard extensions, explicitly configured ones, or all if wildcard set + if matches!(modifier, FileExtensionConfig::Always) { + false // Explicitly allowed + } else if config.extensions.contains_key(ext_str) { + // Explicitly configured - respect the configuration + matches!(modifier, FileExtensionConfig::Never) + } else if config.has_wildcard + && matches!(config.default_config, FileExtensionConfig::Never) + { + // Wildcard "*" is set to "never" - flag all extensions + true + } else { + // Not explicitly configured, no wildcard - only flag standard JS/TS extensions + matches!(ext_str, "js" | "ts" | "mjs" | "cjs" | "mts" | "cts") + } + } + Some(FileExtensionConfig::IgnorePackages) | None => { + // In ignorePackages or default mode, only flag explicitly configured "never" + config.is_never(ext_str) } - Some(FileExtensionConfig::Never) => !config.is_always(ext_str), - _ => config.is_never(ext_str), }; if should_flag { @@ -325,11 +487,16 @@ impl Extensions { is_import, )); } - } else if matches!( - require_extension, - Some(FileExtensionConfig::Always | FileExtensionConfig::IgnorePackages) - ) { - ctx.diagnostic(extension_missing_diagnostic(span, is_import)); + } else { + // Missing extension - check if it should be required + let should_require = matches!( + require_extension, + Some(FileExtensionConfig::Always | FileExtensionConfig::IgnorePackages) + ); + + if should_require { + ctx.diagnostic(extension_missing_diagnostic(span, is_import)); + } } } @@ -347,12 +514,23 @@ impl Extensions { if let Some(file_extension) = file_extension { let ext_str = file_extension.as_str(); + let modifier = config.get_modifier(ext_str); + let should_flag = match require_extension { Some(FileExtensionConfig::Always) => { - config.is_never(ext_str) || !config.is_always(ext_str) + // In "always" mode, flag only if modifier says "never" + // Unknown extensions inherit the "always" default + matches!(modifier, FileExtensionConfig::Never) + } + Some(FileExtensionConfig::Never) => { + // In "never" mode, flag unless modifier says "always" + // Unknown extensions inherit the "never" default + !matches!(modifier, FileExtensionConfig::Always) + } + Some(FileExtensionConfig::IgnorePackages) | None => { + // In ignorePackages or default mode, only flag explicitly configured "never" + config.is_never(ext_str) } - Some(FileExtensionConfig::Never) => !config.is_always(ext_str), - _ => config.is_never(ext_str), }; if should_flag { @@ -362,11 +540,16 @@ impl Extensions { true, )); } - } else if matches!( - require_extension, - Some(FileExtensionConfig::Always | FileExtensionConfig::IgnorePackages) - ) { - ctx.diagnostic(extension_missing_diagnostic(span, true)); + } else { + // Missing extension - check if it should be required + let should_require = matches!( + require_extension, + Some(FileExtensionConfig::Always | FileExtensionConfig::IgnorePackages) + ); + + if should_require { + ctx.diagnostic(extension_missing_diagnostic(span, true)); + } } } } @@ -384,17 +567,57 @@ fn get_file_extension_from_module_name(module_name: &CompactStr) -> Option Date: Fri, 14 Nov 2025 05:24:34 +0000 Subject: [PATCH 2/2] [autofix.ci] apply automated fixes --- crates/oxc_linter/src/rules/import/extensions.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/oxc_linter/src/rules/import/extensions.rs b/crates/oxc_linter/src/rules/import/extensions.rs index 29e0eac632ba8..c0a81db705986 100644 --- a/crates/oxc_linter/src/rules/import/extensions.rs +++ b/crates/oxc_linter/src/rules/import/extensions.rs @@ -372,7 +372,11 @@ impl Extensions { // Check if this is a scoped package (must be @scope/... where scope is alphanumeric) let is_scoped = if module_name.as_str().starts_with('@') { // Must have a scope name after @, not just @/ or @. - module_name.as_str().get(1..).and_then(|s| s.chars().next()).is_some_and(|c| c.is_alphanumeric() || c == '_') + module_name + .as_str() + .get(1..) + .and_then(|s| s.chars().next()) + .is_some_and(|c| c.is_alphanumeric() || c == '_') } else { false };