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

feat(rome_js_analyze): rule noVoid #4619

Merged
merged 4 commits into from
Jun 26, 2023
Merged

feat(rome_js_analyze): rule noVoid #4619

merged 4 commits into from
Jun 26, 2023

Conversation

ematipico
Copy link
Contributor

Summary

Implements the rule noVoid

Test Plan

Added few test cases

@netlify
Copy link

netlify bot commented Jun 26, 2023

Deploy Preview for docs-rometools ready!

Name Link
🔨 Latest commit 56aceea
🔍 Latest deploy log https://app.netlify.com/sites/docs-rometools/deploys/64996726c3fc680008c5cec2
😎 Deploy Preview https://deploy-preview-4619--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 ematipico force-pushed the feat/no-void branch 2 times, most recently from ee9570f to aad55e5 Compare June 26, 2023 08:16
@ematipico ematipico requested a review from Conaclos June 26, 2023 08:16
@github-actions github-actions bot added A-Diagnostic Area: errors and diagnostics A-Linter Area: linter A-Parser Area: parser A-Project Area: project configuration and loading labels Jun 26, 2023
@github-actions
Copy link

Parser conformance results on ubuntu-latest

js/262

Test result main count This PR count Difference
Total 48863 48863 0
Passed 47810 47810 0
Failed 1053 1053 0
Panics 0 0 0
Coverage 97.84% 97.84% 0.00%

jsx/babel

Test result main count This PR count Difference
Total 40 40 0
Passed 37 37 0
Failed 3 3 0
Panics 0 0 0
Coverage 92.50% 92.50% 0.00%

symbols/microsoft

Test result main count This PR count Difference
Total 6212 6212 0
Passed 1764 1764 0
Failed 4448 4448 0
Panics 0 0 0
Coverage 28.40% 28.40% 0.00%

ts/babel

Test result main count This PR count Difference
Total 639 639 0
Passed 573 573 0
Failed 66 66 0
Panics 0 0 0
Coverage 89.67% 89.67% 0.00%

ts/microsoft

Test result main count This PR count Difference
Total 17224 17224 0
Passed 13121 13121 0
Failed 4103 4103 0
Panics 0 0 0
Coverage 76.18% 76.18% 0.00%

Copy link
Contributor

@Conaclos Conaclos left a comment

Choose a reason for hiding this comment

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

Should we accept cases where void is used as a statement?

void f()

This is sometimes used to explicitly drop the returned value of a function call.
However, this is not a widespread pattern.
I think we can keep the rule as it is.

@ematipico
Copy link
Contributor Author

Should we accept cases where void is used as a statement?

void f()

This is sometimes used to explicitly drop the returned value of a function call. However, this is not a widespread pattern. I think we can keep the rule as it is.

Since this is not a widespread pattern, I think it's better to have a suppression comment that explain the case.

@ematipico ematipico force-pushed the feat/no-void branch 2 times, most recently from 37afa06 to 0d839ab Compare June 26, 2023 10:16
@ematipico ematipico requested a review from Conaclos June 26, 2023 10:17
@ematipico ematipico merged commit d639f43 into main Jun 26, 2023
@ematipico ematipico deleted the feat/no-void branch June 26, 2023 10:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Diagnostic Area: errors and diagnostics A-Linter Area: linter A-Parser Area: parser A-Project Area: project configuration and loading
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants