Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

Commit

Permalink
fix(rome_js_analyzer): Big pack of js/ts files that crashes rome #4323
Browse files Browse the repository at this point in the history
  • Loading branch information
denbezrukov committed Jun 11, 2023
1 parent dad13a9 commit 56d6868
Show file tree
Hide file tree
Showing 8 changed files with 66 additions and 12 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
### Formatter
### Linter

- Fixed 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
Expand Down
32 changes: 32 additions & 0 deletions crates/rome_cli/tests/commands/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
@@ -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>
```


Original file line number Diff line number Diff line change
Expand Up @@ -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! {
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) { }"
]
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
---
source: crates/rome_js_analyze/tests/spec_tests.rs
assertion_line: 91
expression: valid.jsonc
---
# Input
Expand Down Expand Up @@ -298,4 +297,9 @@ function foo(a) { a.b.c ||= d; }
function foo(a) { a[b] ??= c; }
```
# Input
```js
function foo(arguments) { }
```
5 changes: 1 addition & 4 deletions crates/rome_js_semantic/src/semantic_model/binding.rs
Original file line number Diff line number Diff line change
@@ -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)]
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion crates/rome_js_semantic/src/semantic_model/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,7 @@ impl SemanticModel {
};

Some(AllCallsIter {
references: identifier.all_reads(self),
references: identifier.as_js_identifier_binding()?.all_reads(self),
})
}
}

0 comments on commit 56d6868

Please sign in to comment.