From 6567e393a73a927a1b2df2ff74b7c0879478800e Mon Sep 17 00:00:00 2001 From: Sysix Date: Thu, 26 Dec 2024 17:34:26 +0100 Subject: [PATCH] fix(linter): rule no-restricted-imports: improve diagnostics --- .../src/rules/eslint/no_restricted_imports.rs | 58 ++++++++++++++----- .../eslint_no_restricted_imports.snap | 10 ++-- 2 files changed, 49 insertions(+), 19 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 03919b8fde7d6..5aaf1239ef7b7 100644 --- a/crates/oxc_linter/src/rules/eslint/no_restricted_imports.rs +++ b/crates/oxc_linter/src/rules/eslint/no_restricted_imports.rs @@ -142,13 +142,18 @@ fn diagnostic_everything_with_allowed_import_name( source: &str, allowed: &str, ) -> OxcDiagnostic { - let msg = message.unwrap_or_else(|| { - CompactStr::new(&format!( + if let Some(message) = message { + return OxcDiagnostic::warn(format!( "* import is invalid because only '{allowed}' from '{source}' is/are allowed." )) - }); + .with_help(message) + .with_label(span); + } - OxcDiagnostic::warn(msg).with_help("Remove the import statement.").with_label(span) + OxcDiagnostic::warn(format!( + "'* import is invalid because only '{allowed}' from '{source}' is/are allowed." + )) + .with_label(span) } fn diagnostic_allowed_import_name_pattern( @@ -421,7 +426,7 @@ fn is_name_span_allowed_in_pattern( enum IsSkipAbleResult { Allowed, GeneralDisallowed(NameSpan), - GeneralDisallowedWithoutSpan, + DefaultDisallowed, NameDisallowed(NameSpan), } @@ -450,7 +455,7 @@ impl RestrictedPath { NameSpanAllowedResult::Allowed => IsSkipAbleResult::Allowed, } } - ImportImportName::NamespaceObject => IsSkipAbleResult::GeneralDisallowedWithoutSpan, + ImportImportName::NamespaceObject => IsSkipAbleResult::DefaultDisallowed, } } @@ -468,7 +473,7 @@ impl RestrictedPath { } } ExportImportName::All | ExportImportName::AllButDefault => { - IsSkipAbleResult::GeneralDisallowedWithoutSpan + IsSkipAbleResult::DefaultDisallowed } ExportImportName::Null => IsSkipAbleResult::Allowed, } @@ -499,7 +504,7 @@ impl RestrictedPattern { NameSpanAllowedResult::Allowed => IsSkipAbleResult::Allowed, } } - ImportImportName::NamespaceObject => IsSkipAbleResult::GeneralDisallowedWithoutSpan, + ImportImportName::NamespaceObject => IsSkipAbleResult::DefaultDisallowed, } } @@ -517,7 +522,7 @@ impl RestrictedPattern { } } ExportImportName::All | ExportImportName::AllButDefault => { - IsSkipAbleResult::GeneralDisallowedWithoutSpan + IsSkipAbleResult::DefaultDisallowed } ExportImportName::Null => IsSkipAbleResult::Allowed, } @@ -710,7 +715,7 @@ impl NoRestrictedImports { source, )); } - IsSkipAbleResult::GeneralDisallowedWithoutSpan => { + IsSkipAbleResult::DefaultDisallowed => { if let Some(import_names) = &path.import_names { ctx.diagnostic(diagnostic_everything( entry.module_request.span(), @@ -718,6 +723,13 @@ impl NoRestrictedImports { import_names.join(", ").as_str(), source, )); + } else if let Some(allowed_import_names) = &path.allow_import_names { + ctx.diagnostic(diagnostic_everything_with_allowed_import_name( + entry.module_request.span(), + path.message.clone(), + source, + allowed_import_names.join(", ").as_str(), + )); } else { ctx.diagnostic(diagnostic_path( entry.module_request.span(), @@ -766,7 +778,7 @@ impl NoRestrictedImports { GlobResult::Found => { let diagnostic: OxcDiagnostic = match result { IsSkipAbleResult::GeneralDisallowed(_) - | IsSkipAbleResult::GeneralDisallowedWithoutSpan => diagnostic_pattern( + | IsSkipAbleResult::DefaultDisallowed => diagnostic_pattern( entry.module_request.span(), pattern.message.clone(), source, @@ -827,10 +839,28 @@ impl NoRestrictedImports { } match path.is_skip_able_export(&entry.import_name) { - IsSkipAbleResult::GeneralDisallowed(_) - | IsSkipAbleResult::GeneralDisallowedWithoutSpan => { + IsSkipAbleResult::GeneralDisallowed(_) => { ctx.diagnostic(diagnostic_path(entry.span, path.message.clone(), source)); } + IsSkipAbleResult::DefaultDisallowed => { + if let Some(import_names) = &path.import_names { + ctx.diagnostic(diagnostic_everything( + entry.span, + path.message.clone(), + import_names.join(", ").as_str(), + source, + )); + } else if let Some(allowed_import_names) = &path.allow_import_names { + ctx.diagnostic(diagnostic_everything_with_allowed_import_name( + entry.span, + path.message.clone(), + source, + allowed_import_names.join(", ").as_str(), + )); + } else { + ctx.diagnostic(diagnostic_path(entry.span, path.message.clone(), source)); + } + } IsSkipAbleResult::NameDisallowed(name_span) => { if let Some(allow_import_names) = &path.allow_import_names { ctx.diagnostic(diagnostic_allowed_import_name( @@ -877,7 +907,7 @@ impl NoRestrictedImports { let diagnostic = match result { IsSkipAbleResult::GeneralDisallowed(_) - | IsSkipAbleResult::GeneralDisallowedWithoutSpan => { + | IsSkipAbleResult::DefaultDisallowed => { diagnostic_pattern(span, pattern.message.clone(), source) } IsSkipAbleResult::NameDisallowed(name_span) => { 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 dd75207cbf373..276ec2b2dd3a9 100644 --- a/crates/oxc_linter/src/snapshots/eslint_no_restricted_imports.snap +++ b/crates/oxc_linter/src/snapshots/eslint_no_restricted_imports.snap @@ -117,7 +117,7 @@ snapshot_kind: text ╰──── help: Don"t import "". - ⚠ eslint(no-restricted-imports): 'fs' import is restricted from being used. + ⚠ eslint(no-restricted-imports): * import is invalid because 'foo' from 'fs' is restricted. ╭─[no_restricted_imports.tsx:1:1] 1 │ export * as ns from "fs"; · ───────────────────────── @@ -159,14 +159,14 @@ snapshot_kind: text ╰──── help: Please import "DisallowedObject" from /bar/ instead. - ⚠ eslint(no-restricted-imports): 'foo' import is restricted from being used. + ⚠ eslint(no-restricted-imports): * import is invalid because 'DisallowedObject' from 'foo' is restricted. ╭─[no_restricted_imports.tsx:1:1] 1 │ export * from "foo"; · ──────────────────── ╰──── help: Please import "DisallowedObject" from /bar/ instead. - ⚠ eslint(no-restricted-imports): 'foo' import is restricted from being used. + ⚠ eslint(no-restricted-imports): * import is invalid because 'DisallowedObject1, DisallowedObject2' from 'foo' is restricted. ╭─[no_restricted_imports.tsx:1:1] 1 │ export * from "foo"; · ──────────────────── @@ -729,13 +729,13 @@ snapshot_kind: text ╰──── help: Only "AllowedObject" is allowed to be imported from "foo". - ⚠ eslint(no-restricted-imports): 'foo' import is restricted from being used. + ⚠ eslint(no-restricted-imports): '* import is invalid because only 'AllowedObject' from 'foo' is/are allowed. ╭─[no_restricted_imports.tsx:1:32] 1 │ import * as AllowedObject from "foo"; · ───── ╰──── - ⚠ eslint(no-restricted-imports): 'foo' import is restricted from being used. + ⚠ eslint(no-restricted-imports): * import is invalid because only 'AllowedObject' from 'foo' is/are allowed. ╭─[no_restricted_imports.tsx:1:32] 1 │ import * as AllowedObject from "foo"; · ─────