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

feat(rome_analyze): noNegationElse #2655

Merged
merged 20 commits into from
Jun 9, 2022

Conversation

IWANABETHATGUY
Copy link
Contributor

@IWANABETHATGUY IWANABETHATGUY commented Jun 4, 2022

@IWANABETHATGUY IWANABETHATGUY changed the title chore: 🤖 basic finish feat/(rome_analyze):noNegationElse Jun 4, 2022
@IWANABETHATGUY
Copy link
Contributor Author

Found there has some differ from our test case, here is I println result

// valid
if (!true) {consequent;};
true ? consequent : alternate;
// invalid
if (true) {alternate} else {consequent}
!condition ? consequent : alternate;
-----------------------
// valid
if (!true) {consequent;};
true ? consequent : alternate;
// invalid
if (!true) {consequent} else {alternate}
condition ? alternate : consequent;

here is the snapshot result:

Input

// valid
if (!true) {consequent;};
true ? consequent : alternate;
// invalid
if (!true) {consequent} else {alternate}
!condition ? consequent : alternate;

Diagnostics

error[noNegationElse]: Invert blocks when performing a negation test.
  ┌─ noNegationElse.js:5:1
  │
5 │ if (!true) {consequent} else {alternate}
  │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Invert blocks when performing a negation test.

Replace with strict equality


You could see here, the replacement of if (!true) {consequent} else {alternate} is empty,
but the println result is

if (true) {alternate} else {consequent}

error[noNegationElse]: Invert blocks when performing a negation test.
┌─ noNegationElse.js:6:1

6 │ !condition ? consequent : alternate;
│ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Invert blocks when performing a negation test.

Replace with strict equality
| @@ -3,4 +3,4 @@
2 2 | true ? consequent : alternate;
3 3 | // invalid
4 4 | if (!true) {consequent} else {alternate}
5 | - !condition ? consequent : alternate;
5 | + condition ? alternate : consequent;

@IWANABETHATGUY IWANABETHATGUY marked this pull request as ready for review June 8, 2022 12:46
@IWANABETHATGUY
Copy link
Contributor Author

@ematipico , @leops this pull request is ready for reviewing

@ematipico ematipico changed the title feat/(rome_analyze):noNegationElse feat(rome_analyze): noNegationElse Jun 9, 2022
@IWANABETHATGUY IWANABETHATGUY requested a review from leops June 9, 2022 15:43
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.

Once the formatting issues are fixed (just running cargo fmt should take care of it) it should be good to merge

@IWANABETHATGUY
Copy link
Contributor Author

@leops , format done.

@leops leops merged commit 827cbf7 into rome:main Jun 9, 2022
@IWANABETHATGUY IWANABETHATGUY deleted the feat/no-negation-else branch July 28, 2022 07:35
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.

📎 noNegationElse
2 participants