eclass: add ShadowedEclassPhase for multiple-eclass defined phases

src/pkgcheck/checks/eclass.py

@@ -129,6 +129,28 @@ def desc(self):
         return f"line {self.lineno}: redundant eclass inherit {self.line!r}, provided by {self.provider!r}"
 
 
+class ShadowedEclassPhase(results.VersionResult, results.Style):
+    """Ebuild does not define a phase when inheriting multiple eclasses
+    exporting that phase.
+
+    When inheriting multiple eclasses exporting the same phase, a custom
+    phase must usually be defined in the ebuild to call the phase exported
+    from each eclass.
+
+    Exceptions exist where the functionality isn't needed, but it should
+    be a deliberate choice, not an accidental omission.
+    """
+
+    def __init__(self, phase, providers, **kwargs):
+        super().__init__(**kwargs)
+        self.phase = phase
+        self.providers = tuple(providers)
+
+    @property
+    def desc(self):
+        return f"missing custom phase for {self.phase!r}, provided by eclasses: {', '.join(self.providers)}"
+
+
 class EclassUsageCheck(Check):
     """Scan packages for various eclass-related issues."""
 
@@ -142,6 +164,7 @@ class EclassUsageCheck(Check):
             EclassUserVariableUsage,
             MisplacedEclassVar,
             ProvidedEclassInherit,
+            ShadowedEclassPhase,
         }
     )
     required_addons = (addons.eclass.EclassAddon,)
@@ -254,41 +277,97 @@ def check_provided_eclasses(self, pkg, inherits: list[tuple[list[str], int]]):
         for provided, (eclass, lineno) in provided_eclasses.items():
             yield ProvidedEclassInherit(eclass, pkg=pkg, line=provided, lineno=lineno)
 
+    def check_exported_eclass_phase(
+        self, pkg: bash.ParseTree, inherits: list[tuple[list[str], int]]
+    ):
+        """Check for eclasses exporting the same phase where the ebuild does not
+        call such phases manually."""
+        latest_eapi = EAPI.known_eapis[sorted(EAPI.known_eapis)[-1]]
+        # all known build phases, e.g. src_configure
+        known_phases = list(latest_eapi.phases_rev)
+        exported_phases = {phase: [] for phase in known_phases}
+
+        # Create a dict of known phases => eclasses exporting them:
+        # we're interested in the cases where the RHS list has > 1 element.
+        for eclasses, _ in inherits:
+            for eclass in eclasses:
+                for func in self.eclass_cache[eclass].functions:
+                    phase = func.name.removeprefix(f"{eclass}_")
+                    if phase in known_phases:
+                        exported_phases[phase].append(eclass)
+
+        if not exported_phases.keys():
+            return
+
+        defined_phases = []
+        for node in bash.func_query.captures(pkg.tree.root_node).get("func", ()):
+            func_name = pkg.node_str(node.child_by_field_name("name"))
+            if func_name in known_phases:
+                defined_phases.append(func_name)
+
+        # XXX: Some eclasses vary their API based on the EAPI, usually to
+        # 'unexport' a phase. self.eclass_cache is generated once per eclass,
+        # not (eclass, EAPI), so it can't handle this. Ditto phases which are only
+        # exported if a variable is set. Blacklist such eclasses here as it
+        # doesn't happen often.
+        #
+        # We could maybe make this more finely-grained for phases we know
+        # are conditionally exported if this list is impacting coverage
+        # severely.
+        blacklisted_eclasses = ["pypi", "vala", "xdg"]
+        exported_phases = {
+            phase: set(eclass) - set(blacklisted_eclasses)
+            for phase, eclass in exported_phases.items()
+        }
+
+        # Strip out phases we already define (even if inside of those, we don't
+        # actually call exported phases from all eclasses inherited). Assume that
+        # a custom phase in the ebuild is intentionally omitting them.
+        missing_custom_phases = set(
+            phase for phase, eclass in exported_phases.items() if len(eclass) > 1
+        ) - set(defined_phases)
+
+        for missing in missing_custom_phases:
+            yield ShadowedEclassPhase(missing, sorted(exported_phases[missing]), pkg=pkg)
+
     def feed(self, pkg):
-        if pkg.inherit:
-            inherited: set[str] = set()
-            inherits: list[tuple[list[str], int]] = []
-            for node in bash.cmd_query.captures(pkg.tree.root_node).get("call", ()):
-                name = pkg.node_str(node.child_by_field_name("name"))
-                if name == "inherit":
-                    call = pkg.node_str(node)
-                    # filter out line continuations and conditional inherits
-                    if eclasses := [x for x in call.split()[1:] if x in pkg.inherit]:
-                        lineno, _colno = node.start_point
-                        if not inherited and eclasses[0] == pkg.inherit[0]:
-                            inherits.append((eclasses, lineno))
-
-                        for eclass in eclasses:
-                            if eclass not in inherited:
-                                inherited.add(eclass)
-                            else:
-                                yield DuplicateEclassInherit(
-                                    eclass, line=call, lineno=lineno + 1, pkg=pkg
-                                )
-
-            yield from self.check_provided_eclasses(pkg, inherits)
-            yield from self.check_user_variables(pkg, inherits)
-            # verify @PRE_INHERIT variable placement
-            yield from self.check_pre_inherits(pkg, inherits)
-            # verify @DEPRECATED variables or functions
-            yield from self.check_deprecated_variables(pkg, inherits)
-            yield from self.check_deprecated_functions(pkg, inherits)
-
-            for eclass in pkg.inherit.intersection(self.deprecated_eclasses):
-                replacement = self.deprecated_eclasses[eclass]
-                if not isinstance(replacement, str):
-                    replacement = None
-                yield DeprecatedEclass(eclass, replacement, pkg=pkg)
+        if not pkg.inherit:
+            return
+
+        inherited: set[str] = set()
+        inherits: list[tuple[list[str], int]] = []
+        for node in bash.cmd_query.captures(pkg.tree.root_node).get("call", ()):
+            name = pkg.node_str(node.child_by_field_name("name"))
+            if name == "inherit":
+                call = pkg.node_str(node)
+                # filter out line continuations and conditional inherits
+                if eclasses := [x for x in call.split()[1:] if x in pkg.inherit]:
+                    lineno, _colno = node.start_point
+                    if not inherited and eclasses[0] == pkg.inherit[0]:
+                        inherits.append((eclasses, lineno))
+
+                    for eclass in eclasses:
+                        if eclass not in inherited:
+                            inherited.add(eclass)
+                        else:
+                            yield DuplicateEclassInherit(
+                                eclass, line=call, lineno=lineno + 1, pkg=pkg
+                            )
+
+        yield from self.check_provided_eclasses(pkg, inherits)
+        yield from self.check_user_variables(pkg, inherits)
+        # verify @PRE_INHERIT variable placement
+        yield from self.check_pre_inherits(pkg, inherits)
+        # verify @DEPRECATED variables or functions
+        yield from self.check_deprecated_variables(pkg, inherits)
+        yield from self.check_deprecated_functions(pkg, inherits)
+        yield from self.check_exported_eclass_phase(pkg, inherits)
+
+        for eclass in pkg.inherit.intersection(self.deprecated_eclasses):
+            replacement = self.deprecated_eclasses[eclass]
+            if not isinstance(replacement, str):
+                replacement = None
+            yield DeprecatedEclass(eclass, replacement, pkg=pkg)
 
 
 class EclassVariableScope(VariableScope, results.EclassResult):

testdata/data/repos/eclass/EclassUsageCheck/ShadowedEclassPhase/expected.json

@@ -0,0 +1 @@
+{"__class__": "ShadowedEclassPhase", "category": "EclassUsageCheck", "package": "ShadowedEclassPhase", "version": "0", "phase": "src_prepare", "providers": ["another-src_prepare", "export-funcs-before-inherit"]}

testdata/data/repos/eclass/EclassUsageCheck/ShadowedEclassPhase/fix.patch

@@ -0,0 +1,12 @@
+--- eclass/EclassUsageCheck/ShadowedEclassPhase/ShadowedEclassPhase-0.ebuild
++++ fixed/EclassUsageCheck/ShadowedEclassPhase/ShadowedEclassPhase-0.ebuild
+@@ -4,3 +4,9 @@ DESCRIPTION="Ebuild"
+ HOMEPAGE="https://github.com/pkgcore/pkgcheck"
+ LICENSE="BSD"
+ SLOT="0"
++
++src_prepare() {
++	default
++	another-src_prepare_src_prepare
++	export-funcs-before-inherit_src_prepare
++}

testdata/repos/eclass/EclassUsageCheck/ShadowedEclassPhase/ShadowedEclassPhase-0.ebuild

@@ -0,0 +1,6 @@
+EAPI=7
+inherit another-src_prepare export-funcs-before-inherit
+DESCRIPTION="Ebuild"
+HOMEPAGE="https://github.com/pkgcore/pkgcheck"
+LICENSE="BSD"
+SLOT="0"