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

feat(rome_js_analyze): new rule useFlatMap #3384

Merged
merged 6 commits into from
Oct 17, 2022
Merged

Conversation

ematipico
Copy link
Contributor

@ematipico ematipico commented Oct 10, 2022

Summary

Closes #3367

The rule checks for usages like arr.map(function).flat(function) and suggests the replace it with arr.flatMap(function) instead.

Clippy marks the code action as safe, so I suppose we can do that too? Please let me know if you think that the code action should be suggested.

Test Plan

Added some simple case.
The rule should not be triggered if at the functionflat are passed some argument.

@ematipico ematipico requested a review from a team October 10, 2022 15:09
@netlify
Copy link

netlify bot commented Oct 10, 2022

Deploy Preview for rometools ready!

Name Link
🔨 Latest commit ab4dfa8
🔍 Latest deploy log https://app.netlify.com/sites/rometools/deploys/634d57f5695081000923e8cb
😎 Deploy Preview https://deploy-preview-3384--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 ematipico force-pushed the feature/use-flat-map branch from 9aaa3d2 to 7199747 Compare October 10, 2022 15:10
@ematipico ematipico temporarily deployed to netlify-playground October 10, 2022 15:10 Inactive
@github-actions
Copy link

github-actions bot commented Oct 10, 2022

Copy link
Contributor

@leops leops left a comment

Choose a reason for hiding this comment

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

I think the order of operations is reversed, the rule is searching for .flat().map() when it should be checking for .map().flat()

@ematipico ematipico force-pushed the feature/use-flat-map branch from be24fff to 36031ad Compare October 11, 2022 10:48
@ematipico ematipico temporarily deployed to netlify-playground October 11, 2022 10:49 Inactive
@ematipico ematipico force-pushed the feature/use-flat-map branch from 36031ad to a19bafa Compare October 11, 2022 10:50
@ematipico ematipico temporarily deployed to netlify-playground October 11, 2022 10:50 Inactive
@ematipico ematipico temporarily deployed to netlify-playground October 12, 2022 08:14 Inactive
@ematipico ematipico requested a review from leops October 12, 2022 08:14
@ematipico ematipico force-pushed the feature/use-flat-map branch from a8b9526 to c2a392c Compare October 12, 2022 08:18
@ematipico ematipico temporarily deployed to netlify-playground October 12, 2022 08:18 Inactive
Copy link
Contributor

@MichaReiser MichaReiser left a comment

Choose a reason for hiding this comment

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

Looking good to me. I would recommend adding a few more tests and more sophisticated heuristics to avoid false positives.

Can you update the PR description to reflect that the rule is testing for .map(...).flat()?

@ematipico ematipico temporarily deployed to netlify-playground October 12, 2022 14:16 Inactive
@ematipico ematipico force-pushed the feature/use-flat-map branch from 8d59827 to 46b5458 Compare October 12, 2022 14:17
@ematipico ematipico temporarily deployed to netlify-playground October 12, 2022 14:17 Inactive
@ematipico ematipico temporarily deployed to netlify-playground October 12, 2022 14:40 Inactive
@ematipico ematipico force-pushed the feature/use-flat-map branch from 002a6ba to ab4dfa8 Compare October 17, 2022 13:26
@ematipico ematipico temporarily deployed to netlify-playground October 17, 2022 13:26 Inactive
@ematipico
Copy link
Contributor Author

@leops could you please review again the PR?

@ematipico ematipico merged commit 004d7b9 into main Oct 17, 2022
@ematipico ematipico deleted the feature/use-flat-map branch October 17, 2022 13:39
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.

map_flatten
3 participants
点击 这是indexloc提供的php浏览器服务,不要输入任何密码和下载