Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Leak of noImplicitAny checking with private static property #6415

Open
falsandtru opened this issue Jan 9, 2016 · 10 comments
Open

Leak of noImplicitAny checking with private static property #6415

falsandtru opened this issue Jan 9, 2016 · 10 comments
Labels
Bug A bug in TypeScript Help Wanted You can do this
Milestone

Comments

@falsandtru
Copy link
Contributor

declare class C {
  private static implicitAnyTypeProperty;
  static inherited: typeof C.implicitAnyTypeProperty;
  inherited: typeof C.implicitAnyTypeProperty;
}

expected:

$ node_modules/.bin/tsc index --noImplicitAny
index.ts(2,3): error TS7008: Member 'implicitAnyTypeProperty' implicitly has an 'any' type.

actual:

$ node_modules/.bin/tsc index --noImplicitAny
@DanielRosenwasser
Copy link
Member

@falsandtru with these issues you'll have to be specific about what the expected behavior is and what the current actual behavior is. Can you elaborate on that?

@falsandtru
Copy link
Contributor Author

Sorry, I updated the description.

@falsandtru falsandtru changed the title Leak of noImplicitAny checking Leak of noImplicitAny checking with private static property Jan 9, 2016
@DanielRosenwasser
Copy link
Member

So the reason we don't error on here with noImplicitAny is because in you're in an ambient context. Private members in classes are intended to be described as any because the idea is to hide implementation details, and if you check out our generated .d.ts files, you'll see that they we use any for private-declared properties.

@falsandtru
Copy link
Contributor Author

I know your idea. Private instance members in classes have inaccessibility.

declare class C {
  private implicitAnyTypeProperty; // not an error because this type is inaccessible
}

but private static members are not.

declare class C {
  private static implicitAnyTypeProperty;
  static inherited: typeof C.implicitAnyTypeProperty;
  inherited: typeof C.implicitAnyTypeProperty;
}
var D: typeof C;
//class D extends C { }
var a = D.inherited; // should be an error because this variable accessed to implicitAny type
var b = new D().inherited; // same as above

This accessibility seems not intended design because this accessibility of private static members escapes inaccessibility of private members (for instance). ImplicitAny type is visible from implementations. Indeed, a and b variables have implicitAny type. These implicitAny type should be an error. Is this really allowed, intended accessibility?

@mhegazy
Copy link
Contributor

mhegazy commented Jan 11, 2016

This is a good point @falsandtru. I do not think we thought of that before. enabling no-implicit-any checks on these declarations would be a breaking change now. Given that this pattern is not common, I wounder if it is worth it though.

@mhegazy mhegazy added Suggestion An idea for TypeScript In Discussion Not yet reached consensus Breaking Change Would introduce errors in existing code labels Jan 11, 2016
@falsandtru
Copy link
Contributor Author

The TypeScript type system policy is a balance, but no-implicit-any checks is probably not. It checks is desired to be perfect. I think that the next major version up to 2.0 is a good chance for breaking changes. If this pattern is not common as you say, this breaking change is easy. TypeScript can get more correct no-implicit-any checks easily.

@mhegazy
Copy link
Contributor

mhegazy commented Jan 12, 2016

Given that this pattern is not common,

i meant the use of a static property as a target of a typeof expression to type something else. The private static property is a common pattern, and making this an error would break a lot of code. for instance all the .d.ts files that were generated by the compiler would fail this check.

@RyanCavanaugh RyanCavanaugh added Help Wanted You can do this and removed Breaking Change Would introduce errors in existing code In Discussion Not yet reached consensus labels May 9, 2016
@RyanCavanaugh RyanCavanaugh added this to the Community milestone May 9, 2016
@RyanCavanaugh
Copy link
Member

We'd like it to be an error for a non-private property to use the typeof a private property.

@mhegazy mhegazy added Bug A bug in TypeScript and removed Suggestion An idea for TypeScript labels May 9, 2016
@RyanCavanaugh RyanCavanaugh modified the milestones: Community, Backlog Mar 7, 2019
@Andarist
Copy link
Contributor

Andarist commented Aug 4, 2023

We'd like it to be an error for a non-private property to use the typeof a private property.

Is this something that you still want? typeof returns the requested type eagerly~ so the result can usually be just inlined in the emitted .d.ts. Also, at that point, we are in the "type world" and not in the "runtime world". For example, this is how indexed access behaves today:

class A {
  private test = 10;
}

type Test = A["test"] // ok 

declare function fn<T extends A>(a: T): T["test"]; // Private or protected member 'test' cannot be accessed on a type parameter.(4105)

And this makes sense because, in the generic context, this indexed access is deferred and this wouldn't be OK. The simple non-generic context works fine though. Should the same principle be applied to the typeof query? Or should it behave differently?

@RyanCavanaugh
Copy link
Member

Maybe the best version of the rule is that typeof on an ambient private property is an error, since that will yield any even though the original type could have been anything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Help Wanted You can do this
Projects
None yet
Development

No branches or pull requests

5 participants