Commit fe41133f authored by dpapad's avatar dpapad Committed by Commit Bot

WebUI style: Allow ES6 let/const usage.

 - Remove obsolete check and tests from js_checker.py
 - Convert Downloads page to use let/const as a proof of concept that it works.

Bug: 671426
Change-Id: I6ef56337c02e93b7389788ded0eaec00954457e5
Reviewed-on: https://chromium-review.googlesource.com/570901Reviewed-by: default avatarMichael Giuffrida <michaelpg@chromium.org>
Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488828}
parent 5f348030
// Copyright 2017 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
module.exports = {
'env': {
'browser': true,
'es6': true,
},
'rules': {
'no-var': 'error',
},
};
...@@ -61,7 +61,7 @@ cr.define('downloads', function() { ...@@ -61,7 +61,7 @@ cr.define('downloads', function() {
/** @param {string} url URL of a file to download. */ /** @param {string} url URL of a file to download. */
download: function(url) { download: function(url) {
var a = document.createElement('a'); const a = document.createElement('a');
a.href = url; a.href = url;
a.setAttribute('download', ''); a.setAttribute('download', '');
a.click(); a.click();
...@@ -113,10 +113,10 @@ cr.define('downloads', function() { ...@@ -113,10 +113,10 @@ cr.define('downloads', function() {
* @return {boolean} Whether |searchText| resulted in new search terms. * @return {boolean} Whether |searchText| resulted in new search terms.
*/ */
search: function(searchText) { search: function(searchText) {
var searchTerms = ActionService.splitTerms(searchText); const searchTerms = ActionService.splitTerms(searchText);
var sameTerms = searchTerms.length == this.searchTerms_.length; let sameTerms = searchTerms.length == this.searchTerms_.length;
for (var i = 0; sameTerms && i < searchTerms.length; ++i) { for (let i = 0; sameTerms && i < searchTerms.length; ++i) {
if (searchTerms[i] != this.searchTerms_[i]) if (searchTerms[i] != this.searchTerms_[i])
sameTerms = false; sameTerms = false;
} }
......
...@@ -27,7 +27,7 @@ ActionServiceUnitTest.prototype = { ...@@ -27,7 +27,7 @@ ActionServiceUnitTest.prototype = {
}; };
TEST_F('ActionServiceUnitTest', 'splitTerms', function() { TEST_F('ActionServiceUnitTest', 'splitTerms', function() {
var ActionService = downloads.ActionService; const ActionService = downloads.ActionService;
assertEquals(str([]), str(ActionService.splitTerms(''))); assertEquals(str([]), str(ActionService.splitTerms('')));
assertEquals(str([]), str(ActionService.splitTerms(' '))); assertEquals(str([]), str(ActionService.splitTerms(' ')));
assertEquals(str(['a']), str(ActionService.splitTerms('a'))); assertEquals(str(['a']), str(ActionService.splitTerms('a')));
...@@ -52,7 +52,7 @@ TEST_F('ActionServiceUnitTest', 'searchWithSimilarTerms', function() { ...@@ -52,7 +52,7 @@ TEST_F('ActionServiceUnitTest', 'searchWithSimilarTerms', function() {
loadMore: function() { /* No chrome.send() for you! */ }, loadMore: function() { /* No chrome.send() for you! */ },
}; };
var actionService = new TestActionService(); const actionService = new TestActionService();
assertTrue(actionService.search('a')); assertTrue(actionService.search('a'));
assertFalse(actionService.search('a ')); // Same term + space should no-op. assertFalse(actionService.search('a ')); // Same term + space should no-op.
......
...@@ -7,7 +7,7 @@ cr.define('downloads', function() { ...@@ -7,7 +7,7 @@ cr.define('downloads', function() {
* Explains why a download is in DANGEROUS state. * Explains why a download is in DANGEROUS state.
* @enum {string} * @enum {string}
*/ */
var DangerType = { const DangerType = {
NOT_DANGEROUS: 'NOT_DANGEROUS', NOT_DANGEROUS: 'NOT_DANGEROUS',
DANGEROUS_FILE: 'DANGEROUS_FILE', DANGEROUS_FILE: 'DANGEROUS_FILE',
DANGEROUS_URL: 'DANGEROUS_URL', DANGEROUS_URL: 'DANGEROUS_URL',
...@@ -22,7 +22,7 @@ cr.define('downloads', function() { ...@@ -22,7 +22,7 @@ cr.define('downloads', function() {
* DownloadsDOMHandler::CreateDownloadItemValue * DownloadsDOMHandler::CreateDownloadItemValue
* @enum {string} * @enum {string}
*/ */
var States = { const States = {
IN_PROGRESS: 'IN_PROGRESS', IN_PROGRESS: 'IN_PROGRESS',
CANCELLED: 'CANCELLED', CANCELLED: 'CANCELLED',
COMPLETE: 'COMPLETE', COMPLETE: 'COMPLETE',
......
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
* @externs * @externs
*/ */
// eslint-disable-next-line no-var
var downloads = {}; var downloads = {};
/** /**
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
// found in the LICENSE file. // found in the LICENSE file.
cr.define('downloads', function() { cr.define('downloads', function() {
var Item = Polymer({ const Item = Polymer({
is: 'downloads-item', is: 'downloads-item',
properties: { properties: {
...@@ -89,7 +89,7 @@ cr.define('downloads', function() { ...@@ -89,7 +89,7 @@ cr.define('downloads', function() {
/** @private */ /** @private */
computeClass_: function() { computeClass_: function() {
var classes = []; const classes = [];
if (this.isActive_) if (this.isActive_)
classes.push('is-active'); classes.push('is-active');
...@@ -114,8 +114,8 @@ cr.define('downloads', function() { ...@@ -114,8 +114,8 @@ cr.define('downloads', function() {
if (!this.data.by_ext_id || !this.data.by_ext_name) if (!this.data.by_ext_id || !this.data.by_ext_name)
return ''; return '';
var url = 'chrome://extensions#' + this.data.by_ext_id; const url = 'chrome://extensions#' + this.data.by_ext_id;
var name = this.data.by_ext_name; const name = this.data.by_ext_name;
return loadTimeData.getStringF('controlledByUrl', url, HTMLEscape(name)); return loadTimeData.getStringF('controlledByUrl', url, HTMLEscape(name));
}, },
...@@ -134,11 +134,11 @@ cr.define('downloads', function() { ...@@ -134,11 +134,11 @@ cr.define('downloads', function() {
/** @private */ /** @private */
computeDescription_: function() { computeDescription_: function() {
var data = this.data; const data = this.data;
switch (data.state) { switch (data.state) {
case downloads.States.DANGEROUS: case downloads.States.DANGEROUS:
var fileName = data.file_name; const fileName = data.file_name;
switch (data.danger_type) { switch (data.danger_type) {
case downloads.DangerType.DANGEROUS_FILE: case downloads.DangerType.DANGEROUS_FILE:
return loadTimeData.getString('dangerFileDesc'); return loadTimeData.getString('dangerFileDesc');
...@@ -201,8 +201,8 @@ cr.define('downloads', function() { ...@@ -201,8 +201,8 @@ cr.define('downloads', function() {
/** @private */ /** @private */
computeRemoveStyle_: function() { computeRemoveStyle_: function() {
var canDelete = loadTimeData.getBoolean('allowDeletingHistory'); const canDelete = loadTimeData.getBoolean('allowDeletingHistory');
var hideRemove = this.isDangerous_ || this.showCancel_ || !canDelete; const hideRemove = this.isDangerous_ || this.showCancel_ || !canDelete;
return hideRemove ? 'visibility: hidden' : ''; return hideRemove ? 'visibility: hidden' : '';
}, },
...@@ -254,8 +254,8 @@ cr.define('downloads', function() { ...@@ -254,8 +254,8 @@ cr.define('downloads', function() {
this.$.url.removeAttribute('href'); this.$.url.removeAttribute('href');
} else { } else {
this.$.url.href = assert(this.data.url); this.$.url.href = assert(this.data.url);
var filePath = encodeURIComponent(this.data.file_path); const filePath = encodeURIComponent(this.data.file_path);
var scaleFactor = '?scale=' + window.devicePixelRatio + 'x'; const scaleFactor = '?scale=' + window.devicePixelRatio + 'x';
this.$['file-icon'].src = 'chrome://fileicon/' + filePath + scaleFactor; this.$['file-icon'].src = 'chrome://fileicon/' + filePath + scaleFactor;
} }
}, },
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
// found in the LICENSE file. // found in the LICENSE file.
cr.define('downloads', function() { cr.define('downloads', function() {
var Manager = Polymer({ const Manager = Polymer({
is: 'downloads-manager', is: 'downloads-manager',
properties: { properties: {
...@@ -151,7 +151,7 @@ cr.define('downloads', function() { ...@@ -151,7 +151,7 @@ cr.define('downloads', function() {
/** @private */ /** @private */
onListScroll_: function() { onListScroll_: function() {
var list = this.$['downloads-list']; const list = this.$['downloads-list'];
if (list.scrollHeight - list.scrollTop - list.offsetHeight <= 100) { if (list.scrollHeight - list.scrollTop - list.offsetHeight <= 100) {
// Approaching the end of the scrollback. Attempt to load more items. // Approaching the end of the scrollback. Attempt to load more items.
downloads.ActionService.getInstance().loadMore(); downloads.ActionService.getInstance().loadMore();
...@@ -193,12 +193,12 @@ cr.define('downloads', function() { ...@@ -193,12 +193,12 @@ cr.define('downloads', function() {
* @private * @private
*/ */
updateHideDates_: function(start, end) { updateHideDates_: function(start, end) {
for (var i = start; i <= end; ++i) { for (let i = start; i <= end; ++i) {
var current = this.items_[i]; const current = this.items_[i];
if (!current) if (!current)
continue; continue;
var prev = this.items_[i - 1]; const prev = this.items_[i - 1];
var hideDate = !!prev && prev.date_string == current.date_string; const hideDate = !!prev && prev.date_string == current.date_string;
this.set('items_.' + i + '.hideDate', hideDate); this.set('items_.' + i + '.hideDate', hideDate);
} }
}, },
...@@ -211,7 +211,7 @@ cr.define('downloads', function() { ...@@ -211,7 +211,7 @@ cr.define('downloads', function() {
updateItem_: function(index, data) { updateItem_: function(index, data) {
this.set('items_.' + index, data); this.set('items_.' + index, data);
this.updateHideDates_(index, index); this.updateHideDates_(index, index);
var list = /** @type {!IronListElement} */ (this.$['downloads-list']); const list = /** @type {!IronListElement} */ (this.$['downloads-list']);
list.updateSizeForItem(index); list.updateSizeForItem(index);
}, },
}); });
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
// found in the LICENSE file. // found in the LICENSE file.
cr.define('downloads', function() { cr.define('downloads', function() {
var Toolbar = Polymer({ const Toolbar = Polymer({
is: 'downloads-toolbar', is: 'downloads-toolbar',
properties: { properties: {
...@@ -61,7 +61,7 @@ cr.define('downloads', function() { ...@@ -61,7 +61,7 @@ cr.define('downloads', function() {
* @private * @private
*/ */
onSearchChanged_: function(event) { onSearchChanged_: function(event) {
var actionService = downloads.ActionService.getInstance(); const actionService = downloads.ActionService.getInstance();
if (actionService.search(/** @type {string} */ (event.detail))) if (actionService.search(/** @type {string} */ (event.detail)))
this.spinnerActive = actionService.isSearching(); this.spinnerActive = actionService.isSearching();
this.updateClearAll_(); this.updateClearAll_();
......
...@@ -5,7 +5,7 @@ ...@@ -5,7 +5,7 @@
/** @fileoverview Tests for the Material Design downloads page. */ /** @fileoverview Tests for the Material Design downloads page. */
/** @const {string} Path to source root. */ /** @const {string} Path to source root. */
var ROOT_PATH = '../../../../../'; const ROOT_PATH = '../../../../../';
// Polymer BrowserTest fixture. // Polymer BrowserTest fixture.
GEN_INCLUDE( GEN_INCLUDE(
......
...@@ -4,7 +4,7 @@ ...@@ -4,7 +4,7 @@
suite('item tests', function() { suite('item tests', function() {
/** @type {!downloads.Item} */ /** @type {!downloads.Item} */
var item; let item;
setup(function() { setup(function() {
item = document.createElement('downloads-item'); item = document.createElement('downloads-item');
......
...@@ -4,7 +4,7 @@ ...@@ -4,7 +4,7 @@
suite('layout tests', function() { suite('layout tests', function() {
/** @type {!downloads.Manager} */ /** @type {!downloads.Manager} */
var manager; let manager;
setup(function() { setup(function() {
PolymerTest.clearBody(); PolymerTest.clearBody();
...@@ -22,7 +22,7 @@ suite('layout tests', function() { ...@@ -22,7 +22,7 @@ suite('layout tests', function() {
Polymer.dom.flush(); Polymer.dom.flush();
var item = manager.$$('downloads-item'); const item = manager.$$('downloads-item');
assertLT(item.$$('#url').offsetWidth, item.offsetWidth); assertLT(item.$$('#url').offsetWidth, item.offsetWidth);
assertEquals(300, item.$$('#url').textContent.length); assertEquals(300, item.$$('#url').textContent.length);
}); });
......
...@@ -4,7 +4,7 @@ ...@@ -4,7 +4,7 @@
suite('toolbar tests', function() { suite('toolbar tests', function() {
/** @type {!downloads.Toolbar} */ /** @type {!downloads.Toolbar} */
var toolbar; let toolbar;
setup(function() { setup(function() {
/** /**
......
...@@ -281,16 +281,6 @@ this document. ...@@ -281,16 +281,6 @@ this document.
--- ---
# Banned Features
The following features are banned for Chromium development.
# Features To Be Discussed
The following features are currently disallowed. See the top of this page on
how to propose moving a feature from this list into the allowed or banned
sections.
## let (Block-Scoped Variables) ## let (Block-Scoped Variables)
`let` declares a variable within the scope of a block. This differs from `var`, `let` declares a variable within the scope of a block. This differs from `var`,
...@@ -330,7 +320,11 @@ function f() { ...@@ -330,7 +320,11 @@ function f() {
**Documentation:** [link](http://www.ecma-international.org/ecma-262/6.0/#sec-let-and-const-declarations) **Documentation:** [link](http://www.ecma-international.org/ecma-262/6.0/#sec-let-and-const-declarations)
**Discussion Notes / Link to Thread:** **Discussion Notes / Link to Thread:** [link](https://groups.google.com/a/chromium.org/d/msg/chromium-dev/MJhTok8Usr8/XCrkisaBBQAJ)
**Note**: `let` is [not fully supported](https://caniuse.com/#feat=let) in iOS9.
Don't use it in code that runs on Chrome for iOS, until support for iOS9 is
dropped.
--- ---
...@@ -358,10 +352,24 @@ frobber.isFrobbing = false; // Works. ...@@ -358,10 +352,24 @@ frobber.isFrobbing = false; // Works.
**See also:** [Object.freeze()](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/freeze) **See also:** [Object.freeze()](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/freeze)
**Discussion Notes / Link to Thread:** **Discussion Notes / Link to Thread:** [link](https://groups.google.com/a/chromium.org/d/msg/chromium-dev/MJhTok8Usr8/XCrkisaBBQAJ)
**Note**: `const` [not fully supported](https://caniuse.com/#feat=let) in iOS9.
Don't use it in code that runs on Chrome for iOS, until support for iOS9 is
dropped.
--- ---
# Banned Features
The following features are banned for Chromium development.
# Features To Be Discussed
The following features are currently disallowed. See the top of this page on
how to propose moving a feature from this list into the allowed or banned
sections.
## Block Scope Functions ## Block Scope Functions
**Usage Example:** **Usage Example:**
......
...@@ -33,15 +33,6 @@ class JSChecker(object): ...@@ -33,15 +33,6 @@ class JSChecker(object):
' // <if expr="chromeos">\n' + ' // <if expr="chromeos">\n' +
' // </if>\n') ' // </if>\n')
def ConstCheck(self, i, line):
"""Check for use of the 'const' keyword."""
if self.input_api.re.search(r'\*\s+@const', line):
# Probably a JsDoc line
return ''
return self.RegexCheck(i, line, r'(?:^|\s|\()(const)\s',
'Use /** @const */ var varName; instead of const varName;')
def EndJsDocCommentCheck(self, i, line): def EndJsDocCommentCheck(self, i, line):
msg = 'End JSDoc comments with */ instead of **/' msg = 'End JSDoc comments with */ instead of **/'
def _check(regex): def _check(regex):
...@@ -128,7 +119,6 @@ class JSChecker(object): ...@@ -128,7 +119,6 @@ class JSChecker(object):
error_lines += filter(None, [ error_lines += filter(None, [
self.ChromeSendCheck(i, line), self.ChromeSendCheck(i, line),
self.CommentIfAndIncludeCheck(i, line), self.CommentIfAndIncludeCheck(i, line),
self.ConstCheck(i, line),
self.EndJsDocCommentCheck(i, line), self.EndJsDocCommentCheck(i, line),
self.ExtraDotInGenericCheck(i, line), self.ExtraDotInGenericCheck(i, line),
self.InheritDocCheck(i, line), self.InheritDocCheck(i, line),
......
...@@ -64,65 +64,6 @@ class JsCheckerTest(SuperMoxTestBase): ...@@ -64,65 +64,6 @@ class JsCheckerTest(SuperMoxTestBase):
for line in lines: for line in lines:
self.ShouldPassCommentCheck(line) self.ShouldPassCommentCheck(line)
def ShouldFailConstCheck(self, line):
"""Checks that the 'const' checker flags |line| as a style error."""
error = self.checker.ConstCheck(1, line)
self.assertNotEqual('', error,
'Should be flagged as style error: ' + line)
self.assertEqual(test_util.GetHighlight(line, error), 'const')
def ShouldPassConstCheck(self, line):
"""Checks that the 'const' checker doesn't flag |line| as a style error."""
self.assertEqual('', self.checker.ConstCheck(1, line),
'Should not be flagged as style error: ' + line)
def testConstFails(self):
lines = [
"const foo = 'bar';",
" const bar = 'foo';",
# Trying to use |const| as a variable name
"var const = 0;",
"var x = 5; const y = 6;",
"for (var i=0, const e=10; i<e; i++) {",
"for (const x=0; x<foo; i++) {",
"while (const x = 7) {",
]
for line in lines:
self.ShouldFailConstCheck(line)
def testConstPasses(self):
lines = [
# sanity check
"var foo = 'bar'",
# @const JsDoc tag
"/** @const */ var SEVEN = 7;",
# @const tag in multi-line comment
" * @const",
" * @const",
# @constructor tag in multi-line comment
" * @constructor",
" * @constructor",
# words containing 'const'
"if (foo.constructor) {",
"var deconstruction = 'something';",
"var madeUpWordconst = 10;",
# Strings containing the word |const|
"var str = 'const at the beginning';",
"var str = 'At the end: const';",
# doing this one with regex is probably not practical
#"var str = 'a const in the middle';",
]
for line in lines:
self.ShouldPassConstCheck(line)
def ShouldFailChromeSendCheck(self, line): def ShouldFailChromeSendCheck(self, line):
"""Checks that the 'chrome.send' checker flags |line| as a style error.""" """Checks that the 'chrome.send' checker flags |line| as a style error."""
error = self.checker.ChromeSendCheck(1, line) error = self.checker.ChromeSendCheck(1, line)
......
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