Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

top-level functions pick up a tempCtor superclass #90

Closed
rkirov opened this issue Sep 8, 2015 · 8 comments
Closed

top-level functions pick up a tempCtor superclass #90

rkirov opened this issue Sep 8, 2015 · 8 comments
Assignees

Comments

@rkirov
Copy link
Contributor

rkirov commented Sep 8, 2015

The following simple definitions transpile to classes that extend tempCtor.

/**
 * To assert to the compiler that an operation is needed when it would
 * otherwise be stripped. For example:
 * <code>
 *     // Force a layout
 *     goog.reflect.sinkValue(dialog.offsetHeight);
 * </code>
 * @type {!Function}
 */
goog.reflect.sinkValue = function(x) {
  goog.reflect.sinkValue[' '](x);
  return x;
};

and

/**
 * @type {!Function}
 * @throws {Error} when invoked to indicate the method should be overridden.
 */
goog.abstractMethod = function() {
  throw Error('unimplemented abstract method');
};

outputs

class sinkValue extends tempCtor 
@alexeagle alexeagle self-assigned this Sep 8, 2015
@alexeagle
Copy link
Contributor

This seems like a bug in closure compiler. Looking into the type at runtime, it claims to have kind==CONSTRUCTOR when the @type is present. If it's removed, it goes back to kind==ORDINARY.

@alexeagle
Copy link
Contributor

Ben says
"/** @type {!Function} */ is new-able as well as callable, so in some
sense it's both a constructor and an ordinary function."

@rkirov
Copy link
Contributor Author

rkirov commented Sep 9, 2015

I didn't know that in closure all functions are newable. TS plain functions are not new-able, so we have to make it explicitly new-able with

interface ClosureFunction {
  (...args: any[]):any;
  new (...args: any[]): any;
}

var abstractMethod: ClosureFunction; 

@mprobst
Copy link
Contributor

mprobst commented Sep 9, 2015

In Closure, function() is a different type from Function. The first is a specific function that can only be called in a specific way, with particular arguments, e.g. function() being a function that takes no args and is void, function(string): number being a function taking a single string, returning a number. A function that's constructable but restricts it's arguments looks like this: function(new:Foo, number, string). Similarly, you can also specify function(this:SomeType).

Function is a weak type assertion that something is any subtype of Function, so could or could not be callable with new and/or any args. JSCompiler allows calling these in any way.

@alexeagle
Copy link
Contributor

The problem seems to be that the information was lost. Introspecting the type system tells us only that the thing is modelled as a class, but not whether it was declared with @constructor.
If we make all these things a class, they will appear to be newable but not callable.
Maybe we need to emit all of them as callable-newable like @rkirov wrote above.

@mprobst
Copy link
Contributor

mprobst commented Sep 9, 2015

Can we get to the JSDocInfo field? That has the @constructor thing in it, I'd hope.

Otherwise we could also just special case on tempCtor and fail if somebody really calls their super class tempCtor (which violates the style guide and common sense ;-)).

@rkirov
Copy link
Contributor Author

rkirov commented Sep 10, 2015

I am changing my original proposal, since this is a rare pattern and honestly new-able + callable sounds like trouble. I suggest ignoring the new-able semantics and just outputting it as var foo: Function; for now.

We have only seen 2 places where this is used (goog.reflect.sinkValue and goog.abstractMethod) and neither is very useful when called with new.

@mprobst
Copy link
Contributor

mprobst commented Sep 10, 2015

Yeah, this seems like a corner case, and as you can specify explicitly when something should be newable, the intent would probably never be to actually call new on one of these.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants