Support attribute references of positional/keyword arguments with string formatting

[ThibaultLemaire]

from loguru import logger

class Foo:
  bar = "baz"

foo = Foo()

# Both the following are valid syntax, but mypy flags them
logger.debug('The bar is "{0.bar}"', foo)  # note: Expected 0 but found 1 positional arguments for log message
logger.debug('The bar is "{my_foo.bar}"', my_foo=foo)  # error: my_foo.bar keyword argument is missing

❯ pip freeze | grep loguru
loguru==0.5.2
loguru-mypy==0.0.2

[issue-label-bot]

Issue-Label Bot is automatically applying the label feature_request to this issue, with a confidence of 0.80. Please mark this comment with 👍 or 👎 to give our bot feedback!

Links: app homepage, dashboard and code for this bot.

[ThibaultLemaire]

(IMHO this is more a bug since it's flagging valid syntax)

[kornicameister]

HI @ThibaultLemaire . I am deeply sorry but I don't think there will be much time for me to take a look. Actually I don't have time even to finish PR that is already here and another code sits in my desk. Dang, it was even 7 days already since reporting this issue.

If you have an idea how to fix that PR are welcome ;)

PS. It actually looks ok, so I think you're right and that's a bug.

If you don't mind dropping loguru version, that'd be great.

[ThibaultLemaire]

I'll take a look at the code and see what I can do. It should be the opportunity to see how a mypy plugin is made, maybe I'll learn a trick or two to enhance mypy checks (I'm thinking mainly of type-checking pymongo/motor queries, as that's something I've been meaning to do for a long time).

I've updated the description with loguru and loguru-mypy versions

[ThibaultLemaire]

Okay, so it looks like you've got some custom code there to support loguru's syntax, but mypy already has the capability to type .format(), like so:

class Foo:
    bar = "baz"

foo = Foo()

'The bar is "{0.bar}"'.format(foo)  # typechecks
'The bar is "{0.bar}"'.format("not foo")  # error: "str" has no attribute "bar"
'The bar is "{my_foo.bar}"'.format(my_foo=foo)  # typechecks
'The bar is "{my_foo.bar}"'.format(my_foo="not foo")  # error: "str" has no attribute "bar"

So I wonder if we could leverage mypy directly to typecheck something like

logger.debug('The bar is "{my_foo.bar}"', my_foo=foo)

as if it was written like this

logger.debug('The bar is "{my_foo.bar}"'.format(my_foo=foo))

[ThibaultLemaire]

Progress report

After wiring up a debugger to mypy I was able to analyse the way it checks calls to .format. The call stack looks roughly like (Most recent call last):

mypy.checkexpr.ExpressionChecker.visit_call_expr
mypy.checkexpr.ExpressionChecker.check_str_format_call
mypy.checkstrformat.StringFormatterChecker.check_str_format_call

Where that last check_str_format_call is the one I'm really interested in, the one that should do all the type checking for us.

Now all I need is a way to call that method with all the necessary arguments as mypy is a very tightly coupled mess.

Hopefully though, the ctx.api that is passed to the plugin seems to have quite a lot of methods available, among which is visit_call_expr, so that looks like a promising line of investigation

[ThibaultLemaire]

Progress report

So after further investigation, ctx.api is an instance of mypy.checker.TypeChecker that's why it has all those nice methods.

Now, I've been able to create a mypy.nodes.CallExpr that looks enough like that of a .format call (I think) and I was able to call ctx.api.visit_call_expr without error, but I don't get the corresponding mypy warnings...

I'll have to trace and debug mypy with the loguru plugin to see exactly what's going wrong.

[ThibaultLemaire]

There we go, I've got some sprout of working code, continuing technical discussion over at PR #43

[ThibaultLemaire]

Hey @kornicameister when do you think this will be released? I'd like to use it at work.

[kornicameister]

@ThibaultLemaire I will release it with 0.0.3 but there are some build errors.I think I have introduced some time ago.

[ThibaultLemaire]

Ah that's too bad, don't worry though, this is low priority for me.

[kornicameister]

Not for me ;-)
I have some time off to play around here, so I will release things soon I believe.

[kornicameister]

@ThibaultLemaire released @ https://pypi.org/project/loguru-mypy/0.0.3

PyPI

loguru-mypy

[ThibaultLemaire]

Thank you very much, I added loguru-mypy to our project on a feature branch and confirmed it's working in the scenario that initally prompted me to open this issue. 👍

However, I'm hitting two new scenarii where the plugin fails to correctly validate the code 🙁

The first, I had anticipated when working on my PR:

from loguru import logger

logger.configure(
    handlers=[
        dict(
            sink=sys.stderr,
            format="Site {extra[site]} broadcasting: {message}",
            backtrace=False,
            diagnose=False,
        ),
    ],
    extra={"site": "unknown"},
)

site = 19
scp = 682
# We're passing the `site` number for local formatting of this message, but we're also passing the `scp` number as `extra` for the formatter of the handler.
logger.warning("SCP-{} containment breach detected", scp, site=site)
# error: Not all arguments converted during string formatting
# But this is easily circumvented:
logger.bind(site=site).warning("SCP-{} containment breach detected", scp)

The workaround isn't too bad, and fixing the issue - I think - would prove a tad more complicated than my previous PR (see #48 ).

But the second issue is a real blocker: essentially it's the 3rd exemple of #49 as reported by @Delgan

from loguru import logger

foo = "bar"
logger.info(foo)  # error: INTERNAL ERROR

So I think I'm going to start working on fix for that, as I think it shouldn't be too complicated.