Merge branch 'feat/sdkconfig_rename_checker_first' into 'master'

Add sdkconfig.rename checker

See merge request espressif/esp-idf-kconfig!56

kconfcheck/core.py

@@ -7,6 +7,7 @@
 import argparse
 import os
 import re
+from typing import Tuple
 
 # output file with suggestions will get this suffix
 OUTPUT_SUFFIX = ".new"
@@ -51,12 +52,18 @@ class BaseChecker(object):
     def __init__(self, path_in_idf):
         self.path_in_idf = path_in_idf
 
-    def __enter__(self):
-        return self
-
-    def __exit__(self, type, value, traceback):
+    def finalize(self):
+        """
+        Abstract method for finalizing the checker
+        """
         pass
 
+    def process_line(self, line, line_number):
+        """
+        Abstract method for processing a line
+        """
+        raise NotImplementedError("process_line method is not implemented in BaseChecker")
+
 
 class SourceChecker(BaseChecker):
     # allow to source only files which will be also checked by the script
@@ -226,8 +233,7 @@ def __init__(self, path_in_idf, debug=False):
             "choice": reg_config_menuconfig_choice,
         }
 
-    def __exit__(self, type, value, traceback):
-        super(IndentAndNameChecker, self).__exit__(type, value, traceback)
+    def finalize(self):
         if len(self.prefix_stack) > 0:
             self.check_common_prefix("", "EOF")
         if len(self.prefix_stack) != 0:
@@ -459,54 +465,112 @@ def process_line(self, line, line_number):
             )
 
 
+class SDKRenameChecker(BaseChecker):
+    """
+    Checks sdkconfig.rename[.target] files:
+    * if the line contains at least two tokens (old and new config name pairs)
+    * if the inversion syntax is used correctly
+    """
+
+    def __init__(self, path_in_idf):
+        super().__init__(path_in_idf)
+
+    def process_line(self, line: str, line_number: int) -> None:
+        # The line should look like CONFIG_OLD_NAME CONFIG_NEW_NAME # optional comment,
+        # just # comment or blank line.
+        if line.startswith("#") or line.isspace():
+            return
+
+        tokens = line.split()
+        if len(tokens) < 2:
+            raise InputError(
+                self.path_in_idf,
+                line_number,
+                "Line should contain at least old and new config names.",
+                line,  # no suggestions for this
+            )
+        else:
+            old_name, new_name = tokens[:2]  # [:2] removes optional comment from tokens
+
+        if new_name.startswith("!"):  # inversion
+            new_name = new_name[1:]
+
+        ############################################
+        # sdkconfig.rename specific checks
+        ############################################
+        # Check inversion syntax
+        if old_name.startswith("!"):
+            raise InputError(
+                self.path_in_idf,
+                line_number,
+                "For inversion, use CONFIG_OLD !CONFIG_NEW syntax.",
+                line[: line.index(old_name)]
+                + old_name[1:]
+                + line[line.index(old_name) + len(old_name) : line.index(new_name)]
+                + "!"
+                + line[line.index(new_name) :],
+            )
+
+
 def valid_directory(path):
     if not os.path.isdir(path):
         raise argparse.ArgumentTypeError("{} is not a valid directory!".format(path))
     return path
 
 
-def validate_kconfig_file(kconfig_full_path: str, verbose: bool = False, replace: bool = False) -> bool:
+def validate_file(file_full_path: str, verbose: bool = False, replace: bool = False) -> bool:
     # Even in case of in_place modification, create a new file with suggestions (original will be replaced later).
-    suggestions_full_path = kconfig_full_path + OUTPUT_SUFFIX
+    suggestions_full_path = file_full_path + OUTPUT_SUFFIX
     fail = False
+    checkers: Tuple[BaseChecker, ...] = tuple()
+    if "Kconfig" in os.path.basename(file_full_path):
+        checkers = (
+            IndentAndNameChecker(file_full_path),  # indent checker has to be before line checker, otherwise
+            # false-positive indent error if error in line_checker
+            LineRuleChecker(file_full_path),
+            SourceChecker(file_full_path),
+        )
+    elif "sdkconfig.rename" in os.path.basename(file_full_path):
+        checkers = (SDKRenameChecker(file_full_path),)
+
+    if not checkers:
+        raise NotImplementedError(
+            f"The {os.path.basename(file_full_path)} files are not currently supported by kconfcheck."
+        )
 
-    with open(kconfig_full_path, "r", encoding="utf-8") as f, open(
+    with open(file_full_path, "r", encoding="utf-8") as f, open(
         suggestions_full_path, "w", encoding="utf-8", newline="\n"
-    ) as f_o, LineRuleChecker(kconfig_full_path) as line_checker, SourceChecker(
-        kconfig_full_path
-    ) as source_checker, IndentAndNameChecker(kconfig_full_path, debug=verbose) as indent_and_name_checker:
+    ) as f_o:
         try:
-            for line_number, line in enumerate(f, start=1):
+            for line_number, line in enumerate(f):
                 try:
-                    for checker in [
-                        indent_and_name_checker,  # indent checker has to be before line checker, otherwise
-                        # false-positive indent error if error in line_checker
-                        line_checker,
-                        source_checker,
-                    ]:
-                        checker.process_line(line, line_number)
+                    for checker in checkers:
+                        checker.process_line(line=line, line_number=line_number)
                     # The line is correct therefore we echo it to the output file
                     f_o.write(line)
                 except InputError as e:
                     print(e)
                     fail = True
                     f_o.write(e.suggested_line)
         except UnicodeDecodeError:
-            raise ValueError("The encoding of {} is not Unicode.".format(kconfig_full_path))
+            raise ValueError("The encoding of {} is not Unicode.".format(file_full_path))
+        finally:
+            for checker in checkers:
+                checker.finalize()
 
     if replace:
-        os.replace(suggestions_full_path, kconfig_full_path)
+        os.replace(suggestions_full_path, file_full_path)
 
     if fail:
         print(
             "\t{} has been saved with suggestions for resolving the issues.\n"
             "\tPlease note that the suggestions can be wrong and "
             "you might need to re-run the checker several times "
-            "for solving all issues".format(suggestions_full_path if not replace else kconfig_full_path)
+            "for solving all issues".format(suggestions_full_path if not replace else file_full_path)
         )
         return False
     else:
-        print("{}: OK".format(kconfig_full_path))
+        print("{}: OK".format(file_full_path))
         try:
             os.remove(suggestions_full_path)
         except Exception:
@@ -541,7 +605,7 @@ def main():
     files = [os.path.abspath(file_path) for file_path in args.files]
 
     for full_path in files:
-        is_valid = validate_kconfig_file(full_path, args.verbose, args.replace)
+        is_valid = validate_file(full_path, args.verbose, args.replace)
         if is_valid:
             success_counter += 1
         else:

test/kconfcheck/sdkconfigs/correct/correct_inversion.sdkconfig.rename

@@ -0,0 +1,3 @@
+# test if correct inversion is OK
+
+CONFIG_OLD !CONFIG_NEW

test/kconfcheck/sdkconfigs/correct/correct_prefix.sdkconfig.rename

@@ -0,0 +1,3 @@
+# test correct prefix (CONFIG_), in other words normal rename
+
+CONFIG_OLD CONFIG_NEW

test/kconfcheck/sdkconfigs/incorrect/only_one_name.sdkconfig.rename

@@ -0,0 +1,3 @@
+# Only one name
+
+CONFIG_OLD

test/kconfcheck/sdkconfigs/incorrect/wrong_inversion.sdkconfig.rename

@@ -0,0 +1,3 @@
+# Test correct placement of inversion operator
+
+!CONFIG_OLD CONFIG_NEW

test/kconfcheck/sdkconfigs/suggestions/only_one_name.sdkconfig.rename.suggestions

@@ -0,0 +1,3 @@
+# Only one name
+
+CONFIG_OLD

test/kconfcheck/sdkconfigs/suggestions/wrong_inversion.sdkconfig.rename.suggestions

@@ -0,0 +1,3 @@
+# Test correct placement of inversion operator
+
+CONFIG_OLD !CONFIG_NEW

test/kconfcheck/test_kconfcheck.py

@@ -12,7 +12,7 @@
 from kconfcheck.core import InputError
 from kconfcheck.core import LineRuleChecker
 from kconfcheck.core import SourceChecker
-from kconfcheck.core import validate_kconfig_file
+from kconfcheck.core import validate_file
 
 
 class ApplyLine(object):
@@ -85,7 +85,7 @@ def setUp(self):
         self.checker.min_prefix_length = 4
 
     def tearDown(self):
-        self.checker.__exit__("Kconfig", None, None)
+        self.checker.finalize()
 
 
 class TestIndent(TestIndentAndNameChecker):
@@ -181,6 +181,18 @@ def test_comment_after_config(self):
         self.expt_success("        text")
         self.expt_success('        # second not realcomment"')
 
+    def test_missing_endmenu(self):
+        """
+        IndentAndNameChecker raises RuntimeError if there is missing endmenu of inner menu
+        """
+        self.expt_success('menu "test"')
+        self.expt_success("    config FOO")
+        self.expt_success("        bool")
+        self.expt_success('    menu "inner menu"')
+        self.expt_success("        config FOO_BAR")
+        self.expt_success("            bool")
+        self.assertRaises(RuntimeError, self.checker.finalize)
+
 
 class TestName(TestIndentAndNameChecker):
     def setUp(self):
@@ -268,7 +280,7 @@ def test_nested_ifendif(self):
 
 class TestReplace(unittest.TestCase):
     """
-    Test the (not only) pre-commit hook in place change by running validate_kconfig_file() with replace=True.
+    Test the (not only) pre-commit hook in place change by running validate_file() with replace=True.
     Original Kconfig should be modified instead of creating new file.
     """
 
@@ -296,18 +308,49 @@ def tearDown(self):
             pass
 
     def test_no_replace(self):
-        validate_kconfig_file(self.TEST_FILE, replace=False)
+        validate_file(self.TEST_FILE, replace=False)
         self.assertTrue(os.path.isfile(self.TEST_FILE + ".new"))
         self.assertTrue(os.path.isfile(self.TEST_FILE))
         self.assertFalse(filecmp.cmp(self.TEST_FILE + ".new", self.ORIGINAL))
         self.assertTrue(filecmp.cmp(self.TEST_FILE, self.ORIGINAL))
 
     def test_replace(self):
-        validate_kconfig_file(os.path.abspath(self.TEST_FILE), replace=True)
+        validate_file(os.path.abspath(self.TEST_FILE), replace=True)
         self.assertTrue(os.path.isfile(self.TEST_FILE))
         self.assertFalse(os.path.isfile(self.TEST_FILE + ".new"))
         self.assertFalse(filecmp.cmp(self.TEST_FILE, self.ORIGINAL))
 
 
+class TestSDKConfigRename(unittest.TestCase):
+    def __init__(self, *args, **kwargs):
+        super(TestSDKConfigRename, self).__init__(*args, **kwargs)
+        current_path = os.path.abspath(os.path.dirname(__file__))
+        self.correct_sdkconfigs = os.path.join(current_path, "sdkconfigs", "correct")
+        self.incorrect_sdkconfigs = os.path.join(current_path, "sdkconfigs", "incorrect")
+        self.suggestions = os.path.join(current_path, "sdkconfigs", "suggestions")
+
+    def test_correct_sdkconfigs(self):
+        correct_files = os.listdir(self.correct_sdkconfigs)
+        for correct_file in correct_files:
+            is_valid = validate_file(os.path.join(self.correct_sdkconfigs, correct_file))
+            self.assertTrue(is_valid)
+
+    def test_incorrect_sdkconfigs(self):
+        incorrect_files = os.listdir(self.incorrect_sdkconfigs)
+        for incorrect_file in incorrect_files:
+            is_valid = validate_file(os.path.join(self.incorrect_sdkconfigs, incorrect_file))
+            self.assertFalse(is_valid)
+            with open(
+                os.path.join(self.suggestions, incorrect_file + ".suggestions"), "r"
+            ) as file_expected_suggestions, open(
+                os.path.join(self.incorrect_sdkconfigs, incorrect_file + ".new"), "r"
+            ) as file_real_suggestion:
+                self.assertEqual(file_expected_suggestions.read(), file_real_suggestion.read())
+            try:
+                os.remove(os.path.join(self.incorrect_sdkconfigs, incorrect_file + ".new"))
+            except FileNotFoundError:
+                pass
+
+
 if __name__ == "__main__":
     unittest.main()