Changeset 58401 in webkit


Ignore:
Timestamp:
Apr 28, 2010 4:19:41 AM (14 years ago)
Author:
eric@webkit.org
Message:

2010-04-28 Chris Jerdonek <Chris Jerdonek>

Reviewed by Shinichiro Hamaji.

Adjusted check-webkit-style so that files with file type NONE
are automatically skipped without warning.

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

This change simplifies configuring which files to skip. It also
addresses an issue whereby check-webkit-style was unintentionally
checking .vcproj files for carriage returns.

  • Scripts/webkitpy/style/checker.py:
    • Moved the C++, Python, and text file extensions to new module-level configuration variables.
    • Removed .pyc from the _SKIPPED_FILES_WITHOUT_WARNING configuration variable.
    • Changed the numeric values of the FileType enum so that FileType.NONE evaluates to False.
    • For ProcessorDispatcher.should_skip_without_warning():
      • Changed the method to return True for FileType.NONE files.
      • Made ChangeLog files an exception to getting skipped.
    • Changed the StyleProcessor.process() method to raise an exception if given a file path that should not be processed.
  • Scripts/webkitpy/style/checker_unittest.py:
    • Updated the unit tests and added more test cases as necessary.
Location:
trunk/WebKitTools
Files:
3 edited

Legend:

Unmodified
Added
Removed

  • trunk/WebKitTools/ChangeLog

    r58397 r58401  
     12010-04-28  Chris Jerdonek  <cjerdonek@webkit.org>
     2
     3        Reviewed by Shinichiro Hamaji.
     4
     5        Adjusted check-webkit-style so that files with file type NONE
     6        are automatically skipped without warning.
     7
     8        https://bugs.webkit.org/show_bug.cgi?id=38197
     9
     10        This change simplifies configuring which files to skip.  It also
     11        addresses an issue whereby check-webkit-style was unintentionally
     12        checking .vcproj files for carriage returns.
     13
     14        * Scripts/webkitpy/style/checker.py:
     15          - Moved the C++, Python, and text file extensions to new
     16            module-level configuration variables.
     17          - Removed .pyc from the _SKIPPED_FILES_WITHOUT_WARNING configuration
     18            variable.
     19          - Changed the numeric values of the FileType enum so that
     20            FileType.NONE evaluates to False.
     21          - For ProcessorDispatcher.should_skip_without_warning():
     22            - Changed the method to return True for FileType.NONE files.
     23            - Made ChangeLog files an exception to getting skipped.
     24          - Changed the StyleProcessor.process() method to raise an
     25            exception if given a file path that should not be processed.
     26
     27        * Scripts/webkitpy/style/checker_unittest.py:
     28          - Updated the unit tests and added more test cases as necessary.
     29
    1302010-04-28  Eric Seidel  <eric@webkit.org>
    231

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

    r58263 r58401  
    158158
    159159
     160_CPP_FILE_EXTENSIONS = [
     161    'c',
     162    'cpp',
     163    'h',
     164    ]
     165
     166_PYTHON_FILE_EXTENSION = 'py'
     167
     168# FIXME: Include 'vcproj' files as text files after creating a mechanism
     169#        for exempting them from the carriage-return checker (since they
     170#        are Windows-only files).
     171_TEXT_FILE_EXTENSIONS = [
     172    'ac',
     173    'cc',
     174    'cgi',
     175    'css',
     176    'exp',
     177    'flex',
     178    'gyp',
     179    'gypi',
     180    'html',
     181    'idl',
     182    'in',
     183    'js',
     184    'mm',
     185    'php',
     186    'pl',
     187    'pm',
     188    'pri',
     189    'pro',
     190    'rb',
     191    'sh',
     192    'txt',
     193#   'vcproj',  # See FIXME above.
     194    'wm',
     195    'xhtml',
     196    'y',
     197    ]
     198
     199
     200# Files to skip that are less obvious.
     201#
    160202# Some files should be skipped when checking style. For example,
    161203# WebKit maintains some files in Mozilla style on purpose to ease
    162204# future merges.
    163 #
    164 # Include a warning for skipped files that are less obvious.
    165205_SKIPPED_FILES_WITH_WARNING = [
    166206    # The Qt API and tests do not follow WebKit style.
    … …  
    175215
    176216
    177 # Don't include a warning for skipped files that are more common
    178 # and more obvious.
     217# Files to skip that are more common or obvious.
     218#
     219# This list should be in addition to files with FileType.NONE.  Files
     220# with FileType.NONE are automatically skipped without warning.
    179221_SKIPPED_FILES_WITHOUT_WARNING = [
    180222    "LayoutTests/",
    181     ".pyc",
    182223    ]
    183224
    … …  
    330371class FileType:
    331372
    332     NONE = 1
     373    NONE = 0  # FileType.NONE evaluates to False.
    333374    # Alphabetize remaining types
    334     CPP = 2
    335     PYTHON = 3
    336     TEXT = 4
     375    CPP = 1
     376    PYTHON = 2
     377    TEXT = 3
    337378
    338379
    … …  
    348389    """Supports determining whether and how to check style, based on path."""
    349390
    350     cpp_file_extensions = (
    351         'c',
    352         'cpp',
    353         'h',
    354         )
    355 
    356     text_file_extensions = (
    357         'css',
    358         'html',
    359         'idl',
    360         'js',
    361         'mm',
    362         'php',
    363         'pm',
    364         'txt',
    365         )
    366 
    367391    def _file_extension(self, file_path):
    368392        """Return the file extension without the leading dot."""
    … …  
    378402    def should_skip_without_warning(self, file_path):
    379403        """Return whether the given file should be skipped without a warning."""
     404        if not self._file_type(file_path):  # FileType.NONE.
     405            return True
     406        # Since "LayoutTests" is in _SKIPPED_FILES_WITHOUT_WARNING, make
     407        # an exception to prevent files like "LayoutTests/ChangeLog" and
     408        # "LayoutTests/ChangeLog-2009-06-16" from being skipped.
     409        #
     410        # FIXME: Figure out a good way to avoid having to add special logic
     411        #        for this special case.
     412        if os.path.basename(file_path).startswith('ChangeLog'):
     413            return False
    380414        for skipped_file in _SKIPPED_FILES_WITHOUT_WARNING:
    381415            if file_path.find(skipped_file) >= 0:
    … …  
    387421        file_extension = self._file_extension(file_path)
    388422
    389         if (file_extension in self.cpp_file_extensions) or (file_path == '-'):
     423        if (file_extension in _CPP_FILE_EXTENSIONS) or (file_path == '-'):
    390424            # FIXME: Do something about the comment below and the issue it
    391425            #        raises since cpp_style already relies on the extension.
    … …  
    395429            # the extension.
    396430            return FileType.CPP
    397         elif file_extension == "py":
     431        elif file_extension == _PYTHON_FILE_EXTENSION:
    398432            return FileType.PYTHON
    399         elif ("ChangeLog" in file_path or
     433        elif (os.path.basename(file_path).startswith('ChangeLog') or
    400434              (not file_extension and "WebKitTools/Scripts/" in file_path) or
    401               file_extension in self.text_file_extensions):
     435              file_extension in _TEXT_FILE_EXTENSIONS):
    402436            return FileType.TEXT
    403437        else:
    404             # FIXME: If possible, change this method to default to
    405             #        returning FileType.TEXT.  The should_process() method
    406             #        should really encapsulate which files not to check.
    407             #        Currently, "skip" logic is spread between both this
    408             #        method and should_process.
    409438            return FileType.NONE
    410439
    … …  
    624653    def should_process(self, file_path):
    625654        """Return whether the file should be checked for style."""
    626 
    627655        if self._dispatcher.should_skip_without_warning(file_path):
    628656            return False
    … …  
    672700
    673701        if checker is None:
    674             # FIXME: Should we really be skipping files that return True
    675             #        for should_process()?  Perhaps this should be a
    676             #        warning or an exception so we can find out if
    677             #        should_process() is missing any files.
    678             _log.debug('File not a recognized type to check. Skipping: "%s"'
    679                        % file_path)
    680             return
     702            raise AssertionError("File should not be checked: '%s'" % file_path)
    681703
    682704        _log.debug("Using class: " + checker.__class__.__name__)

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

    r58263 r58401  
    273273    """Tests the "should skip" methods of the ProcessorDispatcher class."""
    274274
     275    def setUp(self):
     276        self._dispatcher = ProcessorDispatcher()
     277
    275278    def test_should_skip_with_warning(self):
    276279        """Test should_skip_with_warning()."""
    277         dispatcher = ProcessorDispatcher()
    278 
    279280        # Check a non-skipped file.
    280         self.assertFalse(dispatcher.should_skip_with_warning("foo.txt"))
     281        self.assertFalse(self._dispatcher.should_skip_with_warning("foo.txt"))
    281282
    282283        # Check skipped files.
    … …  
    293294
    294295        for path in paths_to_skip:
    295             self.assertTrue(dispatcher.should_skip_with_warning(path),
     296            self.assertTrue(self._dispatcher.should_skip_with_warning(path),
    296297                            "Checking: " + path)
    297298
    298     def test_should_skip_without_warning(self):
    299         """Test should_skip_without_warning()."""
    300         dispatcher = ProcessorDispatcher()
    301 
    302         # Check a non-skipped file.
    303         self.assertFalse(dispatcher.should_skip_without_warning("foo.txt"))
    304 
    305         # Check skipped files.
    306         paths_to_skip = [
    307            # LayoutTests folder
    308            "LayoutTests/foo.txt",
    309             ]
    310 
    311         for path in paths_to_skip:
    312             self.assertTrue(dispatcher.should_skip_without_warning(path),
    313                             "Checking: " + path)
     299    def _assert_should_skip_without_warning(self, path, is_checker_none,
     300                                            expected):
     301        # Check the file type before asserting the return value.
     302        checker = self._dispatcher.dispatch_processor(file_path=path,
     303                                                      handle_style_error=None,
     304                                                      min_confidence=3)
     305        message = 'while checking: %s' % path
     306        self.assertEquals(checker is None, is_checker_none, message)
     307        self.assertEquals(self._dispatcher.should_skip_without_warning(path),
     308                          expected, message)
     309
     310    def test_should_skip_without_warning__true(self):
     311        """Test should_skip_without_warning() for True return values."""
     312        # Check a file with NONE file type.
     313        path = 'foo.asdf'  # Non-sensical file extension.
     314        self._assert_should_skip_without_warning(path,
     315                                                 is_checker_none=True,
     316                                                 expected=True)
     317
     318        # Check files with non-NONE file type.  These examples must be
     319        # drawn from the _SKIPPED_FILES_WITHOUT_WARNING configuration
     320        # variable.
     321        path = os.path.join('LayoutTests', 'foo.txt')
     322        self._assert_should_skip_without_warning(path,
     323                                                 is_checker_none=False,
     324                                                 expected=True)
     325
     326    def test_should_skip_without_warning__false(self):
     327        """Test should_skip_without_warning() for False return values."""
     328        paths = ['foo.txt',
     329                 os.path.join('LayoutTests', 'ChangeLog'),
     330        ]
     331
     332        for path in paths:
     333            self._assert_should_skip_without_warning(path,
     334                                                     is_checker_none=False,
     335                                                     expected=False)
    314336
    315337
    … …  
    412434        paths = [
    413435           "ChangeLog",
     436           "ChangeLog-2009-06-16",
     437           "foo.ac",
     438           "foo.cc",
     439           "foo.cgi",
    414440           "foo.css",
     441           "foo.exp",
     442           "foo.flex",
     443           "foo.gyp",
     444           "foo.gypi",
    415445           "foo.html",
    416446           "foo.idl",
     447           "foo.in",
    417448           "foo.js",
    418449           "foo.mm",
    419450           "foo.php",
     451           "foo.pl",
    420452           "foo.pm",
     453           "foo.pri",
     454           "foo.pro",
     455           "foo.rb",
     456           "foo.sh",
    421457           "foo.txt",
    422            "FooChangeLog.bak",
    423            "WebCore/ChangeLog",
    424            "WebCore/inspector/front-end/inspector.js",
    425            "WebKitTools/Scripts/check-webkit-style",
     458           "foo.wm",
     459           "foo.xhtml",
     460           "foo.y",
     461           os.path.join("WebCore", "ChangeLog"),
     462           os.path.join("WebCore", "inspector", "front-end", "inspector.js"),
     463           os.path.join("WebKitTools", "Scripts", "check-webkit-style"),
    426464        ]
    427465
    … …  
    442480        paths = [
    443481           "Makefile",
     482           "foo.asdf",  # Non-sensical file extension.
    444483           "foo.png",
    445484           "foo.exe",
     485           "foo.vcproj",
    446486            ]
    447487
    … …  
    727767    def test_process__no_checker_dispatched(self):
    728768        """Test the process() method for a path with no dispatched checker."""
    729         self._processor.process(lines=['line1', 'line2'],
    730                                 file_path='foo/do_not_process.txt',
    731                                 line_numbers=[100])
    732 
    733         # As a sanity check, check that the carriage-return checker was
    734         # instantiated.  (This code path was already checked in other test
    735         # methods in this test case.)
    736         carriage_checker = self.carriage_checker
    737         self.assertEquals(carriage_checker.lines, ['line1', 'line2'])
    738 
    739         # Check that the style checker was not dispatched.
    740         self.assertTrue(self._mock_dispatcher.dispatched_checker is None)
     769        path = os.path.join('foo', 'do_not_process.txt')
     770        self.assertRaises(AssertionError, self._processor.process,
     771                          lines=['line1', 'line2'], file_path=path,
     772                          line_numbers=[100])
    741773
    742774
Note: See TracChangeset for help on using the changeset viewer.