Allow extending from any by sandersn · Pull Request #14935 · microsoft/TypeScript

sandersn · Mar 30, 2017

Extending from any adds an index signature: [s: string]: any to both the
instance and static sides of the class.

Extending from any adds an index signature: [s: string]: any to both the instance and static sides of the class.

Also improve how the string indexer for any-inheriting types is added.

alfaproject · Mar 31, 2017

+}
+
+let c = new C();
+c.known.length; // error, 'real' has no 'length' property


real / sreal

sandersn · Apr 5, 2017

@yuit @Andy-MS mind taking a look?

DanielRosenwasser · Apr 5, 2017

                    return type.resolvedBaseConstructorType = unknownType;
                }
-                if (baseConstructorType !== unknownType && baseConstructorType !== nullWideningType && !isConstructorType(baseConstructorType)) {
+                if (baseConstructorType !== anyType && baseConstructorType !== unknownType && baseConstructorType !== nullWideningType && !isConstructorType(baseConstructorType)) {


Hard for me to look up right now whether unknownType includes it, could you check for TypeFlags.Any?

Just check unknownType and anyType both have TypeFlags.Any

Includes anyType, autoType, unknownType. Seems like it should work.

yuit · Apr 5, 2017

        function isValidBaseType(type: Type): boolean {
-            return type.flags & (TypeFlags.Object | TypeFlags.NonPrimitive) && !isGenericMappedType(type) ||
+            return !!(type.flags & TypeFlags.Any) ||
+                type.flags & (TypeFlags.Object | TypeFlags.NonPrimitive) && !isGenericMappedType(type) ||


should this just be type.flags & (TypeFlags.Object | TypeFlags.NonPrimitive | TypeFlags.Any)

I think it should work, although it's kind of a weird pun in that we rely on !isGenericMappedType(anyType) being true.

yuit · Apr 5, 2017

                    callSignatures = concatenate(callSignatures, getSignaturesOfType(instantiatedBaseType, SignatureKind.Call));
                    constructSignatures = concatenate(constructSignatures, getSignaturesOfType(instantiatedBaseType, SignatureKind.Construct));
-                    stringIndexInfo = stringIndexInfo || getIndexInfoOfType(instantiatedBaseType, IndexKind.String);
+                    stringIndexInfo = stringIndexInfo || (instantiatedBaseType === anyType ?


nit: would it be easier to read this way:

if (!stringIndexInfo) { stringIndexInfo = instantiatedBaseType === anyType ? createIndexInfo(anyType, /*isReadonly*/ false) : getIndexInfoOfType(instantiatedBaseType, IndexKind.String); }

I guess so. Done.

ghost · Apr 5, 2017

I wonder if anything from #12352 could be simplified now.
At least the comment about getBaseConstructorTypeOfClass could be updated.

ghost · Apr 5, 2017

                baseType = getTypeFromClassOrInterfaceReference(baseTypeNode, baseConstructorType.symbol);
            }
+            else if (baseConstructorType.flags & TypeFlags.Any) {
+                baseType = anyType;


Could you leave a comment saying why it is necessary to set it to anyType instead of baseConstructorType?

There's no good reason; in fact it reduces test churn to return baseConstructorType for tests that extend undefined variables, which I think get unknownType instead of anyType, meaning that you get slightly more errors.

I'll change it to baseConstructorType.

Returns several baselines to their original states.

sandersn · Apr 5, 2017

@Andy-MS Maybe? I don't understand the differences between using unknownSymbol (which I guess has unknownType) for a missing module vs creating a synthetic symbol. Did the synthetic symbol get created solely for error reporting?

Seems like it would be worth reverting #12352 to see how it behaves, because I'm not able to predict much about module behaviour a priori.

ghost · Apr 5, 2017

OK, we can try reverting #12352 later.
But please update this comment:

// This must be different than unknownSymbol because getBaseConstructorTypeOfClass won't fail for unknownSymbol.

Extending symbols from untyped modules is no longer an error, so #12532 didn't get us anything except slightly better quick info.

sandersn · Apr 5, 2017

Reverting #12352 turned out to be straightforward, so I ended up merging that branch back into this PR.

ghost · Apr 5, 2017

                const name = specifier.propertyName || specifier.name;
                if (name.text) {
-                    if (isUntypedOrShorthandAmbientModuleSymbol(moduleSymbol)) {
+                    if (isShorthandAmbientModuleSymbol(moduleSymbol)) {


I like the old name better, even if it's a bit long.

Test asserts that unused locals error works for untyped modules. Comment no longer claims to check for untyped modules.

sandersn added 5 commits March 29, 2017 15:29

Allow extending from any

f1c1969

Extending from any adds an index signature: [s: string]: any to both the instance and static sides of the class.

Improve handling of any as a base type

765ac7c

Also improve how the string indexer for any-inheriting types is added.

Test: extending from any

650cc29

Update baselines

3624417

Re-swap mistakenly swapped lines of code

41eabbc

msftclas added the cla-not-required label Mar 30, 2017

alfaproject reviewed Mar 31, 2017

View reviewed changes

sandersn assigned yuit and ghost Apr 5, 2017

Update test comments to match property names

e09533d

DanielRosenwasser reviewed Apr 5, 2017

View reviewed changes

Check TypeFlags.Any instead of anyType&unknownType

7f6c49e

yuit reviewed Apr 5, 2017

View reviewed changes

ghost reviewed Apr 5, 2017

View reviewed changes

sandersn added 2 commits April 5, 2017 14:41

A few changes requested by review comments

75f1774

Unknown bases return baseConstructorType, not anyType

7e3c033

Returns several baselines to their original states.

yuit approved these changes Apr 5, 2017

View reviewed changes

Update comment

441e54c

sandersn mentioned this pull request Apr 5, 2017

Synthetic symbol introduced by #12352 is not needed when extending from any is allowed #15040

Closed

Remove special symbol for untyped modules

90d5c29

Extending symbols from untyped modules is no longer an error, so #12532 didn't get us anything except slightly better quick info.

ghost reviewed Apr 5, 2017

View reviewed changes

Improve test and comments

f635042

Test asserts that unused locals error works for untyped modules. Comment no longer claims to check for untyped modules.

ghost approved these changes Apr 6, 2017

View reviewed changes

sandersn merged commit 3029b8f into master Apr 6, 2017

sandersn deleted the allow-extending-from-any branch April 6, 2017 16:18

ghost mentioned this pull request Jul 8, 2017

Can't call 'super' when extending from 'any' #17032

Closed

microsoft locked and limited conversation to collaborators Jun 21, 2018

typescript-bot unassigned yuit Jun 21, 2018

Search code, repositories, users, issues, pull requests...

Conversation

sandersn commented Mar 30, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sandersn commented Apr 5, 2017

Uh oh!

DanielRosenwasser Apr 5, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yuit Apr 5, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghost commented Apr 5, 2017 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sandersn commented Apr 5, 2017

Uh oh!

ghost commented Apr 5, 2017

Uh oh!

sandersn commented Apr 5, 2017

Uh oh!

ghost Apr 5, 2017 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sandersn commented Mar 30, 2017 •

edited

Loading

DanielRosenwasser Apr 5, 2017 •

edited

Loading

yuit Apr 5, 2017 •

edited

Loading

ghost commented Apr 5, 2017 •

edited by ghost

Loading

ghost Apr 5, 2017 •

edited by ghost

Loading