Add ability to write include rules specific to subsets of files in a directory.

This is useful e.g. to limit a set of allowed or temporarily-allowed
include rules to test files, or to allow certain includes in .cc files
but not in .h files.

BUG=138280


Review URL: https://chromiumcodereview.appspot.com/10823271

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@151301 0039d316-1c4b-4281-b951-d872f2087c98

tools/checkdeps/checkdeps.py

@@ -47,6 +47,20 @@ Note that for .java files, there is currently no difference between
     "!base/evil/ok_for_now.h",
   }
 
+If you have certain include rules that should only be applied for some
+files within this directory and subdirectories, you can write a
+section named specific_include_rules that is a hash map of regular
+expressions to the list of rules that should apply to files matching
+them.  Note that such rules will always be applied before the rules
+from 'include_rules' have been applied, but the order in which rules
+associated with different regular expressions is applied is arbitrary.
+
+  specific_include_rules = {
+    ".*_(unit|browser|api)test\.cc": [
+      "+libraries/testsupport",
+    ],
+  }
+
 DEPS files may be placed anywhere in the tree. Each one applies to all
 subdirectories, where there may be more DEPS files that provide additions or
 subtractions for their own sub-trees.
@@ -80,6 +94,11 @@ from rules import Rule, Rules
 # the module-level deps.
 INCLUDE_RULES_VAR_NAME = 'include_rules'
 
+# Variable name used in the DEPS file to add or subtract include files
+# from module-level deps specific to files whose basename (last
+# component of path) matches a given regular expression.
+SPECIFIC_INCLUDE_RULES_VAR_NAME = 'specific_include_rules'
+
 # Optionally present in the DEPS file to list subdirectories which should not
 # be checked. This allows us to skip third party code, for example.
 SKIP_SUBDIRS_VAR_NAME = 'skip_child_includes'
@@ -130,12 +149,14 @@ class DepsChecker(object):
     print '\nSUCCESS\n'
     return 0
 
-  def _ApplyRules(self, existing_rules, includes, cur_dir):
+  def _ApplyRules(self, existing_rules, includes, specific_includes, cur_dir):
     """Applies the given include rules, returning the new rules.
 
     Args:
       existing_rules: A set of existing rules that will be combined.
       include: The list of rules from the "include_rules" section of DEPS.
+      specific_includes: E.g. {'.*_unittest\.cc': ['+foo', '-blat']} rules
+                         from the "specific_include_rules" section of DEPS.
       cur_dir: The current directory, normalized path. We will create an
                implicit rule that allows inclusion from this directory.
 
@@ -158,13 +179,24 @@ class DepsChecker(object):
                       ' for\n  %s and base dir\n  %s' %
                       (cur_dir, self.base_directory))
 
-    # Last, apply the additional explicit rules.
-    for (_, rule_str) in enumerate(includes):
+    def AddRuleWithDescription(rule_str, dependee_regexp=None):
+      rule_block_name = 'include_rules'
+      if dependee_regexp:
+        rule_block_name = 'specific_include_rules'
       if not relative_dir:
-        rule_description = 'the top level include_rules'
+        rule_description = 'the top level %s' % rule_block_name
       else:
-        rule_description = relative_dir + "'s include_rules"
-      rules.AddRule(rule_str, rule_description)
+        rule_description = relative_dir + "'s %s" % rule_block_name
+      rules.AddRule(rule_str, rule_description, dependee_regexp)
+
+    # Apply the additional explicit rules.
+    for (_, rule_str) in enumerate(includes):
+      AddRuleWithDescription(rule_str)
+
+    # Finally, apply the specific rules.
+    for regexp, specific_rules in specific_includes.iteritems():
+      for rule_str in specific_rules:
+        AddRuleWithDescription(rule_str, regexp)
 
     return rules
 
@@ -239,9 +271,12 @@ class DepsChecker(object):
     # Even if a DEPS file does not exist we still invoke ApplyRules
     # to apply the implicit "allow" rule for the current directory
     include_rules = local_scope.get(INCLUDE_RULES_VAR_NAME, [])
+    specific_include_rules = local_scope.get(SPECIFIC_INCLUDE_RULES_VAR_NAME,
+                                             {})
     skip_subdirs = local_scope.get(SKIP_SUBDIRS_VAR_NAME, [])
 
-    return (self._ApplyRules(existing_rules, include_rules, norm_dir_name),
+    return (self._ApplyRules(existing_rules, include_rules,
+                             specific_include_rules, norm_dir_name),
             skip_subdirs)
 
   def _ApplyDirectoryRulesAndSkipSubdirs(self, parent_rules, dir_path):
@@ -357,7 +392,8 @@ class DepsChecker(object):
       rules_for_file = self.GetDirectoryRules(os.path.dirname(file_path))
       if rules_for_file:
         for line in include_lines:
-          is_include, violation = cpp.CheckLine(rules_for_file, line, True)
+          is_include, violation = cpp.CheckLine(
+              rules_for_file, line, file_path, True)
           if violation:
             rule_type = violation.violated_rule.allow
             if rule_type != Rule.ALLOW:

tools/checkdeps/checkdeps_test.py

@@ -24,7 +24,7 @@ class CheckDepsTest(unittest.TestCase):
         os.path.join(self.deps_checker.base_directory,
                      'tools/checkdeps/testdata'))
     problems = self.deps_checker.results_formatter.GetResults()
-    self.failUnlessEqual(3, len(problems))
+    self.failUnlessEqual(4, len(problems))
 
     def VerifySubstringsInProblems(key_path, substrings_in_sequence):
       found = False
@@ -34,7 +34,7 @@ class CheckDepsTest(unittest.TestCase):
         if index != -1:
           for substring in substrings_in_sequence:
             index = problem.find(substring, index + 1)
-            self.failUnless(index != -1)
+            self.failUnless(index != -1, '%s in %s' % (substring, problem))
           found = True
           break
       if not found:
@@ -52,6 +52,8 @@ class CheckDepsTest(unittest.TestCase):
                                ['-third_party/explicitly_disallowed',
                                 'Because of no rule applying',
                                 'Because of no rule applying'])
+    VerifySubstringsInProblems('allowed/not_a_test.cc',
+                               ['-tools/checkdeps/testdata/disallowed'])
 
   def testTempRulesGenerator(self):
     self.deps_checker.results_formatter = results.TemporaryRulesFormatter()
@@ -61,7 +63,8 @@ class CheckDepsTest(unittest.TestCase):
     temp_rules = self.deps_checker.results_formatter.GetResults()
     expected = [u'  "!third_party/explicitly_disallowed/bad.h",',
                 u'  "!third_party/no_rule/bad.h",',
-                u'  "!tools/checkdeps/testdata/disallowed/bad.h",']
+                u'  "!tools/checkdeps/testdata/disallowed/bad.h",',
+                u'  "!tools/checkdeps/testdata/disallowed/teststuff/bad.h",']
     self.failUnlessEqual(expected, temp_rules)
 
   def testCheckAddedIncludesAllGood(self):

tools/checkdeps/cpp_checker.py

@@ -36,7 +36,7 @@ class CppChecker(object):
   def __init__(self, verbose):
     self._verbose = verbose
 
-  def CheckLine(self, rules, line, fail_on_temp_allow=False):
+  def CheckLine(self, rules, line, dependee_path, fail_on_temp_allow=False):
     """Checks the given line with the given rule set.
 
     Returns a tuple (is_include, dependency_violation) where
@@ -62,7 +62,7 @@ class CppChecker(object):
         print ' WARNING: directory specified with no path: ' + include_path
       return True, None
 
-    rule = rules.RuleApplyingTo(include_path)
+    rule = rules.RuleApplyingTo(include_path, dependee_path)
     if (rule.allow == Rule.DISALLOW or
         (fail_on_temp_allow and rule.allow == Rule.TEMP_ALLOW)):
       return True, results.DependencyViolation(include_path, rule, rules)
@@ -94,7 +94,7 @@ class CppChecker(object):
             in_if0 -= 1
           continue
 
-        is_include, violation = self.CheckLine(rules, line)
+        is_include, violation = self.CheckLine(rules, line, filepath)
         if is_include:
           last_include = line_num
         if violation:

tools/checkdeps/java_checker.py

@@ -96,7 +96,7 @@ class JavaChecker(object):
               self._classmap[clazz], self._base_directory)
           # Convert Windows paths to Unix style, as used in DEPS files.
           include_path = include_path.replace(os.path.sep, '/')
-          rule = rules.RuleApplyingTo(include_path)
+          rule = rules.RuleApplyingTo(include_path, filepath)
           if rule.allow == Rule.DISALLOW:
             dependee_status.AddViolation(
                 results.DependencyViolation(include_path, rule, rules))

tools/checkdeps/results.py

@@ -82,7 +82,7 @@ class NormalResultsFormatter(ResultsFormatter):
   def FormatViolation(violation, verbose=False):
     lines = []
     if verbose:
-      lines.append('For %s' % violation.rules)
+      lines.append('  For %s' % violation.rules)
     lines.append(
         '  Illegal include: "%s"\n    Because of %s' %
         (violation.include_path, str(violation.violated_rule)))

tools/checkdeps/rules.py

@@ -5,6 +5,10 @@
 """Base classes to represent dependency rules, used by checkdeps.py"""
 
 
+import os
+import re
+
+
 class Rule(object):
   """Specifies a single rule for an include, which can be one of
   ALLOW, DISALLOW and TEMP_ALLOW.
@@ -36,13 +40,13 @@ class Rule(object):
     return self._dir == other or other.startswith(self._dir + '/')
 
 
-class SpecificRule(Rule):
-  """A rule that has a specific reason not related to directory or
-  source, for failing.
+class MessageRule(Rule):
+  """A rule that has a simple message as the reason for failing,
+  unrelated to directory or source.
   """
 
   def __init__(self, reason):
-    super(SpecificRule, self).__init__(Rule.DISALLOW, '', '')
+    super(MessageRule, self).__init__(Rule.DISALLOW, '', '')
     self._reason = reason
 
   def __str__(self):
@@ -50,8 +54,8 @@ class SpecificRule(Rule):
 
 
 def ParseRuleString(rule_string, source):
-  """Returns a tuple of a boolean indicating whether the directory is an allow
-  rule, and a string holding the directory name.
+  """Returns a tuple of a character indicating what type of rule this
+  is, and a string holding the path the rule applies to.
   """
   if not rule_string:
     raise Exception('The rule string "%s" is empty\nin %s' %
@@ -66,30 +70,82 @@ def ParseRuleString(rule_string, source):
 
 
 class Rules(object):
+  """Sets of rules for files in a directory.
+
+  By default, rules are added to the set of rules applicable to all
+  dependee files in the directory.  Rules may also be added that apply
+  only to dependee files whose filename (last component of their path)
+  matches a given regular expression; hence there is one additional
+  set of rules per unique regular expression.
+  """
+
   def __init__(self):
-    """Initializes the current rules with an empty rule list."""
-    self._rules = []
+    """Initializes the current rules with an empty rule list for all
+    files.
+    """
+    # We keep the general rules out of the specific rules dictionary,
+    # as we need to always process them last.
+    self._general_rules = []
 
-  def __str__(self):
-    return 'Rules = [\n%s]' % '\n'.join(' %s' % x for x in self._rules)
+    # Keys are regular expression strings, values are arrays of rules
+    # that apply to dependee files whose basename matches the regular
+    # expression.  These are applied before the general rules, but
+    # their internal order is arbitrary.
+    self._specific_rules = {}
 
-  def AddRule(self, rule_string, source):
+  def __str__(self):
+    result = ['Rules = {\n    (apply to all files): [\n%s\n    ],' % '\n'.join(
+        '      %s' % x for x in self._general_rules)]
+    for regexp, rules in self._specific_rules.iteritems():
+      result.append('    (limited to files matching %s): [\n%s\n    ]' % (
+          regexp, '\n'.join('      %s' % x for x in rules)))
+    result.append('  }')
+    return '\n'.join(result)
+
+  def AddRule(self, rule_string, source, dependee_regexp=None):
     """Adds a rule for the given rule string.
 
     Args:
       rule_string: The include_rule string read from the DEPS file to apply.
       source: A string representing the location of that string (filename, etc.)
               so that we can give meaningful errors.
+      dependee_regexp: The rule will only be applied to dependee files
+                       whose filename (last component of their path)
+                       matches the expression. None to match all
+                       dependee files.
     """
-    (add_rule, rule_dir) = ParseRuleString(rule_string, source)
+    (rule_type, rule_dir) = ParseRuleString(rule_string, source)
+
+    if not dependee_regexp:
+      rules_to_update = self._general_rules
+    else:
+      if dependee_regexp in self._specific_rules:
+        rules_to_update = self._specific_rules[dependee_regexp]
+      else:
+        rules_to_update = []
+
     # Remove any existing rules or sub-rules that apply. For example, if we're
     # passed "foo", we should remove "foo", "foo/bar", but not "foobar".
-    self._rules = [x for x in self._rules if not x.ParentOrMatch(rule_dir)]
-    self._rules.insert(0, Rule(add_rule, rule_dir, source))
-
-  def RuleApplyingTo(self, allowed_dir):
-    """Returns the rule that applies to 'allowed_dir'."""
-    for rule in self._rules:
-      if rule.ChildOrMatch(allowed_dir):
+    rules_to_update = [x for x in rules_to_update
+                       if not x.ParentOrMatch(rule_dir)]
+    rules_to_update.insert(0, Rule(rule_type, rule_dir, source))
+
+    if not dependee_regexp:
+      self._general_rules = rules_to_update
+    else:
+      self._specific_rules[dependee_regexp] = rules_to_update
+
+  def RuleApplyingTo(self, include_path, dependee_path):
+    """Returns the rule that applies to |include_path| for a dependee
+    file located at |dependee_path|.
+    """
+    dependee_filename = os.path.basename(dependee_path)
+    for regexp, specific_rules in self._specific_rules.iteritems():
+      if re.match(regexp, dependee_filename):
+        for rule in specific_rules:
+          if rule.ChildOrMatch(include_path):
+            return rule
+    for rule in self._general_rules:
+      if rule.ChildOrMatch(include_path):
         return rule
-    return SpecificRule('no rule applying.')
+    return MessageRule('no rule applying.')

tools/checkdeps/testdata/allowed/DEPS

@@ -3,3 +3,9 @@ include_rules = [
   "!tools/checkdeps/testdata/disallowed/temporarily_allowed.h",
   "+third_party/allowed_may_use",
 ]
+
+specific_include_rules = {
+  ".*_unittest\.cc": [
+    "+tools/checkdeps/testdata/disallowed/teststuff",
+  ]
+}

tools/checkdeps/testdata/allowed/foo_unittest.cc

+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "tools/checkdeps/testdata/disallowed/teststuff/good.h"

tools/checkdeps/testdata/allowed/not_a_test.cc

+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "tools/checkdeps/testdata/disallowed/teststuff/bad.h"