Commit e0d46d4d authored by Miguel Casas-Sanchez's avatar Miguel Casas-Sanchez Committed by Commit Bot

PRESUBMIT: add a warning for crbug links w/o https://

Our code search doesn't linkify crbug[.com] if they are
not prefixed with http[s]:// (e.g. [1]). This CL adds a
warning on upload that this is happening and suggests
prefixing https://, since it seems like we are not going
to be able to linkify them automatically in CS anytime
soon (see bug).

It looks a bit like:

** Presubmit Warnings **
Found unprefixed crbug.com URL(s), consider prepending https://
    ui/ozone/platform/drm/gpu/gbm_buffer.h:107 // TODO(mcasas): crbug.com
    ui/ozone/platform/drm/gpu/gbm_buffer.h:108 // TODO(mcasas): crbug/123

[1] https://cs.chromium.org/chromium/src/BUILD.gn?type=cs&q=crbug.com&sq=package:chromium&l=65

Bug: 762061
Change-Id: Ib58bc6b58dfa61a7bf421b0ed55184705cee767c
Reviewed-on: https://chromium-review.googlesource.com/822973Reviewed-by: default avatarJochen Eisinger <jochen@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524073}
parent c710b59f
...@@ -2759,6 +2759,27 @@ def _CheckSyslogUseWarning(input_api, output_api, source_file_filter=None, ...@@ -2759,6 +2759,27 @@ def _CheckSyslogUseWarning(input_api, output_api, source_file_filter=None,
return [] return []
def _CheckCrbugLinksHaveHttps(input_api, output_api):
"""Checks that crbug(.com) links are correctly prefixed by https://"""
white_list = r'.+%s' % _IMPLEMENTATION_EXTENSIONS
black_list = (_EXCLUDED_PATHS + _TEST_CODE_EXCLUDED_PATHS)
sources = lambda f: input_api.FilterSourceFile(
f, white_list=white_list, black_list=black_list)
pattern = input_api.re.compile(r'//.*(?<!:\/\/)crbug[.com]*')
problems = []
for f in input_api.AffectedSourceFiles(sources):
for line_num, line in f.ChangedContents():
if pattern.search(line):
problems.append(' %s:%d %s' % (f.LocalPath(), line_num, line))
if problems:
return [output_api.PresubmitPromptWarning(
'Found unprefixed crbug.com URL(s), consider prepending https://\n'+
'\n'.join(problems))]
return []
def CheckChangeOnUpload(input_api, output_api): def CheckChangeOnUpload(input_api, output_api):
results = [] results = []
results.extend(_CommonChecks(input_api, output_api)) results.extend(_CommonChecks(input_api, output_api))
...@@ -2769,6 +2790,7 @@ def CheckChangeOnUpload(input_api, output_api): ...@@ -2769,6 +2790,7 @@ def CheckChangeOnUpload(input_api, output_api):
results.extend(_AndroidSpecificOnUploadChecks(input_api, output_api)) results.extend(_AndroidSpecificOnUploadChecks(input_api, output_api))
results.extend(_CheckSyslogUseWarning(input_api, output_api)) results.extend(_CheckSyslogUseWarning(input_api, output_api))
results.extend(_CheckGoogleSupportAnswerUrl(input_api, output_api)) results.extend(_CheckGoogleSupportAnswerUrl(input_api, output_api))
results.extend(_CheckCrbugLinksHaveHttps(input_api, output_api))
return results return results
......
...@@ -1288,5 +1288,22 @@ class MojoManifestOwnerTest(unittest.TestCase): ...@@ -1288,5 +1288,22 @@ class MojoManifestOwnerTest(unittest.TestCase):
self.assertEqual([], errors) self.assertEqual([], errors)
class CrbugUrlFormatTest(unittest.TestCase):
def testCheckCrbugLinksHaveHttps(self):
input_api = MockInputApi()
input_api.files = [
MockFile('somewhere/file.cc',
['// TODO(developer): crbug.com should be linkified',
'// TODO(developer): (crbug.com) should be linkified',
'// TODO(developer): crbug/123 should be well formed',
'// TODO(developer): http://crbug.com it\'s OK',
'// TODO(developer): https://crbug.com is just great']),
]
warnings = PRESUBMIT._CheckCrbugLinksHaveHttps(input_api, MockOutputApi())
self.assertEqual(1, len(warnings))
if __name__ == '__main__': if __name__ == '__main__':
unittest.main() unittest.main()
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