Update Chromoting to use /third_party/closure_compiler.

Update Chromoting to use /third_party/closure_compiler.

This cl updates the checker.py script in /third_party/closure_compiler to
add an option to process a set of JS files at once rather than one at a
time (the current default). It also adds a flag to enable additional compiler
checks (since Chromoting wants all checks enabled).

It also updates the Chromoting .gyp files to make use of the new compiler.

The remaining changes were motivated by the errors reported by the new
closure compiler.

BUG=

Committed: https://crrev.com/23605febf70f2c6eb42322e2392522cbf0175b8a
Cr-Commit-Position: refs/heads/master@{#311531}

[garykac, 2015-01-12 20:22:35]

garykac@chromium.org changed reviewers:
+ kelvinp@chromium.org, tbreisacher@chromium.org

[garykac, 2015-01-12 20:22:37]

dtbreisacher: Please look at checker.py (dbeam is OOO until 1/22)

kelvinp: Please review the Chromoting changes

https://codereview.chromium.org/803653004/diff/1/remoting/webapp/base/js/base.js
File remoting/webapp/base/js/base.js (right):

https://codereview.chromium.org/803653004/diff/1/remoting/webapp/base/js/base...
remoting/webapp/base/js/base.js:214: var recordPromise = function(resolve,
reject) {
Hoisted here because @this doesn't seem to work for inline functions.

https://codereview.chromium.org/803653004/diff/1/remoting/webapp/crd/js/host_...
File remoting/webapp/crd/js/host_list.js (right):

https://codereview.chromium.org/803653004/diff/1/remoting/webapp/crd/js/host_...
remoting/webapp/crd/js/host_list.js:204: remoting.HostList.prototype.sortHosts_
= function() {
Moved into a separate function so that the @suppress {reportUnknownTypes} only
applies to the sorting code.

https://codereview.chromium.org/803653004/diff/1/remoting/webapp/crd/js/ui_mo...
File remoting/webapp/crd/js/ui_mode.js (right):

https://codereview.chromium.org/803653004/diff/1/remoting/webapp/crd/js/ui_mo...
remoting/webapp/crd/js/ui_mode.js:194: remoting.showOrHideCallbackIT2Me);
It had trouble with the bind, so this removes the need for the bind by breaking
it into 2 separate functions.

https://codereview.chromium.org/803653004/diff/1/third_party/closure_compiler...
File third_party/closure_compiler/checker.py (left):

https://codereview.chromium.org/803653004/diff/1/third_party/closure_compiler...
third_party/closure_compiler/checker.py:66: self._compiler_jar =
os.path.join(current_dir, "lib", "compiler.jar")
This is never used.

[Tyler Breisacher (Chromium), 2015-01-12 20:35:16]

https://codereview.chromium.org/803653004/diff/20001/third_party/closure_comp...
File third_party/closure_compiler/checker.py (right):

https://codereview.chromium.org/803653004/diff/20001/third_party/closure_comp...
third_party/closure_compiler/checker.py:52: "--jscomp_error=reportUnknownTypes",
We generally don't recommend people use this flag. It's rare that you'll have
type information for every single node in your entire program.

https://codereview.chromium.org/803653004/diff/20001/third_party/closure_comp...
third_party/closure_compiler/checker.py:180: def check(self, source_file,
depends=None, externs=None):
Would it make sense for check() to just call check_multiple passing a
one-element list, so that you don't have to duplicate a lot of logic between the
two?

https://codereview.chromium.org/803653004/diff/20001/third_party/closure_comp...
third_party/closure_compiler/checker.py:317: sys.exit(exit)
It looks like 'exit' is no longer defined. Maybe just sys.exit(1) or something.

[Jamie, 2015-01-12 20:45:10]

https://codereview.chromium.org/803653004/diff/20001/third_party/closure_comp...
File third_party/closure_compiler/checker.py (right):

https://codereview.chromium.org/803653004/diff/20001/third_party/closure_comp...
third_party/closure_compiler/checker.py:52: "--jscomp_error=reportUnknownTypes",
On 2015/01/12 20:35:15, Tyler Breisacher (Chromium) wrote:
> We generally don't recommend people use this flag. It's rare that you'll have
> type information for every single node in your entire program.

We took a decision back at the start of the project that we would do this. It's
sometimes painful to annotate everything, but it's caught some bugs that would
otherwise have gone undetected. Perhaps the compiler has improved to the point
that it's no longer necessary, but the incremental cost of continuing to
annotate everything is relatively low at this point, so we'd like to keep it.

[Tyler Breisacher (Chromium), 2015-01-12 20:47:00]

On 2015/01/12 20:45:10, Jamie wrote:
>
https://codereview.chromium.org/803653004/diff/20001/third_party/closure_comp...
> File third_party/closure_compiler/checker.py (right):
> 
>
https://codereview.chromium.org/803653004/diff/20001/third_party/closure_comp...
> third_party/closure_compiler/checker.py:52:
"--jscomp_error=reportUnknownTypes",
> On 2015/01/12 20:35:15, Tyler Breisacher (Chromium) wrote:
> > We generally don't recommend people use this flag. It's rare that you'll
have
> > type information for every single node in your entire program.
> 
> We took a decision back at the start of the project that we would do this.
It's
> sometimes painful to annotate everything, but it's caught some bugs that would
> otherwise have gone undetected. Perhaps the compiler has improved to the point
> that it's no longer necessary, but the incremental cost of continuing to
> annotate everything is relatively low at this point, so we'd like to keep it.

Okay cool! If you're willing to be that aggressive about typechecking every
line, you might want to try out our new type inference system :)

[Tyler Breisacher (Chromium), 2015-01-12 20:48:30]

On 2015/01/12 20:47:00, Tyler Breisacher (Chromium) wrote:
> On 2015/01/12 20:45:10, Jamie wrote:
> >
>
https://codereview.chromium.org/803653004/diff/20001/third_party/closure_comp...
> > File third_party/closure_compiler/checker.py (right):
> > 
> >
>
https://codereview.chromium.org/803653004/diff/20001/third_party/closure_comp...
> > third_party/closure_compiler/checker.py:52:
> "--jscomp_error=reportUnknownTypes",
> > On 2015/01/12 20:35:15, Tyler Breisacher (Chromium) wrote:
> > > We generally don't recommend people use this flag. It's rare that you'll
> have
> > > type information for every single node in your entire program.
> > 
> > We took a decision back at the start of the project that we would do this.
> It's
> > sometimes painful to annotate everything, but it's caught some bugs that
would
> > otherwise have gone undetected. Perhaps the compiler has improved to the
point
> > that it's no longer necessary, but the incremental cost of continuing to
> > annotate everything is relatively low at this point, so we'd like to keep
it.
> 
> Okay cool! If you're willing to be that aggressive about typechecking every
> line, you might want to try out our new type inference system :)

