Merge pull request #233 from gilles-peskine-arm/uncrustify-warnings

Make code_style.py more user-friendly

scripts/code_style.py

@@ -15,7 +15,6 @@ from mbedtls_framework import build_tree
 
 UNCRUSTIFY_SUPPORTED_VERSION = "0.75.1"
 CONFIG_FILE = ".uncrustify.cfg"
-UNCRUSTIFY_EXE = "uncrustify"
 UNCRUSTIFY_ARGS = ["-c", CONFIG_FILE]
 CHECK_GENERATED_FILES = "tests/scripts/check-generated-files.sh"
 
@@ -164,27 +163,30 @@ def get_submodule_hash(commit: str, submodule: str) -> str:
     output = subprocess.check_output(cmd, universal_newlines=True)
     return output.split()[2]
 
-def get_uncrustify_version() -> str:
+def get_uncrustify_version(uncrustify_exe: str) -> str:
     """
-    Get the version string from Uncrustify
+    Get the version string from Uncrustify.
-    """
-    result = subprocess.run([UNCRUSTIFY_EXE, "--version"],
-                            stdout=subprocess.PIPE, stderr=subprocess.PIPE,
-                            check=False)
-    if result.returncode != 0:
-        print_err("Could not get Uncrustify version:", str(result.stderr, "utf-8"))
-        return ""
-    else:
-        return str(result.stdout, "utf-8")
 
-def check_style_is_correct(src_file_list: List[str]) -> bool:
+    Return an empty string if Uncrustify is not found.
+    """
+    try:
+        output = subprocess.check_output([uncrustify_exe, "--version"],
+                                         stderr=subprocess.PIPE)
+        return str(output, "utf-8").strip()
+    except FileNotFoundError:
+        sys.stderr.write('Fatal: command {} not found in PATH.\n'
+                         .format(uncrustify_exe))
+        return ''
+
+def check_style_is_correct(uncrustify_exe: str,
+                           src_file_list: List[str]) -> bool:
     """
     Check the code style and output a diff for each file whose style is
     incorrect.
     """
     style_correct = True
     for src_file in src_file_list:
-        uncrustify_cmd = [UNCRUSTIFY_EXE] + UNCRUSTIFY_ARGS + [src_file]
+        uncrustify_cmd = [uncrustify_exe] + UNCRUSTIFY_ARGS + [src_file]
         result = subprocess.run(uncrustify_cmd, stdout=subprocess.PIPE,
                                 stderr=subprocess.PIPE, check=False)
         if result.returncode != 0:
@@ -210,13 +212,14 @@ def check_style_is_correct(src_file_list: List[str]) -> bool:
 
     return style_correct
 
-def fix_style_single_pass(src_file_list: List[str]) -> bool:
+def fix_style_single_pass(uncrustify_exe: str,
+                          src_file_list: List[str]) -> bool:
     """
     Run Uncrustify once over the source files.
     """
     code_change_args = UNCRUSTIFY_ARGS + ["--no-backup"]
     for src_file in src_file_list:
-        uncrustify_cmd = [UNCRUSTIFY_EXE] + code_change_args + [src_file]
+        uncrustify_cmd = [uncrustify_exe] + code_change_args + [src_file]
         result = subprocess.run(uncrustify_cmd, check=False)
         if result.returncode != 0:
             print_err("Uncrustify with file returned: " +
@@ -225,18 +228,18 @@ def fix_style_single_pass(src_file_list: List[str]) -> bool:
             return False
     return True
 
-def fix_style(src_file_list: List[str]) -> int:
+def fix_style(uncrustify_exe: str, src_file_list: List[str]) -> int:
     """
     Fix the code style. This takes 2 passes of Uncrustify.
     """
-    if not fix_style_single_pass(src_file_list):
+    if not fix_style_single_pass(uncrustify_exe, src_file_list):
         return 1
-    if not fix_style_single_pass(src_file_list):
+    if not fix_style_single_pass(uncrustify_exe, src_file_list):
         return 1
 
     # Guard against future changes that cause the codebase to require
     # more passes.
-    if not check_style_is_correct(src_file_list):
+    if not check_style_is_correct(uncrustify_exe, src_file_list):
         print_err("Code style still incorrect after second run of Uncrustify.")
         return 1
     else:
@@ -246,13 +249,6 @@ def main() -> int:
     """
     Main with command line arguments.
     """
-    uncrustify_version = get_uncrustify_version().strip()
-    if UNCRUSTIFY_SUPPORTED_VERSION not in uncrustify_version:
-        print("Warning: Using unsupported Uncrustify version '" +
-              uncrustify_version + "'")
-        print("Note: The only supported version is " +
-              UNCRUSTIFY_SUPPORTED_VERSION)
-
     parser = argparse.ArgumentParser()
     parser.add_argument('-f', '--fix', action='store_true',
                         help=('modify source files to fix the code style '
@@ -268,11 +264,23 @@ def main() -> int:
     # way to restyle a possibly empty set of files.
     parser.add_argument('--subset', action='store_true',
                         help='only check the specified files (default with non-option arguments)')
+    parser.add_argument('--uncrustify',
+                        default='uncrustify',
+                        help='uncrustify command to run (default: uncrustify)')
     parser.add_argument('operands', nargs='*', metavar='FILE',
                         help='files to check (files MUST be known to git, if none: check all)')
 
     args = parser.parse_args()
 
+    uncrustify_version = get_uncrustify_version(args.uncrustify)
+    if UNCRUSTIFY_SUPPORTED_VERSION not in uncrustify_version:
+        if uncrustify_version != '':
+            sys.stderr.write('Fatal: wrong uncrustify version ({}).\n'
+                             .format(uncrustify_version))
+        sys.stderr.write('You need uncrustify {} for correct results.\n'
+                         .format(UNCRUSTIFY_SUPPORTED_VERSION))
+        return 2
+
     covered = frozenset(get_src_files(args.since))
     # We only check files that are known to git
     if args.subset or args.operands:
@@ -285,10 +293,10 @@ def main() -> int:
 
     if args.fix:
         # Fix mode
-        return fix_style(src_files)
+        return fix_style(args.uncrustify, src_files)
     else:
         # Check mode
-        if check_style_is_correct(src_files):
+        if check_style_is_correct(args.uncrustify, src_files):
             print("Checked {} files, style ok.".format(len(src_files)))
             return 0
         else: