commit: 2cfc92ccab6ddfedd79ec16b8ebdde4eae8d2ad4 Author: Arthur Zamarin <arthurzam <AT> gentoo <DOT> org> AuthorDate: Tue Aug 1 18:37:33 2023 +0000 Commit: Arthur Zamarin <arthurzam <AT> gentoo <DOT> org> CommitDate: Tue Aug 1 19:40:22 2023 +0000 URL: https://gitweb.gentoo.org/proj/pkgcore/pkgcheck.git/commit/?id=2cfc92cc
VariableScopeCheck: also check for wrong usage on global scope Resolves: https://github.com/pkgcore/pkgcheck/issues/606 Signed-off-by: Arthur Zamarin <arthurzam <AT> gentoo.org> src/pkgcheck/checks/codingstyle.py | 44 +++++++++++++++++++--- .../EbuildVariableScope/expected.json | 3 +- .../EbuildVariableScope/fix.patch | 6 ++- .../EbuildVariableScope-0.ebuild | 2 + 4 files changed, 47 insertions(+), 8 deletions(-) diff --git a/src/pkgcheck/checks/codingstyle.py b/src/pkgcheck/checks/codingstyle.py index 1ab82ad9..a05831cd 100644 --- a/src/pkgcheck/checks/codingstyle.py +++ b/src/pkgcheck/checks/codingstyle.py @@ -72,7 +72,7 @@ class BadCommandsCheck(Check): for node, _ in bash.cmd_query.captures(func_node): call = pkg.node_str(node) name = pkg.node_str(node.child_by_field_name("name")) - lineno, colno = node.start_point + lineno, _colno = node.start_point if name in pkg.eapi.bash_cmds_banned: yield BannedEapiCommand( name, line=call, lineno=lineno + 1, eapi=pkg.eapi, pkg=pkg @@ -942,7 +942,7 @@ class ReadonlyVariableCheck(Check): name = pkg.node_str(node.child_by_field_name("name")) if name in self.readonly_vars: call = pkg.node_str(node) - lineno, colno = node.start_point + lineno, _colno = node.start_point yield ReadonlyVariable(name, line=call, lineno=lineno + 1, pkg=pkg) @@ -969,7 +969,7 @@ class VariableScopeCheck(Check): """Scan ebuilds for variables that are only allowed in certain scopes.""" _source = sources.EbuildParseRepoSource - known_results = frozenset([EbuildVariableScope]) + known_results = frozenset({EbuildVariableScope}) # see https://projects.gentoo.org/pms/7/pms.html#x1-10900011.1 variable_map = ImmutableDict( @@ -987,8 +987,8 @@ class VariableScopeCheck(Check): "SYSROOT": ("src_", "pkg_setup"), "ESYSROOT": ("src_", "pkg_setup"), "BROOT": ("src_", "pkg_setup"), - "D": ("src_install", "pkg_preinst"), - "ED": ("src_install", "pkg_preinst"), + "D": ("src_install", "pkg_preinst", "pkg_postinst"), + "ED": ("src_install", "pkg_preinst", "pkg_postinst"), "DESTTREE": "src_install", "INSDESTTREE": "src_install", "MERGE_TYPE": "pkg_", @@ -997,6 +997,27 @@ class VariableScopeCheck(Check): } ) + not_global_scope = frozenset( + { + "A", + "AA", + "BROOT", + "D", + "DESTTREE", + "ECLASSDIR", + "ED", + "EROOT", + "ESYSROOT", + "INSDESTTREE", + "MERGE_TYPE", + "PORTDIR", + "REPLACED_BY_VERSION", + "REPLACING_VERSIONS", + "ROOT", + "SYSROOT", + } + ) + # mapping of bad variables for each EAPI phase function scoped_vars = {} for eapi in EAPI.known_eapis.values(): @@ -1014,11 +1035,22 @@ class VariableScopeCheck(Check): for var_node, _ in bash.var_query.captures(func_node): var_name = pkg.node_str(var_node) if var_name in variables: - lineno, colno = var_node.start_point + lineno, _colno = var_node.start_point usage[var_name].add(lineno + 1) for var, lines in sorted(usage.items()): yield EbuildVariableScope(var, func_name, lines=sorted(lines), pkg=pkg) + global_usage = defaultdict(set) + for global_node in pkg.tree.root_node.children: + if global_node.type not in ("function_definition", "ERROR"): + for var_node, _ in bash.var_query.captures(global_node): + var_name = pkg.node_str(var_node) + if var_name in self.not_global_scope: + lineno, _colno = var_node.start_point + global_usage[var_name].add(lineno + 1) + for var, lines in sorted(global_usage.items()): + yield EbuildVariableScope(var, "global scope", lines=sorted(lines), pkg=pkg) + class RedundantDodir(results.LineResult, results.Style): """Ebuild using a redundant dodir call.""" diff --git a/testdata/data/repos/standalone/VariableScopeCheck/EbuildVariableScope/expected.json b/testdata/data/repos/standalone/VariableScopeCheck/EbuildVariableScope/expected.json index 48c53690..06f9d9fb 100644 --- a/testdata/data/repos/standalone/VariableScopeCheck/EbuildVariableScope/expected.json +++ b/testdata/data/repos/standalone/VariableScopeCheck/EbuildVariableScope/expected.json @@ -1 +1,2 @@ -{"__class__": "EbuildVariableScope", "category": "VariableScopeCheck", "package": "EbuildVariableScope", "version": "0", "variable": "EROOT", "func": "src_configure", "lines": [10]} +{"__class__": "EbuildVariableScope", "category": "VariableScopeCheck", "package": "EbuildVariableScope", "version": "0", "lines": [12], "variable": "EROOT", "func": "src_configure"} +{"__class__": "EbuildVariableScope", "category": "VariableScopeCheck", "package": "EbuildVariableScope", "version": "0", "lines": [8], "variable": "ROOT", "func": "global scope"} diff --git a/testdata/data/repos/standalone/VariableScopeCheck/EbuildVariableScope/fix.patch b/testdata/data/repos/standalone/VariableScopeCheck/EbuildVariableScope/fix.patch index be4a05bd..f19a76d8 100644 --- a/testdata/data/repos/standalone/VariableScopeCheck/EbuildVariableScope/fix.patch +++ b/testdata/data/repos/standalone/VariableScopeCheck/EbuildVariableScope/fix.patch @@ -1,8 +1,12 @@ diff -Naur standalone/VariableScopeCheck/EbuildVariableScope/EbuildVariableScope-0.ebuild fixed/VariableScopeCheck/EbuildVariableScope/EbuildVariableScope-0.ebuild --- standalone/VariableScopeCheck/EbuildVariableScope/EbuildVariableScope-0.ebuild 2021-03-17 01:20:05.423678951 -0600 +++ fixed/VariableScopeCheck/EbuildVariableScope/EbuildVariableScope-0.ebuild 2021-03-17 01:21:57.421132212 -0600 -@@ -7,7 +7,7 @@ +@@ -5,11 +5,9 @@ HOMEPAGE="https://github.com/pkgcore/pkgcheck" + SLOT="0" + LICENSE="BSD" +-DOC_CONTENTS="Hello ${ROOT}" +- src_configure() { # EROOT isn't allowed in src_* phases - econf --sysconfdir="${EROOT}/etc/${PN}" diff --git a/testdata/repos/standalone/VariableScopeCheck/EbuildVariableScope/EbuildVariableScope-0.ebuild b/testdata/repos/standalone/VariableScopeCheck/EbuildVariableScope/EbuildVariableScope-0.ebuild index 28cc609e..a27c4122 100644 --- a/testdata/repos/standalone/VariableScopeCheck/EbuildVariableScope/EbuildVariableScope-0.ebuild +++ b/testdata/repos/standalone/VariableScopeCheck/EbuildVariableScope/EbuildVariableScope-0.ebuild @@ -5,6 +5,8 @@ HOMEPAGE="https://github.com/pkgcore/pkgcheck" SLOT="0" LICENSE="BSD" +DOC_CONTENTS="Hello ${ROOT}" + src_configure() { # EROOT isn't allowed in src_* phases econf --sysconfdir="${EROOT}/etc/${PN}"
