Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New find and fix and sonar codemod for "inverted boolean checks" #790

Merged
merged 4 commits into from
Aug 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/codemodder/scripts/generate_docs.py
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,10 @@ class DocMetadata:
importance="High",
guidance_explained="This change may disable a feature that was left on purpose. Moreover the rule added may be framework specific.",
),
"invert-boolean-check": DocMetadata(
importance="Low",
guidance_explained="We believe this replacement is safe and should not result in any issues.",
),
}
DEFECTDOJO_CODEMODS = {
"django-secure-set-cookie": DocMetadata(
Expand Down Expand Up @@ -310,6 +314,7 @@ class DocMetadata:
"django-model-without-dunder-str",
"break-or-continue-out-of-loop",
"disable-graphql-introspection",
"invert-boolean-check",
]
SONAR_CODEMODS = {
name: DocMetadata(
Expand Down
4 changes: 4 additions & 0 deletions src/core_codemods/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
from .harden_pyyaml import HardenPyyaml
from .harden_ruamel import HardenRuamel
from .https_connection import HTTPSConnection
from .invert_boolean_check import InvertedBooleanCheck
from .jwt_decode_verify import JwtDecodeVerify
from .lazy_logging import LazyLogging
from .limit_readline import LimitReadline
Expand Down Expand Up @@ -79,6 +80,7 @@
from .sonar.sonar_fix_math_isclose import SonarFixMathIsClose
from .sonar.sonar_fix_missing_self_or_cls import SonarFixMissingSelfOrCls
from .sonar.sonar_flask_json_response_type import SonarFlaskJsonResponseType
from .sonar.sonar_invert_boolean_check import SonarInvertedBooleanCheck
from .sonar.sonar_jwt_decode_verify import SonarJwtDecodeVerify
from .sonar.sonar_literal_or_new_object_identity import SonarLiteralOrNewObjectIdentity
from .sonar.sonar_numpy_nan_equality import SonarNumpyNanEquality
Expand Down Expand Up @@ -167,6 +169,7 @@
FixMathIsClose,
BreakOrContinueOutOfLoop,
DisableGraphQLIntrospection,
InvertedBooleanCheck,
],
)

Expand All @@ -193,6 +196,7 @@
SonarDjangoModelWithoutDunderStr,
SonarBreakOrContinueOutOfLoop,
SonarDisableGraphQLIntrospection,
SonarInvertedBooleanCheck,
],
)

Expand Down
10 changes: 10 additions & 0 deletions src/core_codemods/docs/pixee_python_invert-boolean-check.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
This codemod flips boolean `not` comparisons to their more readable equivalent comparisons.

The changes from this codemod look like this:

```diff
- assert not user_input == "yes"
- z = not m <= n
+ assert user_input != "yes"
+ z = m > n
```
83 changes: 83 additions & 0 deletions src/core_codemods/invert_boolean_check.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
import libcst as cst

from codemodder.codemods.libcst_transformer import (
LibcstResultTransformer,
LibcstTransformerPipeline,
)
from core_codemods.api import CoreCodemod, Metadata, ReviewGuidance


class InvertedBooleanCheckTransformer(LibcstResultTransformer):
change_description = "Flips inverted boolean check."

def leave_UnaryOperation(
self, original_node: cst.UnaryOperation, updated_node: cst.UnaryOperation
) -> cst.BaseExpression:
if not self.node_is_selected(original_node):
return updated_node

if isinstance(updated_node.operator, cst.Not) and isinstance(
(comparison := updated_node.expression), cst.Comparison
):
return self.report_new_comparison(original_node, comparison)
return updated_node

def report_new_comparison(
self, original_node: cst.UnaryOperation, comparison: cst.Comparison
) -> cst.BaseExpression:
if len(comparison.comparisons) == 1 and isinstance(
comparison.comparisons[0].operator, cst.Is
):
# Handle 'not status is True' -> 'not status'
if comparison.comparisons[0].comparator.value == "True":
self.report_change(original_node)
return cst.UnaryOperation(
operator=cst.Not(), expression=comparison.left
)

# Handle 'not status is False' -> 'status'
if comparison.comparisons[0].comparator.value == "False":
self.report_change(original_node)
return comparison.left

inverted_comparisons = self._invert_comparisons(comparison)

