False positive on multiline dict inside decorator

[nrlulz]

def decorator():
    pass

def func():
    pass

@decorator([
    func(
        data={
            'as': 'df',
        },
    ),
])  # <-- JS102
def test1(self, data):
    pass

@decorator([
    func(
        data={'as': 'df'},
    ),
])  # <-- no warning
def test2(self, data):
    pass

@decorator([
    func(
        something=('a', 'b', 'c'),
        data={
            'as': 'df',
        },
    ),
])  # <-- no warning
def test3(self, data):
    pass

I think I'm seeing a false positive here (using v0.0.18). If I put that data dict in a single line, there is no warning. If I pass any other kwarg to func that has a function call or tuple (in one line), there is no warning. It seems to be when there are only dicts as values like this that I get this warning. We use this pattern often for parameterized tests, and I guess I never noticed this until now because I always had some date object or tuple in the arguments as well, which made it not produce a lint warning. I'm not totally sure which is the intended behavior here, since the square brackets and curly braces in this example indeed do not open and close on the same column.

[jsfehler]

If you can give me a working, minimal example and a traceback, I'll take a look at it tomorrow.

[sbabashahi]

Same issue:

@pytest.mark.parametrize(
    (
        'var1',
        'var2',
    ),
    [
        (
            AsyncMock(),
            'error1',
        ),
        (
            AsyncMock(
                var=1,
            ),
            'error2',
        ),
    ],  # <-- flake8 error points this line, JS102 Multi-line container does not close on same column as opening
)
def my_func():
    pass

but this doesn't raise error:

@pytest.mark.parametrize(
    (
        'var1',
        'var2',
    ),
    [
        (
            AsyncMock(),
            'error1',
        ),
        (
            AsyncMock(var=1),
            'error2',
        ),
    ],
)
def my_func():
    pass

[jokerejoker]

@parameterized.expand([
    (
        'reason UNVERIFIED_LOCATION',
        {
            'error': {
                'code': 400,
                'message': 'Request contains an invalid argument.',
                'status': 'INVALID_ARGUMENT',
                'details': [
                    {
                        '@type': 'type.googleapis.com/google.rpc.ErrorInfo',
                        'reason': 'UNVERIFIED_LOCATION',
                        'domain': 'mybusinessqanda.googleapis.com',
                    },
                ],  # <-- JS102
            },  # <-- JS102
        },  # <-- JS102
    ),
    (
        'reason INELIGIBLE_PLACE',
        {
            'error': {
                'code': 400,
                'message': 'Request contains an invalid argument.',
                'status': 'INVALID_ARGUMENT',
                'details': [
                    {
                        '@type': 'type.googleapis.com/google.rpc.ErrorInfo',
                        'reason': 'INELIGIBLE_PLACE',
                        'domain': 'mybusinessqanda.googleapis.com',
                    },
                ],
            },
        },
    ),
])  # <-- JS102
def my_func(self):
    pass

[njiles]

We also ran into this with parametrised tests, but with a bit of trial and error, I managed to reduce it to this MWE:

[(
    foo(
    )
)]

raises "foo.py:4:2: JS102 Multi-line container does not close on same column as opening." I wasn't able to reduce the example any further: for example, if I tried removing either set of outer brackets/parentheses, or putting all of foo() on one line, the error is no longer raised. The error also goes away if I change the outer square brackets to parentheses, or the parentheses to square brackets (or both).

I'm using version 0.0.19.

(Aside: the above example also correctly raises "foo.py:1:2: JS101 Multi-line container not broken after opening character". However, breaking the outer container to

[
    (
        foo(
        )
    )
]

removes this JS101 error, but does still raise "foo.py:6:1: JS102 Multi-line container does not close on same column as opening".)

[njiles]

After a brief investigation, it looks like (line 243) self.function_depth doesn't get decremented when the function call is inside a container: the decrement only occurs when (among other conditions) len(self.last_starts_at) == 0. This means that the second ) is also inferred to be closing a function call (as we still erroneously satisfy self.function_depth > 0), so we do not pop the last start location from self.last_starts_at (which at this point is [0, 4] and should become [0]). Finally, when the index of ], which is 0, is compared to the last value of self.last_starts_at, they disagree, causing the error to be raised.

Note that

(
    (
        foo(
        )
    )
)

will not cause an error to be raised, even though the logic is near-identical to the above. This is because the third ) is still being treated as closing the function call, and this tool does not check calls.