diff --git a/src/pkgcheck/checks/eclass.py b/src/pkgcheck/checks/eclass.py index 5a6b6430e..1e1b70e55 100644 --- a/src/pkgcheck/checks/eclass.py +++ b/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): diff --git a/testdata/data/repos/eclass/EclassUsageCheck/ShadowedEclassPhase/expected.json b/testdata/data/repos/eclass/EclassUsageCheck/ShadowedEclassPhase/expected.json new file mode 100644 index 000000000..71892994d --- /dev/null +++ b/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"]} diff --git a/testdata/data/repos/eclass/EclassUsageCheck/ShadowedEclassPhase/fix.patch b/testdata/data/repos/eclass/EclassUsageCheck/ShadowedEclassPhase/fix.patch new file mode 100644 index 000000000..8e5f65e04 --- /dev/null +++ b/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 ++} diff --git a/testdata/repos/eclass/EclassUsageCheck/ShadowedEclassPhase/ShadowedEclassPhase-0.ebuild b/testdata/repos/eclass/EclassUsageCheck/ShadowedEclassPhase/ShadowedEclassPhase-0.ebuild new file mode 100644 index 000000000..ffdc64167 --- /dev/null +++ b/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"