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

feat(rome_js_analyze): noClassAssign rule #4033

Merged
merged 4 commits into from
Dec 16, 2022

Conversation

kaioduarte
Copy link
Contributor

Summary

Closes #3970.

Test Plan

cargo test -p rome_js_analyze -- no_class_assign

@kaioduarte kaioduarte requested review from leops, ematipico, xunilrj and a team as code owners December 10, 2022 23:16
@netlify
Copy link

netlify bot commented Dec 10, 2022

Deploy Preview for docs-rometools ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 0deb4d7
🔍 Latest deploy log https://app.netlify.com/sites/docs-rometools/deploys/639c4c4a42b7f50008c837a8
😎 Deploy Preview https://deploy-preview-4033--docs-rometools.netlify.app
📱 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.

@ematipico
Copy link
Contributor

ematipico commented Dec 12, 2022

@kaioduarte could you please comment on the issue so I can assign it to you? This will help to show the contributors that the issue is taken already

@kaioduarte kaioduarte force-pushed the feat/no-class-assign branch 2 times, most recently from 215b9d6 to f65086e Compare December 12, 2022 23:07
let node = ctx.query();
let model = ctx.model();

if let Ok(Some(id)) = node.id() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice!
Nit: feel free to ignore:

node.id().and_then(|id| {
    id?.as_js_identifier_binding()?.all_writes(model).collect()
}).unwrap_or_default()

You can avoid the collect() and the allocation with

type Signals = AllBindingsWriteReferenceIter;

@MichaReiser MichaReiser added A-Linter Area: linter L-JavaScript Langauge: JavaScript labels Dec 15, 2022
@MichaReiser MichaReiser added this to the Next milestone Dec 15, 2022
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.

I wonder if there's a way to avoid merge conflicts when merging rules :(
PR looks good, but needs to rebased now and regenerate the files

@xunilrj xunilrj merged commit 128a684 into rome:main Dec 16, 2022
@kaioduarte kaioduarte deleted the feat/no-class-assign branch December 16, 2022 13:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Linter Area: linter L-JavaScript Langauge: JavaScript
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

noClassAssign, no-class-assign
5 participants
点击 这是indexloc提供的php浏览器服务,不要输入任何密码和下载