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

Always use _typeshed.Self, where applicable #6880

Merged
merged 10 commits into from
Jan 10, 2022

Conversation

AlexWaygood
Copy link
Member

This PR was prepared using the following script:
import ast
import re
import subprocess
import sys
from collections import defaultdict
from itertools import chain
from pathlib import Path
from typing import NamedTuple


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


FAILURES = []


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

    # PART 1: FIND AND REPLACE BAD TYPEVARS

    lines = stub.splitlines()
    tree = ast.parse(stub)
    lines_with_bad_typevars: dict[int, str] = {}

    def process_instance_meth(
        instance_meth: ast.FunctionDef | ast.AsyncFunctionDef, return_annotation: ast.Name | ast.Subscript
    ) -> None:
        arg1_annotation = instance_meth.args.args[0].annotation

        if isinstance(arg1_annotation, ast.Name) and arg1_annotation.id == return_annotation.id:
            lines_with_bad_typevars[arg1_annotation.lineno - 1] = arg1_annotation.id
            lines_with_bad_typevars[return_annotation.lineno - 1] = arg1_annotation.id

    def process_class_meth(cls_meth: ast.FunctionDef | ast.AsyncFunctionDef, return_annotation: ast.Name | ast.Subscript) -> None:
        arg1_annotation = cls_meth.args.args[0].annotation

        if not (
            isinstance(arg1_annotation, ast.Subscript)
            and isinstance(arg1_annotation.slice, ast.Name)
            and isinstance(arg1_annotation.value, ast.Name)
            and arg1_annotation.value.id == "type"
        ):
            return

        if isinstance(return_annotation, ast.Name) and return_annotation.id == arg1_annotation.slice.id:
            bad_typevar = True
        elif (
            isinstance(return_annotation, ast.Subscript)
            and isinstance(return_annotation.slice, ast.Name)
            and return_annotation.slice.id == arg1_annotation.slice.id
        ):
            bad_typevar = True
        else:
            bad_typevar = False

        if bad_typevar:
            lines_with_bad_typevars[arg1_annotation.lineno - 1] = arg1_annotation.slice.id
            lines_with_bad_typevars[return_annotation.lineno - 1] = arg1_annotation.slice.id

    class BadTypeVarFinder(ast.NodeVisitor):
        def visit_FunctionDef(self, node: ast.FunctionDef) -> None:
            return_annotation = node.returns

            if isinstance(return_annotation, ast.Name):
                if return_annotation.id == "Self":
                    return
                elif node.name == "__new__":
                    process_class_meth(node, return_annotation)
                    return
            elif isinstance(return_annotation, ast.Subscript):
                if return_annotation.value == "type":
                    if isinstance(return_annotation.slice, ast.Name):
                        if return_annotation.slice.id == "Self":
                            return
                        else:
                            pass
                    else:
                        return
                else:
                    return
            else:
                return

            for decorator in node.decorator_list:
                if isinstance(decorator, ast.Name):
                    decorator_name = decorator.id
                    if decorator_name == "classmethod":
                        process_class_meth(node, return_annotation)
                        return
                    elif decorator_name == "staticmethod":
                        return

            process_instance_meth(node, return_annotation)

        visit_AsyncFunctionDef = visit_FunctionDef

    class ClassFinder(ast.NodeVisitor):
        def visit_ClassDef(self, node: ast.ClassDef) -> None:
            BadTypeVarFinder().visit(node)

    ClassFinder().visit(tree)

    if not lines_with_bad_typevars:
        return

    for lineno, typevar_name in lines_with_bad_typevars.items():
        lines[lineno] = re.sub(fr"(\W){typevar_name}(\W)", r"\1Self\2", lines[lineno])

    # PART TWO: DELETE UNUSED TYPEVAR DEFINITIONS

    tree = ast.parse("\n".join(lines))

    suspect_typevar_defs: dict[str, int] = {}  # Mapping of suspect TypeVar names to the linenos where they're defined
    all_name_occurences: defaultdict[str, int] = defaultdict(int)  # Mapping of each name in the file to the no. of occurences

    class UnusedTypeVarFinder(ast.NodeVisitor):
        def visit_Assign(self, node: ast.Assign) -> None:
            if (
                isinstance(node.targets[0], ast.Name)
                and node.targets[0].id.startswith("_")
                and isinstance(node.value, ast.Call)
                and isinstance(node.value.func, ast.Name)
                and node.value.func.id == "TypeVar"
            ):
                suspect_typevar_defs[node.targets[0].id] = node.lineno
            self.generic_visit(node)

        def visit_Name(self, node: ast.Name) -> None:
            all_name_occurences[node.id] += 1
            self.generic_visit(node)

    UnusedTypeVarFinder().visit(tree)

    for typevar_name, lineno in suspect_typevar_defs.items():
        if all_name_occurences[typevar_name] == 1:
            lines[lineno - 1] = ""

    # PART THREE: DELETE UNUSED IMPORTS, ADD SELF IMPORT IF NECESSARY

    tree = ast.parse("\n".join(lines))
    typevar_import_needed = False

    class TypeVarDefFinder(ast.NodeVisitor):
        def visit_Assign(self, node: ast.Assign) -> None:
            nonlocal typevar_import_needed

            if (
                isinstance(node.targets[0], ast.Name)
                and isinstance(node.value, ast.Call)
                and isinstance(node.value.func, ast.Name)
                and node.value.func.id == "TypeVar"
            ):
                typevar_import_needed = True
            self.generic_visit(node)

    TypeVarDefFinder().visit(tree)

    import_lines = []
    imports_to_delete = {}
    Self_import_present = False

    class BadImportFinder(ast.NodeVisitor):
        def visit_ImportFrom(self, node: ast.ImportFrom) -> None:
            nonlocal Self_import_present

            if not node.col_offset:
                import_lines.append(node.end_lineno)

            if node.module == "_typeshed":
                if any(cls.name == "Self" for cls in node.names):
                    Self_import_present = True
            elif not typevar_import_needed and node.module == "typing" and any(cls.name == "TypeVar" for cls in node.names):
                new_import_list = [cls for cls in node.names if cls.name != "TypeVar"]

                if not new_import_list:
                    imports_to_delete[node.lineno - 1] = DeleteableImport(old=ast.unparse(node), replacement="")

                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)),
                    )
                else:
                    for cls in node.names:
                        if cls.name == "TypeVar":
                            imports_to_delete[cls.lineno - 1] = DeleteableImport(old="TypeVar,", replacement="")

    BadImportFinder().visit(tree)

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

    if not Self_import_present and path != Path("stdlib/_typeshed/__init__.pyi"):
        lines.insert(max(import_lines), "from _typeshed import Self")

    with open(path, "w") as f:
        f.write("\n".join(lines) + "\n")


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")
        elif path == Path("stdlib/typing_extensions.pyi"):
            print("Skipping `typing_extensions`")
        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", "."])

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

    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