self.report_change(original_node)
return cst.Comparison(left=comparison.left, comparisons=inverted_comparisons)

def _invert_comparisons(
self, comparison: cst.Comparison
) -> list[cst.ComparisonTarget]:
inverted_comparisons = []
for comparison_op in comparison.comparisons:
match comparison_op.operator:
case cst.Equal():
new_operator = cst.NotEqual()
case cst.NotEqual():
new_operator = cst.Equal()
case cst.LessThan():
new_operator = cst.GreaterThanEqual()
case cst.GreaterThan():
new_operator = cst.LessThanEqual()
case cst.LessThanEqual():
new_operator = cst.GreaterThan()
case cst.GreaterThanEqual():
new_operator = cst.LessThan()
case _:
new_operator = comparison_op

inverted_comparisons.append(
comparison_op.with_changes(operator=new_operator)
)
return inverted_comparisons


InvertedBooleanCheck = CoreCodemod(
metadata=Metadata(
name="invert-boolean-check",
summary="Invert Boolean Check",
review_guidance=ReviewGuidance.MERGE_WITHOUT_REVIEW,
references=[],
),
transformer=LibcstTransformerPipeline(InvertedBooleanCheckTransformer),
)
9 changes: 9 additions & 0 deletions src/core_codemods/sonar/sonar_invert_boolean_check.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
from core_codemods.invert_boolean_check import InvertedBooleanCheck
from core_codemods.sonar.api import SonarCodemod

SonarInvertedBooleanCheck = SonarCodemod.from_core_codemod(
name="invert-boolean-check",
other=InvertedBooleanCheck,
rule_id="python:S1940",
rule_name="Boolean checks should not be inverted",
)
55 changes: 55 additions & 0 deletions tests/codemods/sonar/test_sonar_invert_boolean_check.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import json

from codemodder.codemods.test import BaseSASTCodemodTest
from core_codemods.sonar.sonar_invert_boolean_check import SonarInvertedBooleanCheck


class TestSonarInvertedBooleanCheck(BaseSASTCodemodTest):
codemod = SonarInvertedBooleanCheck
tool = "sonar"

def test_name(self):
assert self.codemod.name == "invert-boolean-check"

def test_simple(self, tmpdir):
input_code = """
if not a == 2:
b = not i < 10
"""
expected_output = """
if a != 2:
b = i >= 10
"""
issues = {
"issues": [
{
"rule": "python:S1940",
"status": "OPEN",
"component": "code.py",
"textRange": {
"startLine": 2,
"endLine": 2,
"startOffset": 3,
"endOffset": 13,
},
},
{
"rule": "python:S1940",
"status": "OPEN",
"component": "code.py",
"textRange": {
"startLine": 3,
"endLine": 3,
"startOffset": 8,
"endOffset": 18,
},
},
]
}
self.run_and_assert(
tmpdir,
input_code,
expected_output,
results=json.dumps(issues),
num_changes=2,
)
94 changes: 94 additions & 0 deletions tests/codemods/test_invert_boolean_check.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
from codemodder.codemods.test import BaseCodemodTest
from core_codemods.invert_boolean_check import InvertedBooleanCheck


class TestInvertedBooleanCheck(BaseCodemodTest):
codemod = InvertedBooleanCheck

def test_no_change(self, tmpdir):
before = """
if not True:
pass

assert not hello()
"""
self.run_and_assert(tmpdir, before, before)

def test_change(self, tmpdir):
before = """
a = 5
i = 20
x = a
y = x
z = x
n = 5
m = 0
status = True


def finished():
return True


user_input = "hi"

if not a == 2:
b = not i < 10

if not x != y:
z = not m <= n

if not status is True:
pass

if not finished() is False:
pass

assert not user_input == "yes"
"""
after = """
a = 5
i = 20
x = a
y = x
z = x
n = 5
m = 0
status = True


def finished():
return True


user_input = "hi"

if a != 2:
b = i >= 10

if x == y:
z = m > n

if not status:
pass

if finished():
pass

assert user_input != "yes"
"""
self.run_and_assert(tmpdir, before, after, num_changes=7)

def test_exclude_line(self, tmpdir):
input_code = (
expected
) = """
b = not i < 10
"""
lines_to_exclude = [2]
self.run_and_assert(
tmpdir,
input_code,
expected,
lines_to_exclude=lines_to_exclude,
)
Loading