Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use PEP 585 syntax wherever possible #6717

Merged
merged 5 commits into from
Dec 28, 2021
Merged

Conversation

AlexWaygood
Copy link
Member

@AlexWaygood AlexWaygood commented Dec 28, 2021

This PR proposes using PEP 585 syntax where possible. There are two situations where this is currently not possible:

  • Lowercase type still causes mypy errors in some situations (see, e.g., here).
  • Importing from collections.abc rather than typing will often cause pytype to error (see, e.g., here).
This PR was created using the following script
import ast
import re
import subprocess
import sys
from collections import defaultdict
from itertools import chain
from operator import attrgetter
from pathlib import Path
from typing import NamedTuple


class DeleteableImport(NamedTuple):
    old: str
    replacement: str


class NewImport(NamedTuple):
    text: str
    indentation: int


FORBIDDEN_BUILTIN_TYPING_IMPORTS = frozenset({"List", "FrozenSet", "Set", "Dict", "Tuple"})

# The values in the mapping are what these are called in `collections`
IMPORTED_FROM_COLLECTIONS_NOT_TYPING = {
    "Counter": "Counter",
    "Deque": "deque",
    "DefaultDict": "defaultdict",
    "OrderedDict": "OrderedDict",
    "ChainMap": "ChainMap",
}

FAILURES = []


def fix_bad_syntax(path: Path) -> None:
    with open(path) as f:
        stub = f.read()

    lines = stub.splitlines()
    tree = ast.parse(stub)
    imports_to_delete = {}
    imports_to_add = defaultdict(list)
    classes_from_typing = set()

    class BadImportFinder(ast.NodeVisitor):
        def visit_ImportFrom(self, node: ast.ImportFrom) -> None:
            if node.module != "typing":
                return

            bad_builtins_classes_in_this_import = set()
            bad_collections_classes_in_this_import = set()

            for cls in node.names:
                if cls.name in FORBIDDEN_BUILTIN_TYPING_IMPORTS:
                    bad_builtins_classes_in_this_import.add(cls)
                elif cls.name in IMPORTED_FROM_COLLECTIONS_NOT_TYPING:
                    bad_collections_classes_in_this_import.add(cls)

            bad_classes_in_this_import = bad_builtins_classes_in_this_import | bad_collections_classes_in_this_import

            if not bad_classes_in_this_import:
                return

            classes_from_typing.update(cls.name for cls in bad_classes_in_this_import)
            new_import_list = [cls for cls in node.names if cls not in bad_classes_in_this_import]

            ### DEALING WITH EXISTING IMPORT STATEMENTS ###

            # Scenario (1): Now we don't need *any* imports from typing any more.
            if not new_import_list:
                if path == Path("stdlib/csv.pyi"):
                    imports_to_delete[node.lineno - 1] = DeleteableImport(
                        old=ast.unparse(node), replacement="from builtins import dict as _DictReadMapping"
                    )
                else:
                    imports_to_delete[node.lineno - 1] = DeleteableImport(old=ast.unparse(node), replacement="")

            # Scenario (2): we still need imports from typing; the existing import statement is only one line
            elif node.lineno == node.end_lineno:
                imports_to_delete[node.lineno - 1] = DeleteableImport(
                    old=ast.unparse(node),
                    replacement=ast.unparse(ast.ImportFrom(module="typing", names=new_import_list, level=0)),
                )

            # Scenario (3): we still need imports from typing; the existing import statement is multiline.
            else:
                for cls in node.names:
                    if cls in bad_classes_in_this_import:
                        imports_to_delete[cls.lineno - 1] = DeleteableImport(
                            old=f"{cls.name}," if cls.asname is None else f"{cls.name} as {cls.asname},", replacement=""
                        )

            ### ADDING NEW IMPORT STATEMENTS ###

            if bad_collections_classes_in_this_import:
                imports_to_add[node.lineno - 1].append(
                    NewImport(
                        text=ast.unparse(
                            ast.ImportFrom(
                                module="collections",
                                names=[
                                    ast.alias(name=IMPORTED_FROM_COLLECTIONS_NOT_TYPING[cls.name], asname=cls.asname)
                                    for cls in sorted(bad_collections_classes_in_this_import, key=attrgetter("name"))
                                ],
                                level=0,
                            )
                        ),
                        indentation=node.col_offset,
                    )
                )

    BadImportFinder().visit(tree)

    if not classes_from_typing:
        return

    for lineno, (old_syntax, new_syntax) in imports_to_delete.items():
        lines[lineno] = lines[lineno].replace(old_syntax, new_syntax)

    for lineno, import_list in imports_to_add.items():
        for new_import, indentation in import_list:
            if isinstance(new_import, str):
                lines.insert(lineno, f'{" "*indentation}{new_import}')
            else:
                lines = lines[:lineno] + [f'{" "*indentation}{l}' for l in new_import] + lines[lineno:]

    try:
        new_tree = ast.parse("\n".join(lines))
    except SyntaxError:
        sys.stderr.write(f"Error converting new syntax in {path}")
        FAILURES.append(path)
    else:
        lines_with_bad_syntax = defaultdict(list)

        class OldSyntaxFinder(ast.NodeVisitor):
            def visit_Subscript(self, node: ast.Subscript) -> None:
                if isinstance(node.value, ast.Name) and node.value.id in (
                    classes_from_typing & (FORBIDDEN_BUILTIN_TYPING_IMPORTS | {"Deque", "DefaultDict"})
                ):
                    lines_with_bad_syntax[node.lineno - 1].append(node.value.id)
                self.generic_visit(node)

        OldSyntaxFinder().visit(new_tree)

        for i, cls_list in lines_with_bad_syntax.items():
            for cls in cls_list:
                lines[i] = re.sub(fr"(\W){cls}\[", fr"\1{cls.lower()}[", lines[i])

    new_stub = "\n".join(lines) + "\n"

    if path == Path("stdlib/plistlib.pyi"):
        new_stub = new_stub.replace("_Dict", "dict")

    with open(path, "w") as f:
        f.write(new_stub)


