Skip to content

Dont look for properties of Object and Function type when looking to resolve named import from module with export= #37964

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

Merged
merged 6 commits into from
Nov 3, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 12 additions & 10 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2586,7 +2586,7 @@ namespace ts {
let symbolFromVariable: Symbol | undefined;
// First check if module was specified with "export=". If so, get the member from the resolved type
if (moduleSymbol && moduleSymbol.exports && moduleSymbol.exports.get(InternalSymbolName.ExportEquals)) {
symbolFromVariable = getPropertyOfType(getTypeOfSymbol(targetSymbol), name.escapedText);
symbolFromVariable = getPropertyOfType(getTypeOfSymbol(targetSymbol), name.escapedText, /*skipObjectFunctionPropertyAugment*/ true);
Copy link
Member

@weswigham weswigham May 5, 2020

Choose a reason for hiding this comment

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

Actually, rather than threading through this flag, and worrying about the safety of caching it and have two differently calculated symbols for a property lookup, couldn't we

  1. Initialize globalFunctionType and globalObjectType each to a unique empty type (which should naturally cause property lookups that ultimately may have depended on them to fail during global initialization).
  2. Check if symbolFromVariable.parent here is globalObjectType or globalFunctionType and, if so, discard the result? (to ensure consistency with the initialization phase)

Copy link
Member Author

@sheetalkamat sheetalkamat Jul 25, 2020

Choose a reason for hiding this comment

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

  1. We want this to happen irrespective of augmenting that means any time we import the properties from object/function should be error so this might not happen at augmentation time.
  2. Will there be some kind of edge case where in valid scenario return globalfunctionType property symbol. That is what if getTypeOfSymbol(targetSymbol) is globalFunctionType or globalObjectType

}
else {
symbolFromVariable = getPropertyOfVariable(targetSymbol, name.escapedText);
Expand Down Expand Up @@ -10432,7 +10432,7 @@ namespace ts {
t;
}

function createUnionOrIntersectionProperty(containingType: UnionOrIntersectionType, name: __String): Symbol | undefined {
function createUnionOrIntersectionProperty(containingType: UnionOrIntersectionType, name: __String, skipObjectFunctionPropertyAugment?: boolean): Symbol | undefined {
const propSet = createMap<Symbol>();
let indexTypes: Type[] | undefined;
const isUnion = containingType.flags & TypeFlags.Union;
Expand All @@ -10444,7 +10444,7 @@ namespace ts {
for (const current of containingType.types) {
const type = getApparentType(current);
if (!(type === errorType || type.flags & TypeFlags.Never)) {
const prop = getPropertyOfType(type, name);
const prop = getPropertyOfType(type, name, skipObjectFunctionPropertyAugment);
const modifiers = prop ? getDeclarationModifierFlagsFromSymbol(prop) : 0;
if (prop && !(modifiers & excludeModifiers)) {
if (isUnion) {
Expand Down Expand Up @@ -10550,20 +10550,21 @@ namespace ts {
// constituents, in which case the isPartial flag is set when the containing type is union type. We need
// these partial properties when identifying discriminant properties, but otherwise they are filtered out
// and do not appear to be present in the union type.
function getUnionOrIntersectionProperty(type: UnionOrIntersectionType, name: __String): Symbol | undefined {
function getUnionOrIntersectionProperty(type: UnionOrIntersectionType, name: __String, skipObjectFunctionPropertyAugment?: boolean): Symbol | undefined {
const properties = type.propertyCache || (type.propertyCache = createSymbolTable());
let property = properties.get(name);
if (!property) {
property = createUnionOrIntersectionProperty(type, name);
if (property) {
property = createUnionOrIntersectionProperty(type, name, skipObjectFunctionPropertyAugment);
// Dont set the result since it might not be correct if skipping lookup from property of Object and function type
if (property && !skipObjectFunctionPropertyAugment) {
properties.set(name, property);
}
}
return property;
}

function getPropertyOfUnionOrIntersectionType(type: UnionOrIntersectionType, name: __String): Symbol | undefined {
const property = getUnionOrIntersectionProperty(type, name);
function getPropertyOfUnionOrIntersectionType(type: UnionOrIntersectionType, name: __String, skipObjectFunctionPropertyAugment?: boolean): Symbol | undefined {
const property = getUnionOrIntersectionProperty(type, name, skipObjectFunctionPropertyAugment);
// We need to filter out partial properties in union types
return property && !(getCheckFlags(property) & CheckFlags.ReadPartial) ? property : undefined;
}
Expand Down Expand Up @@ -10641,14 +10642,15 @@ namespace ts {
* @param type a type to look up property from
* @param name a name of property to look up in a given type
*/
function getPropertyOfType(type: Type, name: __String): Symbol | undefined {
function getPropertyOfType(type: Type, name: __String, skipObjectFunctionPropertyAugment?: boolean): Symbol | undefined {
type = getApparentType(getReducedType(type));
if (type.flags & TypeFlags.Object) {
const resolved = resolveStructuredTypeMembers(<ObjectType>type);
const symbol = resolved.members.get(name);
if (symbol && symbolIsValue(symbol)) {
return symbol;
}
if (skipObjectFunctionPropertyAugment) return undefined;
const functionType = resolved === anyFunctionType ? globalFunctionType :
resolved.callSignatures.length ? globalCallableFunctionType :
resolved.constructSignatures.length ? globalNewableFunctionType :
Expand All @@ -10662,7 +10664,7 @@ namespace ts {
return getPropertyOfObjectType(globalObjectType, name);
}
if (type.flags & TypeFlags.UnionOrIntersection) {
return getPropertyOfUnionOrIntersectionType(<UnionOrIntersectionType>type, name);
return getPropertyOfUnionOrIntersectionType(<UnionOrIntersectionType>type, name, skipObjectFunctionPropertyAugment);
}
return undefined;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
tests/cases/compiler/bar.d.ts(1,10): error TS2305: Module '"./foo"' has no exported member 'Bar'.


==== tests/cases/compiler/foo.d.ts (0 errors) ====
export = Foo;
export as namespace Foo;

declare namespace Foo {
function foo();
}

declare global {
namespace Bar { }
}

==== tests/cases/compiler/bar.d.ts (1 errors) ====
import { Bar } from './foo';
~~~
!!! error TS2305: Module '"./foo"' has no exported member 'Bar'.
export = Bar;
export as namespace Bar;
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
=== tests/cases/compiler/foo.d.ts ===
export = Foo;
>Foo : Symbol(Foo, Decl(foo.d.ts, 1, 24))

export as namespace Foo;
>Foo : Symbol(Foo, Decl(foo.d.ts, 0, 13))

declare namespace Foo {
>Foo : Symbol(Foo, Decl(foo.d.ts, 1, 24))

function foo();
>foo : Symbol(foo, Decl(foo.d.ts, 3, 23))
}

declare global {
>global : Symbol(global, Decl(foo.d.ts, 5, 1))

namespace Bar { }
>Bar : Symbol(Bar, Decl(foo.d.ts, 7, 16))
}

=== tests/cases/compiler/bar.d.ts ===
import { Bar } from './foo';
>Bar : Symbol(Bar, Decl(bar.d.ts, 0, 8))

export = Bar;
>Bar : Symbol(Bar, Decl(bar.d.ts, 0, 8))

export as namespace Bar;
>Bar : Symbol(Bar, Decl(bar.d.ts, 1, 13))

Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
=== tests/cases/compiler/foo.d.ts ===
export = Foo;
>Foo : typeof Foo

export as namespace Foo;
>Foo : typeof Foo

declare namespace Foo {
>Foo : typeof Foo

function foo();
>foo : () => any
}

declare global {
>global : any

namespace Bar { }
}

=== tests/cases/compiler/bar.d.ts ===
import { Bar } from './foo';
>Bar : any

export = Bar;
>Bar : any

export as namespace Bar;
>Bar : any

37 changes: 37 additions & 0 deletions tests/baselines/reference/namedImportNonExistentName.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
tests/cases/compiler/bar.ts(1,10): error TS2305: Module '"./foo"' has no exported member 'Bar'.
tests/cases/compiler/bar.ts(1,15): error TS2305: Module '"./foo"' has no exported member 'toString'.
tests/cases/compiler/bar.ts(3,10): error TS2305: Module '"./foo2"' has no exported member 'a'.
tests/cases/compiler/bar.ts(3,13): error TS2305: Module '"./foo2"' has no exported member 'b'.
tests/cases/compiler/bar.ts(3,19): error TS2305: Module '"./foo2"' has no exported member 'd'.
tests/cases/compiler/bar.ts(3,22): error TS2305: Module '"./foo2"' has no exported member 'toString'.


==== tests/cases/compiler/foo.d.ts (0 errors) ====
export = Foo;
export as namespace Foo;

declare namespace Foo {
function foo();
}

==== tests/cases/compiler/foo2.ts (0 errors) ====
let x: { a: string; c: string; } | { b: number; c: number; };
export = x

==== tests/cases/compiler/bar.ts (6 errors) ====
import { Bar, toString, foo } from './foo';
~~~
!!! error TS2305: Module '"./foo"' has no exported member 'Bar'.
~~~~~~~~
!!! error TS2305: Module '"./foo"' has no exported member 'toString'.
foo();
import { a, b, c, d, toString as foo2String } from './foo2';
~
!!! error TS2305: Module '"./foo2"' has no exported member 'a'.
~
!!! error TS2305: Module '"./foo2"' has no exported member 'b'.
~
!!! error TS2305: Module '"./foo2"' has no exported member 'd'.
~~~~~~~~
!!! error TS2305: Module '"./foo2"' has no exported member 'toString'.
c;
31 changes: 31 additions & 0 deletions tests/baselines/reference/namedImportNonExistentName.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
//// [tests/cases/compiler/namedImportNonExistentName.ts] ////

//// [foo.d.ts]
export = Foo;
export as namespace Foo;

declare namespace Foo {
function foo();
}

//// [foo2.ts]
let x: { a: string; c: string; } | { b: number; c: number; };
export = x

//// [bar.ts]
import { Bar, toString, foo } from './foo';
foo();
import { a, b, c, d, toString as foo2String } from './foo2';
c;

//// [foo2.js]
"use strict";
var x;
module.exports = x;
//// [bar.js]
"use strict";
exports.__esModule = true;
var foo_1 = require("./foo");
foo_1.foo();
var foo2_1 = require("./foo2");
foo2_1.c;
44 changes: 44 additions & 0 deletions tests/baselines/reference/namedImportNonExistentName.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
=== tests/cases/compiler/foo.d.ts ===
export = Foo;
>Foo : Symbol(Foo, Decl(foo.d.ts, 1, 24))

export as namespace Foo;
>Foo : Symbol(Foo, Decl(foo.d.ts, 0, 13))

declare namespace Foo {
>Foo : Symbol(Foo, Decl(foo.d.ts, 1, 24))

function foo();
>foo : Symbol(foo, Decl(foo.d.ts, 3, 23))
}

=== tests/cases/compiler/foo2.ts ===
let x: { a: string; c: string; } | { b: number; c: number; };
>x : Symbol(x, Decl(foo2.ts, 0, 3))
>a : Symbol(a, Decl(foo2.ts, 0, 8))
>c : Symbol(c, Decl(foo2.ts, 0, 19))
>b : Symbol(b, Decl(foo2.ts, 0, 36))
>c : Symbol(c, Decl(foo2.ts, 0, 47))

export = x
>x : Symbol(x, Decl(foo2.ts, 0, 3))

=== tests/cases/compiler/bar.ts ===
import { Bar, toString, foo } from './foo';
>Bar : Symbol(Bar, Decl(bar.ts, 0, 8))
>toString : Symbol(toString, Decl(bar.ts, 0, 13))
>foo : Symbol(foo, Decl(bar.ts, 0, 23))

foo();
>foo : Symbol(foo, Decl(bar.ts, 0, 23))

import { a, b, c, d, toString as foo2String } from './foo2';
>a : Symbol(a, Decl(bar.ts, 2, 8))
>b : Symbol(b, Decl(bar.ts, 2, 11))
>c : Symbol(c, Decl(bar.ts, 2, 14))
>d : Symbol(d, Decl(bar.ts, 2, 17))
>foo2String : Symbol(foo2String, Decl(bar.ts, 2, 20))

c;
>c : Symbol(c, Decl(bar.ts, 2, 14))

46 changes: 46 additions & 0 deletions tests/baselines/reference/namedImportNonExistentName.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
=== tests/cases/compiler/foo.d.ts ===
export = Foo;
>Foo : typeof Foo

export as namespace Foo;
>Foo : typeof Foo

declare namespace Foo {
>Foo : typeof Foo

function foo();
>foo : () => any
}

=== tests/cases/compiler/foo2.ts ===
let x: { a: string; c: string; } | { b: number; c: number; };
>x : { a: string; c: string; } | { b: number; c: number; }
>a : string
>c : string
>b : number
>c : number

export = x
>x : { a: string; c: string; } | { b: number; c: number; }

=== tests/cases/compiler/bar.ts ===
import { Bar, toString, foo } from './foo';
>Bar : any
>toString : any
>foo : () => any

foo();
>foo() : any
>foo : () => any

import { a, b, c, d, toString as foo2String } from './foo2';
>a : any
>b : any
>c : string | number
>d : any
>toString : any
>foo2String : any

c;
>c : string | number

Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// @filename: foo.d.ts
export = Foo;
export as namespace Foo;

declare namespace Foo {
function foo();
}

declare global {
namespace Bar { }
}

// @filename: bar.d.ts
import { Bar } from './foo';
export = Bar;
export as namespace Bar;
17 changes: 17 additions & 0 deletions tests/cases/compiler/namedImportNonExistentName.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// @filename: foo.d.ts
export = Foo;
export as namespace Foo;

declare namespace Foo {
function foo();
}

// @filename: foo2.ts
let x: { a: string; c: string; } | { b: number; c: number; };
export = x

// @filename: bar.ts
import { Bar, toString, foo } from './foo';
foo();
import { a, b, c, d, toString as foo2String } from './foo2';
c;