Commit a84d0f87 authored by Igor Makarov's avatar Igor Makarov Committed by Commit Bot

Reformat python code in third_party/blink/tools/blinkpy/style for PEP8

- added .style.yapf file
- reformat *.py files

Bug: 1051750
Change-Id: I4d1e6054ce940d975710d8114f2f193304b554c2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2134236Reviewed-by: default avatarKent Tamura <tkent@chromium.org>
Commit-Queue: Kent Tamura <tkent@chromium.org>
Cr-Commit-Position: refs/heads/master@{#758456}
parent 2465afa5
......@@ -27,7 +27,6 @@
# 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.
"""Front end of some style-checker modules."""
import logging
......@@ -51,15 +50,12 @@ from blinkpy.style.filter import FilterConfiguration
from blinkpy.style.optparser import ArgumentParser
from blinkpy.style.optparser import DefaultCommandOptionValues
_log = logging.getLogger(__name__)
# These are default option values for the command-line option parser.
_DEFAULT_MIN_CONFIDENCE = 1
_DEFAULT_OUTPUT_FORMAT = 'emacs'
# FIXME: For style categories we will never want to have, remove them.
# For categories for which we want to have similar functionality,
# modify the implementation and enable them.
......@@ -102,7 +98,6 @@ _BASE_FILTER_RULES = [
# output.
]
# The path-specific filter rules.
#
# This list is order sensitive. Only the first path substring match
......@@ -120,31 +115,34 @@ _PATH_RULES_SPECIFIER = [
# No trailing white space: since this is easy to correct.
# No carriage-return line endings: since this is easy to correct.
#
(['blinkpy/third_party/'],
['-',
'+pep8/W191', # Tabs
'+pep8/W291', # Trailing white space
'+whitespace/carriage_return']),
([ # IDL compiler reference output
# Conforming to style significantly increases the complexity of the code
# generator and decreases *its* readability, which is of more concern
# than style of the machine-generated code itself.
'renderer/bindings/tests/results'],
['-']),
([ # Due to historical reasons scheduler uses Chromium style instead of
# Blink style.
'renderer/platform/scheduler',
'public/platform/scheduler'],
['-readability/control_flow']),
(
['blinkpy/third_party/'],
[
'-',
'+pep8/W191', # Tabs
'+pep8/W291', # Trailing white space
'+whitespace/carriage_return'
]),
(
[ # IDL compiler reference output
# Conforming to style significantly increases the complexity of the code
# generator and decreases *its* readability, which is of more concern
# than style of the machine-generated code itself.
'renderer/bindings/tests/results'
],
['-']),
(
[ # Due to historical reasons scheduler uses Chromium style instead of
# Blink style.
'renderer/platform/scheduler',
'public/platform/scheduler'
],
['-readability/control_flow']),
# perf_tests contains a lot of third-party files.
(['perf_tests/'],
['-whitespace/tab'])
(['perf_tests/'], ['-whitespace/tab'])
]
_CPP_FILE_EXTENSIONS = [
'c',
'cc',
......@@ -191,7 +189,8 @@ _PNG_FILE_EXTENSION = 'png'
# with FileType.NONE are automatically skipped without warning.
_SKIPPED_FILES_WITHOUT_WARNING = [
'web_tests' + os.path.sep,
'third_party' + os.path.sep + 'blink' + os.path.sep + 'renderer' + os.path.sep + 'devtools' + os.path.sep + 'protocol.json',
'third_party' + os.path.sep + 'blink' + os.path.sep + 'renderer' +
os.path.sep + 'devtools' + os.path.sep + 'protocol.json',
]
# Extensions of files which are allowed to contain carriage returns.
......@@ -203,9 +202,7 @@ _CARRIAGE_RETURN_ALLOWED_FILE_EXTENSIONS = [
# 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 = {
'whitespace/carriage_return': 1
}
_MAX_REPORTS_PER_CATEGORY = {'whitespace/carriage_return': 1}
def _all_categories():
......@@ -228,17 +225,19 @@ def _all_categories():
def _check_blink_style_defaults():
"""Return the default command-line options for check_blink_style.py."""
return DefaultCommandOptionValues(min_confidence=_DEFAULT_MIN_CONFIDENCE,
output_format=_DEFAULT_OUTPUT_FORMAT)
return DefaultCommandOptionValues(
min_confidence=_DEFAULT_MIN_CONFIDENCE,
output_format=_DEFAULT_OUTPUT_FORMAT)
# This function assists in optparser not having to import from checker.
def check_blink_style_parser():
all_categories = _all_categories()
default_options = _check_blink_style_defaults()
return ArgumentParser(all_categories=all_categories,
base_filter_rules=_BASE_FILTER_RULES,
default_options=default_options)
return ArgumentParser(
all_categories=all_categories,
base_filter_rules=_BASE_FILTER_RULES,
default_options=default_options)
def check_blink_style_configuration(options):
......@@ -252,11 +251,12 @@ def check_blink_style_configuration(options):
path_specific=_PATH_RULES_SPECIFIER,
user_rules=options.filter_rules)
return StyleProcessorConfiguration(filter_configuration=filter_configuration,
max_reports_per_category=_MAX_REPORTS_PER_CATEGORY,
min_confidence=options.min_confidence,
output_format=options.output_format,
stderr_write=sys.stderr.write)
return StyleProcessorConfiguration(
filter_configuration=filter_configuration,
max_reports_per_category=_MAX_REPORTS_PER_CATEGORY,
min_confidence=options.min_confidence,
output_format=options.output_format,
stderr_write=sys.stderr.write)
def _create_log_handlers(stream):
......@@ -338,8 +338,8 @@ def configure_logging(stream, logger=None, is_verbose=False):
logging_level = logging.INFO
handlers = _create_log_handlers(stream)
handlers = _configure_logging(logging_level=logging_level, logger=logger,
handlers=handlers)
handlers = _configure_logging(
logging_level=logging_level, logger=logger, handlers=handlers)
return handlers
......@@ -361,7 +361,6 @@ class FileType:
class CheckerDispatcher(object):
"""Supports determining whether and how to check style, based on path."""
def _file_extension(self, file_path):
......@@ -397,7 +396,8 @@ class CheckerDispatcher(object):
return False
def should_check_and_strip_carriage_returns(self, file_path):
return self._file_extension(file_path) not in _CARRIAGE_RETURN_ALLOWED_FILE_EXTENSIONS
return (self._file_extension(file_path) not in
_CARRIAGE_RETURN_ALLOWED_FILE_EXTENSIONS)
def _file_type(self, file_path):
"""Return the file type corresponding to the given file."""
......@@ -421,7 +421,8 @@ class CheckerDispatcher(object):
return FileType.XCODEPROJ
elif file_extension == _PNG_FILE_EXTENSION:
return FileType.PNG
elif file_extension in _TEXT_FILE_EXTENSIONS or os.path.basename(file_path) == 'TestExpectations':
elif (file_extension in _TEXT_FILE_EXTENSIONS
or os.path.basename(file_path) == 'TestExpectations'):
return FileType.TEXT
else:
return FileType.NONE
......@@ -433,8 +434,8 @@ class CheckerDispatcher(object):
checker = None
elif file_type == FileType.CPP:
file_extension = self._file_extension(file_path)
checker = CppChecker(file_path, file_extension,
handle_style_error, min_confidence)
checker = CppChecker(file_path, file_extension, handle_style_error,
min_confidence)
elif file_type == FileType.JSON:
checker = JSONChecker(file_path, handle_style_error)
elif file_type == FileType.PYTHON:
......@@ -448,16 +449,19 @@ class CheckerDispatcher(object):
elif file_type == FileType.TEXT:
basename = os.path.basename(file_path)
if basename == 'TestExpectations':
checker = TestExpectationsChecker(file_path, handle_style_error)
checker = TestExpectationsChecker(file_path,
handle_style_error)
else:
checker = TextChecker(file_path, handle_style_error)
else:
raise ValueError('Invalid file type "%(file_type)s": the only valid file types '
'are %(NONE)s, %(CPP)s, and %(TEXT)s.'
% {'file_type': file_type,
'NONE': FileType.NONE,
'CPP': FileType.CPP,
'TEXT': FileType.TEXT})
raise ValueError(
'Invalid file type "%(file_type)s": the only valid file types '
'are %(NONE)s, %(CPP)s, and %(TEXT)s.' % {
'file_type': file_type,
'NONE': FileType.NONE,
'CPP': FileType.CPP,
'TEXT': FileType.TEXT
})
return checker
......@@ -465,17 +469,14 @@ class CheckerDispatcher(object):
"""Instantiate and return a style checker based on file path."""
file_type = self._file_type(file_path)
checker = self._create_checker(file_type,
file_path,
handle_style_error,
min_confidence)
checker = self._create_checker(file_type, file_path,
handle_style_error, min_confidence)
return checker
# FIXME: Remove the stderr_write attribute from this class and replace
# its use with calls to a logging module logger.
class StyleProcessorConfiguration(object):
"""Stores configuration values for the StyleProcessor class.
Attributes:
......@@ -489,12 +490,8 @@ class StyleProcessorConfiguration(object):
serves as stderr.write.
"""
def __init__(self,
filter_configuration,
max_reports_per_category,
min_confidence,
output_format,
stderr_write):
def __init__(self, filter_configuration, max_reports_per_category,
min_confidence, output_format, stderr_write):
"""Create a StyleProcessorConfiguration instance.
Args:
......@@ -542,27 +539,19 @@ class StyleProcessorConfiguration(object):
return self._filter_configuration.should_check(category, file_path)
def write_style_error(self,
category,
confidence_in_error,
file_path,
line_number,
message):
def write_style_error(self, category, confidence_in_error, 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_in_error))
self.stderr_write(format_string % (file_path, line_number, message,
category, confidence_in_error))
class ProcessorBase(object):
"""The base class for processors of lists of lines."""
def should_process(self, file_path):
......@@ -592,7 +581,6 @@ class ProcessorBase(object):
class StyleProcessor(ProcessorBase):
"""A ProcessorBase for checking style.
Attributes:
......@@ -600,7 +588,9 @@ class StyleProcessor(ProcessorBase):
errors for the lifetime of this instance.
"""
def __init__(self, configuration, mock_dispatcher=None,
def __init__(self,
configuration,
mock_dispatcher=None,
mock_increment_error_count=None,
mock_carriage_checker_class=None):
"""Create an instance.
......@@ -679,12 +669,12 @@ class StyleProcessor(ProcessorBase):
lines = carriage_checker.check(lines)
min_confidence = self._configuration.min_confidence
checker = self._dispatcher.dispatch(file_path,
style_error_handler,
checker = self._dispatcher.dispatch(file_path, style_error_handler,
min_confidence)
if checker is None:
raise AssertionError("File should not be checked: '%s'" % file_path)
raise AssertionError(
"File should not be checked: '%s'" % file_path)
_log.debug('Using class: ' + checker.__class__.__name__)
......
......@@ -30,7 +30,6 @@
# 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 style.py."""
import logging
......@@ -63,7 +62,6 @@ from blinkpy.style.optparser import CommandOptionValues
class ConfigureLoggingTestBase(unittest.TestCase):
"""Base class for testing configure_logging().
Sub-classes should implement:
......@@ -88,8 +86,8 @@ class ConfigureLoggingTestBase(unittest.TestCase):
# the root logger).
logger.propagate = False
self._handlers = configure_logging(stream=log_stream, logger=logger,
is_verbose=is_verbose)
self._handlers = configure_logging(
stream=log_stream, logger=logger, is_verbose=is_verbose)
self._log = logger
self._log_stream = log_stream
......@@ -109,7 +107,6 @@ class ConfigureLoggingTestBase(unittest.TestCase):
class ConfigureLoggingTest(ConfigureLoggingTestBase):
"""Tests the configure_logging() function."""
is_verbose = False
......@@ -137,7 +134,6 @@ class ConfigureLoggingTest(ConfigureLoggingTestBase):
class ConfigureLoggingVerboseTest(ConfigureLoggingTestBase):
"""Tests the configure_logging() function with is_verbose True."""
is_verbose = True
......@@ -148,7 +144,6 @@ class ConfigureLoggingVerboseTest(ConfigureLoggingTestBase):
class GlobalVariablesTest(unittest.TestCase):
"""Tests validity of the global variables."""
def _all_categories(self):
......@@ -180,9 +175,10 @@ class GlobalVariablesTest(unittest.TestCase):
# FIXME: We should not need to call parse() to determine
# whether the default arguments are valid.
parser = ArgumentParser(all_categories=self._all_categories(),
base_filter_rules=[],
default_options=default_options)
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.
#
......@@ -201,14 +197,12 @@ class GlobalVariablesTest(unittest.TestCase):
def assert_no_check(path, category):
"""Assert that the given category should not be checked."""
message = ('Should not check category "%s" for path "%s".'
% (category, path))
message = ('Should not check category "%s" for path "%s".' %
(category, path))
self.assertFalse(config.should_check(category, path), message)
assert_check("random_path.cpp",
"build/include")
assert_check("random_path.cpp",
"readability/naming")
assert_check("random_path.cpp", "build/include")
assert_check("random_path.cpp", "readability/naming")
# Third-party Python code: blinkpy/third_party
path = "tools/blinkpy/third_party/mock.py"
......@@ -227,7 +221,6 @@ class GlobalVariablesTest(unittest.TestCase):
class CheckBlinkStyleFunctionTest(unittest.TestCase):
"""Tests the functions with names of the form check_blink_style_*."""
def test_check_blink_style_configuration(self):
......@@ -241,7 +234,6 @@ class CheckBlinkStyleFunctionTest(unittest.TestCase):
class CheckerDispatcherSkipTest(unittest.TestCase):
"""Tests the "should skip" methods of the CheckerDispatcher class."""
def setUp(self):
......@@ -250,44 +242,41 @@ class CheckerDispatcherSkipTest(unittest.TestCase):
def _assert_should_skip_without_warning(self, path, is_checker_none,
expected):
# Check the file type before asserting the return value.
checker = self._dispatcher.dispatch(file_path=path,
handle_style_error=None,
min_confidence=3)
checker = self._dispatcher.dispatch(
file_path=path, handle_style_error=None, min_confidence=3)
message = 'while checking: %s' % path
self.assertEqual(checker is None, is_checker_none, message)
self.assertEqual(self._dispatcher.should_skip_without_warning(path),
expected, message)
self.assertEqual(
self._dispatcher.should_skip_without_warning(path), expected,
message)
def test_should_skip_without_warning__true(self):
"""Test should_skip_without_warning() for True return values."""
# Check a file with NONE file type.
path = 'foo.asdf' # Non-sensical file extension.
self._assert_should_skip_without_warning(path,
is_checker_none=True,
expected=True)
self._assert_should_skip_without_warning(
path, is_checker_none=True, expected=True)
# Check files with non-NONE file type. These examples must be
# drawn from the _SKIPPED_FILES_WITHOUT_WARNING configuration
# variable.
path = os.path.join('web_tests', 'foo.txt')
self._assert_should_skip_without_warning(path,
is_checker_none=False,
expected=True)
self._assert_should_skip_without_warning(
path, is_checker_none=False, expected=True)
def test_should_skip_without_warning__false(self):
"""Test should_skip_without_warning() for False return values."""
paths = ['foo.txt',
os.path.join('web_tests', 'TestExpectations'),
]
paths = [
'foo.txt',
os.path.join('web_tests', 'TestExpectations'),
]
for path in paths:
self._assert_should_skip_without_warning(path,
is_checker_none=False,
expected=False)
self._assert_should_skip_without_warning(
path, is_checker_none=False, expected=False)
class CheckerDispatcherCarriageReturnTest(unittest.TestCase):
def test_should_check_and_strip_carriage_returns(self):
files = {
'foo.txt': True,
......@@ -298,21 +287,21 @@ class CheckerDispatcherCarriageReturnTest(unittest.TestCase):
dispatcher = CheckerDispatcher()
for file_path, expected_result in files.items():
self.assertEqual(dispatcher.should_check_and_strip_carriage_returns(
file_path), expected_result, 'Checking: %s' % file_path)
self.assertEqual(
dispatcher.should_check_and_strip_carriage_returns(file_path),
expected_result, 'Checking: %s' % file_path)
class CheckerDispatcherDispatchTest(unittest.TestCase):
"""Tests dispatch() method of CheckerDispatcher class."""
def dispatch(self, file_path):
"""Call dispatch() with the given file path."""
dispatcher = CheckerDispatcher()
self.mock_handle_style_error = DefaultStyleErrorHandler('', None, None, [])
checker = dispatcher.dispatch(file_path,
self.mock_handle_style_error,
min_confidence=3)
self.mock_handle_style_error = DefaultStyleErrorHandler(
'', None, None, [])
checker = dispatcher.dispatch(
file_path, self.mock_handle_style_error, min_confidence=3)
return checker
def assert_checker_none(self, file_path):
......@@ -324,12 +313,14 @@ class CheckerDispatcherDispatchTest(unittest.TestCase):
"""Assert the type of the dispatched checker."""
checker = self.dispatch(file_path)
got_class = checker.__class__
self.assertEqual(got_class, expected_class,
'For path "%(file_path)s" got %(got_class)s when '
"expecting %(expected_class)s."
% {"file_path": file_path,
"got_class": got_class,
"expected_class": expected_class})
self.assertEqual(
got_class, expected_class,
'For path "%(file_path)s" got %(got_class)s when '
"expecting %(expected_class)s." % {
"file_path": file_path,
"got_class": got_class,
"expected_class": expected_class
})
def assert_checker_cpp(self, file_path):
"""Assert that the dispatched checker is a CppChecker."""
......@@ -372,7 +363,8 @@ class CheckerDispatcherDispatchTest(unittest.TestCase):
checker = self.dispatch(file_path)
self.assertEqual(checker.file_extension, file_extension)
self.assertEqual(checker.file_path, file_path)
self.assertEqual(checker.handle_style_error, self.mock_handle_style_error)
self.assertEqual(checker.handle_style_error,
self.mock_handle_style_error)
self.assertEqual(checker.min_confidence, 3)
# Check "-" for good measure.
file_base = "-"
......@@ -442,7 +434,8 @@ class CheckerDispatcherDispatchTest(unittest.TestCase):
"foo.txt",
"foo.xhtml",
"foo.y",
os.path.join("Source", "WebCore", "inspector", "front-end", "Main.js"),
os.path.join("Source", "WebCore", "inspector", "front-end",
"Main.js"),
]
for path in paths:
......@@ -455,7 +448,8 @@ class CheckerDispatcherDispatchTest(unittest.TestCase):
self.assert_checker_text(file_path)
checker = self.dispatch(file_path)
self.assertEqual(checker.file_path, file_path)
self.assertEqual(checker.handle_style_error, self.mock_handle_style_error)
self.assertEqual(checker.handle_style_error,
self.mock_handle_style_error)
def test_xml_paths(self):
"""Test paths that should be checked as XML."""
......@@ -489,11 +483,11 @@ class CheckerDispatcherDispatchTest(unittest.TestCase):
class StyleProcessorConfigurationTest(unittest.TestCase):
"""Tests the StyleProcessorConfiguration class."""
def setUp(self):
self._error_messages = [] # The messages written to _mock_stderr_write() of this class.
# The messages written to _mock_stderr_write() of this class.
self._error_messages = []
def _mock_stderr_write(self, message):
self._error_messages.append(message)
......@@ -534,11 +528,12 @@ class StyleProcessorConfigurationTest(unittest.TestCase):
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_in_error=5,
file_path="foo.h",
line_number=100,
message="message")
config.write_style_error(
category="whitespace/tab",
confidence_in_error=5,
file_path="foo.h",
line_number=100,
message="message")
def test_write_style_error_emacs(self):
"""Test the write_style_error() method."""
......@@ -554,7 +549,6 @@ class StyleProcessorConfigurationTest(unittest.TestCase):
class StyleProcessor_EndToEndTest(LoggingTestCase):
"""Test the StyleProcessor class with an emphasis on end-to-end tests."""
def setUp(self):
......@@ -587,23 +581,23 @@ class StyleProcessor_EndToEndTest(LoggingTestCase):
stderr_write=self._mock_stderr_write)
processor = StyleProcessor(configuration)
processor.process(lines=['line1', 'Line with tab:\t'],
file_path='foo.txt')
processor.process(
lines=['line1', 'Line with tab:\t'], file_path='foo.txt')
self.assertEqual(processor.error_count, 1)
expected_messages = ['foo.txt(2): Line contains tab character. '
'[whitespace/tab] [5]\n']
expected_messages = [
'foo.txt(2): Line contains tab character. '
'[whitespace/tab] [5]\n'
]
self.assertEqual(self._messages, expected_messages)
class StyleProcessor_CodeCoverageTest(LoggingTestCase):
"""Test the StyleProcessor class with an emphasis on code coverage.
This class makes heavy use of mock objects.
"""
class MockDispatchedChecker(object):
"""A mock checker dispatched by the MockDispatcher."""
def __init__(self, file_path, min_confidence, style_error_handler):
......@@ -616,7 +610,6 @@ class StyleProcessor_CodeCoverageTest(LoggingTestCase):
self.lines = lines
class MockDispatcher(object):
"""A mock CheckerDispatcher class."""
def __init__(self):
......@@ -633,9 +626,7 @@ class StyleProcessor_CodeCoverageTest(LoggingTestCase):
return None
checker = StyleProcessor_CodeCoverageTest.MockDispatchedChecker(
file_path,
min_confidence,
style_error_handler)
file_path, min_confidence, style_error_handler)
# Save the dispatched checker so the current test case has a
# way to access and check it.
......@@ -660,10 +651,11 @@ class StyleProcessor_CodeCoverageTest(LoggingTestCase):
# incrementing is tested instead in the end-to-end test case above.
mock_increment_error_count = self._do_nothing
processor = StyleProcessor(configuration=configuration,
mock_carriage_checker_class=mock_carriage_checker_class,
mock_dispatcher=mock_dispatcher,
mock_increment_error_count=mock_increment_error_count)
processor = StyleProcessor(
configuration=configuration,
mock_carriage_checker_class=mock_carriage_checker_class,
mock_dispatcher=mock_dispatcher,
mock_increment_error_count=mock_increment_error_count)
self._configuration = configuration
self._mock_dispatcher = mock_dispatcher
......@@ -688,7 +680,6 @@ class StyleProcessor_CodeCoverageTest(LoggingTestCase):
test_case = self
class MockCarriageChecker(object):
"""A mock carriage-return checker."""
def __init__(self, style_error_handler):
......@@ -731,9 +722,8 @@ class StyleProcessor_CodeCoverageTest(LoggingTestCase):
increment_error_count=self._do_nothing,
line_numbers=line_numbers)
self._processor.process(lines=lines,
file_path=file_path,
line_numbers=line_numbers)
self._processor.process(
lines=lines, file_path=file_path, line_numbers=line_numbers)
# Check that the carriage-return checker was instantiated correctly
# and was passed lines correctly.
......@@ -756,17 +746,15 @@ class StyleProcessor_CodeCoverageTest(LoggingTestCase):
path = os.path.join('foo', 'do_not_process.txt')
with self.assertRaises(AssertionError):
self._processor.process(
lines=['line1', 'line2'], file_path=path,
line_numbers=[100])
lines=['line1', 'line2'], file_path=path, line_numbers=[100])
def test_process__carriage_returns_not_stripped(self):
"""Test that carriage returns aren't stripped from files that are allowed to contain them."""
file_path = 'carriage_returns_allowed.txt'
lines = ['line1\r', 'line2\r']
line_numbers = [100]
self._processor.process(lines=lines,
file_path=file_path,
line_numbers=line_numbers)
self._processor.process(
lines=lines, file_path=file_path, line_numbers=line_numbers)
# The carriage return checker should never have been invoked, and so
# should not have saved off any lines.
self.assertFalse(hasattr(self.carriage_checker, 'lines'))
......@@ -19,22 +19,17 @@
# 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 style checking not specific to any one file type."""
# FIXME: Test this list in the same way that the list of CppChecker
# categories is tested, for example by checking that all of its
# elements appear in the unit tests. This should probably be done
# after moving the relevant cpp_unittest.ErrorCollector code
# into a shared location and refactoring appropriately.
categories = set([
'whitespace/carriage_return',
'whitespace/tab'])
categories = set(['whitespace/carriage_return', 'whitespace/tab'])
class CarriageReturnChecker(object):
"""Supports checking for and handling carriage returns."""
def __init__(self, handle_style_error):
......@@ -46,11 +41,12 @@ class CarriageReturnChecker(object):
if not lines[line_number].endswith('\r'):
continue
self._handle_style_error(line_number + 1, # Correct for offset.
'whitespace/carriage_return',
1,
'One or more unexpected \\r (^M) found; '
'better to use only a \\n')
self._handle_style_error(
line_number + 1, # Correct for offset.
'whitespace/carriage_return',
1,
'One or more unexpected \\r (^M) found; '
'better to use only a \\n')
lines[line_number] = lines[line_number].rstrip('\r')
......@@ -58,7 +54,6 @@ class CarriageReturnChecker(object):
class TabChecker(object):
"""Supports checking for and handling tabs."""
def __init__(self, file_path, handle_style_error):
......@@ -69,6 +64,5 @@ class TabChecker(object):
# FIXME: share with cpp_style.
for line_number, line in enumerate(lines):
if '\t' in line:
self.handle_style_error(line_number + 1,
'whitespace/tab', 5,
self.handle_style_error(line_number + 1, 'whitespace/tab', 5,
'Line contains tab character.')
......@@ -19,7 +19,6 @@
# 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 common.py."""
import unittest
......@@ -36,7 +35,6 @@ from blinkpy.style.checkers.common import TabChecker
class CarriageReturnCheckerTest(unittest.TestCase):
"""Tests check_no_carriage_return()."""
_category = 'whitespace/carriage_return'
......@@ -69,35 +67,29 @@ class CarriageReturnCheckerTest(unittest.TestCase):
self.assertEqual(self._style_errors, expected_errors)
def test_ends_with_carriage(self):
self.assert_carriage_return(['carriage return\r'],
['carriage return'],
self.assert_carriage_return(['carriage return\r'], ['carriage return'],
[1])
def test_ends_with_nothing(self):
self.assert_carriage_return(['no carriage return'],
['no carriage return'],
[])
['no carriage return'], [])
def test_ends_with_newline(self):
self.assert_carriage_return(['no carriage return\n'],
['no carriage return\n'],
[])
['no carriage return\n'], [])
def test_carriage_in_middle(self):
# The CarriageReturnChecker checks only the final character
# of each line.
self.assert_carriage_return(['carriage\r in a string'],
['carriage\r in a string'],
[])
['carriage\r in a string'], [])
def test_multiple_errors(self):
self.assert_carriage_return(['line1', 'line2\r', 'line3\r'],
['line1', 'line2', 'line3'],
[2, 3])
['line1', 'line2', 'line3'], [2, 3])
class TabCheckerTest(unittest.TestCase):
"""Tests for TabChecker."""
def assert_tab(self, input_lines, error_lines):
......
......@@ -33,7 +33,6 @@
# This is the modified version of Google's cpplint. The original code is
# https://github.com/google/styleguide/tree/gh-pages/cpplint
"""Support for check_blink_style.py."""
import math # for log
......@@ -48,49 +47,132 @@ from blinkpy.common.system.filesystem import FileSystem
# Headers that we consider STL headers.
_STL_HEADERS = frozenset([
'algobase.h', 'algorithm', 'alloc.h', 'bitset', 'deque', 'exception',
'function.h', 'functional', 'hash_map', 'hash_map.h', 'hash_set',
'hash_set.h', 'iterator', 'list', 'list.h', 'map', 'memory', 'pair.h',
'pthread_alloc', 'queue', 'set', 'set.h', 'sstream', 'stack',
'stl_alloc.h', 'stl_relops.h', 'type_traits.h',
'utility', 'vector', 'vector.h',
'algobase.h',
'algorithm',
'alloc.h',
'bitset',
'deque',
'exception',
'function.h',
'functional',
'hash_map',
'hash_map.h',
'hash_set',
'hash_set.h',
'iterator',
'list',
'list.h',
'map',
'memory',
'pair.h',
'pthread_alloc',
'queue',
'set',
'set.h',
'sstream',
'stack',
'stl_alloc.h',
'stl_relops.h',
'type_traits.h',
'utility',
'vector',
'vector.h',
])
# Non-STL C++ system headers.
_CPP_HEADERS = frozenset([
'algo.h', 'builtinbuf.h', 'bvector.h', 'cassert', 'cctype',
'cerrno', 'cfloat', 'ciso646', 'climits', 'clocale', 'cmath',
'complex', 'complex.h', 'csetjmp', 'csignal', 'cstdarg', 'cstddef',
'cstdio', 'cstdlib', 'cstring', 'ctime', 'cwchar', 'cwctype',
'defalloc.h', 'deque.h', 'editbuf.h', 'exception', 'fstream',
'fstream.h', 'hashtable.h', 'heap.h', 'indstream.h', 'iomanip',
'iomanip.h', 'ios', 'iosfwd', 'iostream', 'iostream.h', 'istream.h',
'iterator.h', 'limits', 'map.h', 'multimap.h', 'multiset.h',
'numeric', 'ostream.h', 'parsestream.h', 'pfstream.h', 'PlotFile.h',
'procbuf.h', 'pthread_alloc.h', 'rope', 'rope.h', 'ropeimpl.h',
'SFile.h', 'slist', 'slist.h', 'stack.h', 'stdexcept',
'stdiostream.h', 'streambuf.h', 'stream.h', 'strfile.h', 'string',
'strstream', 'strstream.h', 'tempbuf.h', 'tree.h', 'typeinfo', 'valarray',
'algo.h',
'builtinbuf.h',
'bvector.h',
'cassert',
'cctype',
'cerrno',
'cfloat',
'ciso646',
'climits',
'clocale',
'cmath',
'complex',
'complex.h',
'csetjmp',
'csignal',
'cstdarg',
'cstddef',
'cstdio',
'cstdlib',
'cstring',
'ctime',
'cwchar',
'cwctype',
'defalloc.h',
'deque.h',
'editbuf.h',
'exception',
'fstream',
'fstream.h',
'hashtable.h',
'heap.h',
'indstream.h',
'iomanip',
'iomanip.h',
'ios',
'iosfwd',
'iostream',
'iostream.h',
'istream.h',
'iterator.h',
'limits',
'map.h',
'multimap.h',
'multiset.h',
'numeric',
'ostream.h',
'parsestream.h',
'pfstream.h',
'PlotFile.h',
'procbuf.h',
'pthread_alloc.h',
'rope',
'rope.h',
'ropeimpl.h',
'SFile.h',
'slist',
'slist.h',
'stack.h',
'stdexcept',
'stdiostream.h',
'streambuf.h',
'stream.h',
'strfile.h',
'string',
'strstream',
'strstream.h',
'tempbuf.h',
'tree.h',
'typeinfo',
'valarray',
])
# Assertion macros. These are defined in base/logging.h and
# testing/base/gunit.h. Note that the _M versions need to come first
# for substring matching to work.
_CHECK_MACROS = [
'DCHECK', 'CHECK',
'EXPECT_TRUE_M', 'EXPECT_TRUE',
'ASSERT_TRUE_M', 'ASSERT_TRUE',
'EXPECT_FALSE_M', 'EXPECT_FALSE',
'ASSERT_FALSE_M', 'ASSERT_FALSE',
'DCHECK',
'CHECK',
'EXPECT_TRUE_M',
'EXPECT_TRUE',
'ASSERT_TRUE_M',
'ASSERT_TRUE',
'EXPECT_FALSE_M',
'EXPECT_FALSE',
'ASSERT_FALSE_M',
'ASSERT_FALSE',
]
# Replacement macros for CHECK/DCHECK/EXPECT_TRUE/EXPECT_FALSE
_CHECK_REPLACEMENT = dict([(m, {}) for m in _CHECK_MACROS])
for op, replacement in [('==', 'EQ'), ('!=', 'NE'),
('>=', 'GE'), ('>', 'GT'),
for op, replacement in [('==', 'EQ'), ('!=', 'NE'), ('>=', 'GE'), ('>', 'GT'),
('<=', 'LE'), ('<', 'LT')]:
_CHECK_REPLACEMENT['DCHECK'][op] = 'DCHECK_%s' % replacement
_CHECK_REPLACEMENT['CHECK'][op] = 'CHECK_%s' % replacement
......@@ -99,15 +181,13 @@ for op, replacement in [('==', 'EQ'), ('!=', 'NE'),
_CHECK_REPLACEMENT['EXPECT_TRUE_M'][op] = 'EXPECT_%s_M' % replacement
_CHECK_REPLACEMENT['ASSERT_TRUE_M'][op] = 'ASSERT_%s_M' % replacement
for op, inv_replacement in [('==', 'NE'), ('!=', 'EQ'),
('>=', 'LT'), ('>', 'LE'),
('<=', 'GT'), ('<', 'GE')]:
for op, inv_replacement in [('==', 'NE'), ('!=', 'EQ'), ('>=', 'LT'),
('>', 'LE'), ('<=', 'GT'), ('<', 'GE')]:
_CHECK_REPLACEMENT['EXPECT_FALSE'][op] = 'EXPECT_%s' % inv_replacement
_CHECK_REPLACEMENT['ASSERT_FALSE'][op] = 'ASSERT_%s' % inv_replacement
_CHECK_REPLACEMENT['EXPECT_FALSE_M'][op] = 'EXPECT_%s_M' % inv_replacement
_CHECK_REPLACEMENT['ASSERT_FALSE_M'][op] = 'ASSERT_%s_M' % inv_replacement
# The regexp compilation caching is inlined in all regexp functions for
# performance reasons; factoring it out into a separate function turns out
# to be noticeably expensive.
......@@ -171,7 +251,8 @@ def iteratively_replace_matches_with_char(pattern, char_replacement, s):
start_match_index = matched.start(0)
end_match_index = matched.end(0)
match_length = end_match_index - start_match_index
s = s[:start_match_index] + char_replacement * match_length + s[end_match_index:]
s = (s[:start_match_index] + char_replacement * match_length +
s[end_match_index:])
def _find_in_lines(regex, lines, start_position, not_found_position):
......@@ -314,7 +395,8 @@ class SingleLineView(object):
# Create a single line with all of the parameters.
self.single_line = ' '.join(trimmed_lines)
self.single_line = _RE_PATTERN_CLEANSE_MULTIPLE_STRINGS.sub('""', self.single_line)
self.single_line = _RE_PATTERN_CLEANSE_MULTIPLE_STRINGS.sub(
'""', self.single_line)
# Keep the row lengths, so we can calculate the original row number
# given a column in the single line (adding 1 due to the space added
......@@ -331,7 +413,7 @@ class _FunctionState(object):
"""
_NORMAL_TRIGGER = 250 # for --v=0, 500 for --v=1, etc.
_TEST_TRIGGER = 400 # about 50% more than _NORMAL_TRIGGER.
_TEST_TRIGGER = 400 # about 50% more than _NORMAL_TRIGGER.
def __init__(self, min_confidence):
self.min_confidence = min_confidence
......@@ -343,8 +425,9 @@ class _FunctionState(object):
self.body_start_position = Position(-1000, 0)
self.end_position = Position(-1000, 0)
def begin(self, function_name, function_name_start_position, body_start_position, end_position,
parameter_start_position, parameter_end_position, clean_lines):
def begin(self, function_name, function_name_start_position,
body_start_position, end_position, parameter_start_position,
parameter_end_position, clean_lines):
"""Start analyzing function body.
Args:
......@@ -362,14 +445,17 @@ class _FunctionState(object):
self.function_name_start_position = function_name_start_position
self.body_start_position = body_start_position
self.end_position = end_position
self.is_declaration = clean_lines.elided[body_start_position.row][body_start_position.column] == ';'
self.is_declaration = clean_lines.elided[body_start_position.row][
body_start_position.column] == ';'
self.parameter_start_position = parameter_start_position
self.parameter_end_position = parameter_end_position
self.is_pure = False
if self.is_declaration:
characters_after_parameters = SingleLineView(
clean_lines.elided, parameter_end_position, body_start_position).single_line
self.is_pure = bool(match(r'\s*=\s*0\s*', characters_after_parameters))
clean_lines.elided, parameter_end_position,
body_start_position).single_line
self.is_pure = bool(
match(r'\s*=\s*0\s*', characters_after_parameters))
self._clean_lines = clean_lines
def count(self, line_number):
......@@ -388,18 +474,20 @@ class _FunctionState(object):
base_trigger = self._TEST_TRIGGER
else:
base_trigger = self._NORMAL_TRIGGER
trigger = base_trigger * 2 ** self.min_confidence
trigger = base_trigger * 2**self.min_confidence
if self.lines_in_function > trigger:
error_level = int(math.log(self.lines_in_function / base_trigger, 2))
error_level = int(
math.log(self.lines_in_function / base_trigger, 2))
# 50 => 0, 100 => 1, 200 => 2, 400 => 3, 800 => 4, 1600 => 5, ...
if error_level > 5:
error_level = 5
error(line_number, 'readability/fn_size', error_level,
'Small and focused functions are preferred:'
' %s has %d non-comment lines'
' (error triggered by exceeding %d lines).' % (
self.current_function, self.lines_in_function, trigger))
error(
line_number, 'readability/fn_size', error_level,
'Small and focused functions are preferred:'
' %s has %d non-comment lines'
' (error triggered by exceeding %d lines).' %
(self.current_function, self.lines_in_function, trigger))
def end(self):
"""Stop analyzing function body."""
......@@ -463,7 +551,7 @@ class FileInfo:
googlename = self.repository_name()
project, rest = os.path.split(googlename)
return (project,) + os.path.splitext(rest)
return (project, ) + os.path.splitext(rest)
def base_name(self):
"""File base name - text after the final slash, before the final period."""
......@@ -521,7 +609,8 @@ def is_cpp_string(line):
"""
line = line.replace(r'\\', 'XX') # after this, \\" does not match to \"
return ((line.count('"') - line.count(r'\"') - line.count("'\"'")) & 1) == 1
return (
(line.count('"') - line.count(r'\"') - line.count("'\"'")) & 1) == 1
def cleanse_raw_strings(raw_lines):
......@@ -555,7 +644,8 @@ def cleanse_raw_strings(raw_lines):
# line and resume copying the original lines, and also insert
# a "" on the last line.
leading_space = match(r'^(\s*)\S', line)
line = leading_space.group(1) + '""' + line[end + len(delimiter):]
line = (leading_space.group(1) + '""' +
line[end + len(delimiter):])
delimiter = None
else:
# Haven't found the end yet, append a blank line.
......@@ -575,8 +665,11 @@ def cleanse_raw_strings(raw_lines):
# before removing raw strings. This is because there are some
# cpplint checks that requires the comments to be preserved, but
# we don't want to check comments that are inside raw strings.
matched = match(r'^(.*?)\b(?:R|u8R|uR|UR|LR)"([^\s\\()]*)\((.*)$', line)
if matched and not match(r'^([^\'"]|\'(\\.|[^\'])*\'|"(\\.|[^"])*")*//', matched.group(1)):
matched = match(r'^(.*?)\b(?:R|u8R|uR|UR|LR)"([^\s\\()]*)\((.*)$',
line)
if matched and not match(
r'^([^\'"]|\'(\\.|[^\'])*\'|"(\\.|[^"])*")*//',
matched.group(1)):
delimiter = ')' + matched.group(2) + '"'
end = matched.group(3).find(delimiter)
......@@ -630,13 +723,16 @@ def remove_multi_line_comments(lines, error):
"""Removes multiline (c-style) comments from lines."""
line_index = 0
while line_index < len(lines):
line_index_begin = find_next_multi_line_comment_start(lines, line_index)
line_index_begin = find_next_multi_line_comment_start(
lines, line_index)
if line_index_begin >= len(lines):
return
line_index_end = find_next_multi_line_comment_end(lines, line_index_begin)
line_index_end = find_next_multi_line_comment_end(
lines, line_index_begin)
if line_index_end >= len(lines):
return
remove_multi_line_comments_from_range(lines, line_index_begin, line_index_end + 1)
remove_multi_line_comments_from_range(lines, line_index_begin,
line_index_end + 1)
line_index = line_index_end + 1
......@@ -674,8 +770,10 @@ class CleansedLines(object):
self._num_lines = len(lines)
self.lines_without_raw_strings = cleanse_raw_strings(lines)
for line_number in range(len(self.lines_without_raw_strings)):
self.lines.append(cleanse_comments(self.lines_without_raw_strings[line_number]))
elided = self.collapse_strings(self.lines_without_raw_strings[line_number])
self.lines.append(
cleanse_comments(self.lines_without_raw_strings[line_number]))
elided = self.collapse_strings(
self.lines_without_raw_strings[line_number])
self.elided.append(cleanse_comments(elided))
def num_lines(self):
......@@ -767,10 +865,10 @@ def check_for_copyright(lines, error):
for line in xrange(1, min(len(lines), 11)):
if re.search(r'Copyright', lines[line], re.I):
break
else: # means no copyright line was found
error(0, 'legal/copyright', 5,
'No copyright message found. '
'You should have a line: "Copyright [year] <Copyright Owner>"')
else: # means no copyright line was found
error(
0, 'legal/copyright', 5, 'No copyright message found. '
'You should have a line: "Copyright [year] <Copyright Owner>"')
def get_header_guard_cpp_variable(filename):
......@@ -823,9 +921,10 @@ def check_for_header_guard(filename, clean_lines, error):
break
if not ifndef or not define or ifndef != define:
error(0, 'build/header_guard', 5,
'No #ifndef header guard found, suggested CPP variable is: %s' %
cpp_var)
error(
0, 'build/header_guard', 5,
'No #ifndef header guard found, suggested CPP variable is: %s' %
cpp_var)
return
# The guard should be File_h or, for Chromium style, BLINK_PATH_TO_FILE_H_.
......@@ -848,8 +947,10 @@ def check_for_unicode_replacement_characters(lines, error):
"""
for line_number, line in enumerate(lines):
if u'\ufffd' in line:
error(line_number, 'readability/utf8', 5,
'Line contains invalid UTF-8 (or Unicode replacement character).')
error(
line_number, 'readability/utf8', 5,
'Line contains invalid UTF-8 (or Unicode replacement character).'
)
def check_for_new_line_at_eof(lines, error):
......@@ -865,8 +966,9 @@ def check_for_new_line_at_eof(lines, error):
# To verify that the file ends in \n, we just have to make sure the
# last-but-two element of lines() exists and is empty.
if len(lines) < 3 or lines[-2]:
error(len(lines) - 2, 'whitespace/ending_newline', 5,
'Could not find a newline character at the end of the file.')
error(
len(lines) - 2, 'whitespace/ending_newline', 5,
'Could not find a newline character at the end of the file.')
_THREADING_LIST = (
......@@ -904,18 +1006,18 @@ def check_posix_threading(clean_lines, line_number, error):
for single_thread_function, multithread_safe_function in _THREADING_LIST:
index = line.find(single_thread_function)
# Comparisons made explicit for clarity
if index >= 0 and (index == 0 or (not line[index - 1].isalnum()
and line[index - 1] not in ('_', '.', '>'))):
error(line_number, 'runtime/threadsafe_fn', 2,
'Consider using ' + multithread_safe_function +
'...) instead of ' + single_thread_function +
'...) for improved thread safety.')
if index >= 0 and (index == 0 or
(not line[index - 1].isalnum()
and line[index - 1] not in ('_', '.', '>'))):
error(
line_number, 'runtime/threadsafe_fn', 2, 'Consider using ' +
multithread_safe_function + '...) instead of ' +
single_thread_function + '...) for improved thread safety.')
# Matches invalid increment: *count++, which moves pointer instead of
# incrementing a value.
_RE_PATTERN_INVALID_INCREMENT = re.compile(
r'^\s*\*\w+(\+\+|--);')
_RE_PATTERN_INVALID_INCREMENT = re.compile(r'^\s*\*\w+(\+\+|--);')
def check_invalid_increment(clean_lines, line_number, error):
......@@ -935,8 +1037,10 @@ def check_invalid_increment(clean_lines, line_number, error):
"""
line = clean_lines.elided[line_number]
if _RE_PATTERN_INVALID_INCREMENT.match(line):
error(line_number, 'runtime/invalid_increment', 5,
'Changing pointer instead of value (or unused value of operator*).')
error(
line_number, 'runtime/invalid_increment', 5,
'Changing pointer instead of value (or unused value of operator*).'
)
class _ClassInfo(object):
......@@ -968,7 +1072,6 @@ class _ClassState(object):
class _FileState(object):
def __init__(self, clean_lines, file_extension):
self._clean_lines = clean_lines
if file_extension in ['m', 'mm']:
......@@ -1016,8 +1119,8 @@ class _FileState(object):
return self.is_c() or self.is_objective_c()
def check_for_non_standard_constructs(clean_lines, line_number,
class_state, error):
def check_for_non_standard_constructs(clean_lines, line_number, class_state,
error):
"""Logs an error if we see certain non-ANSI constructs ignored by gcc-2.
Complain about several constructs which gcc-2 accepts, but which are
......@@ -1055,9 +1158,11 @@ def check_for_non_standard_constructs(clean_lines, line_number,
classinfo_stack = class_state.classinfo_stack
# Look for a class declaration
class_decl_match = match(
r'\s*(template\s*<[\w\s<>,:]*>\s*)?(class|struct)\s+(\w+(::\w+)*)', line)
r'\s*(template\s*<[\w\s<>,:]*>\s*)?(class|struct)\s+(\w+(::\w+)*)',
line)
if class_decl_match:
classinfo_stack.append(_ClassInfo(class_decl_match.group(3), line_number))
classinfo_stack.append(
_ClassInfo(class_decl_match.group(3), line_number))
# Everything else in this function uses the top of the stack if it's
# not empty.
......@@ -1087,13 +1192,12 @@ def check_for_non_standard_constructs(clean_lines, line_number,
# Look for single-argument constructors that aren't marked explicit.
# Technically a valid construct, but against style.
args = match(r'(?<!explicit)\s+%s\s*\(([^,()]+)\)'
% re.escape(base_classname),
line)
if (args
and args.group(1) != 'void'
and not match(r'(const\s+)?%s\s*&' % re.escape(base_classname),
args.group(1).strip())):
args = match(
r'(?<!explicit)\s+%s\s*\(([^,()]+)\)' % re.escape(base_classname),
line)
if (args and args.group(1) != 'void'
and not match(r'(const\s+)?%s\s*&' % re.escape(base_classname),
args.group(1).strip())):
error(line_number, 'runtime/explicit', 5,
'Single-argument constructors should be marked explicit.')
......@@ -1119,10 +1223,11 @@ def check_for_non_standard_constructs(clean_lines, line_number,
if ((classinfo.virtual_method_line_number is not None)
and (not classinfo.has_virtual_destructor)
and (not classinfo.is_derived)): # Only warn for base classes
error(classinfo.line_number, 'runtime/virtual', 4,
'The class %s probably needs a virtual destructor due to '
'having virtual method(s), one declared at line %d.'
% (classinfo.name, classinfo.virtual_method_line_number))
error(
classinfo.line_number, 'runtime/virtual', 4,
'The class %s probably needs a virtual destructor due to '
'having virtual method(s), one declared at line %d.' %
(classinfo.name, classinfo.virtual_method_line_number))
else:
classinfo.brace_depth = brace_depth
......@@ -1130,25 +1235,28 @@ def check_for_non_standard_constructs(clean_lines, line_number,
# Look for bool <name> : 1 declarations.
args = search(r'\bbool\s+(\S*)\s*:\s*\d+\s*;', line)
if args:
classinfo.bool_bitfields.append('%d: %s' % (line_number, args.group(1)))
classinfo.bool_bitfields.append(
'%d: %s' % (line_number, args.group(1)))
well_typed_bitfield = True
# Look for unsigned <name> : n declarations.
args = search(r'\bunsigned\s+(?:int\s+)?(\S+)\s*:\s*\d+\s*;', line)
if args:
classinfo.unsigned_bitfields.append('%d: %s' % (line_number, args.group(1)))
classinfo.unsigned_bitfields.append(
'%d: %s' % (line_number, args.group(1)))
well_typed_bitfield = True
# Look for other bitfield declarations. We don't care about those in
# size-matching structs.
if not (well_typed_bitfield or classinfo.name.startswith('SameSizeAs') or
classinfo.name.startswith('Expected')):
if not (well_typed_bitfield or classinfo.name.startswith('SameSizeAs')
or classinfo.name.startswith('Expected')):
args = match(r'\s*(\S+)\s+(\S+)\s*:\s*\d+\s*;', line)
if args:
error(line_number, 'runtime/bitfields', 4,
'Member %s of class %s defined as a bitfield of type %s. '
'Please declare all bitfields as unsigned.'
% (args.group(2), classinfo.name, args.group(1)))
error(
line_number, 'runtime/bitfields', 4,
'Member %s of class %s defined as a bitfield of type %s. '
'Please declare all bitfields as unsigned.' %
(args.group(2), classinfo.name, args.group(1)))
def is_blank_line(line):
......@@ -1205,7 +1313,8 @@ def detect_functions(clean_lines, line_number, function_state, error):
# If the name is all caps and underscores, figure it's a macro and
# ignore it, unless it's TEST or TEST_F.
function_name = match_result.group(1).split()[-1]
if function_name != 'TEST' and function_name != 'TEST_F' and match(r'[A-Z_]+$', function_name):
if (function_name != 'TEST' and function_name != 'TEST_F'
and match(r'[A-Z_]+$', function_name)):
return
joined_line = ''
......@@ -1214,44 +1323,55 @@ def detect_functions(clean_lines, line_number, function_state, error):
joined_line += ' ' + start_line.lstrip()
body_match = search(r'{|;', start_line)
if body_match:
body_start_position = Position(start_line_number, body_match.start(0))
body_start_position = Position(start_line_number,
body_match.start(0))
# Replace template constructs with _ so that no spaces remain in the function name,
# while keeping the column numbers of other characters the same as "line".
line_with_no_templates = iteratively_replace_matches_with_char(r'<[^<>]*>', '_', line)
match_function = search(r'((\w|:|<|>|,|~|(operator\s*(/|-|=|!|\+)+))*)\(', line_with_no_templates)
line_with_no_templates = iteratively_replace_matches_with_char(
r'<[^<>]*>', '_', line)
match_function = search(
r'((\w|:|<|>|,|~|(operator\s*(/|-|=|!|\+)+))*)\(',
line_with_no_templates)
if not match_function:
return # The '(' must have been inside of a template.
# Use the column numbers from the modified line to find the
# function name in the original line.
function = line[match_function.start(1):match_function.end(1)]
function_name_start_position = Position(line_number, match_function.start(1))
function_name_start_position = Position(line_number,
match_function.start(1))
if match(r'TEST', function): # Handle TEST... macros
if match(r'TEST', function): # Handle TEST... macros
parameter_regexp = search(r'(\(.*\))', joined_line)
if parameter_regexp: # Ignore bad syntax
if parameter_regexp: # Ignore bad syntax
function += parameter_regexp.group(1)
else:
function += '()'
parameter_start_position = Position(line_number, match_function.end(1))
parameter_end_position = close_expression(clean_lines.elided, parameter_start_position)
parameter_start_position = Position(line_number,
match_function.end(1))
parameter_end_position = close_expression(
clean_lines.elided, parameter_start_position)
if parameter_end_position.row == len(clean_lines.elided):
# No end was found.
return
if start_line[body_start_position.column] == ';':
end_position = Position(body_start_position.row, body_start_position.column + 1)
end_position = Position(body_start_position.row,
body_start_position.column + 1)
else:
end_position = close_expression(clean_lines.elided, body_start_position)
end_position = close_expression(clean_lines.elided,
body_start_position)
# Check for nonsensical positions. (This happens in test cases which check code snippets.)
if parameter_end_position > body_start_position:
return
function_state.begin(function, function_name_start_position, body_start_position, end_position,
parameter_start_position, parameter_end_position, clean_lines)
function_state.begin(function, function_name_start_position,
body_start_position, end_position,
parameter_start_position,
parameter_end_position, clean_lines)
return
# No body for the function (or evidence of a non-function) was found.
......@@ -1259,7 +1379,8 @@ def detect_functions(clean_lines, line_number, function_state, error):
'Lint failed to find start of function body.')
def check_for_function_lengths(clean_lines, line_number, function_state, error):
def check_for_function_lengths(clean_lines, line_number, function_state,
error):
"""Reports for long function bodies.
For an overview why this is done, see:
......@@ -1308,9 +1429,10 @@ def check_pass_ptr_usage(clean_lines, line_number, function_state, error):
matched_pass_ptr = match(r'^\s*Pass([A-Z][A-Za-z]*)Ptr<', line)
if matched_pass_ptr:
type_name = 'Pass%sPtr' % matched_pass_ptr.group(1)
error(line_number, 'readability/pass_ptr', 5,
'Local variables should never be %s (see '
'http://webkit.org/coding/RefPtr.html).' % type_name)
error(
line_number, 'readability/pass_ptr', 5,
'Local variables should never be %s (see '
'http://webkit.org/coding/RefPtr.html).' % type_name)
def get_previous_non_blank_line(clean_lines, line_number):
......@@ -1330,7 +1452,7 @@ def get_previous_non_blank_line(clean_lines, line_number):
previous_line_number = line_number - 1
while previous_line_number >= 0:
previous_line = clean_lines.elided[previous_line_number]
if not is_blank_line(previous_line): # if not a blank line...
if not is_blank_line(previous_line): # if not a blank line...
return (previous_line, previous_line_number)
previous_line_number -= 1
return ('', -1)
......@@ -1350,16 +1472,18 @@ def check_ctype_functions(clean_lines, line_number, file_state, error):
line = clean_lines.elided[line_number] # Get rid of comments and strings.
ctype_function_search = search(
(r'\b(?P<ctype_function>(isalnum|isalpha|isascii|isblank|iscntrl|isdigit|isgraph|'
r'islower|isprint|ispunct|isspace|isupper|isxdigit|toascii|tolower|toupper))\s*\('), line)
ctype_function_search = search((
r'\b(?P<ctype_function>(isalnum|isalpha|isascii|isblank|iscntrl|isdigit|isgraph|'
r'islower|isprint|ispunct|isspace|isupper|isxdigit|toascii|tolower|toupper))\s*\('
), line)
if not ctype_function_search:
return
ctype_function = ctype_function_search.group('ctype_function')
error(line_number, 'runtime/ctype_function', 4,
'Use equivalent function in <wtf/ASCIICType.h> instead of the %s() function.'
% (ctype_function))
error(
line_number, 'runtime/ctype_function', 4,
'Use equivalent function in <wtf/ASCIICType.h> instead of the %s() function.'
% (ctype_function))
def replaceable_check(operator, macro, line):
......@@ -1385,10 +1509,9 @@ def replaceable_check(operator, macro, line):
# This means we can't catch all the cases where a more specific
# CHECK is possible, but it's less annoying than dealing with
# extraneous warnings.
match_this = (r'\s*' + macro + r'\((\s*' +
match_constant + r'\s*' + operator + r'[^<>].*|'
r'.*[^<>]' + operator + r'\s*' + match_constant +
r'\s*\))')
match_this = (r'\s*' + macro + r'\((\s*' + match_constant + r'\s*' +
operator + r'[^<>].*|'
r'.*[^<>]' + operator + r'\s*' + match_constant + r'\s*\))')
# Don't complain about CHECK(x == NULL) or similar because
# CHECK_EQ(x, NULL) won't compile (requires a cast).
......@@ -1417,15 +1540,16 @@ def check_check(clean_lines, line_number, error):
# Don't waste time here if line doesn't contain 'CHECK' or 'EXPECT'
return
line = clean_lines.elided[line_number] # get rid of comments and strings
line = clean_lines.elided[line_number] # get rid of comments and strings
# Encourage replacing plain CHECKs with CHECK_EQ/CHECK_NE/etc.
for operator in ['==', '!=', '>=', '>', '<=', '<']:
if replaceable_check(operator, current_macro, line):
error(line_number, 'readability/check', 2,
'Consider using %s(a, b) instead of %s(a %s b)' % (
_CHECK_REPLACEMENT[current_macro][operator],
current_macro, operator))
error(
line_number, 'readability/check', 2,
'Consider using %s(a, b) instead of %s(a %s b)' %
(_CHECK_REPLACEMENT[current_macro][operator], current_macro,
operator))
break
......@@ -1435,10 +1559,13 @@ def check_for_comparisons_to_boolean(clean_lines, line_number, error):
# Must include NULL here, as otherwise users will convert NULL to 0 and
# then we can't catch it, since it looks like a valid integer comparison.
if search(r'[=!]=\s*(NULL|nullptr|true|false)[^\w.]', line) or search(r'[^\w.](NULL|nullptr|true|false)\s*[=!]=', line):
if search(r'[=!]=\s*(NULL|nullptr|true|false)[^\w.]', line) or search(
r'[^\w.](NULL|nullptr|true|false)\s*[=!]=', line):
if not search('LIKELY', line) and not search('UNLIKELY', line):
error(line_number, 'readability/comparison_to_boolean', 5,
'Tests for true/false and null/non-null should be done without equality comparisons.')
error(
line_number, 'readability/comparison_to_boolean', 5,
'Tests for true/false and null/non-null should be done without equality comparisons.'
)
def get_line_width(line):
......@@ -1462,7 +1589,8 @@ def get_line_width(line):
return len(line)
def check_conditional_and_loop_bodies_for_brace_violations(clean_lines, line_number, error):
def check_conditional_and_loop_bodies_for_brace_violations(
clean_lines, line_number, error):
"""Scans the bodies of conditionals and loops, and in particular
all the arms of conditionals, for violations in the use of braces.
......@@ -1532,8 +1660,10 @@ def check_conditional_and_loop_bodies_for_brace_violations(clean_lines, line_num
current_arm_uses_brace = True
if know_whether_using_braces:
if using_braces != current_arm_uses_brace:
error(current_pos.row, 'whitespace/braces', 4,
'If one part of an if-else statement uses curly braces, the other part must too.')
error(
current_pos.row, 'whitespace/braces', 4,
'If one part of an if-else statement uses curly braces, the other part must too.'
)
return
know_whether_using_braces = True
using_braces = current_arm_uses_brace
......@@ -1554,8 +1684,10 @@ def check_conditional_and_loop_bodies_for_brace_violations(clean_lines, line_num
# fire this error for expressions ending on the same line; that
# is a different error, handled elsewhere.)
if current_pos.row > 1 + end_line_of_conditional:
error(current_pos.row, 'whitespace/braces', 4,
'A conditional or loop body must use braces if the statement is more than one line long.')
error(
current_pos.row, 'whitespace/braces', 4,
'A conditional or loop body must use braces if the statement is more than one line long.'
)
return
current_pos = Position(current_pos.row, 1 + current_pos.column)
......@@ -1602,8 +1734,8 @@ def check_redundant_virtual(clean_lines, linenum, error):
# Ignore "virtual" keywords that are near access-specifiers. These
# are only used in class base-specifier and do not apply to member
# functions.
if (search(r'\b(public|protected|private)\s+$', virtual.group(1)) or
match(r'^\s+(public|protected|private)\b', virtual.group(3))):
if (search(r'\b(public|protected|private)\s+$', virtual.group(1))
or match(r'^\s+(public|protected|private)\b', virtual.group(3))):
return
# Ignore the "virtual" keyword from virtual base classes. Usually
......@@ -1619,13 +1751,15 @@ def check_redundant_virtual(clean_lines, linenum, error):
# that this is rare.
end_position = Position(-1, -1)
start_col = len(virtual.group(2))
for start_line in xrange(linenum, min(linenum + 3, clean_lines.num_lines())):
for start_line in xrange(linenum, min(linenum + 3,
clean_lines.num_lines())):
line = clean_lines.elided[start_line][start_col:]
parameter_list = match(r'^([^(]*)\(', line)
if parameter_list:
# Match parentheses to find the end of the parameter list
end_position = close_expression(
clean_lines.elided, Position(start_line, start_col + len(parameter_list.group(1))))
clean_lines.elided,
Position(start_line, start_col + len(parameter_list.group(1))))
break
start_col = 0
......@@ -1634,7 +1768,8 @@ def check_redundant_virtual(clean_lines, linenum, error):
# Look for "override" or "final" after the parameter list
# (possibly on the next few lines).
for i in xrange(end_position.row, min(end_position.row + 3, clean_lines.num_lines())):
for i in xrange(end_position.row,
min(end_position.row + 3, clean_lines.num_lines())):
line = clean_lines.elided[i][end_position.column:]
override_or_final = search(r'\b(override|final)\b', line)
if override_or_final:
......@@ -1710,7 +1845,8 @@ _RE_PATTERN_INCLUDE = re.compile(r'^\s*#\s*include\s*([<"])([^>"]*)[>"].*$')
_RE_FIRST_COMPONENT = re.compile(r'^[^-_.]+')
def check_include_line(filename, file_extension, clean_lines, line_number, include_state, error):
def check_include_line(filename, file_extension, clean_lines, line_number,
include_state, error):
"""Check rules that are applicable to #include lines.
Strings on #include lines are NOT removed from elided line, to make
......@@ -1745,8 +1881,8 @@ def check_include_line(filename, file_extension, clean_lines, line_number, inclu
include_state[include] = line_number
def check_language(filename, clean_lines, line_number, file_extension, include_state,
file_state, error):
def check_language(filename, clean_lines, line_number, file_extension,
include_state, file_state, error):
"""Checks rules from the 'C++ language rules' section of cppguide.html.
Some of these rules are hard to test (function overloading, using
......@@ -1770,7 +1906,8 @@ def check_language(filename, clean_lines, line_number, file_extension, include_s
matched = _RE_PATTERN_INCLUDE.search(line)
if matched:
check_include_line(filename, file_extension, clean_lines, line_number, include_state, error)
check_include_line(filename, file_extension, clean_lines, line_number,
include_state, error)
return
# FIXME: figure out if they're using default arguments in fn proto.
......@@ -1778,30 +1915,31 @@ def check_language(filename, clean_lines, line_number, file_extension, include_s
# Check if they're using a precise-width integer type.
matched = search(r'\b((un)?signed\s+)?(short|(long\s+)?long)\b', line)
if matched:
error(line_number, 'runtime/int', 1,
'Use a precise-width integer type from <stdint.h> or <cstdint>'
' such as uint16_t instead of %s' % matched.group(0))
error(
line_number, 'runtime/int', 1,
'Use a precise-width integer type from <stdint.h> or <cstdint>'
' such as uint16_t instead of %s' % matched.group(0))
# Check to see if they're using an conversion function cast.
# I just try to capture the most common basic types, though there are more.
# Parameterless conversion functions, such as bool(), are allowed as they are
# probably a member operator declaration or default constructor.
matched = search(
r'\b(int|float|double|bool|char|int32|uint32|int64|uint64)\([^)]', line)
r'\b(int|float|double|bool|char|int32|uint32|int64|uint64)\([^)]',
line)
if matched:
# gMock methods are defined using some variant of MOCK_METHODx(name, type)
# where type may be float(), int(string), etc. Without context they are
# virtually indistinguishable from int(x) casts.
if not match(r'^\s*MOCK_(CONST_)?METHOD\d+(_T)?\(', line):
error(line_number, 'readability/casting', 4,
'Using deprecated casting style. '
'Use static_cast<%s>(...) instead' %
matched.group(1))
check_c_style_cast(line_number, line, clean_lines.raw_lines[line_number],
'static_cast',
r'\((int|float|double|bool|char|u?int(16|32|64))\)',
error)
error(
line_number, 'readability/casting', 4,
'Using deprecated casting style. '
'Use static_cast<%s>(...) instead' % matched.group(1))
check_c_style_cast(
line_number, line, clean_lines.raw_lines[line_number], 'static_cast',
r'\((int|float|double|bool|char|u?int(16|32|64))\)', error)
# This doesn't catch all cases. Consider (const char * const)"hello".
check_c_style_cast(line_number, line, clean_lines.raw_lines[line_number],
'reinterpret_cast', r'\((\w+\s?\*+\s?)\)', error)
......@@ -1815,9 +1953,10 @@ def check_language(filename, clean_lines, line_number, file_extension, include_s
# When snprintf is used, the second argument shouldn't be a literal.
matched = search(r'snprintf\s*\(([^,]*),\s*([0-9]*)\s*,', line)
if matched:
error(line_number, 'runtime/printf', 3,
'If you can, use sizeof(%s) instead of %s as the 2nd arg '
'to snprintf.' % (matched.group(1), matched.group(2)))
error(
line_number, 'runtime/printf', 3,
'If you can, use sizeof(%s) instead of %s as the 2nd arg '
'to snprintf.' % (matched.group(1), matched.group(2)))
# Check if some verboten C functions are being used.
if search(r'\bsprintf\b', line):
......@@ -1835,23 +1974,27 @@ def check_language(filename, clean_lines, line_number, file_extension, include_s
# Check for potential format string bugs like printf(foo).
# We constrain the pattern not to pick things like DocidForPrintf(foo).
# Not perfect but it can catch printf(foo.c_str()) and printf(foo->c_str())
matched = re.search(r'\b((?:string)?printf)\s*\(([\w.\->()]+)\)', line, re.I)
matched = re.search(r'\b((?:string)?printf)\s*\(([\w.\->()]+)\)', line,
re.I)
if matched:
error(line_number, 'runtime/printf', 4,
'Potential format string bug. Do %s("%%s", %s) instead.'
% (matched.group(1), matched.group(2)))
error(
line_number, 'runtime/printf', 4,
'Potential format string bug. Do %s("%%s", %s) instead.' %
(matched.group(1), matched.group(2)))
# Check for potential memset bugs like memset(buf, sizeof(buf), 0).
matched = search(r'memset\s*\(([^,]*),\s*([^,]*),\s*0\s*\)', line)
if matched and not match(r"^''|-?[0-9]+|0x[0-9A-Fa-f]$", matched.group(2)):
error(line_number, 'runtime/memset', 4,
'Did you mean "memset(%s, 0, %s)"?'
% (matched.group(1), matched.group(2)))
error(
line_number, 'runtime/memset', 4,
'Did you mean "memset(%s, 0, %s)"?' % (matched.group(1),
matched.group(2)))
# Detect variable-length arrays.
matched = match(r'\s*(.+::)?(\w+) [a-z]\w*\[(.+)];', line)
if (matched and matched.group(2) != 'return' and matched.group(2) != 'delete' and
matched.group(3).find(']') == -1):
if (matched and matched.group(2) != 'return'
and matched.group(2) != 'delete'
and matched.group(3).find(']') == -1):
# Split the size using space and arithmetic operators as delimiters.
# If any of the resulting tokens are not compile time constants then
# report the error.
......@@ -1891,26 +2034,33 @@ def check_language(filename, clean_lines, line_number, file_extension, include_s
is_const = False
break
if not is_const:
error(line_number, 'runtime/arrays', 1,
'Do not use variable-length arrays. Use an appropriately named '
"('k' followed by CamelCase) compile-time constant for the size.")
error(
line_number, 'runtime/arrays', 1,
'Do not use variable-length arrays. Use an appropriately named '
"('k' followed by CamelCase) compile-time constant for the size."
)
# Check for plain bitfields declared without either "singed" or "unsigned".
# Most compilers treat such bitfields as signed, but there are still compilers like
# RVCT 4.0 that use unsigned by default.
matched = re.match(
r'\s*((const|mutable)\s+)?(char|(short(\s+int)?)|int|long(\s+(long|int))?)\s+[a-zA-Z_][a-zA-Z0-9_]*\s*:\s*\d+\s*;', line)
r'\s*((const|mutable)\s+)?(char|(short(\s+int)?)|int|long(\s+(long|int))?)\s+[a-zA-Z_][a-zA-Z0-9_]*\s*:\s*\d+\s*;',
line)
if matched:
error(line_number, 'runtime/bitfields', 5,
'Please declare integral type bitfields with either signed or unsigned.')
error(
line_number, 'runtime/bitfields', 5,
'Please declare integral type bitfields with either signed or unsigned.'
)
check_identifier_name_in_declaration(filename, line_number, line, file_state, error)
check_identifier_name_in_declaration(filename, line_number, line,
file_state, error)
# Check for usage of static_cast<Classname*>.
check_for_object_static_cast(filename, line_number, line, error)
def check_identifier_name_in_declaration(filename, line_number, line, file_state, error):
def check_identifier_name_in_declaration(filename, line_number, line,
file_state, error):
"""Checks if identifier names contain any underscores.
As identifiers in libraries we are using have a bunch of
......@@ -1939,7 +2089,9 @@ def check_identifier_name_in_declaration(filename, line_number, line, file_state
line = sub(r'long (long )?(?=long|double|int)', '', 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)
line = sub(
r'\b(inline|using|static|const|volatile|auto|register|extern|typedef|restrict|struct|class|virtual)(?=\W)',
'', line)
# Remove "new" and "new (expr)" to simplify, too.
line = sub(r'new\s*(\([^)]*\))?', '', line)
......@@ -1947,7 +2099,8 @@ def check_identifier_name_in_declaration(filename, line_number, line, file_state
# Remove all template parameters by removing matching < and >.
# Loop until no templates are removed to remove nested templates.
while True:
line, number_of_replacements = subn(r'<([\w\s:]|::)+\s*[*&]*\s*>', '', line)
line, number_of_replacements = subn(r'<([\w\s:]|::)+\s*[*&]*\s*>', '',
line)
if not number_of_replacements:
break
......@@ -1965,7 +2118,8 @@ def check_identifier_name_in_declaration(filename, line_number, line, file_state
#
# and we will need different treatments for them.
line = sub(r'^\s*for\s*\(', '', line)
line, control_statement = subn(r'^\s*(while|else if|if|switch)\s*\(', '', line)
line, control_statement = subn(r'^\s*(while|else if|if|switch)\s*\(', '',
line)
# Detect variable and functions.
type_regexp = r'\w([\w]|\s*[*&]\s*|::)+'
......@@ -1989,7 +2143,8 @@ def check_identifier_name_in_declaration(filename, line_number, line, file_state
if not matched:
return
identifier = matched.group('identifier')
character_after_identifier = matched.group('character_after_identifier')
character_after_identifier = matched.group(
'character_after_identifier')
# If we removed a non-for-control statement, the character after
# the identifier should be '='. With this rule, we can avoid
......@@ -2025,9 +2180,11 @@ def check_for_toFoo_definition(filename, pattern, error):
pattern: The conversion function pattern to grep for.
error: The function to call with any errors found.
"""
def get_abs_filepath(filename):
fileSystem = FileSystem()
base_dir = fileSystem.path_to_module(FileSystem.__module__).split('WebKit', 1)[0]
base_dir = fileSystem.path_to_module(FileSystem.__module__).split(
'WebKit', 1)[0]
base_dir = ''.join((base_dir, 'WebKit/Source'))
for root, _, names in os.walk(base_dir):
if filename in names:
......@@ -2048,7 +2205,10 @@ def check_for_toFoo_definition(filename, pattern, error):
# catch invalid conversions and shouldn't be part of possible alternatives.
result = re.search(r'%s(\s+)%s' % ('void', pattern), line)
if not result:
matches.append([line, function_state.body_start_position.row, function_state.end_position.row + 1])
matches.append([
line, function_state.body_start_position.row,
function_state.end_position.row + 1
])
function_state = None
except UnicodeDecodeError:
# There would be no non-ascii characters in the codebase ever. The only exception
......@@ -2120,7 +2280,8 @@ def check_for_object_static_cast(processing_file, line_number, line, error):
class_name = class_name[namespace_pos + 2:]
header_file = ''.join((class_name, '.h'))
matches = check_for_toFoo_definition(header_file, ''.join(('to', class_name)), error)
matches = check_for_toFoo_definition(header_file, ''.join(
('to', class_name)), error)
# Ignore (for now) if not able to find the header where toFoo might be defined.
# TODO: Handle cases where Classname might be defined in some other header or cpp file.
if matches is None:
......@@ -2138,19 +2299,20 @@ def check_for_object_static_cast(processing_file, line_number, line, error):
if len(matches):
# toFoo is defined - enforce using it.
# TODO: Suggest an appropriate toFoo from the alternatives present in matches.
error(line_number, 'runtime/casting', 4,
'static_cast of class objects is not allowed. Use to%s defined in %s.' %
(class_name, header_file))
error(
line_number, 'runtime/casting', 4,
'static_cast of class objects is not allowed. Use to%s defined in %s.'
% (class_name, header_file))
else:
# No toFoo defined - enforce definition & usage.
# TODO: Automate the generation of toFoo() to avoid any slippages ever.
error(line_number, 'runtime/casting', 4,
'static_cast of class objects is not allowed. Add to%s in %s and use it instead.' %
(class_name, header_file))
error(
line_number, 'runtime/casting', 4,
'static_cast of class objects is not allowed. Add to%s in %s and use it instead.'
% (class_name, header_file))
def check_c_style_cast(line_number, line, raw_line, cast_type, pattern,
error):
def check_c_style_cast(line_number, line, raw_line, cast_type, pattern, error):
"""Checks for a C-style cast by looking for the pattern.
This also handles sizeof(type) warnings, due to similarity of content.
......@@ -2193,46 +2355,86 @@ def check_c_style_cast(line_number, line, raw_line, cast_type, pattern,
return
# At this point, all that should be left is actual casts.
error(line_number, 'readability/casting', 4,
'Using C-style cast. Use %s<%s>(...) instead' %
(cast_type, matched.group(1)))
error(
line_number, 'readability/casting', 4,
'Using C-style cast. Use %s<%s>(...) instead' % (cast_type,
matched.group(1)))
_HEADERS_CONTAINING_TEMPLATES = (
('<deque>', ('deque',)),
('<functional>', ('unary_function', 'binary_function',
'plus', 'minus', 'multiplies', 'divides', 'modulus',
'negate',
'equal_to', 'not_equal_to', 'greater', 'less',
'greater_equal', 'less_equal',
'logical_and', 'logical_or', 'logical_not',
'unary_negate', 'not1', 'binary_negate', 'not2',
'bind1st', 'bind2nd',
'pointer_to_unary_function',
'pointer_to_binary_function',
'ptr_fun',
'mem_fun_t', 'mem_fun', 'mem_fun1_t', 'mem_fun1_ref_t',
'mem_fun_ref_t',
'const_mem_fun_t', 'const_mem_fun1_t',
'const_mem_fun_ref_t', 'const_mem_fun1_ref_t',
'mem_fun_ref',
)),
('<limits>', ('numeric_limits',)),
('<list>', ('list',)),
('<map>', ('map', 'multimap',)),
('<memory>', ('allocator',)),
('<queue>', ('queue', 'priority_queue',)),
('<set>', ('set', 'multiset',)),
('<stack>', ('stack',)),
('<string>', ('char_traits', 'basic_string',)),
('<utility>', ('pair',)),
('<vector>', ('vector',)),
('<deque>', ('deque', )),
('<functional>', (
'unary_function',
'binary_function',
'plus',
'minus',
'multiplies',
'divides',
'modulus',
'negate',
'equal_to',
'not_equal_to',
'greater',
'less',
'greater_equal',
'less_equal',
'logical_and',
'logical_or',
'logical_not',
'unary_negate',
'not1',
'binary_negate',
'not2',
'bind1st',
'bind2nd',
'pointer_to_unary_function',
'pointer_to_binary_function',
'ptr_fun',
'mem_fun_t',
'mem_fun',
'mem_fun1_t',
'mem_fun1_ref_t',
'mem_fun_ref_t',
'const_mem_fun_t',
'const_mem_fun1_t',
'const_mem_fun_ref_t',
'const_mem_fun1_ref_t',
'mem_fun_ref',
)),
('<limits>', ('numeric_limits', )),
('<list>', ('list', )),
('<map>', (
'map',
'multimap',
)),
('<memory>', ('allocator', )),
('<queue>', (
'queue',
'priority_queue',
)),
('<set>', (
'set',
'multiset',
)),
('<stack>', ('stack', )),
('<string>', (
'char_traits',
'basic_string',
)),
('<utility>', ('pair', )),
('<vector>', ('vector', )),
# gcc extensions.
# Note: std::hash is their hash, ::hash is our hash
('<hash_map>', ('hash_map', 'hash_multimap',)),
('<hash_set>', ('hash_set', 'hash_multiset',)),
('<slist>', ('slist',)),
('<hash_map>', (
'hash_map',
'hash_multimap',
)),
('<hash_set>', (
'hash_set',
'hash_multiset',
)),
('<slist>', ('slist', )),
)
_HEADERS_ACCEPTED_BUT_NOT_PROMOTED = {
......@@ -2248,16 +2450,14 @@ for _template in ('copy', 'max', 'min', 'min_element', 'sort', 'swap',
# Match max<type>(..., ...), max(..., ...), but not foo->max, foo.max or
# type::max().
_re_pattern_algorithm_header.append(
(re.compile(r'[^>.]\b' + _template + r'(<.*?>)?\([^\)]'),
_template,
(re.compile(r'[^>.]\b' + _template + r'(<.*?>)?\([^\)]'), _template,
'<algorithm>'))
_re_pattern_templates = []
for _header, _templates in _HEADERS_CONTAINING_TEMPLATES:
for _template in _templates:
_re_pattern_templates.append(
(re.compile(r'(\<|\b)' + _template + r'\s*\<'),
_template + '<>',
(re.compile(r'(\<|\b)' + _template + r'\s*\<'), _template + '<>',
_header))
......@@ -2341,11 +2541,13 @@ def update_include_state(filename, include_state):
include = matched.group(2)
# The value formatting is cute, but not really used right now.
# What matters here is that the key is in include_state.
include_state.setdefault(include, '%s:%d' % (filename, line_number))
include_state.setdefault(include,
'%s:%d' % (filename, line_number))
return True
def check_for_include_what_you_use(filename, clean_lines, include_state, error):
def check_for_include_what_you_use(filename, clean_lines, include_state,
error):
"""Reports for missing stl includes.
This function will output warnings to make sure you are including the headers
......@@ -2360,7 +2562,8 @@ def check_for_include_what_you_use(filename, clean_lines, include_state, error):
include_state: An _IncludeState instance.
error: The function to call with any errors found.
"""
required = {} # A map of header name to line_number and the template entity.
# A map of header name to line_number and the template entity.
required = {}
# Example of required: { '<functional>': (1219, 'less<>') }
for line_number in xrange(clean_lines.num_lines()):
......@@ -2407,7 +2610,8 @@ def check_for_include_what_you_use(filename, clean_lines, include_state, error):
# include_state is modified during iteration, so we iterate over a copy of
# the keys.
for header in include_state.keys(): # NOLINT
(same_module, common_path) = files_belong_to_same_module(abs_filename, header)
(same_module, common_path) = files_belong_to_same_module(
abs_filename, header)
fullpath = common_path + header
if same_module and update_include_state(fullpath, include_state):
header_found = True
......@@ -2428,14 +2632,14 @@ def check_for_include_what_you_use(filename, clean_lines, include_state, error):
if [True for header in headers if header in include_state]:
continue
if required_header_unstripped.strip('<>"') not in include_state:
error(required[required_header_unstripped][0],
'build/include_what_you_use', 4,
'Add #include ' + required_header_unstripped + ' for ' + template)
error(
required[required_header_unstripped][0],
'build/include_what_you_use', 4, 'Add #include ' +
required_header_unstripped + ' for ' + template)
def process_line(filename, file_extension,
clean_lines, line, include_state, function_state,
class_state, file_state, error):
def process_line(filename, file_extension, clean_lines, line, include_state,
function_state, class_state, file_state, error):
"""Processes a single line in the file.
Args:
......@@ -2458,7 +2662,8 @@ def process_line(filename, file_extension,
check_for_function_lengths(clean_lines, line, function_state, error)
if search(r'\bNOLINT\b', raw_lines[line]): # ignore nolint lines
return
if match(r'\s*\b__asm\b', raw_lines[line]): # Ignore asm lines as they format differently.
# Ignore asm lines as they format differently.
if match(r'\s*\b__asm\b', raw_lines[line]):
return
check_pass_ptr_usage(clean_lines, line, function_state, error)
check_style(clean_lines, line, file_state, error)
......@@ -2467,7 +2672,8 @@ def process_line(filename, file_extension,
check_for_non_standard_constructs(clean_lines, line, class_state, error)
check_posix_threading(clean_lines, line, error)
check_invalid_increment(clean_lines, line, error)
check_conditional_and_loop_bodies_for_brace_violations(clean_lines, line, error)
check_conditional_and_loop_bodies_for_brace_violations(
clean_lines, line, error)
check_redundant_virtual(clean_lines, line, error)
check_redundant_override(clean_lines, line, error)
......@@ -2482,8 +2688,8 @@ def _process_lines(filename, file_extension, lines, error, min_confidence):
last element being empty if the file is terminated with a newline.
error: A callable to which errors are reported, which takes 4 arguments:
"""
lines = (['// marker so line numbers and indices both start at 1'] + lines +
['// marker so line numbers end in a known way'])
lines = (['// marker so line numbers and indices both start at 1'] + lines
+ ['// marker so line numbers end in a known way'])
include_state = _IncludeState()
function_state = _FunctionState(min_confidence)
......@@ -2512,7 +2718,6 @@ def _process_lines(filename, file_extension, lines, error, min_confidence):
class CppChecker(object):
"""Processes C++ lines for checking style."""
# This list is used to--
......@@ -2554,8 +2759,12 @@ class CppChecker(object):
fs = None
def __init__(self, file_path, file_extension, handle_style_error,
min_confidence, fs=None):
def __init__(self,
file_path,
file_extension,
handle_style_error,
min_confidence,
fs=None):
"""Create a CppChecker instance.
Args:
......@@ -2593,6 +2802,11 @@ class CppChecker(object):
# FIXME: Remove this function (requires refactoring unit tests).
def process_file_data(filename, file_extension, lines, error, min_confidence, fs=None):
def process_file_data(filename,
file_extension,
lines,
error,
min_confidence,
fs=None):
checker = CppChecker(filename, file_extension, error, min_confidence, fs)
checker.check(lines)
This source diff could not be displayed because it is too large. You can view the blob instead.
......@@ -19,7 +19,6 @@
# 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.
"""Checks WebKit style for JSON files."""
import json
......@@ -29,7 +28,7 @@ import re
class JSONChecker(object):
"""Processes JSON lines for checking style."""
categories = set(('json/syntax',))
categories = set(('json/syntax', ))
def __init__(self, _, handle_style_error):
self._handle_style_error = handle_style_error
......@@ -40,8 +39,8 @@ class JSONChecker(object):
json.loads('\n'.join(lines) + '\n')
except ValueError as error:
self._handle_style_error(
self.line_number_from_json_exception(error),
'json/syntax', 5, str(error))
self.line_number_from_json_exception(error), 'json/syntax', 5,
str(error))
@staticmethod
def line_number_from_json_exception(error):
......
......@@ -19,7 +19,6 @@
# 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 test for jsonchecker.py."""
import unittest
......@@ -28,7 +27,6 @@ from blinkpy.style.checkers import jsonchecker
class MockErrorHandler(object):
def __init__(self, handle_style_error):
self.turned_off_filtering = False
self._handle_style_error = handle_style_error
......@@ -37,7 +35,8 @@ class MockErrorHandler(object):
self.turned_off_filtering = True
def __call__(self, line_number, category, confidence, message):
self._handle_style_error(self, line_number, category, confidence, message)
self._handle_style_error(self, line_number, category, confidence,
message)
return True
......@@ -52,11 +51,16 @@ class JSONCheckerTest(unittest.TestCase):
(9, 'Expecting property name: line 9 column 21 (char 478)'),
)
for expected_line, message in tests:
self.assertEqual(expected_line, jsonchecker.JSONChecker.line_number_from_json_exception(ValueError(message)))
self.assertEqual(
expected_line,
jsonchecker.JSONChecker.line_number_from_json_exception(
ValueError(message)))
def assert_no_error(self, json_data):
def handle_style_error(mock_error_handler, line_number, category, confidence, message):
self.fail('Unexpected error: %d %s %d %s' % (line_number, category, confidence, message))
def handle_style_error(mock_error_handler, line_number, category,
confidence, message):
self.fail('Unexpected error: %d %s %d %s' % (line_number, category,
confidence, message))
error_handler = MockErrorHandler(handle_style_error)
checker = jsonchecker.JSONChecker('foo.json', error_handler)
......@@ -64,7 +68,8 @@ class JSONCheckerTest(unittest.TestCase):
self.assertTrue(error_handler.turned_off_filtering)
def assert_error(self, expected_line_number, expected_category, json_data):
def handle_style_error(mock_error_handler, line_number, category, confidence, message):
def handle_style_error(mock_error_handler, line_number, category,
confidence, message):
mock_error_handler.had_error = True
self.assertEqual(expected_line_number, line_number)
self.assertEqual(expected_category, category)
......
......@@ -20,7 +20,6 @@
# 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 checking WebKit style in png files."""
from blinkpy.common import read_checksum_from_png
......@@ -38,9 +37,12 @@ class PNGChecker(object):
self._fs = self._host.filesystem
def check(self, inline=None):
if self._fs.exists(self._file_path) and self._file_path.endswith('-expected.png'):
with self._fs.open_binary_file_for_reading(self._file_path) as filehandle:
if self._fs.exists(
self._file_path) and self._file_path.endswith('-expected.png'):
with self._fs.open_binary_file_for_reading(
self._file_path) as filehandle:
if not read_checksum_from_png.read_checksum(filehandle):
self._handle_style_error(
0, 'image/png', 5,
'Image lacks a checksum. Generate pngs using run_web_tests.py to ensure they have a checksum.')
'Image lacks a checksum. Generate pngs using run_web_tests.py to ensure they have a checksum.'
)
......@@ -20,7 +20,6 @@
# 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 test for png.py."""
import unittest
......@@ -39,14 +38,16 @@ class PNGCheckerTest(unittest.TestCase):
def mock_handle_style_error(self):
pass
checker = PNGChecker("test/config", mock_handle_style_error, MockSystemHost())
checker = PNGChecker("test/config", mock_handle_style_error,
MockSystemHost())
self.assertEqual(checker._file_path, "test/config")
self.assertEqual(checker._handle_style_error, mock_handle_style_error)
def test_check(self):
errors = []
def mock_handle_style_error(line_number, category, confidence, message):
def mock_handle_style_error(line_number, category, confidence,
message):
error = (line_number, category, confidence, message)
errors.append(error)
......@@ -55,16 +56,19 @@ class PNGCheckerTest(unittest.TestCase):
file_path = "foo.png"
fs.write_binary_file(file_path, "Dummy binary data")
errors = []
checker = PNGChecker(file_path, mock_handle_style_error, MockSystemHost(os_name='linux', filesystem=fs))
checker = PNGChecker(file_path, mock_handle_style_error,
MockSystemHost(os_name='linux', filesystem=fs))
checker.check()
self.assertEqual(len(errors), 0)
file_path = "foo-expected.png"
fs.write_binary_file(file_path, "Dummy binary data")
errors = []
checker = PNGChecker(file_path, mock_handle_style_error, MockSystemHost(os_name='linux', filesystem=fs))
checker = PNGChecker(file_path, mock_handle_style_error,
MockSystemHost(os_name='linux', filesystem=fs))
checker.check()
self.assertEqual(len(errors), 1)
self.assertEqual(
errors[0],
(0, 'image/png', 5, 'Image lacks a checksum. Generate pngs using run_web_tests.py to ensure they have a checksum.'))
self.assertEqual(errors[0], (
0, 'image/png', 5,
'Image lacks a checksum. Generate pngs using run_web_tests.py to ensure they have a checksum.'
))
......@@ -19,14 +19,12 @@
# 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 checking WebKit style in Python files."""
import os
import re
import sys
from blinkpy.common.path_finder import PathFinder
from blinkpy.common.path_finder import get_blink_tools_dir
from blinkpy.common.path_finder import get_blinkpy_thirdparty_dir
......@@ -94,7 +92,9 @@ class PythonChecker(object):
'--output-format=parseable',
'--rcfile=' + finder.path_from_blink_tools('blinkpy', 'pylintrc'),
path,
], env=env, error_handler=executive.ignore_error)
],
env=env,
error_handler=executive.ignore_error)
def _parse_pylint_output(self, output):
# We filter out these messages because they are bugs in pylint that produce false positives.
......@@ -123,7 +123,8 @@ class PythonChecker(object):
category_and_method = match_obj.group(3).split(', ')
category = 'pylint/' + (category_and_method[0])
if len(category_and_method) > 1:
message = '[%s] %s' % (category_and_method[1], match_obj.group(4))
message = '[%s] %s' % (category_and_method[1],
match_obj.group(4))
else:
message = match_obj.group(4)
errors.append((line_number, category, message))
......
......@@ -19,7 +19,6 @@
# 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 python.py."""
import os
......@@ -29,7 +28,6 @@ from blinkpy.style.checkers.python import PythonChecker
class PythonCheckerTest(unittest.TestCase):
"""Tests the PythonChecker class."""
def test_init(self):
......@@ -40,8 +38,7 @@ class PythonCheckerTest(unittest.TestCase):
checker = PythonChecker("foo.txt", _mock_handle_style_error)
self.assertEqual(checker._file_path, "foo.txt")
self.assertEqual(checker._handle_style_error,
_mock_handle_style_error)
self.assertEqual(checker._handle_style_error, _mock_handle_style_error)
# TODO(crbug.com/757067): Figure out why this is failing on LUCI mac/win.
def disable_test_check(self):
......@@ -59,13 +56,13 @@ class PythonCheckerTest(unittest.TestCase):
checker = PythonChecker(file_path, _mock_handle_style_error)
checker.check()
self.assertEqual(
[
(2, 'pep8/W291', 5, 'trailing whitespace'),
(3, 'pep8/E261', 5, 'at least two spaces before inline comment'),
(3, 'pep8/E262', 5, "inline comment should start with '# '"),
(2, 'pylint/C0303(trailing-whitespace)', 5, '[] Trailing whitespace'),
(2, 'pylint/E0602(undefined-variable)', 5, u"[] Undefined variable 'error'"),
(3, 'pylint/W0611(unused-import)', 5, '[] Unused import math'),
],
errors)
self.assertEqual([
(2, 'pep8/W291', 5, 'trailing whitespace'),
(3, 'pep8/E261', 5, 'at least two spaces before inline comment'),
(3, 'pep8/E262', 5, "inline comment should start with '# '"),
(2, 'pylint/C0303(trailing-whitespace)', 5,
'[] Trailing whitespace'),
(2, 'pylint/E0602(undefined-variable)', 5,
u"[] Undefined variable 'error'"),
(3, 'pylint/W0611(unused-import)', 5, '[] Unused import math'),
], errors)
......@@ -25,7 +25,6 @@
# 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.
"""Checks WebKit style for test_expectations files."""
import logging
......@@ -51,7 +50,8 @@ class TestExpectationsChecker(object):
self._port_obj = host.port_factory.get()
# Suppress error messages of test_expectations module since they will be reported later.
log = logging.getLogger('blinkpy.web_tests.layout_package.test_expectations')
log = logging.getLogger(
'blinkpy.web_tests.layout_package.test_expectations')
log.setLevel(logging.CRITICAL)
def check_test_expectations(self, expectations_str, tests=None):
......@@ -66,7 +66,8 @@ class TestExpectationsChecker(object):
def check(self, lines):
expectations = '\n'.join(lines)
if self._port_obj:
self.check_test_expectations(expectations_str=expectations, tests=None)
self.check_test_expectations(
expectations_str=expectations, tests=None)
# Warn tabs in lines as well
self.check_tabs(lines)
......@@ -42,7 +42,11 @@ class ErrorCollector(object):
def turn_off_line_filtering(self):
self.turned_off_filtering = True
def __call__(self, lineno=0, category='test/expectations', confidence=100, message=''):
def __call__(self,
lineno=0,
category='test/expectations',
confidence=100,
message=''):
self._errors.append('%s\n[%s] [%d]' % (message, category, confidence))
return True
......@@ -65,20 +69,21 @@ class TestExpectationsTestCase(unittest.TestCase):
self._error_collector.reset_errors()
host = MockHost()
checker = TestExpectationsChecker('test/TestExpectations',
self._error_collector, host=host)
checker = TestExpectationsChecker(
'test/TestExpectations', self._error_collector, host=host)
# We should have a valid port, but override it with a test port so we
# can check the lines.
self.assertIsNotNone(checker._port_obj)
checker._port_obj = host.port_factory.get('test-mac-mac10.10')
checker.check_test_expectations(expectations_str='\n'.join(lines),
tests=[self._test_file])
checker.check_test_expectations(
expectations_str='\n'.join(lines), tests=[self._test_file])
checker.check_tabs(lines)
if should_pass:
self.assertEqual('', self._error_collector.get_errors())
elif expected_output:
self.assertEqual(expected_output, self._error_collector.get_errors())
self.assertEqual(expected_output,
self._error_collector.get_errors())
else:
self.assertNotEquals('', self._error_collector.get_errors())
......@@ -92,16 +97,25 @@ class TestExpectationsTestCase(unittest.TestCase):
self.assert_lines_lint([
'# tags: [ Mac ]\n'
'# results: [ Pass Failure ]\n'
'crbug.com/1234 [ Mac ] passes/text.html [ Pass Failure ]'], should_pass=True)
'crbug.com/1234 [ Mac ] passes/text.html [ Pass Failure ]'
],
should_pass=True)
def test_invalid_expectations(self):
self.assert_lines_lint(['Bug(me) passes/text.html [ Give Up]'], should_pass=False)
self.assert_lines_lint(['Bug(me) passes/text.html [ Give Up]'],
should_pass=False)
def test_tab(self):
self.assert_lines_lint([
'# results: [ Pass ]\n'
'\tcrbug.com/b/1 passes/text.html [ Pass ]'], should_pass=False,
expected_output='Line contains tab character.\n[whitespace/tab] [5]')
self.assert_lines_lint(
[
'# results: [ Pass ]\n'
'\tcrbug.com/b/1 passes/text.html [ Pass ]'
],
should_pass=False,
expected_output='Line contains tab character.\n[whitespace/tab] [5]'
)
def test_missing_expectation_not_allowed(self):
self.assert_lines_lint(['crbug.com/1234 [ Mac ] passes/text.html [ Missing ]'], should_pass=False)
self.assert_lines_lint(
['crbug.com/1234 [ Mac ] passes/text.html [ Missing ]'],
should_pass=False)
......@@ -26,7 +26,6 @@
# 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.
"""Checks WebKit style for text files."""
from blinkpy.style.checkers.common import TabChecker
......
......@@ -43,7 +43,8 @@ class TextStyleTestCase(unittest.TestCase):
self.had_error = True
process_file_data('', lines, error_for_test)
self.assertFalse(self.had_error, '%s should not have any errors.' % lines)
self.assertFalse(self.had_error,
'%s should not have any errors.' % lines)
def assertError(self, lines, expected_line_number):
"""Asserts that the specified lines has an error."""
......@@ -56,7 +57,8 @@ class TextStyleTestCase(unittest.TestCase):
self.had_error = True
process_file_data('', lines, error_for_test)
self.assertTrue(self.had_error, '%s should have an error [whitespace/tab].' % lines)
self.assertTrue(self.had_error,
'%s should have an error [whitespace/tab].' % lines)
def test_no_error(self):
"""Tests for no error cases."""
......@@ -66,13 +68,13 @@ class TextStyleTestCase(unittest.TestCase):
def test_error(self):
"""Tests for error cases."""
self.assertError(['2009-12-16\tKent Tamura\t<tkent@chromium.org>'], 1)
self.assertError(['2009-12-16 Kent Tamura <tkent@chromium.org>',
'',
'\tReviewed by NOBODY.'], 3)
self.assertError([
'2009-12-16 Kent Tamura <tkent@chromium.org>', '',
'\tReviewed by NOBODY.'
], 3)
class TextCheckerTest(unittest.TestCase):
"""Tests TextChecker class."""
def mock_handle_style_error(self):
......@@ -82,4 +84,5 @@ class TextCheckerTest(unittest.TestCase):
"""Test __init__ constructor."""
checker = TextChecker('foo.txt', self.mock_handle_style_error)
self.assertEqual(checker.file_path, 'foo.txt')
self.assertEqual(checker.handle_style_error, self.mock_handle_style_error)
self.assertEqual(checker.handle_style_error,
self.mock_handle_style_error)
......@@ -20,21 +20,20 @@
# 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.
"""Checks Xcode project files."""
import re
class XcodeProjectFileChecker(object):
"""Processes Xcode project file lines for checking style."""
def __init__(self, file_path, handle_style_error):
self.file_path = file_path
self.handle_style_error = handle_style_error
self.handle_style_error.turn_off_line_filtering()
self._development_region_regex = re.compile('developmentRegion = (?P<region>.+);')
self._development_region_regex = re.compile(
'developmentRegion = (?P<region>.+);')
def _check_development_region(self, line_index, line):
"""Returns True when developmentRegion is detected."""
......@@ -42,8 +41,7 @@ class XcodeProjectFileChecker(object):
if not matched:
return False
if matched.group('region') != 'English':
self.handle_style_error(line_index,
'xcodeproj/settings', 5,
self.handle_style_error(line_index, 'xcodeproj/settings', 5,
'developmentRegion is not English.')
return True
......@@ -54,6 +52,6 @@ class XcodeProjectFileChecker(object):
development_region_is_detected = True
if not development_region_is_detected:
self.handle_style_error(len(lines),
'xcodeproj/settings', 5,
'Missing "developmentRegion = English".')
self.handle_style_error(
len(lines), 'xcodeproj/settings', 5,
'Missing "developmentRegion = English".')
......@@ -20,7 +20,6 @@
# 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 test for xcodeproj.py."""
import unittest
......@@ -47,7 +46,8 @@ class XcodeProjectFileCheckerTest(unittest.TestCase):
def assert_no_error(self, lines):
def handler(error_handler, line_number, category, confidence, message):
self.fail('Unexpected error: %d %s %d %s' % (line_number, category, confidence, message))
self.fail('Unexpected error: %d %s %d %s' % (line_number, category,
confidence, message))
error_handler = TestErrorHandler(handler)
checker = xcodeproj.XcodeProjectFileChecker('', error_handler)
......@@ -59,10 +59,13 @@ class XcodeProjectFileCheckerTest(unittest.TestCase):
def handler(error_handler, line_number, category, confidence, message):
self.assertEqual(expected_message, message)
self.had_error = True
error_handler = TestErrorHandler(handler)
checker = xcodeproj.XcodeProjectFileChecker('', error_handler)
checker.check(lines)
self.assertTrue(self.had_error, '%s should have error: %s.' % (lines, expected_message))
self.assertTrue(
self.had_error,
'%s should have error: %s.' % (lines, expected_message))
def test_detect_development_region(self):
self.assert_no_error(['developmentRegion = English;'])
......
......@@ -19,7 +19,6 @@
# 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.
"""Checks WebKit style for XML files."""
from __future__ import absolute_import
......@@ -42,4 +41,5 @@ class XMLChecker(object):
parser.Parse('\n')
parser.Parse('', True)
except expat.ExpatError as error:
self._handle_style_error(error.lineno, 'xml/syntax', 5, expat.ErrorString(error.code))
self._handle_style_error(error.lineno, 'xml/syntax', 5,
expat.ErrorString(error.code))
......@@ -19,7 +19,6 @@
# 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 test for xml.py."""
import unittest
......@@ -28,7 +27,6 @@ from blinkpy.style.checkers import xml
class MockErrorHandler(object):
def __init__(self, handle_style_error):
self.turned_off_filtering = False
self._handle_style_error = handle_style_error
......@@ -37,7 +35,8 @@ class MockErrorHandler(object):
self.turned_off_filtering = True
def __call__(self, line_number, category, confidence, message):
self._handle_style_error(self, line_number, category, confidence, message)
self._handle_style_error(self, line_number, category, confidence,
message)
return True
......@@ -45,8 +44,10 @@ class XMLCheckerTest(unittest.TestCase):
"""Tests XMLChecker class."""
def assert_no_error(self, xml_data):
def handle_style_error(mock_error_handler, line_number, category, confidence, message):
self.fail('Unexpected error: %d %s %d %s' % (line_number, category, confidence, message))
def handle_style_error(mock_error_handler, line_number, category,
confidence, message):
self.fail('Unexpected error: %d %s %d %s' % \
(line_number, category, confidence, message))
error_handler = MockErrorHandler(handle_style_error)
checker = xml.XMLChecker('foo.xml', error_handler)
......@@ -54,7 +55,8 @@ class XMLCheckerTest(unittest.TestCase):
self.assertTrue(error_handler.turned_off_filtering)
def assert_error(self, expected_line_number, expected_category, xml_data):
def handle_style_error(mock_error_handler, line_number, category, confidence, message):
def handle_style_error(mock_error_handler, line_number, category,
confidence, message):
mock_error_handler.had_error = True
self.assertEqual(expected_line_number, line_number)
self.assertEqual(expected_category, category)
......
......@@ -19,7 +19,6 @@
# 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.
"""Defines style error handler classes.
A style error handler is a function to call when a style error is
......@@ -49,10 +48,12 @@ Methods:
class DefaultStyleErrorHandler(object):
"""The default style error handler."""
def __init__(self, file_path, configuration, increment_error_count,
def __init__(self,
file_path,
configuration,
increment_error_count,
line_numbers=None):
"""Create a default style error handler.
......@@ -126,7 +127,11 @@ class DefaultStyleErrorHandler(object):
def turn_off_line_filtering(self):
self._line_numbers = None
def __call__(self, line_number=0, category='error', confidence='1', message=''):
def __call__(self,
line_number=0,
category='error',
confidence='1',
message=''):
"""Handle the occurrence of a style error.
See the docstring of this module for more information.
......@@ -134,9 +139,10 @@ class DefaultStyleErrorHandler(object):
if not self.should_line_be_checked(line_number):
return False
if not self._configuration.is_reportable(category=category,
confidence_in_error=confidence,
file_path=self._file_path):
if not self._configuration.is_reportable(
category=category,
confidence_in_error=confidence,
file_path=self._file_path):
return False
category_total = self._add_reportable_error(category)
......@@ -147,12 +153,14 @@ class DefaultStyleErrorHandler(object):
# Then suppress displaying the error.
return False
self._configuration.write_style_error(category=category,
confidence_in_error=confidence,
file_path=self._file_path,
line_number=line_number,
message=message)
self._configuration.write_style_error(
category=category,
confidence_in_error=confidence,
file_path=self._file_path,
line_number=line_number,
message=message)
if category_total == max_reports:
self._configuration.stderr_write('Suppressing further [%s] reports '
'for this file.\n' % category)
self._configuration.stderr_write(
'Suppressing further [%s] reports '
'for this file.\n' % category)
return True
......@@ -19,7 +19,6 @@
# 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 error_handlers.py."""
import unittest
......@@ -30,7 +29,6 @@ from blinkpy.style.filter import FilterConfiguration
class DefaultStyleErrorHandlerTest(unittest.TestCase):
"""Tests the DefaultStyleErrorHandler class."""
def setUp(self):
......@@ -62,10 +60,11 @@ class DefaultStyleErrorHandlerTest(unittest.TestCase):
stderr_write=self._mock_stderr_write)
def _error_handler(self, configuration, line_numbers=None):
return DefaultStyleErrorHandler(configuration=configuration,
file_path=self._file_path,
increment_error_count=self._mock_increment_error_count,
line_numbers=line_numbers)
return DefaultStyleErrorHandler(
configuration=configuration,
file_path=self._file_path,
increment_error_count=self._mock_increment_error_count,
line_numbers=line_numbers)
def _check_initialized(self):
"""Check that count and error messages are initialized."""
......@@ -74,10 +73,11 @@ class DefaultStyleErrorHandlerTest(unittest.TestCase):
def _call_error_handler(self, handle_error, confidence, line_number=100):
"""Call the given error handler with a test error."""
handle_error(line_number=line_number,
category=self._category,
confidence=confidence,
message='message')
handle_error(
line_number=line_number,
category=self._category,
confidence=confidence,
message='message')
def test_eq__true_return_value(self):
"""Test the __eq__() method for the return value of True."""
......@@ -88,14 +88,17 @@ class DefaultStyleErrorHandlerTest(unittest.TestCase):
def test_eq__false_return_value(self):
"""Test the __eq__() method for the return value of False."""
def make_handler(configuration=self._style_checker_configuration(),
file_path='foo.txt', increment_error_count=lambda: True,
file_path='foo.txt',
increment_error_count=lambda: True,
line_numbers=None):
line_numbers = line_numbers or [100]
return DefaultStyleErrorHandler(configuration=configuration,
file_path=file_path,
increment_error_count=increment_error_count,
line_numbers=line_numbers)
return DefaultStyleErrorHandler(
configuration=configuration,
file_path=file_path,
increment_error_count=increment_error_count,
line_numbers=line_numbers)
handler = make_handler()
......@@ -105,7 +108,8 @@ class DefaultStyleErrorHandlerTest(unittest.TestCase):
# Verify that a difference in any argument causes equality to fail.
self.assertFalse(handler.__eq__(make_handler(configuration=None)))
self.assertFalse(handler.__eq__(make_handler(file_path='bar.txt')))
self.assertFalse(handler.__eq__(make_handler(increment_error_count=None)))
self.assertFalse(
handler.__eq__(make_handler(increment_error_count=None)))
self.assertFalse(handler.__eq__(make_handler(line_numbers=[50])))
def test_ne(self):
......@@ -125,9 +129,9 @@ class DefaultStyleErrorHandlerTest(unittest.TestCase):
confidence = 1
# Confirm the error is not reportable.
self.assertFalse(configuration.is_reportable(self._category,
confidence,
self._file_path))
self.assertFalse(
configuration.is_reportable(self._category, confidence,
self._file_path))
error_handler = self._error_handler(configuration)
self._call_error_handler(error_handler, confidence)
......@@ -158,9 +162,10 @@ class DefaultStyleErrorHandlerTest(unittest.TestCase):
self.assertEqual(3, len(self._error_messages))
self.assertEqual(self._error_messages[-2],
'foo.h(100): message [whitespace/tab] [5]\n')
self.assertEqual(self._error_messages[-1],
'Suppressing further [whitespace/tab] reports '
'for this file.\n')
self.assertEqual(
self._error_messages[-1],
'Suppressing further [whitespace/tab] reports '
'for this file.\n')
# Third call: no report.
self._call_error_handler(error_handler, confidence)
......@@ -171,8 +176,7 @@ class DefaultStyleErrorHandlerTest(unittest.TestCase):
"""Test the line_numbers parameter."""
self._check_initialized()
configuration = self._style_checker_configuration()
error_handler = self._error_handler(configuration,
line_numbers=[50])
error_handler = self._error_handler(configuration, line_numbers=[50])
confidence = 5
# Error on non-modified line: no error.
......@@ -190,7 +194,8 @@ class DefaultStyleErrorHandlerTest(unittest.TestCase):
error_handler.turn_off_line_filtering()
self._call_error_handler(error_handler, confidence, line_number=60)
self.assertEqual(2, self._error_count)
self.assertEqual(self._error_messages,
['foo.h(50): message [whitespace/tab] [5]\n',
'foo.h(60): message [whitespace/tab] [5]\n',
'Suppressing further [whitespace/tab] reports for this file.\n'])
self.assertEqual(self._error_messages, [
'foo.h(50): message [whitespace/tab] [5]\n',
'foo.h(60): message [whitespace/tab] [5]\n',
'Suppressing further [whitespace/tab] reports for this file.\n'
])
......@@ -27,7 +27,6 @@
# 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 reading and processing text files."""
import codecs
......@@ -35,12 +34,10 @@ import logging
import os
import sys
_log = logging.getLogger(__name__)
class TextFileReader(object):
"""Supports reading and processing text files.
Attributes:
......@@ -109,7 +106,8 @@ class TextFileReader(object):
if not self.filesystem.exists(file_path) and file_path != '-':
_log.error("File does not exist: '%s'", file_path)
sys.exit(1) # FIXME: This should throw or return instead of exiting directly.
# FIXME: This should throw or return instead of exiting directly.
sys.exit(1)
if not self._processor.should_process(file_path):
_log.debug("Skipping file: '%s'", file_path)
......@@ -119,7 +117,8 @@ class TextFileReader(object):
try:
lines = self._read_lines(file_path)
except IOError as err:
message = ("Could not read file. Skipping: '%s'\n %s" % (file_path, err))
message = (
"Could not read file. Skipping: '%s'\n %s" % (file_path, err))
_log.warning(message)
return
......
......@@ -20,7 +20,6 @@
# 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.
from blinkpy.common.system.filesystem import FileSystem
from blinkpy.common.system.log_testing import LoggingTestCase
from blinkpy.style.checker import ProcessorBase
......@@ -28,9 +27,7 @@ from blinkpy.style.filereader import TextFileReader
class TextFileReaderTest(LoggingTestCase):
class MockProcessor(ProcessorBase):
"""A processor for test purposes.
This processor simply records the parameters passed to its process()
......@@ -101,7 +98,9 @@ class TextFileReaderTest(LoggingTestCase):
# remain.
message = log_messages.pop()
self.assertTrue(message.startswith("WARNING: Could not read file. Skipping: '%s'\n " % temp_dir))
self.assertTrue(
message.startswith(
"WARNING: Could not read file. Skipping: '%s'\n " % temp_dir))
self._assert_file_reader([], 1)
......@@ -143,8 +142,7 @@ class TextFileReaderTest(LoggingTestCase):
file_path2 = self._create_file(rel_path, 'bar')
self._file_reader.process_paths([dir, file_path1])
processed = [(['bar'], file_path2, None),
(['foo'], file_path1, None)]
processed = [(['bar'], file_path2, None), (['foo'], file_path1, None)]
self._assert_file_reader(processed, 2)
def test_count_delete_only_file(self):
......
......@@ -19,7 +19,6 @@
# 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.
"""Contains filter-related code."""
......@@ -53,7 +52,6 @@ def validate_filter_rules(filter_rules, all_categories):
class _CategoryFilter(object):
"""Filters whether to check style categories."""
def __init__(self, filter_rules=None):
......@@ -74,7 +72,8 @@ class _CategoryFilter(object):
filter_rules = []
self._filter_rules = filter_rules
self._should_check_category = {} # Cached dictionary of category to True/False
# Cached dictionary of category to True/False
self._should_check_category = {}
def __str__(self):
return ','.join(self._filter_rules)
......@@ -115,7 +114,6 @@ class _CategoryFilter(object):
class FilterConfiguration(object):
"""Supports filtering with path-specific and user-specified rules."""
def __init__(self, base_rules=None, path_specific=None, user_rules=None):
......
......@@ -19,7 +19,6 @@
# 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 filter.py."""
import unittest
......@@ -51,7 +50,6 @@ from blinkpy.style.filter import FilterConfiguration
class ValidateFilterRulesTest(unittest.TestCase):
"""Tests validate_filter_rules() function."""
def test_validate_filter_rules(self):
......@@ -66,11 +64,7 @@ class ValidateFilterRulesTest(unittest.TestCase):
"+xxx",
]
good_rules = [
"+tabs",
"-tabs",
"+build"
]
good_rules = ["+tabs", "-tabs", "+build"]
for rule in bad_rules:
with self.assertRaises(ValueError):
......@@ -82,7 +76,6 @@ class ValidateFilterRulesTest(unittest.TestCase):
class CategoryFilterTest(unittest.TestCase):
"""Tests CategoryFilter class."""
def test_init(self):
......@@ -144,14 +137,14 @@ class CategoryFilterTest(unittest.TestCase):
class FilterConfigurationTest(unittest.TestCase):
"""Tests FilterConfiguration class."""
def _config(self, base_rules, path_specific, user_rules):
"""Return a FilterConfiguration instance."""
return FilterConfiguration(base_rules=base_rules,
path_specific=path_specific,
user_rules=user_rules)
return FilterConfiguration(
base_rules=base_rules,
path_specific=path_specific,
user_rules=user_rules)
def test_init(self):
"""Test __init__ method."""
......@@ -189,12 +182,12 @@ class FilterConfigurationTest(unittest.TestCase):
path_specific = [(["path"], ["+a"])]
user_rules = ["+"]
self.assertFalse(config.__eq__(FilterConfiguration(
base_rules=base_rules)))
self.assertFalse(config.__eq__(FilterConfiguration(
path_specific=path_specific)))
self.assertFalse(config.__eq__(FilterConfiguration(
user_rules=user_rules)))
self.assertFalse(
config.__eq__(FilterConfiguration(base_rules=base_rules)))
self.assertFalse(
config.__eq__(FilterConfiguration(path_specific=path_specific)))
self.assertFalse(
config.__eq__(FilterConfiguration(user_rules=user_rules)))
def test_ne(self):
"""Test __ne__ method."""
......@@ -220,8 +213,7 @@ 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)
......
......@@ -30,7 +30,6 @@ from blinkpy.style.checker import StyleProcessor
from blinkpy.style.filereader import TextFileReader
from blinkpy.style.patchreader import PatchReader
_log = logging.getLogger(__name__)
......@@ -86,8 +85,7 @@ def change_directory(filesystem, checkout_root, paths):
Pass only files below the checkout root to ensure correct results.
See the help documentation for more info.
""",
path, checkout_root)
""", path, checkout_root)
return paths
rel_paths.append(rel_path)
......@@ -101,14 +99,12 @@ def change_directory(filesystem, checkout_root, paths):
class CheckBlinkStyle(object):
def _engage_awesome_stderr_hacks(self):
# Change stderr to write with replacement characters so we don't die
# if we try to print something containing non-ASCII characters.
stderr = codecs.StreamReaderWriter(sys.stderr,
codecs.getreader('utf8'),
codecs.getwriter('utf8'),
'replace')
codecs.getwriter('utf8'), 'replace')
# Setting an "encoding" attribute on the stream is necessary to
# prevent the logging module from raising an error. See
# the checker.configure_logging() function for more information.
......@@ -140,7 +136,10 @@ class CheckBlinkStyle(object):
configuration = checker.check_blink_style_configuration(options)
paths = change_directory(host.filesystem, checkout_root=host.git().checkout_root, paths=paths)
paths = change_directory(
host.filesystem,
checkout_root=host.git().checkout_root,
paths=paths)
style_processor = StyleProcessor(configuration)
file_reader = TextFileReader(host.filesystem, style_processor)
......@@ -149,7 +148,8 @@ class CheckBlinkStyle(object):
file_reader.process_paths(paths)
else:
changed_files = paths if options.diff_files else None
patch = host.git().create_patch(options.git_commit, changed_files=changed_files)
patch = host.git().create_patch(
options.git_commit, changed_files=changed_files)
patch_checker = PatchReader(file_reader)
patch_checker.check(patch)
......@@ -157,6 +157,7 @@ class CheckBlinkStyle(object):
file_count = file_reader.file_count
delete_only_file_count = file_reader.delete_only_file_count
_log.info('Total errors found: %d in %d files', error_count, file_count)
_log.info('Total errors found: %d in %d files', error_count,
file_count)
# We fail when style errors are found.
return error_count > 0
......@@ -31,10 +31,13 @@ class ChangeDirectoryTest(LoggingTestCase):
def setUp(self):
super(ChangeDirectoryTest, self).setUp()
self.filesystem = MockFileSystem(dirs=[self._original_directory, self._checkout_root], cwd=self._original_directory)
self.filesystem = MockFileSystem(
dirs=[self._original_directory, self._checkout_root],
cwd=self._original_directory)
def _change_directory(self, paths, checkout_root):
return change_directory(self.filesystem, paths=paths, checkout_root=checkout_root)
return change_directory(
self.filesystem, paths=paths, checkout_root=checkout_root)
def _assert_result(self, actual_return_value, expected_return_value,
expected_log_messages, expected_current_directory):
......@@ -43,17 +46,21 @@ class ChangeDirectoryTest(LoggingTestCase):
self.assertEqual(self.filesystem.getcwd(), expected_current_directory)
def test_paths_none(self):
paths = self._change_directory(checkout_root=self._checkout_root, paths=None)
paths = self._change_directory(
checkout_root=self._checkout_root, paths=None)
self._assert_result(paths, None, [], self._checkout_root)
def test_paths_convertible(self):
paths = ['/chromium/src/foo1.txt', '/chromium/src/foo2.txt']
paths = self._change_directory(checkout_root=self._checkout_root, paths=paths)
self._assert_result(paths, ['foo1.txt', 'foo2.txt'], [], self._checkout_root)
paths = self._change_directory(
checkout_root=self._checkout_root, paths=paths)
self._assert_result(paths, ['foo1.txt', 'foo2.txt'], [],
self._checkout_root)
def test_with_git_paths_unconvertible(self):
paths = ['/chromium/src/foo1.txt', '/outside/foo2.txt']
paths = self._change_directory(checkout_root=self._checkout_root, paths=paths)
paths = self._change_directory(
checkout_root=self._checkout_root, paths=paths)
log_messages = [
"""WARNING: Path-dependent style checks may not work correctly:
......@@ -66,5 +73,7 @@ class ChangeDirectoryTest(LoggingTestCase):
Pass only files below the checkout root to ensure correct results.
See the help documentation for more info.
"""]
self._assert_result(paths, paths, log_messages, self._original_directory)
"""
]
self._assert_result(paths, paths, log_messages,
self._original_directory)
......@@ -19,7 +19,6 @@
# 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_blink_style.py."""
import logging
......@@ -102,7 +101,6 @@ _EPILOG = ('This script can miss errors and does not substitute for '
# This class should not have knowledge of the flag key names.
class DefaultCommandOptionValues(object):
"""Stores the default check_blink_style.py command-line options.
Attributes:
......@@ -117,7 +115,6 @@ class DefaultCommandOptionValues(object):
# 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:
......@@ -192,7 +189,6 @@ class CommandOptionValues(object):
class ArgumentPrinter(object):
"""Supports the printing of check_blink_style.py command arguments."""
def _flag_pair_to_string(self, flag_key, flag_value):
......@@ -282,55 +278,86 @@ class ArgumentParser(object):
self.default_options = default_options
self.stderr_write = stderr.write
self._parser = self._create_option_parser(stderr=stderr,
usage=usage,
default_min_confidence=self.default_options.min_confidence,
default_output_format=self.default_options.output_format)
self._parser = self._create_option_parser(
stderr=stderr,
usage=usage,
default_min_confidence=self.default_options.min_confidence,
default_output_format=self.default_options.output_format)
def _create_option_parser(self, stderr, usage,
default_min_confidence, default_output_format):
def _create_option_parser(self, stderr, usage, default_min_confidence,
default_output_format):
# Since the epilog string is short, it is not necessary to replace
# the epilog string with a mock epilog string when testing.
# For this reason, we use _EPILOG directly rather than passing it
# as an argument like we do for the usage string.
parser = OptionParser(usage=usage, epilog=_EPILOG)
filter_help = ('set a filter to control what categories of style '
'errors to report. Specify a filter using a comma-'
'delimited list of boolean filter rules, for example '
'"--filter -whitespace,+whitespace/braces". To display '
'all categories and which are enabled by default, pass '
"""no value (e.g. '-f ""' or '--filter=').""")
parser.add_option('-f', '--filter-rules', metavar='RULES',
dest='filter_value', help=filter_help)
git_commit_help = ('check all changes in the given commit. '
"Use 'commit_id..' to check all changes after commit_id")
parser.add_option('-g', '--git-diff', '--git-commit',
metavar='COMMIT', dest='git_commit', help=git_commit_help,)
filter_help = (
'set a filter to control what categories of style '
'errors to report. Specify a filter using a comma-'
'delimited list of boolean filter rules, for example '
'"--filter -whitespace,+whitespace/braces". To display '
'all categories and which are enabled by default, pass '
"""no value (e.g. '-f ""' or '--filter=').""")
parser.add_option(
'-f',
'--filter-rules',
metavar='RULES',
dest='filter_value',
help=filter_help)
git_commit_help = (
'check all changes in the given commit. '
"Use 'commit_id..' to check all changes after commit_id")
parser.add_option(
'-g',
'--git-diff',
'--git-commit',
metavar='COMMIT',
dest='git_commit',
help=git_commit_help,
)
diff_files_help = 'diff the files passed on the command line rather than checking the style of every line'
parser.add_option('--diff-files', action='store_true', dest='diff_files', default=False, help=diff_files_help)
parser.add_option(
'--diff-files',
action='store_true',
dest='diff_files',
default=False,
help=diff_files_help)
min_confidence_help = ('set the minimum confidence of style errors '
'to report. Can be an integer 1-5, with 1 '
'displaying all errors. Defaults to %default.')
parser.add_option('-m', '--min-confidence', metavar='INT',
type='int', dest='min_confidence',
default=default_min_confidence,
help=min_confidence_help)
parser.add_option(
'-m',
'--min-confidence',
metavar='INT',
type='int',
dest='min_confidence',
default=default_min_confidence,
help=min_confidence_help)
output_format_help = ('set the output format, which can be "emacs" '
'or "vs7" (for Visual Studio). '
'Defaults to "%default".')
parser.add_option('-o', '--output-format', metavar='FORMAT',
choices=['emacs', 'vs7'],
dest='output_format', default=default_output_format,
help=output_format_help)
parser.add_option(
'-o',
'--output-format',
metavar='FORMAT',
choices=['emacs', 'vs7'],
dest='output_format',
default=default_output_format,
help=output_format_help)
verbose_help = 'enable verbose logging.'
parser.add_option('-v', '--verbose', dest='is_verbose', default=False,
action='store_true', help=verbose_help)
parser.add_option(
'-v',
'--verbose',
dest='is_verbose',
default=False,
action='store_true',
help=verbose_help)
# Override OptionParser's error() method so that option help will
# also display when an error occurs. Normally, just the usage
......@@ -425,9 +452,9 @@ class ArgumentParser(object):
min_confidence = int(min_confidence)
if (min_confidence < 1) or (min_confidence > 5):
self._parse_error('option --min-confidence: invalid integer: '
'%s: value must be between 1 and 5'
% min_confidence)
self._parse_error(
'option --min-confidence: invalid integer: '
'%s: value must be between 1 and 5' % min_confidence)
if filter_value:
filter_rules = self._parse_filter_flag(filter_value)
......@@ -439,11 +466,12 @@ class ArgumentParser(object):
except ValueError as err:
self._parse_error(err)
options = CommandOptionValues(filter_rules=filter_rules,
git_commit=git_commit,
diff_files=diff_files,
is_verbose=is_verbose,
min_confidence=min_confidence,
output_format=output_format)
options = CommandOptionValues(
filter_rules=filter_rules,
git_commit=git_commit,
diff_files=diff_files,
is_verbose=is_verbose,
min_confidence=min_confidence,
output_format=output_format)
return (paths, options)
......@@ -19,7 +19,6 @@
# 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
......@@ -32,7 +31,6 @@ from blinkpy.style.optparser import DefaultCommandOptionValues
class ArgumentPrinterTest(unittest.TestCase):
"""Tests the ArgumentPrinter class."""
_printer = ArgumentPrinter()
......@@ -42,16 +40,18 @@ class ArgumentPrinterTest(unittest.TestCase):
min_confidence=3,
filter_rules=None,
git_commit=None):
return ProcessorOptions(filter_rules=filter_rules,
git_commit=git_commit,
min_confidence=min_confidence,
output_format=output_format)
return ProcessorOptions(
filter_rules=filter_rules,
git_commit=git_commit,
min_confidence=min_confidence,
output_format=output_format)
def test_to_flag_string(self):
options = self._create_options('vs7', 5, ['+foo', '-bar'], 'git')
self.assertEqual('--filter=+foo,-bar --git-commit=git '
'--min-confidence=5 --output=vs7',
self._printer.to_flag_string(options))
self.assertEqual(
'--filter=+foo,-bar --git-commit=git '
'--min-confidence=5 --output=vs7',
self._printer.to_flag_string(options))
# This is to check that --filter and --git-commit do not
# show up when not user-specified.
......@@ -61,11 +61,9 @@ class ArgumentPrinterTest(unittest.TestCase):
class ArgumentParserTest(LoggingTestCase):
"""Test the ArgumentParser class."""
class _MockStdErr(object):
def write(self, _):
# We do not want the usage string or style categories
# to print during unit tests, so print nothing.
......@@ -78,8 +76,8 @@ class ArgumentParserTest(LoggingTestCase):
def _create_defaults(self):
"""Return a DefaultCommandOptionValues instance for testing."""
return DefaultCommandOptionValues(min_confidence=3,
output_format='vs7')
return DefaultCommandOptionValues(
min_confidence=3, output_format='vs7')
def _create_parser(self):
"""Return an ArgumentParser instance for testing."""
......@@ -89,11 +87,12 @@ class ArgumentParserTest(LoggingTestCase):
mock_stderr = self._MockStdErr()
return ArgumentParser(all_categories=all_categories,
base_filter_rules=[],
default_options=default_options,
mock_stderr=mock_stderr,
usage='test usage')
return ArgumentParser(
all_categories=all_categories,
base_filter_rules=[],
default_options=default_options,
mock_stderr=mock_stderr,
usage='test usage')
def test_parse_documentation(self):
parse = self._parse
......@@ -118,30 +117,40 @@ class ArgumentParserTest(LoggingTestCase):
with self.assertRaises(SystemExit):
parse(['--min-confidence=bad'])
self.assertLog(['ERROR: option --min-confidence: '
"invalid integer value: 'bad'\n"])
self.assertLog([
'ERROR: option --min-confidence: '
"invalid integer value: 'bad'\n"
])
with self.assertRaises(SystemExit):
parse(['--min-confidence=0'])
self.assertLog(['ERROR: option --min-confidence: invalid integer: 0: '
'value must be between 1 and 5\n'])
self.assertLog([
'ERROR: option --min-confidence: invalid integer: 0: '
'value must be between 1 and 5\n'
])
with self.assertRaises(SystemExit):
parse(['--min-confidence=6'])
self.assertLog(['ERROR: option --min-confidence: invalid integer: 6: '
'value must be between 1 and 5\n'])
self.assertLog([
'ERROR: option --min-confidence: invalid integer: 6: '
'value must be between 1 and 5\n'
])
parse(['--min-confidence=1']) # works
parse(['--min-confidence=5']) # works
with self.assertRaises(SystemExit):
parse(['--output=bad'])
self.assertLog(['ERROR: option --output-format: invalid choice: '
"'bad' (choose from 'emacs', 'vs7')\n"])
self.assertLog([
'ERROR: option --output-format: invalid choice: '
"'bad' (choose from 'emacs', 'vs7')\n"
])
parse(['--output=vs7']) # works
# Pass a filter rule not beginning with + or -.
with self.assertRaises(SystemExit):
parse(['--filter=build'])
self.assertLog(['ERROR: Invalid filter rule "build": '
'every rule must start with + or -.\n'])
self.assertLog([
'ERROR: Invalid filter rule "build": '
'every rule must start with + or -.\n'
])
parse(['--filter=+build']) # works
def test_parse_default_arguments(self):
......@@ -179,13 +188,11 @@ class ArgumentParserTest(LoggingTestCase):
# Pass user_rules.
_, options = parse(['--filter=+build,-whitespace'])
self.assertEqual(options.filter_rules,
['+build', '-whitespace'])
self.assertEqual(options.filter_rules, ['+build', '-whitespace'])
# Pass spurious white space in user rules.
_, options = parse(['--filter=+build, -whitespace'])
self.assertEqual(options.filter_rules,
['+build', '-whitespace'])
self.assertEqual(options.filter_rules, ['+build', '-whitespace'])
def test_parse_files(self):
parse = self._parse
......@@ -199,7 +206,6 @@ class ArgumentParserTest(LoggingTestCase):
class CommandOptionValuesTest(unittest.TestCase):
"""Tests CommandOptionValues class."""
def test_init(self):
......@@ -222,11 +228,12 @@ class CommandOptionValuesTest(unittest.TestCase):
ProcessorOptions(min_confidence=5) # works
# Check attributes.
options = ProcessorOptions(filter_rules=['+'],
git_commit='commit',
is_verbose=True,
min_confidence=3,
output_format='vs7')
options = ProcessorOptions(
filter_rules=['+'],
git_commit='commit',
is_verbose=True,
min_confidence=3,
output_format='vs7')
self.assertEqual(options.filter_rules, ['+'])
self.assertEqual(options.git_commit, 'commit')
self.assertTrue(options.is_verbose)
......@@ -242,11 +249,12 @@ class CommandOptionValuesTest(unittest.TestCase):
# Explicitly create a ProcessorOptions instance with all default
# values. We do this to be sure we are assuming the right default
# values in our self.assertFalse() calls below.
options = ProcessorOptions(filter_rules=[],
git_commit=None,
is_verbose=False,
min_confidence=1,
output_format='emacs')
options = ProcessorOptions(
filter_rules=[],
git_commit=None,
is_verbose=False,
min_confidence=1,
output_format='emacs')
# Verify that we created options correctly.
self.assertTrue(options.__eq__(ProcessorOptions()))
......
......@@ -32,7 +32,6 @@ import logging
from blinkpy.common.checkout.diff_parser import DiffParser
_log = logging.getLogger(__name__)
......@@ -53,7 +52,8 @@ class PatchReader(object):
for path, diff_file in patch_files.iteritems():
line_numbers = diff_file.added_or_modified_line_numbers()
_log.debug('Found %s new or modified lines in: %s', len(line_numbers), path)
_log.debug('Found %s new or modified lines in: %s',
len(line_numbers), path)
if not line_numbers:
# Don't check files which contain only deleted lines
......@@ -62,4 +62,5 @@ class PatchReader(object):
self._text_file_reader.count_delete_only_file()
continue
self._text_file_reader.process_file(file_path=path, line_numbers=line_numbers)
self._text_file_reader.process_file(
file_path=path, line_numbers=line_numbers)
......@@ -35,11 +35,10 @@ from blinkpy.style.patchreader import PatchReader
class PatchReaderTest(unittest.TestCase):
class MockTextFileReader(object):
def __init__(self):
self.passed_to_process_file = [] # A list of (file_path, line_numbers) pairs.
# A list of (file_path, line_numbers) pairs.
self.passed_to_process_file = []
self.delete_only_file_count = 0 # A number of times count_delete_only_file() called.
def process_file(self, file_path, line_numbers):
......@@ -52,8 +51,10 @@ class PatchReaderTest(unittest.TestCase):
self._file_reader = self.MockTextFileReader()
def _assert_checked(self, passed_to_process_file, delete_only_file_count):
self.assertEqual(self._file_reader.passed_to_process_file, passed_to_process_file)
self.assertEqual(self._file_reader.delete_only_file_count, delete_only_file_count)
self.assertEqual(self._file_reader.passed_to_process_file,
passed_to_process_file)
self.assertEqual(self._file_reader.delete_only_file_count,
delete_only_file_count)
def test_check_patch(self):
PatchReader(self._file_reader).check(
......@@ -78,7 +79,8 @@ class PatchReaderTest(unittest.TestCase):
'@@ -1 +0,0 @@\n'
'-foobar\n')
# The deleted file isn't be processed.
self._assert_checked(passed_to_process_file=[], delete_only_file_count=1)
self._assert_checked(
passed_to_process_file=[], delete_only_file_count=1)
def test_check_patch_with_png_deletion(self):
PatchReader(self._file_reader).check(
......@@ -86,4 +88,5 @@ class PatchReaderTest(unittest.TestCase):
'deleted file mode 100644\n'
'index ef65bee..0000000\n'
'Binary files a/foo-expected.png and /dev/null differ\n')
self._assert_checked(passed_to_process_file=[], delete_only_file_count=1)
self._assert_checked(
passed_to_process_file=[], delete_only_file_count=1)
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment