From e0f578401d3c914bf2d1c766c43b5173e09b2450 Mon Sep 17 00:00:00 2001
From: Denis Bezrukov <6227442+denbezrukov@users.noreply.github.com>
Date: Mon, 12 Jun 2023 10:40:37 +0300
Subject: [PATCH] fix(rome_js_analyzer): Big pack of js/ts files that crashes
 rome #4323 (#4560)

---
 CHANGELOG.md                                  |  2 ++
 crates/rome_cli/tests/commands/check.rs       | 32 +++++++++++++++++++
 .../apply_bogus_argument.snap                 | 18 +++++++++++
 .../style/no_parameter_assign.rs              | 10 +++---
 .../specs/style/noParameterAssign/valid.jsonc |  3 +-
 .../style/noParameterAssign/valid.jsonc.snap  |  6 +++-
 .../src/semantic_model/binding.rs             |  5 +--
 .../src/semantic_model/model.rs               |  2 +-
 8 files changed, 66 insertions(+), 12 deletions(-)
 create mode 100644 crates/rome_cli/tests/snapshots/main_commands_check/apply_bogus_argument.snap

diff --git a/CHANGELOG.md b/CHANGELOG.md
index fdb9b5614be..c70274d632f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -32,6 +32,8 @@
 ### Formatter
 ### Linter
 
+- Fix a crash in the `NoParameterAssign` rule that occurred when there was a bogus binding. [#4323](https://github.com/rome/tools/issues/4323)
+
 #### Other changes
 
 - The rules [`useExhaustiveDependencies`](https://docs.rome.tools/lint/rules/useexhaustivedependencies/) and [`useHookAtTopLevel`](https://docs.rome.tools/lint/rules/usehookattoplevel/) accept a different
diff --git a/crates/rome_cli/tests/commands/check.rs b/crates/rome_cli/tests/commands/check.rs
index 1817f2c3110..14333f57db5 100644
--- a/crates/rome_cli/tests/commands/check.rs
+++ b/crates/rome_cli/tests/commands/check.rs
@@ -2274,6 +2274,38 @@ if (true) {
     ));
 }
 
