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.

Sign up for GitHub

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

Jump to bottom

PR: Add type annotations for rope.contrib.generate.create_generate() #641

Merged
merged 9 commits into from
Jan 5, 2023
Merged

PR: Add type annotations for rope.contrib.generate.create_generate() #641

Changes from 2 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
8fc5abb
Improve docstring for create_generate
edreamleo Jan 4, 2023
39a26e2
Fully annotate create_generate
edreamleo Jan 4, 2023
9b1fa56
Back out of Literal annotation
edreamleo Jan 4, 2023
7d220ea
Merge branch 'master' into ekr-ropemode-docstring
edreamleo Jan 4, 2023
af8f5c0
Merge branch 'master' into ekr-ropemode-docstring
edreamleo Jan 4, 2023
d85914b
Fix recent problems w/ annotations. Punt on the return type for now
edreamleo Jan 4, 2023
c9929a0
Merge branch 'ekr-ropemode-docstring' of https://github.com/edreamleo…
edreamleo Jan 4, 2023
4242e94
Merge branch 'master' into ekr-ropemode-docstring
lieryan Jan 4, 2023
2fc4bb8
Annotate rope.contrib.generate.create_generate(kind) with Literal
lieryan Jan 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
40 changes: 35 additions & 5 deletions rope/contrib/generate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from __future__ import annotations
from typing import Literal, Union, TYPE_CHECKING
from rope.base import (
change,
codeanalyze,
Expand All @@ -10,12 +12,40 @@
)
from rope.refactor import functionutils, importutils, sourceutils, suites


def create_generate(kind, project, resource, offset, goal_resource=None):
if TYPE_CHECKING:
from rope.base.pyobjects import PyObject
from rope.base.resources import Resource

GenerateKind = Literal[
"class",
"function",
"module",
"package",
"variable",
]

GenerateVal = Union[
"_Generate",
"GenerateClass",
"GenerateFunction",
"GenerateModule",
"GeneratePackage",
"GenerateVariable",
]
Copy link
Member

Choose a reason for hiding this comment

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

This is accurate annotation, but I think this would be promising too much to the caller.

create_generate() is a factory method for creating refactoring operation; so rather than documenting the exact types that it can return, we should just document what it is able to promise: create_generate() will return an object that matches the Refactoring interface.

rope currently doesn't have a formal interface for Refactoring class, but if it does, it would be an Protocol or abc that looks somewhat like this:

class Refactoring(typing.Protocol):
    def get_changes(self, *args, **kwargs) -> ChangeSet:
        ...

callers of create_generate() should only depend on it returning a class that matches the Refactoring interface, and they shouldn't rely on the concrete classes.

Copy link
Member

@lieryan lieryan Jan 4, 2023 •
edited

Choose a reason for hiding this comment

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

Alternatively, we can annotate create_generate() as returning a more specific GenerateRefactoring Protocol, which can further add a promise that get_changes() doesn't take any parameters and that there is also an additional get_location() method:

class GenerateRefactoring(typing.Protocol):
    def get_changes(self) -> ChangeSet:
        ...

    def get_location(self) -> Tuple[Resource, LineNumber (i.e. int)]:
        ...

this would be a more risky to promise in the API, since if we later change our mind and want to add a parameter to _Generate.get_changes(), that would need to be an API breaking change.



def create_generate(
kind: GenerateKind,
project: PyObject,
lieryan marked this conversation as resolved.
Show resolved Hide resolved
resource: Resource,
offset: int,
goal_resource: Resource = None,
) -> GenerateVal:
"""A factory for creating `Generate` objects

`kind` can be 'variable', 'function', 'class', 'module' or
'package'.
`kind`: 'variable', 'function', 'class', 'module' or 'package'.
edreamleo marked this conversation as resolved.
Show resolved Hide resolved

Used in https://github.com/python-rope/ropemode but not in Rope itself.

"""
d = {
Expand All @@ -25,7 +55,7 @@ def create_generate(kind, project, resource, offset, goal_resource=None):
"package": GeneratePackage,
"variable": GenerateVariable,
}
generate = d.get(kind)
generate = d.get(kind, _Generate)
lieryan marked this conversation as resolved.
Show resolved Hide resolved
return generate(project, resource, offset, goal_resource=goal_resource)


Expand Down