util: More enhancements to config_option_check.py.

This commit enhances the config_option_check.py script a little bit
more.

Firstly, I fixed a bug where lines beginning with an '*' were treated as a
comment where it was not so.

  ex: *status = (CONFIG_BAR_PORT & 0x23);

Additionally, I added support for considering deletions.  This allows
the script to check to see if a CONFIG_* option being removed is being
used anywhere else in the repo.  If the option isn't used elsewhere,
then it appears to be the removal of the last use.  An error is flagged
informing the user to remove that option from the main config file.
This helps to keep the config file up to date without leaving stale
CONFIG_* options where one might not know if they still work or not.
Debug config options are always assumed to be used as those are
typically enabled locally.

BUG=chromium:510672
BRANCH=None
TEST=Used a new config option without adding it to the main config file
and watched the error be flagged.
TEST=Removed the last use of a CONFIG_* option while leaving the option
in the main config file. Observed that it was flagged.
TEST=cros lint --debug util/config_option_check.py
TEST=make -j buildall tests

Change-Id: I8702ad06d9856c14f7bcd4592e917a5d3fcb6b57
Signed-off-by: Aseda Aboagye <aaboagye@google.com>
Reviewed-on: https://chromium-review.googlesource.com/294620
Trybot-Ready: Aseda Aboagye <aaboagye@chromium.org>
Tested-by: Aseda Aboagye <aaboagye@chromium.org>
Reviewed-by: Randall Spangler <rspangler@chromium.org>
Commit-Queue: Aseda Aboagye <aaboagye@chromium.org>

1 files changed, 139 insertions, 51 deletions

diff --git a/util/config_option_check.py b/util/config_option_check.py
index e114780df1..ea681a2787 100755
--- a/util/config_option_check.py
+++ b/util/config_option_check.py
@@ -19,12 +19,15 @@ class Line(object):
   Attributes:
     line_num: The integer line number that this line appears in the file.
     string: The literal string of this line.
+    line_type: '+' or '-' indicating if this line was an addition or
+      deletion.
   """
 
-  def __init__(self, line_num, string):
+  def __init__(self, line_num, string, line_type):
     """Inits Line with the line number and the actual string."""
     self.line_num = line_num
     self.string = string
+    self.line_type = line_type
 
 
 class Hunk(object):
@@ -54,7 +57,7 @@ def obtain_current_config_options():
   options.
 
   Returns:
-    A list of all the config options in the master CONFIG_FILE.
+    config_options: A list of all the config options in the master CONFIG_FILE.
   """
 
   config_options = []
@@ -69,12 +72,63 @@ def obtain_current_config_options():
         config_options.append(word)
   return config_options
 
+def obtain_config_options_in_use():
+  """Obtains all the config options in use in the repo.
+
+  Scans through the entire repo looking for all CONFIG_* options actively used.
+
+  Returns:
+   options_in_use: A set of all the config options in use in the repo.
+  """
+  file_list = []
+  cwd = os.getcwd()
+  config_option_re = re.compile(r'\b(CONFIG_[a-zA-Z0-9_]+)')
+  config_debug_option_re = re.compile(r'\b(CONFIG_DEBUG_[a-zA-Z0-9_]+)')
+  options_in_use = set()
+  for (dirpath, dirnames, filenames) in os.walk(cwd, topdown=True):
+    # Ignore the build and private directories (taken from .gitignore)
+    if 'build' in dirnames:
+      dirnames.remove('build')
+    if 'private' in dirnames:
+      dirnames.remove('private')
+    for f in filenames:
+      # Only consider C source, assembler, and Make-style files.
+      if (os.path.splitext(f)[1] in ('.c', '.h', '.inc', '.S', '.mk') or
+          'Makefile' in f):
+        file_list.append(os.path.join(dirpath, f))
+
+  # Search through each file and build a set of the CONFIG_* options being
+  # used.
+
+  for f in file_list:
+    if CONFIG_FILE in f:
+      continue
+    with open(f, 'r') as cur_file:
+      for line in cur_file:
+        match = config_option_re.findall(line)
+        if match:
+          for option in match:
+            if not in_comment(f, line, option):
+              if option not in options_in_use:
+                options_in_use.add(option)
+
+  # Since debug options can be turned on at any time, assume that they are
+  # always in use in case any aren't being used.
+
+  with open(CONFIG_FILE, 'r') as config_file:
+    for line in config_file:
+      match = config_debug_option_re.findall(line)
+      if match:
+        for option in match:
+          if not in_comment(CONFIG_FILE, line, option):
+            if option not in options_in_use:
+              options_in_use.add(option)
+
+  return options_in_use
+
 def print_missing_config_options(hunks, config_options):
   """Searches thru all the changes in hunks for missing options and prints them.
 
-  TODO(aaboagye): Improve upon this to detect when files are being removed and a
-  CONFIG_* option is no longer used elsewhere in the repo.
-
   Args:
     hunks: A list of Hunk objects which represent the hunks from the git
       diff output.
@@ -82,15 +136,18 @@ def print_missing_config_options(hunks, config_options):
 
   Returns:
     missing_config_option: A boolean indicating if any CONFIG_* options
-      are missing from the master CONFIG_FILE in this commit.
+      are missing from the master CONFIG_FILE in this commit or if any CONFIG_*
+      options removed are no longer being used in the repo.
   """
   missing_config_option = False
   print_banner = True
+  deprecated_options = set()
   # Determine longest CONFIG_* length to be used for formatting.
   max_option_length = max(len(option) for option in config_options)
   config_option_re = re.compile(r'\b(CONFIG_[a-zA-Z0-9_]+)')
-  c_style_ext = ('.c', '.h', '.inc', '.S')
-  make_style_ext = ('.mk')
+
+  # Search for all CONFIG_* options in use in the repo.
+  options_in_use = obtain_config_options_in_use()
 
   # Check each hunk's line for a missing config option.
   for h in hunks:
@@ -100,41 +157,22 @@ def print_missing_config_options(hunks, config_options):
       if not match:
         continue
 
-      # At this point, an option was found in the string.  However, we need to
-      # verify that it is not within a comment.  Assume every option is a
-      # violation until proven otherwise.
-
+      # At this point, an option was found in the line.  However, we need to
+      # verify that it is not within a comment.
       violations = set()
+
       for option in match:
-        violations.add(option)
-
-      # Different files have different comment syntax;  Handle appropriately.
-      extension = os.path.splitext(h.filename)[1]
-      if extension in c_style_ext:
-        beg_comment_idx = l.string.find('/*')
-        end_comment_idx = l.string.find('*/')
-        if end_comment_idx == -1:
-          end_comment_idx = len(l.string)
-        for option in match:
-          option_idx = l.string.find(option)
-          if beg_comment_idx == -1:
-            # Check to see if this line is from a multi-line comment.
-            if l.string.lstrip()[0] == '*':
-              # It _seems_ like it is, therefore ignore this instance.
-              violations.remove(option)
+        if not in_comment(h.filename, l.string, option):
+          # Since the CONFIG_* option is not within a comment, we've found a
+          # violation.  We now need to determine if this line is a deletion or
+          # not.  For deletions, we will need to verify if this CONFIG_* option
+          # is no longer being used in the entire repo.
+
+          if l.line_type is '-':
+            if option not in options_in_use and option in config_options:
+              deprecated_options.add(option)
           else:
-            # Check to see if its actually inside the comment.
-            if beg_comment_idx < option_idx < end_comment_idx:
-              # The config option is in the comment.  Ignore it.
-              violations.remove(option)
-      elif extension in make_style_ext or 'Makefile' in h.filename:
-        beg_comment_idx = l.string.find('#')
-        for option in match:
-          option_idx = l.string.find(option)
-          # Ignore everything to the right of the hash.
-          if beg_comment_idx < option_idx and beg_comment_idx != -1:
-            # The option is within a comment.  Ignore it.
-            violations.remove(option)
+            violations.add(option)
 
       # Check to see if the CONFIG_* option is in the config file and print the
       # violations.