+#[test]
+fn apply_bogus_argument() {
+    let mut fs = MemoryFileSystem::default();
+    let mut console = BufferConsole::default();
+
+    let file_path = Path::new("fix.js");
+    fs.insert(
+        file_path.into(),
+        "function _13_1_3_fun(arguments) { }".as_bytes(),
+    );
+
+    let result = run_cli(
+        DynRef::Borrowed(&mut fs),
+        &mut console,
+        Args::from(&[
+            ("check"),
+            file_path.as_os_str().to_str().unwrap(),
+            ("--apply-unsafe"),
+        ]),
+    );
+
+    assert!(result.is_ok(), "run_cli returned {result:?}");
+
+    assert_cli_snapshot(SnapshotPayload::new(
+        module_path!(),
+        "apply_bogus_argument",
+        fs,
+        console,
+        result,
+    ));
+}
+
 #[test]
 fn ignores_unknown_file() {
     let mut fs = MemoryFileSystem::default();
diff --git a/crates/rome_cli/tests/snapshots/main_commands_check/apply_bogus_argument.snap b/crates/rome_cli/tests/snapshots/main_commands_check/apply_bogus_argument.snap
new file mode 100644
index 00000000000..f5dd858ad26
--- /dev/null
+++ b/crates/rome_cli/tests/snapshots/main_commands_check/apply_bogus_argument.snap
@@ -0,0 +1,18 @@
+---
+source: crates/rome_cli/tests/snap_test.rs
+expression: content
+---
+## `fix.js`
+
+```js
+function _13_1_3_fun(arguments) { }
+
+```
+
+# Emitted Messages
+
+```block
+Fixed 1 file(s) in <TIME>
+```
+
+
diff --git a/crates/rome_js_analyze/src/semantic_analyzers/style/no_parameter_assign.rs b/crates/rome_js_analyze/src/semantic_analyzers/style/no_parameter_assign.rs
index 595e1ec820d..b4a06952154 100644
--- a/crates/rome_js_analyze/src/semantic_analyzers/style/no_parameter_assign.rs
+++ b/crates/rome_js_analyze/src/semantic_analyzers/style/no_parameter_assign.rs
@@ -2,7 +2,7 @@ use crate::semantic_services::Semantic;
 use rome_analyze::{context::RuleContext, declare_rule, Rule, RuleDiagnostic};
 use rome_console::markup;
 use rome_js_semantic::{AllBindingWriteReferencesIter, Reference, ReferencesExtensions};
-use rome_js_syntax::{AnyJsBindingPattern, AnyJsFormalParameter, AnyJsParameter};
+use rome_js_syntax::{AnyJsBinding, AnyJsBindingPattern, AnyJsFormalParameter, AnyJsParameter};
 use rome_rowan::AstNode;
 
 declare_rule! {
@@ -71,10 +71,10 @@ impl Rule for NoParameterAssign {
     fn run(ctx: &RuleContext<Self>) -> Self::Signals {
         let param = ctx.query();
         let model = ctx.model();
-        if let Some(binding) = binding_of(param) {
-            if let Some(binding) = binding.as_any_js_binding() {
-                return binding.all_writes(model);
-            }
+        if let Some(AnyJsBindingPattern::AnyJsBinding(AnyJsBinding::JsIdentifierBinding(binding))) =
+            binding_of(param)
+        {
+            return binding.all_writes(model);
         }
         // Empty iterator that conforms to `AllBindingWriteReferencesIter` type.
         std::iter::successors(None, |_| None)
diff --git a/crates/rome_js_analyze/tests/specs/style/noParameterAssign/valid.jsonc b/crates/rome_js_analyze/tests/specs/style/noParameterAssign/valid.jsonc
index d0ed837e685..f7979183ad1 100644
--- a/crates/rome_js_analyze/tests/specs/style/noParameterAssign/valid.jsonc
+++ b/crates/rome_js_analyze/tests/specs/style/noParameterAssign/valid.jsonc
@@ -57,5 +57,6 @@
 	"function foo(a) { for ([a.b] of []); }",
 	"function foo(a) { a.b &&= c; }",
 	"function foo(a) { a.b.c ||= d; }",
-	"function foo(a) { a[b] ??= c; }"
+	"function foo(a) { a[b] ??= c; }",
+	"function foo(arguments) { }"
 ]
diff --git a/crates/rome_js_analyze/tests/specs/style/noParameterAssign/valid.jsonc.snap b/crates/rome_js_analyze/tests/specs/style/noParameterAssign/valid.jsonc.snap
index ec4ad13a442..826994ec9f0 100644
--- a/crates/rome_js_analyze/tests/specs/style/noParameterAssign/valid.jsonc.snap
+++ b/crates/rome_js_analyze/tests/specs/style/noParameterAssign/valid.jsonc.snap
@@ -1,6 +1,5 @@
 ---
 source: crates/rome_js_analyze/tests/spec_tests.rs
-assertion_line: 91
 expression: valid.jsonc
 ---
 # Input
@@ -298,4 +297,9 @@ function foo(a) { a.b.c ||= d; }
 function foo(a) { a[b] ??= c; }
 ```
 
+# Input
+```js
+function foo(arguments) { }
+```
+
 
diff --git a/crates/rome_js_semantic/src/semantic_model/binding.rs b/crates/rome_js_semantic/src/semantic_model/binding.rs
index bd524d022f6..153fc56b77e 100644
--- a/crates/rome_js_semantic/src/semantic_model/binding.rs
+++ b/crates/rome_js_semantic/src/semantic_model/binding.rs
@@ -1,7 +1,5 @@
 use super::*;
-use rome_js_syntax::{
-    binding_ext::AnyJsIdentifierBinding, AnyJsBinding, TextRange, TsTypeParameterName,
-};
+use rome_js_syntax::{binding_ext::AnyJsIdentifierBinding, TextRange, TsTypeParameterName};
 
 /// Internal type with all the semantic data of a specific binding
 #[derive(Debug)]
@@ -135,7 +133,6 @@ pub trait IsBindingAstNode: AstNode<Language = JsLanguage> {
 impl IsBindingAstNode for JsIdentifierBinding {}
 impl IsBindingAstNode for TsIdentifierBinding {}
 impl IsBindingAstNode for AnyJsIdentifierBinding {}
-impl IsBindingAstNode for AnyJsBinding {}
 impl IsBindingAstNode for TsTypeParameterName {}
 
 /// Extension method to allow nodes that have declaration to easily
diff --git a/crates/rome_js_semantic/src/semantic_model/model.rs b/crates/rome_js_semantic/src/semantic_model/model.rs
index 161066a9afc..2e2d361aa13 100644
--- a/crates/rome_js_semantic/src/semantic_model/model.rs
+++ b/crates/rome_js_semantic/src/semantic_model/model.rs
@@ -373,7 +373,7 @@ impl SemanticModel {
         };
 
         Some(AllCallsIter {
-            references: identifier.all_reads(self),
+            references: identifier.as_js_identifier_binding()?.all_reads(self),
         })
     }
 }