Skip to content

Commit

Permalink
ES6 Style: add presubmit prompt about => in code that might run on iOS9
Browse files Browse the repository at this point in the history
An action item from this ES6 proposal:
https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/iJrC4PVSfoU

R=dpranke@chromium.org
BUG=671426

Review-Url: https://codereview.chromium.org/2576253002
Cr-Commit-Position: refs/heads/master@{#438754}
  • Loading branch information
danbeam authored and Commit bot committed Dec 15, 2016
1 parent d55955b commit 1ec68ac
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 10 deletions.
34 changes: 28 additions & 6 deletions PRESUBMIT.py
Original file line number Diff line number Diff line change
Expand Up @@ -1925,7 +1925,7 @@ def FileFilter(affected_file):
return []


def _CheckNoDeprecatedCompiledResourcesGYP(input_api, output_api):
def _CheckNoDeprecatedCompiledResourcesGyp(input_api, output_api):
"""Checks for old style compiled_resources.gyp files."""
is_compiled_resource = lambda fp: fp.endswith('compiled_resources.gyp')

Expand Down Expand Up @@ -1969,7 +1969,7 @@ def _CheckNoDeprecatedCompiledResourcesGYP(input_api, output_api):
( "-webkit-repeating-radial-gradient", "repeating-radial-gradient" ),
]

def _CheckNoDeprecatedCSS(input_api, output_api):
def _CheckNoDeprecatedCss(input_api, output_api):
""" Make sure that we don't use deprecated CSS
properties, functions or values. Our external
documentation and iOS CSS for dom distiller
Expand Down Expand Up @@ -2004,7 +2004,7 @@ def _CheckNoDeprecatedCSS(input_api, output_api):
( "__defineSetter__", "Object.defineProperty" ),
]

def _CheckNoDeprecatedJS(input_api, output_api):
def _CheckNoDeprecatedJs(input_api, output_api):
"""Make sure that we don't use deprecated JS in Chrome code."""
results = []
file_inclusion_pattern = (r".+\.js$",) # TODO(dbeam): .html?
Expand All @@ -2022,6 +2022,27 @@ def _CheckNoDeprecatedJS(input_api, output_api):
return results


def _CheckForRiskyJsFeatures(input_api, output_api):
maybe_ios_js = (r"^(ios|components|ui\/webui\/resources)\/.+\.js$", )
file_filter = lambda f: input_api.FilterSourceFile(f, white_list=maybe_ios_js)

arrow_lines = []
for f in input_api.AffectedFiles(file_filter=file_filter):
for lnum, line in f.ChangedContents():
if ' => ' in line:
arrow_lines.append((f.LocalPath(), lnum))

if not arrow_lines:
return []

return [output_api.PresubmitPromptWarning("""
Use of => operator detected in:
%s
Please ensure your code does not run on iOS9 (=> (arrow) does not work there).
https://chromium.googlesource.com/chromium/src/+/master/docs/es6_chromium.md#Arrow-Functions
""" % "\n".join(" %s:%d\n" % line for line in arrow_lines))]


def _AndroidSpecificOnUploadChecks(input_api, output_api):
"""Groups checks that target android code."""
results = []
Expand Down Expand Up @@ -2070,18 +2091,19 @@ def _CommonChecks(input_api, output_api):
results.extend(_CheckForAnonymousVariables(input_api, output_api))
results.extend(_CheckCygwinShell(input_api, output_api))
results.extend(_CheckUserActionUpdate(input_api, output_api))
results.extend(_CheckNoDeprecatedCSS(input_api, output_api))
results.extend(_CheckNoDeprecatedJS(input_api, output_api))
results.extend(_CheckNoDeprecatedCss(input_api, output_api))
results.extend(_CheckNoDeprecatedJs(input_api, output_api))
results.extend(_CheckParseErrors(input_api, output_api))
results.extend(_CheckForIPCRules(input_api, output_api))
results.extend(_CheckForWindowsLineEndings(input_api, output_api))
results.extend(_CheckSingletonInHeaders(input_api, output_api))
results.extend(_CheckNoDeprecatedCompiledResourcesGYP(input_api, output_api))
results.extend(_CheckNoDeprecatedCompiledResourcesGyp(input_api, output_api))
results.extend(_CheckPydepsNeedsUpdating(input_api, output_api))
results.extend(_CheckJavaStyle(input_api, output_api))
results.extend(_CheckIpcOwners(input_api, output_api))
results.extend(_CheckMojoUsesNewWrapperTypes(input_api, output_api))
results.extend(_CheckUselessForwardDeclarations(input_api, output_api))
results.extend(_CheckForRiskyJsFeatures(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
44 changes: 40 additions & 4 deletions PRESUBMIT_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -412,16 +412,16 @@ def testSingletonInCC(self):
self.assertEqual(0, len(warnings))


class CheckNoDeprecatedCompiledResourcesGYPTest(unittest.TestCase):
def testNoDeprecatedCompiledResourcsGYP(self):
class CheckNoDeprecatedCompiledResourcesGypTest(unittest.TestCase):
def testNoDeprecatedCompiledResourcsGyp(self):
mock_input_api = MockInputApi()
mock_input_api.files = [MockFile('some/js/compiled_resources.gyp', [])]
errors = PRESUBMIT._CheckNoDeprecatedCompiledResourcesGYP(mock_input_api,
errors = PRESUBMIT._CheckNoDeprecatedCompiledResourcesGyp(mock_input_api,
MockOutputApi())
self.assertEquals(1, len(errors))

mock_input_api.files = [MockFile('some/js/compiled_resources2.gyp', [])]
errors = PRESUBMIT._CheckNoDeprecatedCompiledResourcesGYP(mock_input_api,
errors = PRESUBMIT._CheckNoDeprecatedCompiledResourcesGyp(mock_input_api,
MockOutputApi())
self.assertEquals(0, len(errors))

Expand Down Expand Up @@ -1188,5 +1188,41 @@ def testBlinkHeaders(self):
self.assertEqual(4, len(warnings))


class RiskyJsTest(unittest.TestCase):
def testArrowWarnInIos9Code(self):
mock_input_api = MockInputApi()
mock_output_api = MockOutputApi()

mock_input_api.files = [
MockAffectedFile('components/blah.js', ["shouldn't use => here"]),
]
warnings = PRESUBMIT._CheckForRiskyJsFeatures(
mock_input_api, mock_output_api)
self.assertEqual(1, len(warnings))

mock_input_api.files = [
MockAffectedFile('ios/blee.js', ['might => break folks']),
]
warnings = PRESUBMIT._CheckForRiskyJsFeatures(
mock_input_api, mock_output_api)
self.assertEqual(1, len(warnings))

mock_input_api.files = [
MockAffectedFile('ui/webui/resources/blarg.js', ['on => iOS9']),
]
warnings = PRESUBMIT._CheckForRiskyJsFeatures(
mock_input_api, mock_output_api)
self.assertEqual(1, len(warnings))

def testArrowsAllowedInChromeCode(self):
mock_input_api = MockInputApi()
mock_input_api.files = [
MockAffectedFile('chrome/browser/resources/blah.js', 'arrow => OK here'),
]
warnings = PRESUBMIT._CheckForRiskyJsFeatures(
mock_input_api, MockOutputApi())
self.assertEqual(0, len(warnings))


if __name__ == '__main__':
unittest.main()

0 comments on commit 1ec68ac

Please sign in to comment.