Changeset 53374 in webkit


Ignore:
Timestamp:
Jan 17, 2010 4:04:35 PM (14 years ago)
Author:
eric@webkit.org
Message:

2010-01-17 Chris Jerdonek <Chris Jerdonek>

Reviewed by Adam Barth.

Eliminated the error_count global variable and related
check-webkit-style refactoring.

​https://bugs.webkit.org/show_bug.cgi?id=33678

  • Scripts/check-webkit-style:
    • Updated to use webkit_argument_defaults().
    • Renamed styleChecker to style_checker.
  • Scripts/webkitpy/style/checker.py:
    • Prefixed the three default arguments with WEBKIT_DEFAULT.
    • Added webkit_argument_defaults().
    • Added default filter_rules parameter to CategoryFilter constructor.
    • Added ne() to CategoryFilter class.
    • Added eq() and ne() to ProcessorOptions class.
    • Added error_count and _write_error attributes to StyleChecker class.
    • Made StyleChecker._handle_error() increment the error count.
  • Scripts/webkitpy/style/checker_unittest.py:
    • Improved CategoryFilterTest.test_eq().
    • Added CategoryFilterTest.test_ne().
    • Added test_eq() and test_ne() to ProcessorOptionsTest class.
    • Updated unit tests to use webkit_argument_defaults().
    • Added StyleCheckerTest class.
  • Scripts/webkitpy/style/cpp_style.py:
    • Removed references to global error_count.
  • Scripts/webkitpy/style/cpp_style_unittest.py:
    • Removed CppStyleStateTest class.