Copy link
Member Author

The third-party stubtest failure for requests is unrelated to this PR.

@AlexWaygood
Copy link
Member Author

The third-party stubtest failure for requests is unrelated to this PR.

Here's a fix for the requests issue: #6882

@github-actions

This comment has been minimized.

@srittau srittau closed this Jan 9, 2022
@srittau srittau reopened this Jan 9, 2022
@github-actions

This comment has been minimized.

@@ -30,7 +31,7 @@ class Markdown:
def registerExtensions(self, extensions: Sequence[Extension | str], configs: Mapping[str, Mapping[str, Any]]) -> Markdown: ...
def build_extension(self, ext_name: Text, configs: Mapping[str, str]) -> Extension: ...
def registerExtension(self, extension: Extension) -> Markdown: ...
def reset(self: Markdown) -> Markdown: ...
def reset(self: Self) -> Self: ...
Copy link
Member Author

Choose a reason for hiding this comment

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

This is semantically different to the stub as it is currently, but looks more correct to me. https://github.com/Python-Markdown/markdown/blob/1d41f13c774696d651921601c827ed500e2aa285/markdown/core.py#L190

@@ -44,7 +44,7 @@ def retry_on_signal(function: Callable[[], _T]) -> _T: ...
def constant_time_bytes_eq(a: AnyStr, b: AnyStr) -> bool: ...

class ClosingContextManager:
def __enter__(self: _TC) -> _TC: ...
def __enter__(self: Self) -> Self: ...
Copy link
Member Author

Choose a reason for hiding this comment

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

This is also semantically different to the existing stub, but looks more correct to me. https://github.com/paramiko/paramiko/blob/88f35a537428e430f7f26eee8026715e357b55d6/paramiko/util.py#L300

@AlexWaygood AlexWaygood marked this pull request as draft January 9, 2022 23:49
@AlexWaygood AlexWaygood marked this pull request as ready for review January 10, 2022 00:01
@github-actions

This comment has been minimized.

@AlexWaygood AlexWaygood marked this pull request as draft January 10, 2022 00:07
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@AlexWaygood AlexWaygood marked this pull request as ready for review January 10, 2022 00:18
@github-actions

This comment has been minimized.

@AlexWaygood
Copy link
Member Author

The last 5 commits were prepared manually by grepping for def __enter__ and looking for annotations that seemed suspect, then comparing the stub to the source code. I've linked to the source code in each commit description.

@github-actions

This comment has been minimized.

1 similar comment
@github-actions
Copy link
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

steam.py (https://github.com/Gobot1234/steam.py)
- steam/enums.py:172: note:     def [_TT <: type] __new__(cls, cls: Type[_TT], str, Tuple[type, ...], Dict[str, Any], **kwds: Any) -> _TT
+ steam/enums.py:172: note:     def [Self] __new__(cls, cls: Type[Self], str, Tuple[type, ...], Dict[str, Any], **kwds: Any) -> Self
- steam/channel.py:162: note:     def [_TT <: type] __new__(cls, cls: Type[_TT], str, Tuple[type, ...], Dict[str, Any], **kwds: Any) -> _TT
+ steam/channel.py:162: note:     def [Self] __new__(cls, cls: Type[Self], str, Tuple[type, ...], Dict[str, Any], **kwds: Any) -> Self

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.

3 participants