google / closure-compiler

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

"non-existent type" warning on type only used in @return declaration #838

Closed myphysicslab closed 9 years ago

myphysicslab commented 9 years ago

With new type inference --new_type_inf, getting the warning "Type annotation references non-existent type" on a type that has not been goog.required, but is only used in a @return type declaration. Here is the error message:

src/lab/engine2D/RigidBody.js:625: WARNING - Type annotation references non-existent type myphysicslab.lab.engine2D.Edge.
@return {myphysicslab.lab.engine2D.Edge} the edge of this body that is at the given position in the list of edges
         ^

Here is some of that file, line 625 is the @return declaration.

/**
* @interface
* @extends {myphysicslab.lab.model.PhysicsObject}
*/
myphysicslab.lab.engine2D.RigidBody = function() {};
var RigidBody = myphysicslab.lab.engine2D.RigidBody;

/** Return the edge of this body that is at the given position in the list of edges.
@param {number} index  the position within the list of edges
@return {myphysicslab.lab.engine2D.Edge} the edge of this body that is at the given position in the list of edges
*/
RigidBody.prototype.getEdge;

In the book Closure, The Definitive Guide by Michael Bolin, p. 50 it is stated that this should be OK:

Note that goog.require() is not exactly the same as import in Java. In Closure, defining a function that takes a parameter of a particular type does not necessitate a goog.require() call to ensure the type has been loaded. For example, the following code does not require goog.math.Coordinate:

goog.provide('example.radius');
/**

* @param {number} radius

* @param {goog.math.Coordinate} point

* @return {boolean} whether point is within the specified radius
*/
example.radius.isWithinRadius = function(radius, point) {
  return Math.sqrt(point.x * point.x + point.y * point.y) <= radius;
};

If this is changing with new type inference, how can we deal with circular references like this?

compiler version:

Closure Compiler (http://github.com/google/closure-compiler)
Version: v20150126-82-g85da5b6
Built on: 2015/02/11 20:55
myphysicslab commented 9 years ago

Using goog.forwardDeclare() solves this issue.

dimvar commented 9 years ago

When compiling ths isWithinRadius code, do you compile it with any other file that defines goog.math.Coordinate?

If not, then yes, you have to use goog.forwardDeclare. Note that in that case, the only thing that NTI knows about Coordinate is that it's defined somewhere, and nothing else. So it doesn't do typechecking on uses of Coordinate. See this example:

http://closure-compiler-debugger.appspot.com/home#input0%3Dgoog.forwardDeclare('Foo')%253B%250A%250A%252F**%2520%2540param%2520%257BFoo%257D%2520x%2520*%252F%250Afunction%2520f(x)%2520%257B%257D%250A%250Af(123)%253B%2520%252F%252F%2520no%2520warning%26input1%26conformanceConfig%26externs%3D%252F**%2520%2540const%2520*%252F%250Avar%2520goog%2520%253D%2520%257B%257D%253B%250Agoog.forwardDeclare%253B%26refasterjs-template%26includeDefaultExterns%3D1%26CHECK_TYPES_NEW_INFERENCE%3D1

We're passing a number to the function that takes a Foo without a warning.

myphysicslab commented 9 years ago

The isWithinRadius is example code from Bolin's Closure book. In my code, yes the forward-declared file is being compiled as well. You are saying that anything forward-declared won't be type checked (in that file)?

I've refactored some of this code, a current example is:

file: AbstractEdge.js

goog.provide('myphysicslab.lab.engine2D.AbstractEdge');

goog.forwardDeclare('myphysicslab.lab.engine2D.Polygon');
goog.require('myphysicslab.lab.engine2D.Edge');
goog.require('myphysicslab.lab.engine2D.Vertex');
goog.require('myphysicslab.lab.engine2D.UtilEngine');
goog.require('myphysicslab.lab.util.UtilityCore');
goog.require('myphysicslab.lab.util.Vector');

/**
* @param {!myphysicslab.lab.engine2D.Polygon} body
* @param {!myphysicslab.lab.engine2D.Vertex} vertex1 the previous vertex, in body
  coords; matches the next (second) vertex of the previous edge
* @param {!myphysicslab.lab.engine2D.Vertex} vertex2 the next vertex, in body coords
* @constructor
* @struct
* @implements {myphysicslab.lab.engine2D.Edge}
*/
myphysicslab.lab.engine2D.AbstractEdge = function(body, vertex1, vertex2) {
  // code omitted...
  /**  the {Polygon} that this edge is a part of
  * @type {!myphysicslab.lab.engine2D.Polygon}
  * @protected
  */
  this.body_ = body;
};

I can't really avoid circular references among these classes (well... not easily). A Polygon has Edges and it's very handy for an Edge to know which Polygon it belongs to.

It's pretty unlikely that a wrong type of object would be passed in this case so it's not that worrisome, but it's good to know what is not being type-checked.

dimvar commented 9 years ago

You are saying that anything forward-declared won't be type checked (in that file)?

(I think that) if you include in the same compilation job the file that defines the forward-declared types, then you will get type checking as usual. You can try this to verify.

If you don't include the definition, then the forward-declared type name is treated as unknown.

myphysicslab commented 9 years ago

Confirmed: I just removed all the goog.forwardDeclare() statements and I no longer get the warning "Type annotation references non-existent type".

If you don't include the definition, then the forward-declared type name is treated as unknown.

This would only come up with modules, right? A situation where you are not compiling everything together. (A while back I tried and failed to understand how to do modules with closure-compiler).

dimvar commented 9 years ago

This would only come up with modules, right?

Yes, I think so.