Skip to content

Commit

Permalink
fix(linter): rule no-restricted-imports: improve diagnostics
Browse files Browse the repository at this point in the history
  • Loading branch information
@Sysix
Sysix committed Dec 26, 2024
1 parent e069fcb commit 6567e39
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 19 deletions.
58 changes: 44 additions & 14 deletions crates/oxc_linter/src/rules/eslint/no_restricted_imports.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -421,7 +426,7 @@ fn is_name_span_allowed_in_pattern(
enum IsSkipAbleResult {
Allowed,
GeneralDisallowed(NameSpan),
GeneralDisallowedWithoutSpan,
DefaultDisallowed,
NameDisallowed(NameSpan),
}

Expand Down Expand Up @@ -450,7 +455,7 @@ impl RestrictedPath {
NameSpanAllowedResult::Allowed => IsSkipAbleResult::Allowed,
}
}
ImportImportName::NamespaceObject => IsSkipAbleResult::GeneralDisallowedWithoutSpan,
ImportImportName::NamespaceObject => IsSkipAbleResult::DefaultDisallowed,
}
}

Expand All @@ -468,7 +473,7 @@ impl RestrictedPath {
}
}
ExportImportName::All | ExportImportName::AllButDefault => {
IsSkipAbleResult::GeneralDisallowedWithoutSpan
IsSkipAbleResult::DefaultDisallowed
}
ExportImportName::Null => IsSkipAbleResult::Allowed,
}
Expand Down Expand Up @@ -499,7 +504,7 @@ impl RestrictedPattern {
NameSpanAllowedResult::Allowed => IsSkipAbleResult::Allowed,
}
}
ImportImportName::NamespaceObject => IsSkipAbleResult::GeneralDisallowedWithoutSpan,
ImportImportName::NamespaceObject => IsSkipAbleResult::DefaultDisallowed,
}
}

Expand All @@ -517,7 +522,7 @@ impl RestrictedPattern {
}
}
ExportImportName::All | ExportImportName::AllButDefault => {
IsSkipAbleResult::GeneralDisallowedWithoutSpan
IsSkipAbleResult::DefaultDisallowed
}
ExportImportName::Null => IsSkipAbleResult::Allowed,
}
Expand Down Expand Up @@ -710,14 +715,21 @@ impl NoRestrictedImports {
source,
));
}
IsSkipAbleResult::GeneralDisallowedWithoutSpan => {
IsSkipAbleResult::DefaultDisallowed => {
if let Some(import_names) = &path.import_names {
ctx.diagnostic(diagnostic_everything(
entry.module_request.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.module_request.span(),
path.message.clone(),
source,
allowed_import_names.join(", ").as_str(),
));
} else {
ctx.diagnostic(diagnostic_path(
entry.module_request.span(),
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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) => {
Expand Down
10 changes: 5 additions & 5 deletions crates/oxc_linter/src/snapshots/eslint_no_restricted_imports.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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";
· ─────────────────────────
Expand Down Expand Up @@ -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";
· ────────────────────
Expand Down Expand Up @@ -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";
· ─────
Expand Down

0 comments on commit 6567e39

Please sign in to comment.