def main() -> None:
    print("STARTING RUN: Will attempt to fix new syntax in typeshed directory...\n\n")
    for path in chain(Path("stdlib").rglob("*.pyi"), Path("stubs").rglob("*.pyi")):
        if "@python2" in path.parts:
            print(f"Skipping {path}: Python-2 stub")
        elif Path("stubs/protobuf/google/protobuf") in path.parents:
            print(f"Skipping {path}: protobuf stub")
        else:
            print(f"Attempting to convert {path} to new syntax.")
            fix_bad_syntax(path)

    print("\n\nSTARTING ISORT...\n\n")
    for folder in {"stdlib", "stubs", "tests"}:
        subprocess.run([sys.executable, "-m", "isort", folder])

    print("\n\nSTARTING BLACK...\n\n")
    subprocess.run([sys.executable, "-m", "black", "."])

    if FAILURES:
        print("\n\nFAILED to convert the following files to new syntax:\n")
        for path in FAILURES:
            print(f"- {path}")
    else:
        print("\n\nThere were ZERO failures in converting to new syntax. HOORAY!!\n\n")

    print('\n\nRunning "check_new_syntax.py"...\n\n')
    subprocess.run([sys.executable, "tests/check_new_syntax.py"])

    print('\n\nRunning "stubtest_stdlib.py"...\n\n')
    subprocess.run([sys.executable, "tests/stubtest_stdlib.py"])


if __name__ == "__main__":
    main()

@github-actions

This comment has been minimized.

@AlexWaygood AlexWaygood marked this pull request as ready for review December 28, 2021 00:14
@github-actions
Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@AlexWaygood AlexWaygood changed the title Use PEP 585 syntax where possible Use PEP 585 syntax wherever possible Dec 28, 2021
Copy link
Collaborator

@srittau srittau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spot checking showed no problems. Apart from that, I trust our tests.

@srittau srittau merged commit 8d5d252 into python:master Dec 28, 2021
@AlexWaygood AlexWaygood deleted the replace-syntax branch December 28, 2021 10:36
@AlexWaygood
Copy link
Member Author

Thanks @srittau! 😀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants