Issue warnings for some deprecated attributes of modules

Author: EliahKagan

git/__init__.py

@@ -88,7 +88,10 @@
 
 __version__ = "git"
 
-from typing import List, Optional, Sequence, TYPE_CHECKING, Tuple, Union
+from typing import Any, List, Optional, Sequence, TYPE_CHECKING, Tuple, Union
+
+if TYPE_CHECKING:
+    from types import ModuleType
 
 from gitdb.util import to_hex_sha
 
@@ -144,11 +147,6 @@
         SymbolicReference,
         Tag,
         TagReference,
-        head,  # noqa: F401  # Nonpublic. May disappear! Use git.refs.head.
-        log,  # noqa: F401  # Nonpublic. May disappear! Use git.refs.log.
-        reference,  # noqa: F401  # Nonpublic. May disappear! Use git.refs.reference.
-        symbolic,  # noqa: F401  # Nonpublic. May disappear! Use git.refs.symbolic.
-        tag,  # noqa: F401  # Nonpublic. May disappear! Use git.refs.tag.
     )
     from git.diff import (  # @NoMove
         INDEX,
@@ -169,21 +167,6 @@
         IndexEntry,
         IndexFile,
         StageType,
-        base,  # noqa: F401  # Nonpublic. May disappear! Use git.index.base.
-        fun,  # noqa: F401  # Nonpublic. May disappear! Use git.index.fun.
-        typ,  # noqa: F401  # Nonpublic. May disappear! Use git.index.typ.
-        #
-        # NOTE: The expression `git.util` evaluates to git.index.util, and the import
-        # `from git import util` imports git.index.util, NOT git.util. It may not be
-        # feasible to change this until the next major version, to avoid breaking code
-        # inadvertently relying on it. If git.index.util really is what you want, use or
-        # import from that name, to avoid confusion. To use the "real" git.util module,
-        # write `from git.util import ...`, or access it as `sys.modules["git.util"]`.
-        # (This differs from other historical indirect-submodule imports that are
-        # unambiguously nonpublic and are subject to immediate removal. Here, the public
-        # git.util module, even though different, makes it less discoverable that the
-        # expression `git.util` refers to a non-public attribute of the git module.)
-        util,  # noqa: F401
     )
     from git.util import (  # @NoMove
         Actor,
@@ -196,7 +179,72 @@
 except GitError as _exc:
     raise ImportError("%s: %s" % (_exc.__class__.__name__, _exc)) from _exc
 
+
+# NOTE: The expression `git.util` evaluates to git.index.util and `from git import util`
+# imports git.index.util, NOT git.util. It may not be feasible to change this until the
+# next major version, to avoid breaking code inadvertently relying on it.
+#
+# - If git.index.util *is* what you want, use or import from that, to avoid confusion.
+#
+# - To use the "real" git.util module, write `from git.util import ...`, or if necessary
+#   access it as `sys.modules["git.util"]`.
+#
+# (This differs from other indirect-submodule imports that are unambiguously non-public
+# and subject to immediate removal. Here, the public git.util module, though different,
+# makes less discoverable that the expression `git.util` refers to a non-public
+# attribute of the git module.)
+#
+# This had come about by a wildcard import. Now that all intended imports are explicit,
+# the intuitive but potentially incompatible binding occurs due to the usual rules for
+# Python submodule bindings. So for now we delete that and let __getattr__ handle it.
+#
+del util  # type: ignore[name-defined]  # noqa: F821
+
+
+def _warned_import(message: str, fullname: str) -> "ModuleType":
+    import importlib
+    import warnings
+
+    warnings.warn(message, DeprecationWarning, stacklevel=3)
+    return importlib.import_module(fullname)
+
+
+def _getattr(name: str) -> Any:
+    # TODO: If __version__ is made dynamic and lazily fetched, put that case right here.
+
+    if name == "util":
+        return _warned_import(
+            "The expression `git.util` and the import `from git import util` actually "
+            "reference git.index.util, and not the git.util module accessed in "
+            '`from git.util import XYZ` or `sys.modules["git.util"]`. This potentially '
+            "confusing behavior is currently preserved for compatibility, but may be "
+            "changed in the future and should not be relied on.",
+            fullname="git.index.util",
+        )
+
+    for names, prefix in (
+        ({"head", "log", "reference", "symbolic", "tag"}, "git.refs"),
+        ({"base", "fun", "typ"}, "git.index"),
+    ):
+        if name not in names:
+            continue
+
+        fullname = f"{prefix}.{name}"
+
+        return _warned_import(
+            f"{__name__}.{name} is a private alias of {fullname} and subject to "
+            f"immediate removal. Use {fullname} instead.",
+            fullname=fullname,
+        )
+
+    raise AttributeError(f"module {__name__!r} has no attribute {name!r}")
+
+
+if not TYPE_CHECKING:  # Preserve static checking for undefined/misspelled attributes.
+    __getattr__ = _getattr
+
 # { Initialize git executable path
+
 GIT_OK = None
 
 
@@ -232,12 +280,9 @@ def refresh(path: Optional[PathLike] = None) -> None:
     GIT_OK = True
 
 
-# } END initialize git executable path
-
-
-#################
 try:
     refresh()
 except Exception as _exc:
     raise ImportError("Failed to initialize: {0}".format(_exc)) from _exc
-#################
+
+# } END initialize git executable path

git/compat.py

@@ -23,7 +23,9 @@
     AnyStr,
     Dict,  # noqa: F401
     IO,  # noqa: F401
+    List,
     Optional,
+    TYPE_CHECKING,
     Tuple,  # noqa: F401
     Type,  # noqa: F401
     Union,
@@ -33,7 +35,39 @@
 # ---------------------------------------------------------------------------
 
 
-is_win = os.name == "nt"
+_deprecated_platform_aliases = {
+    "is_win": os.name == "nt",
+    "is_posix": os.name == "posix",
+    "is_darwin": sys.platform == "darwin",
+}
+
+
+def _getattr(name: str) -> Any:
+    try:
+        value = _deprecated_platform_aliases[name]
+    except KeyError:
+        raise AttributeError(f"module {__name__!r} has no attribute {name!r}") from None
+
+    import warnings
+
+    warnings.warn(
+        f"{__name__}.{name} and other is_<platform> aliases are deprecated. "
+        "Write the desired os.name or sys.platform check explicitly instead.",
+        DeprecationWarning,
+        stacklevel=2,
+    )
+    return value
+
+
+if not TYPE_CHECKING:  # Preserve static checking for undefined/misspelled attributes.
+    __getattr__ = _getattr
+
+
+def __dir__() -> List[str]:
+    return [*globals(), *_deprecated_platform_aliases]
+
+
+is_win: bool
 """Deprecated alias for ``os.name == "nt"`` to check for native Windows.
 
 This is deprecated because it is clearer to write out :attr:`os.name` or
@@ -45,7 +79,7 @@
     Cygwin, use ``sys.platform == "cygwin"``.
 """
 
-is_posix = os.name == "posix"
+is_posix: bool
 """Deprecated alias for ``os.name == "posix"`` to check for Unix-like ("POSIX") systems.
 
 This is deprecated because it clearer to write out :attr:`os.name` or
@@ -58,7 +92,7 @@
     (Darwin).
 """
 
-is_darwin = sys.platform == "darwin"
+is_darwin: bool
 """Deprecated alias for ``sys.platform == "darwin"`` to check for macOS (Darwin).
 
 This is deprecated because it clearer to write out :attr:`os.name` or

git/types.py

@@ -7,11 +7,13 @@
     Any,
     Callable,
     Dict,
+    List,
     NoReturn,
     Optional,
     Sequence as Sequence,
     Tuple,
     TYPE_CHECKING,
+    Type,
     TypeVar,
     Union,
 )
@@ -127,21 +129,40 @@
 https://git-scm.com/docs/gitglossary#def_object_type
 """
 
-Lit_commit_ish = Literal["commit", "tag"]
-"""Deprecated. Type of literal strings identifying sometimes-commitish git object types.
+Lit_commit_ish: Type[Literal["commit", "tag"]]
+"""Deprecated. Type of literal strings identifying typically-commitish git object types.
 
 Prior to a bugfix, this type had been defined more broadly. Any usage is in practice
-ambiguous and likely to be incorrect. Instead of this type:
+ambiguous and likely to be incorrect. This type has therefore been made a static type
+error to appear in annotations. It is preserved, with a deprecated status, to avoid
+introducing runtime errors in code that refers to it, but it should not be used.
+
+Instead of this type:
 
 * For the type of the string literals associated with :class:`Commit_ish`, use
   ``Literal["commit", "tag"]`` or create a new type alias for it. That is equivalent to
-  this type as currently defined.
+  this type as currently defined (but usable in statically checked type annotations).
 
 * For the type of all four string literals associated with :class:`AnyGitObject`, use
   :class:`GitObjectTypeString`. That is equivalent to the old definition of this type
-  prior to the bugfix.
+  prior to the bugfix (and is also usable in statically checked type annotations).
 """
 
+
+def _getattr(name: str) -> Any:
+    if name == "Lit_commit_ish":
+        return Literal["commit", "tag"]
+    raise AttributeError(f"module {__name__!r} has no attribute {name!r}")
+
+
+if not TYPE_CHECKING:  # Preserve static checking for undefined/misspelled attributes.
+    __getattr__ = _getattr
+
+
+def __dir__() -> List[str]:
+    return [*globals(), "Lit_commit_ish"]
+
+
 # Config_levels ---------------------------------------------------------
 
 Lit_config_levels = Literal["system", "global", "user", "repository"]
@@ -188,12 +209,12 @@ def assert_never(inp: NoReturn, raise_error: bool = True, exc: Union[Exception,
 
     :param inp:
         If all members are handled, the argument for `inp` will have the
-        :class:`~typing.Never`/:class:`~typing.NoReturn` type. Otherwise, the type will
-        mismatch and cause a mypy error.
+        :class:`~typing.Never`/:class:`~typing.NoReturn` type.
+        Otherwise, the type will mismatch and cause a mypy error.
 
     :param raise_error:
-        If ``True``, will also raise :exc:`ValueError` with a general "unhandled
-        literal" message, or the exception object passed as `exc`.
+        If ``True``, will also raise :exc:`ValueError` with a general
+        "unhandled literal" message, or the exception object passed as `exc`.
 
     :param exc:
         It not ``None``, this should be an already-constructed exception object, to be