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

πŸ› noNegationElse is not considering double negation #3141

Closed
1 task done
xunilrj opened this issue Aug 31, 2022 · 3 comments Β· Fixed by #3491
Closed
1 task done

πŸ› noNegationElse is not considering double negation #3141

xunilrj opened this issue Aug 31, 2022 · 3 comments Β· Fixed by #3491
Assignees
Labels
A-Linter Area: linter S-Bug: confirmed Status: report has been confirmed as a valid bug S-Planned Status: planned by the team, but not in the short term
Milestone

Comments

@xunilrj
Copy link
Contributor

xunilrj commented Aug 31, 2022

Environment information

This is the result on the branch main.

What happened?

error[js/noNegationElse]: Invert blocks when performing a negation test.
   β”Œβ”€ ./src/components/container.jsx:28:14
   β”‚
28 β”‚       return !!specs.variables ? specs.variables(props) : {};
   β”‚              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Suggested fix: Exchange alternate and consequent of the node
      | @@ -25,7 +25,7 @@
24 24 |       );
25 25 |   
26 26 |       function mapPropsToVariables(props) {
27    | -       return !!specs.variables ? specs.variables(props) : {};
   27 | +       return !specs.variables ? {} : specs.variables(props);
28 28 |       }
29 29 |   
30 30 |       return class extends Component {

This is also in conflict with:

error[js/noExtraBooleanCast]: Avoid redundant double-negation.
   β”Œβ”€ ./src/components/container.jsx:28:14
   β”‚
28 β”‚       return !!specs.variables ? specs.variables(props) : {};
   β”‚              ^^^^^^^^^^^^^^^^^

Suggested fix: Remove redundant double-negation
      | @@ -25,7 +25,7 @@
24 24 |       );
25 25 |   
26 26 |       function mapPropsToVariables(props) {
27    | -       return !!specs.variables ? specs.variables(props) : {};
   27 | +       return specs.variables ? specs.variables(props) : {};
28 28 |       }
29 29 |   
30 30 |       return class extends Component {

=  note: It is not necessary to use double-negation when a value will already be coerced to a boolean.

Expected result

I am not entirely sure the suggested fix is exactly the same as the original code in all cases.
And secondly, the fix would be flagged again for the same rule.

I would expect the rule to not flag anything using double negation.

Code of Conduct

  • I agree to follow Rome's Code of Conduct
@xunilrj xunilrj added the S-To triage Status: user report of a possible bug that needs to be triaged label Aug 31, 2022
@github-actions
Copy link

This issue is stale because it has been open 14 days with no activity.

@ematipico ematipico added S-Bug: confirmed Status: report has been confirmed as a valid bug A-Linter Area: linter and removed S-To triage Status: user report of a possible bug that needs to be triaged S-Stale labels Sep 15, 2022
@ematipico ematipico added this to the 0.10.0 milestone Sep 15, 2022
@github-actions
Copy link

This issue is stale because it has been open 14 days with no activity.

@ematipico ematipico modified the milestones: 0.10.0, 10.0.0 Oct 3, 2022
@github-actions github-actions bot removed the S-Stale label Oct 4, 2022
@github-actions
Copy link

This issue is stale because it has been open 14 days with no activity.

@ematipico ematipico added S-Planned Status: planned by the team, but not in the short term and removed S-Stale labels Oct 19, 2022
@ematipico ematipico self-assigned this Oct 26, 2022
@ematipico ematipico moved this to Done in Rome 2022 Oct 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Linter Area: linter S-Bug: confirmed Status: report has been confirmed as a valid bug S-Planned Status: planned by the team, but not in the short term
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants