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

9 files changed, 147 insertions, 36 deletions

diff --git a/tools/checkdeps/checkdeps.py b/tools/checkdeps/checkdeps.py
index a7cff8b..74b61fa 100755
--- a/tools/checkdeps/checkdeps.py
+++ b/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:
diff --git a/tools/checkdeps/checkdeps_test.py b/tools/checkdeps/checkdeps_test.py
index 0729a8f..25db694 100755
--- a/tools/checkdeps/checkdeps_test.py
+++ b/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):
diff --git a/tools/checkdeps/cpp_checker.py b/tools/checkdeps/cpp_checker.py
index 30241a1..b975c19 100644
--- a/tools/checkdeps/cpp_checker.py
+++ b/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:
diff --git a/tools/checkdeps/java_checker.py b/tools/checkdeps/java_checker.py
index d6633b8..670eac2 100644
--- a/tools/checkdeps/java_checker.py
+++ b/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))
diff --git a/tools/checkdeps/results.py b/tools/checkdeps/results.py
index 5091137..5a50e0c 100644
--- a/tools/checkdeps/results.py
+++ b/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)))
diff --git a/tools/checkdeps/rules.py b/tools/checkdeps/rules.py
index 4ec5cf4..09d718c 100644
--- a/tools/checkdeps/rules.py
+++ b/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.')
diff --git a/tools/checkdeps/testdata/allowed/DEPS b/tools/checkdeps/testdata/allowed/DEPS
index ae750e2..362623c 100644
--- a/tools/checkdeps/testdata/allowed/DEPS
+++ b/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",
+  ]
+}
diff --git a/tools/checkdeps/testdata/allowed/foo_unittest.cc b/tools/checkdeps/testdata/allowed/foo_unittest.cc
new file mode 100644
index 0000000..754f295
--- /dev/null
+++ b/tools/checkdeps/testdata/allowed/foo_unittest.cc
@@ -0,0 +1,5 @@
+// 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"
diff --git a/tools/checkdeps/testdata/allowed/not_a_test.cc b/tools/checkdeps/testdata/allowed/not_a_test.cc
new file mode 100644
index 0000000..7b2a105
--- /dev/null
+++ b/tools/checkdeps/testdata/allowed/not_a_test.cc
@@ -0,0 +1,5 @@
+// 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"