(ping dimvar<at>google.com for details on that. I tried to CC him on this review
but I'm not sure if it worked)

[kelvinp, 2015-01-12 22:51:28]

Thx for doing this Gary.  Mostly nits feedback here.
lgtm once comments are addressed.

https://codereview.chromium.org/803653004/diff/20001/remoting/webapp/base/js/...
File remoting/webapp/base/js/base.js (right):

https://codereview.chromium.org/803653004/diff/20001/remoting/webapp/base/js/...
remoting/webapp/base/js/base.js:49: } catch (/** @type {*} */ e) {
I noticed that in some other files it is no longer have the temp var error.

It is possible to do
} catch {/** @type {Error} */ e) {
and get rid of the var error?

https://codereview.chromium.org/803653004/diff/20001/remoting/webapp/base/js/...
remoting/webapp/base/js/base.js:214: var recordPromise = function(resolve,
reject) {
rename to initPromise?

https://codereview.chromium.org/803653004/diff/20001/remoting/webapp/crd/js/c...
File remoting/webapp/crd/js/client_session.js (right):

https://codereview.chromium.org/803653004/diff/20001/remoting/webapp/crd/js/c...
remoting/webapp/crd/js/client_session.js:173: * @type {function(boolean=):void}
should this be 
@type {function(boolean):void}
as the boolean parameter is not optional?

https://codereview.chromium.org/803653004/diff/20001/remoting/webapp/crd/js/h...
File remoting/webapp/crd/js/hangout_session.js (right):

https://codereview.chromium.org/803653004/diff/20001/remoting/webapp/crd/js/h...
remoting/webapp/crd/js/hangout_session.js:52: } catch (/** @type {*} */ e) {
Can we use /** @type {Error} */ e here and get rid of var error on line 55?

https://codereview.chromium.org/803653004/diff/20001/remoting/webapp/crd/js/h...
File remoting/webapp/crd/js/host_daemon_facade.js (right):

https://codereview.chromium.org/803653004/diff/20001/remoting/webapp/crd/js/h...
remoting/webapp/crd/js/host_daemon_facade.js:108: reject(false);
why false here?

https://codereview.chromium.org/803653004/diff/20001/remoting/webapp/crd/js/i...
File remoting/webapp/crd/js/it2me_helpee_channel.js (right):

https://codereview.chromium.org/803653004/diff/20001/remoting/webapp/crd/js/i...
remoting/webapp/crd/js/it2me_helpee_channel.js:191: } catch(/** @type {*} */ e)
{
Is is possible to do the following on line 191
} catch(/** @type {Error} */ e) { 
and get rid of the extra variable on 192?

https://codereview.chromium.org/803653004/diff/20001/remoting/webapp/crd/js/s...
File remoting/webapp/crd/js/smart_reconnector.js (right):

https://codereview.chromium.org/803653004/diff/20001/remoting/webapp/crd/js/s...
remoting/webapp/crd/js/smart_reconnector.js:84: * @param
{remoting.ClientSession.StateEvent=} event
why the =?

[garykac, 2015-01-13 00:28:39]

https://codereview.chromium.org/803653004/diff/20001/remoting/webapp/base/js/...
File remoting/webapp/base/js/base.js (right):

https://codereview.chromium.org/803653004/diff/20001/remoting/webapp/base/js/...
remoting/webapp/base/js/base.js:49: } catch (/** @type {*} */ e) {
On 2015/01/12 22:51:28, kelvinp wrote:
> I noticed that in some other files it is no longer have the temp var error.
> 
> It is possible to do
> } catch {/** @type {Error} */ e) {
> and get rid of the var error?

Done.

https://codereview.chromium.org/803653004/diff/20001/remoting/webapp/base/js/...
remoting/webapp/base/js/base.js:214: var recordPromise = function(resolve,
reject) {
On 2015/01/12 22:51:28, kelvinp wrote:
> rename to initPromise?

Done.

https://codereview.chromium.org/803653004/diff/20001/remoting/webapp/crd/js/c...
File remoting/webapp/crd/js/client_session.js (right):

https://codereview.chromium.org/803653004/diff/20001/remoting/webapp/crd/js/c...
remoting/webapp/crd/js/client_session.js:173: * @type {function(boolean=):void}
On 2015/01/12 22:51:28, kelvinp wrote:
> should this be 
> @type {function(boolean):void}
> as the boolean parameter is not optional?

addListener and removeListener expect a function(boolean=):void.

https://codereview.chromium.org/803653004/diff/20001/remoting/webapp/crd/js/h...
File remoting/webapp/crd/js/hangout_session.js (right):

https://codereview.chromium.org/803653004/diff/20001/remoting/webapp/crd/js/h...
remoting/webapp/crd/js/hangout_session.js:52: } catch (/** @type {*} */ e) {
On 2015/01/12 22:51:28, kelvinp wrote:
> Can we use /** @type {Error} */ e here and get rid of var error on line 55?

Done.

https://codereview.chromium.org/803653004/diff/20001/remoting/webapp/crd/js/h...
File remoting/webapp/crd/js/host_daemon_facade.js (right):

https://codereview.chromium.org/803653004/diff/20001/remoting/webapp/crd/js/h...
remoting/webapp/crd/js/host_daemon_facade.js:108: reject(false);
On 2015/01/12 22:51:28, kelvinp wrote:
> why false here?

reject() requires at least 1 argument(s) and no more than 1 argument(s)

The value 'false' is arbitrary, but it seemed better than true. Is something
like '0' better here?

https://codereview.chromium.org/803653004/diff/20001/remoting/webapp/crd/js/i...
File remoting/webapp/crd/js/it2me_helpee_channel.js (right):

https://codereview.chromium.org/803653004/diff/20001/remoting/webapp/crd/js/i...
remoting/webapp/crd/js/it2me_helpee_channel.js:191: } catch(/** @type {*} */ e)
{
On 2015/01/12 22:51:28, kelvinp wrote:
> Is is possible to do the following on line 191
> } catch(/** @type {Error} */ e) { 
> and get rid of the extra variable on 192?

Done.

https://codereview.chromium.org/803653004/diff/20001/remoting/webapp/crd/js/s...
File remoting/webapp/crd/js/smart_reconnector.js (right):

https://codereview.chromium.org/803653004/diff/20001/remoting/webapp/crd/js/s...
remoting/webapp/crd/js/smart_reconnector.js:84: * @param
{remoting.ClientSession.StateEvent=} event
On 2015/01/12 22:51:28, kelvinp wrote:
> why the =?

base.EventSource.prototype.addEventListener is defined to take a
function(?=):void. The new compiler is actually checking that now.

https://codereview.chromium.org/803653004/diff/20001/third_party/closure_comp...
File third_party/closure_compiler/checker.py (right):

https://codereview.chromium.org/803653004/diff/20001/third_party/closure_comp...
third_party/closure_compiler/checker.py:180: def check(self, source_file,
depends=None, externs=None):
On 2015/01/12 20:35:15, Tyler Breisacher (Chromium) wrote:
> Would it make sense for check() to just call check_multiple passing a
> one-element list, so that you don't have to duplicate a lot of logic between
the
> two?

The single-file code path does a bunch of stuff:
* combine source + includes into a temp file
* fixup the error messages because they no longer match the source
* filter out some error messages
That we would rather not do - for our use it seems unnecessary and more complex
that just using the source files directly.

But it's easy enough to extract the common code into a helper function. PTAL

https://codereview.chromium.org/803653004/diff/20001/third_party/closure_comp...
third_party/closure_compiler/checker.py:317: sys.exit(exit)
On 2015/01/12 20:35:15, Tyler Breisacher (Chromium) wrote:
> It looks like 'exit' is no longer defined. Maybe just sys.exit(1) or
something.

Done.

[garykac, 2015-01-13 00:28:39]

https://codereview.chromium.org/803653004/diff/20001/remoting/webapp/base/js/...
File remoting/webapp/base/js/base.js (right):

https://codereview.chromium.org/803653004/diff/20001/remoting/webapp/base/js/...
remoting/webapp/base/js/base.js:49: } catch (/** @type {*} */ e) {
On 2015/01/12 22:51:28, kelvinp wrote:
> I noticed that in some other files it is no longer have the temp var error.
> 
> It is possible to do
> } catch {/** @type {Error} */ e) {
> and get rid of the var error?

Done.

https://codereview.chromium.org/803653004/diff/20001/remoting/webapp/base/js/...
remoting/webapp/base/js/base.js:214: var recordPromise = function(resolve,
reject) {
On 2015/01/12 22:51:28, kelvinp wrote:
> rename to initPromise?

Done.

https://codereview.chromium.org/803653004/diff/20001/remoting/webapp/crd/js/c...
File remoting/webapp/crd/js/client_session.js (right):

https://codereview.chromium.org/803653004/diff/20001/remoting/webapp/crd/js/c...
remoting/webapp/crd/js/client_session.js:173: * @type {function(boolean=):void}
On 2015/01/12 22:51:28, kelvinp wrote:
> should this be 
> @type {function(boolean):void}
> as the boolean parameter is not optional?

addListener and removeListener expect a function(boolean=):void.

https://codereview.chromium.org/803653004/diff/20001/remoting/webapp/crd/js/h...
File remoting/webapp/crd/js/hangout_session.js (right):

https://codereview.chromium.org/803653004/diff/20001/remoting/webapp/crd/js/h...
remoting/webapp/crd/js/hangout_session.js:52: } catch (/** @type {*} */ e) {
On 2015/01/12 22:51:28, kelvinp wrote:
> Can we use /** @type {Error} */ e here and get rid of var error on line 55?

Done.

https://codereview.chromium.org/803653004/diff/20001/remoting/webapp/crd/js/h...
File remoting/webapp/crd/js/host_daemon_facade.js (right):

https://codereview.chromium.org/803653004/diff/20001/remoting/webapp/crd/js/h...
remoting/webapp/crd/js/host_daemon_facade.js:108: reject(false);
On 2015/01/12 22:51:28, kelvinp wrote:
> why false here?

reject() requires at least 1 argument(s) and no more than 1 argument(s)

The value 'false' is arbitrary, but it seemed better than true. Is something
like '0' better here?

https://codereview.chromium.org/803653004/diff/20001/remoting/webapp/crd/js/i...
File remoting/webapp/crd/js/it2me_helpee_channel.js (right):

https://codereview.chromium.org/803653004/diff/20001/remoting/webapp/crd/js/i...
remoting/webapp/crd/js/it2me_helpee_channel.js:191: } catch(/** @type {*} */ e)
{
On 2015/01/12 22:51:28, kelvinp wrote:
> Is is possible to do the following on line 191
> } catch(/** @type {Error} */ e) { 
> and get rid of the extra variable on 192?

Done.

https://codereview.chromium.org/803653004/diff/20001/remoting/webapp/crd/js/s...
File remoting/webapp/crd/js/smart_reconnector.js (right):

https://codereview.chromium.org/803653004/diff/20001/remoting/webapp/crd/js/s...
remoting/webapp/crd/js/smart_reconnector.js:84: * @param
{remoting.ClientSession.StateEvent=} event
On 2015/01/12 22:51:28, kelvinp wrote:
> why the =?

base.EventSource.prototype.addEventListener is defined to take a
function(?=):void. The new compiler is actually checking that now.

https://codereview.chromium.org/803653004/diff/20001/third_party/closure_comp...
File third_party/closure_compiler/checker.py (right):

https://codereview.chromium.org/803653004/diff/20001/third_party/closure_comp...
third_party/closure_compiler/checker.py:180: def check(self, source_file,
depends=None, externs=None):
On 2015/01/12 20:35:15, Tyler Breisacher (Chromium) wrote:
> Would it make sense for check() to just call check_multiple passing a
> one-element list, so that you don't have to duplicate a lot of logic between
the
> two?

The single-file code path does a bunch of stuff:
* combine source + includes into a temp file
* fixup the error messages because they no longer match the source
* filter out some error messages
That we would rather not do - for our use it seems unnecessary and more complex
that just using the source files directly.

But it's easy enough to extract the common code into a helper function. PTAL

https://codereview.chromium.org/803653004/diff/20001/third_party/closure_comp...
third_party/closure_compiler/checker.py:317: sys.exit(exit)
On 2015/01/12 20:35:15, Tyler Breisacher (Chromium) wrote:
> It looks like 'exit' is no longer defined. Maybe just sys.exit(1) or
something.

Done.

[Tyler Breisacher (Chromium), 2015-01-13 03:01:01]

lgtm with nits, for checker.py -- I didn't really look at the rest very much.

https://codereview.chromium.org/803653004/diff/20001/remoting/webapp/crd/js/h...
File remoting/webapp/crd/js/host_daemon_facade.js (right):

https://codereview.chromium.org/803653004/diff/20001/remoting/webapp/crd/js/h...
remoting/webapp/crd/js/host_daemon_facade.js:108: reject(false);
On 2015/01/13 00:28:39, garykac wrote:
> On 2015/01/12 22:51:28, kelvinp wrote:
> > why false here?
> 
> reject() requires at least 1 argument(s) and no more than 1 argument(s)
> 
> The value 'false' is arbitrary, but it seemed better than true. Is something
> like '0' better here?

We actually changed this in
https://github.com/google/closure-compiler/commit/921c464b40f3068f17c4c8ab017...
so you could probably remove the 'false' (or null or whatever you decide to
pass) after the next time you bump the compiler version.

https://codereview.chromium.org/803653004/diff/60001/third_party/closure_comp...
File third_party/closure_compiler/checker.py (right):

https://codereview.chromium.org/803653004/diff/60001/third_party/closure_comp...
third_party/closure_compiler/checker.py:195: (_, stderr) =
runner_cmd.communicate()
nit: You don't need the parens here:

_, stderr = runner_cmd.communicate()

https://codereview.chromium.org/803653004/diff/60001/third_party/closure_comp...
third_party/closure_compiler/checker.py:243: (errors, stderr) =
self.run_js_check(self._expanded_file, depends)
same here

https://codereview.chromium.org/803653004/diff/60001/third_party/closure_comp...
third_party/closure_compiler/checker.py:269: (errors, stderr) =
self.run_js_check(sources)
and here

https://codereview.chromium.org/803653004/diff/60001/third_party/closure_comp...
third_party/closure_compiler/checker.py:320: print _
I think the convention is that '_' is only for unused variables. Since you're
using it, give it a name.

[garykac, 2015-01-13 18:11:28]

The CQ bit was checked by garykac@chromium.org

[garykac, 2015-01-13 18:11:36]

https://codereview.chromium.org/803653004/diff/60001/third_party/closure_comp...
File third_party/closure_compiler/checker.py (right):

https://codereview.chromium.org/803653004/diff/60001/third_party/closure_comp...
third_party/closure_compiler/checker.py:195: (_, stderr) =
runner_cmd.communicate()
On 2015/01/13 03:01:00, Tyler Breisacher (Chromium) wrote:
> nit: You don't need the parens here:
> 
> _, stderr = runner_cmd.communicate()

Done.

https://codereview.chromium.org/803653004/diff/60001/third_party/closure_comp...
third_party/closure_compiler/checker.py:243: (errors, stderr) =
self.run_js_check(self._expanded_file, depends)
On 2015/01/13 03:01:00, Tyler Breisacher (Chromium) wrote:
> same here

Done.

https://codereview.chromium.org/803653004/diff/60001/third_party/closure_comp...
third_party/closure_compiler/checker.py:269: (errors, stderr) =
self.run_js_check(sources)
On 2015/01/13 03:01:01, Tyler Breisacher (Chromium) wrote:
> and here

Done.

https://codereview.chromium.org/803653004/diff/60001/third_party/closure_comp...
third_party/closure_compiler/checker.py:320: print _
On 2015/01/13 03:01:00, Tyler Breisacher (Chromium) wrote:
> I think the convention is that '_' is only for unused variables. Since you're
> using it, give it a name.

Done.

[commit-bot: I haz the power, 2015-01-13 18:12:47]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/803653004/80001

[commit-bot: I haz the power, 2015-01-13 19:18:33]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-01-13 19:18:34]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[garykac, 2015-01-14 18:46:51]

The CQ bit was checked by garykac@chromium.org

[commit-bot: I haz the power, 2015-01-14 18:47:47]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/803653004/100001

[commit-bot: I haz the power, 2015-01-14 19:37:49]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-01-14 19:38:51]

Patchset 6 (id:??) landed as
https://crrev.com/23605febf70f2c6eb42322e2392522cbf0175b8a
Cr-Commit-Position: refs/heads/master@{#311531}

[Dan Beam, 2015-01-21 22:21:54]

dbeam@chromium.org changed reviewers:
+ dbeam@chromium.org

[Dan Beam, 2015-01-21 22:21:55]

did you see our compiled_resources.gyp files that let you do multiple file
compilation/stamps?  certainly seems like this CL adds complexity we've already
handled.

[Dan Beam, 2015-01-21 22:26:56]

https://codereview.chromium.org/803653004/diff/100001/third_party/closure_com...
File third_party/closure_compiler/checker.py (right):

https://codereview.chromium.org/803653004/diff/100001/third_party/closure_com...
third_party/closure_compiler/checker.py:180: def run_js_check(self, sources,
externs=None):
this argument is named externs...

https://codereview.chromium.org/803653004/diff/100001/third_party/closure_com...
third_party/closure_compiler/checker.py:185: if externs:
|externs| seems to always be passed.  can we make it unnamed or just remove the
if check?

https://codereview.chromium.org/803653004/diff/100001/third_party/closure_com...
third_party/closure_compiler/checker.py:243: errors, stderr =
self.run_js_check(self._expanded_file, depends)
..but run_js_check() is called here with "depends"

https://codereview.chromium.org/803653004/diff/100001/third_party/closure_com...
third_party/closure_compiler/checker.py:280: help="Process each source file
individually")
why can't we just use a list always and just pass 1 file if it's singular?

https://codereview.chromium.org/803653004/diff/100001/third_party/closure_com...
third_party/closure_compiler/checker.py:281:
single_file_group.add_argument("--no-single-file", dest="single_file",
this is a bad name

https://codereview.chromium.org/803653004/diff/100001/third_party/closure_com...
third_party/closure_compiler/checker.py:294:
parser.set_defaults(single_file=True, strict=False)
can't we annotate this in the add_argument() call itself?