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

🐛 rule style/noNonNullAssertions emit incorrect code action #4348

Closed
1 task done
ematipico opened this issue Apr 4, 2023 · 1 comment · Fixed by #4414
Closed
1 task done

🐛 rule style/noNonNullAssertions emit incorrect code action #4348

ematipico opened this issue Apr 4, 2023 · 1 comment · Fixed by #4414
Assignees
Labels
A-Linter Area: linter S-Bug: confirmed Status: report has been confirmed as a valid bug

Comments

@ematipico
Copy link
Contributor

ematipico commented Apr 4, 2023

Environment information

Rome v12

What happened?

The rule style/noNonNullAssertions emits an incorrect code action. In particular, its AST is wrong!

Given this code:

session!.hey

It seems to return a correct code:

session?.hey

Although, the AST is wrong:

CommitChange {
            parent_depth: 11,
            parent: Some(
                0: TS_NON_NULL_ASSERTION_EXPRESSION@23..31
                  0: JS_IDENTIFIER_EXPRESSION@23..30
                    0: JS_REFERENCE_IDENTIFIER@23..30
                      0: IDENT@23..30 "session" [] []
                  1: BANG@30..31 "!" [] []
                ,
            ),
            parent_range: Some(
                (
                    23,
                    31,
                ),
            ),
            new_node_slot: 1,
            new_node: Some(
                Token(
                    QUESTION@0..1 "?" [] [],
                ),
            ),
        },

Which is wrong and it results in an endless loop when calling rome check --apply-unsafe because there's always a TS_NON_NULL_ASSERTION_EXPRESSION.

The correct way is to return a new code, which is a JsStaticMemberExpression. Playground

Expected result

The rule to emit a correct AST and not result in an endless loop when calling rome check --apply-unsafe

Code of Conduct

  • I agree to follow Rome's Code of Conduct
@ematipico ematipico added S-To triage Status: user report of a possible bug that needs to be triaged S-Bug: confirmed Status: report has been confirmed as a valid bug A-Linter Area: linter and removed S-To triage Status: user report of a possible bug that needs to be triaged labels Apr 4, 2023
@ematipico
Copy link
Contributor Author

@notmd cc you as you worked on the rule, in case you are interested at applying the fix

@Conaclos Conaclos self-assigned this Apr 27, 2023
Conaclos added a commit that referenced this issue Apr 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Linter Area: linter S-Bug: confirmed Status: report has been confirmed as a valid bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants