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.

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

feat(linter): eslint-plugin-import no-named-as-default-member rule #1988

Merged
merged 1 commit into from
Jan 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions crates/oxc_linter/fixtures/import/named-and-default-export.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default {};

export const foo = 10
2 changes: 2 additions & 0 deletions crates/oxc_linter/src/rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ mod import {
pub mod named;
pub mod no_amd;
pub mod no_cycle;
pub mod no_named_as_default_member;
pub mod no_self_import;
}

Expand Down Expand Up @@ -504,6 +505,7 @@ oxc_macros::declare_all_lint_rules! {
react::no_unknown_property,
react::require_render_return,
import::default,
import::no_named_as_default_member,
import::named,
import::no_cycle,
import::no_self_import,
Expand Down
175 changes: 175 additions & 0 deletions crates/oxc_linter/src/rules/import/no_named_as_default_member.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
#![allow(clippy::significant_drop_tightening)]
use std::collections::HashMap;

use dashmap::mapref::one::Ref;
use oxc_ast::{
ast::{BindingPatternKind, Expression, MemberExpression},
AstKind,
};
use oxc_diagnostics::{
miette::{self, Diagnostic},
thiserror::Error,
};
use oxc_macros::declare_oxc_lint;
use oxc_span::{Atom, Span};
use oxc_syntax::module_record::ImportImportName;

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

#[derive(Debug, Error, Diagnostic)]
#[error("eslint-plugin-import(no-named-as-default-member): {1:?} also has a named export {2:?}")]
#[diagnostic(severity(warning), help("Check if you meant to write `import {{{2:}}} from {3:?}`"))]
struct NoNamedAsDefaultMemberDignostic(#[label] pub Span, String, String, String);

/// <https://github.com/import-js/eslint-plugin-import/blob/main/docs/rules/no-named-as-default-member.md>
#[derive(Debug, Default, Clone)]
pub struct NoNamedAsDefaultMember;

declare_oxc_lint!(
/// ### What it does
///
/// Reports use of an exported name as a property on the default export.
///
/// ### Example
///
/// ```javascript
/// // ./bar.js
/// export function bar() { return null }
/// export default () => { return 1 }
///
/// // ./foo.js
/// import bar from './bar'
/// const bar = foo.bar // trying to access named export via default
/// ```
NoNamedAsDefaultMember,
nursery
);

impl Rule for NoNamedAsDefaultMember {
fn run_once(&self, ctx: &LintContext<'_>) {
let module_record = ctx.semantic().module_record();

let mut has_members_map: HashMap<&Atom, (Ref<'_, Atom, _, _>, Atom)> = HashMap::default();
for import_entry in &module_record.import_entries {
let ImportImportName::Default(_) = import_entry.import_name else {
continue;
};

let specifier = import_entry.module_request.name();
let Some(remote_module_record_ref) = module_record.loaded_modules.get(specifier) else {
continue;
};

if !remote_module_record_ref.exported_bindings.is_empty() {
has_members_map.insert(
import_entry.local_name.name(),
(remote_module_record_ref, import_entry.module_request.name().to_owned()),
);
}
}

if has_members_map.is_empty() {
return;
};
let get_external_module_name_if_has_entry = |module_name: &Atom, entry_name: &Atom| {
has_members_map.get(&module_name).and_then(|it| {
if it.0.exported_bindings.contains_key(entry_name) {
mysteryven marked this conversation as resolved.
Show resolved Hide resolved
Some(it.1.to_string())
} else {
None
}
})
};

let process_member_expr = |member_expr: &MemberExpression| {
let Expression::Identifier(ident) = member_expr.object() else {
return;
};
let Some(prop_str) = member_expr.static_property_name() else {
return;
};
if let Some(module_name) =
get_external_module_name_if_has_entry(&ident.name, &Atom::new_inline(prop_str))
{
ctx.diagnostic(NoNamedAsDefaultMemberDignostic(
match member_expr {
MemberExpression::ComputedMemberExpression(it) => it.span,
MemberExpression::StaticMemberExpression(it) => it.span,
MemberExpression::PrivateFieldExpression(it) => it.span,
},
ident.name.to_string(),
prop_str.to_string(),
module_name,
));
};
};

for item in ctx.semantic().nodes().iter() {
match item.kind() {
AstKind::MemberExpression(member_expr) => process_member_expr(member_expr),
AstKind::VariableDeclarator(decl) => {
if let Some(Expression::MemberExpression(member_expr)) = &decl.init {
process_member_expr(member_expr);
return;
Copy link
Member

@mysteryven mysteryven Jan 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@XantreGodlike I know why, these return should be continue? 🙃 Change this will let the below case fail.

import baz from './named-exports';
{
    const baz = {};
    const a = baz.a;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, will fix it in this week

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}
let Some(Expression::Identifier(ident)) = &decl.init else {
return;
};
let BindingPatternKind::ObjectPattern(object_pattern) = &decl.id.kind else {
return;
};

for prop in &*object_pattern.properties {
let Some(name) = prop.key.static_name() else { return };
if let Some(module_name) =
get_external_module_name_if_has_entry(&ident.name, &name)
{
ctx.diagnostic(NoNamedAsDefaultMemberDignostic(
decl.span,
ident.name.to_string(),
name.to_string(),
module_name,
));
}
}
}
_ => {}
}
}
}
}

#[test]
fn test() {
use crate::tester::Tester;

let pass = vec![
r#"import baz, {a} from "./named-exports""#,
r#"import baz from "./named-exports"; const jjj = bar.jjj"#,
r#"import {a} from "./named-exports"; const baz = a.baz"#,
r#"import baz from "./default_export_default_property"; const d = baz.default;"#,
r#"import baz, {foo} from "./named-and-default-export"; const d = baz.default;"#,
r"import baz from './named-exports';
{
const baz = {};
const a = baz.a;
}",
];

let fail = vec![
r#"import baz from "./named-exports"; const a = baz.a;"#,
r#"import baz from "./named-exports"; const a = baz["a"];"#,
r#"import baz from "./named-exports"; baz.a();"#,
r"import baz from './named-exports';
{
const a = baz.a;
}",
r#"import baz, { bar } from "./named-exports"; const {a} = baz"#,
r#"import baz from "./named-and-default-export"; const {foo: _foo} = baz"#,
];

Tester::new(NoNamedAsDefaultMember::NAME, pass, fail)
.change_rule_path("index.js")
.with_import_plugin(true)
.test_and_snapshot();
}
49 changes: 49 additions & 0 deletions crates/oxc_linter/src/snapshots/no_named_as_default_member.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
---
source: crates/oxc_linter/src/tester.rs
expression: no_named_as_default_member
---
⚠ eslint-plugin-import(no-named-as-default-member): "baz" also has a named export "a"
╭─[index.js:1:1]
1 │ import baz from "./named-exports"; const a = baz.a;
· ─────
╰────
help: Check if you meant to write `import {a} from "./named-exports"`

⚠ eslint-plugin-import(no-named-as-default-member): "baz" also has a named export "a"
╭─[index.js:1:1]
1 │ import baz from "./named-exports"; const a = baz["a"];
· ────────
╰────
help: Check if you meant to write `import {a} from "./named-exports"`

⚠ eslint-plugin-import(no-named-as-default-member): "baz" also has a named export "a"
╭─[index.js:1:1]
1 │ import baz from "./named-exports"; baz.a();
· ─────
╰────
help: Check if you meant to write `import {a} from "./named-exports"`

⚠ eslint-plugin-import(no-named-as-default-member): "baz" also has a named export "a"
╭─[index.js:2:1]
2 │ {
3 │ const a = baz.a;
· ─────
4 │ }
╰────
help: Check if you meant to write `import {a} from "./named-exports"`

⚠ eslint-plugin-import(no-named-as-default-member): "baz" also has a named export "a"
╭─[index.js:1:1]
1 │ import baz, { bar } from "./named-exports"; const {a} = baz
· ─────────
╰────
help: Check if you meant to write `import {a} from "./named-exports"`

⚠ eslint-plugin-import(no-named-as-default-member): "baz" also has a named export "foo"
╭─[index.js:1:1]
1 │ import baz from "./named-and-default-export"; const {foo: _foo} = baz
· ─────────────────
╰────
help: Check if you meant to write `import {foo} from "./named-and-default-export"`


2 changes: 1 addition & 1 deletion tasks/coverage/babel
Submodule babel updated 810 files
2 changes: 1 addition & 1 deletion tasks/coverage/test262
Submodule test262 updated 257 files
2 changes: 1 addition & 1 deletion tasks/coverage/typescript
Submodule typescript updated 751 files
2 changes: 1 addition & 1 deletion tasks/prettier_conformance/prettier
Submodule prettier updated 528 files
Loading