From b3f38aeab9d6b5e691f93f9d69a56d41ddd83f7f Mon Sep 17 00:00:00 2001 From: "Alexander S." Date: Fri, 20 Dec 2024 13:56:05 +0100 Subject: [PATCH] fix(linter): rule `no-restricted-imports`: support option `allowImportNames` (#8002) waiting for #7943 --- .../src/rules/eslint/no_restricted_imports.rs | 345 +++++++++--------- .../eslint_no_restricted_imports.snap | 28 ++ 2 files changed, 205 insertions(+), 168 deletions(-) diff --git a/crates/oxc_linter/src/rules/eslint/no_restricted_imports.rs b/crates/oxc_linter/src/rules/eslint/no_restricted_imports.rs index b2f21251e5243..4b0c1ffe94b9b 100644 --- a/crates/oxc_linter/src/rules/eslint/no_restricted_imports.rs +++ b/crates/oxc_linter/src/rules/eslint/no_restricted_imports.rs @@ -36,10 +36,11 @@ struct NoRestrictedImportsConfig { } #[derive(Debug, Clone, Deserialize)] +#[serde(rename_all = "camelCase")] struct RestrictedPath { name: CompactStr, - #[serde(rename = "importNames")] import_names: Option>, + allow_import_names: Option>, message: Option, } @@ -110,6 +111,7 @@ fn add_configuration_from_string(paths: &mut Vec, module_name: & paths.push(RestrictedPath { name: CompactStr::new(module_name), import_names: None, + allow_import_names: None, message: None, }); } @@ -155,42 +157,11 @@ impl Rule for NoRestrictedImports { continue; } - if let Some(import_names) = &path.import_names { - match &entry.import_name { - ImportImportName::Name(import) => { - let name = CompactStr::new(import.name()); - - if import_names.contains(&name) { - no_restricted_imports_diagnostic( - ctx, - span, - path.message.clone(), - source, - ); - } - } - ImportImportName::Default(_) => { - if import_names.contains(&CompactStr::new("default")) { - no_restricted_imports_diagnostic( - ctx, - span, - path.message.clone(), - source, - ); - } - } - ImportImportName::NamespaceObject => { - no_restricted_imports_diagnostic( - ctx, - span, - path.message.clone(), - source, - ); - } - } - } else { - no_restricted_imports_diagnostic(ctx, span, path.message.clone(), source); + if is_import_name_allowed(&entry.import_name, path) { + continue; } + + no_restricted_imports_diagnostic(ctx, span, path.message.clone(), source); } for (source, requests) in &module_record.requested_modules { @@ -228,31 +199,11 @@ impl Rule for NoRestrictedImports { continue; } - if let Some(import_names) = &path.import_names { - match &entry.import_name { - ExportImportName::Name(import_name) - if import_names.contains(&import_name.name) => - { - no_restricted_imports_diagnostic( - ctx, - span, - path.message.clone(), - source, - ); - } - ExportImportName::All | ExportImportName::AllButDefault => { - no_restricted_imports_diagnostic( - ctx, - span, - path.message.clone(), - source, - ); - } - _ => (), - } - } else { - no_restricted_imports_diagnostic(ctx, span, path.message.clone(), source); + if is_export_name_allowed(&entry.import_name, path) { + continue; } + + no_restricted_imports_diagnostic(ctx, span, path.message.clone(), source); } } @@ -265,37 +216,95 @@ impl Rule for NoRestrictedImports { continue; } - if let Some(import_names) = &path.import_names { - match &entry.import_name { - ExportImportName::Name(import_name) - if import_names.contains(&import_name.name) => - { - no_restricted_imports_diagnostic( - ctx, - span, - path.message.clone(), - source, - ); - } - ExportImportName::All | ExportImportName::AllButDefault => { - no_restricted_imports_diagnostic( - ctx, - span, - path.message.clone(), - source, - ); - } - _ => (), - } - } else { - no_restricted_imports_diagnostic(ctx, span, path.message.clone(), source); + if is_export_name_allowed(&entry.import_name, path) { + continue; } + + no_restricted_imports_diagnostic(ctx, span, path.message.clone(), source); } } } } } +fn is_import_name_allowed(name: &ImportImportName, path: &RestrictedPath) -> bool { + match &name { + ImportImportName::Name(import) => { + // fast check if this name is allowed + if path + .allow_import_names + .as_ref() + .is_some_and(|allowed| allowed.contains(&import.name)) + { + return true; + } + + // when no importNames option is provided, no import in general is allowed + if path.import_names.as_ref().is_none() { + return false; + } + + // the name is found is the importNames list + if path + .import_names + .as_ref() + .is_some_and(|disallowed| disallowed.contains(&import.name)) + { + return false; + } + + // we allow it + true + } + ImportImportName::Default(_) => { + if path + .import_names + .as_ref() + .is_some_and(|disallowed| disallowed.contains(&CompactStr::new("default"))) + || path.import_names.as_ref().is_none() + { + return false; + } + + true + } + ImportImportName::NamespaceObject => false, + } +} + +fn is_export_name_allowed(name: &ExportImportName, path: &RestrictedPath) -> bool { + match &name { + ExportImportName::Name(import_name) => { + // fast check if this name is allowed + if path + .allow_import_names + .as_ref() + .is_some_and(|allowed| allowed.contains(&import_name.name)) + { + return true; + } + + // when no importNames option is provided, no import in general is allowed + if path.import_names.as_ref().is_none() { + return false; + } + + // the name is found is the importNames list + if path + .import_names + .as_ref() + .is_some_and(|disallowed| disallowed.contains(&import_name.name)) + { + return false; + } + + true + } + ExportImportName::All | ExportImportName::AllButDefault => false, + ExportImportName::Null => true, + } +} + #[test] fn test() { use crate::tester::Tester; @@ -627,52 +636,52 @@ fn test() { }] }])), ), - // ( - // r#"import { AllowedObject } from "foo";"#, - // Some(serde_json::json!([{ - // "paths": [{ - // "name": "foo", - // "allowImportNames": ["AllowedObject"], - // "message": r#"Please import anything except "AllowedObject" from /bar/ instead."# - // }] - // }])), - // ), - // ( - // "import { foo } from 'foo';", - // Some(serde_json::json!([{ - // "paths": [{ - // "name": "foo", - // "allowImportNames": ["foo"] - // }] - // }])), - // ), - // ( - // "import { foo } from 'foo';", - // Some(serde_json::json!([{ - // "patterns": [{ - // "group": ["foo"], - // "allowImportNames": ["foo"] - // }] - // }])), - // ), - // ( - // "export { bar } from 'foo';", - // Some(serde_json::json!([{ - // "paths": [{ - // "name": "foo", - // "allowImportNames": ["bar"] - // }] - // }])), - // ), - // ( - // "export { bar } from 'foo';", - // Some(serde_json::json!([{ - // "patterns": [{ - // "group": ["foo"], - // "allowImportNames": ["bar"] - // }] - // }])), - // ), + ( + r#"import { AllowedObject } from "foo";"#, + Some(serde_json::json!([{ + "paths": [{ + "name": "foo", + "allowImportNames": ["AllowedObject"], + "message": r#"Please import anything except "AllowedObject" from /bar/ instead."# + }] + }])), + ), + ( + "import { foo } from 'foo';", + Some(serde_json::json!([{ + "paths": [{ + "name": "foo", + "allowImportNames": ["foo"] + }] + }])), + ), + ( + "import { foo } from 'foo';", + Some(serde_json::json!([{ + "patterns": [{ + "group": ["foo"], + "allowImportNames": ["foo"] + }] + }])), + ), + ( + "export { bar } from 'foo';", + Some(serde_json::json!([{ + "paths": [{ + "name": "foo", + "allowImportNames": ["bar"] + }] + }])), + ), + ( + "export { bar } from 'foo';", + Some(serde_json::json!([{ + "patterns": [{ + "group": ["foo"], + "allowImportNames": ["bar"] + }] + }])), + ), ( "import { Foo } from 'foo';", Some(serde_json::json!([{ @@ -1507,25 +1516,25 @@ fn test() { // }] // }])), // ), - // ( - // r#"import { AllowedObject, DisallowedObject } from "foo";"#, - // Some(serde_json::json!([{ - // "paths": [{ - // "name": "foo", - // "allowImportNames": ["AllowedObject"] - // }] - // }])), - // ), - // ( - // r#"import { AllowedObject, DisallowedObject } from "foo";"#, - // Some(serde_json::json!([{ - // "paths": [{ - // "name": "foo", - // "allowImportNames": ["AllowedObject"], - // "message": r#"Only "AllowedObject" is allowed to be imported from "foo"."# - // }] - // }])), - // ), + ( + r#"import { AllowedObject, DisallowedObject } from "foo";"#, + Some(serde_json::json!([{ + "paths": [{ + "name": "foo", + "allowImportNames": ["AllowedObject"] + }] + }])), + ), + ( + r#"import { AllowedObject, DisallowedObject } from "foo";"#, + Some(serde_json::json!([{ + "paths": [{ + "name": "foo", + "allowImportNames": ["AllowedObject"], + "message": r#"Only "AllowedObject" is allowed to be imported from "foo"."# + }] + }])), + ), // ( // r#"import { AllowedObject, DisallowedObject } from "foo";"#, // Some(serde_json::json!([{ @@ -1545,25 +1554,25 @@ fn test() { // }] // }])), // ), - // ( - // r#"import * as AllowedObject from "foo";"#, - // Some(serde_json::json!([{ - // "paths": [{ - // "name": "foo", - // "allowImportNames": ["AllowedObject"] - // }] - // }])), - // ), - // ( - // r#"import * as AllowedObject from "foo";"#, - // Some(serde_json::json!([{ - // "paths": [{ - // "name": "foo", - // "allowImportNames": ["AllowedObject"], - // "message": r#"Only "AllowedObject" is allowed to be imported from "foo"."# - // }] - // }])), - // ), + ( + r#"import * as AllowedObject from "foo";"#, + Some(serde_json::json!([{ + "paths": [{ + "name": "foo", + "allowImportNames": ["AllowedObject"] + }] + }])), + ), + ( + r#"import * as AllowedObject from "foo";"#, + Some(serde_json::json!([{ + "paths": [{ + "name": "foo", + "allowImportNames": ["AllowedObject"], + "message": r#"Only "AllowedObject" is allowed to be imported from "foo"."# + }] + }])), + ), // ( // r#"import * as AllowedObject from "foo/bar";"#, // Some(serde_json::json!([{ diff --git a/crates/oxc_linter/src/snapshots/eslint_no_restricted_imports.snap b/crates/oxc_linter/src/snapshots/eslint_no_restricted_imports.snap index 2de639d3a56f4..131a7e8bdd204 100644 --- a/crates/oxc_linter/src/snapshots/eslint_no_restricted_imports.snap +++ b/crates/oxc_linter/src/snapshots/eslint_no_restricted_imports.snap @@ -484,3 +484,31 @@ snapshot_kind: text · ────── ╰──── help: Remove the import statement. + + ⚠ eslint(no-restricted-imports): 'foo' import is restricted from being used. + ╭─[no_restricted_imports.tsx:1:49] + 1 │ import { AllowedObject, DisallowedObject } from "foo"; + · ───── + ╰──── + help: Remove the import statement. + + ⚠ eslint(no-restricted-imports): Only "AllowedObject" is allowed to be imported from "foo". + ╭─[no_restricted_imports.tsx:1:49] + 1 │ import { AllowedObject, DisallowedObject } from "foo"; + · ───── + ╰──── + help: Remove the import statement. + + ⚠ eslint(no-restricted-imports): 'foo' import is restricted from being used. + ╭─[no_restricted_imports.tsx:1:32] + 1 │ import * as AllowedObject from "foo"; + · ───── + ╰──── + help: Remove the import statement. + + ⚠ eslint(no-restricted-imports): Only "AllowedObject" is allowed to be imported from "foo". + ╭─[no_restricted_imports.tsx:1:32] + 1 │ import * as AllowedObject from "foo"; + · ───── + ╰──── + help: Remove the import statement.