Commit ba0f172b authored by Miyoung Shin's avatar Miyoung Shin Committed by Commit Bot

Add PRESUBMIT warning messange for blink's mojom namespace rule in Blink

This CL is a follow-up CL of https://crrev.com/c/2010448 and adds
the warning message for blink's mojom namespace rule in Blink.

Bug: 1043557
Change-Id: I1839ab7cf235cbff0ef3d3c7321c40c2f0adc71d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2014217Reviewed-by: default avatarKinuko Yasuda <kinuko@chromium.org>
Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Commit-Queue: Miyoung Shin <myid.shin@igalia.com>
Cr-Commit-Position: refs/heads/master@{#734386}
parent eed55b3a
...@@ -178,7 +178,10 @@ def _CheckForForbiddenChromiumCode(input_api, output_api): ...@@ -178,7 +178,10 @@ def _CheckForForbiddenChromiumCode(input_api, output_api):
path, error.line, error.identifier) path, error.line, error.identifier)
if error.advice: if error.advice:
msg += ". Advice: %s" % "\n".join(error.advice) msg += ". Advice: %s" % "\n".join(error.advice)
results.append(output_api.PresubmitError(msg)) if error.warning:
results.append(output_api.PresubmitPromptWarning(msg))
else:
results.append(output_api.PresubmitError(msg))
return results return results
......
...@@ -433,7 +433,9 @@ _CONFIG = [ ...@@ -433,7 +433,9 @@ _CONFIG = [
'Use WTF containers like WTF::Deque, WTF::HashMap, WTF::HashSet or WTF::Vector instead of the banned std containers. ' 'Use WTF containers like WTF::Deque, WTF::HashMap, WTF::HashSet or WTF::Vector instead of the banned std containers. '
'However, it is fine to use std containers at the boundary layer between Blink and Chromium. ' 'However, it is fine to use std containers at the boundary layer between Blink and Chromium. '
'If you are in this case, you can use --bypass-hooks option to avoid the presubmit check when uploading your CL.'), 'If you are in this case, you can use --bypass-hooks option to avoid the presubmit check when uploading your CL.'),
'(blink::)?mojom::(?!blink).+', ('([a-zA-Z]*::)?mojom::(?!blink).+',
'Using non-blink mojom types, consider using "::mojom::blink::Foo" instead of "::mojom::Foo" unless you have clear reasons not to do so',
'Warning'),
], ],
}, },
{ {
...@@ -989,13 +991,15 @@ def _precompile_config(): ...@@ -989,13 +991,15 @@ def _precompile_config():
"""Turns the raw config into a config of compiled regex.""" """Turns the raw config into a config of compiled regex."""
match_nothing_re = re.compile('.^') match_nothing_re = re.compile('.^')
def compile_regexp(match_list): def compile_regexp(match_list, is_list=True):
"""Turns a match list into a compiled regexp. """Turns a match list into a compiled regexp.
If match_list is None, a regexp that matches nothing is returned. If match_list is None, a regexp that matches nothing is returned.
""" """
if (match_list and is_list):
match_list = '(?:%s)$' % '|'.join(match_list)
if match_list: if match_list:
return re.compile('(?:%s)$' % '|'.join(match_list)) return re.compile(match_list)
return match_nothing_re return match_nothing_re
def compile_disallowed(disallowed_list): def compile_disallowed(disallowed_list):
...@@ -1006,9 +1010,13 @@ def _precompile_config(): ...@@ -1006,9 +1010,13 @@ def _precompile_config():
advice_list = [] advice_list = []
for entry in disallowed_list: for entry in disallowed_list:
if isinstance(entry, tuple): if isinstance(entry, tuple):
match, advice = entry warning = ''
if len(entry) == 2:
match, advice = entry
else:
match, advice, warning = entry
match_list.append(match) match_list.append(match)
advice_list.append((compile_regexp(match), advice)) advice_list.append((compile_regexp(match, False), advice, warning == 'Warning'))
else: else:
# Just a string # Just a string
match_list.append(entry) match_list.append(entry)
...@@ -1075,18 +1083,19 @@ def _check_entries_for_identifier(entries, identifier): ...@@ -1075,18 +1083,19 @@ def _check_entries_for_identifier(entries, identifier):
def _find_advice_for_identifier(entries, identifier): def _find_advice_for_identifier(entries, identifier):
advice_list = [] advice_list = []
for entry in entries: for entry in entries:
for matcher, advice in entry.get('advice', []): for matcher, advice, warning in entry.get('advice', []):
if matcher.match(identifier): if matcher.match(identifier):
advice_list.append(advice) advice_list.append(advice)
return advice_list return advice_list, warning
class BadIdentifier(object): class BadIdentifier(object):
"""Represents a single instance of a bad identifier.""" """Represents a single instance of a bad identifier."""
def __init__(self, identifier, line, advice=None): def __init__(self, identifier, line, advice=None, warning=False):
self.identifier = identifier self.identifier = identifier
self.line = line self.line = line
self.advice = advice self.advice = advice
self.warning = warning
def check(path, contents): def check(path, contents):
...@@ -1125,9 +1134,9 @@ def check(path, contents): ...@@ -1125,9 +1134,9 @@ def check(path, contents):
if match: if match:
identifier = match.group(0) identifier = match.group(0)
if not _check_entries_for_identifier(entries, identifier): if not _check_entries_for_identifier(entries, identifier):
advice = _find_advice_for_identifier(entries, identifier) advice, warning = _find_advice_for_identifier(entries, identifier)
results.append( results.append(
BadIdentifier(identifier, line_number, advice)) BadIdentifier(identifier, line_number, advice, warning))
return results return results
......
...@@ -23,7 +23,7 @@ class TestAuditNonBlinkUsageTest(unittest.TestCase): ...@@ -23,7 +23,7 @@ class TestAuditNonBlinkUsageTest(unittest.TestCase):
self.assertIsInstance(entry['allowed'], self._REGEXP_CLASS) self.assertIsInstance(entry['allowed'], self._REGEXP_CLASS)
if 'disallowed' in entry: if 'disallowed' in entry:
self.assertIsInstance(entry['disallowed'], self._REGEXP_CLASS) self.assertIsInstance(entry['disallowed'], self._REGEXP_CLASS)
for match, advice in entry.get('advice', []): for match, advice, warning in entry.get('advice', []):
self.assertIsInstance(match, self._REGEXP_CLASS) self.assertIsInstance(match, self._REGEXP_CLASS)
self.assertIsInstance(advice, str) self.assertIsInstance(advice, str)
......
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