feat(es/lints): Support module/script mode in duplicate_bindings (#3880)

Co-authored-by: Donny/강동윤 <kdy1997.dev@gmail.com>

.github/workflows/integration.yml

@@ -94,9 +94,7 @@ jobs:
           # Download three.js
           git clone --depth 1 https://github.com/mrdoob/three.js.git -b r117 tests/integration/three-js/repo
 
-          swc --sync tests/integration/three-js/repo/ -d tests/integration/three-js/build/
-          # swc tests/integration/three-js/repo/src/ -d tests/integration/three-js/repo/build/
-          # swc tests/integration/three-js/repo/test/unit/**/*.js -d tests/integration/three-js/repo/test/unit/build/
+          swc -C isModule=unknown --sync tests/integration/three-js/repo/ -d tests/integration/three-js/build/
 
           (cd tests/integration/three-js/build/test && qunit -r failonlyreporter unit/three.source.unit.js)
 

crates/swc/tests/error_msg.rs

@@ -62,7 +62,7 @@ fn fixture(input: PathBuf) {
             handler,
             &Options {
                 swcrc: true,
-                is_module: IsModule::Bool(true),
+                is_module: IsModule::Unknown,
 
                 ..Default::default()
             },

crates/swc/tests/errors/lints/duplicate-bindings/const-let/1/input.js

@@ -1,2 +0,0 @@
-const a = 5;
-let a = 2;

crates/swc/tests/errors/lints/duplicate-bindings/const-let/1/output.swc-stderr

@@ -1,12 +0,0 @@
-error: Duplicate binding
- 
-  |
-2 | let a = 2;
-  |     ^
-  |
-note: a was declared at here
- 
-  |
-1 | const a = 5;
-  |       ^
-

crates/swc/tests/errors/lints/duplicate-bindings/module/export/input.js

@@ -0,0 +1,10 @@
+function foo() {}
+const foo = 1; // error
+
+function bar() {}
+var bar; // error
+
+function baz() {}
+function baz() {} // error
+
+export {};

crates/swc/tests/errors/lints/duplicate-bindings/module/export/output.swc-stderr

@@ -0,0 +1,36 @@
+error: Duplicate binding
+ 
+  |
+2 | const foo = 1; // error
+  |       ^^^
+  |
+note: foo was declared at here
+ 
+  |
+1 | function foo() {}
+  |          ^^^
+
+error: Duplicate binding
+ 
+  |
+5 | var bar; // error
+  |     ^^^
+  |
+note: bar was declared at here
+ 
+  |
+4 | function bar() {}
+  |          ^^^
+
+error: Duplicate binding
+ 
+  |
+8 | function baz() {} // error
+  |          ^^^
+  |
+note: baz was declared at here
+ 
+  |
+7 | function baz() {}
+  |          ^^^
+

crates/swc/tests/errors/lints/duplicate-bindings/module/import/input.js

@@ -0,0 +1,4 @@
+import { hi } from "foo";
+
+function foo() {}
+function foo() {}

crates/swc/tests/errors/lints/duplicate-bindings/module/import/output.swc-stderr

@@ -0,0 +1,12 @@
+error: Duplicate binding
+ 
+  |
+4 | function foo() {}
+  |          ^^^
+  |
+note: foo was declared at here
+ 
+  |
+3 | function foo() {}
+  |          ^^^
+

crates/swc/tests/errors/lints/duplicate-bindings/var/1/input.js

@@ -0,0 +1,11 @@
+const a = 5;
+let a = 2; // error
+
+const b = 1;
+var b = 2; // error
+
+let c = 3;
+var c = 4; // error
+
+var d = 1;
+var d = 1; // ok

crates/swc/tests/errors/lints/duplicate-bindings/var/1/output.swc-stderr

@@ -0,0 +1,36 @@
+error: Duplicate binding
+ 
+  |
+2 | let a = 2; // error
+  |     ^
+  |
+note: a was declared at here
+ 
+  |
+1 | const a = 5;
+  |       ^
+
+error: Duplicate binding
+ 
+  |
+5 | var b = 2; // error
+  |     ^
+  |
+note: b was declared at here
+ 
+  |
+4 | const b = 1;
+  |       ^
+
+error: Duplicate binding
+ 
+  |
+8 | var c = 4; // error
+  |     ^
+  |
+note: c was declared at here
+ 
+  |
+7 | let c = 3;
+  |     ^
+

crates/swc/tests/errors/lints/duplicate-bindings/var/2/input.js

@@ -0,0 +1,8 @@
+function foo() {}
+const foo = 1; // error
+
+function bar() {}
+var bar; // ok
+
+function baz() {}
+function baz() {} // ok

crates/swc/tests/errors/lints/duplicate-bindings/var/2/output.swc-stderr

@@ -0,0 +1,12 @@
+error: Duplicate binding
+ 
+  |
+2 | const foo = 1; // error
+  |       ^^^
+  |
+note: foo was declared at here
+ 
+  |
+1 | function foo() {}
+  |          ^^^
+

crates/swc/tests/errors/lints/no-loop-func/default/output.swc-stderr

@@ -1,3 +1,15 @@
+error: Duplicate binding
+  
+   |
+56 |     var a = 0;
+   |         ^
+   |
+note: a was declared at here
+  
+   |
+36 | let a; for (let i in {}) { (function() { a; }); a = 1; }
+   |     ^
+
 error: Duplicate binding
   
    |

crates/swc_ecma_lints/src/rules/duplicate_bindings.rs

@@ -1,7 +1,10 @@
 use std::collections::hash_map::Entry;
 
-use ahash::AHashSet;
-use swc_common::{collections::AHashMap, errors::HANDLER, Span};
+use swc_common::{
+    collections::{AHashMap, AHashSet},
+    errors::HANDLER,
+    Span,
+};
 use swc_ecma_ast::*;
 use swc_ecma_utils::ident::IdentLike;
 use swc_ecma_visit::{noop_visit_type, Visit, VisitWith};
@@ -12,41 +15,61 @@ pub fn duplicate_bindings() -> Box<dyn Rule> {
     visitor_rule(DuplicateBindings::default())
 }
 
+#[derive(Debug, Default, Clone, Copy)]
+struct BindingInfo {
+    span: Span,
+    unique: bool,
+}
+
 #[derive(Debug, Default)]
 struct DuplicateBindings {
-    bindings: AHashMap<Id, Span>,
+    bindings: AHashMap<Id, BindingInfo>,
     type_bindings: AHashSet<Id>,
 
     var_decl_kind: Option<VarDeclKind>,
     is_pat_decl: bool,
+
+    is_module: bool,
 }
 
 impl DuplicateBindings {
     /// Add a binding.
-    fn add(&mut self, id: &Ident, check_for_var_kind: bool) {
-        if check_for_var_kind {
-            if let Some(VarDeclKind::Var) = self.var_decl_kind {
-                return;
-            }
-        }
-
+    fn add(&mut self, id: &Ident, unique: bool) {
         match self.bindings.entry(id.to_id()) {
             Entry::Occupied(mut prev) => {
-                HANDLER.with(|handler| {
-                    handler
-                        .struct_span_err(id.span, "Duplicate binding")
-                        .span_note(*prev.get(), &format!("{} was declared at here", id.sym))
-                        .emit();
-                });
+                if unique || prev.get().unique {
+                    HANDLER.with(|handler| {
+                        handler
+                            .struct_span_err(id.span, "Duplicate binding")
+                            .span_note(prev.get().span, &format!("{} was declared at here", id.sym))
+                            .emit();
+                    });
+                }
 
                 // Next span.
-                *prev.get_mut() = id.span;
+                if unique || !prev.get().unique {
+                    *prev.get_mut() = BindingInfo {
+                        span: id.span,
+                        unique,
+                    }
+                }
             }
             Entry::Vacant(e) => {
-                e.insert(id.span);
+                e.insert(BindingInfo {
+                    span: id.span,
+                    unique,
+                });
             }
         }
     }
+
+    /// `const` or `let`
+    fn is_unique_var_kind(&self) -> bool {
+        matches!(
+            self.var_decl_kind,
+            Some(VarDeclKind::Const) | Some(VarDeclKind::Let)
+        )
+    }
 }
 
 impl Visit for DuplicateBindings {
@@ -57,6 +80,7 @@ impl Visit for DuplicateBindings {
             type_bindings: &mut self.type_bindings,
         });
 
+        self.is_module = true;
         m.visit_children_with(self);
     }
 
@@ -72,12 +96,12 @@ impl Visit for DuplicateBindings {
         p.visit_children_with(self);
 
         if self.is_pat_decl {
-            self.add(&p.key, true);
+            self.add(&p.key, self.is_unique_var_kind());
         }
     }
 
     fn visit_class_decl(&mut self, d: &ClassDecl) {
-        self.add(&d.ident, false);
+        self.add(&d.ident, true);
 
         d.visit_children_with(self);
     }
@@ -97,7 +121,7 @@ impl Visit for DuplicateBindings {
 
     fn visit_fn_decl(&mut self, d: &FnDecl) {
         if d.function.body.is_some() {
-            self.add(&d.ident, false);
+            self.add(&d.ident, self.is_module);
         }
 
         d.visit_children_with(self);
@@ -107,7 +131,7 @@ impl Visit for DuplicateBindings {
         s.visit_children_with(self);
 
         if !self.type_bindings.contains(&s.local.to_id()) {
-            self.add(&s.local, false);
+            self.add(&s.local, true);
         }
     }
 
@@ -115,7 +139,7 @@ impl Visit for DuplicateBindings {
         s.visit_children_with(self);
 
         if !s.is_type_only && !self.type_bindings.contains(&s.local.to_id()) {
-            self.add(&s.local, false);
+            self.add(&s.local, true);
         }
     }
 
@@ -123,7 +147,7 @@ impl Visit for DuplicateBindings {
         s.visit_children_with(self);
 
         if !self.type_bindings.contains(&s.local.to_id()) {
-            self.add(&s.local, false);
+            self.add(&s.local, true);
         }
     }
 
@@ -132,7 +156,7 @@ impl Visit for DuplicateBindings {
 
         if let Pat::Ident(p) = p {
             if self.is_pat_decl {
-                self.add(&p.id, true);
+                self.add(&p.id, self.is_unique_var_kind());
             }
         }
     }

crates/swc_ecma_lints/tests/fixture.rs

@@ -40,21 +40,19 @@ fn pass(input: PathBuf) {
 
         let mut parser = Parser::new_from(lexer);
 
-        let mut m = parser.parse_module().unwrap();
+        let mut program = parser.parse_program().unwrap();
         let top_level_mark = Mark::fresh(Mark::root());
 
         if input.extension().unwrap() == "ts" || input.extension().unwrap() == "tsx" {
-            m.visit_mut_with(&mut ts_resolver(top_level_mark));
+            program.visit_mut_with(&mut ts_resolver(top_level_mark));
         } else {
-            m.visit_mut_with(&mut resolver_with_mark(top_level_mark));
+            program.visit_mut_with(&mut resolver_with_mark(top_level_mark));
         }
 
         let top_level_ctxt = SyntaxContext::empty().apply_mark(top_level_mark);
 
         let config = LintConfig::default();
 
-        let program = Program::Module(m);
-
         let rules = all(LintParams {
             program: &program,
             lint_config: &config,

crates/swc_ecma_lints/tests/pass/issue-3852/1/input.js

@@ -0,0 +1,10 @@
+function sum(i, j) {
+    return i + j;
+}
+
+function sum() {
+    return 0;
+}
+
+console.log(sum());
+console.log(sum(1, 2));