refactor: Rework "exported" and "public" logic

We observed that Griffe's internals/API were a bit confusing, so decided to refactor them.

Griffe has a concept of "exported member". A **module member** is considered:

- **explicitly exported** when its name appears in the module's `__all__`
- **implicitly exported** when the module's `__all__` is undefined, the member is available at runtime (for example not guarded behind `if TYPE_CHECKING`), and the member is an alias (defined elsewhere), *except* if it's a submodule, *unless* it was also imported in the current module

A **class member** is considered **implicitly exported**, the same way as module members are considered implicitly exported. It is never considered explicitly exported because we don't/can't use `__all__` in class bodies.

This "exported" marker is then used in a few places, 3 of of them being of interest to us here.

- if `__all__` is defined in the wildcard imported module, add all objects referenced in the imported module's `__all__` to the importing module
- else, add all objects that are implicitly exported by the imported module to the importing module

There is a first issue here: members starting with `_` are considered implicitly exported. This is wrong: actual wildcard imports will not import these objects at runtime.

Between the same old and same new module, if a member disappeared, and it was either a submodule, or it was exported (explicitly or implicitly), we consider its disappearance a breaking change. Again, it's wrong here to consider that a private object's disappearance makes a breaking change. This issue wasn't spotted before because the code that checks breaking changes has an earlier condition on the object names: it skips private objects anyway.

Griffe objects have an `is_public()` method that tell if an object is public or not. An object is:

- **strictly public** if:
  - its `public` attribute is true
  - or it is explicitly exported
- **public** if:
  - it is strictly public
  - or it's not private (name not starting with `_`) and it's not imported

Griffe gained this method later on, to be used by mkdocstrings-python.

---

"Exported" is usually used to say "public" (we export an object to make it public), but in Griffe it's rather used to say "the object is exposed in regard to the wildcard import runtime mechanism". So we renamed "exported" to "wildcard exposed".

Then we fixed the issue mentioned above about private members being considered implicitly exposed. And since wildcard imports are not configurable, we dropped the distinction between explicitly and implicitly exposed. An object is either exposed, or it's not.

Finally the code that checks breaking changes now uses the "public" API instead of the "exposed" one.

Issue-281: #281

src/griffe/dataclasses.py

@@ -411,27 +411,14 @@ def has_docstrings(self) -> bool:
             return True
         return any(member.has_docstrings for member in self.members.values())
 
-    def member_is_exported(self, member: Object | Alias, *, explicitely: bool = True) -> bool:
-        """Whether a member of this object is "exported".
-
-        By exported, we mean that the object is included in the `__all__` attribute
-        of its parent module or class. When `__all__` is not defined,
-        we consider the member to be *implicitely* exported,
-        unless it's a module and it was not imported,
-        and unless it's not defined at runtime.
-
-        Parameters:
-            member: The member to verify.
-            explicitely: Whether to only return True when `__all__` is defined.
-
-        Returns:
-            True or False.
-        """
-        if not member.runtime:
-            return False
-        if self.exports is None:
-            return not explicitely and (member.is_alias or not member.is_module or member.name in self.imports)
-        return member.name in self.exports
+    def member_is_exported(self, member: Object | Alias, *, explicitely: bool = True) -> bool:  # noqa: ARG002
+        """Deprecated. Use [`member.is_exported`][griffe.dataclasses.Object.is_exported] instead."""
+        warnings.warn(
+            "Method `member_is_exported` is deprecated. Use `member.is_exported` instead.",
+            DeprecationWarning,
+            stacklevel=2,
+        )
+        return member.is_exported
 
     def is_kind(self, kind: str | Kind | set[str | Kind]) -> bool:
         """Tell if this object is of the given kind.
@@ -1029,23 +1016,14 @@ def aliases(self) -> dict[str, Alias]:
         """The aliases pointing to this object."""
         return self.final_target.aliases
 
-    def member_is_exported(self, member: Object | Alias, *, explicitely: bool = True) -> bool:
-        """Whether a member of this object is "exported".
-
-        By exported, we mean that the object is included in the `__all__` attribute
-        of its parent module or class. When `__all__` is not defined,
-        we consider the member to be *implicitely* exported,
-        unless it's a module and it was not imported,
-        and unless it's not defined at runtime.
-
-        Parameters:
-            member: The member to verify.
-            explicitely: Whether to only return True when `__all__` is defined.
-
-        Returns:
-            True or False.
-        """
-        return self.final_target.member_is_exported(member, explicitely=explicitely)
+    def member_is_exported(self, member: Object | Alias, *, explicitely: bool = True) -> bool:  # noqa: ARG002
+        """Deprecated. Use [`member.is_exported`][griffe.dataclasses.Alias.is_exported] instead."""
+        warnings.warn(
+            "Method `member_is_exported` is deprecated. Use `member.is_exported` instead.",
+            DeprecationWarning,
+            stacklevel=2,
+        )
+        return member.is_exported
 
     def is_kind(self, kind: str | Kind | set[str | Kind]) -> bool:
         """Tell if this object is of the given kind.

src/griffe/diff.py

@@ -3,6 +3,7 @@
 from __future__ import annotations
 
 import contextlib
+import warnings
 from pathlib import Path
 from typing import TYPE_CHECKING, Any, Iterable, Iterator
 
@@ -339,13 +340,12 @@ def _class_incompatibilities(
     old_class: Class,
     new_class: Class,
     *,
-    ignore_private: bool = True,
     seen_paths: set[str],
 ) -> Iterable[Breakage]:
     yield from ()
     if new_class.bases != old_class.bases and len(new_class.bases) < len(old_class.bases):
         yield ClassRemovedBaseBreakage(new_class, old_class.bases, new_class.bases)
-    yield from _member_incompatibilities(old_class, new_class, ignore_private=ignore_private, seen_paths=seen_paths)
+    yield from _member_incompatibilities(old_class, new_class, seen_paths=seen_paths)
 
 
 # TODO: Check decorators? Maybe resolved by extensions and/or dynamic analysis.
@@ -438,51 +438,43 @@ def _alias_incompatibilities(
     old_obj: Object | Alias,
     new_obj: Object | Alias,
     *,
-    ignore_private: bool,
     seen_paths: set[str],
 ) -> Iterable[Breakage]:
-    if not ignore_private:
-        return
     try:
         old_member = old_obj.target if old_obj.is_alias else old_obj  # type: ignore[union-attr]
         new_member = new_obj.target if new_obj.is_alias else new_obj  # type: ignore[union-attr]
     except AliasResolutionError:
         logger.debug(f"API check: {old_obj.path} | {new_obj.path}: skip alias with unknown target")
         return
 
-    yield from _type_based_yield(old_member, new_member, ignore_private=ignore_private, seen_paths=seen_paths)
+    yield from _type_based_yield(old_member, new_member, seen_paths=seen_paths)
 
 
 def _member_incompatibilities(
     old_obj: Object | Alias,
     new_obj: Object | Alias,
     *,
-    ignore_private: bool = True,
     seen_paths: set[str] | None = None,
 ) -> Iterator[Breakage]:
     seen_paths = set() if seen_paths is None else seen_paths
     for name, old_member in old_obj.all_members.items():
-        if ignore_private and name.startswith("_"):
-            logger.debug(f"API check: {old_obj.path}.{name}: skip private object")
+        if not old_member.is_public:
+            logger.debug(f"API check: {old_obj.path}.{name}: skip non-public object")
             continue
-
         logger.debug(f"API check: {old_obj.path}.{name}")
         try:
             new_member = new_obj.all_members[name]
         except KeyError:
-            is_module = not old_member.is_alias and old_member.is_module
-            if is_module or old_member.is_exported(explicitely=False):
+            if (not old_member.is_alias and old_member.is_module) or old_member.is_public:
                 yield ObjectRemovedBreakage(old_member, old_member, None)  # type: ignore[arg-type]
-            continue
-
-        yield from _type_based_yield(old_member, new_member, ignore_private=ignore_private, seen_paths=seen_paths)
+        else:
+            yield from _type_based_yield(old_member, new_member, seen_paths=seen_paths)
 
 
 def _type_based_yield(
     old_member: Object | Alias,
     new_member: Object | Alias,
     *,
-    ignore_private: bool,
     seen_paths: set[str],
 ) -> Iterator[Breakage]:
     if old_member.path in seen_paths:
