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

feat(rome_js_analyze): Suggested fix for Conditional Assignment #3771

Merged
merged 2 commits into from
Nov 17, 2022
Merged

feat(rome_js_analyze): Suggested fix for Conditional Assignment #3771

merged 2 commits into from
Nov 17, 2022

Conversation

95th
Copy link
Contributor

@95th 95th commented Nov 17, 2022

Summary

Suggested fix for Conditional Assignment. (Followup from #3750 (review))

Test Plan

Added/Updated tests.

@95th 95th requested a review from a team November 17, 2022 01:18
@netlify
Copy link

netlify bot commented Nov 17, 2022

Deploy Preview for docs-rometools ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 1949e64
🔍 Latest deploy log https://app.netlify.com/sites/docs-rometools/deploys/637616248e8135000856af7c
😎 Deploy Preview https://deploy-preview-3771--docs-rometools.netlify.app/playground
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

if let JsAssignmentOperator::Assign = op {
let mut mutation = ctx.root().begin();
let token = state.operator_token().ok()?;
mutation.replace_token(token, make::token(JsSyntaxKind::EQ3));
Copy link
Contributor

Choose a reason for hiding this comment

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

This mutation leaves the JsAssignmentExpression node in place but replaces its operator token with ===. It won't cause any issue with linting right now since the only thing we do with the resulting syntax tree is to print it to a string, and the rule being flagged as MaybeIncorrect means it won't be included in the "Fix All" action. We don't even detect the resulting syntax tree as invalid right now since we don't check if the kind of individual tokens matches the definition for the surrounding AST node, I just thought I should flag this as a potential future issue

@MichaReiser MichaReiser merged commit 9f0ba14 into rome:main Nov 17, 2022
@95th 95th deleted the no_conditional_assignment_action branch November 19, 2022 08:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants