[WebUI] Fix 0-length unary term false positive in CSS presubmit.

chrome/browser/resources/web_dev_style/css_checker.py

 # 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.
 
 """Presubmit script for Chromium WebUI resources.
 
 See http://dev.chromium.org/developers/how-tos/depottools/presubmit-scripts
 for more details about the presubmit API built into gcl/git cl, and see
 http://www.chromium.org/developers/web-development-style-guide for the rules
 we're checking against here.
@@ skipping 106 matching lines @@
     def suggest_rgb_from_hex(line):
       suggestions = ['rgb(%d, %d, %d)' % _rgb_from_hex(h.group(1))
           for h in re.finditer(hex_reg, line)]
       return ' (replace with %s)' % ', '.join(suggestions)
 
     def suggest_short_hex(line):
       h = re.search(hex_reg, line).group(1)
       return ' (replace with #%s)' % (h[0] + h[2] + h[4])
 
     hsl = r'hsl\([^\)]*(?:[, ]|(?<=\())(?:0?\.?)?0%'
-    zeros = (r'(?:^|\D)'
+    zeros = (r'^.*(?:^|\D)'

[Yoyo Zhou, 2012/04/20 01:31:40]

It seems like for the example "state0." in the test case, it'd be better to
change this \D to \W, or maybe the whole blob to \b.

              r'(?:\.0|0(?:\.0?|px|em|%|in|cm|mm|pc|pt|ex|deg|g?rad|m?s|k?hz))'
-             r'(?:\D|$)(?!\s*\{)')
-    def zero_length_values(line):
-      return (re.search(zeros, line) and not re.search(hsl, line))
+             r'(?:\D|$)(?=[^{}]+?}).*$')

[Yoyo Zhou, 2012/04/20 01:31:40]

The last } is literal, right? It's confusing without an escape.
Won't this regex just not match anything outside of any curly braces?

+    def zero_length_values(contents):
+      errors = []
+      for z in re.finditer(re.compile(zeros, re.MULTILINE), contents):
+        first_line = z.group(0).strip().splitlines()[0]
+        if not re.search(hsl, first_line):
+          errors.append('    ' + first_line)
+      return errors
 
     added_or_modified_files_checks = [
         { 'desc': 'Alphabetize properties and list vendor specific (i.e. '
                   '-webkit) above standard.',
           'test': alphabetize_props,
           'multiline': True,
         },
         { 'desc': 'Start braces ({) end a selector, have a space before them '
                   'and no rules after.',
           'test': braces_have_space_before_and_nothing_after,
@@ skipping 29 matching lines @@
           'test': one_selector_per_line,
           'multiline': True,
         },
         { 'desc': 'Use rgb() over #hex when not a shade of gray (like #333).',
           'test': rgb_if_not_gray,
           'after': suggest_rgb_from_hex,
         },
         { 'desc': 'Make all zero length terms (i.e. 0px) 0 unless inside of '
                   'hsl() or part of @keyframe.',
           'test': zero_length_values,
+          'multiline': True,
         },
     ]
 
     results = []
     affected_files = self.input_api.AffectedFiles(include_deletes=False,
                                                   file_filter=self.file_filter)
     files = []
     for f in affected_files:
       # Remove all /*comments*/, @at-keywords, and grit <if|include> tags; we're
       # not using a real parser. TODO(dbeam): Check alpha in <if> blocks.
@@ skipping 31 matching lines @@
             '%s:\n%s' % (f[0], '\n\n'.join(file_errors))))
 
     if results:
       # Add your name if you're here often mucking around in the code.
       authors = ['dbeam@chromium.org']
       results.append(self.output_api.PresubmitNotifyResult(
           'Was the CSS checker useful? Send feedback or hate mail to %s.' %
           ', '.join(authors)))
 
     return results