Python 3.9: codegen creates invalid file

[bnavigator]

Trying to package libcst 0.3.17 for Python 3.9 (after fixing typing_inspect) I run into the following test failure:

[   44s] ======================================================================
[   44s] FAIL: test_codegen_clean_matcher_classes (libcst.codegen.tests.test_codegen_clean.TestCodegenClean)
[   44s] Verifies that codegen of matcher classes would not result in a
[   44s] ----------------------------------------------------------------------
[   44s] Traceback (most recent call last):
[   44s]   File "/home/abuild/rpmbuild/BUILD/libcst-0.3.16/libcst/codegen/tests/test_codegen_clean.py", line 81, in test_codegen_clean_matcher_classes
[   44s]     self.assertTrue(
[   44s] AssertionError: False is not true : libcst.matchers.__init__ needs new codegen!
[   44s] 
[   44s] ----------------------------------------------------------------------
[   44s] Ran 1788 tests in 34.980s

python3.9 -m libcst.codegen.generate matchers creates a vastly different libcst/matchers/__init__.py from python3.8, specifically one that creates

[   27s] ======================================================================
[   27s] ERROR: libcst.codegen.tests.test_codegen_clean (unittest.loader._FailedTest)
[   27s] ----------------------------------------------------------------------
[   27s] ImportError: Failed to import test module: libcst.codegen.tests.test_codegen_clean
[   27s] Traceback (most recent call last):
[   27s]   File "/usr/lib64/python3.9/unittest/loader.py", line 436, in _find_test_path
[   27s]     module = self._get_module_from_name(name)
[   27s]   File "/usr/lib64/python3.9/unittest/loader.py", line 377, in _get_module_from_name
[   27s]     __import__(name)
[   27s]   File "/home/abuild/rpmbuild/BUILD/libcst-0.3.17/libcst/codegen/tests/test_codegen_clean.py", line 12, in <module>
[   27s]     from libcst.codegen.generate import clean_generated_code, format_file
[   27s]   File "/home/abuild/rpmbuild/BUILD/libcst-0.3.17/libcst/codegen/generate.py", line 21, in <module>
[   27s]     from libcst.codegen.transforms import (
[   27s]   File "/home/abuild/rpmbuild/BUILD/libcst-0.3.17/libcst/codegen/transforms.py", line 18, in <module>
[   27s]     import libcst.matchers as m
[   27s]   File "/home/abuild/rpmbuild/BUILD/libcst-0.3.17/libcst/matchers/__init__.py", line 243, in <module>
[   27s]     class AnnAssign(BaseSmallStatement, BaseMatcherNode):
[   27s]   File "/home/abuild/rpmbuild/BUILD/libcst-0.3.17/libcst/matchers/__init__.py", line 257, in AnnAssign
[   27s]     Optional["BaseExpression"],
[   27s] NameError: name 'Optional' is not defined

Related to Instagram/MonkeyType#74 ?

[bnavigator]

Found a fix in the PKGBUILD for Archlinux: sed -i 's/import AbstractBaseMatcherNodeMeta/import Optional, AbstractBaseMatcherNodeMeta/' libcst/codegen/gen_matcher_classes.py

@FFY00, did you report this upstream? I could not find anything.

[FFY00]

Hi, this somehow got lost in my sea of notifications. I did not, the fix didn't come from me.

[zsol]

The problem is that dataclasses.fields returns a different type representation for Optional fields starting from 3.9. Before 3.9, this script:

import dataclasses
from typing import Optional


@dataclasses.dataclass
class Foo:
  bar: Optional[int]

if __name__ == "__main__":
  print(dataclasses.fields(Foo))

printed

(Field(name='bar',type=typing.Union[int, NoneType],...

But since 3.9 it prints

(Field(name='bar',type=typing.Optional[int],...

I think the right fix here is to

• run the codegen with python 3.9
• add typing.Optional to the import list for the generated matcher file
• disable the codegen cleanliness CI check for python versions < 3.9

[lpetre]

I think the right fix here is to

• run the codegen with python 3.9
• add typing.Optional to the import list for the generated matcher file
• disable the codegen cleanliness CI check for python versions < 3.9

I've done 1 and 2 of that in #506

I'm curious if we can make the codegen treat Union[X, NoneType] generate identical code to Optional[X]

There is something curious here: https://github.com/Instagram/LibCST/blob/master/libcst/codegen/gen_matcher_classes.py#L299-L301

I feel like that code is trying to handle Union[Something, None]?

[lpetre]

I'm curious if we can make the codegen treat Union[X, NoneType] generate identical code to Optional[X]

I've implemented this in #508