Warnings in the Closure Library

[Michael Bolin]

As some of you have discovered, the number of issues in the Closure
Library as identified by the Closure Compiler (with all checks
enabled) is nonzero. As of last week, the list of warnings will be
pushed out with the Closure Library:

http://code.google.com/p/closure-library/source/detail?r=1340

As you can see, currently there are 169 warnings (almost half of them
appear to come from goog.net.xpc!). Over time, it would be great to
get that number to zero.

Patches from the community can help; however, sometimes it is not easy
to ingest a patch. For example, I recently introduced a small change
to the Closure Compiler externs to change the declared return type of
eval() from {Object} to {*}:

http://code.google.com/p/closure-compiler/source/detail?r=1473
http://code.google.com/p/closure-library/source/detail?r=1301

This caused a ton of problems inside Google because it was hard to
find all of the places where people were using eval() and were relying
on the return type being {Object}. That means that when this change
was checked in, it broke a number of builds and made some people
angry. (Yes, even though this makes the Library better/more accurate,
that doesn't really placate developers who just discovered their JS is
no longer compiling cleanly without any changes to their application
code.)

I bring this up because some of you may look at these warnings and
think, "That looks so trivial -- why can't they just go and fix it?"
Often it's because it may be hard to determine how many people depend
on something internally, so it can take a lot of time and energy
before a fix can be applied. Generally, at Google, if you change an
API, then you are responsible for changing all of the call sites of
that API, as well.

A simple example of this is goog.local.getLocale(). At first glance,
the warning says that it's deprecated and suggests an alternative, so
you might think it would be a drop-in replacement to fix the warning.
The problem is that localization is tricky, and if someone's
localization process inside Google depends on how things work as-is,
it would not be right to disrupt an entire product's push cycle just
to eliminate that warning. It's not that the warning will never get
fixed: it's just that it may not be a quick fix.

That said, I would love to see people try to pitch in and help fix
warnings. I just think you should be aware that you may propose a
patch that seems like a clean, simple solution from the outside,
though it might not be from the inside, in which case it may not be
able to be ingested.

--Michael

[jos...@gmail.com]

Thanks Michael.

Its kind of crappy if you can't iterate on the closure library because warnings break the build of some other google project. Using version numbers internally would let you fix that, but thats not your choice.

Maybe I'll just update my build script to strip the warnings it finds in the closure library when it builds. I'm nervous about that, but it sounds like there are never going to be any API-breaking changes in closure anyway.

Cheers

Joseph

[Guido Tapia]

Just write your own compiler pass and ignore the goog namespace. Their is actually a class that u can extend that does just that. Forgot the name and on mobile.

[Joseph Gentle]

Thanks, but I'm currently just using google's REST service for
building. I don't care enough to muck around in java.

-J

[John Lenz]

We can have different warning defaults internally to Google and externally (and do, in fact), the external version can be stricter.   There isn't much need to a custom compiler pass, although if you have something useful, it is nice if you share.

[Guido Tapia]

John,

Ave a look ere:

https://gist.github.com/1296502 

It wasn't a custom pass, its just a option in the compiler config object, very straight forward, basically:

  @Override

  protected CompilerOptions createOptions() {

    CompilerOptions options = super.createOptions();        

    options.setCodingConvention(new GoogleCodingConvention());    

    options.addWarningsGuard(new ShowByPathWarningsGuard("goog", ShowByPathWarningsGuard.ShowType.EXCLUDE));         

    options.addWarningsGuard(new ShowByPathWarningsGuard("externs.zip", ShowByPathWarningsGuard.ShowType.EXCLUDE));         

    return options;

  }

As a sidenote to the compiler devs, can we add a StatisticsGuard to the compiler, would love to see the compiler stats not being influenced by goog namespace.  By statistics I mean:

0 error(s), 1 warning(s), 97.6% typed

Tnx

[GordonHo]

good posting.

i stumbled across the same issue (and actually haven't resolved it).

perhaps i am not familiar enough with the compiler, but how could i suppress those warning?

regards gordon