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}"

Reply via email to