google / closure-compiler

A JavaScript checker and optimizer.
https://developers.google.com/closure/compiler/
Apache License 2.0
7.36k stars 1.14k forks source link

Warning accessing protected properties in extern base class #1001

Open jklein24 opened 9 years ago

jklein24 commented 9 years ago

Here's a reduction of the problem:

Externs:

/** @constructor */
var Foo = function() {};
/** @protected */
Foo.prototype.doIt = function() {};

Main Module:

/**
 * @extends {Foo}
 * @constructor
 */
var Bar = function() {};

/** @this {Bar} */
var doThing = function() {
  this.doIt();
};

This produces the warning: WARNING - Access to protected property doIt of Bar not allowed here. this.doIt();

Some Notes: This doesn't occur if I move the base class into the main module. This doesn't occur if I use Bar.prototype.doThing instead of the @this annotation.

jklein24 commented 9 years ago

@MatrixFrog

concavelenz commented 9 years ago

Generally, a "this" annotation shouldn't give you access to a protected property unless the function definition is within a class method (a "static", a prototype method). That you don't get a warning in some cases would be a bug. For your case, would defining it as a static would be sufficient?

/** @this {Bar} */
Bar.doThing = function() {
  this.doIt();
};

Example:

http://closure-compiler-debugger.appspot.com/#input0%3D%252F**%250A%2520*%2520%2540extends%2520%257BFoo%257D%250A%2520*%2520%2540constructor%250A%2520*%252F%250Avar%2520Bar%2520%253D%2520function()%2520%257B%257D%253B%250A%250A%252F**%2520%2540this%2520%257BBar%257D%2520*%252F%250ABar.doThing%2520%253D%2520function()%2520%257B%250A%2520%2520this.doIt()%253B%250A%257D%253B%26input1%26conformanceConfig%26externs%3D%252F**%2520%2540constructor%2520*%252F%250Avar%2520Foo%2520%253D%2520function()%2520%257B%257D%253B%250A%252F**%2520%2540protected%2520*%252F%250AFoo.prototype.doIt%2520%253D%2520function()%2520%257B%257D%253B%26refasterjs-template%26includeDefaultExterns%3D1%26CHECK_SYMBOLS%3D1%26CHECK_TYPES%3D1%26CHECK_VISIBILITY%3D1%26MISSING_PROPERTIES%3D1

jklein24 commented 9 years ago

Thanks, John. That makes sense, but I guess I should take a step back and give a bit more context here because the real problem may be entirely different then. The generated code for a Polymer element is something like:

/**
 * @extends {Foo}
 * @constructor
 */
var Bar = function() {};
Polymer(/** @lends {Bar.prototype} */ {
  /** @this {Bar} */
  doThing: function() { this.doIt(); },
  ...
});

The @this is really just a workaround for the fact the lends doesn't give the right this context to functions on its own. In this case, these functions really should be "on the prototype". Perhaps the real solution here is to just fix the issues with lends.