closure-library icon indicating copy to clipboard operation
closure-library copied to clipboard

Compiler lintChecks warn about many closure-library statements

Open ChadKillingsworth opened this issue 10 years ago • 6 comments

I'm getting quite a few linting errors in closure-library. If these are intentional, can we at least add @suppress annotations to them?

I have to keep the linting rules turned off on my own code because of this.

/node_modules/google-closure-library/closure/goog/array/array.js:888: ERROR - This use of the arguments object is discouraged. See https://github.com/google/closure-compiler/wiki/Lint-warning-about-%60arguments%60
  return Array.prototype.splice.apply(arr, goog.array.slice(arguments, 1));
                                                            ^

/node_modules/google-closure-library/closure/goog/asserts/asserts.js:152: ERROR - This use of the arguments object is discouraged. See https://github.com/google/closure-compiler/wiki/Lint-warning-about-%60arguments%60
        '', null, opt_message, Array.prototype.slice.call(arguments, 2));
                                                          ^

/node_modules/google-closure-library/closure/goog/asserts/asserts.js:181: ERROR - This use of the arguments object is discouraged. See https://github.com/google/closure-compiler/wiki/Lint-warning-about-%60arguments%60
            Array.prototype.slice.call(arguments, 1)));
                                       ^

/node_modules/google-closure-library/closure/goog/asserts/asserts.js:198: ERROR - This use of the arguments object is discouraged. See https://github.com/google/closure-compiler/wiki/Lint-warning-about-%60arguments%60
        opt_message, Array.prototype.slice.call(arguments, 2));
                                                ^

/node_modules/google-closure-library/closure/goog/asserts/asserts.js:216: ERROR - This use of the arguments object is discouraged. See https://github.com/google/closure-compiler/wiki/Lint-warning-about-%60arguments%60
        opt_message, Array.prototype.slice.call(arguments, 2));
                                                ^

/node_modules/google-closure-library/closure/goog/asserts/asserts.js:235: ERROR - This use of the arguments object is discouraged. See https://github.com/google/closure-compiler/wiki/Lint-warning-about-%60arguments%60
        opt_message, Array.prototype.slice.call(arguments, 2));
                                                ^

/node_modules/google-closure-library/closure/goog/asserts/asserts.js:253: ERROR - This use of the arguments object is discouraged. See https://github.com/google/closure-compiler/wiki/Lint-warning-about-%60arguments%60
        opt_message, Array.prototype.slice.call(arguments, 2));
                                                ^

/node_modules/google-closure-library/closure/goog/asserts/asserts.js:271: ERROR - This use of the arguments object is discouraged. See https://github.com/google/closure-compiler/wiki/Lint-warning-about-%60arguments%60
        opt_message, Array.prototype.slice.call(arguments, 2));
                                                ^

/node_modules/google-closure-library/closure/goog/asserts/asserts.js:290: ERROR - This use of the arguments object is discouraged. See https://github.com/google/closure-compiler/wiki/Lint-warning-about-%60arguments%60
        opt_message, Array.prototype.slice.call(arguments, 2));
                                                ^

/node_modules/google-closure-library/closure/goog/asserts/asserts.js:310: ERROR - This use of the arguments object is discouraged. See https://github.com/google/closure-compiler/wiki/Lint-warning-about-%60arguments%60
        opt_message, Array.prototype.slice.call(arguments, 2));
                                                ^

/node_modules/google-closure-library/closure/goog/asserts/asserts.js:336: ERROR - This use of the arguments object is discouraged. See https://github.com/google/closure-compiler/wiki/Lint-warning-about-%60arguments%60
        opt_message, Array.prototype.slice.call(arguments, 3));
                                                ^

/node_modules/google-closure-library/closure/goog/base.js:1818: ERROR - This use of the arguments object is discouraged. See https://github.com/google/closure-compiler/wiki/Lint-warning-about-%60arguments%60
    var boundArgs = Array.prototype.slice.call(arguments, 2);
                                               ^

/node_modules/google-closure-library/closure/goog/base.js:1821: ERROR - This use of the arguments object is discouraged. See https://github.com/google/closure-compiler/wiki/Lint-warning-about-%60arguments%60
      var newArgs = Array.prototype.slice.call(arguments);
                                               ^

/node_modules/google-closure-library/closure/goog/base.js:1888: ERROR - This use of the arguments object is discouraged. See https://github.com/google/closure-compiler/wiki/Lint-warning-about-%60arguments%60
  var args = Array.prototype.slice.call(arguments, 1);
                                        ^

/node_modules/google-closure-library/closure/goog/base.js:2290: ERROR - This use of the arguments object is discouraged. See https://github.com/google/closure-compiler/wiki/Lint-warning-about-%60arguments%60
  var caller = arguments.callee.caller;
               ^

/node_modules/google-closure-library/closure/goog/object/object.js:250: ERROR - This use of the arguments object is discouraged. See https://github.com/google/closure-compiler/wiki/Lint-warning-about-%60arguments%60
  var keys = isArrayLike ? var_args : arguments;
                                      ^

/node_modules/google-closure-library/closure/goog/string/string.js:124: ERROR - This use of the arguments object is discouraged. See https://github.com/google/closure-compiler/wiki/Lint-warning-about-%60arguments%60
  var subsArguments = Array.prototype.slice.call(arguments, 1);
                                                 ^

/node_modules/google-closure-library/closure/goog/string/string.js:1211: ERROR - This use of the arguments object is discouraged. See https://github.com/google/closure-compiler/wiki/Lint-warning-about-%60arguments%60
  return Array.prototype.join.call(arguments, '');
                                   ^

ChadKillingsworth avatar Feb 11 '16 13:02 ChadKillingsworth

I generally the "arguments" lint check is too aggressive. This is really ES6 specific, right now I think you can only suppress all lint checks which is a little too broad. I feel like you should be able to toggle checks more specifically.

On Thu, Feb 11, 2016 at 5:14 AM, Chad Killingsworth < [email protected]> wrote:

Even excluding the nullable reference check, I'm getting quite a few linting errors. If these are intentional, can we at least add @suppress annotations to them?

I have to keep the linting rules turned off on my own code because of this.

/node_modules/google-closure-library/closure/goog/array/array.js:888: ERROR - This use of the arguments object is discouraged. See https://github.com/google/closure-compiler/wiki/Lint-warning-about-%60arguments%60 return Array.prototype.splice.apply(arr, goog.array.slice(arguments, 1)); ^

/node_modules/google-closure-library/closure/goog/asserts/asserts.js:152: ERROR - This use of the arguments object is discouraged. See https://github.com/google/closure-compiler/wiki/Lint-warning-about-%60arguments%60 '', null, opt_message, Array.prototype.slice.call(arguments, 2)); ^

/node_modules/google-closure-library/closure/goog/asserts/asserts.js:181: ERROR - This use of the arguments object is discouraged. See https://github.com/google/closure-compiler/wiki/Lint-warning-about-%60arguments%60 Array.prototype.slice.call(arguments, 1))); ^

/node_modules/google-closure-library/closure/goog/asserts/asserts.js:198: ERROR - This use of the arguments object is discouraged. See https://github.com/google/closure-compiler/wiki/Lint-warning-about-%60arguments%60 opt_message, Array.prototype.slice.call(arguments, 2)); ^

/node_modules/google-closure-library/closure/goog/asserts/asserts.js:216: ERROR - This use of the arguments object is discouraged. See https://github.com/google/closure-compiler/wiki/Lint-warning-about-%60arguments%60 opt_message, Array.prototype.slice.call(arguments, 2)); ^

/node_modules/google-closure-library/closure/goog/asserts/asserts.js:235: ERROR - This use of the arguments object is discouraged. See https://github.com/google/closure-compiler/wiki/Lint-warning-about-%60arguments%60 opt_message, Array.prototype.slice.call(arguments, 2)); ^

/node_modules/google-closure-library/closure/goog/asserts/asserts.js:253: ERROR - This use of the arguments object is discouraged. See https://github.com/google/closure-compiler/wiki/Lint-warning-about-%60arguments%60 opt_message, Array.prototype.slice.call(arguments, 2)); ^

/node_modules/google-closure-library/closure/goog/asserts/asserts.js:271: ERROR - This use of the arguments object is discouraged. See https://github.com/google/closure-compiler/wiki/Lint-warning-about-%60arguments%60 opt_message, Array.prototype.slice.call(arguments, 2)); ^

/node_modules/google-closure-library/closure/goog/asserts/asserts.js:290: ERROR - This use of the arguments object is discouraged. See https://github.com/google/closure-compiler/wiki/Lint-warning-about-%60arguments%60 opt_message, Array.prototype.slice.call(arguments, 2)); ^

/node_modules/google-closure-library/closure/goog/asserts/asserts.js:310: ERROR - This use of the arguments object is discouraged. See https://github.com/google/closure-compiler/wiki/Lint-warning-about-%60arguments%60 opt_message, Array.prototype.slice.call(arguments, 2)); ^

/node_modules/google-closure-library/closure/goog/asserts/asserts.js:336: ERROR - This use of the arguments object is discouraged. See https://github.com/google/closure-compiler/wiki/Lint-warning-about-%60arguments%60 opt_message, Array.prototype.slice.call(arguments, 3)); ^

/node_modules/google-closure-library/closure/goog/base.js:1818: ERROR - This use of the arguments object is discouraged. See https://github.com/google/closure-compiler/wiki/Lint-warning-about-%60arguments%60 var boundArgs = Array.prototype.slice.call(arguments, 2); ^

/node_modules/google-closure-library/closure/goog/base.js:1821: ERROR - This use of the arguments object is discouraged. See https://github.com/google/closure-compiler/wiki/Lint-warning-about-%60arguments%60 var newArgs = Array.prototype.slice.call(arguments); ^

/node_modules/google-closure-library/closure/goog/base.js:1888: ERROR - This use of the arguments object is discouraged. See https://github.com/google/closure-compiler/wiki/Lint-warning-about-%60arguments%60 var args = Array.prototype.slice.call(arguments, 1); ^

/node_modules/google-closure-library/closure/goog/base.js:2290: ERROR - This use of the arguments object is discouraged. See https://github.com/google/closure-compiler/wiki/Lint-warning-about-%60arguments%60 var caller = arguments.callee.caller; ^

/node_modules/google-closure-library/closure/goog/object/object.js:250: ERROR - This use of the arguments object is discouraged. See https://github.com/google/closure-compiler/wiki/Lint-warning-about-%60arguments%60 var keys = isArrayLike ? var_args : arguments; ^

/node_modules/google-closure-library/closure/goog/string/string.js:124: ERROR - This use of the arguments object is discouraged. See https://github.com/google/closure-compiler/wiki/Lint-warning-about-%60arguments%60 var subsArguments = Array.prototype.slice.call(arguments, 1); ^

/node_modules/google-closure-library/closure/goog/string/string.js:1211: ERROR - This use of the arguments object is discouraged. See https://github.com/google/closure-compiler/wiki/Lint-warning-about-%60arguments%60 return Array.prototype.join.call(arguments, ''); ^

— Reply to this email directly or view it on GitHub https://github.com/google/closure-library/issues/654.

concavelenz avatar Feb 11 '16 15:02 concavelenz

Is there still a performance hit for passing the arguments array out of the function?

ChadKillingsworth avatar Feb 11 '16 16:02 ChadKillingsworth

yes, but to do otherwise involves work. I think we should rewrite "slice" in the compiler. I wrote about this. I'll copy it here:

Array.prototype.slice.call(arguments) and its twin [].slice.call(arguments) are known to perform badly because they escape "arguments" but both are common. I thinking of adding a compiler pass to rewrite them, but that would expanding them into a for loop wouldn't be good for code size. But I think I've found a compromise:

http://jsperf.com/arguments-to-array/40

"f6" calls an parameters-to-array helper method using "apply", which performs 10x better than "slice.call" on Chrome (2x on Firefox) but avoids the verbosity of a loop. But it is always possible I've fooled myself. It is also possible that this performance improvement doesn't actually matter in a more complex function.

concavelenz avatar Feb 12 '16 17:02 concavelenz

Is the code size difference here really even worth discussing? We can't be talking about much - even on large projects.

It's an easy enough peephole optimization to write. I wouldn't even mind doing it.

ChadKillingsworth avatar Feb 12 '16 17:02 ChadKillingsworth

I don't think code size is an issue if you are calling a helper method (but the helper method means you should only do it in advanced mode). Generally, I'm more concerned with adding code complexity for the user due to satisfying the lint check.

concavelenz avatar Feb 12 '16 17:02 concavelenz

I think this is closed by https://github.com/google/closure-compiler/commit/3734b26b1215693d5c46636d91b7a5bb2715f690.

Dominator008 avatar Mar 15 '16 19:03 Dominator008