Pass literals as kwargs #9
Conversation
# Conflicts: # mypy/checkexpr.py
# Conflicts: # mypy/checkexpr.py
|
Could you please merge the conflicts? btw, just one code style nit: how about expand the compound bool expressions into several |
|
Previous PR: python#9629 |
|
Diff from mypy_primer, showing the effect of this PR on open source code: boostedblob (https://github.com/hauntsaninja/boostedblob)
- Warning: unused section(s) in mypy.ini: [mypy-tests.*]
+ boostedblob/globals.py:144: error: INTERNAL ERROR -- Please try using mypy master on GitHub:
+ https://mypy.readthedocs.io/en/stable/common_issues.html#using-a-development-mypy-build
+ Please report a bug at https://github.com/python/mypy/issues
+ version: 0.970+dev.cb7c148c62d50edc1c70943b209b82d1e0940fd7
+ boostedblob/globals.py:144: : note: use --pdb to drop into pdb
+ Traceback (most recent call last):
+ File "", line 8, in <module>
+ sys.exit(console_entry())
+ File "/__main__.py", line 12, in console_entry
+ main(None, sys.stdout, sys.stderr)
+ File "/main.py", line 96, in main
+ res, messages, blockers = run_build(sources, options, fscache, t0, stdout, stderr)
+ File "/main.py", line 173, in run_build
+ res = build.build(sources, options, None, flush_errors, fscache, stdout, stderr)
+ File "/build.py", line 154, in build
+ result = _build(
+ File "/build.py", line 230, in _build
+ graph = dispatch(sources, manager, stdout)
+ File "/build.py", line 2729, in dispatch
+ process_graph(graph, manager)
+ File "/build.py", line 3087, in process_graph
+ process_stale_scc(graph, scc, manager)
+ File "/build.py", line 3185, in process_stale_scc
+ graph[id].type_check_first_pass()
+ File "/build.py", line 2180, in type_check_first_pass
+ self.type_checker().check_first_pass()
+ File "/checker.py", line 325, in check_first_pass
+ self.accept(d)
+ File "/checker.py", line 431, in accept
+ stmt.accept(self)
+ File "/nodes.py", line 840, in accept
+ return visitor.visit_decorator(self)
+ File "/checker.py", line 3984, in visit_decorator
+ self.check_func_item(e.func, name=e.func.name)
+ File "/checker.py", line 848, in check_func_item
+ self.check_func_def(defn, typ, name)
+ File "/checker.py", line 1033, in check_func_def
+ self.accept(item.body)
+ File "/checker.py", line 431, in accept
+ stmt.accept(self)
+ File "/nodes.py", line 1100, in accept
+ return visitor.visit_block(self)
+ File "/checker.py", line 2178, in visit_block
+ self.accept(s)
+ File "/checker.py", line 431, in accept
+ stmt.accept(self)
+ File "/nodes.py", line 1118, in accept
+ return visitor.visit_expression_stmt(self)
+ File "/checker.py", line 3487, in visit_expression_stmt
+ expr_type = self.expr_checker.accept(s.expr, allow_none_return=True, always_allow_any=True)
+ File "/checkexpr.py", line 4006, in accept
+ typ = self.visit_call_expr(node, allow_none_return=True)
+ File "/checkexpr.py", line 305, in visit_call_expr
+ return self.visit_call_expr_inner(e, allow_none_return=allow_none_return)
+ File "/checkexpr.py", line 383, in visit_call_expr_inner
+ ret_type = self.check_call_expr_with_callee_type(callee_type, e, fullname,
+ File "/checkexpr.py", line 892, in check_call_expr_with_callee_type
+ ret_type, callee_type = self.check_call(
+ File "/checkexpr.py", line 960, in check_call
+ return self.check_overload_call(callee, args, arg_kinds, arg_names, callable_name,
+ File "/checkexpr.py", line 1608, in check_overload_call
+ plausible_targets = self.plausible_overload_call_targets(arg_types, arg_kinds,
+ File "/checkexpr.py", line 1737, in plausible_overload_call_targets
+ if self.check_argument_count(typ, arg_types, arg_kinds, arg_names,
+ File "/checkexpr.py", line 1417, in check_argument_count
+ ok, is_unexpected_arg_error = self.check_for_extra_actual_arguments(
+ File "/checkexpr.py", line 1499, in check_for_extra_actual_arguments
+ if messages and supertype.args:
+ NameError: name 'messages' is not defined
mypy_primer (https://github.com/hauntsaninja/mypy_primer)
+ /tmp/mypy_primer/projects/mypy_primer/mypy_primer.py:99: error: INTERNAL ERROR -- Please try using mypy master on GitHub:
+ https://mypy.readthedocs.io/en/stable/common_issues.html#using-a-development-mypy-build
+ Please report a bug at https://github.com/python/mypy/issues
+ version: 0.970+dev.cb7c148c62d50edc1c70943b209b82d1e0940fd7
+ Traceback (most recent call last):
+ File "", line 8, in <module>
+ sys.exit(console_entry())
+ File "/__main__.py", line 12, in console_entry
+ main(None, sys.stdout, sys.stderr)
+ File "/main.py", line 96, in main
+ res, messages, blockers = run_build(sources, options, fscache, t0, stdout, stderr)
+ File "/main.py", line 173, in run_build
+ res = build.build(sources, options, None, flush_errors, fscache, stdout, stderr)
+ File "/build.py", line 154, in build
+ result = _build(
+ File "/build.py", line 230, in _build
+ graph = dispatch(sources, manager, stdout)
+ File "/build.py", line 2729, in dispatch
+ process_graph(graph, manager)
+ File "/build.py", line 3087, in process_graph
+ process_stale_scc(graph, scc, manager)
+ File "/build.py", line 3185, in process_stale_scc
+ graph[id].type_check_first_pass()
+ File "/build.py", line 2180, in type_check_first_pass
+ self.type_checker().check_first_pass()
+ File "/checker.py", line 325, in check_first_pass
+ self.accept(d)
+ File "/checker.py", line 431, in accept
+ stmt.accept(self)
+ File "/nodes.py", line 748, in accept
+ return visitor.visit_func_def(self)
+ File "/checker.py", line 782, in visit_func_def
+ self._visit_func_def(defn)
+ File "/checker.py", line 786, in _visit_func_def
+ self.check_func_item(defn, name=defn.name)
+ File "/checker.py", line 848, in check_func_item
+ self.check_func_def(defn, typ, name)
+ File "/checker.py", line 1033, in check_func_def
+ self.accept(item.body)
+ File "/checker.py", line 431, in accept
+ stmt.accept(self)
+ File "/nodes.py", line 1100, in accept
+ return visitor.visit_block(self)
+ File "/checker.py", line 2178, in visit_block
+ self.accept(s)
+ File "/checker.py", line 431, in accept
+ stmt.accept(self)
+ File "/nodes.py", line 1397, in accept
+ return visitor.visit_with_stmt(self)
+ File "/checker.py", line 4078, in visit_with_stmt
+ self.accept(s.body)
+ File "/checker.py", line 431, in accept
+ stmt.accept(self)
+ File "/nodes.py", line 1100, in accept
+ return visitor.visit_block(self)
+ File "/checker.py", line 2178, in visit_block
+ self.accept(s)
+ File "/checker.py", line 431, in accept
+ stmt.accept(self)
+ File "/nodes.py", line 1324, in accept
+ return visitor.visit_if_stmt(self)
+ File "/checker.py", line 3597, in visit_if_stmt
+ self.accept(b)
+ File "/checker.py", line 431, in accept
+ stmt.accept(self)
+ File "/nodes.py", line 1100, in accept
+ return visitor.visit_block(self)
+ File "/checker.py", line 2178, in visit_block
+ self.accept(s)
+ File "/checker.py", line 431, in accept
+ stmt.accept(self)
+ File "/nodes.py", line 1166, in accept
+ return visitor.visit_assignment_stmt(self)
+ File "/checker.py", line 2220, in visit_assignment_stmt
+ self.check_assignment(s.lvalues[-1], s.rvalue, s.type is None, s.new_syntax)
+ File "/checker.py", line 2414, in check_assignment
+ rvalue_type = self.expr_checker.accept(rvalue)
+ File "/checkexpr.py", line 4012, in accept
+ typ = node.accept(self)
+ File "/nodes.py", line 2508, in accept
+ return visitor.visit_await_expr(self)
+ File "/checkexpr.py", line 4137, in visit_await_expr
+ actual_type = get_proper_type(self.accept(e.expr, expected_type))
+ File "/checkexpr.py", line 4012, in accept
+ typ = node.accept(self)
+ File "/nodes.py", line 1761, in accept
+ return visitor.visit_call_expr(self)
+ File "/checkexpr.py", line 305, in visit_call_expr
+ return self.visit_call_expr_inner(e, allow_none_return=allow_none_return)
+ File "/checkexpr.py", line 383, in visit_call_expr_inner
+ ret_type = self.check_call_expr_with_callee_type(callee_type, e, fullname,
+ File "/checkexpr.py", line 892, in check_call_expr_with_callee_type
+ ret_type, callee_type = self.check_call(
+ File "/checkexpr.py", line 957, in check_call
+ return self.check_callable_call(callee, args, arg_kinds, context, arg_names,
+ File "/checkexpr.py", line 1067, in check_callable_call
+ self.check_argument_count(callee, arg_types, arg_kinds,
+ File "/checkexpr.py", line 1417, in check_argument_count
+ ok, is_unexpected_arg_error = self.check_for_extra_actual_arguments(
+ File "/checkexpr.py", line 1499, in check_for_extra_actual_arguments
+ if messages and supertype.args:
+ NameError: name 'messages' is not defined
ignite (https://github.com/pytorch/ignite)
+ ignite/distributed/comp_models/base.py:175: error: INTERNAL ERROR -- Please try using mypy master on GitHub:
+ https://mypy.readthedocs.io/en/stable/common_issues.html#using-a-development-mypy-build
+ Please report a bug at https://github.com/python/mypy/issues
+ version: 0.970+dev.cb7c148c62d50edc1c70943b209b82d1e0940fd7
+ ignite/distributed/comp_models/base.py:175: : note: use --pdb to drop into pdb
- ignite/engine/deterministic.py:38: error: Class cannot subclass "BatchSampler" (has type "Any") [misc]
- ignite/engine/deterministic.py:227: error: Unused "type: ignore" comment
- ignite/engine/deterministic.py:254: error: Unused "type: ignore" comment
- ignite/engine/deterministic.py:259: error: Unused "type: ignore" comment
- ignite/distributed/auto.py:282: error: Class cannot subclass "DistributedSampler" (has type "Any") [misc]
- ignite/distributed/auto.py:308: error: Unused "type: ignore" comment
- ignite/distributed/auto.py:351: error: Class cannot subclass "Optimizer" (has type "Any") [misc]
- ignite/distributed/auto.py:353: error: Unused "type: ignore" comment
- ignite/metrics/precision.py:50: error: Unused "type: ignore" comment
- ignite/metrics/precision.py:51: error: Unused "type: ignore" comment
- ignite/metrics/metric.py:267: error: Untyped decorator makes function "iteration_completed" untyped [misc]
- ignite/metrics/gan/utils.py:8: error: Class cannot subclass "Module" (has type "Any") [misc]
- ignite/metrics/gan/utils.py:33: error: Untyped decorator makes function "forward" untyped [misc]
- ignite/metrics/gan/utils.py:64: error: Cannot determine type of "_feature_extractor" [has-type]
- ignite/engine/__init__.py:375: error: Unused "type: ignore" comment
- ignite/handlers/param_scheduler.py:854: error: Unused "type: ignore" comment
- ignite/handlers/param_scheduler.py:864: error: Unused "type: ignore" comment
- ignite/handlers/param_scheduler.py:870: error: Unused "type: ignore" comment
- ignite/handlers/param_scheduler.py:875: error: Unused "type: ignore" comment
- ignite/handlers/param_scheduler.py:877: error: Unused "type: ignore" comment
- ignite/handlers/param_scheduler.py:911: error: Unused "type: ignore" comment
- ignite/handlers/param_scheduler.py:924: error: Unused "type: ignore" comment
- ignite/handlers/param_scheduler.py:1406: error: Unused "type: ignore" comment
- ignite/handlers/param_scheduler.py:1411: error: Unused "type: ignore" comment
- ignite/handlers/param_scheduler.py:1413: error: Unused "type: ignore" comment
- ignite/handlers/param_scheduler.py:1420: error: Unused "type: ignore" comment
- ignite/handlers/param_scheduler.py:1544: error: Unused "type: ignore" comment
- ignite/handlers/param_scheduler.py:1564: error: Unused "type: ignore" comment
- ignite/handlers/param_scheduler.py:1565: error: Unused "type: ignore" comment
- ignite/handlers/lr_finder.py:476: error: Class cannot subclass "_LRScheduler" (has type "Any") [misc]
- ignite/handlers/lr_finder.py:498: error: Unused "type: ignore" comment
- ignite/handlers/lr_finder.py:499: error: Unused "type: ignore" comment
- ignite/contrib/handlers/wandb_logger.py:138: error: Unused "type: ignore" comment
- ignite/contrib/handlers/visdom_logger.py:188: error: Unused "type: ignore" comment
- ignite/contrib/handlers/visdom_logger.py:196: error: Unused "type: ignore" comment
- ignite/contrib/handlers/visdom_logger.py:241: error: Unused "type: ignore" comment
- ignite/contrib/handlers/tensorboard_logger.py:153: error: Unused "type: ignore" comment
+ Traceback (most recent call last):
+ File "", line 8, in <module>
+ sys.exit(console_entry())
+ File "/__main__.py", line 12, in console_entry
+ main(None, sys.stdout, sys.stderr)
+ File "/main.py", line 96, in main
+ res, messages, blockers = run_build(sources, options, fscache, t0, stdout, stderr)
+ File "/main.py", line 173, in run_build
+ res = build.build(sources, options, None, flush_errors, fscache, stdout, stderr)
+ File "/build.py", line 154, in build
+ result = _build(
+ File "/build.py", line 230, in _build
+ graph = dispatch(sources, manager, stdout)
+ File "/build.py", line 2729, in dispatch
+ process_graph(graph, manager)
+ File "/build.py", line 3087, in process_graph
+ process_stale_scc(graph, scc, manager)
+ File "/build.py", line 3185, in process_stale_scc
+ graph[id].type_check_first_pass()
+ File "/build.py", line 2180, in type_check_first_pass
+ self.type_checker().check_first_pass()
+ File "/checker.py", line 325, in check_first_pass
+ self.accept(d)
+ File "/checker.py", line 431, in accept
+ stmt.accept(self)
+ File "/nodes.py", line 1029, in accept
+ return visitor.visit_class_def(self)
+ File "/checker.py", line 1816, in visit_class_def
+ self.accept(defn.defs)
+ File "/checker.py", line 431, in accept
+ stmt.accept(self)
+ File "/nodes.py", line 1100, in accept
+ return visitor.visit_block(self)
+ File "/checker.py", line 2178, in visit_block
+ self.accept(s)
+ File "/checker.py", line 431, in accept
+ stmt.accept(self)
+ File "/nodes.py", line 748, in accept
+ return visitor.visit_func_def(self)
+ File "/checker.py", line 782, in visit_func_def
+ self._visit_func_def(defn)
+ File "/checker.py", line 786, in _visit_func_def
+ self.check_func_item(defn, name=defn.name)
+ File "/checker.py", line 848, in check_func_item
+ self.check_func_def(defn, typ, name)
+ File "/checker.py", line 1033, in check_func_def
+ self.accept(item.body)
+ File "/checker.py", line 431, in accept
+ stmt.accept(self)
+ File "/nodes.py", line 1100, in accept
+ return visitor.visit_block(self)
+ File "/checker.py", line 2178, in visit_block
+ self.accept(s)
+ File "/checker.py", line 431, in accept
+ stmt.accept(self)
+ File "/nodes.py", line 1166, in accept
+ return visitor.visit_assignment_stmt(self)
+ File "/checker.py", line 2220, in visit_assignment_stmt
+ self.check_assignment(s.lvalues[-1], s.rvalue, s.type is None, s.new_syntax)
+ File "/checker.py", line 2389, in check_assignment
+ rvalue_type = self.check_simple_assignment(lvalue_type, rvalue, context=rvalue,
+ File "/checker.py", line 3307, in check_simple_assignment
+ rvalue_type = self.expr_checker.accept(rvalue, lvalue_type,
+ File "/checkexpr.py", line 4012, in accept
+ typ = node.accept(self)
+ File "/nodes.py", line 1761, in accept
+ return visitor.visit_call_expr(self)
+ File "/checkexpr.py", line 305, in visit_call_expr
+ return self.visit_call_expr_inner(e, allow_none_return=allow_none_return)
+ File "/checkexpr.py", line 383, in visit_call_expr_inner
+ ret_type = self.check_call_expr_with_callee_type(callee_type, e, fullname,
+ File "/checkexpr.py", line 892, in check_call_expr_with_callee_type
+ ret_type, callee_type = self.check_call(
+ File "/checkexpr.py", line 957, in check_call
+ return self.check_callable_call(callee, args, arg_kinds, context, arg_names,
+ File "/checkexpr.py", line 1067, in check_callable_call
+ self.check_argument_count(callee, arg_types, arg_kinds,
+ File "/checkexpr.py", line 1417, in check_argument_count
+ ok, is_unexpected_arg_error = self.check_for_extra_actual_arguments(
+ File "/checkexpr.py", line 1499, in check_for_extra_actual_arguments
+ if messages and supertype.args:
+ NameError: name 'messages' is not defined
bidict (https://github.com/jab/bidict)
+ bidict/_base.py:453: error: INTERNAL ERROR -- Please try using mypy master on GitHub:
+ https://mypy.readthedocs.io/en/stable/common_issues.html#using-a-development-mypy-build
+ Please report a bug at https://github.com/python/mypy/issues
+ version: 0.970+dev.cb7c148c62d50edc1c70943b209b82d1e0940fd7
+ bidict/_base.py:453: : note: use --pdb to drop into pdb
+ Traceback (most recent call last):
+ File "", line 8, in <module>
+ sys.exit(console_entry())
+ File "/__main__.py", line 12, in console_entry
+ main(None, sys.stdout, sys.stderr)
+ File "/main.py", line 96, in main
+ res, messages, blockers = run_build(sources, options, fscache, t0, stdout, stderr)
+ File "/main.py", line 173, in run_build
+ res = build.build(sources, options, None, flush_errors, fscache, stdout, stderr)
+ File "/build.py", line 154, in build
+ result = _build(
+ File "/build.py", line 230, in _build
+ graph = dispatch(sources, manager, stdout)
+ File "/build.py", line 2729, in dispatch
+ process_graph(graph, manager)
+ File "/build.py", line 3087, in process_graph
+ process_stale_scc(graph, scc, manager)
+ File "/build.py", line 3185, in process_stale_scc
+ graph[id].type_check_first_pass()
+ File "/build.py", line 2180, in type_check_first_pass
+ self.type_checker().check_first_pass()
+ File "/checker.py", line 325, in check_first_pass
+ self.accept(d)
+ File "/checker.py", line 431, in accept
+ stmt.accept(self)
+ File "/nodes.py", line 1029, in accept
+ return visitor.visit_class_def(self)
+ File "/checker.py", line 1816, in visit_class_def
+ self.accept(defn.defs)
+ File "/checker.py", line 431, in accept
+ stmt.accept(self)
+ File "/nodes.py", line 1100, in accept
+ return visitor.visit_block(self)
+ File "/checker.py", line 2178, in visit_block
+ self.accept(s)
+ File "/checker.py", line 431, in accept
+ stmt.accept(self)
+ File "/nodes.py", line 748, in accept
+ return visitor.visit_func_def(self)
+ File "/checker.py", line 782, in visit_func_def
+ self._visit_func_def(defn)
+ File "/checker.py", line 786, in _visit_func_def
+ self.check_func_item(defn, name=defn.name)
+ File "/checker.py", line 848, in check_func_item
+ self.check_func_def(defn, typ, name)
+ File "/checker.py", line 1033, in check_func_def
+ self.accept(item.body)
+ File "/checker.py", line 431, in accept
+ stmt.accept(self)
+ File "/nodes.py", line 1100, in accept
+ return visitor.visit_block(self)
+ File "/checker.py", line 2178, in visit_block
+ self.accept(s)
+ File "/checker.py", line 431, in accept
+ stmt.accept(self)
+ File "/nodes.py", line 1244, in accept
+ return visitor.visit_for_stmt(self)
+ File "/checker.py", line 3895, in visit_for_stmt
+ iterator_type, item_type = self.analyze_iterable_item_type(s.expr)
+ File "/checker.py", line 3914, in analyze_iterable_item_type
+ iterable = get_proper_type(echk.accept(expr))
+ File "/checkexpr.py", line 4012, in accept
+ typ = node.accept(self)
+ File "/nodes.py", line 1761, in accept
+ return visitor.visit_call_expr(self)
+ File "/checkexpr.py", line 305, in visit_call_expr
+ return self.visit_call_expr_inner(e, allow_none_return=allow_none_return)
+ File "/checkexpr.py", line 383, in visit_call_expr_inner
+ ret_type = self.check_call_expr_with_callee_type(callee_type, e, fullname,
+ File "/checkexpr.py", line 892, in check_call_expr_with_callee_type
+ ret_type, callee_type = self.check_call(
+ File "/checkexpr.py", line 957, in check_call
+ return self.check_callable_call(callee, args, arg_kinds, context, arg_names,
+ File "/checkexpr.py", line 1067, in check_callable_call
+ self.check_argument_count(callee, arg_types, arg_kinds,
+ File "/checkexpr.py", line 1417, in check_argument_count
+ ok, is_unexpected_arg_error = self.check_for_extra_actual_arguments(
+ File "/checkexpr.py", line 1499, in check_for_extra_actual_arguments
+ if messages and supertype.args:
+ NameError: name 'messages' is not defined
python-sop (https://gitlab.com/dkg/python-sop)
+ sop/__init__.py:456: error: INTERNAL ERROR -- Please try using mypy master on GitHub:
+ https://mypy.readthedocs.io/en/stable/common_issues.html#using-a-development-mypy-build
+ Please report a bug at https://github.com/python/mypy/issues
+ version: 0.970+dev.cb7c148c62d50edc1c70943b209b82d1e0940fd7
+ sop/__init__.py:456: : note: use --pdb to drop into pdb
+ Traceback (most recent call last):
+ File "", line 8, in <module>
+ sys.exit(console_entry())
+ File "/__main__.py", line 12, in console_entry
+ main(None, sys.stdout, sys.stderr)
+ File "/main.py", line 96, in main
+ res, messages, blockers = run_build(sources, options, fscache, t0, stdout, stderr)
+ File "/main.py", line 173, in run_build
+ res = build.build(sources, options, None, flush_errors, fscache, stdout, stderr)
+ File "/build.py", line 154, in build
+ result = _build(
+ File "/build.py", line 230, in _build
+ graph = dispatch(sources, manager, stdout)
+ File "/build.py", line 2729, in dispatch
+ process_graph(graph, manager)
+ File "/build.py", line 3087, in process_graph
+ process_stale_scc(graph, scc, manager)
+ File "/build.py", line 3185, in process_stale_scc
+ graph[id].type_check_first_pass()
+ File "/build.py", line 2180, in type_check_first_pass
+ self.type_checker().check_first_pass()
+ File "/checker.py", line 325, in check_first_pass
+ self.accept(d)
+ File "/checker.py", line 431, in accept
+ stmt.accept(self)
+ File "/nodes.py", line 1029, in accept
+ return visitor.visit_class_def(self)
+ File "/checker.py", line 1816, in visit_class_def
+ self.accept(defn.defs)
+ File "/checker.py", line 431, in accept
+ stmt.accept(self)
+ File "/nodes.py", line 1100, in accept
+ return visitor.visit_block(self)
+ File "/checker.py", line 2178, in visit_block
+ self.accept(s)
+ File "/checker.py", line 431, in accept
+ stmt.accept(self)
+ File "/nodes.py", line 748, in accept
+ return visitor.visit_func_def(self)
+ File "/checker.py", line 782, in visit_func_def
+ self._visit_func_def(defn)
+ File "/checker.py", line 786, in _visit_func_def
+ self.check_func_item(defn, name=defn.name)
+ File "/checker.py", line 848, in check_func_item
+ self.check_func_def(defn, typ, name)
+ File "/checker.py", line 1033, in check_func_def
+ self.accept(item.body)
+ File "/checker.py", line 431, in accept
+ stmt.accept(self)
+ File "/nodes.py", line 1100, in accept
+ return visitor.visit_block(self)
+ File "/checker.py", line 2178, in visit_block
+ self.accept(s)
+ File "/checker.py", line 431, in accept
+ stmt.accept(self)
+ File "/nodes.py", line 1257, in accept
+ return visitor.visit_return_stmt(self)
+ File "/checker.py", line 3498, in visit_return_stmt
+ self.check_return_stmt(s)
+ File "/checker.py", line 3530, in check_return_stmt
+ typ = get_proper_type(self.expr_checker.accept(
+ File "/checkexpr.py", line 4012, in accept
+ typ = node.accept(self)
+ File "/nodes.py", line 1761, in accept
+ return visitor.visit_call_expr(self)
+ File "/checkexpr.py", line 305, in visit_call_expr
+ return self.visit_call_expr_inner(e, allow_none_return=allow_none_return)
+ File "/checkexpr.py", line 383, in visit_call_expr_inner
+ ret_type = self.check_call_expr_with_callee_type(callee_type, e, fullname,
+ File "/checkexpr.py", line 892, in check_call_expr_with_callee_type
+ ret_type, callee_type = self.check_call(
+ File "/checkexpr.py", line 957, in check_call
+ return self.check_callable_call(callee, args, arg_kinds, context, arg_names,
+ File "/checkexpr.py", line 1067, in check_callable_call
+ self.check_argument_count(callee, arg_types, arg_kinds,
+ File "/checkexpr.py", line 1417, in check_argument_count
+ ok, is_unexpected_arg_error = self.check_for_extra_actual_arguments(
+ File "/checkexpr.py", line 1499, in check_for_extra_actual_arguments
+ if messages and supertype.args:
+ NameError: name 'messages' is not defined
pegen (https://github.com/we-like-parsers/pegen)
- src/pegen/parser.py:297: error: "Parser" has no attribute "start" [attr-defined]
- src/pegen/python_generator.py:220: error: Need type annotation for "cleanup_statements" (hint: "cleanup_statements: List[<type>] = ...") [var-annotated]
- src/pegen/utils.py:19: error: Argument 1 to "module_from_spec" has incompatible type "Optional[ModuleSpec]"; expected "ModuleSpec" [arg-type]
- src/pegen/utils.py:23: error: Item "None" of "Optional[ModuleSpec]" has no attribute "loader" [union-attr]
- src/pegen/utils.py:52: error: "Parser" has no attribute "start" [attr-defined]
+ src/pegen/grammar.py:31: error: INTERNAL ERROR -- Please try using mypy master on GitHub:
+ https://mypy.readthedocs.io/en/stable/common_issues.html#using-a-development-mypy-build
+ Please report a bug at https://github.com/python/mypy/issues
+ version: 0.970+dev.cb7c148c62d50edc1c70943b209b82d1e0940fd7
+ src/pegen/grammar.py:31: : note: use --pdb to drop into pdb
+ Traceback (most recent call last):
+ File "", line 8, in <module>
+ sys.exit(console_entry())
+ File "/__main__.py", line 12, in console_entry
+ main(None, sys.stdout, sys.stderr)
+ File "/main.py", line 96, in main
+ res, messages, blockers = run_build(sources, options, fscache, t0, stdout, stderr)
+ File "/main.py", line 173, in run_build
+ res = build.build(sources, options, None, flush_errors, fscache, stdout, stderr)
+ File "/build.py", line 154, in build
+ result = _build(
+ File "/build.py", line 230, in _build
+ graph = dispatch(sources, manager, stdout)
+ File "/build.py", line 2729, in dispatch
+ process_graph(graph, manager)
+ File "/build.py", line 3087, in process_graph
+ process_stale_scc(graph, scc, manager)
+ File "/build.py", line 3185, in process_stale_scc
+ graph[id].type_check_first_pass()
+ File "/build.py", line 2180, in type_check_first_pass
+ self.type_checker().check_first_pass()
+ File "/checker.py", line 325, in check_first_pass
+ self.accept(d)
+ File "/checker.py", line 431, in accept
+ stmt.accept(self)
+ File "/nodes.py", line 1029, in accept
+ return visitor.visit_class_def(self)
+ File "/checker.py", line 1816, in visit_class_def
+ self.accept(defn.defs)
+ File "/checker.py", line 431, in accept
+ stmt.accept(self)
+ File "/nodes.py", line 1100, in accept
+ return visitor.visit_block(self)
+ File "/checker.py", line 2178, in visit_block
+ self.accept(s)
+ File "/checker.py", line 431, in accept
+ stmt.accept(self)
+ File "/nodes.py", line 748, in accept
+ return visitor.visit_func_def(self)
+ File "/checker.py", line 782, in visit_func_def
+ self._visit_func_def(defn)
+ File "/checker.py", line 786, in _visit_func_def
+ self.check_func_item(defn, name=defn.name)
+ File "/checker.py", line 848, in check_func_item
+ self.check_func_def(defn, typ, name)
+ File "/checker.py", line 1033, in check_func_def
+ self.accept(item.body)
+ File "/checker.py", line 431, in accept
+ stmt.accept(self)
+ File "/nodes.py", line 1100, in accept
+ return visitor.visit_block(self)
+ File "/checker.py", line 2178, in visit_block
+ self.accept(s)
+ File "/checker.py", line 431, in accept
+ stmt.accept(self)
+ File "/nodes.py", line 1257, in accept
+ return visitor.visit_return_stmt(self)
+ File "/checker.py", line 3498, in visit_return_stmt
+ self.check_return_stmt(s)
+ File "/checker.py", line 3530, in check_return_stmt
+ typ = get_proper_type(self.expr_checker.accept(
+ File "/checkexpr.py", line 4006, in accept
+ typ = self.visit_call_expr(node, allow_none_return=True)
+ File "/checkexpr.py", line 305, in visit_call_expr
+ return self.visit_call_expr_inner(e, allow_none_return=allow_none_return)
+ File "/checkexpr.py", line 383, in visit_call_expr_inner
+ ret_type = self.check_call_expr_with_callee_type(callee_type, e, fullname,
+ File "/checkexpr.py", line 892, in check_call_expr_with_callee_type
+ ret_type, callee_type = self.check_call(
+ File "/checkexpr.py", line 965, in check_call
+ return self.check_union_call(callee, args, arg_kinds, arg_names, context)
+ File "/checkexpr.py", line 2080, in check_union_call
+ results = [
+ File "/checkexpr.py", line 2081, in <listcomp>
+ self.check_call(
+ File "/checkexpr.py", line 957, in check_call
+ return self.check_callable_call(callee, args, arg_kinds, context, arg_names,
+ File "/checkexpr.py", line 1067, in check_callable_call
+ self.check_argument_count(callee, arg_types, arg_kinds,
+ File "/checkexpr.py", line 1417, in check_argument_count
+ ok, is_unexpected_arg_error = self.check_for_extra_actual_arguments(
+ File "/checkexpr.py", line 1499, in check_for_extra_actual_arguments
+ if messages and supertype.args:
+ NameError: name 'messages' is not defined
SinbadCogs (https://github.com/mikeshardmind/SinbadCogs)
- Warning: unused section(s) in mypy.ini: [mypy-websockets.*], [mypy-numba.*], [mypy-numpy.*]
+ ./rolemanagement/converters.py:46: error: INTERNAL ERROR -- Please try using mypy master on GitHub:
+ https://mypy.readthedocs.io/en/stable/common_issues.html#using-a-development-mypy-build
+ Please report a bug at https://github.com/python/mypy/issues
+ version: 0.970+dev.cb7c148c62d50edc1c70943b209b82d1e0940fd7
+ ./rolemanagement/converters.py:46: : note: use --pdb to drop into pdb
- devtools/runner.py:26: error: Unused "type: ignore" comment
- scheduler/message.py:87: error: Unused "type: ignore" comment
- schedul
... (truncated 533294 chars) ... |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThe type checker now handles instances of types with a typing.Mapping base when passed as **kwargs, extracting string-literal keys to report unexpected keywords or missing positional args. Related validation for keyword var-arg types was refined. Tests were modernized with inline annotations and a new literal-mapping test added. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant TypeChecker
participant Callee
Caller->>TypeChecker: call f(**mapping)
Note right of TypeChecker: map actual type to Mapping[K, V]
TypeChecker->>TypeChecker: is subtype of Mapping[str, Any]? / try_getting_str_literals_from_type(K)
alt mapping keys are string Literals
TypeChecker->>TypeChecker: extract literal keys
alt callee does not accept **kwargs
TypeChecker->>Caller: report unexpected keyword(s) for each literal key
else callee accepts positional args but missing names
TypeChecker->>Caller: report too few positional arguments (keys not matched)
end
else not valid mapping for **kwargs
TypeChecker->>Caller: treat as invalid **kwargs type
end
TypeChecker->>Callee: supply validated arguments (if any)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🧪 Early access (Sonnet 4.5): enabledWe are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hello @arvi18, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
This pull request, authored by arvi18, addresses issue python#10023 by enhancing mypy's type checking for keyword arguments, particularly when dealing with typing.Mapping types and literal types as keys. The changes modify mypy/checkexpr.py to improve the validation of **kwargs and add new test cases to test-data/unit/check-kwargs.test to cover these scenarios. The goal is to provide more accurate error messages when keyword arguments don't match the expected types, especially when literal types are involved.
Highlights
- Keyword Argument Validation: The pull request improves the validation of keyword arguments, especially when
typing.Mappingtypes are used with literal types as keys. This ensures that incorrect keyword arguments are flagged with appropriate error messages. - Typing Mapping: The code now checks if a kwarg is a Mapping type and if the list of strings extracted from the mapping key type is not empty.
- New Test Cases: Several new test cases have been added to
test-data/unit/check-kwargs.testto cover various scenarios involving keyword arguments, including those with dynamically typed callables, function objects, and literal types.
Changelog
Click here to see the changelog
- mypy/checkexpr.py
- Added
Iterableto the imports from the typing module (line 7). - Imported
try_getting_str_literals_from_type(line 72). - Added a check within
check_for_extra_actual_argumentsto handle cases where the actual type is an instance oftyping.Mapping(lines 1493-1512). This check extracts string literals from the mapping key type and generates appropriate error messages if the arguments don't match. - Modified
is_valid_keyword_var_argto improve the validation of types used as**kwargs(lines 4047-4068). This includes checking for subtypes oftyping.Mappingand handling empty dictionaries passed as**kwargs.
- Added
- test-data/unit/check-kwargs.test
- Modified existing test cases to use explicit type annotations instead of inline comments (e.g.,
f: Any = Noneinstead off = None # type: Any). - Added new test cases for passing mapping literals as keyword arguments, including scenarios with
Literaltypes and incorrect key types (lines 350-379). - Added test cases to cover scenarios with varargs and kwargs, as well as comment signatures (lines 428-441).
- Added test cases for passing keyword varargs to varargs only functions (lines 428-434).
- Added test cases for passing keyword varargs to non-varargs functions (lines 393-401).
- Added test cases for both kinds of varargs (lines 404-411).
- Added test cases for keyword arguments and comment signatures (lines 436-458).
- Added test cases for passing mapping literals for keyword vararg (lines 350-379).
- Added test cases for passing keyword varargs to varargs only function (lines 428-434).
- Added test cases for passing keyword varargs to non-varargs function (lines 393-401).
- Added test cases for both kinds of varargs (lines 404-411).
- Added test cases for keyword arguments and comment signatures (lines 436-458).
- Modified existing test cases to use explicit type annotations instead of inline comments (e.g.,
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
A map of strings,
Passed as keyword arguments,
Mypy checks with care.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
The pull request introduces checks to ensure that keyword arguments passed to functions are valid, especially when dealing with typing.Mapping types and literal types. The changes look good overall, but there are a few areas where improvements can be made to enhance clarity and correctness.
Summary of Findings
- Missing review comments for medium severity issues: There were several medium severity issues identified, but no review comments were provided for them as instructed. Please ensure that review comments are added for all medium severity issues.
- Potential infinite recursion: The code introduces a new function
try_getting_str_literals_from_typewhich callsmap_instance_to_supertype. It's crucial to ensure that this combination doesn't lead to infinite recursion, especially with complex type hierarchies. The code should be reviewed for potential edge cases that could trigger this. - Inconsistent error message: In
check_for_extra_actual_arguments, the code generates an 'unexpected keyword argument' error. However, the logic for handling this error seems a bit convoluted. Consider simplifying the error reporting logic to make it more readable and maintainable.
Merge Readiness
The pull request introduces important checks for keyword arguments and literal types. However, there are some medium and high severity issues that need to be addressed before merging. I recommend addressing these issues before merging, and also having others review and approve this code before merging. I am unable to directly approve this pull request.
| is_subtype(typ, self.chk.named_type('typing.Mapping')) and | ||
| try_getting_str_literals_from_type(map_instance_to_supertype( | ||
| typ, mapping_type.type).args[0]) is not None) or |
There was a problem hiding this comment.
Consider adding a check to ensure that typ is not None before calling map_instance_to_supertype. This would prevent a potential AttributeError if typ is unexpectedly None.
(isinstance(typ, Instance) and typ is not None and
is_subtype(typ, self.chk.named_type('typing.Mapping')) and
try_getting_str_literals_from_type(map_instance_to_supertype(
typ, mapping_type.type).args[0]) is not None) or| elif (args and nodes.ARG_POS in callee.arg_kinds and | ||
| not all(arg in callee.arg_names for arg in args) and | ||
| isinstance(actual_names, Iterable)): | ||
| act_names = [name for name, kind in | ||
| zip(iter(actual_names), actual_kinds) | ||
| if kind != nodes.ARG_STAR2] | ||
| messages.too_few_arguments(callee, context, act_names) |
There was a problem hiding this comment.
This block seems to duplicate the logic from lines 1501-1504. It might be beneficial to refactor this into a single, more general block to reduce redundancy and improve maintainability.
| (isinstance(typ, Instance) and | ||
| is_subtype(typ, self.chk.named_type('typing.Mapping')) and | ||
| try_getting_str_literals_from_type(map_instance_to_supertype( | ||
| typ, mapping_type.type).args[0]) is not None) or |
There was a problem hiding this comment.
This condition checks if typ is an instance and a subtype of typing.Mapping, and then calls try_getting_str_literals_from_type. It might be beneficial to add a comment explaining why this specific check is necessary and what kind of types it's intended to handle.
| u: Mapping[Literal['c'], int] = {'b':2} \ | ||
| # E: Dict entry 0 has incompatible type "Literal['b']": "int"; expected "Literal['c']": "int" | ||
| f(**u) |
There was a problem hiding this comment.
It seems like the type annotation for d is not being used, and mypy is inferring the type as Dict[str, Any]. Consider adding a test case that specifically checks the interaction of Any and Dict with keyword arguments.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
mypy/checkexpr.py(4 hunks)test-data/unit/check-kwargs.test(13 hunks)
🧰 Additional context used
🪛 Ruff (0.8.2)
mypy/checkexpr.py
1499-1499: Undefined name messages
(F821)
1502-1502: Undefined name messages
(F821)
1511-1511: Undefined name messages
(F821)
🔇 Additional comments (4)
test-data/unit/check-kwargs.test (4)
138-138: Good modernization of type annotations.Updated from comment-style type hints to inline variable annotations, which follows modern Python typing practices.
145-145: Consistent modernization of type annotations throughout the file.All type annotations have been updated from comment-style type hints to inline variable annotations, improving code readability and maintainability. This follows modern Python typing best practices and makes the type information more explicit.
Also applies to: 215-216, 240-240, 265-267, 274-276, 296-296, 299-299, 332-334, 345-345, 385-385, 396-396, 398-398, 407-408, 419-420, 431-431, 519-519, 523-523
438-438: Function signature annotations improved.Function signatures have been updated from comment-style type hints to inline parameter and return type annotations, which is the preferred modern approach.
Also applies to: 447-447, 455-455
350-379: Well-designed test case for mapping literals as keyword arguments.The new test case
testPassingMappingLiteralsForKeywordVarArgthoroughly tests:
- Detection of non-string literal keys (triggering appropriate errors)
- Support for string literal keys in mappings
- Validation of literal type mismatches
- Handling of multiple literal key options
- Error reporting for unexpected or missing arguments
This implementation aligns with the PR objective to improve handling of literals passed as keyword arguments and covers both positive and negative test cases.
| elif (isinstance(actual_type, Instance) and | ||
| actual_type.type.has_base('typing.Mapping')): | ||
| any_type = AnyType(TypeOfAny.special_form) | ||
| mapping_info = self.chk.named_generic_type('typing.Mapping', | ||
| [any_type, any_type]).type | ||
| supertype = map_instance_to_supertype(actual_type, mapping_info) | ||
| if messages and supertype.args: | ||
| args = try_getting_str_literals_from_type(supertype.args[0]) | ||
| if args and nodes.ARG_STAR2 not in callee.arg_kinds: | ||
| messages.unexpected_keyword_argument( | ||
| callee, args[0], supertype.args[0], context) | ||
| is_unexpected_arg_error = True | ||
| elif (args and nodes.ARG_POS in callee.arg_kinds and | ||
| not all(arg in callee.arg_names for arg in args) and | ||
| isinstance(actual_names, Iterable)): | ||
| act_names = [name for name, kind in | ||
| zip(iter(actual_names), actual_kinds) | ||
| if kind != nodes.ARG_STAR2] | ||
| messages.too_few_arguments(callee, context, act_names) | ||
| ok = False | ||
|
|
There was a problem hiding this comment.
messages is undefined – will raise NameError at runtime
The new branch inside check_for_extra_actual_arguments() invokes messages.* methods, but the local name messages is never defined in this scope (nor passed as a parameter).
During real execution this path will therefore crash with:
NameError: name 'messages' is not defined
Use the existing MessageBuilder available as self.msg (or add a parameter) to avoid the failure:
- if messages and supertype.args:
+ if supertype.args:
args = try_getting_str_literals_from_type(supertype.args[0])
- if args and nodes.ARG_STAR2 not in callee.arg_kinds:
- messages.unexpected_keyword_argument(
+ if args and nodes.ARG_STAR2 not in callee.arg_kinds:
+ self.msg.unexpected_keyword_argument(
callee, args[0], supertype.args[0], context)
is_unexpected_arg_error = True
- elif (args and nodes.ARG_POS in callee.arg_kinds and
+ elif (args and nodes.ARG_POS in callee.arg_kinds and
not all(arg in callee.arg_names for arg in args) and
- isinstance(actual_names, Iterable)):
+ isinstance(actual_names, Iterable)):
act_names = [name for name, kind in
zip(iter(actual_names), actual_kinds)
if kind != nodes.ARG_STAR2]
- messages.too_few_arguments(callee, context, act_names)
+ self.msg.too_few_arguments(callee, context, act_names)
ok = FalseWithout this change every project exercising this branch (see primer failures in the PR description) will encounter internal errors, masking the intended functionality.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| elif (isinstance(actual_type, Instance) and | |
| actual_type.type.has_base('typing.Mapping')): | |
| any_type = AnyType(TypeOfAny.special_form) | |
| mapping_info = self.chk.named_generic_type('typing.Mapping', | |
| [any_type, any_type]).type | |
| supertype = map_instance_to_supertype(actual_type, mapping_info) | |
| if messages and supertype.args: | |
| args = try_getting_str_literals_from_type(supertype.args[0]) | |
| if args and nodes.ARG_STAR2 not in callee.arg_kinds: | |
| messages.unexpected_keyword_argument( | |
| callee, args[0], supertype.args[0], context) | |
| is_unexpected_arg_error = True | |
| elif (args and nodes.ARG_POS in callee.arg_kinds and | |
| not all(arg in callee.arg_names for arg in args) and | |
| isinstance(actual_names, Iterable)): | |
| act_names = [name for name, kind in | |
| zip(iter(actual_names), actual_kinds) | |
| if kind != nodes.ARG_STAR2] | |
| messages.too_few_arguments(callee, context, act_names) | |
| ok = False | |
| elif (isinstance(actual_type, Instance) and | |
| actual_type.type.has_base('typing.Mapping')): | |
| any_type = AnyType(TypeOfAny.special_form) | |
| mapping_info = self.chk.named_generic_type('typing.Mapping', | |
| [any_type, any_type]).type | |
| supertype = map_instance_to_supertype(actual_type, mapping_info) | |
| if supertype.args: | |
| args = try_getting_str_literals_from_type(supertype.args[0]) | |
| if args and nodes.ARG_STAR2 not in callee.arg_kinds: | |
| self.msg.unexpected_keyword_argument( | |
| callee, args[0], supertype.args[0], context) | |
| is_unexpected_arg_error = True | |
| elif (args and nodes.ARG_POS in callee.arg_kinds and | |
| not all(arg in callee.arg_names for arg in args) and | |
| isinstance(actual_names, Iterable)): | |
| act_names = [name for name, kind in | |
| zip(iter(actual_names), actual_kinds) | |
| if kind != nodes.ARG_STAR2] | |
| self.msg.too_few_arguments(callee, context, act_names) | |
| ok = False |
🧰 Tools
🪛 Ruff (0.8.2)
1499-1499: Undefined name messages
(F821)
1502-1502: Undefined name messages
(F821)
1511-1511: Undefined name messages
(F821)
|
/refacto-test |
|
Refacto is reviewing this PR. Please wait for the review comments to be posted. |
Code Review: Type Annotation Improvements👍 Well Done
📌 Files Processed
📝 Additional Comments
|
![refacto-test[bot]](https://avatars.githubusercontent.com/u/196302493?s=60&v=4){kind=small}
| if args and nodes.ARG_STAR2 not in callee.arg_kinds: | ||
| messages.unexpected_keyword_argument( | ||
| callee, args[0], supertype.args[0], context) | ||
| is_unexpected_arg_error = True |
There was a problem hiding this comment.
Incomplete Mapping Validation
The code only checks the first literal string (args[0]) when reporting an unexpected keyword argument error, ignoring other potential invalid arguments in the args list. This creates a logical inconsistency where only the first invalid argument is reported even when multiple invalid arguments exist.
if args and nodes.ARG_STAR2 not in callee.arg_kinds:
for arg in args:
if arg not in callee.arg_names:
messages.unexpected_keyword_argument(
callee, arg, supertype.args[0], context)
is_unexpected_arg_error = True
elif (args and nodes.ARG_POS in callee.arg_kinds and
not all(arg in callee.arg_names for arg in args) and
isinstance(actual_names, Iterable)):
Commitable Suggestion
| if args and nodes.ARG_STAR2 not in callee.arg_kinds: | |
| messages.unexpected_keyword_argument( | |
| callee, args[0], supertype.args[0], context) | |
| is_unexpected_arg_error = True | |
| if args and nodes.ARG_STAR2 not in callee.arg_kinds: | |
| for arg in args: | |
| if arg not in callee.arg_names: | |
| messages.unexpected_keyword_argument( | |
| callee, arg, supertype.args[0], context) | |
| is_unexpected_arg_error = True | |
| elif (args and nodes.ARG_POS in callee.arg_kinds and | |
| not all(arg in callee.arg_names for arg in args) and | |
| isinstance(actual_names, Iterable)): |
Standards
- Algorithm-Correctness-Loop-Logic
- Business-Rule-Validation-Completeness
| if args and nodes.ARG_STAR2 not in callee.arg_kinds: | ||
| messages.unexpected_keyword_argument( | ||
| callee, args[0], supertype.args[0], context) | ||
| is_unexpected_arg_error = True |
There was a problem hiding this comment.
Literal Type Validation
Missing null check for 'args' before accessing args[0] could cause IndexError if try_getting_str_literals_from_type returns an empty list. This would lead to runtime failure when processing keyword arguments with certain mapping types.
Standards
- ISO-IEC-25010-Reliability-Fault-Tolerance
- ISO-IEC-25010-Functional-Correctness-Appropriateness
| elif (isinstance(actual_type, Instance) and | ||
| actual_type.type.has_base('typing.Mapping')): | ||
| any_type = AnyType(TypeOfAny.special_form) | ||
| mapping_info = self.chk.named_generic_type('typing.Mapping', | ||
| [any_type, any_type]).type | ||
| supertype = map_instance_to_supertype(actual_type, mapping_info) | ||
| if messages and supertype.args: | ||
| args = try_getting_str_literals_from_type(supertype.args[0]) | ||
| if args and nodes.ARG_STAR2 not in callee.arg_kinds: | ||
| messages.unexpected_keyword_argument( | ||
| callee, args[0], supertype.args[0], context) | ||
| is_unexpected_arg_error = True | ||
| elif (args and nodes.ARG_POS in callee.arg_kinds and | ||
| not all(arg in callee.arg_names for arg in args) and | ||
| isinstance(actual_names, Iterable)): | ||
| act_names = [name for name, kind in | ||
| zip(iter(actual_names), actual_kinds) | ||
| if kind != nodes.ARG_STAR2] | ||
| messages.too_few_arguments(callee, context, act_names) | ||
| ok = False | ||
|
|
There was a problem hiding this comment.
Literal Type Validation
The implementation uses string literals from mapping types as keyword arguments without proper validation. If user-controlled input can influence these literals, it could lead to unexpected keyword argument injection. This could potentially bypass type checking constraints or cause runtime errors.
Standards
- CWE-20
- OWASP-A03
| elif (isinstance(actual_type, Instance) and | ||
| actual_type.type.has_base('typing.Mapping')): | ||
| any_type = AnyType(TypeOfAny.special_form) | ||
| mapping_info = self.chk.named_generic_type('typing.Mapping', | ||
| [any_type, any_type]).type | ||
| supertype = map_instance_to_supertype(actual_type, mapping_info) | ||
| if messages and supertype.args: | ||
| args = try_getting_str_literals_from_type(supertype.args[0]) | ||
| if args and nodes.ARG_STAR2 not in callee.arg_kinds: | ||
| messages.unexpected_keyword_argument( | ||
| callee, args[0], supertype.args[0], context) | ||
| is_unexpected_arg_error = True | ||
| elif (args and nodes.ARG_POS in callee.arg_kinds and | ||
| not all(arg in callee.arg_names for arg in args) and | ||
| isinstance(actual_names, Iterable)): | ||
| act_names = [name for name, kind in | ||
| zip(iter(actual_names), actual_kinds) | ||
| if kind != nodes.ARG_STAR2] | ||
| messages.too_few_arguments(callee, context, act_names) | ||
| ok = False | ||
|
|
There was a problem hiding this comment.
Mapping Literal Detection
The nested conditional logic for handling Mapping literal detection creates high cognitive complexity. Extracting this into a dedicated method would improve readability and make the code more maintainable by isolating the Mapping literal validation logic.
Standards
- Clean-Code-Function-Size
- Maintainability-Quality-Complexity
- Clean-Code-Single-Responsibility
|
/refacto-visz |
|
Refacto is reviewing this PR. Please wait for the review comments to be posted. |
Code Review: Mapping Type Validation for Keyword Arguments👍 Well Done
📌 Files Processed
📝 Additional Comments
|
| args = try_getting_str_literals_from_type(supertype.args[0]) | ||
| if args and nodes.ARG_STAR2 not in callee.arg_kinds: | ||
| messages.unexpected_keyword_argument( | ||
| callee, args[0], supertype.args[0], context) | ||
| is_unexpected_arg_error = True | ||
| elif (args and nodes.ARG_POS in callee.arg_kinds and | ||
| not all(arg in callee.arg_names for arg in args) and |
There was a problem hiding this comment.
Missing Error Handling
The function extracts string literals from the mapping key type but doesn't handle the case where extraction fails (args is None). This creates a logical gap in error reporting when non-string literals are used as mapping keys in **kwargs expressions.
Standards
- Algorithm-Correctness-Edge-Cases
- Business-Rule-Error-Handling
- Logic-Verification-Control-Flow
| elif (isinstance(actual_type, Instance) and | ||
| actual_type.type.has_base('typing.Mapping')): | ||
| any_type = AnyType(TypeOfAny.special_form) | ||
| mapping_info = self.chk.named_generic_type('typing.Mapping', | ||
| [any_type, any_type]).type | ||
| supertype = map_instance_to_supertype(actual_type, mapping_info) | ||
| if messages and supertype.args: | ||
| args = try_getting_str_literals_from_type(supertype.args[0]) | ||
| if args and nodes.ARG_STAR2 not in callee.arg_kinds: | ||
| messages.unexpected_keyword_argument( | ||
| callee, args[0], supertype.args[0], context) | ||
| is_unexpected_arg_error = True | ||
| elif (args and nodes.ARG_POS in callee.arg_kinds and | ||
| not all(arg in callee.arg_names for arg in args) and | ||
| isinstance(actual_names, Iterable)): | ||
| act_names = [name for name, kind in | ||
| zip(iter(actual_names), actual_kinds) | ||
| if kind != nodes.ARG_STAR2] | ||
| messages.too_few_arguments(callee, context, act_names) | ||
| ok = False |
There was a problem hiding this comment.
Keyword Validation Bypass
The implementation doesn't properly validate all keyword arguments when handling Mapping types. When args exist but not all are in callee.arg_names, it reports too_few_arguments instead of unexpected_keyword_argument, which could allow passing unexpected keywords without proper error detection. This could lead to potential argument injection vulnerabilities in typed code.
Standards
- CWE-20
- OWASP-A03
mypy/checkexpr.py
Outdated
| messages.unexpected_keyword_argument( | ||
| callee, args[0], supertype.args[0], context) | ||
| is_unexpected_arg_error = True | ||
| elif (args and nodes.ARG_POS in callee.arg_kinds and | ||
| not all(arg in callee.arg_names for arg in args) and | ||
| isinstance(actual_names, Iterable)): | ||
| act_names = [name for name, kind in |
There was a problem hiding this comment.
Potential False Positives
The unexpected_keyword_argument error is raised without checking if the argument name exists in callee.arg_names first. This can lead to false positive errors when a valid keyword argument is passed via a Mapping with string literal types.
Standards
- ISO-IEC-25010-Functional-Correctness-Appropriateness
- ISO-IEC-25010-Reliability-Maturity
Co-authored-by: refacto-visz[bot] <230139763+refacto-visz[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
mypy/checkexpr.py (1)
1493-1514: Critical: Undefined variablemessageswill cause NameError at runtimeThe variable
messagesis not defined in this scope. Based on the class structure (line 176), you should useself.msginstead. This code will crash with aNameErrorwhen executed.Apply this diff to fix the undefined variable:
elif (isinstance(actual_type, Instance) and actual_type.type.has_base('typing.Mapping')): any_type = AnyType(TypeOfAny.special_form) mapping_info = self.chk.named_generic_type('typing.Mapping', [any_type, any_type]).type supertype = map_instance_to_supertype(actual_type, mapping_info) - if messages and supertype.args: + if supertype.args: args = try_getting_str_literals_from_type(supertype.args[0]) if args and nodes.ARG_STAR2 not in callee.arg_kinds: - messages.unexpected_keyword_argument( + self.msg.unexpected_keyword_argument( callee, args[0], supertype.args[0], context) is_unexpected_arg_error = True elif (args and nodes.ARG_POS in callee.arg_kinds and not all(arg in callee.arg_names for arg in args) and isinstance(actual_names, Iterable)): act_names = [name for name, kind in zip(iter(actual_names), actual_kinds) if kind != nodes.ARG_STAR2] - messages.too_few_arguments(callee, context, act_names) + self.msg.too_few_arguments(callee, context, act_names) ok = False
🧹 Nitpick comments (1)
mypy/checkexpr.py (1)
4049-4071: Add test for empty dict literal (**{}) in keyword var-arg checks
Tests already cover literal key mappings (integer and string) and non-string literals, but there’s no case forf(**{})to exercise theMapping[Uninhabited, Uninhabited]branch—please add a unit test intest-data/unit/check-kwargs.testforf(**{})to confirm it’s accepted.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
mypy/checkexpr.py(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
mypy/checkexpr.py (6)
mypy/typeops.py (1)
try_getting_str_literals_from_type(633-641)mypy/types.py (9)
Instance(1080-1232)AnyType(875-944)TypeOfAny(164-192)name(1854-1855)get_proper_type(2533-2533)get_proper_type(2535-2535)get_proper_type(2538-2555)UninhabitedType(947-996)ParamSpecType(561-660)mypy/checker.py (2)
named_generic_type(5317-5326)named_type(5302-5315)mypy/maptype.py (1)
map_instance_to_supertype(8-24)mypy/messages.py (2)
unexpected_keyword_argument(691-717)too_few_arguments(630-649)mypy/subtypes.py (1)
is_subtype(51-102)
🪛 Ruff (0.13.1)
mypy/checkexpr.py
1499-1499: Undefined name messages
(F821)
1503-1503: Undefined name messages
(F821)
1512-1512: Undefined name messages
(F821)
4 participants
Closes python#10023
Checks if kwarg is Mapping type AND check if list of strings extracted from mapping key type is not empty.
Summary by CodeRabbit
New Features
Style
Bug Fixes