Commit 9a748a3a authored by Dan Beam's avatar Dan Beam Committed by Commit Bot

Mojo/web_dev_style: add a presubmit discouraging mojo_bindings.js

mojo_bindings_lite.js should be used in new (and eventually current)
code.

R=dpapad@chromium.org
BUG=931798

Change-Id: I6499a02c848412d7a2ba4c3d84fcc202e2813bf3
Reviewed-on: https://chromium-review.googlesource.com/c/1474517
Commit-Queue: Dan Beam <dbeam@chromium.org>
Reviewed-by: default avatarDemetrios Papadopoulos <dpapad@chromium.org>
Auto-Submit: Dan Beam <dbeam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#634547}
parent 2b5fe283
...@@ -15,9 +15,13 @@ class ResourceChecker(object): ...@@ -15,9 +15,13 @@ class ResourceChecker(object):
self.output_api = output_api self.output_api = output_api
self.file_filter = file_filter self.file_filter = file_filter
def DeprecatedMojoBindingsCheck(self, line_number, line):
return regex_check.RegexCheck(self.input_api.re, line_number, line,
'(mojo_bindings\.js)', 'Please use mojo_bindings_lite.js in new code')
def DisallowIncludeCheck(self, msg, line_number, line): def DisallowIncludeCheck(self, msg, line_number, line):
return regex_check.RegexCheck(self.input_api.re, line_number, line, return regex_check.RegexCheck(self.input_api.re, line_number, line,
'^\s*(?:\/[\*\/])?\s*(<include)\s*src=', msg) '^\s*(?:\/[\*\/])?\s*(<include)\s*src=', msg)
# This is intentionally not included in RunChecks(). It's an optional check # This is intentionally not included in RunChecks(). It's an optional check
# that can be used from a PRESUBMIT.py in a directory that does not wish to # that can be used from a PRESUBMIT.py in a directory that does not wish to
...@@ -30,13 +34,17 @@ class ResourceChecker(object): ...@@ -30,13 +34,17 @@ class ResourceChecker(object):
def SelfClosingIncludeCheck(self, line_number, line): def SelfClosingIncludeCheck(self, line_number, line):
return regex_check.RegexCheck(self.input_api.re, line_number, line, return regex_check.RegexCheck(self.input_api.re, line_number, line,
"(</include>|<include.*/>)", "Closing <include> tags is unnecessary.") '(</include>|<include.*/>)', 'Closing <include> tags is unnecessary.')
def RunChecks(self): def RunChecks(self):
return self._RunCheckOnAffectedFiles( msg = 'Found resources style issues in %s'
self.SelfClosingIncludeCheck, 'Found resources style issues in %s') # TODO(crbug.com/931798): is_error for Mojo check when -lite is majority?
return self._RunCheckOnAffectedFiles(self.DeprecatedMojoBindingsCheck,
msg, only_changed_lines=True) + \
self._RunCheckOnAffectedFiles(self.SelfClosingIncludeCheck, msg)
def _RunCheckOnAffectedFiles(self, check, msg_template, is_error=False): def _RunCheckOnAffectedFiles(self, check, msg_template, is_error=False,
only_changed_lines=False):
"""Check for violations of the Chromium web development style guide. See """Check for violations of the Chromium web development style guide. See
https://chromium.googlesource.com/chromium/src/+/master/styleguide/web/web.md https://chromium.googlesource.com/chromium/src/+/master/styleguide/web/web.md
""" """
...@@ -46,8 +54,11 @@ class ResourceChecker(object): ...@@ -46,8 +54,11 @@ class ResourceChecker(object):
include_deletes=False) include_deletes=False)
for f in affected_files: for f in affected_files:
errors = [] errors = []
if only_changed_lines:
for line_number, line in enumerate(f.NewContents(), start=1): contents = f.ChangedContents()
else:
contents = enumerate(f.NewContents(), start=1)
for line_number, line in contents:
error = check(line_number, line) error = check(line_number, line)
if error: if error:
errors.append(error) errors.append(error)
......
...@@ -26,40 +26,30 @@ class ResourceCheckerTest(SuperMoxTestBase): ...@@ -26,40 +26,30 @@ class ResourceCheckerTest(SuperMoxTestBase):
output_api = self.mox.CreateMockAnything() output_api = self.mox.CreateMockAnything()
self.checker = resource_checker.ResourceChecker(input_api, output_api) self.checker = resource_checker.ResourceChecker(input_api, output_api)
def ShouldFailSelfClosingIncludeCheck(self, line): def ShouldPassDeprecatedMojoBindingCheck(self, line):
"""Checks that the '</include>' checker flags |line| as a style error.""" error = self.checker.DeprecatedMojoBindingsCheck(1, line)
error = self.checker.SelfClosingIncludeCheck(1, line) self.assertEqual('', error, 'Should not be flagged as error: ' + line)
self.assertNotEqual('', error,
'Should be flagged as style error: ' + line)
highlight = test_util.GetHighlight(line, error).strip()
self.assertTrue('include' in highlight and highlight[0] == '<')
def ShouldPassSelfClosingIncludeCheck(self, line): def ShouldFailDeprecatedMojoBindingCheck(self, line):
"""Checks that the '</include>' checker doesn't flag |line| as an error.""" error = self.checker.DeprecatedMojoBindingsCheck(1, line)
self.assertEqual('', self.checker.SelfClosingIncludeCheck(1, line), self.assertNotEqual('', error, 'Should be flagged as error: ' + line)
'Should not be flagged as style error: ' + line) self.assertEquals('mojo_bindings.js', test_util.GetHighlight(line, error))
def testIncludeFails(self): def testDeprecatedMojoBindingsCheckPasses(self):
lines = [ lines = [
"</include> ", '<script src="chrome://resources/js/mojo_bindings_lite.js">',
" </include>", "script.src = 'chrome://resources/js/mojo_bindings_lite.js';",
" </include> ",
' <include src="blah.js" /> ',
'<include src="blee.js"/>',
] ]
for line in lines: for line in lines:
self.ShouldFailSelfClosingIncludeCheck(line) self.ShouldPassDeprecatedMojoBindingCheck(line)
def testIncludePasses(self): def testDeprecatedMojoBindingsCheckFails(self):
lines = [ lines = [
'<include src="assert.js">', '<script src="chrome://resources/js/mojo_bindings.js">',
"<include src='../../assert.js'>", "script.src = 'chrome://resources/js/mojo_bindings.js';",
"<i>include src='blah'</i>",
"</i>nclude",
"</i>include",
] ]
for line in lines: for line in lines:
self.ShouldPassSelfClosingIncludeCheck(line) self.ShouldFailDeprecatedMojoBindingCheck(line)
def ShouldPassDisallowIncludeCheck(self, line): def ShouldPassDisallowIncludeCheck(self, line):
self.assertEqual('', self.checker.DisallowIncludeCheck('msg', 1, line), self.assertEqual('', self.checker.DisallowIncludeCheck('msg', 1, line),
...@@ -87,6 +77,41 @@ class ResourceCheckerTest(SuperMoxTestBase): ...@@ -87,6 +77,41 @@ class ResourceCheckerTest(SuperMoxTestBase):
for line in lines: for line in lines:
self.ShouldPassDisallowIncludeCheck(line) self.ShouldPassDisallowIncludeCheck(line)
def ShouldFailSelfClosingIncludeCheck(self, line):
"""Checks that the '</include>' checker flags |line| as a style error."""
error = self.checker.SelfClosingIncludeCheck(1, line)
self.assertNotEqual('', error,
'Should be flagged as style error: ' + line)
highlight = test_util.GetHighlight(line, error).strip()
self.assertTrue('include' in highlight and highlight[0] == '<')
def ShouldPassSelfClosingIncludeCheck(self, line):
"""Checks that the '</include>' checker doesn't flag |line| as an error."""
self.assertEqual('', self.checker.SelfClosingIncludeCheck(1, line),
'Should not be flagged as style error: ' + line)
def testIncludeFails(self):
lines = [
"</include> ",
" </include>",
" </include> ",
' <include src="blah.js" /> ',
'<include src="blee.js"/>',
]
for line in lines:
self.ShouldFailSelfClosingIncludeCheck(line)
def testIncludePasses(self):
lines = [
'<include src="assert.js">',
"<include src='../../assert.js'>",
"<i>include src='blah'</i>",
"</i>nclude",
"</i>include",
]
for line in lines:
self.ShouldPassSelfClosingIncludeCheck(line)
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