Skip to content

Allow extending any, without noImplicitAny errors#23153

Merged
sandersn merged 3 commits intomasterfrom
allow-extending-any
Apr 5, 2018
Merged

Allow extending any, without noImplicitAny errors#23153
sandersn merged 3 commits intomasterfrom
allow-extending-any

Conversation

@sandersn
Copy link
Copy Markdown
Member

@sandersn sandersn commented Apr 4, 2018

Whenever there is a reference to super, or a reference to a property on this that is unknown, you will get an implicit any error. Previously you would only an error on calling super(), but it always appeared, not just with --noImplicitAny.

After discussing it with @weswigham and @rbuckton, I decided that an assertion escape hatch for super was too problematic to add.

Edit: After further discussion, we decided to always allow extending any.

Fixes #22443

Whenever there is a reference to super, or a reference to a property on
`this` that is unknown, you will get an implicit any error.
@sandersn sandersn requested review from mhegazy and weswigham April 4, 2018 22:10
Comment thread src/compiler/checker.ts Outdated
const type = nodeCheckFlag === NodeCheckFlags.SuperStatic
? getBaseConstructorTypeOfClass(classType)
: getTypeWithThisArgument(baseClassType, classType.thisType);
if (noImplicitAny && type.flags & TypeFlags.Any) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should probably use the helper isTypeAny.

Comment thread src/compiler/checker.ts Outdated
const type = nodeCheckFlag === NodeCheckFlags.SuperStatic
? getBaseConstructorTypeOfClass(classType)
: getTypeWithThisArgument(baseClassType, classType.thisType);
if (noImplicitAny && type.flags & TypeFlags.Any) {
Copy link
Copy Markdown
Contributor

@mhegazy mhegazy Apr 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we do allow you to call a value of type any with no errors under --noImplcitAny, not sure why super would be different..
it seems to me that the error should be in only one place, the extends clause.. since this is the "source" of all the anys that will come later on.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is it the source? If you write

declare var Base: any;
class Q extends Base {
  constructor() {
    super();
  }
}

tbqh, none of that should be implicitly any. It's quite explicitly any.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

then we do not need an error altogether..

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our methodology here was to issue one error per declaration, and not per use.. that is why you only get one error on:

var a;  // error
var b = a();  // No error here
var c = a.b.c; // no error here

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in this PR we are making every reference to super/ this an error.. do not think ppl need that much nagging.. they get it, it is any.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After some discussion we decided to drop the error. In normal javascript usage, as soon as you turn on --noImplicitAny you will get an error on untyped modules, after which you will have to assume that anything from that module is also of type any.

Copy link
Copy Markdown
Member

@weswigham weswigham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a version of classExtendingAny.ts with noImplicitAny: false just to verify that these errors do not appear when it's off?

@sandersn sandersn changed the title Allow extending any, with noImplicitAny errors Allow extending any, without noImplicitAny errors Apr 5, 2018
@sandersn sandersn merged commit 154ac34 into master Apr 5, 2018
@sandersn sandersn deleted the allow-extending-any branch April 5, 2018 15:53
@microsoft microsoft locked and limited conversation to collaborators Jul 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants