From e2f647b08b30265c14dbc3cc5ece8ff29c87d38a Mon Sep 17 00:00:00 2001
From: Ulf Magnusson <Ulf.Magnusson@nordicsemi.no>
Date: Mon, 27 Jan 2020 06:31:03 +0100
Subject: [PATCH] scripts: kconfig: lint.py: Add check for missing CONFIG_
 prefix

Add a new --check-missing-config-prefix check that looks for references
like

    #if MACRO
    #if(n)def MACRO
    defined(MACRO)
    IS_ENABLED(MACRO)

where MACRO is the name of a defined Kconfig symbol but doesn't have a
CONFIG_ prefix. Could be a typo.

Skip MACRO if it is #define'd somewhere, even if it looks like a Kconfig
symbol.

Found e.g. https://github.com/zephyrproject-rtos/zephyr/pull/22195.

Piggyback skipping binary files when grepping for Kconfig symbol
references.

Signed-off-by: Ulf Magnusson <Ulf.Magnusson@nordicsemi.no>
---
 scripts/kconfig/lint.py | 75 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 70 insertions(+), 5 deletions(-)

diff --git a/scripts/kconfig/lint.py b/scripts/kconfig/lint.py
index 212c96c9538..fa784735ee9 100755
--- a/scripts/kconfig/lint.py
+++ b/scripts/kconfig/lint.py
@@ -40,7 +40,8 @@ def main():
         checks = (check_always_n,
                   check_unused,
                   check_pointless_menuconfigs,
-                  check_defconfig_only_definition)
+                  check_defconfig_only_definition,
+                  check_missing_config_prefix)
 
     first = True
     for check in checks:
@@ -99,6 +100,22 @@ files. A common base definition should probably be added
 somewhere for such symbols, and the type declaration ('int',
 'hex', etc.) removed from Kconfig.defconfig.""")
 
+    parser.add_argument(
+        "-p", "--check-missing-config-prefix",
+        action="append_const", dest="checks", const=check_missing_config_prefix,
+        help="""\
+Look for references like
+
+    #if MACRO
+    #if(n)def MACRO
+    defined(MACRO)
+    IS_ENABLED(MACRO)
+
+where MACRO is the name of a defined Kconfig symbol but
+doesn't have a CONFIG_ prefix. Could be a typo.
+
+Macros that are #define'd somewhere are not flagged.""")
+
     return parser.parse_args()
 
 
@@ -134,6 +151,50 @@ def check_defconfig_only_definition():
             print(name_and_locs(sym))
 
 
+def check_missing_config_prefix():
+    print_header("Symbol references that might be missing a CONFIG_ prefix")
+
+    # Paths to modules
+    modpaths = run(("west", "list", "-f{abspath}")).splitlines()
+
+    # Gather #define'd macros that might overlap with symbol names, so that
+    # they don't trigger false positives
+    defined = set()
+    for modpath in modpaths:
+        regex = r"#\s*define\s+([A-Z0-9_]+)\b"
+        defines = run(("git", "grep", "--extended-regexp", regex),
+                      cwd=modpath, check=False)
+        # Could pass --only-matching to git grep as well, but it was added
+        # pretty recently (2018)
+        defined.update(re.findall(regex, defines))
+
+    # Filter out symbols whose names are #define'd too. Preserve definition
+    # order to make the output consistent.
+    syms = [sym for sym in kconf.unique_defined_syms
+            if sym.name not in defined]
+
+    # grep for symbol references in #ifdef/defined() that are missing a CONFIG_
+    # prefix. Work around an "argument list too long" error from 'git grep' by
+    # checking symbols in batches.
+    for batch in split_list(syms, 200):
+        # grep for '#if((n)def) <symbol>', 'defined(<symbol>', and
+        # 'IS_ENABLED(<symbol>', with a missing CONFIG_ prefix
+        regex = r"(?:#\s*if(?:n?def)\s+|\bdefined\s*\(\s*|IS_ENABLED\(\s*)(?:" + \
+                "|".join(sym.name for sym in batch) + r")\b"
+        cmd = ("git", "grep", "--line-number", "-I", "--perl-regexp", regex)
+
+        for modpath in modpaths:
+            print(run(cmd, cwd=modpath, check=False), end="")
+
+
+def split_list(lst, batch_size):
+    # check_missing_config_prefix() helper generator that splits a list into
+    # equal-sized batches (possibly with a shorter batch at the end)
+
+    for i in range(0, len(lst), batch_size):
+        yield lst[i:i + batch_size]
+
+
 def print_header(s):
     print(s + "\n" + len(s)*"=")
 
@@ -189,7 +250,7 @@ def referenced_outside_kconfig():
 
     # 'git grep' all modules
     for modpath in run(("west", "list", "-f{abspath}")).splitlines():
-        for line in run(("git", "grep", "-h", "--extended-regexp", regex),
+        for line in run(("git", "grep", "-h", "-I", "--extended-regexp", regex),
                         cwd=modpath).splitlines():
             # Don't record lines starting with "CONFIG_FOO=" or "# CONFIG_FOO="
             # as references, so that symbols that are only assigned in .config
@@ -229,9 +290,11 @@ def name_and_locs(sym):
         ", ".join("{0.filename}:{0.linenr}".format(node) for node in sym.nodes))
 
 
-def run(cmd, cwd=TOP_DIR):
+def run(cmd, cwd=TOP_DIR, check=True):
     # Runs 'cmd' with subprocess, returning the decoded stdout output. 'cwd' is
     # the working directory. It defaults to the top-level Zephyr directory.
+    # Exits with an error if the command exits with a non-zero return code if
+    # 'check' is True.
 
     cmd_s = " ".join(shlex.quote(word) for word in cmd)
 
@@ -242,9 +305,11 @@ def run(cmd, cwd=TOP_DIR):
         err("Failed to run '{}': {}".format(cmd_s, e))
 
     stdout, stderr = process.communicate()
-    stdout = stdout.decode("utf-8")
+    # errors="ignore" temporarily works around
+    # https://github.com/zephyrproject-rtos/esp-idf/pull/2
+    stdout = stdout.decode("utf-8", errors="ignore")
     stderr = stderr.decode("utf-8")
-    if process.returncode:
+    if check and process.returncode:
         err("""\
 '{}' exited with status {}.