Instagram / LibCST

A concrete syntax tree parser and serializer library for Python that preserves many aspects of Python's abstract syntax tree
https://libcst.readthedocs.io/
Other
1.52k stars 186 forks source link

Merge class attribute annotations in ApplyTypeAnnotationsVisitor #323

Open mkurnikov opened 4 years ago

mkurnikov commented 4 years ago

Currently, it just ignores class attributes

class MyClass:
    name: str

name: str won't appear in the sources.

jimmylai commented 4 years ago

Nice find! Looks like ApplyTypeAnnotationsVisitor only try to restore the type annotation when the source code is an Assign (e.g. name = "a"). In this case of a simple name, it's not handled properly. https://github.com/Instagram/LibCST/blob/5992a7d83d234e7575ad81107d352846ff1abb37/libcst/codemod/visitors/_apply_type_annotations.py#L468-L470

That can be fixed by match the Name node with pattern like this and reuse the code from leave_Assign to apply the type nnotation.

SimpleStatementLine(
                        body=[
                            Expr(
                                value=Name(
                                ),
                            ),
                        ],
                    )

Feel free to submit a PR to propose a fix. Test case can be added in https://github.com/Instagram/LibCST/blob/5992a7d83d234e7575ad81107d352846ff1abb37/libcst/codemod/visitors/tests/test_apply_type_annotations.py

CC @pradeep90

stroxler commented 3 years ago

I'm hoping to fix this in the next month or two, it is affecting our ability to add annotations that empower static analysis

shrik450 commented 3 months ago

The referencing commit here says that this should work, but I tested this today and it doesn't. The test case in that commit still has an assign, it's just at the top level of the class. If there is no declaration of or assignment to that variable altogether ApplyTypeAnnotationsVisitor ignores it.

stroxler commented 3 months ago

Yeah, that commit just demonstrated that we do annotate attributes that already exist. But we don't inject new ones, which we ought to do.