Location:
trunk/WebKitTools
Files:
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/WebKitTools/ChangeLog

    r53368 r53374  
     12010-01-17  Chris Jerdonek  <cjerdonek@webkit.org>
     2
     3        Reviewed by Adam Barth.
     4
     5        Eliminated the error_count global variable and related
     6        check-webkit-style refactoring.
     7
     8        https://bugs.webkit.org/show_bug.cgi?id=33678
     9
     10        * Scripts/check-webkit-style:
     11          - Updated to use webkit_argument_defaults().
     12          - Renamed styleChecker to style_checker.
     13
     14        * Scripts/webkitpy/style/checker.py:
     15          - Prefixed the three default arguments with WEBKIT_DEFAULT.
     16          - Added webkit_argument_defaults().
     17          - Added default filter_rules parameter to CategoryFilter constructor.
     18          - Added __ne__() to CategoryFilter class.
     19          - Added __eq__() and __ne__() to ProcessorOptions class.
     20          - Added error_count and _write_error attributes to StyleChecker class.
     21          - Made StyleChecker._handle_error() increment the error count.
     22
     23        * Scripts/webkitpy/style/checker_unittest.py:
     24          - Improved CategoryFilterTest.test_eq().
     25          - Added CategoryFilterTest.test_ne().
     26          - Added test_eq() and test_ne() to ProcessorOptionsTest class.
     27          - Updated unit tests to use webkit_argument_defaults().
     28          - Added StyleCheckerTest class.
     29
     30        * Scripts/webkitpy/style/cpp_style.py:
     31          - Removed references to global error_count.
     32
     33        * Scripts/webkitpy/style/cpp_style_unittest.py:
     34          - Removed CppStyleStateTest class.
     35
    1362010-01-15  Jon Honeycutt  <jhoneycutt@apple.com>
    237

  • trunk/WebKitTools/Scripts/check-webkit-style

    r53251 r53374  
    6060                                           'replace')
    6161
    62     defaults = checker.ArgumentDefaults(checker.DEFAULT_OUTPUT_FORMAT,
    63                                         checker.DEFAULT_VERBOSITY,
    64                                         checker.WEBKIT_FILTER_RULES)
     62    defaults = checker.webkit_argument_defaults()
    6563
    6664    parser = checker.ArgumentParser(defaults)
    6765    (files, options) = parser.parse(sys.argv[1:])
    6866
    69     styleChecker = checker.StyleChecker(options)
     67    style_checker = checker.StyleChecker(options)
    7068
    7169    if files:
    7270        for filename in files:
    73             styleChecker.process_file(filename)
     71            style_checker.process_file(filename)
    7472
    7573    else:
    … …  
    8785        else:
    8886            patch = scm.create_patch()
    89         styleChecker.process_patch(patch)
     87        style_checker.process_patch(patch)
    9088
    91     sys.stderr.write('Total errors found: %d\n' % checker.error_count())
    92     sys.exit(checker.error_count() > 0)
     89    sys.stderr.write('Total errors found: %d\n' % style_checker.error_count)
     90    sys.exit(style_checker.error_count > 0)
    9391
    9492

  • trunk/WebKitTools/Scripts/webkitpy/style/checker.py

    r53251 r53374  
    4242
    4343# These defaults are used by check-webkit-style.
    44 DEFAULT_VERBOSITY = 1
    45 DEFAULT_OUTPUT_FORMAT = 'emacs'
     44WEBKIT_DEFAULT_VERBOSITY = 1
     45WEBKIT_DEFAULT_OUTPUT_FORMAT = 'emacs'
    4646
    4747
    … …  
    5858# rules. Since by default all errors are on, only include rules that
    5959# begin with a - sign.
    60 WEBKIT_FILTER_RULES = [
     60WEBKIT_DEFAULT_FILTER_RULES = [
    6161    '-build/endif_comment',
    6262    '-build/include_what_you_use',  # <string> for std::string
    … …  
    159159
    160160
     161def webkit_argument_defaults():
     162    """Return the DefaultArguments instance for use by check-webkit-style."""
     163    return ArgumentDefaults(WEBKIT_DEFAULT_OUTPUT_FORMAT,
     164                            WEBKIT_DEFAULT_VERBOSITY,
     165                            WEBKIT_DEFAULT_FILTER_RULES)
     166
     167
    161168def _create_usage(defaults):
    162169    """Return the usage string to display for command help.
    … …  
    235242    """Filters whether to check style categories."""
    236243
    237     def __init__(self, filter_rules):
     244    def __init__(self, filter_rules=None):
    238245        """Create a category filter.
    239246
    … …  
    246253                        symbol (+/-). The list should include any
    247254                        default filter rules at the beginning.
     255                        Defaults to the empty list.
    248256
    249257        Raises:
    … …  
    252260
    253261        """
     262        if filter_rules is None:
     263            filter_rules = []
     264
    254265        for rule in filter_rules:
    255266            if not (rule.startswith('+') or rule.startswith('-')):
    … …  
    264275        return ",".join(self._filter_rules)
    265276
     277    # Useful for unit testing.
    266278    def __eq__(self, other):
    267         # This is useful for unit testing.
    268         # Two category filters are the same if and only if their
    269         # constituent filter rules are the same.
    270         return (str(self) == str(other))
     279        """Return whether this CategoryFilter instance is equal to another."""
     280        return self._filter_rules == other._filter_rules
     281
     282    # Useful for unit testing.
     283    def __ne__(self, other):
     284        # Python does not automatically deduce from __eq__().
     285        return not (self == other)
    271286
    272287    def should_check(self, category):
    … …  
    325340                 git_commit=None, extra_flag_values=None):
    326341        if filter is None:
    327             filter = CategoryFilter([])
     342            filter = CategoryFilter()
    328343        if extra_flag_values is None:
    329344            extra_flag_values = {}
    … …  
    344359        self.git_commit = git_commit
    345360        self.extra_flag_values = extra_flag_values
     361
     362    # Useful for unit testing.
     363    def __eq__(self, other):
     364        """Return whether this ProcessorOptions instance is equal to another."""
     365        if self.output_format != other.output_format:
     366            return False
     367        if self.verbosity != other.verbosity:
     368            return False
     369        if self.filter != other.filter:
     370            return False
     371        if self.git_commit != other.git_commit:
     372            return False
     373        if self.extra_flag_values != other.extra_flag_values:
     374            return False
     375
     376        return True
     377
     378    # Useful for unit testing.
     379    def __ne__(self, other):
     380        # Python does not automatically deduce from __eq__().
     381        return not (self == other)
    346382
    347383    def should_report_error(self, category, confidence_in_error):
    … …  
    586622class StyleChecker(object):
    587623
    588     """Supports checking style in files and patches."""
    589 
    590     def __init__(self, options):
     624    """Supports checking style in files and patches.
     625
     626       Attributes:
     627         error_count: An integer that is the total number of reported
     628                      errors for the lifetime of this StyleChecker
     629                      instance.
     630         options: A ProcessorOptions instance that controls the behavior
     631                  of style checking.
     632
     633    """
     634
     635    def __init__(self, options, write_error=sys.stderr.write):
    591636        """Create a StyleChecker instance.
    592637
    593638        Args:
    594           options: A ProcessorOptions instance.
    595 
    596         """
     639          options: See options attribute.
     640          stderr_write: A function that takes a string as a parameter
     641                        and that is called when a style error occurs.
     642
     643        """
     644        self._write_error = write_error
    597645        self.options = options
     646        self.error_count = 0
    598647
    599648        # FIXME: Eliminate the need to set global state here.
    … …  
    601650
    602651    def _handle_error(self, filename, line_number, category, confidence, message):
    603         """Logs the fact we've found a lint error.
    604 
    605         We log the error location and our confidence in the error, i.e.
    606         how certain we are the error is a legitimate style regression
    607         versus a misidentification or justified use.
     652        """Handle the occurrence of a style error while checking.
     653
     654        Check whether an error should be reported. If so, increment the
     655        global error count and report the error details.
    608656
    609657        Args:
    … …  
    624672            return
    625673
    626         # FIXME: Eliminate the need to reference cpp_style here.
    627         cpp_style._cpp_style_state.increment_error_count()
     674        self.error_count += 1
    628675
    629676        if self.options.output_format == 'vs7':
    630             sys.stderr.write('%s(%s):  %s  [%s] [%d]\n' % (
    631                 filename, line_number, message, category, confidence))
     677            format_string = "%s(%s):  %s  [%s] [%d]\n"
    632678        else:
    633             sys.stderr.write('%s:%s:  %s  [%s] [%d]\n' % (
    634                 filename, line_number, message, category, confidence))
     679            format_string = "%s:%s:  %s  [%s] [%d]\n"
     680
     681        self._write_error(format_string % (filename, line_number, message,
     682                                           category, confidence))
    635683
    636684    def process_file(self, filename):
    … …  
    677725            elif text_style.can_handle(filename):
    678726                text_style.process_file(filename, error_for_patch)
    679 
    680 
    681 def error_count():
    682     """Returns the total error count."""
    683     return cpp_style.error_count()

  • trunk/WebKitTools/Scripts/webkitpy/style/checker_unittest.py

    r53251 r53374  
    4040from checker import CategoryFilter
    4141from checker import ProcessorOptions
     42from checker import StyleChecker
    4243
    4344class CategoryFilterTest(unittest.TestCase):
    … …  
    4849        """Test __init__ constructor."""
    4950        self.assertRaises(ValueError, CategoryFilter, ["no_prefix"])
    50         CategoryFilter([]) # No ValueError: works
     51        CategoryFilter() # No ValueError: works
    5152        CategoryFilter(["+"]) # No ValueError: works
    5253        CategoryFilter(["-"]) # No ValueError: works
    … …  
    5859
    5960    def test_eq(self):
    60         """Test __eq__ equality operator."""
     61        """Test __eq__ equality function."""
    6162        filter1 = CategoryFilter(["+a", "+b"])
    6263        filter2 = CategoryFilter(["+a", "+b"])
    6364        filter3 = CategoryFilter(["+b", "+a"])
    64         self.assertEquals(filter1, filter2)
    65         self.assertNotEquals(filter1, filter3)
     65
     66        # == calls __eq__.
     67        self.assertTrue(filter1 == filter2)
     68        self.assertFalse(filter1 == filter3) # Cannot test with assertNotEqual.
     69
     70    def test_ne(self):
     71        """Test __ne__ inequality function."""
     72        # != calls __ne__.
     73        # By default, __ne__ always returns true on different objects.
     74        # Thus, just check the distinguishing case to verify that the
     75        # code defines __ne__.
     76        self.assertFalse(CategoryFilter() != CategoryFilter())
    6677
    6778    def test_should_check(self):
    6879        """Test should_check() method."""
    69         filter = CategoryFilter([])
     80        filter = CategoryFilter()
    7081        self.assertTrue(filter.should_check("everything"))
    7182        # Check a second time to exercise cache.
    … …  
    95106        options = ProcessorOptions()
    96107        self.assertEquals(options.extra_flag_values, {})
    97         self.assertEquals(options.filter, CategoryFilter([]))
     108        self.assertEquals(options.filter, CategoryFilter())
    98109        self.assertEquals(options.git_commit, None)
    99110        self.assertEquals(options.output_format, "emacs")
    100111        self.assertEquals(options.verbosity, 1)
    101112
     113        # Check argument validation.
    102114        self.assertRaises(ValueError, ProcessorOptions, output_format="bad")
    103115        ProcessorOptions(output_format="emacs") # No ValueError: works
    … …  
    120132        self.assertEquals(options.verbosity, 3)
    121133
     134    def test_eq(self):
     135        """Test __eq__ equality function."""
     136        # == calls __eq__.
     137        self.assertTrue(ProcessorOptions() == ProcessorOptions())
     138
     139        # Verify that a difference in any argument cause equality to fail.
     140        options = ProcessorOptions(extra_flag_values={"extra_value" : 1},
     141                                   filter=CategoryFilter(["+"]),
     142                                   git_commit="commit",
     143                                   output_format="vs7",
     144                                   verbosity=1)
     145        self.assertFalse(options == ProcessorOptions(extra_flag_values={"extra_value" : 2}))
     146        self.assertFalse(options == ProcessorOptions(filter=CategoryFilter(["-"])))
     147        self.assertFalse(options == ProcessorOptions(git_commit="commit2"))
     148        self.assertFalse(options == ProcessorOptions(output_format="emacs"))
     149        self.assertFalse(options == ProcessorOptions(verbosity=2))
     150
     151    def test_ne(self):
     152        """Test __ne__ inequality function."""
     153        # != calls __ne__.
     154        # By default, __ne__ always returns true on different objects.
     155        # Thus, just check the distinguishing case to verify that the
     156        # code defines __ne__.
     157        self.assertFalse(ProcessorOptions() != ProcessorOptions())
     158
    122159    def test_should_report_error(self):
    123160        """Test should_report_error()."""
    … …  
    134171
    135172
    136 class DefaultArgumentsTest(unittest.TestCase):
     173class WebKitArgumentDefaultsTest(unittest.TestCase):
    137174
    138175    """Tests validity of default arguments used by check-webkit-style."""
    139176
     177    def defaults(self):
     178        return style.webkit_argument_defaults()
     179
    140180    def test_filter_rules(self):
     181        defaults = self.defaults()
    141182        already_seen = []
    142         for rule in style.WEBKIT_FILTER_RULES:
     183        for rule in defaults.filter_rules:
    143184            # Check no leading or trailing white space.
    144185            self.assertEquals(rule, rule.strip())
    … …  
    147188            self.assertTrue(rule.startswith('-'))
    148189            self.assertTrue(rule[1:] in style.STYLE_CATEGORIES)
     190            # Check no rule occurs twice.
    149191            self.assertFalse(rule in already_seen)
    150192            already_seen.append(rule)
    … …  
    152194    def test_defaults(self):
    153195        """Check that default arguments are valid."""
    154         defaults = style.ArgumentDefaults(style.DEFAULT_OUTPUT_FORMAT,
    155                                           style.DEFAULT_VERBOSITY,
    156                                           style.WEBKIT_FILTER_RULES)
     196        defaults = self.defaults()
     197
    157198        # FIXME: We should not need to call parse() to determine
    158199        #        whether the default arguments are valid.
    … …  
    312353
    313354
     355class StyleCheckerTest(unittest.TestCase):
     356
     357    """Test the StyleChecker class.
     358
     359    Attributes:
     360      error_message: A string that is the last error message reported
     361                     by the test StyleChecker instance.
     362
     363    """
     364
     365    error_message = ""
     366
     367    def mock_write_error(self, error_message):
     368        """Store an error message without printing it."""
     369        self.error_message = error_message
     370        pass
     371
     372    def style_checker(self, options):
     373        return StyleChecker(options, self.mock_write_error)
     374
     375    def test_init(self):
     376        """Test __init__ constructor."""
     377        options = ProcessorOptions()
     378        style_checker = self.style_checker(options)
     379
     380        self.assertEquals(style_checker.error_count, 0)
     381        self.assertEquals(style_checker.options, options)
     382
     383    def write_sample_error(self, style_checker, error_confidence):
     384        """Write an error to the given style_checker."""
     385        style_checker._handle_error(filename="filename",
     386                                    line_number=1,
     387                                    category="category",
     388                                    confidence=error_confidence,
     389                                    message="message")
     390
     391    def test_handle_error(self):
     392        """Test _handler_error() function."""
     393        options = ProcessorOptions(output_format="emacs",
     394                                   verbosity=3)
     395        style_checker = self.style_checker(options)
     396
     397        # Verify initialized properly.
     398        self.assertEquals(style_checker.error_count, 0)
     399        self.assertEquals(self.error_message, "")
     400
     401        # Check that should_print_error is getting called appropriately.
     402        self.write_sample_error(style_checker, 2)
     403        self.assertEquals(style_checker.error_count, 0) # Error confidence too low.
     404        self.assertEquals(self.error_message, "")
     405
     406        self.write_sample_error(style_checker, 3)
     407        self.assertEquals(style_checker.error_count, 1) # Error confidence just high enough.
     408        self.assertEquals(self.error_message, "filename:1:  message  [category] [3]\n")
     409
     410        # Check "vs7" output format.
     411        style_checker.options.output_format = "vs7"
     412        self.write_sample_error(style_checker, 3)
     413        self.assertEquals(style_checker.error_count, 2) # Error confidence just high enough.
     414        self.assertEquals(self.error_message, "filename(1):  message  [category] [3]\n")
     415
     416
    314417if __name__ == '__main__':
    315418    import sys

  • trunk/WebKitTools/Scripts/webkitpy/style/cpp_style.py

    r53251 r53374  
    248248    def __init__(self):
    249249        self.verbose_level = 1  # global setting.
    250         self.error_count = 0    # global count of reported errors
    251250
    252251    def set_verbose_level(self, level):
    … …  
    256255        return last_verbose_level
    257256
    258     def reset_error_count(self):
    259         """Sets the module's error statistic back to zero."""
    260         self.error_count = 0
    261 
    262     def increment_error_count(self):
    263         """Bumps the module's error statistic."""
    264         self.error_count += 1
    265 
    266257
    267258_cpp_style_state = _CppStyleState()
    … …  
    276267    """Sets the module's verbosity, and returns the previous setting."""
    277268    return _cpp_style_state.set_verbose_level(level)
    278 
    279 
    280 def error_count():
    281     """Returns the global count of reported errors."""
    282     return _cpp_style_state.error_count
    283269
    284270

  • trunk/WebKitTools/Scripts/webkitpy/style/cpp_style_unittest.py

    r53251 r53374  
    26202620
    26212621
    2622 class CppStyleStateTest(unittest.TestCase):
    2623     def test_error_count(self):
    2624         self.assertEquals(0, cpp_style.error_count())
    2625         cpp_style._cpp_style_state.increment_error_count()
    2626         cpp_style._cpp_style_state.increment_error_count()
    2627         self.assertEquals(2, cpp_style.error_count())
    2628         cpp_style._cpp_style_state.reset_error_count()
    2629         self.assertEquals(0, cpp_style.error_count())
    2630 
    2631 
    26322622class WebKitStyleTest(CppStyleTestBase):
    26332623
Note: See TracChangeset for help on using the changeset viewer.