Changeset 54206 in webkit


Ignore:
Timestamp:
Feb 1, 2010 10:41:28 PM (14 years ago)
Author:
Chris Jerdonek
Message:

Addressed FIXME in check-webkit-style so that the carriage-return
check will work for patches.

Reviewed by Shinichiro Hamaji.

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

Also added support for limiting the number of errors reported
per category, per file.

  • Scripts/webkitpy/style/checker.py:
    • Added new "whitespace/carriage_return" category from common.py.
    • Added MAX_REPORTS_PER_CATEGORY dictionary.
    • Added max_reports_per_category attribute to ProcessorOptions class.
    • Refactored StyleChecker._process_file().
  • Scripts/webkitpy/style/checker_unittest.py:
    • Updated ProcessorOptionsTest tests.
    • Added test to check MAX_REPORTS_PER_CATEGORY.
  • Scripts/webkitpy/style/error_handlers.py:
    • Added support for suppressing the display of errors after reaching a per-category maximum (from max_reports_per_category).
  • Scripts/webkitpy/style/error_handlers_unittest.py:
    • Added test for suppressing error display.
  • Scripts/webkitpy/style/processors/common.py: Added.
    • Moved carriage-return check to new file.
  • Scripts/webkitpy/style/processors/common_unittest.py: Added.
    • Added unit tests for carriage-return check.
  • Scripts/webkitpy/style/unittests.py:
    • Added reference to common_unittest.py.
Location:
trunk/WebKitTools
Files:
2 added
6 edited

Legend:

Unmodified
Added
Removed
  • trunk/WebKitTools/ChangeLog

    r54205 r54206  
     12010-02-01  Chris Jerdonek  <cjerdonek@webkit.org>
     2
     3        Reviewed by Shinichiro Hamaji.
     4
     5        Addressed FIXME in check-webkit-style so that the carriage-return
     6        check will work for patches.
     7
     8        https://bugs.webkit.org/show_bug.cgi?id=34260
     9
     10        Also added support for limiting the number of errors reported
     11        per category, per file.
     12
     13        * Scripts/webkitpy/style/checker.py:
     14          - Added new "whitespace/carriage_return" category from common.py.
     15          - Added MAX_REPORTS_PER_CATEGORY dictionary.
     16          - Added max_reports_per_category attribute to ProcessorOptions class.
     17          - Refactored StyleChecker._process_file().
     18
     19        * Scripts/webkitpy/style/checker_unittest.py:
     20          - Updated ProcessorOptionsTest tests.
     21          - Added test to check MAX_REPORTS_PER_CATEGORY.
     22
     23        * Scripts/webkitpy/style/error_handlers.py:
     24          - Added support for suppressing the display of errors after
     25            reaching a per-category maximum (from max_reports_per_category).
     26
     27        * Scripts/webkitpy/style/error_handlers_unittest.py:
     28          - Added test for suppressing error display.
     29
     30        * Scripts/webkitpy/style/processors/common.py: Added.
     31          - Moved carriage-return check to new file.
     32
     33        * Scripts/webkitpy/style/processors/common_unittest.py: Added.
     34          - Added unit tests for carriage-return check.
     35
     36        * Scripts/webkitpy/style/unittests.py:
     37          - Added reference to common_unittest.py.
     38
    1392010-02-01  Shinichiro Hamaji  <hamaji@chromium.org>
    240
  • trunk/WebKitTools/Scripts/webkitpy/style/checker.py

    r54192 r54206  
    3838from error_handlers import DefaultStyleErrorHandler
    3939from error_handlers import PatchStyleErrorHandler
     40from processors.common import check_no_carriage_return
     41from processors.common import categories as CommonCategories
    4042from processors.cpp import CppProcessor
    4143from processors.text import TextProcessor
     
    107109
    108110
     111# FIXME: Check that the keys are in _style_categories().
     112#
     113# The maximum number of errors to report per file, per category.
     114# If a category is not a key, then it has no maximum.
     115MAX_REPORTS_PER_CATEGORY = {
     116    "whitespace/carriage_return": 1
     117}
     118
     119
    109120def style_categories():
    110121    """Return the set of all categories used by check-webkit-style."""
    111     # If other processors had categories, we would take their union here.
    112     return CppProcessor.categories
     122    # Take the union across all processors.
     123    return CommonCategories.union(CppProcessor.categories)
    113124
    114125
     
    291302    """
    292303
    293     def __init__(self, output_format="emacs", verbosity=1, filter=None,
    294                  git_commit=None, extra_flag_values=None):
     304    def __init__(self,
     305                 output_format="emacs",
     306                 verbosity=1,
     307                 filter=None,
     308                 max_reports_per_category=None,
     309                 git_commit=None,
     310                 extra_flag_values=None):
     311        if extra_flag_values is None:
     312            extra_flag_values = {}
    295313        if filter is None:
    296314            filter = CategoryFilter()
    297         if extra_flag_values is None:
    298             extra_flag_values = {}
     315        if max_reports_per_category is None:
     316            max_reports_per_category = {}
    299317
    300318        if output_format not in ("emacs", "vs7"):
     
    308326                             'Value given: "%s".' % verbosity)
    309327
     328        self.extra_flag_values = extra_flag_values
     329        self.filter = filter
     330        self.git_commit = git_commit
     331        self.max_reports_per_category = max_reports_per_category
    310332        self.output_format = output_format
    311333        self.verbosity = verbosity
    312         self.filter = filter
    313         self.git_commit = git_commit
    314         self.extra_flag_values = extra_flag_values
    315334
    316335    # Useful for unit testing.
    317336    def __eq__(self, other):
    318337        """Return whether this ProcessorOptions instance is equal to another."""
    319         if self.output_format != other.output_format:
    320             return False
    321         if self.verbosity != other.verbosity:
     338        if self.extra_flag_values != other.extra_flag_values:
    322339            return False
    323340        if self.filter != other.filter:
     
    325342        if self.git_commit != other.git_commit:
    326343            return False
    327         if self.extra_flag_values != other.extra_flag_values:
     344        if self.max_reports_per_category != other.max_reports_per_category:
     345            return False
     346        if self.output_format != other.output_format:
     347            return False
     348        if self.verbosity != other.verbosity:
    328349            return False
    329350
     
    569590        filter = CategoryFilter(filter_rules)
    570591
    571         options = ProcessorOptions(output_format, verbosity, filter,
    572                                    git_commit, extra_flag_values)
     592        options = ProcessorOptions(extra_flag_values=extra_flag_values,
     593                      filter=filter,
     594                      git_commit=git_commit,
     595                      max_reports_per_category=MAX_REPORTS_PER_CATEGORY,
     596                      output_format=output_format,
     597                      verbosity=verbosity)
    573598
    574599        return (filenames, options)
     
    721746            # is processed.
    722747            if file_path == '-':
    723                 lines = codecs.StreamReaderWriter(sys.stdin,
    724                                                   codecs.getreader('utf8'),
    725                                                   codecs.getwriter('utf8'),
    726                                                   'replace').read().split('\n')
     748                file = codecs.StreamReaderWriter(sys.stdin,
     749                                                 codecs.getreader('utf8'),
     750                                                 codecs.getwriter('utf8'),
     751                                                 'replace')
    727752            else:
    728                 lines = codecs.open(file_path, 'r', 'utf8', 'replace').read().split('\n')
    729 
    730             carriage_return_found = False
    731             # Remove trailing '\r'.
    732             for line_number in range(len(lines)):
    733                 if lines[line_number].endswith('\r'):
    734                     lines[line_number] = lines[line_number].rstrip('\r')
    735                     carriage_return_found = True
     753                file = codecs.open(file_path, 'r', 'utf8', 'replace')
     754
     755            contents = file.read()
    736756
    737757        except IOError:
     
    739759            return
    740760
     761        lines = contents.split("\n")
     762
     763        for line_number in range(len(lines)):
     764            # FIXME: We should probably use the SVN "eol-style" property
     765            #        or a white list to decide whether or not to do
     766            #        the carriage-return check. Originally, we did the
     767            #        check only if (os.linesep != '\r\n').
     768            #
     769            # FIXME: As a minor optimization, we can have
     770            #        check_no_carriage_return() return whether
     771            #        the line ends with "\r".
     772            check_no_carriage_return(lines[line_number], line_number,
     773                                     handle_style_error)
     774            if lines[line_number].endswith("\r"):
     775                lines[line_number] = lines[line_number].rstrip("\r")
     776
    741777        processor.process(lines)
    742 
    743         if carriage_return_found and os.linesep != '\r\n':
    744             # FIXME: Make sure this error also shows up when checking
    745             #        patches, if appropriate.
    746             #
    747             # Use 0 for line_number since outputting only one error for
    748             # potentially several lines.
    749             handle_style_error(0, 'whitespace/newline', 1,
    750                                'One or more unexpected \\r (^M) found;'
    751                                'better to use only a \\n')
    752778
    753779    def check_file(self, file_path, handle_style_error=None, process_file=None):
  • trunk/WebKitTools/Scripts/webkitpy/style/checker_unittest.py

    r54126 r54206  
    111111        self.assertEquals(options.filter, CategoryFilter())
    112112        self.assertEquals(options.git_commit, None)
     113        self.assertEquals(options.max_reports_per_category, {})
    113114        self.assertEquals(options.output_format, "emacs")
    114115        self.assertEquals(options.verbosity, 1)
     
    127128                                   filter=CategoryFilter(["+"]),
    128129                                   git_commit="commit",
     130                                   max_reports_per_category={"category": 3},
    129131                                   output_format="vs7",
    130132                                   verbosity=3)
     
    132134        self.assertEquals(options.filter, CategoryFilter(["+"]))
    133135        self.assertEquals(options.git_commit, "commit")
     136        self.assertEquals(options.max_reports_per_category, {"category": 3})
    134137        self.assertEquals(options.output_format, "vs7")
    135138        self.assertEquals(options.verbosity, 3)
     
    144147                                   filter=CategoryFilter(["+"]),
    145148                                   git_commit="commit",
     149                                   max_reports_per_category={"category": 3},
    146150                                   output_format="vs7",
    147151                                   verbosity=1)
     
    149153        self.assertFalse(options == ProcessorOptions(filter=CategoryFilter(["-"])))
    150154        self.assertFalse(options == ProcessorOptions(git_commit="commit2"))
     155        self.assertFalse(options == ProcessorOptions(max_reports_per_category=
     156                                                     {"category": 2}))
    151157        self.assertFalse(options == ProcessorOptions(output_format="emacs"))
    152158        self.assertFalse(options == ProcessorOptions(verbosity=2))
     
    174180
    175181
    176 class WebKitArgumentDefaultsTest(unittest.TestCase):
    177 
    178     """Tests validity of default arguments used by check-webkit-style."""
     182class GlobalVariablesTest(unittest.TestCase):
     183
     184    """Tests validity of the global variables."""
    179185
    180186    def defaults(self):
     
    207213        parser.parse([]) # arguments valid: no error or SystemExit
    208214
     215    def test_max_reports_per_category(self):
     216        """Check that MAX_REPORTS_PER_CATEGORY is valid."""
     217        categories = style.style_categories()
     218        for category in style.MAX_REPORTS_PER_CATEGORY.iterkeys():
     219            self.assertTrue(category in categories,
     220                            'Key "%s" is not a category' % category)
     221
    209222
    210223class ArgumentPrinterTest(unittest.TestCase):
     
    218231                        extra_flag_values={}):
    219232        filter = CategoryFilter(filter_rules)
    220         return style.ProcessorOptions(output_format, verbosity, filter,
    221                                       git_commit, extra_flag_values)
     233        return style.ProcessorOptions(extra_flag_values=extra_flag_values,
     234                                      filter=filter,
     235                                      git_commit=git_commit,
     236                                      output_format=output_format,
     237                                      verbosity=verbosity)
    222238
    223239    def test_to_flag_string(self):
  • trunk/WebKitTools/Scripts/webkitpy/style/error_handlers.py

    r54126 r54206  
    3333    Handle the occurrence of a style error.
    3434
    35     Check whether the error is reportable. If so, report the details.
     35    Check whether the error is reportable. If so, increment the total
     36    error count and report the details. Note that error reporting can
     37    be suppressed after reaching a certain number of reports.
    3638
    3739    Args:
     
    8082        self._stderr_write = stderr_write
    8183
     84        # A string to integer dictionary cache of the number of reportable
     85        # errors per category passed to this instance.
     86        self._category_totals = { }
     87
     88    def _add_reportable_error(self, category):
     89        """Increment the error count and return the new category total."""
     90        self._increment_error_count() # Increment the total.
     91
     92        # Increment the category total.
     93        if not category in self._category_totals:
     94            self._category_totals[category] = 1
     95        else:
     96            self._category_totals[category] += 1
     97
     98        return self._category_totals[category]
     99
     100    def _max_reports(self, category):
     101        """Return the maximum number of errors to report."""
     102        if not category in self._options.max_reports_per_category:
     103            return None
     104        return self._options.max_reports_per_category[category]
     105
    82106    def __call__(self, line_number, category, confidence, message):
    83107        """Handle the occurrence of a style error.
     
    89113            return
    90114
    91         self._increment_error_count()
     115        category_total = self._add_reportable_error(category)
     116
     117        max_reports = self._max_reports(category)
     118
     119        if (max_reports is not None) and (category_total > max_reports):
     120            # Then suppress displaying the error.
     121            return
    92122
    93123        if self._options.output_format == 'vs7':
     
    95125        else:
    96126            format_string = "%s:%s:  %s  [%s] [%d]\n"
     127
     128        if category_total == max_reports:
     129            format_string += ("Suppressing further [%s] reports for this "
     130                              "file.\n" % category)
    97131
    98132        self._stderr_write(format_string % (self._file_path,
     
    131165            for line in self._diff.lines:
    132166                # When deleted line is not set, it means that
    133                 # the line is newly added.
     167                # the line is newly added (or modified).
    134168                if not line[0]:
    135169                    self._line_numbers.add(line[1])
     
    141175
    142176        This function does not report errors occurring in lines not
    143         modified or added.
     177        marked as modified or added in the patch.
    144178
    145         Args: see the DefaultStyleErrorHandler.__call__() documentation.
     179        See the docstring of this module for more information.
    146180
    147181        """
  • trunk/WebKitTools/Scripts/webkitpy/style/error_handlers_unittest.py

    r54126 r54206  
    5151    _category = "whitespace/tab"
    5252
    53     def _options(self, output_format):
    54         return ProcessorOptions(verbosity=3, output_format=output_format)
    55 
    5653    def _error_handler(self, options):
    5754        file_path = "foo.h"
     
    6158                                        self._mock_stderr_write)
    6259
    63     def _prepare_call(self, output_format="emacs"):
    64         """Return options after initializing."""
    65         options = self._options(output_format)
    66 
    67         # Test that count is initialized to zero.
     60    def _check_initialized(self):
     61        """Check that count and error messages are initialized."""
    6862        self.assertEquals(0, self._error_count)
    6963        self.assertEquals("", self._error_messages)
    7064
    71         return options
    72 
    73     def _call_error_handler(self, options, confidence):
    74         """Handle an error with given confidence."""
    75         handle_error = self._error_handler(options)
    76 
     65    def _call(self, handle_error, options, confidence):
     66        """Handle an error with the given error handler."""
    7767        line_number = 100
    7868        message = "message"
     
    8070        handle_error(line_number, self._category, confidence, message)
    8171
     72    def _call_error_handler(self, options, confidence):
     73        """Handle an error using a new error handler."""
     74        handle_error = self._error_handler(options)
     75        self._call(handle_error, options, confidence)
     76
    8277    def test_call_non_reportable(self):
    8378        """Test __call__() method with a non-reportable error."""
    8479        confidence = 1
    85         options = self._prepare_call()
     80        options = ProcessorOptions(verbosity=3)
     81        self._check_initialized()
    8682
    8783        # Confirm the error is not reportable.
     
    9692        """Test __call__() method with a reportable error and emacs format."""
    9793        confidence = 5
    98         options = self._prepare_call("emacs")
     94        options = ProcessorOptions(verbosity=3, output_format="emacs")
     95        self._check_initialized()
    9996
    10097        self._call_error_handler(options, confidence)
     
    107104        """Test __call__() method with a reportable error and vs7 format."""
    108105        confidence = 5
    109         options = self._prepare_call("vs7")
     106        options = ProcessorOptions(verbosity=3, output_format="vs7")
     107        self._check_initialized()
    110108
    111109        self._call_error_handler(options, confidence)
     
    114112        self.assertEquals(self._error_messages,
    115113                          "foo.h(100):  message  [whitespace/tab] [5]\n")
     114
     115    def test_call_max_reports_per_category(self):
     116        """Test error report suppression in __call__() method."""
     117        confidence = 5
     118        options = ProcessorOptions(verbosity=3,
     119                                   max_reports_per_category={self._category: 2})
     120        error_handler = self._error_handler(options)
     121
     122        self._check_initialized()
     123
     124        # First call: usual reporting.
     125        self._call(error_handler, options, confidence)
     126        self.assertEquals(1, self._error_count)
     127        self.assertEquals(self._error_messages,
     128                          "foo.h:100:  message  [whitespace/tab] [5]\n")
     129
     130        # Second call: suppression message reported.
     131        self._error_messages = ""
     132        self._call(error_handler, options, confidence)
     133        self.assertEquals(2, self._error_count)
     134        self.assertEquals(self._error_messages,
     135                          "foo.h:100:  message  [whitespace/tab] [5]\n"
     136                          "Suppressing further [%s] reports for this file.\n"
     137                          % self._category)
     138
     139        # Third call: no report.
     140        self._error_messages = ""
     141        self._call(error_handler, options, confidence)
     142        self.assertEquals(3, self._error_count)
     143        self.assertEquals(self._error_messages, "")
    116144
    117145
  • trunk/WebKitTools/Scripts/webkitpy/style/unittests.py

    r54126 r54206  
    3838from checker_unittest import *
    3939from error_handlers_unittest import *
     40from processors.common_unittest import *
    4041from processors.cpp_unittest import *
    4142from processors.text_unittest import *
Note: See TracChangeset for help on using the changeset viewer.