diff options
Diffstat (limited to 'WebKitTools/Scripts')
31 files changed, 1481 insertions, 1060 deletions
diff --git a/WebKitTools/Scripts/check-webkit-style b/WebKitTools/Scripts/check-webkit-style index 501264b..ea2e943 100755 --- a/WebKitTools/Scripts/check-webkit-style +++ b/WebKitTools/Scripts/check-webkit-style @@ -57,13 +57,11 @@ def main(): codecs.getreader('utf8'), codecs.getwriter('utf8'), 'replace') - - defaults = checker.webkit_argument_defaults() - - parser = checker.ArgumentParser(defaults) + parser = checker.check_webkit_style_parser() (files, options) = parser.parse(sys.argv[1:]) - style_checker = checker.StyleChecker(options) + configuration = checker.check_webkit_style_configuration(options) + style_checker = checker.StyleChecker(configuration) if files: for filename in files: @@ -87,8 +85,11 @@ def main(): style_checker.check_patch(patch) error_count = style_checker.error_count - sys.stderr.write('Total errors found: %d\n' % error_count) - sys.exit(error_count > 0) + file_count = style_checker.file_count + sys.stderr.write('Total errors found: %d in %d files\n' + % (error_count, file_count)) + # We fail when style errors are found or there are no checked files. + sys.exit(error_count > 0 or file_count == 0) if __name__ == "__main__": diff --git a/WebKitTools/Scripts/rebaseline-chromium-webkit-tests b/WebKitTools/Scripts/rebaseline-chromium-webkit-tests index aea0edc..302995c 100755 --- a/WebKitTools/Scripts/rebaseline-chromium-webkit-tests +++ b/WebKitTools/Scripts/rebaseline-chromium-webkit-tests @@ -33,6 +33,7 @@ import sys sys.path.append(os.path.join(os.path.dirname(os.path.abspath(sys.argv[0])), "webkitpy", "layout_tests")) +sys.path.append(os.path.dirname(os.path.dirname(os.path.abspath(sys.argv[0])))) import rebaseline_chromium_webkit_tests diff --git a/WebKitTools/Scripts/resolve-ChangeLogs b/WebKitTools/Scripts/resolve-ChangeLogs index 1a2d2af..3238350 100755 --- a/WebKitTools/Scripts/resolve-ChangeLogs +++ b/WebKitTools/Scripts/resolve-ChangeLogs @@ -34,6 +34,7 @@ use FindBin; use lib $FindBin::Bin; use File::Basename; +use File::Copy; use File::Path; use File::Spec; use Getopt::Long; @@ -64,28 +65,30 @@ my $GIT = "git"; my $fixMerged; my $gitRebaseContinue = 0; +my $mergeDriver = 0; my $printWarnings = 1; my $showHelp; my $getOptionsResult = GetOptions( - 'c|continue!' => \$gitRebaseContinue, - 'f|fix-merged:s' => \&parseFixMerged, - 'h|help' => \$showHelp, - 'w|warnings!' => \$printWarnings, + 'c|continue!' => \$gitRebaseContinue, + 'f|fix-merged:s' => \&parseFixMerged, + 'm|merge-driver!' => \$mergeDriver, + 'h|help' => \$showHelp, + 'w|warnings!' => \$printWarnings, ); my $relativePath = isInGitFilterBranch() ? '.' : chdirReturningRelativePath(determineVCSRoot()); my @changeLogFiles = removeChangeLogArguments($relativePath); -if (!defined $fixMerged && scalar(@changeLogFiles) == 0) { +if (!defined $fixMerged && !$mergeDriver && scalar(@changeLogFiles) == 0) { @changeLogFiles = findUnmergedChangeLogs(); } -if (scalar(@ARGV) > 0) { +if (!$mergeDriver && scalar(@ARGV) > 0) { print STDERR "ERROR: Files listed on command-line that are not ChangeLogs.\n"; undef $getOptionsResult; -} elsif (!defined $fixMerged && scalar(@changeLogFiles) == 0) { +} elsif (!defined $fixMerged && !$mergeDriver && scalar(@changeLogFiles) == 0) { print STDERR "ERROR: No ChangeLog files listed on command-line or found unmerged.\n"; undef $getOptionsResult; } elsif ($gitRebaseContinue && !$isGit) { @@ -94,6 +97,12 @@ if (scalar(@ARGV) > 0) { } elsif (defined $fixMerged && !$isGit) { print STDERR "ERROR: --fix-merged may only be used with a git repository\n"; undef $getOptionsResult; +} elsif ($mergeDriver && !$isGit) { + print STDERR "ERROR: --merge-driver may only be used with a git repository\n"; + undef $getOptionsResult; +} elsif ($mergeDriver && scalar(@ARGV) < 3) { + print STDERR "ERROR: --merge-driver expects %O %A %B as arguments\n"; + undef $getOptionsResult; } sub usageAndExit() @@ -104,6 +113,7 @@ Usage: @{[ basename($0) ]} [options] [path/to/ChangeLog] [path/to/another/Change entries (default: --no-continue) -f|--fix-merged [revision-range] fix git-merged ChangeLog entries; if a revision-range is specified, run git filter-branch on the range + -m|--merge-driver %O %A %B act as a git merge-driver on files %O %A %B -h|--help show this help message -w|--[no-]warnings show or suppress warnings (default: show warnings) __END__ @@ -118,6 +128,14 @@ if (defined $fixMerged && length($fixMerged) > 0) { my $commitRange = $fixMerged; $commitRange = $commitRange . "..HEAD" if index($commitRange, "..") < 0; fixMergedChangeLogs($commitRange, @changeLogFiles); +} elsif ($mergeDriver) { + my ($base, $theirs, $ours) = @ARGV; + if (mergeChanges($ours, $base, $theirs)) { + unlink($ours); + copy($theirs, $ours) or die $!; + } else { + exit 1; + } } elsif (@changeLogFiles) { for my $file (@changeLogFiles) { if (defined $fixMerged) { diff --git a/WebKitTools/Scripts/run-gtk-tests b/WebKitTools/Scripts/run-gtk-tests new file mode 100644 index 0000000..9a57319 --- /dev/null +++ b/WebKitTools/Scripts/run-gtk-tests @@ -0,0 +1,35 @@ +#!/usr/bin/perl +# +# Copyright (C) 2009 Gustavo Noronha Silva <gns@gnome.org> +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Library General Public +# License as published by the Free Software Foundation; either +# version 2 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Library General Public License for more details. +# +# You should have received a copy of the GNU Library General Public License +# along with this library; see the file COPYING.LIB. If not, write to +# the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, +# Boston, MA 02110-1301, USA. + +use strict; +use warnings; + +use FindBin; +use lib $FindBin::Bin; +use webkitdirs; + +# This initializes the correct configuration (Release/Debug) +setConfiguration(); + +my $productDir = productDir(); +my @unitTests = glob $productDir . "/Programs/unittests/*"; +if ($#unitTests < 1) { + die "ERROR: tests not found in $productDir.\n"; +} +system "gtester -k @unitTests" diff --git a/WebKitTools/Scripts/test-webkitpy b/WebKitTools/Scripts/test-webkitpy index cfd3434..8617330 100755 --- a/WebKitTools/Scripts/test-webkitpy +++ b/WebKitTools/Scripts/test-webkitpy @@ -44,6 +44,7 @@ from webkitpy.credentials_unittest import * from webkitpy.diff_parser_unittest import * from webkitpy.executive_unittest import * from webkitpy.grammar_unittest import * +from webkitpy.layout_tests.port.mac_unittest import * from webkitpy.multicommandtool_unittest import * from webkitpy.networktransaction_unittest import * from webkitpy.patchcollection_unittest import * diff --git a/WebKitTools/Scripts/webkit-build-directory b/WebKitTools/Scripts/webkit-build-directory index a85c587..bf7d66d 100755 --- a/WebKitTools/Scripts/webkit-build-directory +++ b/WebKitTools/Scripts/webkit-build-directory @@ -34,31 +34,35 @@ use Getopt::Long; use lib $FindBin::Bin; use webkitdirs; -my $showBaseProductDirectory = 0; +my $showConfigurationDirectory = 0; my $showHelp = 0; +my $showTopLevelDirectory = 0; + my $programName = basename($0); my $usage = <<EOF; Usage: $programName [options] - --base Show the root build directory instead of one corresponding to the current target (e.g. Debug, Release) - --debug Show build directory for the Debug target - -h|--help Show this help message - --release Show build directory for the Release target + --configuration Show the build directory for a specific configuration (e.g. Debug, Release. Defaults to the active configuration set by set-webkit-configuration) + -h|--help Show this help message + --top-level Show the top-level build directory + +Either --configuration or --top-level is required. EOF setConfiguration(); # Figure out from the command line if we're --debug or --release or the default. my $getOptionsResult = GetOptions( - 'base' => \$showBaseProductDirectory, + 'configuration' => \$showConfigurationDirectory, + 'top-level' => \$showTopLevelDirectory, 'help|h' => \$showHelp, ); -if (!$getOptionsResult || $showHelp) { +if (!$getOptionsResult || $showHelp || (!$showConfigurationDirectory && !$showTopLevelDirectory)) { print STDERR $usage; exit 1; } -if ($showBaseProductDirectory) { +if ($showTopLevelDirectory) { print baseProductDir() . "\n"; } else { print productDir() . "\n"; diff --git a/WebKitTools/Scripts/webkitdirs.pm b/WebKitTools/Scripts/webkitdirs.pm index a788b3d..7985790 100644 --- a/WebKitTools/Scripts/webkitdirs.pm +++ b/WebKitTools/Scripts/webkitdirs.pm @@ -614,9 +614,17 @@ sub qtFeatureDefaults() return %qtFeatureDefaults; } +sub commandExists($) +{ + my $command = shift; + my $devnull = File::Spec->devnull(); + return `$command --version 2> $devnull`; +} + sub determineQtFeatureDefaults() { return if %qtFeatureDefaults; + die "ERROR: qmake missing but required to build WebKit.\n" if not commandExists("qmake"); my $originalCwd = getcwd(); chdir File::Spec->catfile(sourceDir(), "WebCore"); my $defaults = `qmake CONFIG+=compute_defaults 2>&1`; @@ -891,10 +899,9 @@ sub checkRequiredSystemConfig my @cmds = qw(flex bison gperf); my @missing = (); foreach my $cmd (@cmds) { - if (not `$cmd --version`) { - push @missing, $cmd; - } + push @missing, $cmd if not commandExists($cmd); } + if (@missing) { my $list = join ", ", @missing; die "ERROR: $list missing but required to build WebKit.\n"; @@ -1162,7 +1169,7 @@ sub qtMakeCommand($) #print "default spec: " . $mkspec . "\n"; #print "compiler found: " . $compiler . "\n"; - if ($compiler eq "cl") { + if ($compiler && $compiler eq "cl") { return "nmake"; } @@ -1454,7 +1461,7 @@ sub runSafari my ($debugger) = @_; if (isAppleMacWebKit()) { - return system "$FindBin::Bin/gdb-safari", @ARGV if $debugger; + return system "$FindBin::Bin/gdb-safari", argumentsForConfiguration() if $debugger; my $productDir = productDir(); print "Starting Safari with DYLD_FRAMEWORK_PATH set to point to built WebKit in $productDir.\n"; diff --git a/WebKitTools/Scripts/webkitpy/committers.py b/WebKitTools/Scripts/webkitpy/committers.py index 7af0987..6413243 100644 --- a/WebKitTools/Scripts/webkitpy/committers.py +++ b/WebKitTools/Scripts/webkitpy/committers.py @@ -82,7 +82,7 @@ committers_unable_to_review = [ Committer("Brian Weinstein", "bweinstein@apple.com"), Committer("Cameron McCormack", "cam@webkit.org"), Committer("Carol Szabo", "carol.szabo@nokia.com"), - Committer("Chang Shu", "chang.shu@nokia.com"), + Committer("Chang Shu", "Chang.Shu@nokia.com"), Committer("Chris Fleizach", "cfleizach@apple.com"), Committer("Chris Jerdonek", "cjerdonek@webkit.org"), Committer("Chris Marrin", "cmarrin@apple.com"), diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/__init__.py b/WebKitTools/Scripts/webkitpy/layout_tests/__init__.py new file mode 100644 index 0000000..ef65bee --- /dev/null +++ b/WebKitTools/Scripts/webkitpy/layout_tests/__init__.py @@ -0,0 +1 @@ +# Required for Python to search this directory for module files diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/test_expectations.py b/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/test_expectations.py index a3650ed..01add62 100644 --- a/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/test_expectations.py +++ b/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/test_expectations.py @@ -35,7 +35,6 @@ import logging import os import re import sys -import time import simplejson @@ -133,10 +132,9 @@ class TestExpectations: def has_modifier(self, test, modifier): return self._expected_failures.has_modifier(test, modifier) - def remove_platform_from_file(self, tests, platform, backup=False): - return self._expected_failures.remove_platform_from_file(tests, - platform, - backup) + def remove_platform_from_expectations(self, tests, platform): + return self._expected_failures.remove_platform_from_expectations( + tests, platform) def strip_comments(line): @@ -373,8 +371,9 @@ class TestExpectationsFile: def contains(self, test): return test in self._test_to_expectations - def remove_platform_from_file(self, tests, platform, backup=False): - """Remove the platform option from test expectations file. + def remove_platform_from_expectations(self, tests, platform): + """Returns a copy of the expectations with the tests matching the + platform remove. If a test is in the test list and has an option that matches the given platform, remove the matching platform and save the updated test back @@ -384,24 +383,13 @@ class TestExpectationsFile: Args: tests: list of tests that need to update.. platform: which platform option to remove. - backup: if true, the original test expectations file is saved as - [self.TEST_LIST].orig.YYYYMMDDHHMMSS Returns: - no + the updated string. """ - # FIXME - remove_platform_from file worked by writing a new - # test_expectations.txt file over the old one. Now that we're just - # parsing strings, we need to change this to return the new - # expectations string. - raise NotImplementedException('remove_platform_from_file') - - new_file = self._path + '.new' - logging.debug('Original file: "%s"', self._path) - logging.debug('New file: "%s"', new_file) f_orig = self._get_iterable_expectations() - f_new = open(new_file, 'w') + f_new = [] tests_removed = 0 tests_updated = 0 @@ -413,7 +401,7 @@ class TestExpectationsFile: if action == NO_CHANGE: # Save the original line back to the file logging.debug('No change to test: %s', line) - f_new.write(line) + f_new.append(line) elif action == REMOVE_TEST: tests_removed += 1 logging.info('Test removed: %s', line) @@ -421,7 +409,7 @@ class TestExpectationsFile: parts = line.split(':') new_options = parts[0].replace(platform.upper() + ' ', '', 1) new_line = ('%s:%s' % (new_options, parts[1])) - f_new.write(new_line) + f_new.append(new_line) tests_updated += 1 logging.info('Test updated: ') logging.info(' old: %s', line) @@ -440,7 +428,7 @@ class TestExpectationsFile: if not p in (platform.upper(), 'WIN-VISTA', 'WIN-7'): new_options += p + ' ' new_line = ('%s:%s' % (new_options, parts[1])) - f_new.write(new_line) + f_new.append(new_line) tests_updated += 1 logging.info('Test updated: ') logging.info(' old: %s', line) @@ -452,23 +440,7 @@ class TestExpectationsFile: logging.info('Total tests removed: %d', tests_removed) logging.info('Total tests updated: %d', tests_updated) - f_orig.close() - f_new.close() - - if backup: - date_suffix = time.strftime('%Y%m%d%H%M%S', - time.localtime(time.time())) - backup_file = ('%s.orig.%s' % (self._path, date_suffix)) - if os.path.exists(backup_file): - os.remove(backup_file) - logging.info('Saving original file to "%s"', backup_file) - os.rename(self._path, backup_file) - else: - os.remove(self._path) - - logging.debug('Saving new file to "%s"', self._path) - os.rename(new_file, self._path) - return True + return "".join(f_new) def parse_expectations_line(self, line, lineno): """Parses a line from test_expectations.txt and returns a tuple diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/port/base.py b/WebKitTools/Scripts/webkitpy/layout_tests/port/base.py index ce06b44..2b25e29 100644 --- a/WebKitTools/Scripts/webkitpy/layout_tests/port/base.py +++ b/WebKitTools/Scripts/webkitpy/layout_tests/port/base.py @@ -83,18 +83,20 @@ class Port(object): interface so that it can be overriden for testing purposes.""" return actual_text != expected_text - def diff_image(self, actual_filename, expected_filename, diff_filename): + def diff_image(self, actual_filename, expected_filename, + diff_filename=None): """Compare two image files and produce a delta image file. Return 1 if the two files are different, 0 if they are the same. Also produce a delta image of the two images and write that into - |diff_filename|. + |diff_filename| if it is not None. While this is a generic routine, we include it in the Port interface so that it can be overriden for testing purposes.""" executable = self._path_to_image_diff() - cmd = [executable, '--diff', actual_filename, expected_filename, - diff_filename] + cmd = [executable, '--diff', actual_filename, expected_filename] + if diff_filename: + cmd.append(diff_filename) result = 1 try: result = subprocess.call(cmd) @@ -267,6 +269,7 @@ class Port(object): used by run-chromium-webkit-tests.""" raise NotImplementedError('Port.num_cores') + # FIXME: This could be replaced by functions in webkitpy.scm. def path_from_webkit_base(self, *comps): """Returns the full path to path made by joining the top of the WebKit source tree and the list of path components in |*comps|.""" @@ -275,6 +278,17 @@ class Port(object): self._webkit_base_dir = abspath[0:abspath.find('WebKitTools')] return os.path.join(self._webkit_base_dir, *comps) + # FIXME: Callers should eventually move to scm.script_path. + def script_path(self, script_name): + return self.path_from_webkit_base("WebKitTools", "Scripts", script_name) + + def path_to_test_expectations_file(self): + """Update the test expectations to the passed-in string. + + This is used by the rebaselining tool. Raises NotImplementedError + if the port does not use expectations files.""" + raise NotImplementedError('Port.path_to_test_expectations_file') + def remove_directory(self, *path): """Recursively removes a directory, even if it's marked read-only. diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium.py b/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium.py index 70a8dea..1123376 100644 --- a/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium.py +++ b/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium.py @@ -88,6 +88,10 @@ class ChromiumPort(base.Port): self._chromium_base_dir = abspath[0:abspath.find('third_party')] return os.path.join(self._chromium_base_dir, *comps) + def path_to_test_expectations_file(self): + return self.path_from_chromium_base('webkit', 'tools', 'layout_tests', + 'test_expectations.txt') + def results_directory(self): return self.path_from_chromium_base('webkit', self._options.target, self._options.results_directory) @@ -128,13 +132,12 @@ class ChromiumPort(base.Port): def test_base_platform_names(self): return ('linux', 'mac', 'win') - def test_expectations(self, options=None): + def test_expectations(self): """Returns the test expectations for this port. Basically this string should contain the equivalent of a test_expectations file. See test_expectations.py for more details.""" - expectations_file = self.path_from_chromium_base('webkit', 'tools', - 'layout_tests', 'test_expectations.txt') + expectations_file = self.path_to_test_expectations_file() return file(expectations_file, "r").read() def test_platform_names(self): @@ -154,7 +157,6 @@ class ChromiumPort(base.Port): return self.path_from_chromium_base('webkit', 'data', 'layout_tests', 'platform', platform, 'LayoutTests') - class ChromiumDriver(base.Driver): """Abstract interface for the DumpRenderTree interface.""" diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_linux.py b/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_linux.py index 8fd5343..b817251 100644 --- a/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_linux.py +++ b/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_linux.py @@ -43,6 +43,8 @@ class ChromiumLinuxPort(chromium.ChromiumPort): def __init__(self, port_name=None, options=None): if port_name is None: port_name = 'chromium-linux' + if options and not hasattr(options, 'target'): + options.target = 'Release' chromium.ChromiumPort.__init__(self, port_name, options) def baseline_search_path(self): diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_mac.py b/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_mac.py index 7e7b4ca..bcffcf8 100644 --- a/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_mac.py +++ b/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_mac.py @@ -43,6 +43,8 @@ class ChromiumMacPort(chromium.ChromiumPort): def __init__(self, port_name=None, options=None): if port_name is None: port_name = 'chromium-mac' + if options and not hasattr(options, 'target'): + options.target = 'Release' chromium.ChromiumPort.__init__(self, port_name, options) def baseline_search_path(self): diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_win.py b/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_win.py index 352916c..5eb0ba1 100644 --- a/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_win.py +++ b/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_win.py @@ -44,6 +44,8 @@ class ChromiumWinPort(chromium.ChromiumPort): def __init__(self, port_name=None, options=None): if port_name is None: port_name = 'chromium-win' + self.version() + if options and not hasattr(options, 'target'): + options.target = 'Release' chromium.ChromiumPort.__init__(self, port_name, options) def baseline_search_path(self): diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/port/mac.py b/WebKitTools/Scripts/webkitpy/layout_tests/port/mac.py index 4b73cec..d355f62 100644 --- a/WebKitTools/Scripts/webkitpy/layout_tests/port/mac.py +++ b/WebKitTools/Scripts/webkitpy/layout_tests/port/mac.py @@ -67,8 +67,16 @@ class MacPort(base.Port): return dirs def check_sys_deps(self): - # FIXME: This should run build-dumprendertree. - # This should also validate that all of the tool paths are valid. + if executive.run_command([self.script_path("build-dumprendertree")], return_exit_code=True) != 0: + return False + + driver_path = self._path_to_driver() + if not os.path.exists(driver_path): + logging.error("DumpRenderTree was not found at %s" % driver_path) + return False + + # This should also validate that the ImageDiff path is valid (once this script knows how to use ImageDiff). + # https://bugs.webkit.org/show_bug.cgi?id=34826 return True def num_cores(self): @@ -103,8 +111,67 @@ class MacPort(base.Port): # to return something. return ('mac',) + def _skipped_file_paths(self): + # FIXME: This method will need to be made work for non-mac platforms and moved into base.Port. + skipped_files = [] + if self._name in ('mac-tiger', 'mac-leopard', 'mac-snowleopard'): + skipped_files.append(os.path.join( + self._webkit_baseline_path(self._name), 'Skipped')) + skipped_files.append(os.path.join(self._webkit_baseline_path('mac'), + 'Skipped')) + return skipped_files + + def _tests_for_other_platforms(self): + # The original run-webkit-tests builds up a "whitelist" of tests to run, and passes that to DumpRenderTree. + # run-chromium-webkit-tests assumes we run *all* tests and test_expectations.txt functions as a blacklist. + # FIXME: This list could be dynamic based on platform name and pushed into base.Port. + return [ + "platform/chromium", + "platform/gtk", + "platform/qt", + "platform/win", + ] + + def _tests_for_disabled_features(self): + # FIXME: This should use the feature detection from webkitperl/features.pm to match run-webkit-tests. + # For now we hard-code a list of features known to be disabled on the Mac platform. + disabled_feature_tests = [ + "fast/xhtmlmp", + "http/tests/wml", + "mathml", + "wml", + ] + # FIXME: webarchive tests expect to read-write from -expected.webarchive files instead of .txt files. + # This script doesn't know how to do that yet, so pretend they're just "disabled". + webarchive_tests = [ + "webarchive", + "svg/webarchive", + "http/tests/webarchive", + "svg/custom/image-with-prefix-in-webarchive.svg", + ] + return disabled_feature_tests + webarchive_tests + + def _tests_from_skipped_file(self, skipped_file): + tests_to_skip = [] + for line in skipped_file.readlines(): + line = line.strip() + if line.startswith('#') or not len(line): + continue + tests_to_skip.append(line) + return tests_to_skip + + def _expectations_from_skipped_files(self): + tests_to_skip = [] + for filename in self._skipped_file_paths(): + if not os.path.exists(filename): + logging.warn("Failed to open Skipped file: %s" % filename) + continue + skipped_file = file(filename) + tests_to_skip.extend(self._tests_from_skipped_file(skipped_file)) + skipped_file.close() + return tests_to_skip + def test_expectations(self): - # # The WebKit mac port uses 'Skipped' files at the moment. Each # file contains a list of files or directories to be skipped during # the test run. The total list of tests to skipped is given by the @@ -112,44 +179,11 @@ class MacPort(base.Port): # a version-specific file found in platform/X-version. Duplicate # entries are allowed. This routine reads those files and turns # contents into the format expected by test_expectations. - expectations = [] - skipped_files = [] - if self._name in ('mac-tiger', 'mac-leopard', 'mac-snowleopard'): - skipped_files.append(os.path.join( - self._webkit_baseline_path(self._name), 'Skipped')) - skipped_files.append(os.path.join(self._webkit_baseline_path('mac'), - 'Skipped')) - for filename in skipped_files: - if os.path.exists(filename): - f = file(filename) - for l in f.readlines(): - l = l.strip() - if not l.startswith('#') and len(l): - l = 'BUG_SKIPPED SKIP : ' + l + ' = FAIL' - if l not in expectations: - expectations.append(l) - f.close() - - # TODO - figure out how to check for these dynamically - expectations.append('BUG_SKIPPED SKIP : fast/wcss = FAIL') - expectations.append('BUG_SKIPPED SKIP : fast/xhtmlmp = FAIL') - expectations.append('BUG_SKIPPED SKIP : http/tests/wml = FAIL') - expectations.append('BUG_SKIPPED SKIP : mathml = FAIL') - expectations.append('BUG_SKIPPED SKIP : platform/chromium = FAIL') - expectations.append('BUG_SKIPPED SKIP : platform/gtk = FAIL') - expectations.append('BUG_SKIPPED SKIP : platform/qt = FAIL') - expectations.append('BUG_SKIPPED SKIP : platform/win = FAIL') - expectations.append('BUG_SKIPPED SKIP : wml = FAIL') - - # TODO - figure out how to handle webarchive tests - expectations.append('BUG_SKIPPED SKIP : webarchive = PASS') - expectations.append('BUG_SKIPPED SKIP : svg/webarchive = PASS') - expectations.append('BUG_SKIPPED SKIP : http/tests/webarchive = PASS') - expectations.append('BUG_SKIPPED SKIP : svg/custom/' - 'image-with-prefix-in-webarchive.svg = PASS') - - expectations_str = '\n'.join(expectations) - return expectations_str + tests_to_skip = set(self._expectations_from_skipped_files()) # Use a set to allow duplicates + tests_to_skip.update(self._tests_for_other_platforms()) + tests_to_skip.update(self._tests_for_disabled_features()) + expectations = map(lambda test_path: "BUG_SKIPPED SKIP : %s = FAIL" % test_path, tests_to_skip) + return "\n".join(expectations) def test_platform_name(self): # At the moment we don't use test platform names, but we have @@ -180,7 +214,7 @@ class MacPort(base.Port): def _build_path(self, *comps): if not self._cached_build_root: - self._cached_build_root = executive.run_command(["webkit-build-directory", "--base"]).rstrip() + self._cached_build_root = executive.run_command([self.script_path("webkit-build-directory"), "--top-level"]).rstrip() return os.path.join(self._cached_build_root, self._options.target, *comps) def _kill_process(self, pid): @@ -390,7 +424,6 @@ class MacDriver(base.Driver): return (crash, timeout, actual_image_hash, ''.join(output), ''.join(error)) - pass def stop(self): if self._proc: diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/port/mac_unittest.py b/WebKitTools/Scripts/webkitpy/layout_tests/port/mac_unittest.py new file mode 100644 index 0000000..e47a4a4 --- /dev/null +++ b/WebKitTools/Scripts/webkitpy/layout_tests/port/mac_unittest.py @@ -0,0 +1,66 @@ +# Copyright (C) 2010 Google Inc. All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# * Redistributions in binary form must reproduce the above +# copyright notice, this list of conditions and the following disclaimer +# in the documentation and/or other materials provided with the +# distribution. +# * Neither the name of Google Inc. nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +import unittest +import mac +import StringIO + +class MacTest(unittest.TestCase): + + def test_skipped_file_paths(self): + port = mac.MacPort() + skipped_paths = port._skipped_file_paths() + # FIXME: _skipped_file_paths should return WebKit-relative paths. + # So to make it unit testable, we strip the WebKit directory from the path. + relative_paths = [path[len(port.path_from_webkit_base()):] for path in skipped_paths] + self.assertEqual(relative_paths, ['LayoutTests/platform/mac-leopard/Skipped', 'LayoutTests/platform/mac/Skipped']) + + example_skipped_file = """ +# <rdar://problem/5647952> fast/events/mouseout-on-window.html needs mac DRT to issue mouse out events +fast/events/mouseout-on-window.html + +# <rdar://problem/5643675> window.scrollTo scrolls a window with no scrollbars +fast/events/attempt-scroll-with-no-scrollbars.html + +# see bug <rdar://problem/5646437> REGRESSION (r28015): svg/batik/text/smallFonts fails +svg/batik/text/smallFonts.svg +""" + example_skipped_tests = [ + "fast/events/mouseout-on-window.html", + "fast/events/attempt-scroll-with-no-scrollbars.html", + "svg/batik/text/smallFonts.svg", + ] + + def test_skipped_file_paths(self): + port = mac.MacPort() + skipped_file = StringIO.StringIO(self.example_skipped_file) + self.assertEqual(port._tests_from_skipped_file(skipped_file), self.example_skipped_tests) + + +if __name__ == '__main__': + unittest.main() diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/port/test.py b/WebKitTools/Scripts/webkitpy/layout_tests/port/test.py index 0bc6e7c..c3e97be 100644 --- a/WebKitTools/Scripts/webkitpy/layout_tests/port/test.py +++ b/WebKitTools/Scripts/webkitpy/layout_tests/port/test.py @@ -55,7 +55,8 @@ class TestPort(base.Port): def check_sys_deps(self): return True - def diff_image(self, actual_filename, expected_filename, diff_filename): + def diff_image(self, actual_filename, expected_filename, + diff_filename=None): return False def compare_text(self, actual_text, expected_text): diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/port/websocket_server.py b/WebKitTools/Scripts/webkitpy/layout_tests/port/websocket_server.py index ba8a5e9..54c2f6f 100644 --- a/WebKitTools/Scripts/webkitpy/layout_tests/port/websocket_server.py +++ b/WebKitTools/Scripts/webkitpy/layout_tests/port/websocket_server.py @@ -159,6 +159,7 @@ class PyWebSocket(http_server.Lighttpd): '-p', str(self._port), '-d', self._layout_tests, '-s', self._web_socket_tests, + '-x', '/websocket/tests/cookies', '-l', error_log, ] diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests.py b/WebKitTools/Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests.py index 83cf99de..4604a1a 100644 --- a/WebKitTools/Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests.py +++ b/WebKitTools/Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests.py @@ -54,7 +54,7 @@ import urllib import webbrowser import zipfile -from layout_package import path_utils +import port from layout_package import test_expectations from test_types import image_diff from test_types import text_diff @@ -200,9 +200,10 @@ class Rebaseliner(object): REVISION_REGEX = r'<a href=\"(\d+)/\">' - def __init__(self, platform, options): - self._file_dir = path_utils.path_from_base('webkit', 'tools', + def __init__(self, port, platform, options): + self._file_dir = port.path_from_chromium_base('webkit', 'tools', 'layout_tests') + self._port = port self._platform = platform self._options = options self._rebaselining_tests = [] @@ -212,10 +213,12 @@ class Rebaseliner(object): # -. compile list of tests that need rebaselining. # -. update the tests in test_expectations file after rebaseline # is done. + expectations_str = self._port.test_expectations() self._test_expectations = \ - test_expectations.TestExpectations(None, - self._file_dir, - platform, + test_expectations.TestExpectations(self._port, + None, + expectations_str, + self._platform, False, False) @@ -359,7 +362,6 @@ class Rebaseliner(object): latest_revision = self._get_latest_revision(url_base) if latest_revision is None or latest_revision <= 0: return None - archive_url = ('%s%s/layout-test-results.zip' % (url_base, latest_revision)) logging.info('Archive url: "%s"', archive_url) @@ -399,7 +401,7 @@ class Rebaseliner(object): for name in zip_namelist: logging.debug(' ' + name) - platform = path_utils.platform_name(self._platform) + platform = self._port.name() logging.debug('Platform dir: "%s"', platform) test_no = 1 @@ -412,7 +414,7 @@ class Rebaseliner(object): test_basename = os.path.splitext(test)[0] for suffix in BASELINE_SUFFIXES: archive_test_name = ('layout-test-results/%s-actual%s' % - (test_basename, suffix)) + (test_basename, suffix)) logging.debug(' Archive test file name: "%s"', archive_test_name) if not archive_test_name in zip_namelist: @@ -431,7 +433,7 @@ class Rebaseliner(object): expected_filename = '%s-expected%s' % (test_basename, suffix) expected_fullpath = os.path.join( - path_utils.chromium_baseline_path(platform), + self._port._chromium_baseline_path(platform), expected_filename) expected_fullpath = os.path.normpath(expected_fullpath) logging.debug(' Expected file full path: "%s"', @@ -443,17 +445,17 @@ class Rebaseliner(object): # and lower # levels and remove all duplicated baselines. if self._is_dup_baseline(temp_name, - expected_fullpath, - test, - suffix, - self._platform): + expected_fullpath, + test, + suffix, + self._platform): os.remove(temp_name) self._delete_baseline(expected_fullpath) continue # Create the new baseline directory if it doesn't already # exist. - path_utils.maybe_make_directory( + self._port.maybe_make_directory( os.path.dirname(expected_fullpath)) shutil.move(temp_name, expected_fullpath) @@ -497,9 +499,9 @@ class Rebaseliner(object): True if the baseline is unnecessary. False otherwise. """ - test_filepath = os.path.join(path_utils.layout_tests_dir(), test) - all_baselines = path_utils.expected_baselines(test_filepath, - suffix, platform, True) + test_filepath = os.path.join(self._port.layout_tests_dir(), test) + all_baselines = self._port.expected_baselines(test_filepath, + suffix, True) for (fallback_dir, fallback_file) in all_baselines: if fallback_dir and fallback_file: fallback_fullpath = os.path.normpath( @@ -534,11 +536,11 @@ class Rebaseliner(object): return True if ext1 == '.PNG': - return image_diff.ImageDiff(self._platform, '').diff_files(file1, - file2) + return image_diff.ImageDiff(self._port, self._platform, + '').diff_files(self._port, file1, file2) else: - return text_diff.TestTextDiff(self._platform, '').diff_files(file1, - file2) + return text_diff.TestTextDiff(self._port, self._platform, + '').diff_files(self._port, file1, file2) def _delete_baseline(self, filename): """Remove the file from repository and delete it from disk. @@ -570,8 +572,21 @@ class Rebaseliner(object): """ if self._rebaselined_tests: - self._test_expectations.remove_platform_from_file( - self._rebaselined_tests, self._platform, backup) + new_expectations = ( + self._test_expectations.remove_platform_from_expectations( + self._rebaselined_tests, self._platform)) + path = self._port.path_to_test_expectations_file() + if backup: + date_suffix = time.strftime('%Y%m%d%H%M%S', + time.localtime(time.time())) + backup_file = ('%s.orig.%s' % (path, date_suffix)) + if os.path.exists(backup_file): + os.remove(backup_file) + logging.info('Saving original file to "%s"', backup_file) + os.rename(path, backup_file) + f = open(path, "w") + f.write(new_expectations) + f.close() else: logging.info('No test was rebaselined so nothing to remove.') @@ -777,8 +792,9 @@ class HtmlGenerator(object): '<img style="width: 200" src="%(uri)s" /></a></td>') HTML_TR = '<tr>%s</tr>' - def __init__(self, options, platforms, rebaselining_tests): + def __init__(self, port, options, platforms, rebaselining_tests): self._html_directory = options.html_directory + self._port = port self._platforms = platforms self._rebaselining_tests = rebaselining_tests self._html_file = os.path.join(options.html_directory, @@ -817,7 +833,7 @@ class HtmlGenerator(object): logging.info('Launching html: "%s"', self._html_file) - html_uri = path_utils.filename_to_uri(self._html_file) + html_uri = self._port.filename_to_uri(self._html_file) webbrowser.open(html_uri, 1) logging.info('Html launched.') @@ -855,13 +871,13 @@ class HtmlGenerator(object): links = '' if os.path.exists(old_file): links += html_td_link % { - 'uri': path_utils.filename_to_uri(old_file), + 'uri': self._port.filename_to_uri(old_file), 'name': baseline_filename} else: logging.info(' No old baseline file: "%s"', old_file) links += self.HTML_TD_NOLINK % '' - links += html_td_link % {'uri': path_utils.filename_to_uri(new_file), + links += html_td_link % {'uri': self._port.filename_to_uri(new_file), 'name': baseline_filename} diff_file = get_result_file_fullpath(self._html_directory, @@ -869,7 +885,7 @@ class HtmlGenerator(object): 'diff') logging.info(' Baseline diff file: "%s"', diff_file) if os.path.exists(diff_file): - links += html_td_link % {'uri': path_utils.filename_to_uri( + links += html_td_link % {'uri': self._port.filename_to_uri( diff_file), 'name': 'Diff'} else: logging.info(' No baseline diff file: "%s"', diff_file) @@ -908,8 +924,8 @@ class HtmlGenerator(object): rows.append(self.HTML_TR % row) if rows: - test_path = os.path.join(path_utils.layout_tests_dir(), test) - html = self.HTML_TR_TEST % (path_utils.filename_to_uri(test_path), + test_path = os.path.join(self._port.layout_tests_dir(), test) + html = self.HTML_TR_TEST % (self._port.filename_to_uri(test_path), test) html += self.HTML_TEST_DETAIL % ' '.join(rows) @@ -967,6 +983,7 @@ def main(): ' rebaselining comparison.')) options = option_parser.parse_args()[0] + port_obj = port.get(None, options) # Set up our logging format. log_level = logging.INFO @@ -1002,7 +1019,7 @@ def main(): rebaselining_tests = set() backup = options.backup for platform in rebaseline_platforms: - rebaseliner = Rebaseliner(platform, options) + rebaseliner = Rebaseliner(port_obj, platform, options) logging.info('') log_dashed_string('Rebaseline started', platform) @@ -1017,7 +1034,8 @@ def main(): logging.info('') log_dashed_string('Rebaselining result comparison started', None) - html_generator = HtmlGenerator(options, + html_generator = HtmlGenerator(port_obj, + options, rebaseline_platforms, rebaselining_tests) html_generator.generate_html() diff --git a/WebKitTools/Scripts/webkitpy/style/checker.py b/WebKitTools/Scripts/webkitpy/style/checker.py index fbda8cb..9beda9e 100644 --- a/WebKitTools/Scripts/webkitpy/style/checker.py +++ b/WebKitTools/Scripts/webkitpy/style/checker.py @@ -30,24 +30,24 @@ """Front end of some style-checker modules.""" import codecs -import getopt import os.path import sys from .. style_references import parse_patch from error_handlers import DefaultStyleErrorHandler from error_handlers import PatchStyleErrorHandler -from filter import validate_filter_rules from filter import FilterConfiguration +from optparser import ArgumentParser +from optparser import DefaultCommandOptionValues from processors.common import check_no_carriage_return from processors.common import categories as CommonCategories from processors.cpp import CppProcessor from processors.text import TextProcessor -# These defaults are used by check-webkit-style. -WEBKIT_DEFAULT_VERBOSITY = 1 -WEBKIT_DEFAULT_OUTPUT_FORMAT = 'emacs' +# These are default option values for the command-line option parser. +_DEFAULT_VERBOSITY = 1 +_DEFAULT_OUTPUT_FORMAT = 'emacs' # FIXME: For style categories we will never want to have, remove them. @@ -55,14 +55,16 @@ WEBKIT_DEFAULT_OUTPUT_FORMAT = 'emacs' # modify the implementation and enable them. # # Throughout this module, we use "filter rule" rather than "filter" -# for an individual boolean filter flag like "+foo". This allows us to +# for an individual boolean filter flag like "+foo". This allows us to # reserve "filter" for what one gets by collectively applying all of # the filter rules. # -# The _WEBKIT_FILTER_RULES are prepended to any user-specified filter -# rules. Since by default all errors are on, only include rules that -# begin with a - sign. -WEBKIT_DEFAULT_FILTER_RULES = [ +# The base filter rules are the filter rules that begin the list of +# filter rules used to check style. For example, these rules precede +# any user-specified filter rules. Since by default all categories are +# checked, this list should normally include only rules that begin +# with a "-" sign. +_BASE_FILTER_RULES = [ '-build/endif_comment', '-build/include_what_you_use', # <string> for std::string '-build/storage_class', # const static @@ -86,26 +88,24 @@ WEBKIT_DEFAULT_FILTER_RULES = [ ] -# FIXME: Change the second value of each tuple from a tuple to a list, -# and alter the filter code so it accepts lists instead. (The -# filter code will need to convert incoming values from a list -# to a tuple prior to caching). This will make this -# configuration setting a bit simpler since tuples have an -# unusual syntax case. -# # The path-specific filter rules. # # This list is order sensitive. Only the first path substring match # is used. See the FilterConfiguration documentation in filter.py # for more information on this list. +# +# Each string appearing in this nested list should have at least +# one associated unit test assertion. These assertions are located, +# for example, in the test_path_rules_specifier() unit test method of +# checker_unittest.py. _PATH_RULES_SPECIFIER = [ # Files in these directories are consumers of the WebKit # API and therefore do not follow the same header including # discipline as WebCore. (["WebKitTools/WebKitAPITest/", "WebKit/qt/QGVLauncher/"], - ("-build/include", - "-readability/streams")), + ["-build/include", + "-readability/streams"]), ([# The GTK+ APIs use GTK+ naming style, which includes # lower-cased, underscore-separated values. "WebKit/gtk/webkit/", @@ -116,13 +116,7 @@ _PATH_RULES_SPECIFIER = [ # QtTest module. "WebKit/qt/tests/", "JavaScriptCore/qt/tests"], - ("-readability/naming",)), - # These are test file patterns. - (["_test.cpp", - "_unittest.cpp", - "_regtest.cpp"], - ("-readability/streams", # Many unit tests use cout. - "-runtime/rtti")), + ["-readability/naming"]), ] @@ -131,7 +125,7 @@ _PATH_RULES_SPECIFIER = [ # future merges. # # Include a warning for skipped files that are less obvious. -SKIPPED_FILES_WITH_WARNING = [ +_SKIPPED_FILES_WITH_WARNING = [ # The Qt API and tests do not follow WebKit style. # They follow Qt style. :) "gtk2drawing.c", # WebCore/platform/gtk/gtk2drawing.c @@ -145,439 +139,56 @@ SKIPPED_FILES_WITH_WARNING = [ # Don't include a warning for skipped files that are more common # and more obvious. -SKIPPED_FILES_WITHOUT_WARNING = [ +_SKIPPED_FILES_WITHOUT_WARNING = [ "LayoutTests/" ] # The maximum number of errors to report per file, per category. # If a category is not a key, then it has no maximum. -MAX_REPORTS_PER_CATEGORY = { +_MAX_REPORTS_PER_CATEGORY = { "whitespace/carriage_return": 1 } -def style_categories(): +def _all_categories(): """Return the set of all categories used by check-webkit-style.""" # Take the union across all processors. return CommonCategories.union(CppProcessor.categories) -def webkit_argument_defaults(): - """Return the DefaultArguments instance for use by check-webkit-style.""" - return ArgumentDefaults(WEBKIT_DEFAULT_OUTPUT_FORMAT, - WEBKIT_DEFAULT_VERBOSITY, - WEBKIT_DEFAULT_FILTER_RULES) - - -def _create_usage(defaults): - """Return the usage string to display for command help. - - Args: - defaults: An ArgumentDefaults instance. - - """ - usage = """ -Syntax: %(program_name)s [--verbose=#] [--git-commit=<SingleCommit>] [--output=vs7] - [--filter=-x,+y,...] [file] ... - - The style guidelines this tries to follow are here: - http://webkit.org/coding/coding-style.html - - Every style error is given a confidence score from 1-5, with 5 meaning - we are certain of the problem, and 1 meaning it could be a legitimate - construct. This can miss some errors and does not substitute for - code review. - - To prevent specific lines from being linted, add a '// NOLINT' comment to the - end of the line. - - Linted extensions are .cpp, .c and .h. Other file types are ignored. - - The file parameter is optional and accepts multiple files. Leaving - out the file parameter applies the check to all files considered changed - by your source control management system. - - Flags: - - verbose=# - A number 1-5 that restricts output to errors with a confidence - score at or above this value. In particular, the value 1 displays - all errors. The default is %(default_verbosity)s. - - git-commit=<SingleCommit> - Checks the style of everything from the given commit to the local tree. - - output=vs7 - The output format, which may be one of - emacs : to ease emacs parsing - vs7 : compatible with Visual Studio - Defaults to "%(default_output_format)s". Other formats are unsupported. - - filter=-x,+y,... - A comma-separated list of boolean filter rules used to filter - which categories of style guidelines to check. The script checks - a category if the category passes the filter rules, as follows. - - Any webkit category starts out passing. All filter rules are then - evaluated left to right, with later rules taking precedence. For - example, the rule "+foo" passes any category that starts with "foo", - and "-foo" fails any such category. The filter input "-whitespace, - +whitespace/braces" fails the category "whitespace/tab" and passes - "whitespace/braces". - - Examples: --filter=-whitespace,+whitespace/braces - --filter=-whitespace,-runtime/printf,+runtime/printf_format - --filter=-,+build/include_what_you_use - - Category names appear in error messages in brackets, for example - [whitespace/indent]. To see a list of all categories available to - %(program_name)s, along with which are enabled by default, pass - the empty filter as follows: - --filter= -""" % {'program_name': os.path.basename(sys.argv[0]), - 'default_verbosity': defaults.verbosity, - 'default_output_format': defaults.output_format} - - return usage - - -# FIXME: Eliminate support for "extra_flag_values". -# -# FIXME: Remove everything from ProcessorOptions except for the -# information that can be passed via the command line, and -# rename to something like CheckWebKitStyleOptions. This -# includes, but is not limited to, removing the -# max_reports_per_error attribute and the is_reportable() -# method. See also the FIXME below to create a new class -# called something like CheckerConfiguration. -# -# This class should not have knowledge of the flag key names. -class ProcessorOptions(object): - - """A container to store options passed via the command line. - - Attributes: - extra_flag_values: A string-string dictionary of all flag key-value - pairs that are not otherwise represented by this - class. The default is the empty dictionary. - - filter_configuration: A FilterConfiguration instance. The default - is the "empty" filter configuration, which - means that all errors should be checked. - - git_commit: A string representing the git commit to check. - The default is None. - - max_reports_per_error: The maximum number of errors to report - per file, per category. - - output_format: A string that is the output format. The supported - output formats are "emacs" which emacs can parse - and "vs7" which Microsoft Visual Studio 7 can parse. - - verbosity: An integer between 1-5 inclusive that restricts output - to errors with a confidence score at or above this value. - The default is 1, which reports all errors. - - """ - def __init__(self, - extra_flag_values=None, - filter_configuration = None, - git_commit=None, - max_reports_per_category=None, - output_format="emacs", - verbosity=1): - if extra_flag_values is None: - extra_flag_values = {} - if filter_configuration is None: - filter_configuration = FilterConfiguration() - if max_reports_per_category is None: - max_reports_per_category = {} - - if output_format not in ("emacs", "vs7"): - raise ValueError('Invalid "output_format" parameter: ' - 'value must be "emacs" or "vs7". ' - 'Value given: "%s".' % output_format) - - if (verbosity < 1) or (verbosity > 5): - raise ValueError('Invalid "verbosity" parameter: ' - "value must be an integer between 1-5 inclusive. " - 'Value given: "%s".' % verbosity) - - self.extra_flag_values = extra_flag_values - self.filter_configuration = filter_configuration - self.git_commit = git_commit - self.max_reports_per_category = max_reports_per_category - self.output_format = output_format - self.verbosity = verbosity - - # Useful for unit testing. - def __eq__(self, other): - """Return whether this ProcessorOptions instance is equal to another.""" - if self.extra_flag_values != other.extra_flag_values: - return False - if self.filter_configuration != other.filter_configuration: - return False - if self.git_commit != other.git_commit: - return False - if self.max_reports_per_category != other.max_reports_per_category: - return False - if self.output_format != other.output_format: - return False - if self.verbosity != other.verbosity: - return False - - return True - - # Useful for unit testing. - def __ne__(self, other): - # Python does not automatically deduce this from __eq__(). - return not self.__eq__(other) - - def is_reportable(self, category, confidence_in_error, path): - """Return whether an error is reportable. - - An error is reportable if the confidence in the error - is at least the current verbosity level, and if the current - filter says that the category should be checked for the - given path. - - Args: - category: A string that is a style category. - confidence_in_error: An integer between 1 and 5, inclusive, that - represents the application's confidence in - the error. A higher number signifies greater - confidence. - path: The path of the file being checked - - """ - if confidence_in_error < self.verbosity: - return False - - return self.filter_configuration.should_check(category, path) - - -# This class should not have knowledge of the flag key names. -class ArgumentDefaults(object): - - """A container to store default argument values. - - Attributes: - output_format: A string that is the default output format. - verbosity: An integer that is the default verbosity level. - base_filter_rules: A list of strings that are boolean filter rules - to prepend to any user-specified rules. - - """ - - def __init__(self, default_output_format, default_verbosity, - default_base_filter_rules): - self.output_format = default_output_format - self.verbosity = default_verbosity - self.base_filter_rules = default_base_filter_rules - - -class ArgumentPrinter(object): - - """Supports the printing of check-webkit-style command arguments.""" - - def _flag_pair_to_string(self, flag_key, flag_value): - return '--%(key)s=%(val)s' % {'key': flag_key, 'val': flag_value } - - def to_flag_string(self, options): - """Return a flag string yielding the given ProcessorOptions instance. - - This method orders the flag values alphabetically by the flag key. - - Args: - options: A ProcessorOptions instance. - - """ - flags = options.extra_flag_values.copy() +def _check_webkit_style_defaults(): + """Return the default command-line options for check-webkit-style.""" + return DefaultCommandOptionValues(output_format=_DEFAULT_OUTPUT_FORMAT, + verbosity=_DEFAULT_VERBOSITY) - flags['output'] = options.output_format - flags['verbose'] = options.verbosity - # Only include the filter flag if user-provided rules are present. - user_rules = options.filter_configuration.user_rules - if user_rules: - flags['filter'] = ",".join(user_rules) - if options.git_commit: - flags['git-commit'] = options.git_commit - flag_string = '' - # Alphabetizing lets us unit test this method. - for key in sorted(flags.keys()): - flag_string += self._flag_pair_to_string(key, flags[key]) + ' ' +# This function assists in optparser not having to import from checker. +def check_webkit_style_parser(): + all_categories = _all_categories() + default_options = _check_webkit_style_defaults() + return ArgumentParser(all_categories=all_categories, + base_filter_rules=_BASE_FILTER_RULES, + default_options=default_options) - return flag_string.strip() +def check_webkit_style_configuration(options): + """Return a StyleCheckerConfiguration instance for check-webkit-style. -class ArgumentParser(object): - - """Supports the parsing of check-webkit-style command arguments. - - Attributes: - defaults: An ArgumentDefaults instance. - create_usage: A function that accepts an ArgumentDefaults instance - and returns a string of usage instructions. - This defaults to the function used to generate the - usage string for check-webkit-style. - doc_print: A function that accepts a string parameter and that is - called to display help messages. This defaults to - sys.stderr.write(). + Args: + options: A CommandOptionValues instance. """ + filter_configuration = FilterConfiguration( + base_rules=_BASE_FILTER_RULES, + path_specific=_PATH_RULES_SPECIFIER, + user_rules=options.filter_rules) - def __init__(self, argument_defaults, create_usage=None, doc_print=None): - if create_usage is None: - create_usage = _create_usage - if doc_print is None: - doc_print = sys.stderr.write - - self.defaults = argument_defaults - self.create_usage = create_usage - self.doc_print = doc_print - - def _exit_with_usage(self, error_message=''): - """Exit and print a usage string with an optional error message. - - Args: - error_message: A string that is an error message to print. - - """ - usage = self.create_usage(self.defaults) - self.doc_print(usage) - if error_message: - sys.exit('\nFATAL ERROR: ' + error_message) - else: - sys.exit(1) - - def _exit_with_categories(self): - """Exit and print the style categories and default filter rules.""" - self.doc_print('\nAll categories:\n') - categories = style_categories() - for category in sorted(categories): - self.doc_print(' ' + category + '\n') - - self.doc_print('\nDefault filter rules**:\n') - for filter_rule in sorted(self.defaults.base_filter_rules): - self.doc_print(' ' + filter_rule + '\n') - self.doc_print('\n**The command always evaluates the above rules, ' - 'and before any --filter flag.\n\n') - - sys.exit(0) - - def _parse_filter_flag(self, flag_value): - """Parse the --filter flag, and return a list of filter rules. - - Args: - flag_value: A string of comma-separated filter rules, for - example "-whitespace,+whitespace/indent". - - """ - filters = [] - for uncleaned_filter in flag_value.split(','): - filter = uncleaned_filter.strip() - if not filter: - continue - filters.append(filter) - return filters - - def parse(self, args, extra_flags=None): - """Parse the command line arguments to check-webkit-style. - - Args: - args: A list of command-line arguments as returned by sys.argv[1:]. - extra_flags: A list of flags whose values we want to extract, but - are not supported by the ProcessorOptions class. - An example flag "new_flag=". This defaults to the - empty list. - - Returns: - A tuple of (filenames, options) - - filenames: The list of filenames to check. - options: A ProcessorOptions instance. - - """ - if extra_flags is None: - extra_flags = [] - - output_format = self.defaults.output_format - verbosity = self.defaults.verbosity - base_rules = self.defaults.base_filter_rules - - # The flags already supported by the ProcessorOptions class. - flags = ['help', 'output=', 'verbose=', 'filter=', 'git-commit='] - - for extra_flag in extra_flags: - if extra_flag in flags: - raise ValueError('Flag \'%(extra_flag)s is duplicated ' - 'or already supported.' % - {'extra_flag': extra_flag}) - flags.append(extra_flag) - - try: - (opts, filenames) = getopt.getopt(args, '', flags) - except getopt.GetoptError: - # FIXME: Settle on an error handling approach: come up - # with a consistent guideline as to when and whether - # a ValueError should be raised versus calling - # sys.exit when needing to interrupt execution. - self._exit_with_usage('Invalid arguments.') - - extra_flag_values = {} - git_commit = None - user_rules = [] - - for (opt, val) in opts: - if opt == '--help': - self._exit_with_usage() - elif opt == '--output': - output_format = val - elif opt == '--verbose': - verbosity = val - elif opt == '--git-commit': - git_commit = val - elif opt == '--filter': - if not val: - self._exit_with_categories() - # Prepend the defaults. - user_rules = self._parse_filter_flag(val) - else: - extra_flag_values[opt] = val - - # Check validity of resulting values. - if filenames and (git_commit != None): - self._exit_with_usage('It is not possible to check files and a ' - 'specific commit at the same time.') - - if output_format not in ('emacs', 'vs7'): - raise ValueError('Invalid --output value "%s": The only ' - 'allowed output formats are emacs and vs7.' % - output_format) - - all_categories = style_categories() - validate_filter_rules(user_rules, all_categories) - - verbosity = int(verbosity) - if (verbosity < 1) or (verbosity > 5): - raise ValueError('Invalid --verbose value %s: value must ' - 'be between 1-5.' % verbosity) - - filter_configuration = FilterConfiguration(base_rules=base_rules, - path_specific=_PATH_RULES_SPECIFIER, - user_rules=user_rules) - - options = ProcessorOptions(extra_flag_values=extra_flag_values, - filter_configuration=filter_configuration, - git_commit=git_commit, - max_reports_per_category=MAX_REPORTS_PER_CATEGORY, - output_format=output_format, - verbosity=verbosity) - - return (filenames, options) + return StyleCheckerConfiguration(filter_configuration=filter_configuration, + max_reports_per_category=_MAX_REPORTS_PER_CATEGORY, + output_format=options.output_format, + stderr_write=sys.stderr.write, + verbosity=options.verbosity) # Enum-like idiom @@ -617,14 +228,14 @@ class ProcessorDispatcher(object): def should_skip_with_warning(self, file_path): """Return whether the given file should be skipped with a warning.""" - for skipped_file in SKIPPED_FILES_WITH_WARNING: + for skipped_file in _SKIPPED_FILES_WITH_WARNING: if file_path.find(skipped_file) >= 0: return True return False def should_skip_without_warning(self, file_path): """Return whether the given file should be skipped without a warning.""" - for skipped_file in SKIPPED_FILES_WITHOUT_WARNING: + for skipped_file in _SKIPPED_FILES_WITHOUT_WARNING: if file_path.find(skipped_file) >= 0: return True return False @@ -678,16 +289,95 @@ class ProcessorDispatcher(object): return processor -# FIXME: When creating the new CheckWebKitStyleOptions class as -# described in a FIXME above, add a new class here called -# something like CheckerConfiguration. The class should contain -# attributes for options needed to process a file. This includes -# a subset of the CheckWebKitStyleOptions attributes, a -# FilterConfiguration attribute, an stderr_write attribute, a -# max_reports_per_category attribute, etc. It can also include -# the is_reportable() method. The StyleChecker should accept -# an instance of this class rather than a ProcessorOptions -# instance. +class StyleCheckerConfiguration(object): + + """Stores configuration values for the StyleChecker class. + + Attributes: + max_reports_per_category: The maximum number of errors to report + per category, per file. + + stderr_write: A function that takes a string as a parameter and + serves as stderr.write. + + verbosity: An integer between 1-5 inclusive that restricts output + to errors with a confidence score at or above this value. + + """ + + def __init__(self, + filter_configuration, + max_reports_per_category, + output_format, + stderr_write, + verbosity): + """Create a StyleCheckerConfiguration instance. + + Args: + filter_configuration: A FilterConfiguration instance. The default + is the "empty" filter configuration, which + means that all errors should be checked. + + max_reports_per_category: The maximum number of errors to report + per category, per file. + + output_format: A string that is the output format. The supported + output formats are "emacs" which emacs can parse + and "vs7" which Microsoft Visual Studio 7 can parse. + + stderr_write: A function that takes a string as a parameter and + serves as stderr.write. + + verbosity: An integer between 1-5 inclusive that restricts output + to errors with a confidence score at or above this value. + The default is 1, which reports all errors. + + """ + self._filter_configuration = filter_configuration + self._output_format = output_format + + self.max_reports_per_category = max_reports_per_category + self.stderr_write = stderr_write + self.verbosity = verbosity + + def is_reportable(self, category, confidence_in_error, file_path): + """Return whether an error is reportable. + + An error is reportable if both the confidence in the error is + at least the current verbosity level and the current filter + says the category should be checked for the given path. + + Args: + category: A string that is a style category. + confidence_in_error: An integer between 1 and 5, inclusive, that + represents the application's confidence in + the error. A higher number signifies greater + confidence. + file_path: The path of the file being checked + + """ + if confidence_in_error < self.verbosity: + return False + + return self._filter_configuration.should_check(category, file_path) + + def write_style_error(self, + category, + confidence, + file_path, + line_number, + message): + """Write a style error to the configured stderr.""" + if self._output_format == 'vs7': + format_string = "%s(%s): %s [%s] [%d]\n" + else: + format_string = "%s:%s: %s [%s] [%d]\n" + + self.stderr_write(format_string % (file_path, + line_number, + message, + category, + confidence)) class StyleChecker(object): @@ -698,28 +388,26 @@ class StyleChecker(object): error_count: An integer that is the total number of reported errors for the lifetime of this StyleChecker instance. - options: A ProcessorOptions instance that controls the behavior - of style checking. + file_count: An integer that is the total number of processed + files. Note that the number of skipped files is + included in this value. """ - def __init__(self, options, stderr_write=None): + def __init__(self, configuration): """Create a StyleChecker instance. Args: - options: See options attribute. - stderr_write: A function that takes a string as a parameter - and that is called when a style error occurs. - Defaults to sys.stderr.write. This should be - used only for unit tests. + configuration: A StyleCheckerConfiguration instance that controls + the behavior of style checking. """ - if stderr_write is None: - stderr_write = sys.stderr.write - - self._stderr_write = stderr_write + self._configuration = configuration self.error_count = 0 - self.options = options + self.file_count = 0 + + def _stderr_write(self, message): + self._configuration.stderr_write(message) def _increment_error_count(self): """Increment the total count of reported errors.""" @@ -784,13 +472,16 @@ class StyleChecker(object): """ if handle_style_error is None: - handle_style_error = DefaultStyleErrorHandler(file_path, - self.options, - self._increment_error_count, - self._stderr_write) + handle_style_error = DefaultStyleErrorHandler( + configuration=self._configuration, + file_path=file_path, + increment_error_count= + self._increment_error_count) if process_file is None: process_file = self._process_file + self.file_count += 1 + dispatcher = ProcessorDispatcher() if dispatcher.should_skip_without_warning(file_path): @@ -800,7 +491,7 @@ class StyleChecker(object): "style guide.\n" % file_path) return - verbosity = self.options.verbosity + verbosity = self._configuration.verbosity processor = dispatcher.dispatch_processor(file_path, handle_style_error, verbosity) @@ -820,9 +511,7 @@ class StyleChecker(object): for file_path, diff in patch_files.iteritems(): style_error_handler = PatchStyleErrorHandler(diff, file_path, - self.options, - self._increment_error_count, - self._stderr_write) + self._configuration, + self._increment_error_count) self.check_file(file_path, style_error_handler) - diff --git a/WebKitTools/Scripts/webkitpy/style/checker_unittest.py b/WebKitTools/Scripts/webkitpy/style/checker_unittest.py index e1c9baf..fe12512 100755 --- a/WebKitTools/Scripts/webkitpy/style/checker_unittest.py +++ b/WebKitTools/Scripts/webkitpy/style/checker_unittest.py @@ -37,119 +37,40 @@ import unittest import checker as style +from checker import _BASE_FILTER_RULES +from checker import _MAX_REPORTS_PER_CATEGORY from checker import _PATH_RULES_SPECIFIER as PATH_RULES_SPECIFIER -from checker import style_categories +from checker import _all_categories +from checker import check_webkit_style_configuration +from checker import check_webkit_style_parser from checker import ProcessorDispatcher -from checker import ProcessorOptions from checker import StyleChecker +from checker import StyleCheckerConfiguration from filter import validate_filter_rules from filter import FilterConfiguration +from optparser import ArgumentParser +from optparser import CommandOptionValues from processors.cpp import CppProcessor from processors.text import TextProcessor -class ProcessorOptionsTest(unittest.TestCase): - - """Tests ProcessorOptions class.""" - - def test_init(self): - """Test __init__ constructor.""" - # Check default parameters. - options = ProcessorOptions() - self.assertEquals(options.extra_flag_values, {}) - self.assertEquals(options.filter_configuration, FilterConfiguration()) - self.assertEquals(options.git_commit, None) - self.assertEquals(options.max_reports_per_category, {}) - self.assertEquals(options.output_format, "emacs") - self.assertEquals(options.verbosity, 1) - - # Check argument validation. - self.assertRaises(ValueError, ProcessorOptions, output_format="bad") - ProcessorOptions(output_format="emacs") # No ValueError: works - ProcessorOptions(output_format="vs7") # works - self.assertRaises(ValueError, ProcessorOptions, verbosity=0) - self.assertRaises(ValueError, ProcessorOptions, verbosity=6) - ProcessorOptions(verbosity=1) # works - ProcessorOptions(verbosity=5) # works - - # Check attributes. - filter_configuration = FilterConfiguration(base_rules=["+"]) - options = ProcessorOptions(extra_flag_values={"extra_value" : 2}, - filter_configuration=filter_configuration, - git_commit="commit", - max_reports_per_category={"category": 3}, - output_format="vs7", - verbosity=3) - self.assertEquals(options.extra_flag_values, {"extra_value" : 2}) - self.assertEquals(options.filter_configuration, filter_configuration) - self.assertEquals(options.git_commit, "commit") - self.assertEquals(options.max_reports_per_category, {"category": 3}) - self.assertEquals(options.output_format, "vs7") - self.assertEquals(options.verbosity, 3) - - def test_eq(self): - """Test __eq__ equality function.""" - # == calls __eq__. - self.assertTrue(ProcessorOptions() == ProcessorOptions()) - - # Verify that a difference in any argument causes equality to fail. - filter_configuration = FilterConfiguration(base_rules=["+"]) - options = ProcessorOptions(extra_flag_values={"extra_value" : 1}, - filter_configuration=filter_configuration, - git_commit="commit", - max_reports_per_category={"category": 3}, - output_format="vs7", - verbosity=1) - self.assertFalse(options == ProcessorOptions(extra_flag_values={"extra_value" : 2})) - new_config = FilterConfiguration(base_rules=["-"]) - self.assertFalse(options == - ProcessorOptions(filter_configuration=new_config)) - self.assertFalse(options == ProcessorOptions(git_commit="commit2")) - self.assertFalse(options == ProcessorOptions(max_reports_per_category= - {"category": 2})) - self.assertFalse(options == ProcessorOptions(output_format="emacs")) - self.assertFalse(options == ProcessorOptions(verbosity=2)) - - def test_ne(self): - """Test __ne__ inequality function.""" - # != calls __ne__. - # By default, __ne__ always returns true on different objects. - # Thus, just check the distinguishing case to verify that the - # code defines __ne__. - self.assertFalse(ProcessorOptions() != ProcessorOptions()) - - def test_is_reportable(self): - """Test is_reportable().""" - filter_configuration = FilterConfiguration(base_rules=["-xyz"]) - options = ProcessorOptions(filter_configuration=filter_configuration, - verbosity=3) - - # Test verbosity - self.assertTrue(options.is_reportable("abc", 3, "foo.h")) - self.assertFalse(options.is_reportable("abc", 2, "foo.h")) - - # Test filter - self.assertTrue(options.is_reportable("xy", 3, "foo.h")) - self.assertFalse(options.is_reportable("xyz", 3, "foo.h")) - - class GlobalVariablesTest(unittest.TestCase): """Tests validity of the global variables.""" def _all_categories(self): - return style.style_categories() + return _all_categories() def defaults(self): - return style.webkit_argument_defaults() + return style._check_webkit_style_defaults() - def test_filter_rules(self): + def test_webkit_base_filter_rules(self): + base_filter_rules = _BASE_FILTER_RULES defaults = self.defaults() already_seen = [] - validate_filter_rules(defaults.base_filter_rules, - self._all_categories()) + validate_filter_rules(base_filter_rules, self._all_categories()) # Also do some additional checks. - for rule in defaults.base_filter_rules: + for rule in base_filter_rules: # Check no leading or trailing white space. self.assertEquals(rule, rule.strip()) # All categories are on by default, so defaults should @@ -161,203 +82,77 @@ class GlobalVariablesTest(unittest.TestCase): def test_defaults(self): """Check that default arguments are valid.""" - defaults = self.defaults() + default_options = self.defaults() # FIXME: We should not need to call parse() to determine # whether the default arguments are valid. - parser = style.ArgumentParser(defaults) + parser = ArgumentParser(all_categories=self._all_categories(), + base_filter_rules=[], + default_options=default_options) # No need to test the return value here since we test parse() # on valid arguments elsewhere. parser.parse([]) # arguments valid: no error or SystemExit def test_path_rules_specifier(self): - all_categories = style_categories() + all_categories = self._all_categories() for (sub_paths, path_rules) in PATH_RULES_SPECIFIER: - self.assertTrue(isinstance(path_rules, tuple), - "Checking: " + str(path_rules)) validate_filter_rules(path_rules, self._all_categories()) - # Try using the path specifier (as an "end-to-end" check). config = FilterConfiguration(path_specific=PATH_RULES_SPECIFIER) - self.assertTrue(config.should_check("xxx_any_category", - "xxx_non_matching_path")) - self.assertTrue(config.should_check("xxx_any_category", - "WebKitTools/WebKitAPITest/")) - self.assertFalse(config.should_check("build/include", - "WebKitTools/WebKitAPITest/")) - self.assertFalse(config.should_check("readability/naming", - "WebKit/qt/tests/qwebelement/tst_qwebelement.cpp")) + + def assertCheck(path, category): + """Assert that the given category should be checked.""" + message = ('Should check category "%s" for path "%s".' + % (category, path)) + self.assertTrue(config.should_check(category, path)) + + def assertNoCheck(path, category): + """Assert that the given category should not be checked.""" + message = ('Should not check category "%s" for path "%s".' + % (category, path)) + self.assertFalse(config.should_check(category, path), message) + + assertCheck("random_path.cpp", + "build/include") + assertNoCheck("WebKitTools/WebKitAPITest/main.cpp", + "build/include") + assertNoCheck("WebKit/qt/QGVLauncher/main.cpp", + "build/include") + assertNoCheck("WebKit/qt/QGVLauncher/main.cpp", + "readability/streams") + + assertCheck("random_path.cpp", + "readability/naming") + assertNoCheck("WebKit/gtk/webkit/webkit.h", + "readability/naming") + assertNoCheck("WebCore/css/CSSParser.cpp", + "readability/naming") + assertNoCheck("WebKit/qt/tests/qwebelement/tst_qwebelement.cpp", + "readability/naming") + assertNoCheck( + "JavaScriptCore/qt/tests/qscriptengine/tst_qscriptengine.cpp", + "readability/naming") def test_max_reports_per_category(self): - """Check that MAX_REPORTS_PER_CATEGORY is valid.""" + """Check that _MAX_REPORTS_PER_CATEGORY is valid.""" all_categories = self._all_categories() - for category in style.MAX_REPORTS_PER_CATEGORY.iterkeys(): + for category in _MAX_REPORTS_PER_CATEGORY.iterkeys(): self.assertTrue(category in all_categories, 'Key "%s" is not a category' % category) -class ArgumentPrinterTest(unittest.TestCase): - - """Tests the ArgumentPrinter class.""" - - _printer = style.ArgumentPrinter() - - def _create_options(self, - output_format='emacs', - verbosity=3, - user_rules=[], - git_commit=None, - extra_flag_values={}): - filter_configuration = FilterConfiguration(user_rules=user_rules) - return style.ProcessorOptions(extra_flag_values=extra_flag_values, - filter_configuration=filter_configuration, - git_commit=git_commit, - output_format=output_format, - verbosity=verbosity) +class CheckWebKitStyleFunctionTest(unittest.TestCase): + + """Tests the functions with names of the form check_webkit_style_*.""" + + def test_check_webkit_style_configuration(self): + # Exercise the code path to make sure the function does not error out. + option_values = CommandOptionValues() + configuration = check_webkit_style_configuration(option_values) - def test_to_flag_string(self): - options = self._create_options('vs7', 5, ['+foo', '-bar'], 'git', - {'a': 0, 'z': 1}) - self.assertEquals('--a=0 --filter=+foo,-bar --git-commit=git ' - '--output=vs7 --verbose=5 --z=1', - self._printer.to_flag_string(options)) - - # This is to check that --filter and --git-commit do not - # show up when not user-specified. - options = self._create_options() - self.assertEquals('--output=emacs --verbose=3', - self._printer.to_flag_string(options)) - - -class ArgumentParserTest(unittest.TestCase): - - """Test the ArgumentParser class.""" - - def _parse(self): - """Return a default parse() function for testing.""" - return self._create_parser().parse - - def _create_defaults(self, default_output_format='vs7', - default_verbosity=3, - default_filter_rules=['-', '+whitespace']): - """Return a default ArgumentDefaults instance for testing.""" - return style.ArgumentDefaults(default_output_format, - default_verbosity, - default_filter_rules) - - def _create_parser(self, defaults=None): - """Return an ArgumentParser instance for testing.""" - def create_usage(_defaults): - """Return a usage string for testing.""" - return "usage" - - def doc_print(message): - # We do not want the usage string or style categories - # to print during unit tests, so print nothing. - return - - if defaults is None: - defaults = self._create_defaults() - - return style.ArgumentParser(defaults, create_usage, doc_print) - - def test_parse_documentation(self): - parse = self._parse() - - # FIXME: Test both the printing of the usage string and the - # filter categories help. - - # Request the usage string. - self.assertRaises(SystemExit, parse, ['--help']) - # Request default filter rules and available style categories. - self.assertRaises(SystemExit, parse, ['--filter=']) - - def test_parse_bad_values(self): - parse = self._parse() - - # Pass an unsupported argument. - self.assertRaises(SystemExit, parse, ['--bad']) - - self.assertRaises(ValueError, parse, ['--verbose=bad']) - self.assertRaises(ValueError, parse, ['--verbose=0']) - self.assertRaises(ValueError, parse, ['--verbose=6']) - parse(['--verbose=1']) # works - parse(['--verbose=5']) # works - - self.assertRaises(ValueError, parse, ['--output=bad']) - parse(['--output=vs7']) # works - - # Pass a filter rule not beginning with + or -. - self.assertRaises(ValueError, parse, ['--filter=build']) - parse(['--filter=+build']) # works - # Pass files and git-commit at the same time. - self.assertRaises(SystemExit, parse, ['--git-commit=', 'file.txt']) - # Pass an extra flag already supported. - self.assertRaises(ValueError, parse, [], ['filter=']) - parse([], ['extra=']) # works - # Pass an extra flag with typo. - self.assertRaises(SystemExit, parse, ['--extratypo='], ['extra=']) - parse(['--extra='], ['extra=']) # works - self.assertRaises(ValueError, parse, [], ['extra=', 'extra=']) - - - def test_parse_default_arguments(self): - parse = self._parse() - - (files, options) = parse([]) - - self.assertEquals(files, []) - - self.assertEquals(options.output_format, 'vs7') - self.assertEquals(options.verbosity, 3) - self.assertEquals(options.filter_configuration, - FilterConfiguration(base_rules=["-", "+whitespace"], - path_specific=PATH_RULES_SPECIFIER)) - self.assertEquals(options.git_commit, None) - - def test_parse_explicit_arguments(self): - parse = self._parse() - - # Pass non-default explicit values. - (files, options) = parse(['--output=emacs']) - self.assertEquals(options.output_format, 'emacs') - (files, options) = parse(['--verbose=4']) - self.assertEquals(options.verbosity, 4) - (files, options) = parse(['--git-commit=commit']) - self.assertEquals(options.git_commit, 'commit') - - # Pass user_rules. - (files, options) = parse(['--filter=+build,-whitespace']) - config = options.filter_configuration - self.assertEquals(options.filter_configuration, - FilterConfiguration(base_rules=["-", "+whitespace"], - path_specific=PATH_RULES_SPECIFIER, - user_rules=["+build", "-whitespace"])) - - # Pass spurious white space in user rules. - (files, options) = parse(['--filter=+build, -whitespace']) - self.assertEquals(options.filter_configuration, - FilterConfiguration(base_rules=["-", "+whitespace"], - path_specific=PATH_RULES_SPECIFIER, - user_rules=["+build", "-whitespace"])) - - # Pass extra flag values. - (files, options) = parse(['--extra'], ['extra']) - self.assertEquals(options.extra_flag_values, {'--extra': ''}) - (files, options) = parse(['--extra='], ['extra=']) - self.assertEquals(options.extra_flag_values, {'--extra': ''}) - (files, options) = parse(['--extra=x'], ['extra=']) - self.assertEquals(options.extra_flag_values, {'--extra': 'x'}) - - def test_parse_files(self): - parse = self._parse() - - (files, options) = parse(['foo.cpp']) - self.assertEquals(files, ['foo.cpp']) - - # Pass multiple files. - (files, options) = parse(['--output=emacs', 'foo.cpp', 'bar.cpp']) - self.assertEquals(files, ['foo.cpp', 'bar.cpp']) + def test_check_webkit_style_parser(self): + # Exercise the code path to make sure the function does not error out. + parser = check_webkit_style_parser() class ProcessorDispatcherSkipTest(unittest.TestCase): @@ -519,30 +314,96 @@ class ProcessorDispatcherDispatchTest(unittest.TestCase): self.assert_processor_none(path) -class StyleCheckerTest(unittest.TestCase): +class StyleCheckerConfigurationTest(unittest.TestCase): - """Test the StyleChecker class. + """Tests the StyleCheckerConfiguration class.""" - Attributes: - error_messages: A string containing all of the warning messages - written to the mock_stderr_write method of - this class. + def setUp(self): + self._error_messages = [] + """The messages written to _mock_stderr_write() of this class.""" - """ + def _mock_stderr_write(self, message): + self._error_messages.append(message) + + def _style_checker_configuration(self, output_format="vs7"): + """Return a StyleCheckerConfiguration instance for testing.""" + base_rules = ["-whitespace", "+whitespace/tab"] + filter_configuration = FilterConfiguration(base_rules=base_rules) + + return StyleCheckerConfiguration( + filter_configuration=filter_configuration, + max_reports_per_category={"whitespace/newline": 1}, + output_format=output_format, + stderr_write=self._mock_stderr_write, + verbosity=3) + + def test_init(self): + """Test the __init__() method.""" + configuration = self._style_checker_configuration() + + # Check that __init__ sets the "public" data attributes correctly. + self.assertEquals(configuration.max_reports_per_category, + {"whitespace/newline": 1}) + self.assertEquals(configuration.stderr_write, self._mock_stderr_write) + self.assertEquals(configuration.verbosity, 3) + + def test_is_reportable(self): + """Test the is_reportable() method.""" + config = self._style_checker_configuration() + + self.assertTrue(config.is_reportable("whitespace/tab", 3, "foo.txt")) + + # Test the confidence check code path by varying the confidence. + self.assertFalse(config.is_reportable("whitespace/tab", 2, "foo.txt")) + + # Test the category check code path by varying the category. + self.assertFalse(config.is_reportable("whitespace/line", 4, "foo.txt")) + + def _call_write_style_error(self, output_format): + config = self._style_checker_configuration(output_format=output_format) + config.write_style_error(category="whitespace/tab", + confidence=5, + file_path="foo.h", + line_number=100, + message="message") + + def test_write_style_error_emacs(self): + """Test the write_style_error() method.""" + self._call_write_style_error("emacs") + self.assertEquals(self._error_messages, + ["foo.h:100: message [whitespace/tab] [5]\n"]) + + def test_write_style_error_vs7(self): + """Test the write_style_error() method.""" + self._call_write_style_error("vs7") + self.assertEquals(self._error_messages, + ["foo.h(100): message [whitespace/tab] [5]\n"]) + + +class StyleCheckerTest(unittest.TestCase): + + """Test the StyleChecker class.""" def _mock_stderr_write(self, message): pass - def _style_checker(self, options): - return StyleChecker(options, self._mock_stderr_write) + def _style_checker(self, configuration): + return StyleChecker(configuration) def test_init(self): """Test __init__ constructor.""" - options = ProcessorOptions() - style_checker = self._style_checker(options) + configuration = StyleCheckerConfiguration( + filter_configuration=FilterConfiguration(), + max_reports_per_category={}, + output_format="vs7", + stderr_write=self._mock_stderr_write, + verbosity=3) + + style_checker = self._style_checker(configuration) + self.assertEquals(style_checker._configuration, configuration) self.assertEquals(style_checker.error_count, 0) - self.assertEquals(style_checker.options, options) + self.assertEquals(style_checker.file_count, 0) class StyleCheckerCheckFileTest(unittest.TestCase): @@ -609,20 +470,21 @@ class StyleCheckerCheckFileTest(unittest.TestCase): # Confirm that the attributes are reset. self.assert_attributes(None, None, None, "") - # Create a test StyleChecker instance. - # - # The verbosity attribute is the only ProcessorOptions - # attribute that needs to be checked in this test. - # This is because it is the only option is directly - # passed to the constructor of a style processor. - options = ProcessorOptions(verbosity=3) + configuration = StyleCheckerConfiguration( + filter_configuration=FilterConfiguration(), + max_reports_per_category={"whitespace/newline": 1}, + output_format="vs7", + stderr_write=self.mock_stderr_write, + verbosity=3) - style_checker = StyleChecker(options, self.mock_stderr_write) + style_checker = StyleChecker(configuration) style_checker.check_file(file_path, self.mock_handle_style_error, self.mock_process_file) + self.assertEquals(1, style_checker.file_count) + def test_check_file_on_skip_without_warning(self): """Test check_file() for a skipped-without-warning file.""" @@ -678,4 +540,3 @@ if __name__ == '__main__': import sys unittest.main() - diff --git a/WebKitTools/Scripts/webkitpy/style/error_handlers.py b/WebKitTools/Scripts/webkitpy/style/error_handlers.py index 1940e03..6bc3f15 100644 --- a/WebKitTools/Scripts/webkitpy/style/error_handlers.py +++ b/WebKitTools/Scripts/webkitpy/style/error_handlers.py @@ -56,30 +56,21 @@ class DefaultStyleErrorHandler(object): """The default style error handler.""" - def __init__(self, file_path, options, increment_error_count, - stderr_write=None): + def __init__(self, file_path, configuration, increment_error_count): """Create a default style error handler. Args: file_path: The path to the file containing the error. This is used for reporting to the user. - options: A ProcessorOptions instance. + configuration: A StyleCheckerConfiguration instance. increment_error_count: A function that takes no arguments and increments the total count of reportable errors. - stderr_write: A function that takes a string as a parameter - and that is called when a style error occurs. - Defaults to sys.stderr.write. This should be - used only for unit tests. """ - if stderr_write is None: - stderr_write = sys.stderr.write - self._file_path = file_path + self._configuration = configuration self._increment_error_count = increment_error_count - self._options = options - self._stderr_write = stderr_write # A string to integer dictionary cache of the number of reportable # errors per category passed to this instance. @@ -99,9 +90,9 @@ class DefaultStyleErrorHandler(object): def _max_reports(self, category): """Return the maximum number of errors to report.""" - if not category in self._options.max_reports_per_category: + if not category in self._configuration.max_reports_per_category: return None - return self._options.max_reports_per_category[category] + return self._configuration.max_reports_per_category[category] def __call__(self, line_number, category, confidence, message): """Handle the occurrence of a style error. @@ -109,9 +100,9 @@ class DefaultStyleErrorHandler(object): See the docstring of this module for more information. """ - if not self._options.is_reportable(category, - confidence, - self._file_path): + if not self._configuration.is_reportable(category=category, + confidence_in_error=confidence, + file_path=self._file_path): return category_total = self._add_reportable_error(category) @@ -122,28 +113,22 @@ class DefaultStyleErrorHandler(object): # Then suppress displaying the error. return - if self._options.output_format == 'vs7': - format_string = "%s(%s): %s [%s] [%d]\n" - else: - format_string = "%s:%s: %s [%s] [%d]\n" + self._configuration.write_style_error(category=category, + confidence=confidence, + file_path=self._file_path, + line_number=line_number, + message=message) if category_total == max_reports: - format_string += ("Suppressing further [%s] reports for this " - "file.\n" % category) - - self._stderr_write(format_string % (self._file_path, - line_number, - message, - category, - confidence)) + self._configuration.stderr_write("Suppressing further [%s] reports " + "for this file.\n" % category) class PatchStyleErrorHandler(object): """The style error function for patch files.""" - def __init__(self, diff, file_path, options, increment_error_count, - stderr_write): + def __init__(self, diff, file_path, configuration, increment_error_count): """Create a patch style error handler for the given path. Args: @@ -153,10 +138,12 @@ class PatchStyleErrorHandler(object): """ self._diff = diff - self._default_error_handler = DefaultStyleErrorHandler(file_path, - options, - increment_error_count, - stderr_write) + + self._default_error_handler = DefaultStyleErrorHandler( + configuration=configuration, + file_path=file_path, + increment_error_count= + increment_error_count) # The line numbers of the modified lines. This is set lazily. self._line_numbers = set() diff --git a/WebKitTools/Scripts/webkitpy/style/error_handlers_unittest.py b/WebKitTools/Scripts/webkitpy/style/error_handlers_unittest.py index 1d7e998..a39ba2a 100644 --- a/WebKitTools/Scripts/webkitpy/style/error_handlers_unittest.py +++ b/WebKitTools/Scripts/webkitpy/style/error_handlers_unittest.py @@ -26,124 +26,111 @@ import unittest from .. style_references import parse_patch -from checker import ProcessorOptions +from checker import StyleCheckerConfiguration from error_handlers import DefaultStyleErrorHandler from error_handlers import PatchStyleErrorHandler - +from filter import FilterConfiguration class StyleErrorHandlerTestBase(unittest.TestCase): def setUp(self): - self._error_messages = "" + self._error_messages = [] self._error_count = 0 def _mock_increment_error_count(self): self._error_count += 1 def _mock_stderr_write(self, message): - self._error_messages += message + self._error_messages.append(message) + + def _style_checker_configuration(self): + """Return a StyleCheckerConfiguration instance for testing.""" + base_rules = ["-whitespace", "+whitespace/tab"] + filter_configuration = FilterConfiguration(base_rules=base_rules) + + return StyleCheckerConfiguration( + filter_configuration=filter_configuration, + max_reports_per_category={"whitespace/tab": 2}, + output_format="vs7", + stderr_write=self._mock_stderr_write, + verbosity=3) class DefaultStyleErrorHandlerTest(StyleErrorHandlerTestBase): """Tests DefaultStyleErrorHandler class.""" - _file_path = "foo.h" - _category = "whitespace/tab" + """The category name for the tests in this class.""" - def _error_handler(self, options): - return DefaultStyleErrorHandler(self._file_path, - options, - self._mock_increment_error_count, - self._mock_stderr_write) + _file_path = "foo.h" + """The file path for the tests in this class.""" def _check_initialized(self): """Check that count and error messages are initialized.""" self.assertEquals(0, self._error_count) - self.assertEquals("", self._error_messages) - - def _call(self, handle_error, options, confidence): - """Handle an error with the given error handler.""" - line_number = 100 - message = "message" - - handle_error(line_number, self._category, confidence, message) - - def _call_error_handler(self, options, confidence): - """Handle an error using a new error handler.""" - handle_error = self._error_handler(options) - self._call(handle_error, options, confidence) - - def test_call_non_reportable(self): - """Test __call__() method with a non-reportable error.""" - confidence = 1 - options = ProcessorOptions(verbosity=3) + self.assertEquals(0, len(self._error_messages)) + + def _error_handler(self, configuration): + return DefaultStyleErrorHandler(configuration=configuration, + file_path=self._file_path, + increment_error_count=self._mock_increment_error_count) + + def _call_error_handler(self, handle_error, confidence): + """Call the given error handler with a test error.""" + handle_error(line_number=100, + category=self._category, + confidence=confidence, + message="message") + + def test_non_reportable_error(self): + """Test __call__() with a non-reportable error.""" self._check_initialized() + configuration = self._style_checker_configuration() + confidence = 1 # Confirm the error is not reportable. - self.assertFalse(options.is_reportable(self._category, - confidence, - self._file_path)) - - self._call_error_handler(options, confidence) + self.assertFalse(configuration.is_reportable(self._category, + confidence, + self._file_path)) + error_handler = self._error_handler(configuration) + self._call_error_handler(error_handler, confidence) self.assertEquals(0, self._error_count) - self.assertEquals("", self._error_messages) - - def test_call_reportable_emacs(self): - """Test __call__() method with a reportable error and emacs format.""" - confidence = 5 - options = ProcessorOptions(verbosity=3, output_format="emacs") - self._check_initialized() + self.assertEquals([], self._error_messages) - self._call_error_handler(options, confidence) - - self.assertEquals(1, self._error_count) - self.assertEquals(self._error_messages, - "foo.h:100: message [whitespace/tab] [5]\n") - - def test_call_reportable_vs7(self): - """Test __call__() method with a reportable error and vs7 format.""" - confidence = 5 - options = ProcessorOptions(verbosity=3, output_format="vs7") + # Also serves as a reportable error test. + def test_max_reports_per_category(self): + """Test error report suppression in __call__() method.""" self._check_initialized() + configuration = self._style_checker_configuration() + error_handler = self._error_handler(configuration) - self._call_error_handler(options, confidence) - - self.assertEquals(1, self._error_count) - self.assertEquals(self._error_messages, - "foo.h(100): message [whitespace/tab] [5]\n") - - def test_call_max_reports_per_category(self): - """Test error report suppression in __call__() method.""" confidence = 5 - options = ProcessorOptions(verbosity=3, - max_reports_per_category={self._category: 2}) - error_handler = self._error_handler(options) - - self._check_initialized() # First call: usual reporting. - self._call(error_handler, options, confidence) + self._call_error_handler(error_handler, confidence) self.assertEquals(1, self._error_count) + self.assertEquals(1, len(self._error_messages)) self.assertEquals(self._error_messages, - "foo.h:100: message [whitespace/tab] [5]\n") + ["foo.h(100): message [whitespace/tab] [5]\n"]) # Second call: suppression message reported. - self._error_messages = "" - self._call(error_handler, options, confidence) + self._call_error_handler(error_handler, confidence) + # The "Suppressing further..." message counts as an additional + # message (but not as an addition to the error count). self.assertEquals(2, self._error_count) - self.assertEquals(self._error_messages, - "foo.h:100: message [whitespace/tab] [5]\n" - "Suppressing further [%s] reports for this file.\n" - % self._category) + self.assertEquals(3, len(self._error_messages)) + self.assertEquals(self._error_messages[-2], + "foo.h(100): message [whitespace/tab] [5]\n") + self.assertEquals(self._error_messages[-1], + "Suppressing further [whitespace/tab] reports " + "for this file.\n") # Third call: no report. - self._error_messages = "" - self._call(error_handler, options, confidence) + self._call_error_handler(error_handler, confidence) self.assertEquals(3, self._error_count) - self.assertEquals(self._error_messages, "") + self.assertEquals(3, len(self._error_messages)) class PatchStyleErrorHandlerTest(StyleErrorHandlerTestBase): @@ -166,22 +153,22 @@ index ef65bee..e3db70e 100644 patch_files = parse_patch(self._patch_string) diff = patch_files[self._file_path] - options = ProcessorOptions(verbosity=3) + configuration = self._style_checker_configuration() - handle_error = PatchStyleErrorHandler(diff, - self._file_path, - options, - self._mock_increment_error_count, - self._mock_stderr_write) + handle_error = PatchStyleErrorHandler(diff=diff, + file_path=self._file_path, + configuration=configuration, + increment_error_count= + self._mock_increment_error_count) category = "whitespace/tab" confidence = 5 message = "message" # Confirm error is reportable. - self.assertTrue(options.is_reportable(category, - confidence, - self._file_path)) + self.assertTrue(configuration.is_reportable(category, + confidence, + self._file_path)) # Confirm error count initialized to zero. self.assertEquals(0, self._error_count) diff --git a/WebKitTools/Scripts/webkitpy/style/filter.py b/WebKitTools/Scripts/webkitpy/style/filter.py index 19c2f4d..608a9e6 100644 --- a/WebKitTools/Scripts/webkitpy/style/filter.py +++ b/WebKitTools/Scripts/webkitpy/style/filter.py @@ -139,12 +139,8 @@ class FilterConfiguration(object): are appended. The first substring match takes precedence, i.e. only the first match triggers an append. - The "path_rules" value is the tuple of filter + The "path_rules" value is a list of filter rules that can be appended to the base rules. - The value is a tuple rather than a list so it - can be used as a dictionary key. The dictionary - is for caching purposes in the implementation of - this class. user_rules: A list of filter rules that is always appended to the base rules and any path rules. In other @@ -165,11 +161,7 @@ class FilterConfiguration(object): self._path_specific_lower = None """The backing store for self._get_path_specific_lower().""" - # FIXME: Make user rules internal after the FilterConfiguration - # attribute is removed from ProcessorOptions (since at - # that point ArgumentPrinter will no longer need to - # access FilterConfiguration.user_rules). - self.user_rules = user_rules + self._user_rules = user_rules self._path_rules_to_filter = {} """Cached dictionary of path rules to CategoryFilter instance.""" @@ -188,7 +180,7 @@ class FilterConfiguration(object): return False if self._path_specific != other._path_specific: return False - if self.user_rules != other.user_rules: + if self._user_rules != other._user_rules: return False return True @@ -210,22 +202,34 @@ class FilterConfiguration(object): return self._path_specific_lower def _path_rules_from_path(self, path): - """Determine the path-specific rules to use, and return as a tuple.""" + """Determine the path-specific rules to use, and return as a tuple. + + This method returns a tuple rather than a list so the return + value can be passed to _filter_from_path_rules() without change. + + """ path = path.lower() for (sub_paths, path_rules) in self._get_path_specific_lower(): for sub_path in sub_paths: if path.find(sub_path) > -1: - return path_rules + return tuple(path_rules) return () # Default to the empty tuple. def _filter_from_path_rules(self, path_rules): - """Return the CategoryFilter associated to a path rules tuple.""" + """Return the CategoryFilter associated to the given path rules. + + Args: + path_rules: A tuple of path rules. We require a tuple rather + than a list so the value can be used as a dictionary + key in self._path_rules_to_filter. + + """ # We reuse the same CategoryFilter where possible to take # advantage of the caching they do. if path_rules not in self._path_rules_to_filter: rules = list(self._base_rules) # Make a copy rules.extend(path_rules) - rules.extend(self.user_rules) + rules.extend(self._user_rules) self._path_rules_to_filter[path_rules] = _CategoryFilter(rules) return self._path_rules_to_filter[path_rules] diff --git a/WebKitTools/Scripts/webkitpy/style/filter_unittest.py b/WebKitTools/Scripts/webkitpy/style/filter_unittest.py index 84760a5..7b8a5402 100644 --- a/WebKitTools/Scripts/webkitpy/style/filter_unittest.py +++ b/WebKitTools/Scripts/webkitpy/style/filter_unittest.py @@ -157,14 +157,14 @@ class FilterConfigurationTest(unittest.TestCase): # Test that the attributes are getting set correctly. # We use parameter values that are different from the defaults. base_rules = ["-"] - path_specific = [(["path"], ("+a",))] + path_specific = [(["path"], ["+a"])] user_rules = ["+"] config = self._config(base_rules, path_specific, user_rules) self.assertEquals(base_rules, config._base_rules) self.assertEquals(path_specific, config._path_specific) - self.assertEquals(user_rules, config.user_rules) + self.assertEquals(user_rules, config._user_rules) def test_default_arguments(self): # Test that the attributes are getting set correctly to the defaults. @@ -172,7 +172,7 @@ class FilterConfigurationTest(unittest.TestCase): self.assertEquals([], config._base_rules) self.assertEquals([], config._path_specific) - self.assertEquals([], config.user_rules) + self.assertEquals([], config._user_rules) def test_eq(self): """Test __eq__ method.""" @@ -185,7 +185,7 @@ class FilterConfigurationTest(unittest.TestCase): # These parameter values are different from the defaults. base_rules = ["-"] - path_specific = [(["path"], ("+a",))] + path_specific = [(["path"], ["+a"])] user_rules = ["+"] self.assertFalse(config.__eq__(FilterConfiguration( @@ -219,8 +219,8 @@ class FilterConfigurationTest(unittest.TestCase): def test_path_specific(self): """Test effect of path_rules_specifier on should_check().""" base_rules = ["-"] - path_specific = [(["path1"], ("+b",)), - (["path2"], ("+c",))] + path_specific = [(["path1"], ["+b"]), + (["path2"], ["+c"])] user_rules = [] config = self._config(base_rules, path_specific, user_rules) @@ -233,7 +233,7 @@ class FilterConfigurationTest(unittest.TestCase): def test_path_with_different_case(self): """Test a path that differs only in case.""" base_rules = ["-"] - path_specific = [(["Foo/"], ("+whitespace",))] + path_specific = [(["Foo/"], ["+whitespace"])] user_rules = [] config = self._config(base_rules, path_specific, user_rules) diff --git a/WebKitTools/Scripts/webkitpy/style/optparser.py b/WebKitTools/Scripts/webkitpy/style/optparser.py new file mode 100644 index 0000000..4137c8b --- /dev/null +++ b/WebKitTools/Scripts/webkitpy/style/optparser.py @@ -0,0 +1,424 @@ +# Copyright (C) 2010 Chris Jerdonek (cjerdonek@webkit.org) +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions +# are met: +# 1. Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# 2. Redistributions in binary form must reproduce the above copyright +# notice, this list of conditions and the following disclaimer in the +# documentation and/or other materials provided with the distribution. +# +# THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS'' AND ANY +# EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED +# WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE +# DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS BE LIABLE FOR ANY +# DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES +# (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; +# LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON +# ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS +# SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +"""Supports the parsing of command-line options for check-webkit-style.""" + +import getopt +import os.path +import sys + +from filter import validate_filter_rules +# This module should not import anything from checker.py. + + +def _create_usage(default_options): + """Return the usage string to display for command help. + + Args: + default_options: A DefaultCommandOptionValues instance. + + """ + usage = """ +Syntax: %(program_name)s [--verbose=#] [--git-commit=<SingleCommit>] [--output=vs7] + [--filter=-x,+y,...] [file] ... + + The style guidelines this tries to follow are here: + http://webkit.org/coding/coding-style.html + + Every style error is given a confidence score from 1-5, with 5 meaning + we are certain of the problem, and 1 meaning it could be a legitimate + construct. This can miss some errors and does not substitute for + code review. + + To prevent specific lines from being linted, add a '// NOLINT' comment to the + end of the line. + + Linted extensions are .cpp, .c and .h. Other file types are ignored. + + The file parameter is optional and accepts multiple files. Leaving + out the file parameter applies the check to all files considered changed + by your source control management system. + + Flags: + + verbose=# + A number 1-5 that restricts output to errors with a confidence + score at or above this value. In particular, the value 1 displays + all errors. The default is %(default_verbosity)s. + + git-commit=<SingleCommit> + Checks the style of everything from the given commit to the local tree. + + output=vs7 + The output format, which may be one of + emacs : to ease emacs parsing + vs7 : compatible with Visual Studio + Defaults to "%(default_output_format)s". Other formats are unsupported. + + filter=-x,+y,... + A comma-separated list of boolean filter rules used to filter + which categories of style guidelines to check. The script checks + a category if the category passes the filter rules, as follows. + + Any webkit category starts out passing. All filter rules are then + evaluated left to right, with later rules taking precedence. For + example, the rule "+foo" passes any category that starts with "foo", + and "-foo" fails any such category. The filter input "-whitespace, + +whitespace/braces" fails the category "whitespace/tab" and passes + "whitespace/braces". + + Examples: --filter=-whitespace,+whitespace/braces + --filter=-whitespace,-runtime/printf,+runtime/printf_format + --filter=-,+build/include_what_you_use + + Category names appear in error messages in brackets, for example + [whitespace/indent]. To see a list of all categories available to + %(program_name)s, along with which are enabled by default, pass + the empty filter as follows: + --filter= +""" % {'program_name': os.path.basename(sys.argv[0]), + 'default_verbosity': default_options.verbosity, + 'default_output_format': default_options.output_format} + + return usage + + +# This class should not have knowledge of the flag key names. +class DefaultCommandOptionValues(object): + + """Stores the default check-webkit-style command-line options. + + Attributes: + output_format: A string that is the default output format. + verbosity: An integer that is the default verbosity level. + + """ + + def __init__(self, output_format, verbosity): + self.output_format = output_format + self.verbosity = verbosity + + +# FIXME: Eliminate support for "extra_flag_values". +# +# This class should not have knowledge of the flag key names. +class CommandOptionValues(object): + + """Stores the option values passed by the user via the command line. + + Attributes: + extra_flag_values: A string-string dictionary of all flag key-value + pairs that are not otherwise represented by this + class. The default is the empty dictionary. + + filter_rules: The list of filter rules provided by the user. + These rules are appended to the base rules and + path-specific rules and so take precedence over + the base filter rules, etc. + + git_commit: A string representing the git commit to check. + The default is None. + + output_format: A string that is the output format. The supported + output formats are "emacs" which emacs can parse + and "vs7" which Microsoft Visual Studio 7 can parse. + + verbosity: An integer between 1-5 inclusive that restricts output + to errors with a confidence score at or above this value. + The default is 1, which reports all errors. + + """ + def __init__(self, + extra_flag_values=None, + filter_rules=None, + git_commit=None, + output_format="emacs", + verbosity=1): + if extra_flag_values is None: + extra_flag_values = {} + if filter_rules is None: + filter_rules = [] + + if output_format not in ("emacs", "vs7"): + raise ValueError('Invalid "output_format" parameter: ' + 'value must be "emacs" or "vs7". ' + 'Value given: "%s".' % output_format) + + if (verbosity < 1) or (verbosity > 5): + raise ValueError('Invalid "verbosity" parameter: ' + "value must be an integer between 1-5 inclusive. " + 'Value given: "%s".' % verbosity) + + self.extra_flag_values = extra_flag_values + self.filter_rules = filter_rules + self.git_commit = git_commit + self.output_format = output_format + self.verbosity = verbosity + + # Useful for unit testing. + def __eq__(self, other): + """Return whether this instance is equal to another.""" + if self.extra_flag_values != other.extra_flag_values: + return False + if self.filter_rules != other.filter_rules: + return False + if self.git_commit != other.git_commit: + return False + if self.output_format != other.output_format: + return False + if self.verbosity != other.verbosity: + return False + + return True + + # Useful for unit testing. + def __ne__(self, other): + # Python does not automatically deduce this from __eq__(). + return not self.__eq__(other) + + +class ArgumentPrinter(object): + + """Supports the printing of check-webkit-style command arguments.""" + + def _flag_pair_to_string(self, flag_key, flag_value): + return '--%(key)s=%(val)s' % {'key': flag_key, 'val': flag_value } + + def to_flag_string(self, options): + """Return a flag string of the given CommandOptionValues instance. + + This method orders the flag values alphabetically by the flag key. + + Args: + options: A CommandOptionValues instance. + + """ + flags = options.extra_flag_values.copy() + + flags['output'] = options.output_format + flags['verbose'] = options.verbosity + # Only include the filter flag if user-provided rules are present. + filter_rules = options.filter_rules + if filter_rules: + flags['filter'] = ",".join(filter_rules) + if options.git_commit: + flags['git-commit'] = options.git_commit + + flag_string = '' + # Alphabetizing lets us unit test this method. + for key in sorted(flags.keys()): + flag_string += self._flag_pair_to_string(key, flags[key]) + ' ' + + return flag_string.strip() + + +# FIXME: Replace the use of getopt.getopt() with optparse.OptionParser. +class ArgumentParser(object): + + # FIXME: Move the documentation of the attributes to the __init__ + # docstring after making the attributes internal. + """Supports the parsing of check-webkit-style command arguments. + + Attributes: + create_usage: A function that accepts a DefaultCommandOptionValues + instance and returns a string of usage instructions. + Defaults to the function that generates the usage + string for check-webkit-style. + default_options: A DefaultCommandOptionValues instance that provides + the default values for options not explicitly + provided by the user. + stderr_write: A function that takes a string as a parameter and + serves as stderr.write. Defaults to sys.stderr.write. + This parameter should be specified only for unit tests. + + """ + + def __init__(self, + all_categories, + default_options, + base_filter_rules=None, + create_usage=None, + stderr_write=None): + """Create an ArgumentParser instance. + + Args: + all_categories: The set of all available style categories. + default_options: See the corresponding attribute in the class + docstring. + Keyword Args: + base_filter_rules: The list of filter rules at the beginning of + the list of rules used to check style. This + list has the least precedence when checking + style and precedes any user-provided rules. + The class uses this parameter only for display + purposes to the user. Defaults to the empty list. + create_usage: See the documentation of the corresponding + attribute in the class docstring. + stderr_write: See the documentation of the corresponding + attribute in the class docstring. + + """ + if base_filter_rules is None: + base_filter_rules = [] + if create_usage is None: + create_usage = _create_usage + if stderr_write is None: + stderr_write = sys.stderr.write + + self._all_categories = all_categories + self._base_filter_rules = base_filter_rules + # FIXME: Rename these to reflect that they are internal. + self.create_usage = create_usage + self.default_options = default_options + self.stderr_write = stderr_write + + def _exit_with_usage(self, error_message=''): + """Exit and print a usage string with an optional error message. + + Args: + error_message: A string that is an error message to print. + + """ + usage = self.create_usage(self.default_options) + self.stderr_write(usage) + if error_message: + sys.exit('\nFATAL ERROR: ' + error_message) + else: + sys.exit(1) + + def _exit_with_categories(self): + """Exit and print the style categories and default filter rules.""" + self.stderr_write('\nAll categories:\n') + for category in sorted(self._all_categories): + self.stderr_write(' ' + category + '\n') + + self.stderr_write('\nDefault filter rules**:\n') + for filter_rule in sorted(self._base_filter_rules): + self.stderr_write(' ' + filter_rule + '\n') + self.stderr_write('\n**The command always evaluates the above rules, ' + 'and before any --filter flag.\n\n') + + sys.exit(0) + + def _parse_filter_flag(self, flag_value): + """Parse the --filter flag, and return a list of filter rules. + + Args: + flag_value: A string of comma-separated filter rules, for + example "-whitespace,+whitespace/indent". + + """ + filters = [] + for uncleaned_filter in flag_value.split(','): + filter = uncleaned_filter.strip() + if not filter: + continue + filters.append(filter) + return filters + + def parse(self, args, extra_flags=None): + """Parse the command line arguments to check-webkit-style. + + Args: + args: A list of command-line arguments as returned by sys.argv[1:]. + extra_flags: A list of flags whose values we want to extract, but + are not supported by the CommandOptionValues class. + An example flag "new_flag=". This defaults to the + empty list. + + Returns: + A tuple of (filenames, options) + + filenames: The list of filenames to check. + options: A CommandOptionValues instance. + + """ + if extra_flags is None: + extra_flags = [] + + output_format = self.default_options.output_format + verbosity = self.default_options.verbosity + + # The flags already supported by the CommandOptionValues class. + flags = ['help', 'output=', 'verbose=', 'filter=', 'git-commit='] + + for extra_flag in extra_flags: + if extra_flag in flags: + raise ValueError('Flag \'%(extra_flag)s is duplicated ' + 'or already supported.' % + {'extra_flag': extra_flag}) + flags.append(extra_flag) + + try: + (opts, filenames) = getopt.getopt(args, '', flags) + except getopt.GetoptError: + # FIXME: Settle on an error handling approach: come up + # with a consistent guideline as to when and whether + # a ValueError should be raised versus calling + # sys.exit when needing to interrupt execution. + self._exit_with_usage('Invalid arguments.') + + extra_flag_values = {} + git_commit = None + filter_rules = [] + + for (opt, val) in opts: + if opt == '--help': + self._exit_with_usage() + elif opt == '--output': + output_format = val + elif opt == '--verbose': + verbosity = val + elif opt == '--git-commit': + git_commit = val + elif opt == '--filter': + if not val: + self._exit_with_categories() + filter_rules = self._parse_filter_flag(val) + else: + extra_flag_values[opt] = val + + # Check validity of resulting values. + if filenames and (git_commit != None): + self._exit_with_usage('It is not possible to check files and a ' + 'specific commit at the same time.') + + if output_format not in ('emacs', 'vs7'): + raise ValueError('Invalid --output value "%s": The only ' + 'allowed output formats are emacs and vs7.' % + output_format) + + validate_filter_rules(filter_rules, self._all_categories) + + verbosity = int(verbosity) + if (verbosity < 1) or (verbosity > 5): + raise ValueError('Invalid --verbose value %s: value must ' + 'be between 1-5.' % verbosity) + + options = CommandOptionValues(extra_flag_values=extra_flag_values, + filter_rules=filter_rules, + git_commit=git_commit, + output_format=output_format, + verbosity=verbosity) + + return (filenames, options) + diff --git a/WebKitTools/Scripts/webkitpy/style/optparser_unittest.py b/WebKitTools/Scripts/webkitpy/style/optparser_unittest.py new file mode 100644 index 0000000..f23c5d1 --- /dev/null +++ b/WebKitTools/Scripts/webkitpy/style/optparser_unittest.py @@ -0,0 +1,258 @@ +# Copyright (C) 2010 Chris Jerdonek (cjerdonek@webkit.org) +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions +# are met: +# 1. Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# 2. Redistributions in binary form must reproduce the above copyright +# notice, this list of conditions and the following disclaimer in the +# documentation and/or other materials provided with the distribution. +# +# THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS'' AND ANY +# EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED +# WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE +# DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS BE LIABLE FOR ANY +# DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES +# (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; +# LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON +# ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS +# SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +"""Unit tests for parser.py.""" + +import unittest + +from optparser import _create_usage +from optparser import ArgumentParser +from optparser import ArgumentPrinter +from optparser import CommandOptionValues as ProcessorOptions +from optparser import DefaultCommandOptionValues + + +class CreateUsageTest(unittest.TestCase): + + """Tests the _create_usage() function.""" + + def test_create_usage(self): + default_options = DefaultCommandOptionValues(output_format="vs7", + verbosity=3) + # Exercise the code path to make sure the function does not error out. + _create_usage(default_options) + + +class ArgumentPrinterTest(unittest.TestCase): + + """Tests the ArgumentPrinter class.""" + + _printer = ArgumentPrinter() + + def _create_options(self, + output_format='emacs', + verbosity=3, + filter_rules=[], + git_commit=None, + extra_flag_values={}): + return ProcessorOptions(extra_flag_values=extra_flag_values, + filter_rules=filter_rules, + git_commit=git_commit, + output_format=output_format, + verbosity=verbosity) + + def test_to_flag_string(self): + options = self._create_options('vs7', 5, ['+foo', '-bar'], 'git', + {'a': 0, 'z': 1}) + self.assertEquals('--a=0 --filter=+foo,-bar --git-commit=git ' + '--output=vs7 --verbose=5 --z=1', + self._printer.to_flag_string(options)) + + # This is to check that --filter and --git-commit do not + # show up when not user-specified. + options = self._create_options() + self.assertEquals('--output=emacs --verbose=3', + self._printer.to_flag_string(options)) + + +class ArgumentParserTest(unittest.TestCase): + + """Test the ArgumentParser class.""" + + def _parse(self): + """Return a default parse() function for testing.""" + return self._create_parser().parse + + def _create_defaults(self): + """Return a DefaultCommandOptionValues instance for testing.""" + base_filter_rules = ["-", "+whitespace"] + return DefaultCommandOptionValues(output_format="vs7", + verbosity=3) + + def _create_parser(self): + """Return an ArgumentParser instance for testing.""" + def stderr_write(message): + # We do not want the usage string or style categories + # to print during unit tests, so print nothing. + return + + default_options = self._create_defaults() + + all_categories = ["build" ,"whitespace"] + return ArgumentParser(all_categories=all_categories, + base_filter_rules=[], + default_options=default_options, + stderr_write=stderr_write) + + def test_parse_documentation(self): + parse = self._parse() + + # FIXME: Test both the printing of the usage string and the + # filter categories help. + + # Request the usage string. + self.assertRaises(SystemExit, parse, ['--help']) + # Request default filter rules and available style categories. + self.assertRaises(SystemExit, parse, ['--filter=']) + + def test_parse_bad_values(self): + parse = self._parse() + + # Pass an unsupported argument. + self.assertRaises(SystemExit, parse, ['--bad']) + + self.assertRaises(ValueError, parse, ['--verbose=bad']) + self.assertRaises(ValueError, parse, ['--verbose=0']) + self.assertRaises(ValueError, parse, ['--verbose=6']) + parse(['--verbose=1']) # works + parse(['--verbose=5']) # works + + self.assertRaises(ValueError, parse, ['--output=bad']) + parse(['--output=vs7']) # works + + # Pass a filter rule not beginning with + or -. + self.assertRaises(ValueError, parse, ['--filter=build']) + parse(['--filter=+build']) # works + # Pass files and git-commit at the same time. + self.assertRaises(SystemExit, parse, ['--git-commit=', 'file.txt']) + # Pass an extra flag already supported. + self.assertRaises(ValueError, parse, [], ['filter=']) + parse([], ['extra=']) # works + # Pass an extra flag with typo. + self.assertRaises(SystemExit, parse, ['--extratypo='], ['extra=']) + parse(['--extra='], ['extra=']) # works + self.assertRaises(ValueError, parse, [], ['extra=', 'extra=']) + + + def test_parse_default_arguments(self): + parse = self._parse() + + (files, options) = parse([]) + + self.assertEquals(files, []) + + self.assertEquals(options.output_format, 'vs7') + self.assertEquals(options.verbosity, 3) + self.assertEquals(options.filter_rules, []) + self.assertEquals(options.git_commit, None) + + def test_parse_explicit_arguments(self): + parse = self._parse() + + # Pass non-default explicit values. + (files, options) = parse(['--output=emacs']) + self.assertEquals(options.output_format, 'emacs') + (files, options) = parse(['--verbose=4']) + self.assertEquals(options.verbosity, 4) + (files, options) = parse(['--git-commit=commit']) + self.assertEquals(options.git_commit, 'commit') + + # Pass user_rules. + (files, options) = parse(['--filter=+build,-whitespace']) + self.assertEquals(options.filter_rules, + ["+build", "-whitespace"]) + + # Pass spurious white space in user rules. + (files, options) = parse(['--filter=+build, -whitespace']) + self.assertEquals(options.filter_rules, + ["+build", "-whitespace"]) + + # Pass extra flag values. + (files, options) = parse(['--extra'], ['extra']) + self.assertEquals(options.extra_flag_values, {'--extra': ''}) + (files, options) = parse(['--extra='], ['extra=']) + self.assertEquals(options.extra_flag_values, {'--extra': ''}) + (files, options) = parse(['--extra=x'], ['extra=']) + self.assertEquals(options.extra_flag_values, {'--extra': 'x'}) + + def test_parse_files(self): + parse = self._parse() + + (files, options) = parse(['foo.cpp']) + self.assertEquals(files, ['foo.cpp']) + + # Pass multiple files. + (files, options) = parse(['--output=emacs', 'foo.cpp', 'bar.cpp']) + self.assertEquals(files, ['foo.cpp', 'bar.cpp']) + + +class CommandOptionValuesTest(unittest.TestCase): + + """Tests CommandOptionValues class.""" + + def test_init(self): + """Test __init__ constructor.""" + # Check default parameters. + options = ProcessorOptions() + self.assertEquals(options.extra_flag_values, {}) + self.assertEquals(options.filter_rules, []) + self.assertEquals(options.git_commit, None) + self.assertEquals(options.output_format, "emacs") + self.assertEquals(options.verbosity, 1) + + # Check argument validation. + self.assertRaises(ValueError, ProcessorOptions, output_format="bad") + ProcessorOptions(output_format="emacs") # No ValueError: works + ProcessorOptions(output_format="vs7") # works + self.assertRaises(ValueError, ProcessorOptions, verbosity=0) + self.assertRaises(ValueError, ProcessorOptions, verbosity=6) + ProcessorOptions(verbosity=1) # works + ProcessorOptions(verbosity=5) # works + + # Check attributes. + options = ProcessorOptions(extra_flag_values={"extra_value" : 2}, + filter_rules=["+"], + git_commit="commit", + output_format="vs7", + verbosity=3) + self.assertEquals(options.extra_flag_values, {"extra_value" : 2}) + self.assertEquals(options.filter_rules, ["+"]) + self.assertEquals(options.git_commit, "commit") + self.assertEquals(options.output_format, "vs7") + self.assertEquals(options.verbosity, 3) + + def test_eq(self): + """Test __eq__ equality function.""" + # == calls __eq__. + self.assertTrue(ProcessorOptions() == ProcessorOptions()) + + # Verify that a difference in any argument causes equality to fail. + options = ProcessorOptions(extra_flag_values={"extra_value" : 1}, + filter_rules=["+"], + git_commit="commit", + output_format="vs7", + verbosity=1) + self.assertFalse(options == ProcessorOptions(extra_flag_values= + {"extra_value" : 2})) + self.assertFalse(options == ProcessorOptions(filter_rules=["-"])) + self.assertFalse(options == ProcessorOptions(git_commit="commit2")) + self.assertFalse(options == ProcessorOptions(output_format="emacs")) + self.assertFalse(options == ProcessorOptions(verbosity=2)) + + def test_ne(self): + """Test __ne__ inequality function.""" + # != calls __ne__. + # By default, __ne__ always returns true on different objects. + # Thus, just check the distinguishing case to verify that the + # code defines __ne__. + self.assertFalse(ProcessorOptions() != ProcessorOptions()) + diff --git a/WebKitTools/Scripts/webkitpy/style/processors/cpp.py b/WebKitTools/Scripts/webkitpy/style/processors/cpp.py index 182c967..f83ae6a 100644 --- a/WebKitTools/Scripts/webkitpy/style/processors/cpp.py +++ b/WebKitTools/Scripts/webkitpy/style/processors/cpp.py @@ -1868,6 +1868,10 @@ def check_for_null(file_extension, clean_lines, line_number, error): if search(r'\bg_object_[sg]et\b', line): return + # Don't warn about NULL usage in g_str{join,concat}(). See Bug 34834 + if search(r'\bg_str(join|concat)\b', line): + return + if search(r'\bNULL\b', line): error(line_number, 'readability/null', 5, 'Use 0 instead of NULL.') return @@ -2421,7 +2425,9 @@ def check_identifier_name_in_declaration(filename, line_number, line, error): # Convert "long long", "long double", and "long long int" to # simple types, but don't remove simple "long". line = sub(r'long (long )?(?=long|double|int)', '', line) - line = sub(r'\b(unsigned|signed|inline|using|static|const|volatile|auto|register|extern|typedef|restrict|struct|class|virtual)(?=\W)', '', line) + # Convert unsigned/signed types to simple types, too. + line = sub(r'(unsigned|signed) (?=char|short|int|long)', '', line) + line = sub(r'\b(inline|using|static|const|volatile|auto|register|extern|typedef|restrict|struct|class|virtual)(?=\W)', '', line) # Remove all template parameters by removing matching < and >. # Loop until no templates are removed to remove nested templates. @@ -2449,8 +2455,9 @@ def check_identifier_name_in_declaration(filename, line_number, line, error): # Detect variable and functions. type_regexp = r'\w([\w]|\s*[*&]\s*|::)+' identifier_regexp = r'(?P<identifier>[\w:]+)' + maybe_bitfield_regexp = r'(:\s*\d+\s*)?' character_after_identifier_regexp = r'(?P<character_after_identifier>[[;()=,])(?!=)' - declaration_without_type_regexp = r'\s*' + identifier_regexp + r'\s*' + character_after_identifier_regexp + declaration_without_type_regexp = r'\s*' + identifier_regexp + r'\s*' + maybe_bitfield_regexp + character_after_identifier_regexp declaration_with_type_regexp = r'\s*' + type_regexp + r'\s' + declaration_without_type_regexp is_function_arguments = False number_of_identifiers = 0 @@ -2982,4 +2989,3 @@ class CppProcessor(object): def process_file_data(filename, file_extension, lines, error, verbosity): processor = CppProcessor(filename, file_extension, error, verbosity) processor.process(lines) - diff --git a/WebKitTools/Scripts/webkitpy/style/processors/cpp_unittest.py b/WebKitTools/Scripts/webkitpy/style/processors/cpp_unittest.py index fb5a487..c786b8e 100644 --- a/WebKitTools/Scripts/webkitpy/style/processors/cpp_unittest.py +++ b/WebKitTools/Scripts/webkitpy/style/processors/cpp_unittest.py @@ -3392,13 +3392,26 @@ class WebKitStyleTest(CppStyleTestBase): '', 'foo.m') - # Make sure that the NULL check does not apply to g_object_{set,get} + # Make sure that the NULL check does not apply to g_object_{set,get} and + # g_str{join,concat} self.assert_lint( 'g_object_get(foo, "prop", &bar, NULL);', '') self.assert_lint( 'g_object_set(foo, "prop", bar, NULL);', '') + self.assert_lint( + 'gchar* result = g_strconcat("part1", "part2", "part3", NULL);', + '') + self.assert_lint( + 'gchar* result = g_strconcat("part1", NULL);', + '') + self.assert_lint( + 'gchar* result = g_strjoin(",", "part1", "part2", "part3", NULL);', + '') + self.assert_lint( + 'gchar* result = g_strjoin(",", "part1", NULL);', + '') # 2. C++ and C bool values should be written as true and # false. Objective-C BOOL values should be written as YES and NO. @@ -3503,6 +3516,12 @@ class WebKitStyleTest(CppStyleTestBase): '_length' + name_error_message) self.assert_lint('short length_;', 'length_' + name_error_message) + self.assert_lint('unsigned _length;', + '_length' + name_error_message) + self.assert_lint('unsigned int _length;', + '_length' + name_error_message) + self.assert_lint('unsigned long long _length;', + '_length' + name_error_message) # Pointers, references, functions, templates, and adjectives. self.assert_lint('char* under_score;', @@ -3599,6 +3618,10 @@ class WebKitStyleTest(CppStyleTestBase): # const_iterator is allowed as well. self.assert_lint('typedef VectorType::const_iterator const_iterator;', '') + # Bitfields. + self.assert_lint('unsigned _fillRule : 1;', + '_fillRule' + name_error_message) + def test_comments(self): # A comment at the beginning of a line is ok. diff --git a/WebKitTools/Scripts/webkitpy/style/unittests.py b/WebKitTools/Scripts/webkitpy/style/unittests.py index f8e3f71..62615ab 100644 --- a/WebKitTools/Scripts/webkitpy/style/unittests.py +++ b/WebKitTools/Scripts/webkitpy/style/unittests.py @@ -38,6 +38,7 @@ import unittest from checker_unittest import * from error_handlers_unittest import * from filter_unittest import * +from optparser_unittest import * from processors.common_unittest import * from processors.cpp_unittest import * from processors.text_unittest import * |