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 codemod to replace hasattr(obj, "__call__") with callable #329

Merged
merged 2 commits into from
Mar 6, 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
32 changes: 32 additions & 0 deletions integration_tests/test_fix_hasattr_call.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
from codemodder.codemods.test import (
BaseIntegrationTest,
original_and_expected_from_code_path,
)
from core_codemods.fix_hasattr_call import TransformFixHasattrCall


class TestTransformFixHasattrCall(BaseIntegrationTest):
codemod = TransformFixHasattrCall
code_path = "tests/samples/fix_hasattr_call.py"
original_code, expected_new_code = original_and_expected_from_code_path(
code_path,
[
(4, """callable(obj)\n"""),
],
)

# fmt: off
expected_diff =(
"""--- \n"""
"""+++ \n"""
"""@@ -2,4 +2,4 @@\n"""
""" pass\n"""
""" \n"""
""" obj = Test()\n"""
"""-hasattr(obj, "__call__")\n"""
"""+callable(obj)\n"""
)
# fmt: on

expected_line_change = "5"
change_description = TransformFixHasattrCall.change_description
4 changes: 4 additions & 0 deletions src/codemodder/scripts/generate_docs.py
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,10 @@ class DocMetadata:
importance="Low",
guidance_explained="This codemod is a great starting point for models with few fields. We encourage you to write custom `__str__` methods that best suit your Django application.",
),
"fix-hasattr-call": DocMetadata(
importance="Low",
guidance_explained="We believe this change is safe because using `callable` is a more reliable way to check if an object is a callable.",
),
}

METADATA = CORE_METADATA | {
Expand Down
2 changes: 2 additions & 0 deletions src/core_codemods/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from .fix_deprecated_abstractproperty import FixDeprecatedAbstractproperty
from .fix_deprecated_logging_warn import FixDeprecatedLoggingWarn
from .fix_empty_sequence_comparison import FixEmptySequenceComparison
from .fix_hasattr_call import TransformFixHasattrCall
from .fix_mutable_params import FixMutableParams
from .flask_enable_csrf_protection import FlaskEnableCSRFProtection
from .flask_json_response_type import FlaskJsonResponseType
Expand Down Expand Up @@ -121,6 +122,7 @@
StrConcatInSeqLiteral,
FixAsyncTaskInstantiation,
DjangoModelWithoutDunderStr,
TransformFixHasattrCall,
],
)

Expand Down
11 changes: 11 additions & 0 deletions src/core_codemods/docs/pixee_python_fix-hasattr-call.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
This codemod fixes cases where `hasattr` is used to check if an object is a callable. You likely want to use `callable` instead. This is because using `hasattr` will return different results in some cases, such as when the class implements a `__getattr__` method.

Our changes look something like this:
```diff
class Test:
pass

obj = Test()
- hasattr(obj, "__call__")
+ callable(obj)
```
29 changes: 29 additions & 0 deletions src/core_codemods/fix_hasattr_call.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import libcst as cst

from core_codemods.api import Metadata, Reference, ReviewGuidance, SimpleCodemod


class TransformFixHasattrCall(SimpleCodemod):
metadata = Metadata(
name="fix-hasattr-call",
summary="Use `callable` builtin to check for callables",
review_guidance=ReviewGuidance.MERGE_WITHOUT_REVIEW,
references=[
Reference(url="https://docs.python.org/3/library/functions.html#callable"),
Reference(url="https://docs.python.org/3/library/functions.html#hasattr"),
],
)
detector_pattern = """
- patterns:
- pattern: hasattr(..., "__call__")
- pattern-not: $MODULE.hasattr(...)
"""

change_description = "Replace `hasattr` function call with `callable`"

def on_result_found(self, original_node, updated_node):
del original_node
return updated_node.with_changes(
func=updated_node.func.with_changes(value="callable"),
args=[updated_node.args[0].with_changes(comma=cst.MaybeSentinel.DEFAULT)],
)
79 changes: 79 additions & 0 deletions tests/codemods/test_fix_hasattr_call.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
from codemodder.codemods.test import BaseSemgrepCodemodTest
from core_codemods.fix_hasattr_call import TransformFixHasattrCall


class TestTransformFixHasattrCall(BaseSemgrepCodemodTest):
codemod = TransformFixHasattrCall

def test_name(self):
assert self.codemod.name == "fix-hasattr-call"

def test_combine(self, tmpdir):
input_code = """
class Test:
pass

hasattr(Test(), "__call__")
hasattr("hi", '__call__')

assert hasattr(1, '__call__')
obj = Test()
var = hasattr(obj, "__call__")

if hasattr(obj, "__call__"):
print(1)
"""
expected = """
class Test:
pass

callable(Test())
callable("hi")

assert callable(1)
obj = Test()
var = callable(obj)

if callable(obj):
print(1)
"""
self.run_and_assert(tmpdir, input_code, expected, num_changes=5)

def test_other_hasattr(self, tmpdir):
code = """
from myscript import hasattr

class Test:
pass

hasattr(Test(), "__call__")
hasattr("hi", '__call__')
"""
self.run_and_assert(tmpdir, code, code)

def test_other_attr(self, tmpdir):
code = """
class Test:
pass

hasattr(Test(), "__repr__")
hasattr("hi", '__repr__')
"""
self.run_and_assert(tmpdir, code, code)

def test_exclude_line(self, tmpdir):
input_code = (
expected
) = """
class Test:
pass
obj = Test()
hasattr(obj, "__call__")
"""
lines_to_exclude = [5]
self.run_and_assert(
tmpdir,
input_code,
expected,
lines_to_exclude=lines_to_exclude,
)
5 changes: 5 additions & 0 deletions tests/samples/fix_hasattr_call.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class Test:
pass

obj = Test()
hasattr(obj, "__call__")
Loading