zheller / flake8-quotes

Flake8 extension for checking quotes in python
MIT License
178 stars 39 forks source link

Buggy behavior with empty dict literal and string: #97

Closed squeaky-pl closed 4 years ago

squeaky-pl commented 4 years ago

Consider the following example:

def test():
    {}["a"]

and this configuration:

[flake8]
max-line-length = 119
select = Q0
inline-quotes = double

this ends up with Q002 Remove bad quotes from docstring when there is no docstring here involved.

Splitting this over two lines like:

def test():
    d = {}
    d["a"]

Resolves it.

twolfson commented 4 years ago

Oh wow, interesting edge case. I'll look into reproducing this either today or tomorrow

On Mon, May 11, 2020, 5:37 AM Paweł Piotr Przeradowski < notifications@github.com> wrote:

Consider the following example:

def test(): {}["a"]

and following confiburation:

[flake8] max-line-length = 119 select = Q0 inline-quotes = double

this ends up with Q002 Remove bad quotes from docstring when there is no docstring here involved.

Splitting this over two lines like:

def test(): d = {} d["a"]

Resolves it.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/zheller/flake8-quotes/issues/97, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAG4KWAK4Y6QBLRK4PDE733RQ7WPNANCNFSM4M534GDQ .

twolfson commented 4 years ago

I've successfully reproduced the issue, at first glance it looks like an issue in our custom docstring detection function but I'll need to dig deeper which I don't have time for at the moment. I'll take a shot at fixing this by Sunday

Also going to @ @oxitnik since they've worked on this code last/might not have forgotten as much as I have

oxitnik commented 4 years ago

Yes. issue in get_docstring_tokens. state needs to be reset when brackets are handled. I tried to replace https://github.com/zheller/flake8-quotes/blob/26fe39c9b9a160a4dc2f3c975f71f0479bc193b7/flake8_quotes/docstring_detection.py#L64-L67 with

        elif token.type == tokenize.OP and token.string in ['(', '[', '{']:
            bracket_count += 1
            if state in [STATE_EXPECT_MODULE_DOCSTRING, STATE_EXPECT_CLASS_DOCSTRING,
                       STATE_EXPECT_FUNCTION_DOCSTRING]:
                state = STATE_OTHER
        elif token.type == tokenize.OP and token.string in [')', ']', '}']:
            bracket_count -= 1
            if state in [STATE_EXPECT_MODULE_DOCSTRING, STATE_EXPECT_CLASS_DOCSTRING,
                       STATE_EXPECT_FUNCTION_DOCSTRING]:
                state = STATE_OTHER

and no docstrings have been found in snippet from issue.

twolfson commented 4 years ago

:+1: That patch works great =D Will do my best to properly attribute the commit in a PR

twolfson commented 4 years ago

This has been released in 3.2.0. Thanks for the bug report and fixes y'all =D