@@ -152,8 +190,60 @@ def print_missing_config_options(hunks, config_options):
           print('> %-*s %s:%s' % (max_option_length, option,
                                   h.filename,
                                   l.line_num))
+
+  if deprecated_options:
+    print('\n\nThe following config options are being removed and also appear'
+          ' to be the last uses\nof that option.  Please remove these '
+          'options from %s.\n\n' % CONFIG_FILE)
+    for option in deprecated_options:
+      print('> %s' % option)
+      missing_config_option = True
+
   return missing_config_option
 
+def in_comment(filename, line, substr):
+  """Checks if given substring appears in a comment.
+
+  Args:
+    filename: The filename where this line is from.  This is used to determine
+      what kind of comments to look for.
+    line: String of line to search in.
+    substr: Substring to search for in the line.
+
+  Returns:
+    is_in_comment: Boolean indicating if substr was in a comment.
+  """
+
+  c_style_ext = ('.c', '.h', '.inc', '.S')
+  make_style_ext = ('.mk')
+  is_in_comment = False
+
+  extension = os.path.splitext(filename)[1]
+  substr_idx = line.find(substr)
+
+  # Different files have different comment syntax;  Handle appropriately.
+  if extension in c_style_ext:
+    beg_comment_idx = line.find('/*')
+    end_comment_idx = line.find('*/')
+    if end_comment_idx == -1:
+      end_comment_idx = len(line)
+
+    if beg_comment_idx == -1:
+      # Check to see if this line is from a multi-line comment.
+      if line.lstrip().startswith('* '):
+        # It _seems_ like it is.
+        is_in_comment = True
+    else:
+      # Check to see if its actually inside the comment.
+      if beg_comment_idx < substr_idx < end_comment_idx:
+        is_in_comment = True
+  elif extension in make_style_ext or 'Makefile' in filename:
+    beg_comment_idx = line.find('#')
+    # Ignore everything to the right of the hash.
+    if beg_comment_idx < substr_idx and beg_comment_idx != -1:
+      is_in_comment = True
+  return is_in_comment
+
 def get_hunks():
   """Gets the hunks of the most recent commit.
 
@@ -168,7 +258,6 @@ def get_hunks():
       output.
   """
 
-  # Get the diff output.
   diff = []
   hunks = []
   hunk_lines = []
@@ -181,8 +270,9 @@ def get_hunks():
   new_file_re = re.compile(r'^diff --git')
   filename_re = re.compile(r'^[+]{3} (.*)')
   hunk_line_num_re = re.compile(r'^@@ -[0-9]+,[0-9]+ \+([0-9]+),[0-9]+ @@.*')
-  line_re = re.compile(r'^([+| ])(.*)')
+  line_re = re.compile(r'^([+| |-])(.*)')
 
+  # Get the diff output.
   cmd = 'git diff --cached -GCONFIG_* --no-prefix --no-ext-diff HEAD~1'
   diff = subprocess.check_output(cmd.split()).split('\n')
   line = diff[0]
@@ -197,11 +287,6 @@ def get_hunks():
 
     # Search the diff output for a file name.
     elif current_state is 'filename_search':
-      # If we're deleting a file, ignore it entirely.
-      if 'deleted' in line:
-        current_state = 'new_file'
-        continue
-
       # Search for a file name.
       match = filename_re.search(line)
       if match:
@@ -238,10 +323,13 @@ def get_hunks():
 
       match = line_re.search(line)
       if match:
-        # Consider this line iff it's an addition.
-        if match.groups(1)[0] == '+':
-          hunk_lines.append(Line(line_num, match.groups(2)[1]))
-        line_num += 1
+        line_type = match.groups(1)[0]
+        # We only care about modifications.
+        if line_type is not ' ':
+          hunk_lines.append(Line(line_num, match.groups(2)[1], line_type))
+        # Deletions don't count towards the line numbers.
+        if line_type is not '-':
+          line_num += 1
 
     # Advance to the next line
     try: