clutz icon indicating copy to clipboard operation
clutz copied to clipboard

Type of implicit class property is any.

Open bowenni opened this issue 7 years ago • 8 comments

A very common pattern I saw from the recent migrations:

class A {
  /** @param {boolean}  p */
  constructor(p) {
    /** @private */
    this.p_ = p;
  }
}

This code is converted to:

class A {
  private p_: any;
  constructor(p: boolean) {
    this.p_ = p;
  }
}

Would it make sense to emit this instead?

class A {
  private p_: boolean;
  constructor(p: boolean) {
    this.p_ = p;
  }
}

The renaming the parameter property collapsing is left to TSLint.

bowenni avatar Dec 29 '17 04:12 bowenni

That seems useful!

mprobst avatar Dec 29 '17 17:12 mprobst

Clutz only sees the types that Closure sees.

I tried the below program, which suggests that Closure thinks this.p_ really is any.

class A {
  /** @param {boolean}  p */
  constructor(p) {
    /** @private */
    this.p_ = p;
  }

  foo() {
    this.p_ = 'hi';
  }
}

new A(true).foo();

evmar avatar Jan 02 '18 21:01 evmar

Yes this.p_ is indeed any.

But we can refer the type from the constructor parameters. Since constructor(p: boolean) and this.p_ = p we can guess that this.p_ is also a boolean.

bowenni avatar Jan 02 '18 22:01 bowenni

iirc, you need @const for Closure to infer this for you.

On Tue, Jan 2, 2018, 1:21 PM Evan Martin [email protected] wrote:

Clutz only sees the types that Closure ses.

I tried the below program, which suggests that Closure thinks this.p_ really is any.

class A { /** @param {boolean} p / constructor(p) { /* @private */ this.p_ = p; }

foo() { this.p_ = 'hi'; } }

new A(true).foo();

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/angular/clutz/issues/645#issuecomment-354878238, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAn6ACZy-tZOjAiWdJdV-fiebgYDpsMks5tGp3DgaJpZM4RO27Y .

dpurp avatar Jan 02 '18 22:01 dpurp

Adding a @const makes it private readonly p_: any;. The type is still any, I think.

bowenni avatar Jan 02 '18 22:01 bowenni

I guess, my point is if we start inferring types beyond what Closure does we're moving into the space of implementing our own impl of a type system. I wonder if we could instead get the user to fix their code to have types. For example, isn't there some noImplicitAny-equivalent flag for Closure code? We could ask users to turn that on before migration.

evmar avatar Jan 02 '18 22:01 evmar

There are a few options with js-conformance, and they are suggested as part of the migration workflow. I suspect that most teams want to skip the pre-work and go straight to TypeScript, instead of fixing the errors in Closure-land.

dpurp avatar Jan 03 '18 05:01 dpurp

I think the problem is that gents does not run type checking or type inference at all. That's a conscious choice - you can (could?) only run type checking after es5 down translation, which we didn't want to do for more fidelity with the original code.

mprobst avatar Jan 03 '18 06:01 mprobst