Skip to content

Commit

Permalink
Adding Presubmit error when OVERRIDE and FINAL is not used as C++11 s…
Browse files Browse the repository at this point in the history
…tandard

Alerting the error for use to use c++11 constructs |override| and |final|
instead of OVERRIDE and FINAL respctively

BUG=417463

Review URL: https://codereview.chromium.org/653883002

Cr-Commit-Position: refs/heads/master@{#299880}
  • Loading branch information
mohan-486 authored and Commit bot committed Oct 16, 2014
1 parent 88e2318 commit f21db96
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 11 deletions.
39 changes: 28 additions & 11 deletions PRESUBMIT.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,6 @@
"""


import re
import sys


_EXCLUDED_PATHS = (
r"^breakpad[\\\/].*",
r"^native_client_sdk[\\\/]src[\\\/]build_tools[\\\/]make_rules.py",
Expand Down Expand Up @@ -517,6 +513,7 @@ def _CheckUnwantedDependencies(input_api, output_api):
change. Breaking - rules is an error, breaking ! rules is a
warning.
"""
import sys
# We need to wait until we have an input_api object and use this
# roundabout construct to import checkdeps because this file is
# eval-ed and thus doesn't have __file__.
Expand Down Expand Up @@ -569,8 +566,8 @@ def _CheckFilePermissions(input_api, output_api):
"""Check that all files have their permissions properly set."""
if input_api.platform == 'win32':
return []
args = [sys.executable, 'tools/checkperms/checkperms.py', '--root',
input_api.change.RepositoryRoot()]
args = [input_api.python_executable, 'tools/checkperms/checkperms.py',
'--root', input_api.change.RepositoryRoot()]
for f in input_api.AffectedFiles():
args += ['--file', f.LocalPath()]
checkperms = input_api.subprocess.Popen(args,
Expand Down Expand Up @@ -969,14 +966,14 @@ def _CheckSpamLogging(input_api, output_api):

for f in input_api.AffectedSourceFiles(source_file_filter):
contents = input_api.ReadFile(f, 'rb')
if re.search(r"\bD?LOG\s*\(\s*INFO\s*\)", contents):
if input_api.re.search(r"\bD?LOG\s*\(\s*INFO\s*\)", contents):
log_info.append(f.LocalPath())
elif re.search(r"\bD?LOG_IF\s*\(\s*INFO\s*,", contents):
elif input_api.re.search(r"\bD?LOG_IF\s*\(\s*INFO\s*,", contents):
log_info.append(f.LocalPath())

if re.search(r"\bprintf\(", contents):
if input_api.re.search(r"\bprintf\(", contents):
printf.append(f.LocalPath())
elif re.search(r"\bfprintf\((stdout|stderr)", contents):
elif input_api.re.search(r"\bfprintf\((stdout|stderr)", contents):
printf.append(f.LocalPath())

if log_info:
Expand Down Expand Up @@ -1198,6 +1195,7 @@ def FilterFile(affected_file):

def _CheckJavaStyle(input_api, output_api):
"""Runs checkstyle on changed java files and returns errors if any exist."""
import sys
original_sys_path = sys.path
try:
sys.path = sys.path + [input_api.os_path.join(
Expand Down Expand Up @@ -1259,6 +1257,23 @@ def _CheckNoDeprecatedCSS(input_api, output_api):
(fpath.LocalPath(), line_num, deprecated_value, value)))
return results


def _CheckForOverrideAndFinalRules(input_api, output_api):
"""Checks for final and override used as per C++11"""
problems = []
for f in input_api.AffectedFiles():
if (f.LocalPath().endswith(('.cc', '.cpp', '.h', '.mm'))):
for line_num, line in f.ChangedContents():
if (input_api.re.search(r'\b(FINAL|OVERRIDE)\b', line)):
problems.append(' %s:%d' % (f.LocalPath(), line_num))

if not problems:
return []
return [output_api.PresubmitError('Use C++11\'s |final| and |override| '
'rather than FINAL and OVERRIDE.',
problems)]


def _CommonChecks(input_api, output_api):
"""Checks common to both upload and commit."""
results = []
Expand Down Expand Up @@ -1300,6 +1315,7 @@ def _CommonChecks(input_api, output_api):
results.extend(_CheckNoDeprecatedCSS(input_api, output_api))
results.extend(_CheckParseErrors(input_api, output_api))
results.extend(_CheckForIPCRules(input_api, output_api))
results.extend(_CheckForOverrideAndFinalRules(input_api, output_api))

if any('PRESUBMIT.py' == f.LocalPath() for f in input_api.AffectedFiles()):
results.extend(input_api.canned_checks.RunUnitTestsInDirectory(
Expand Down Expand Up @@ -1453,7 +1469,7 @@ def _CheckForUsingSideEffectsOfPass(input_api, output_api):
if f.LocalPath().endswith(('.h', '.c', '.cc', '.m', '.mm')):
for lnum, line in f.ChangedContents():
# Disallow Foo(*my_scoped_thing.Pass()); See crbug.com/418297.
if re.search(r'\*[a-zA-Z0-9_]+\.Pass\(\)', line):
if input_api.re.search(r'\*[a-zA-Z0-9_]+\.Pass\(\)', line):
errors.append(output_api.PresubmitError(
('%s:%d uses *foo.Pass() to delete the contents of scoped_ptr. ' +
'See crbug.com/418297.') % (f.LocalPath(), lnum)))
Expand Down Expand Up @@ -1647,6 +1663,7 @@ def CheckChangeOnCommit(input_api, output_api):


def GetPreferredTryMasters(project, change):
import re
files = change.LocalPaths()

if not files or all(re.search(r'[\\\/]OWNERS$', f) for f in files):
Expand Down
60 changes: 60 additions & 0 deletions PRESUBMIT_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,66 @@ def testValidOSMacroNames(self):
self.assertEqual(0, len(errors))


class InvalidOverideAndFinalTest(unittest.TestCase):
def testValidOverrideConstructs(self):
mock_input_api = MockInputApi()
lines = ['foo1() override;',
'foo2() final;',
'#DEFINE OVERRIDE_METHOD_OVERLOAD',
'#DEFINE FINAL_METHOD',
'#DEFINE OVERRIDE_OVERRIDE_SOMETHING',
'#DEFINE SOMETHING_OVERRIDE_SOMETHING',
'#DEFINE SOMETHING_SOMETHING_OVERRIDE',
'#DEFINE FINAL_OVERRIDE_FINAL',
'#DEFINE SOMETHING_OVERRIDE_FINAL',
'#DEFINE OVERRIDE_OVERRIDE_OVERRIDE',
'#endif // FINAL_METHOD',
'#endif // OVERRIDE_METHOD_OVERLOAD']
mock_file_h = MockFile('something.h', lines)
mock_input_api.files = [mock_file_h]
errors = PRESUBMIT._CheckForOverrideAndFinalRules(mock_input_api,
MockOutputApi())
self.assertEqual(0, len(errors))

def testInvalidOverrideConstructsInHeaders(self):
mock_input_api = MockInputApi()
lines_cpp = ['foo2() FINAL;']
lines_h = ['foo1() OVERRIDE;']
mock_file_cpp = MockFile('something.cpp', lines_cpp)
mock_file_h = MockFile('something.h', lines_h)
mock_input_api.files = [mock_file_cpp, mock_file_h]
errors = PRESUBMIT._CheckForOverrideAndFinalRules(mock_input_api,
MockOutputApi())
self.assertEqual(1, len(errors))

def testInvalidOverrideConstructsInCpp(self):
mock_input_api = MockInputApi()
lines_cpp = ['foo2() FINAL;']
mock_file_cpp = MockFile('something.cpp', lines_cpp)
mock_input_api.files = [mock_file_cpp]
errors = PRESUBMIT._CheckForOverrideAndFinalRules(mock_input_api,
MockOutputApi())
self.assertEqual(1, len(errors))

def testInvalidOverrideConstructsInCc(self):
mock_input_api = MockInputApi()
lines_cc = ['foo3() override FINAL;']
mock_file_cc = MockFile('something.cc', lines_cc)
mock_input_api.files = [mock_file_cc]
errors = PRESUBMIT._CheckForOverrideAndFinalRules(mock_input_api,
MockOutputApi())
self.assertEqual(1, len(errors))

def testInvalidOverrideConstructsInMm(self):
mock_input_api = MockInputApi()
lines_mm = ['foo4() OVERRIDE final;']
mock_file_mm = MockFile('something.mm', lines_mm)
mock_input_api.files = [mock_file_mm]
errors = PRESUBMIT._CheckForOverrideAndFinalRules(mock_input_api,
MockOutputApi())
self.assertEqual(1, len(errors))


class InvalidIfDefinedMacroNamesTest(unittest.TestCase):
def testInvalidIfDefinedMacroNames(self):
lines = ['#if defined(TARGET_IPHONE_SIMULATOR)',
Expand Down

0 comments on commit f21db96

Please sign in to comment.