From 1734127e5998f6874beee5430081f15cfcb33ce6 Mon Sep 17 00:00:00 2001
From: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Date: Sun, 26 Dec 2021 15:49:15 -0300
Subject: [PATCH] utils/check-package: prepare to run external tools

Some file formats have well-established syntax checkers.
One example of this is the tool 'shellcheck' that can analyse shell
scripts for common mistakes.

There is no reason to reimplement such tools in check-package, when we
can just call them.

Add the ability to check-package to call external tools that will run
once for each file to be analysed.
For simplicity, when the tool generated one or more warnings, count it
as a single warning from check-package, that can display something like
this:

|$ ./utils/check-package  package/unscd/S46unscd
|package/unscd/S46unscd:0: run 'shellcheck' and fix the warnings
|25 lines processed
|1 warnings generated

|$ ./utils/check-package -vvvvvvvvvvvvvvvv package/unscd/S46unscd
|package/unscd/S46unscd:0: run 'shellcheck' and fix the warnings
|In package/unscd/S46unscd line 9:
|        printf "Starting ${NAME}: "
|               ^------------------^ SC2059: Don't use variables in the printf format string. Use printf "..%s.." "$foo".
|In package/unscd/S46unscd line 11:
|        [ $? -eq 0 ] && echo "OK" || echo "FAIL"
|          ^-- SC2181: Check exit code directly with e.g. 'if mycmd;', not indirectly with $?.
|In package/unscd/S46unscd line 14:
|        printf "Stopping ${NAME}: "
|               ^------------------^ SC2059: Don't use variables in the printf format string. Use printf "..%s.." "$foo".
|In package/unscd/S46unscd line 16:
|        [ $? -eq 0 ] && echo "OK" || echo "FAIL"
|          ^-- SC2181: Check exit code directly with e.g. 'if mycmd;', not indirectly with $?.
|For more information:
|  https://www.shellcheck.net/wiki/SC2059 -- Don't use variables in the printf...
|  https://www.shellcheck.net/wiki/SC2181 -- Check exit code directly with e.g...
|25 lines processed
|1 warnings generated

In this first commit, add only the ability for check-package to call
external tools and not an example of such tool, as adding each tool to
call may need update to the docker image and can lead to it's own
discussion on how to implement.

Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com>
Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 utils/check-package           | 34 ++++++++++++++++++++++++++++------
 utils/checkpackagelib/base.py |  8 ++++++++
 2 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/utils/check-package b/utils/check-package
index a959fef079..5fb430902d 100755
--- a/utils/check-package
+++ b/utils/check-package
@@ -8,6 +8,7 @@ import re
 import six
 import sys
 
+import checkpackagelib.base
 import checkpackagelib.lib_config
 import checkpackagelib.lib_hash
 import checkpackagelib.lib_mk
@@ -87,9 +88,7 @@ def get_lib_from_filename(fname):
     return None
 
 
-def is_a_check_function(m):
-    if not inspect.isclass(m):
-        return False
+def common_inspect_rules(m):
     # do not call the base class
     if m.__name__.startswith("_"):
         return False
@@ -100,6 +99,22 @@ def is_a_check_function(m):
     return True
 
 
+def is_a_check_function(m):
+    if not inspect.isclass(m):
+        return False
+    if not issubclass(m, checkpackagelib.base._CheckFunction):
+        return False
+    return common_inspect_rules(m)
+
+
+def is_external_tool(m):
+    if not inspect.isclass(m):
+        return False
+    if not issubclass(m, checkpackagelib.base._Tool):
+        return False
+    return common_inspect_rules(m)
+
+
 def print_warnings(warnings):
     # Avoid the need to use 'return []' at the end of every check function.
     if warnings is None:
@@ -121,14 +136,16 @@ def check_file_using_lib(fname):
         if flags.verbose >= VERBOSE_LEVEL_TO_SHOW_IGNORED_FILES:
             print("{}: ignored".format(fname))
         return nwarnings, nlines
-    classes = inspect.getmembers(lib, is_a_check_function)
+    internal_functions = inspect.getmembers(lib, is_a_check_function)
+    external_tools = inspect.getmembers(lib, is_external_tool)
+    all_checks = internal_functions + external_tools
 
     if flags.dry_run:
-        functions_to_run = [c[0] for c in classes]
+        functions_to_run = [c[0] for c in all_checks]
         print("{}: would run: {}".format(fname, functions_to_run))
         return nwarnings, nlines
 
-    objects = [c[1](fname, flags.manual_url) for c in classes]
+    objects = [c[1](fname, flags.manual_url) for c in internal_functions]
 
     for cf in objects:
         nwarnings += print_warnings(cf.before())
@@ -148,6 +165,11 @@ def check_file_using_lib(fname):
     for cf in objects:
         nwarnings += print_warnings(cf.after())
 
+    tools = [c[1](fname) for c in external_tools]
+
+    for tool in tools:
+        nwarnings += print_warnings(tool.run())
+
     return nwarnings, nlines
 
 
diff --git a/utils/checkpackagelib/base.py b/utils/checkpackagelib/base.py
index 9544a64e5a..73da925a03 100644
--- a/utils/checkpackagelib/base.py
+++ b/utils/checkpackagelib/base.py
@@ -16,3 +16,11 @@ class _CheckFunction(object):
 
     def after(self):
         pass
+
+
+class _Tool(object):
+    def __init__(self, filename):
+        self.filename = filename
+
+    def run(self):
+        pass