Skip to content

Commit

Permalink
Merge pull request #50 from SuadeLabs/49-fix-constant-in-namable-erro…
Browse files Browse the repository at this point in the history
…r-in-017

Fix constant in namable error in 0.1.7
  • Loading branch information
BPHarris authored Jul 24, 2023
2 parents 9af861b + e995bf7 commit 84a6898
Show file tree
Hide file tree
Showing 4 changed files with 135 additions and 20 deletions.
2 changes: 1 addition & 1 deletion rattr/analyser/context/context.py
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,7 @@ def register_AnyAssign(self, node: AnyAssign) -> None:

if namedtuple_in_rhs(node):
if not assignment_is_one_to_one(node):
error.fatal("namedtuple assignment must be one-to-one", node)
error.fatal("namedtuple assignment must be one-to-one", culprit=node)

name = get_fullname(targets[0])
self.add(Class(name, args=get_namedtuple_attrs_from_call(node)))
Expand Down
2 changes: 1 addition & 1 deletion rattr/analyser/function.py
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ def visit_NamedTupleAssign(self, node: AnyAssign, targets: List[ast.expr]) -> No
target = targets[0]

if not assignment_is_one_to_one(node):
error.fatal("namedtuple assignment must be one-to-one", node)
error.fatal("namedtuple assignment must be one-to-one", culprit=node)

name = get_fullname(target)
cls = Class(name, args=get_namedtuple_attrs_from_call(node))
Expand Down
66 changes: 48 additions & 18 deletions rattr/analyser/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -819,19 +819,57 @@ def _target_is_namedtuple(call: ast.Call) -> bool:
# This is a naive approach, when enum / class-namedtuple / etc detection is
# refactored we can do this a bit better.
# See: cls.py::is_enum, cls.py::is_namedtuple
name = get_fullname(call.func)
name = get_fullname(call.func, safe=True)
return name == "namedtuple" or name.endswith(".namedtuple")

if isinstance(node.value, ast.Call):
return _target_is_namedtuple(node.value)
elif isinstance(node.value, _iterable):
return any(
_target_is_namedtuple(v) for v in node.value.elts if isinstance(v, ast.Call)
_target_is_namedtuple(value)
for value in node.value.elts
if isinstance(value, ast.Call)
)
else:
return False


def _attrs_from_list_of_strings(attrs_argument: ast.List) -> list[str]:
attrs: list[str] = [
arg.value
for arg in attrs_argument.elts
if isinstance(arg, ast.Constant)
if isinstance(arg.value, str)
]

if len(attrs) != len(attrs_argument.elts):
raise SyntaxError

return attrs


def _attrs_from_space_delimited_string(attrs_argument: ast.Constant) -> list[str]:
if not isinstance(attrs_argument.value, str):
raise SyntaxError

attrs = attrs_argument.value.split(" ")

if not all(attr.isidentifier() for attr in attrs):
raise SyntaxError

return attrs


def _namedtuple_attrs_from_second_argument(attrs_argument: ast.AST) -> list[str]:
if isinstance(attrs_argument, ast.List):
return _attrs_from_list_of_strings(attrs_argument)

if isinstance(attrs_argument, ast.Constant):
return _attrs_from_space_delimited_string(attrs_argument)

raise SyntaxError


