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

fix(rome_js_analyzer): Big pack of js/ts files that crashes rome #4323 #4560

Merged
merged 1 commit into from
Jun 12, 2023

Conversation

denbezrukov
Copy link
Contributor

Summary

Fix a semantic model crash when we have JsBogusBinding.

function _13_1_3_fun(arguments) { }
        0: JS_FUNCTION_DECLARATION@0..44
          0: (empty)
          1: FUNCTION_KW@0..18 "function" [Newline("\n"), Whitespace("        ")] [Whitespace(" ")]
          2: (empty)
          3: JS_IDENTIFIER_BINDING@18..29
            0: IDENT@18..29 "_13_1_3_fun" [] []
          4: (empty)
          5: JS_PARAMETERS@29..41
            0: L_PAREN@29..30 "(" [] []
            1: JS_PARAMETER_LIST@30..39
              0: JS_FORMAL_PARAMETER@30..39
                0: JS_BOGUS_BINDING@30..39
                  0: IDENT@30..39 "arguments" [] []
                1: (empty)
                2: (empty)
                3: (empty)
            2: R_PAREN@39..41 ")" [] [Whitespace(" ")]
          6: (empty)

We have the crash here for the NoParameterAssign rule:

pub fn as_binding(&self, binding: &impl IsBindingAstNode) -> Binding {
let range = binding.syntax().text_range();
let id = &self.data.bindings_by_range[&range];
Binding {
data: self.data.clone(),
index: (*id).into(),
}
}

I believe that we can ignore any bogus nodes while we do an analysis.

Test Plan

cargo test

Changelog

  • The PR requires a changelog line

Documentation

  • The PR requires documentation
  • I will create a new PR to update the documentation

@netlify
Copy link

netlify bot commented Jun 11, 2023

Deploy Preview for docs-rometools canceled.

Name Link
🔨 Latest commit dc6a766
🔍 Latest deploy log https://app.netlify.com/sites/docs-rometools/deploys/6486c404dadd0a0008bf8c82

Copy link
Contributor

@ematipico ematipico left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. The analyzer ignores broken syntax, so it makes sense that the semantic model did the same

CHANGELOG.md Outdated Show resolved Hide resolved
@denbezrukov denbezrukov merged commit e0f5784 into main Jun 12, 2023
17 checks passed
@denbezrukov denbezrukov deleted the fix/binding branch June 12, 2023 07:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-CLI Area: CLI A-Linter Area: linter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants