Make transformation of class annotations configurable to fix issue with transitive inheritance from TypedDict

[whummer]

First of all, huge thanks for providing this fantastic library! 🙌

We've discovered a small issue with type declarations and transitive class inheritance. If we use this sample code:

from typing import Optional, TypedDict
class A(TypedDict):
    arg1: str
class B(A):
    arg2: Optional[int]
class C(B):
    arg3: Optional[str]

... it would get minified to:

from typing import Optional,TypedDict
class A(TypedDict):arg1:str
class B(A):arg2:0
class C(B):arg3:0

... which is invalid Python code (tested under 3.8, 3.10, but also applies to other versions):

$ python test.py
  ...
  File "~/.pyenv/versions/3.10.4/lib/python3.10/typing.py", line 176, in _type_check
    raise TypeError(f"{msg} Got {arg!r:.100}.")
TypeError: TypedDict('Name', {f0: t0, f1: t1, ...}); each t must be a type Got 0.

The issue is that the library is currently not able to detect transitive inheritance dependencies (for good reasons, as this is a hard problem - probably infeasible to solve in the general case, as class hierarchies may be distributed across different source files, and minification is only performed within the scope of a single source file.)

One solution is to use the remove_annotations=False flag to retain all annotations, but in a larger codebase it can actually be beneficial to remove type declarations, especially from function/method args.

The code in question is this piece:

python-minifier/src/python_minifier/transforms/remove_annotations.py

Lines 100 to 106 in 9748fab

	else:
	# Valueless annotations cause the interpreter to treat the variable as a local.
	# I don't know of another way to do that without assigning to it, so
	# keep it as an AnnAssign, but replace the annotation with '0'
	node.annotation = self.add_child(ast.Num(0), parent=node.parent)
	return node

The ideal case would be if we can introduce a config flag to disable the transformation that is happening for class attributes in this block.

@dflook Would it make sense to distinguish between (1) function annotations and (2) class annotations, and introduce a new flag remove_annotations_class? We could then either introduce a new SuiteTransformer for class annotations, or alternatively leave the current structure and skip executing visit_AnnAssign if remove_annotations_class is False. Happy to work towards a PR, but would like to get your thoughts and guidance first.. 👍 Thanks!

[dflook]

Hi @whummer, thanks for creating an issue!

As you said, I think tracking transitive inheritance across arbitrary modules would be quite difficult and easily defeated by the dynamic nature of python. But there is still some room for improvement - I think it could be done within a module, if not completely at least well enough that it results in safer transformations.

The remove annotations transformation is a tricky one because even if we can supress it for specific classes from the python standard library, there are many other classes that should have annotations preserved that python-minifier can't know.

I think it makes sense to have separate options for removing annotations from variables, functions or class attributes.
Variable annotations are much safer to remove (in the sense of breaking arbitrary code) than function annotations, which are again much safer than class attribute annotations. The minification options are already a bit cluttered though, and coarse in that they apply to an entire module.

I'm considering a mechanism such that you can specify much more precise minification options, per namespace.
Something like (pseudo-yaml):

src/__main__.py::*:
  remove_annotations:
    variables: True
    function_arguments: True
    function_return: True
    class_attributes: True
src/__main__.py::MyClass:
  remove_annotations:
    class_attributes: False

For now though, If you want to work towards a PR to solve your problem I would suggest adding to RemoveAnnotations to accept an argument in __init__ which enables or disables class attribute annotations.