diff --git a/.changeset/every-baths-dream.md b/.changeset/every-baths-dream.md new file mode 100644 index 000000000000..4e0f34d15a70 --- /dev/null +++ b/.changeset/every-baths-dream.md @@ -0,0 +1,12 @@ +--- +"@biomejs/biome": patch +--- + +Fixed [#7628](https://github.com/biomejs/biome/issues/7628): the `noArguments` rule now correctly recognizes the scope of the `arguments` object, as Biome now properly resolves the implicit `arguments` object. + +```js +let arguments = 0; +function func() { + return arguments[0]; // <- now invalid +} +``` diff --git a/crates/biome_js_analyze/tests/specs/complexity/noArguments/invalid.cjs b/crates/biome_js_analyze/tests/specs/complexity/noArguments/invalid.cjs index d49e3feb99f7..f3b7d9dc1f39 100644 --- a/crates/biome_js_analyze/tests/specs/complexity/noArguments/invalid.cjs +++ b/crates/biome_js_analyze/tests/specs/complexity/noArguments/invalid.cjs @@ -1,3 +1,4 @@ +let arguments = 0; function f() { console.log(arguments); @@ -6,7 +7,9 @@ function f() { } } -function f() { +function g() { + function h() { + console.log(arguments); + } let arguments = 1; - console.log(arguments); -} \ No newline at end of file +} diff --git a/crates/biome_js_analyze/tests/specs/complexity/noArguments/invalid.cjs.snap b/crates/biome_js_analyze/tests/specs/complexity/noArguments/invalid.cjs.snap index 629b2405eefd..96a504898661 100644 --- a/crates/biome_js_analyze/tests/specs/complexity/noArguments/invalid.cjs.snap +++ b/crates/biome_js_analyze/tests/specs/complexity/noArguments/invalid.cjs.snap @@ -1,9 +1,11 @@ --- source: crates/biome_js_analyze/tests/spec_tests.rs +assertion_line: 152 expression: invalid.cjs --- # Input ```cjs +let arguments = 0; function f() { console.log(arguments); @@ -12,23 +14,27 @@ function f() { } } -function f() { +function g() { + function h() { + console.log(arguments); + } let arguments = 1; - console.log(arguments); } + ``` # Diagnostics ``` -invalid.cjs:2:17 lint/complexity/noArguments ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +invalid.cjs:3:17 lint/complexity/noArguments ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! Use the rest parameters instead of arguments. - 1 │ function f() { - > 2 │ console.log(arguments); + 1 │ let arguments = 0; + 2 │ function f() { + > 3 │ console.log(arguments); │ ^^^^^^^^^ - 3 │ - 4 │ for(let i = 0;i < arguments.length; ++i) { + 4 │ + 5 │ for(let i = 0;i < arguments.length; ++i) { i arguments does not have Array.prototype methods and can be inconvenient to use. @@ -36,16 +42,16 @@ invalid.cjs:2:17 lint/complexity/noArguments ━━━━━━━━━━━ ``` ``` -invalid.cjs:4:23 lint/complexity/noArguments ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +invalid.cjs:5:23 lint/complexity/noArguments ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! Use the rest parameters instead of arguments. - 2 │ console.log(arguments); - 3 │ - > 4 │ for(let i = 0;i < arguments.length; ++i) { + 3 │ console.log(arguments); + 4 │ + > 5 │ for(let i = 0;i < arguments.length; ++i) { │ ^^^^^^^^^ - 5 │ console.log(arguments[i]); - 6 │ } + 6 │ console.log(arguments[i]); + 7 │ } i arguments does not have Array.prototype methods and can be inconvenient to use. @@ -53,15 +59,32 @@ invalid.cjs:4:23 lint/complexity/noArguments ━━━━━━━━━━━ ``` ``` -invalid.cjs:5:21 lint/complexity/noArguments ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +invalid.cjs:6:21 lint/complexity/noArguments ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ ! Use the rest parameters instead of arguments. - 4 │ for(let i = 0;i < arguments.length; ++i) { - > 5 │ console.log(arguments[i]); + 5 │ for(let i = 0;i < arguments.length; ++i) { + > 6 │ console.log(arguments[i]); │ ^^^^^^^^^ - 6 │ } - 7 │ } + 7 │ } + 8 │ } + + i arguments does not have Array.prototype methods and can be inconvenient to use. + + +``` + +``` +invalid.cjs:12:21 lint/complexity/noArguments ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Use the rest parameters instead of arguments. + + 10 │ function g() { + 11 │ function h() { + > 12 │ console.log(arguments); + │ ^^^^^^^^^ + 13 │ } + 14 │ let arguments = 1; i arguments does not have Array.prototype methods and can be inconvenient to use. diff --git a/crates/biome_js_analyze/tests/specs/complexity/noArguments/valid.cjs b/crates/biome_js_analyze/tests/specs/complexity/noArguments/valid.cjs new file mode 100644 index 000000000000..76cf91508412 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/complexity/noArguments/valid.cjs @@ -0,0 +1,5 @@ +/* should not generate diagnostics */ +function f() { + let arguments = 1; + console.log(arguments); +} diff --git a/crates/biome_js_analyze/tests/specs/complexity/noArguments/valid.cjs.snap b/crates/biome_js_analyze/tests/specs/complexity/noArguments/valid.cjs.snap new file mode 100644 index 000000000000..8a467c4c2094 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/complexity/noArguments/valid.cjs.snap @@ -0,0 +1,14 @@ +--- +source: crates/biome_js_analyze/tests/spec_tests.rs +assertion_line: 152 +expression: valid.cjs +--- +# Input +```cjs +/* should not generate diagnostics */ +function f() { + let arguments = 1; + console.log(arguments); +} + +``` diff --git a/crates/biome_js_semantic/src/events.rs b/crates/biome_js_semantic/src/events.rs index 0b6e01707531..69ccb2a702d8 100644 --- a/crates/biome_js_semantic/src/events.rs +++ b/crates/biome_js_semantic/src/events.rs @@ -288,6 +288,10 @@ enum ScopeHoisting { #[derive(Debug)] struct Scope { scope_id: ScopeId, + /// Range of this scope. + range: TextRange, + /// Kind of this scope. + kind: JsSyntaxKind, /// All bindings declared inside this scope. bindings: Vec, /// References that still needs to be bound and will be solved at the end of the scope. @@ -333,6 +337,7 @@ impl SemanticEventExtractor { JS_MODULE => { self.push_scope( node.text_trimmed_range(), + node.kind(), ScopeHoisting::DontHoistDeclarationsToParent, ScopeOptions { is_closure: false, @@ -345,6 +350,7 @@ impl SemanticEventExtractor { self.is_ambient_context = true; self.push_scope( node.text_trimmed_range(), + node.kind(), ScopeHoisting::DontHoistDeclarationsToParent, ScopeOptions { is_closure: false, @@ -356,6 +362,7 @@ impl SemanticEventExtractor { JS_SCRIPT => { self.push_scope( node.text_trimmed_range(), + node.kind(), ScopeHoisting::DontHoistDeclarationsToParent, ScopeOptions { is_closure: false, @@ -391,6 +398,7 @@ impl SemanticEventExtractor { | JS_SETTER_CLASS_MEMBER => { self.push_scope( node.text_trimmed_range(), + node.kind(), ScopeHoisting::DontHoistDeclarationsToParent, ScopeOptions { is_closure: true, @@ -408,6 +416,7 @@ impl SemanticEventExtractor { | JS_SETTER_OBJECT_MEMBER => { self.push_scope( node.text_trimmed_range(), + node.kind(), ScopeHoisting::DontHoistDeclarationsToParent, ScopeOptions { is_closure: true, @@ -419,6 +428,7 @@ impl SemanticEventExtractor { JS_FUNCTION_EXPORT_DEFAULT_DECLARATION => { self.push_scope( node.text_trimmed_range(), + node.kind(), ScopeHoisting::DontHoistDeclarationsToParent, ScopeOptions { is_closure: true, @@ -430,6 +440,7 @@ impl SemanticEventExtractor { JS_FUNCTION_BODY => { self.push_scope( node.text_trimmed_range(), + node.kind(), ScopeHoisting::DontHoistDeclarationsToParent, ScopeOptions { is_closure: false, @@ -446,6 +457,7 @@ impl SemanticEventExtractor { | TS_ENUM_DECLARATION => { self.push_scope( node.text_trimmed_range(), + node.kind(), ScopeHoisting::DontHoistDeclarationsToParent, ScopeOptions { is_closure: false, @@ -468,6 +480,7 @@ impl SemanticEventExtractor { self.is_ambient_context = true; self.push_scope( node.text_trimmed_range(), + node.kind(), ScopeHoisting::DontHoistDeclarationsToParent, ScopeOptions { is_closure: false, @@ -481,6 +494,7 @@ impl SemanticEventExtractor { | JS_SWITCH_STATEMENT | JS_CATCH_CLAUSE => { self.push_scope( node.text_trimmed_range(), + node.kind(), ScopeHoisting::HoistDeclarationsToParent, ScopeOptions::default(), ); @@ -499,6 +513,7 @@ impl SemanticEventExtractor { if node.in_conditional_true_type() { self.push_scope( node.syntax().text_trimmed_range(), + node.syntax().kind(), ScopeHoisting::DontHoistDeclarationsToParent, ScopeOptions { is_closure: false, @@ -518,6 +533,7 @@ impl SemanticEventExtractor { ) { self.push_scope( node.text_trimmed_range(), + node.kind(), ScopeHoisting::DontHoistDeclarationsToParent, ScopeOptions { is_closure: false, @@ -875,7 +891,7 @@ impl SemanticEventExtractor { | TS_TYPE_ALIAS_DECLARATION | TS_MODULE_DECLARATION | TS_EXTERNAL_MODULE_DECLARATION => { - self.pop_scope(node.text_trimmed_range()); + self.pop_scope(); if let Some(current_scope) = self.scopes.last() { self.is_ambient_context = current_scope.is_ambient; } @@ -893,7 +909,7 @@ impl SemanticEventExtractor { fn leave_any_type(&mut self, node: &AnyTsType) { if node.in_conditional_true_type() { - self.pop_scope(node.syntax().text_trimmed_range()); + self.pop_scope(); return; } let node = node.syntax(); @@ -903,7 +919,7 @@ impl SemanticEventExtractor { | JsSyntaxKind::TS_FUNCTION_TYPE | JsSyntaxKind::TS_MAPPED_TYPE ) { - self.pop_scope(node.text_trimmed_range()); + self.pop_scope(); } // FALLBACK // If the conditional type has a bogus true type, @@ -940,7 +956,13 @@ impl SemanticEventExtractor { } } - fn push_scope(&mut self, range: TextRange, hoisting: ScopeHoisting, options: ScopeOptions) { + fn push_scope( + &mut self, + range: TextRange, + kind: JsSyntaxKind, + hoisting: ScopeHoisting, + options: ScopeOptions, + ) { let scope_id = ScopeId::new(self.scope_count); self.scope_count += 1; self.stash.push_back(SemanticEvent::ScopeStarted { @@ -950,6 +972,8 @@ impl SemanticEventExtractor { }); self.scopes.push(Scope { scope_id, + range, + kind, bindings: vec![], references: FxHashMap::default(), shadowed: vec![], @@ -968,14 +992,67 @@ impl SemanticEventExtractor { /// 2 - Unmatched references are promoted to its parent scope or become [UnresolvedReference] events; /// 3 - All declarations of this scope are removed; /// 4 - All shadowed declarations are restored. - fn pop_scope(&mut self, scope_range: TextRange) { + fn pop_scope(&mut self) { + /// Checks if a scope kind represents a regular (non-arrow) function. + /// Regular functions provide an implicit `arguments` object, while arrow functions do not. + fn is_regular_function_kind(kind: &JsSyntaxKind) -> bool { + const REGULAR_FUNCTION_KINDS: &[JsSyntaxKind] = &[ + JS_FUNCTION_DECLARATION, + JS_FUNCTION_EXPORT_DEFAULT_DECLARATION, + JS_FUNCTION_EXPRESSION, + ]; + REGULAR_FUNCTION_KINDS.contains(kind) + } + + /// Determines if an `arguments` reference should bind to a given declaration. + /// Returns true if the binding is within the enclosing function scope, + /// or if there's no enclosing function (arrow function or global context). + fn can_bind_arguments_reference( + name: &BindingName, + info: &BindingInfo, + enclosing_function_range: Option, + ) -> bool { + match name { + BindingName::Value(token) if token.text() == "arguments" => { + if let Some(function_range) = enclosing_function_range { + function_range.start() <= info.range_start + && info.range_start <= function_range.end() + } else { + true + } + } + _ => true, + } + } + + /// Determines if an `arguments` reference should bind to a given declaration. + /// Returns true if the binding is within the enclosing function scope, + /// or if there's no enclosing function (arrow function or global context). + fn can_promote_arguments_reference(name: &BindingName, scope_kind: JsSyntaxKind) -> bool { + match name { + BindingName::Value(token) if token.text() == "arguments" => { + !is_regular_function_kind(&scope_kind) + } + _ => true, + } + } + + let enclosing_function_range = self + .scopes + .iter() + .rev() + .find(|scope| is_regular_function_kind(&scope.kind)) + .map(|scope| scope.range); + debug_assert!(!self.scopes.is_empty()); let scope = self.scopes.pop().unwrap(); let scope_id = scope.scope_id; // Bind references to declarations for (name, mut references) in scope.references { - if let Some(info) = self.bindings.get(&name) { + if let Some(info) = self.bindings.get(&name) + && can_bind_arguments_reference(&name, info, enclosing_function_range) + { let declaration_at = info.range_start; // We know the declaration of these reference. for reference in references { @@ -1104,7 +1181,9 @@ impl SemanticEventExtractor { } } } - } else if let Some(parent) = self.scopes.last_mut() { + } else if let Some(parent) = self.scopes.last_mut() + && can_promote_arguments_reference(&name, scope.kind) + { // Promote these references to the parent scope let parent_references = parent.references.entry(name).or_default(); parent_references.append(&mut references); @@ -1128,7 +1207,7 @@ impl SemanticEventExtractor { self.bindings.extend(scope.shadowed); self.stash - .push_back(SemanticEvent::ScopeEnded { range: scope_range }); + .push_back(SemanticEvent::ScopeEnded { range: scope.range }); } fn current_scope_mut(&mut self) -> &mut Scope { diff --git a/crates/biome_js_semantic/src/tests/assertions.rs b/crates/biome_js_semantic/src/tests/assertions.rs index 6f20a9907f49..ef569bbbcaf3 100644 --- a/crates/biome_js_semantic/src/tests/assertions.rs +++ b/crates/biome_js_semantic/src/tests/assertions.rs @@ -15,6 +15,15 @@ use std::collections::BTreeMap; /// the tree looking at tokens with trailing comments following a specifically patterns /// specifying if a scope has started or ended. /// +/// ### Source Type Configuration +/// +/// By default, tests are parsed as TSX. To parse as a script file, add a comment at the beginning: +/// +/// ```js +/// // @script +/// var x = 1; +/// ``` +/// /// ### Available Patterns /// /// #### Declaration Assertion @@ -106,7 +115,12 @@ use std::collections::BTreeMap; /// if(true) ;/*NOEVENT*/; /// ``` pub fn assert(code: &str, test_name: &str) { - let r = biome_js_parser::parse(code, JsFileSource::tsx(), JsParserOptions::default()); + let file_source = if code.trim_start().starts_with("// @script") { + JsFileSource::js_script() + } else { + JsFileSource::tsx() + }; + let r = biome_js_parser::parse(code, file_source, JsParserOptions::default()); if r.has_errors() { let mut console = EnvConsole::default(); diff --git a/crates/biome_js_semantic/src/tests/references.rs b/crates/biome_js_semantic/src/tests/references.rs index 09b7f7475248..d566ede7f7f0 100644 --- a/crates/biome_js_semantic/src/tests/references.rs +++ b/crates/biome_js_semantic/src/tests/references.rs @@ -202,6 +202,45 @@ assert_semantics! { console.log(arguments/*?*/[i]); } }"#, + ok_unresolved_reference_arguments_function_declaration, + r#"// @script + const arguments/*#A*/ = 1; + const foo/*#B*/ = 2; + function f() { + console.log(arguments/*?*/); + console.log(foo/*READ B*/); + }"#, + ok_unresolved_reference_arguments_function_expression, + r#"// @script + const arguments/*#A*/ = 1; + const foo/*#B*/ = 2; + const f = function() { + console.log(arguments/*?*/); + console.log(foo/*READ B*/); + }"#, + ok_resolved_reference_arguments_arrow_function_expression, + r#"// @script + const arguments/*#A*/ = 1; + const f = () => { + console.log(arguments/*READ A*/); + }"#, + ok_unresolved_reference_arguments_in_nested_function, + r#"// @script + function f() { + function g() { + console.log(arguments/*?*/); + } + const arguments/*#A*/ = 1; + g(arguments/*READ A*/); + } + const h = function() { + const i = function() { + console.log(arguments/*?*/); + } + const arguments/*#B*/ = 1; + i(arguments/*READ B*/); + } + "#, } // Exports