B006 and B008: Cover additional test cases #239
Conversation
| def compose_call_path(self, node): | ||
| if isinstance(node, ast.Attribute): | ||
| yield from self.compose_call_path(node.value) | ||
| yield node.attr | ||
| elif isinstance(node, ast.Call): | ||
| yield from self.compose_call_path(node.func) | ||
| elif isinstance(node, ast.Name): | ||
| yield node.id |
There was a problem hiding this comment.
This was useful in my other visitor and doesn't need to be in this class so separated it out
| def check_for_b006_and_b008(self, node): | ||
| visitor = FuntionDefDefaultsVisitor(self.b008_extend_immutable_calls) | ||
| visitor.visit(node.args.defaults + node.args.kw_defaults) | ||
| self.errors.extend(visitor.errors) |
There was a problem hiding this comment.
Rules now use a visitor to handle functions at any depth
|
This is a big one. Calling in some much more ast savvy people to help review this one. There are some pieces I need to wrap my head around. Will eventually take the time and work it out, but adding others incase they can beat me. |
bugbear.py
Outdated
| float_arg = node.args[0] | ||
| if sys.version_info < (3, 8, 0): | ||
| # NOTE: pre-3.8, string literals are represented with ast.Str | ||
| if isinstance(float_arg, ast.Str): | ||
| str_val = float_arg.s | ||
| else: | ||
| str_val = "" | ||
| else: | ||
| # NOTE: post-3.8, string literals are represented with ast.Constant | ||
| if isinstance(float_arg, ast.Constant): | ||
| str_val = float_arg.value | ||
| if not isinstance(str_val, str): | ||
| str_val = "" | ||
| else: | ||
| str_val = "" | ||
|
|
||
| # NOTE: regex derived from documentation at: | ||
| # https://docs.python.org/3/library/functions.html#float | ||
| inf_nan_regex = r"^[+-]?(inf|infinity|nan)$" | ||
| re_result = re.search(inf_nan_regex, str_val.lower()) | ||
| is_float_literal = re_result is not None | ||
| else: | ||
| is_float_literal = False | ||
|
|
||
| if not is_float_literal: | ||
| self.errors.append(B008(node.lineno, node.col_offset)) |
There was a problem hiding this comment.
I know this was previously existing code, but we can shorten it considerably by using ast.literal_eval():
| float_arg = node.args[0] | |
| if sys.version_info < (3, 8, 0): | |
| # NOTE: pre-3.8, string literals are represented with ast.Str | |
| if isinstance(float_arg, ast.Str): | |
| str_val = float_arg.s | |
| else: | |
| str_val = "" | |
| else: | |
| # NOTE: post-3.8, string literals are represented with ast.Constant | |
| if isinstance(float_arg, ast.Constant): | |
| str_val = float_arg.value | |
| if not isinstance(str_val, str): | |
| str_val = "" | |
| else: | |
| str_val = "" | |
| # NOTE: regex derived from documentation at: | |
| # https://docs.python.org/3/library/functions.html#float | |
| inf_nan_regex = r"^[+-]?(inf|infinity|nan)$" | |
| re_result = re.search(inf_nan_regex, str_val.lower()) | |
| is_float_literal = re_result is not None | |
| else: | |
| is_float_literal = False | |
| if not is_float_literal: | |
| self.errors.append(B008(node.lineno, node.col_offset)) | |
| try: | |
| value = float(ast.literal_eval(node.args[0])) | |
| except Exception: | |
| pass | |
| else: | |
| if math.isfinite(value): | |
| self.errors.append(B008(node.lineno, node.col_offset)) |
(OK, I admit that this no longer warns on a redundant float() call wrapped around an "infinity literal" like 1e999, but IMO that's fine.)
There was a problem hiding this comment.
Thanks! I've updated the code accordingly and it looks a lot nicer now 😄
|
|
||
|
|
||
| def kwonlyargs_mutable(*, value=[]): |
There was a problem hiding this comment.
Do we need an empty list for coverage? I guess you removed this cause of all the list comprehension tests now ...
| pass | ||
|
|
||
|
|
||
| def kwonlyargs_mutable(*, value=[]): |
There was a problem hiding this comment.
@cooperlees Re: the question on removing this. I rearranged this test file to structure it a bit more and help come up with extra test cases. The function is still here 😄
Closes #229.
Detects function calls at any level of the default expression.
This includes instances of B008 which occur within instances of B006 (e.g.
def f(a=[datetime.now()])) since a user may choose to turn one off but still want to be warned of the other.