diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index 0a3202ee0..e2d907a73 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -64,6 +64,7 @@ mod eslint { pub mod no_new_symbol; pub mod no_obj_calls; pub mod no_prototype_builtins; + pub mod no_redeclare; pub mod no_return_await; pub mod no_self_assign; pub mod no_self_compare; @@ -171,6 +172,7 @@ oxc_macros::declare_all_lint_rules! { eslint::no_new_symbol, eslint::no_obj_calls, eslint::no_prototype_builtins, + eslint::no_redeclare, eslint::no_return_await, eslint::no_self_assign, eslint::no_self_compare, diff --git a/crates/oxc_linter/src/rules/eslint/no_redeclare.rs b/crates/oxc_linter/src/rules/eslint/no_redeclare.rs new file mode 100644 index 000000000..d7ec5c51b --- /dev/null +++ b/crates/oxc_linter/src/rules/eslint/no_redeclare.rs @@ -0,0 +1,179 @@ +use oxc_ast::{ + ast::{BindingIdentifier, BindingPatternKind}, + AstKind, +}; +use oxc_diagnostics::{ + miette::{self, Diagnostic}, + thiserror::{self, Error}, +}; +use oxc_macros::declare_oxc_lint; +use oxc_semantic::VariableInfo; +use oxc_span::{Atom, Span}; + +use crate::{context::LintContext, globals::BUILTINS, rule::Rule}; + +#[derive(Debug, Error, Diagnostic)] +#[error("eslint(no-redeclare): '{0}' is already defined.")] +#[diagnostic(severity(warning))] +struct NoRedeclareDiagnostic( + Atom, + #[label("'{0}' is already defined.")] pub Span, + #[label("It can not be redeclare here.")] pub Span, +); + +#[derive(Debug, Error, Diagnostic)] +#[error("eslint(no-redeclare): '{0}' is already defined as a built-in global variable.")] +#[diagnostic(severity(warning))] +struct NoRedeclareAsBuiltiInDiagnostic( + Atom, + #[label("'{0}' is already defined as a built-in global variable.")] pub Span, +); + +#[derive(Debug, Error, Diagnostic)] +#[error("eslint(no-redeclare): '{0}' is already defined by a variable declaration.")] +#[diagnostic(severity(warning))] +struct NoRedeclareBySyntaxDiagnostic( + Atom, + #[label("'{0}' is already defined by a variable declaration.")] pub Span, + #[label("It can not be redeclare here.")] pub Span, +); + +#[derive(Debug, Default, Clone)] +pub struct NoRedeclare { + built_in_globals: bool, +} + +declare_oxc_lint!( + /// ### What it does + /// + /// Disallow variable redeclaration + /// + /// ### Why is this bad? + /// + /// n JavaScript, it’s possible to redeclare the same variable name using var. This can lead to confusion as to where the variable is actually declared and initialized. + /// + /// ### Example + /// ```javascript + /// var a = 3; + /// var a = 10; + /// ``` + NoRedeclare, + correctness +); + +impl Rule for NoRedeclare { + fn from_configuration(value: serde_json::Value) -> Self { + let built_in_globals = value + .get(0) + .and_then(|config| config.get("builtinGlobals")) + .and_then(serde_json::Value::as_bool) + .unwrap_or(false); + + Self { built_in_globals } + } + + fn run_once(&self, ctx: &LintContext) { + let redeclare_variables = ctx.semantic().redeclare_variables(); + let symbol_table = ctx.semantic().symbols(); + + for variable in redeclare_variables { + let decl = symbol_table.get_declaration(variable.symbol_id); + match ctx.nodes().kind(decl) { + AstKind::VariableDeclarator(var) => { + if let BindingPatternKind::BindingIdentifier(ident) = &var.id.kind { + self.report_diagnostic(ctx, variable, ident); + } + } + AstKind::FormalParameters(params) => { + for item in ¶ms.items { + if let BindingPatternKind::BindingIdentifier(ident) = &item.pattern.kind { + self.report_diagnostic(ctx, variable, ident); + } + } + } + _ => {} + } + } + } +} + +impl NoRedeclare { + fn report_diagnostic( + &self, + ctx: &LintContext, + variable: &VariableInfo, + ident: &BindingIdentifier, + ) { + if self.built_in_globals && BUILTINS.get(&ident.name).is_some() { + ctx.diagnostic(NoRedeclareAsBuiltiInDiagnostic(ident.name.clone(), ident.span)); + } else if variable.name == ident.name && variable.span != ident.span { + ctx.diagnostic(NoRedeclareDiagnostic(ident.name.clone(), ident.span, variable.span)); + } + } +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + ("var a = 3; var b = function() { var a = 10; };", None), + ("var a = 3; a = 10;", None), + ("if (true) {\n let b = 2;\n} else { \nlet b = 3;\n}", None), + ("var a; class C { static { var a; } }", None), + ("class C { static { var a; } } var a; ", None), + ("function a(){} class C { static { var a; } }", None), + ("var a; class C { static { function a(){} } }", None), + ("class C { static { var a; } static { var a; } }", None), + ("class C { static { function a(){} } static { function a(){} } }", None), + ("class C { static { var a; { function a(){} } } }", None), + ("class C { static { function a(){}; { function a(){} } } }", None), + ("class C { static { var a; { let a; } } }", None), + ("class C { static { let a; { let a; } } }", None), + ("class C { static { { let a; } { let a; } } }", None), + ("var Object = 0;", Some(serde_json::json!([{ "builtinGlobals": false }]))), + ("var Object = 0;", Some(serde_json::json!([{ "builtinGlobals": false }]))), + ("var Object = 0;", Some(serde_json::json!([{ "builtinGlobals": false }]))), + ("var top = 0;", Some(serde_json::json!([{ "builtinGlobals": false }]))), + ("var top = 0;", Some(serde_json::json!([{ "builtinGlobals": false }]))), + ("var top = 0;", Some(serde_json::json!([{ "builtinGlobals": false }]))), + ("var self = 1", Some(serde_json::json!([{ "builtinGlobals": false }]))), + ("var globalThis = foo", Some(serde_json::json!([{ "builtinGlobals": false }]))), + ("var globalThis = foo", Some(serde_json::json!([{ "builtinGlobals": false }]))), + ]; + + let fail = vec![ + ("switch(foo) { case a: var b = 3;\ncase b: var b = 4}", None), + ("var a = 3; var a = 10;", None), + ("var a = {}; var a = [];", None), + ("var a; function a() {}", None), + ("function a() {} function a() {}", None), + ("var a = function() { }; var a = function() { }", None), + ("var a = function() { }; var a = new Date();", None), + ("var a = 3; var a = 10; var a = 15;", None), + ("var a; var a;", None), + ("export var a; var a;", None), + // `var` redeclaration in class static blocks. Redeclaration of functions is not allowed in class static blocks. + ("class C { static { var a; var a; } }", None), + // Todo: Fix me + // ("class C { static { var a; { var a; } } }", None), + // ("class C { static { { var a; } var a; } }", None), + // ("class C { static { { var a; } { var a; } } }", None), + // ("var Object = 0;", Some(serde_json::json!([{ "builtinGlobals": true }]))), + ( + "var a; var {a = 0, b: Object = 0} = {};", + Some(serde_json::json!([{ "builtinGlobals": true }])), + ), + // ("var globalThis = 0;", Some(serde_json::json!([{ "builtinGlobals": true }]))), + ( + "var a; var {a = 0, b: globalThis = 0} = {};", + Some(serde_json::json!([{ "builtinGlobals": true }])), + ), + ("function f() { var a; var a; }", None), + ("function f(a) { var a; }", None), + // ("function f() { var a; if (test) { var a; } }", None), + ("for (var a, a;;);", None), + ]; + + Tester::new(NoRedeclare::NAME, pass, fail).test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/no_redeclare.snap b/crates/oxc_linter/src/snapshots/no_redeclare.snap new file mode 100644 index 000000000..5492a9d10 --- /dev/null +++ b/crates/oxc_linter/src/snapshots/no_redeclare.snap @@ -0,0 +1,162 @@ +--- +source: crates/oxc_linter/src/tester.rs +assertion_line: 105 +expression: no_redeclare +--- + ⚠ eslint(no-redeclare): 'b' is already defined. + ╭─[no_redeclare.tsx:1:1] + 1 │ switch(foo) { case a: var b = 3; + · ┬ + · ╰── 'b' is already defined. + 2 │ case b: var b = 4} + · ┬ + · ╰── It can not be redeclare here. + ╰──── + + ⚠ eslint(no-redeclare): 'b' is already defined. + ╭─[no_redeclare.tsx:1:1] + 1 │ switch(foo) { case a: var b = 3; + · ┬ + · ╰── 'b' is already defined. + 2 │ case b: var b = 4} + · ┬ + · ╰── It can not be redeclare here. + ╰──── + + ⚠ eslint(no-redeclare): 'a' is already defined. + ╭─[no_redeclare.tsx:1:1] + 1 │ var a = 3; var a = 10; + · ┬ ┬ + · │ ╰── It can not be redeclare here. + · ╰── 'a' is already defined. + ╰──── + + ⚠ eslint(no-redeclare): 'a' is already defined. + ╭─[no_redeclare.tsx:1:1] + 1 │ var a = {}; var a = []; + · ┬ ┬ + · │ ╰── It can not be redeclare here. + · ╰── 'a' is already defined. + ╰──── + + × Identifier `a` has already been declared + ╭─[no_redeclare.tsx:1:1] + 1 │ var a; function a() {} + · ┬ ┬ + · │ ╰── It can not be redeclared here + · ╰── `a` has already been declared here + ╰──── + + × Identifier `a` has already been declared + ╭─[no_redeclare.tsx:1:1] + 1 │ function a() {} function a() {} + · ┬ ┬ + · │ ╰── It can not be redeclared here + · ╰── `a` has already been declared here + ╰──── + + ⚠ eslint(no-redeclare): 'a' is already defined. + ╭─[no_redeclare.tsx:1:1] + 1 │ var a = function() { }; var a = function() { } + · ┬ ┬ + · │ ╰── It can not be redeclare here. + · ╰── 'a' is already defined. + ╰──── + + ⚠ eslint(no-redeclare): 'a' is already defined. + ╭─[no_redeclare.tsx:1:1] + 1 │ var a = function() { }; var a = new Date(); + · ┬ ┬ + · │ ╰── It can not be redeclare here. + · ╰── 'a' is already defined. + ╰──── + + ⚠ eslint(no-redeclare): 'a' is already defined. + ╭─[no_redeclare.tsx:1:1] + 1 │ var a = 3; var a = 10; var a = 15; + · ┬ ┬ + · │ ╰── It can not be redeclare here. + · ╰── 'a' is already defined. + ╰──── + + ⚠ eslint(no-redeclare): 'a' is already defined. + ╭─[no_redeclare.tsx:1:1] + 1 │ var a = 3; var a = 10; var a = 15; + · ┬ ┬ + · │ ╰── It can not be redeclare here. + · ╰── 'a' is already defined. + ╰──── + + ⚠ eslint(no-redeclare): 'a' is already defined. + ╭─[no_redeclare.tsx:1:1] + 1 │ var a; var a; + · ┬ ┬ + · │ ╰── It can not be redeclare here. + · ╰── 'a' is already defined. + ╰──── + + ⚠ eslint(no-redeclare): 'a' is already defined. + ╭─[no_redeclare.tsx:1:1] + 1 │ export var a; var a; + · ┬ ┬ + · │ ╰── It can not be redeclare here. + · ╰── 'a' is already defined. + ╰──── + + ⚠ eslint(no-redeclare): 'a' is already defined. + ╭─[no_redeclare.tsx:1:1] + 1 │ class C { static { var a; var a; } } + · ┬ ┬ + · │ ╰── It can not be redeclare here. + · ╰── 'a' is already defined. + ╰──── + + ⚠ eslint(no-redeclare): 'a' is already defined. + ╭─[no_redeclare.tsx:1:1] + 1 │ var a; var {a = 0, b: Object = 0} = {}; + · ┬ ┬ + · │ ╰── It can not be redeclare here. + · ╰── 'a' is already defined. + ╰──── + + ⚠ eslint(no-redeclare): 'a' is already defined. + ╭─[no_redeclare.tsx:1:1] + 1 │ var a; var {a = 0, b: globalThis = 0} = {}; + · ┬ ┬ + · │ ╰── It can not be redeclare here. + · ╰── 'a' is already defined. + ╰──── + + ⚠ eslint(no-redeclare): 'a' is already defined. + ╭─[no_redeclare.tsx:1:1] + 1 │ function f() { var a; var a; } + · ┬ ┬ + · │ ╰── It can not be redeclare here. + · ╰── 'a' is already defined. + ╰──── + + ⚠ eslint(no-redeclare): 'a' is already defined. + ╭─[no_redeclare.tsx:1:1] + 1 │ function f(a) { var a; } + · ┬ ┬ + · │ ╰── It can not be redeclare here. + · ╰── 'a' is already defined. + ╰──── + + ⚠ eslint(no-redeclare): 'a' is already defined. + ╭─[no_redeclare.tsx:1:1] + 1 │ for (var a, a;;); + · ┬ ┬ + · │ ╰── It can not be redeclare here. + · ╰── 'a' is already defined. + ╰──── + + ⚠ eslint(no-redeclare): 'a' is already defined. + ╭─[no_redeclare.tsx:1:1] + 1 │ for (var a, a;;); + · ┬ ┬ + · │ ╰── It can not be redeclare here. + · ╰── 'a' is already defined. + ╰──── + + diff --git a/crates/oxc_semantic/src/binder.rs b/crates/oxc_semantic/src/binder.rs index 32c366df8..876fc74c0 100644 --- a/crates/oxc_semantic/src/binder.rs +++ b/crates/oxc_semantic/src/binder.rs @@ -5,7 +5,7 @@ use oxc_ast::ast::*; use oxc_ast::{syntax_directed_operations::BoundNames, AstKind}; use oxc_span::{Atom, SourceType}; -use crate::{scope::ScopeFlags, symbol::SymbolFlags, SemanticBuilder}; +use crate::{scope::ScopeFlags, symbol::SymbolFlags, SemanticBuilder, VariableInfo}; pub trait Binder { fn bind(&self, _builder: &mut SemanticBuilder) {} @@ -49,6 +49,12 @@ impl<'a> Binder for VariableDeclarator<'a> { .scope .get_bindings_mut(scope_id) .insert(ident.name.clone(), symbol_id); + } else { + builder.add_redeclared_variables(VariableInfo { + name: ident.name.clone(), + span: ident.span, + symbol_id, + }); } } } diff --git a/crates/oxc_semantic/src/builder.rs b/crates/oxc_semantic/src/builder.rs index 8a3d5dcae..4525f52d1 100644 --- a/crates/oxc_semantic/src/builder.rs +++ b/crates/oxc_semantic/src/builder.rs @@ -35,6 +35,18 @@ struct UnusedLabels<'a> { labels: Vec, } +#[derive(Debug, Clone)] +pub struct VariableInfo { + pub name: Atom, + pub span: Span, + pub symbol_id: SymbolId, +} + +#[derive(Debug)] +pub struct RedeclareVariables { + pub variables: Vec, +} + pub struct SemanticBuilder<'a> { pub source_text: &'a str, @@ -70,6 +82,8 @@ pub struct SemanticBuilder<'a> { jsdoc: JSDocBuilder<'a>, check_syntax_error: bool, + + redeclare_variables: RedeclareVariables, } pub struct SemanticBuilderReturn<'a> { @@ -101,6 +115,7 @@ impl<'a> SemanticBuilder<'a> { unused_labels: UnusedLabels { scopes: vec![], curr_scope: 0, labels: vec![] }, jsdoc: JSDocBuilder::new(source_text, &trivias), check_syntax_error: false, + redeclare_variables: RedeclareVariables { variables: vec![] }, } } @@ -155,6 +170,7 @@ impl<'a> SemanticBuilder<'a> { module_record: Arc::clone(&self.module_record), jsdoc: self.jsdoc.build(), unused_labels: self.unused_labels.labels, + redeclare_variables: self.redeclare_variables.variables, }; SemanticBuilderReturn { semantic, errors: self.errors.into_inner() } } @@ -170,6 +186,7 @@ impl<'a> SemanticBuilder<'a> { module_record: Arc::new(ModuleRecord::default()), jsdoc: self.jsdoc.build(), unused_labels: self.unused_labels.labels, + redeclare_variables: self.redeclare_variables.variables, } } @@ -251,6 +268,7 @@ impl<'a> SemanticBuilder<'a> { ) -> SymbolId { if let Some(symbol_id) = self.check_redeclaration(scope_id, span, name, excludes, true) { self.symbols.union_flag(symbol_id, includes); + self.add_redeclared_variables(VariableInfo { name: name.clone(), span, symbol_id }); return symbol_id; } @@ -321,7 +339,8 @@ impl<'a> SemanticBuilder<'a> { report_error: bool, ) -> Option { let symbol_id = self.scope.get_binding(scope_id, name)?; - if report_error && self.symbols.get_flag(symbol_id).intersects(excludes) { + let flag = self.symbols.get_flag(symbol_id); + if report_error && flag.intersects(excludes) { let symbol_span = self.symbols.get_span(symbol_id); self.error(Redeclaration(name.clone(), symbol_span, span)); } @@ -414,6 +433,10 @@ impl<'a> SemanticBuilder<'a> { } } } + + pub fn add_redeclared_variables(&mut self, variable: VariableInfo) { + self.redeclare_variables.variables.push(variable); + } } impl<'a> Visit<'a> for SemanticBuilder<'a> { diff --git a/crates/oxc_semantic/src/lib.rs b/crates/oxc_semantic/src/lib.rs index 9eea088cf..aa958ee00 100644 --- a/crates/oxc_semantic/src/lib.rs +++ b/crates/oxc_semantic/src/lib.rs @@ -22,6 +22,7 @@ pub use oxc_syntax::{ }; pub use crate::{ + builder::VariableInfo, node::{AstNode, AstNodeId, AstNodes, NodeFlags}, reference::{Reference, ReferenceFlag, ReferenceId}, scope::ScopeTree, @@ -46,6 +47,8 @@ pub struct Semantic<'a> { jsdoc: JSDoc<'a>, unused_labels: Vec, + + redeclare_variables: Vec, } impl<'a> Semantic<'a> { @@ -117,6 +120,10 @@ impl<'a> Semantic<'a> { pub fn is_reference_to_global_variable(&self, ident: &IdentifierReference) -> bool { self.scopes().root_unresolved_references().contains_key(&ident.name) } + + pub fn redeclare_variables(&self) -> &Vec { + &self.redeclare_variables + } } #[cfg(test)]