@@ -494,7 +486,6 @@ def _type_based_yield(
         yield from _alias_incompatibilities(
             old_member,
             new_member,
-            ignore_private=ignore_private,
             seen_paths=seen_paths,
         )
     elif new_member.kind != old_member.kind:
@@ -503,14 +494,12 @@ def _type_based_yield(
         yield from _member_incompatibilities(
             old_member,
             new_member,
-            ignore_private=ignore_private,
             seen_paths=seen_paths,
         )
     elif old_member.is_class:
         yield from _class_incompatibilities(
             old_member,  # type: ignore[arg-type]
             new_member,  # type: ignore[arg-type]
-            ignore_private=ignore_private,
             seen_paths=seen_paths,
         )
     elif old_member.is_function:
@@ -540,11 +529,14 @@ def _returns_are_compatible(old_function: Function, new_function: Function) -> b
     return True
 
 
+_sentinel = object()
+
+
 def find_breaking_changes(
     old_obj: Object | Alias,
     new_obj: Object | Alias,
     *,
-    ignore_private: bool = True,
+    ignore_private: bool = _sentinel,  # type: ignore[assignment]
 ) -> Iterator[Breakage]:
     """Find breaking changes between two versions of the same API.
 
@@ -565,7 +557,13 @@ def find_breaking_changes(
         >>> for breakage in griffe.find_breaking_changes(old, new)
         ...     print(breakage.explain(style=style), file=sys.stderr)
     """
-    yield from _member_incompatibilities(old_obj, new_obj, ignore_private=ignore_private)
+    if ignore_private is not _sentinel:
+        warnings.warn(
+            "The `ignore_private` parameter is deprecated and will be removed in a future version.",
+            DeprecationWarning,
+            stacklevel=2,
+        )
+    yield from _member_incompatibilities(old_obj, new_obj)
 
 
 __all__ = [

src/griffe/loader.py

@@ -464,7 +464,7 @@ def resolve_module_aliases(
             if member.is_alias:
                 if member.wildcard or member.resolved:  # type: ignore[union-attr]
                     continue
-                if not implicit and not member.is_explicitely_exported:
+                if not implicit and not member.is_exported:
                     continue
 
                 # Try resolving the alias. If it fails, check if it is because it comes
@@ -717,11 +717,10 @@ def _get_or_create_parent_module(
 
     def _expand_wildcard(self, wildcard_obj: Alias) -> list[tuple[Object | Alias, int | None, int | None]]:
         module = self.modules_collection.get_member(wildcard_obj.wildcard)  # type: ignore[arg-type]  # we know it's a wildcard
-        explicitely = "__all__" in module.members
         return [
             (imported_member, wildcard_obj.alias_lineno, wildcard_obj.alias_endlineno)
             for imported_member in module.members.values()
-            if imported_member.is_exported(explicitely=explicitely)
+            if imported_member.is_wildcard_exposed
         ]
 
 

src/griffe/mixins.py

@@ -3,6 +3,7 @@
 from __future__ import annotations
 
 import json
+import warnings
 from contextlib import suppress
 from typing import TYPE_CHECKING, Any, Sequence, TypeVar
 
@@ -293,33 +294,63 @@ def attributes(self) -> dict[str, Attribute]:
         """
         return {name: member for name, member in self.all_members.items() if member.kind is Kind.ATTRIBUTE}  # type: ignore[misc]
 
-    def is_exported(self, *, explicitely: bool = True) -> bool:
-        """Tell if this object/alias is implicitely exported by its parent.
-
-        Parameters:
-            explicitely: Whether to only return True when `__all__` is defined.
+    @property
+    def has_private_name(self) -> bool:
+        """Whether this object/alias has a private name."""
+        return self.name.startswith("_")  # type: ignore[attr-defined]
 
-        Returns:
-            True or False.
-        """
-        return self.parent.member_is_exported(self, explicitely=explicitely)  # type: ignore[attr-defined]
+    @property
+    def is_exported(self) -> bool:
+        """Whether this object/alias is exported (listed in `__all__`)."""
+        result = self.parent.is_module and bool(self.parent.exports and self.name in self.parent.exports)  # type: ignore[attr-defined]
+        return _True if result else _False  # type: ignore[return-value]
 
     @property
     def is_explicitely_exported(self) -> bool:
-        """Whether this object/alias is explicitely exported by its parent."""
-        return self.is_exported(explicitely=True)
+        """Deprecated. Use the [`is_exported`][griffe.mixins.ObjectAliasMixin.is_exported] property instead."""
+        warnings.warn(
+            "The `is_explicitely_exported` property is deprecated. Use `is_exported` instead.",
+            DeprecationWarning,
+            stacklevel=2,
+        )
+        return self.is_exported
 
     @property
     def is_implicitely_exported(self) -> bool:
-        """Whether this object/alias is implicitely exported by its parent."""
-        return self.parent.exports is None  # type: ignore[attr-defined]
-
-    def is_public(
-        self,
-        *,
-        strict: bool = False,
-        check_name: bool = True,
-    ) -> bool:
+        """Deprecated. Use the [`is_exported`][griffe.mixins.ObjectAliasMixin.is_exported] property instead."""
+        warnings.warn(
+            "The `is_implicitely_exported` property is deprecated. Use `is_exported` instead.",
+            DeprecationWarning,
+            stacklevel=2,
+        )
+        return self.is_exported
+
+    @property
+    def is_wildcard_exposed(self) -> bool:
+        """Whether this object/alias is exposed to wildcard imports.
+
+        To be exposed to wildcard imports, an object/alias must:
+
+        - be available at runtime
+        - have a module as parent
+        - be listed in `__all__` if `__all__` is defined
+        - or not be private (having a name starting with an underscore)
+
+        Special case for Griffe trees: a submodule is only exposed if its parent imports it.
+
+        Returns:
+            True or False.
+        """
+        if not self.runtime or not self.parent.is_module:  # type: ignore[attr-defined]
+            return False
+        if self.parent.exports is not None:  # type: ignore[attr-defined]
+            return self.name in self.parent.exports  # type: ignore[attr-defined]
+        if self.has_private_name:
+            return False
+        return self.is_alias or not self.is_module or self.name in self.parent.imports  # type: ignore[attr-defined]
+
+    @property
+    def is_public(self) -> bool:
         """Whether this object is considered public.
 
         In modules, developers can mark objects as public thanks to the `__all__` variable.
@@ -328,25 +359,47 @@ def is_public(
         Therefore, to decide whether an object is public, we follow this algorithm:
 
         - If the object's `public` attribute is set (boolean), return its value.
-        - In strict mode, the object is public only if it is explicitely exported (listed in `__all__`).
-            Strict mode should only be used for module members.
-        - Otherwise, if name checks are enabled, the object is private if its name starts with an underscore.
-        - Otherwise, if the object is an alias, and is neither inherited from a base class,
-            nor a member of a parent alias, it is not public.
+        - If the object is exposed to wildcard imports, it is public.
+        - If the object has a private name, it is private.
+        - If the object was imported from another module, it is private.
         - Otherwise, the object is public.
         """
+        # TODO: Return regular True/False values in next version.
         if self.public is not None:  # type: ignore[attr-defined]
-            return self.public  # type: ignore[attr-defined]
-        if self.is_explicitely_exported:
-            return True
-        if strict:
-            return False
-        if check_name and self.name.startswith("_"):  # type: ignore[attr-defined]
-            return False
-        if self.is_alias and not (self.inherited or self.parent.is_alias):  # type: ignore[attr-defined]
-            return False
-        return True
-
+            return _True if self.public else _False  # type: ignore[return-value,attr-defined]
+        if self.is_wildcard_exposed:
+            return _True  # type: ignore[return-value]
+        if self.has_private_name:
+            return _False  # type: ignore[return-value]
+        # The following condition effectively filters out imported objects.
+        # TODO: In a future version, we will support two conventions regarding imports:
+        # - `from a import x as x` marks `x` as public.
+        # - `from a import *` marks all wildcard imported objects as public.
+        if self.is_alias and not (self.inherited or (self.parent and self.parent.is_alias)):  # type: ignore[attr-defined]
+            return _False  # type: ignore[return-value]
+        return _True  # type: ignore[return-value]
+
+
+# This is used to allow the `is_public` property to be "callable",
+# for backward compatibility with the previous implementation.
+class _Bool:
+    def __init__(self, value: bool) -> None:  # noqa: FBT001
+        self.value = value
+
+    def __bool__(self) -> bool:
+        return self.value
+
+    def __call__(self, *args: Any, **kwargs: Any) -> bool:  # noqa: ARG002
+        warnings.warn(
+            "This method is now a property and should be accessed as such (without parentheses).",
+            DeprecationWarning,
+            stacklevel=2,
+        )
+        return self.value
+
+
+_True = _Bool(True)  # noqa: FBT003
+_False = _Bool(False)  # noqa: FBT003
 
 __all__ = [
     "DelMembersMixin",