def get_namedtuple_attrs_from_call(node: AnyAssign) -> tuple[list[str], dict[str, str]]:
"""Return the args/attrs of the namedtuple constructed by this assignment by call.
Expand All @@ -853,8 +891,9 @@ def get_namedtuple_attrs_from_call(node: AnyAssign) -> tuple[list[str], dict[str
"namedtuple expects exactly two positional arguments (i.e. name, attrs)"
)
_invalid_second_parameter_value_error = (
"namedtuple expects the second positional argument to be a list of "
"string-literals"
"namedtuple expects the second positional argument to be a list of valid "
"identifiers as either a list of string-literals or a space-delimited "
"string-literal"
)

# Parse call arguments
Expand All @@ -864,23 +903,14 @@ def get_namedtuple_attrs_from_call(node: AnyAssign) -> tuple[list[str], dict[str
namedtuple_call_arguments = node.value.args

if len(namedtuple_call_arguments) != 2:
error.fatal(_invalid_signature_error)
error.fatal(_invalid_signature_error, culprit=node.value)

_, namedtuple_attrs_argument = namedtuple_call_arguments

if not isinstance(namedtuple_attrs_argument, ast.List):
error.fatal(_invalid_second_parameter_value_error)

# Get attribute names from second positional argument
attrs: list[str] = [
arg.value
for arg in namedtuple_attrs_argument.elts
if isinstance(arg, ast.Constant)
if isinstance(arg.value, str)
]

if len(attrs) != len(namedtuple_attrs_argument.elts):
error.fatal(_invalid_second_parameter_value_error)
try:
attrs = _namedtuple_attrs_from_second_argument(namedtuple_attrs_argument)
except SyntaxError:
error.fatal(_invalid_second_parameter_value_error, culprit=node.value)

return ["self", *attrs]

Expand Down
85 changes: 85 additions & 0 deletions tests/test_regressions.py
Original file line number Diff line number Diff line change
Expand Up @@ -736,6 +736,51 @@ def fn():
assert file_ir._file_ir == expected
assert _exit.call_count == 1

def test_call_with_space_delimited_string_argument(self, constant, parse):
_ast = parse(
"""
from collections import namedtuple
point = namedtuple("point", "x y")
def my_function():
return point(1, y=2)
"""
)

# This failed at analyse, but we should show that simplification also works
with mock.patch("sys.exit") as _exit:
file_ir = FileAnalyser(_ast, RootContext(_ast)).analyse()
_ = generate_results_from_ir(file_ir, imports_ir={})

point = Class("point", args=["self", "x", "y"])
point_call = Call(
name="point()",
args=["@ReturnValue", constant("Str")],
kwargs={"y": constant("Str")},
target=point,
)

my_function = Func("my_function", [])

expected = {
point: {
"gets": set(),
"sets": set(),
"dels": set(),
"calls": set(),
},
my_function: {
"gets": set(),
"sets": set(),
"dels": set(),
"calls": {point_call},
},
}

assert file_ir._file_ir == expected
assert _exit.call_count == 0


class TestNamedTupleFromInheritance:
# Test namedtuples constructed by inheriting from `typing.NamedTuple`.
Expand Down Expand Up @@ -877,3 +922,43 @@ def by_mixture():

assert file_ir._file_ir == expected
assert _exit.call_count == 0


class TestRattrConstantInNameableOnCheckForNamedTuple:
"""
Brief:
On checking if there is a named-tuple in the right-hand side of an expression,
when the right hand side is a non-safely nameable expression an error is given.
Introduced: 0.1.7
Fixed: 0.1.8
"""

def test_safely_determine_rhs_name_on_namedtuple_check(self, parse, constant):
_ast = parse(
"""
def my_function(foobar, info=None):
i_cause_the_error = "_".join(info.parts)
"""
)

with mock.patch("sys.exit") as _exit:
file_ir = FileAnalyser(_ast, RootContext(_ast)).analyse()
_ = generate_results_from_ir(file_ir, imports_ir={})

my_function = Func("my_function", ["foobar", "info"])

string_join = f"{constant('@Str')}.join()"
string_join_call = Call(string_join, ["info.parts"], {})

expected_file_ir = {
my_function: {
"gets": {Name("info.parts", basename="info")},
"sets": {Name("i_cause_the_error")},
"dels": set(),
"calls": {string_join_call},
}
}

assert not _exit.called
assert file_ir._file_ir == expected_file_ir

0 comments on commit 84a6898

Please sign in to comment.