Add more tests for simple augmented assignments

[tristanlatr]

Description

This PR adds a test for with two augmented assignments, I believe there is a flaw in the logic, so creating a failing test for it.

Type of Changes

	Type
✓	Adds a test

Related Issue

#192

[coveralls]

Pull Request Test Coverage Report for Build 2477304180

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.01%) to 92.233%

---

Totals
Change from base Build 2468320632:	0.01%
Covered Lines:	9358
Relevant Lines:	10146

---

💛 - Coveralls

[tristanlatr]

Weird, the test does not pass locally with astroid-2.11.5.

Did you fixed #192 without closing the issue recently ?

[Pierre-Sassoulas]

Thank you for the regression test and the issue triaging 👍 It's entirely possible that some issues are not up to date and should be closed. Would you mind adding the example from the issue with lists maybe it was the problem and not integer ?

[tristanlatr]

Would you mind adding the example from the issue with lists maybe it was the problem and not integer ?

The exact same test case fails locally. I'll investigate more.

[tristanlatr]

Sorry for all of this, I was just confused. I'm hacking hard and did not see an obvious issue in my test case.

Astroid is working properly ^^ :)

The good thing is that you can close #192 and you have more tests ;)

[tristanlatr]

Nevertheless, I believe the getattr() method does not handle augmented assignments very well. I'll try to provide some more test cases for this.

[tristanlatr]

So here's the weird behaviour:

    def test_except_assign_after_block_overwritten_getattr(self) -> None:
        """When a variable is assigned in an except clause, it is not returned
        when it is reassigned and used after the except block.
        """
        code = """
            try:
                1 / 0
            except ZeroDivisionError:
                x = 10
            except NameError:
                x = 100
            x = 1000
            print(x)
        """
        astroid = builder.parse(code)
        stmts = astroid.getattr('x')
>       self.assertEqual(len(stmts), 1)
E       AssertionError: 3 != 1

Shouldn't it return only the last assignment?

[tristanlatr]

Or is it me that is missing something ?

‘getattr’ seems to return statements that should get flittered-out, because they’re overridden in the main flow.

Tell me what you think.

[DanielNoord]

I believe this works like intended:

❯ cat test.py
from astroid import builder, extract_node

code = """
    try:
        1 / 0
    except ZeroDivisionError:
        x = 10
    except NameError:
        x = 100
    x = 1000
    x
"""
astroid = builder.parse(code)
stmts = astroid.getattr("x")

print(stmts)
node = extract_node(code)
print(list(node.infer()))
❯ python test.py
[<AssignName.x l.5 at 0x1021e8090>, <AssignName.x l.7 at 0x102289650>, <AssignName.x l.8 at 0x1022b6d10>]
[<Const.int l.8 at 0x1022b5d90>]

getattr returns all assignments in a module, whereas inferring the value at the end of the flow correctly infers what that value should be.

[tristanlatr]

Hi @DanielNoord,

Thanks for the explanation, I have a follow-up question:

getattr returns all assignments in a module

If I'm not mistaken, getattr is used to infer Attribute instances.
So in the context of the following code:

class C:
    try:
        1 / 0
    except ZeroDivisionError:
        x = 10
    except NameError:
        x = 100
    x = 1000

C.x

C.x wouldn't resolve to a single node, which seems odd (I've added a test for that).

Tell me what you think,

[DanielNoord]

Yeah, that does seem like a bug.

This is somewhat related to what I describe in pylint-dev/pylint#5264 (comment). Basically attribute and method resolvement on ClassDefs are not able to consider control-flows. Perhaps a solution can fix both at the same time?

[tristanlatr]

I believe it's indeed the same root cause.

I've initially discovered this issue because I'm building work based on astroid that keeps the compatibility with standard library nodes: https://github.com/tristanlatr/astuce, and by replicating the logic, I also replicated the bug.

And I fixed it by adding a sentinel node at the end of every frame nodes, and using it to do the lookups in the context of the frame.

Adding sentinel: https://github.com/tristanlatr/astuce/blob/7704c0be5b36993e64d0981d0c9634d74a6f84c0/astuce/parser.py#L112
Using it in getattr: https://github.com/tristanlatr/astuce/blob/7704c0be5b36993e64d0981d0c9634d74a6f84c0/astuce/inference.py#L204

It's probably not the best way to go though!

Tell me what you think.

[DanielNoord]

I think the issue should probably be resolved by taking all/the last definition into consideration. For __get__ on ClassDef's the last definition should probably be enough while getattr should probably consider all assignments. That would require some control flow knowledge in some cases, but would avoid returning incorrect values.