From 305ef28683359b53342062847d36dbec241285c3 Mon Sep 17 00:00:00 2001 From: Thibault Lemaire Date: Wed, 30 Sep 2020 15:46:33 +0200 Subject: [PATCH] Leverage mypy to check str.format expressions ```py from loguru import logger class Foo: bar = "baz" foo = Foo() logger.debug('The bar is "{my_foo.bar}"', my_foo=foo) logger.debug('The bar is "{my_foo.bar}"', my_foo="not foo") # error: "str" has no attribute "bar" ``` Let's not reinvent the wheel :) --- loguru_mypy/__init__.py | 164 +++++++++++++------------ typesafety/test_builtin_severities.yml | 32 ++--- typesafety/test_dot_access.yml | 33 +++++ 3 files changed, 135 insertions(+), 94 deletions(-) create mode 100644 typesafety/test_dot_access.yml diff --git a/loguru_mypy/__init__.py b/loguru_mypy/__init__.py index d992548..a49046a 100644 --- a/loguru_mypy/__init__.py +++ b/loguru_mypy/__init__.py @@ -2,11 +2,14 @@ import string import typing as t +from mypy.checker import TypeChecker from mypy.errorcodes import ErrorCode from mypy.nodes import ( + CallExpr, Expression, FuncDef, LambdaExpr, + MemberExpr, NameExpr, RefExpr, StrExpr, @@ -47,6 +50,45 @@ class Opts(t.NamedTuple): } # type: te.Final +def _check_str_format_call(log_msg_expr: StrExpr, ctx: MethodContext) -> None: + """ Taps into mypy to typecheck something like this: + + ```py + logger.debug('The bar is "{my_foo.bar}"', my_foo=foo) + ``` + + as if it was written like this: + + ```py + logger.debug('The bar is "{my_foo.bar}"'.format(my_foo=foo)) + ``` + """ + call_expr = CallExpr( + callee=MemberExpr(expr=log_msg_expr, name='format'), + args=ctx.args[1] + ctx.args[2], + arg_kinds=ctx.arg_kinds[1] + ctx.arg_kinds[2], + arg_names=ctx.arg_names[1] + ctx.arg_names[2], + ) + call_expr.set_line(log_msg_expr) + + # WARNING: `ctx.api` *is* technically a `mypy.checker.TypeChecker` so the cast is + # safe to make, however, mypy says this should be an implementation detail. + # So, anything that's not part of the `CheckerPluginInterface` should be expected to + # change. See https://github.com/python/mypy/issues/6617 + try: + type_checker = t.cast(TypeChecker, ctx.api) + type_checker.expr_checker.visit_call_expr(call_expr) + except AttributeError: + ctx.api.msg.fail( + ( + "AttributeError when trying to access mypy's functionality. " + 'This could mean you are trying to use incompatible versions ' + 'of mypy and loguru-mypy.' + ), + context=log_msg_expr, + ) + + def _loguru_logger_call_handler( loggers: t.Dict[ProperType, Opts], ctx: MethodContext, @@ -56,77 +98,50 @@ def _loguru_logger_call_handler( assert isinstance(log_msg_expr, StrExpr), type(log_msg_expr) - # collect call args/kwargs - # due to funky structure mypy offers here, it's easier - # to beg for forgiveness here - try: - call_args = ctx.args[1] - call_args_count = len(call_args) - except IndexError: - call_args = [] - call_args_count = 0 - try: - call_kwargs = { - kwarg_name: ctx.args[2][idx] - for idx, kwarg_name in enumerate(ctx.arg_names[2]) - } - except IndexError: - call_kwargs = {} - - # collect args/kwargs from string interpolation - log_msg_value: str = log_msg_expr.value - log_msg_expected_args_count = 0 - log_msg_expected_kwargs = [] - for res in string.Formatter().parse(log_msg_value): - if res[1] is None: - continue - elif not res[1].strip(): - log_msg_expected_args_count += 1 - else: - log_msg_expected_kwargs.append(res[1].strip()) - - if log_msg_expected_args_count > call_args_count: - ctx.api.msg.fail( - f'Missing {log_msg_expected_args_count - call_args_count} ' - 'positional arguments for log message', - context=log_msg_expr, - code=ERROR_BAD_ARG, - ) - return ctx.default_return_type - elif log_msg_expected_args_count < call_args_count: - ctx.api.msg.note( - f'Expected {log_msg_expected_args_count} but found {call_args_count} ' - 'positional arguments for log message', - context=log_msg_expr, - code=ERROR_BAD_ARG, - ) - return ctx.default_return_type - elif logger_opts.lazy: - for call_pos, call_arg in enumerate(call_args): - if isinstance(call_arg, LambdaExpr) and call_arg.arguments: - ctx.api.msg.fail( - f'Expected 0 arguments for : {call_pos} arg', - context=call_arg, - code=ERROR_BAD_ARG, - ) - elif isinstance(call_arg, RefExpr) and isinstance( - call_arg.node, FuncDef) and call_arg.node.arguments: - ctx.api.msg.fail( - f'Expected 0 arguments for {call_arg.fullname}: {call_pos} arg', - context=call_arg, - code=ERROR_BAD_ARG, - ) - - for log_msg_kwarg in log_msg_expected_kwargs: - maybe_kwarg_expr = call_kwargs.pop(log_msg_kwarg, None) - if maybe_kwarg_expr is None: - ctx.api.msg.fail( - f'{log_msg_kwarg} keyword argument is missing', - context=log_msg_expr, - code=ERROR_BAD_KWARG, - ) - return ctx.default_return_type - elif logger_opts.lazy: + _check_str_format_call(log_msg_expr, ctx) + + if logger_opts.lazy: + # collect call args/kwargs + # due to funky structure mypy offers here, it's easier + # to beg for forgiveness here + try: + call_args = ctx.args[1] + except IndexError: + call_args = [] + try: + call_kwargs = { + kwarg_name: ctx.args[2][idx] + for idx, kwarg_name in enumerate(ctx.arg_names[2]) + } + except IndexError: + call_kwargs = {} + + # collect args/kwargs from string interpolation + log_msg_value: str = log_msg_expr.value + log_msg_expected_kwargs = [] + for res in string.Formatter().parse(log_msg_value): + if res[1] is None: + continue + else: + log_msg_expected_kwargs.append(res[1].strip()) + + for call_pos, call_arg in enumerate(call_args): + if isinstance(call_arg, LambdaExpr) and call_arg.arguments: + ctx.api.msg.fail( + f'Expected 0 arguments for : {call_pos} arg', + context=call_arg, + code=ERROR_BAD_ARG, + ) + elif isinstance(call_arg, RefExpr) and isinstance( + call_arg.node, FuncDef) and call_arg.node.arguments: + ctx.api.msg.fail( + f'Expected 0 arguments for {call_arg.fullname}: {call_pos} arg', + context=call_arg, + code=ERROR_BAD_ARG, + ) + + for log_msg_kwarg in log_msg_expected_kwargs: + maybe_kwarg_expr = call_kwargs.pop(log_msg_kwarg, None) if isinstance(maybe_kwarg_expr, LambdaExpr) and maybe_kwarg_expr.arguments: ctx.api.msg.fail( f'Expected 0 arguments for : {log_msg_kwarg} kwarg', @@ -142,13 +157,6 @@ def _loguru_logger_call_handler( code=ERROR_BAD_KWARG, ) - for extra_kwarg_name in call_kwargs: - ctx.api.msg.fail( - f'{extra_kwarg_name} keyword argument not found in log message', - context=log_msg_expr, - code=ERROR_BAD_KWARG, - ) - return ctx.default_return_type diff --git a/typesafety/test_builtin_severities.yml b/typesafety/test_builtin_severities.yml index 82a01de..52a8add 100644 --- a/typesafety/test_builtin_severities.yml +++ b/typesafety/test_builtin_severities.yml @@ -60,11 +60,11 @@ logger.error('test {} {} {}', 11, 22, 33, 44) logger.exception('{}', 1, 2) out: | - main:3: note: Expected 0 but found 1 positional arguments for log message - main:4: note: Expected 1 but found 3 positional arguments for log message - main:5: note: Expected 2 but found 3 positional arguments for log message - main:6: note: Expected 3 but found 4 positional arguments for log message - main:7: note: Expected 1 but found 2 positional arguments for log message + main:3: error: Not all arguments converted during string formatting [str-format] + main:4: error: Not all arguments converted during string formatting [str-format] + main:5: error: Not all arguments converted during string formatting [str-format] + main:6: error: Not all arguments converted during string formatting [str-format] + main:7: error: Not all arguments converted during string formatting [str-format] - case: missing_arg main: | from loguru import logger @@ -75,10 +75,10 @@ logger.error('test {} {} {} {} {}', 11, 22, 33, 44) logger.exception('{} {} {}', 1, 2) out: | - main:3: error: Missing 1 positional arguments for log message [logger-arg] - main:4: error: Missing 1 positional arguments for log message [logger-arg] - main:6: error: Missing 1 positional arguments for log message [logger-arg] - main:7: error: Missing 1 positional arguments for log message [logger-arg] + main:3: error: Cannot find replacement for positional format specifier 1 [str-format] + main:4: error: Cannot find replacement for positional format specifier 3 [str-format] + main:6: error: Cannot find replacement for positional format specifier 4 [str-format] + main:7: error: Cannot find replacement for positional format specifier 2 [str-format] - case: extra_kwarg main: | from loguru import logger @@ -96,11 +96,11 @@ except ZeroDivisionError: logger.exception('{a} / {b}', a=2, b=0, c=0) out: | - main:3: error: a keyword argument not found in log message [logger-kwarg] - main:6: error: b keyword argument not found in log message [logger-kwarg] - main:8: error: a keyword argument not found in log message [logger-kwarg] - main:9: error: a keyword argument not found in log message [logger-kwarg] - main:14: error: c keyword argument not found in log message [logger-kwarg] + main:3: error: Not all arguments converted during string formatting [str-format] + main:6: error: Not all arguments converted during string formatting [str-format] + main:8: error: Not all arguments converted during string formatting [str-format] + main:9: error: Not all arguments converted during string formatting [str-format] + main:14: error: Not all arguments converted during string formatting [str-format] - case: missing_kwarg main: | import random @@ -113,8 +113,8 @@ bar=random.randint(1, 10), ) out: | - main:4: error: b keyword argument is missing [logger-kwarg] - main:6: error: car keyword argument is missing [logger-kwarg] + main:4: error: Cannot find replacement for named format specifier "b" [str-format] + main:6: error: Cannot find replacement for named format specifier "car" [str-format] - case: bad_callable main: | from operator import add diff --git a/typesafety/test_dot_access.yml b/typesafety/test_dot_access.yml new file mode 100644 index 0000000..c49b49e --- /dev/null +++ b/typesafety/test_dot_access.yml @@ -0,0 +1,33 @@ +--- +- case: ok__dot_access + parametrized: + - level: info + - level: debug + - level: warning + - level: error + - level: trace + - level: exception + main: | + from loguru import logger + + class Foo: + bar = "baz" + + foo = Foo() + + logger.{{ level }}('The bar is "{0.bar}"', foo) + logger.{{ level }}('The bar is "{my_foo.bar}"', my_foo=foo) +- case: no_attribute__dot_access + main: | + from loguru import logger + + class Foo: + bar = "baz" + + foo = Foo() + + logger.info('The bar is "{0.baz}"', foo) + logger.info('The bar is "{my_foo.baz}"', my_foo=foo) + out: | + main:8: error: "Foo" has no attribute "baz" [attr-defined] + main:9: error: "Foo" has no attribute "baz" [attr-defined]