Skip to content

fix typeof import("path").Foo.#private #48763

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

fix typeof import("path").Foo.#private #48763

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 7 additions & 1 deletion src/compiler/parser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3817,7 +3817,13 @@ namespace ts {
assertions = parseImportTypeAssertions();
}
parseExpected(SyntaxKind.CloseParenToken);
const qualifier = parseOptional(SyntaxKind.DotToken) ? parseEntityNameOfTypeReference() : undefined;
const qualifier = parseOptional(SyntaxKind.DotToken)
? parseEntityName(
/*allowReservedWords*/ true,
/*allowPrivateIdentifiers*/ true,
Diagnostics.Type_expected
)
: undefined;
const typeArguments = parseTypeArgumentsOfTypeReference();
return finishNode(factory.createImportTypeNode(type, assertions, qualifier, typeArguments, isTypeOf), pos);
}
Expand Down
14 changes: 14 additions & 0 deletions tests/baselines/reference/privateNameInTypeQueryImport.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
tests/cases/conformance/classes/members/privateNames/main.ts(3,43): error TS2694: Namespace '"tests/cases/conformance/classes/members/privateNames/lib".Bar' has no exported member '#m'.


==== tests/cases/conformance/classes/members/privateNames/main.ts (1 errors) ====
export class Foo {
static #m = 1;
static #f(x: typeof import("./lib").Bar.#m) {}
~~
!!! error TS2694: Namespace '"tests/cases/conformance/classes/members/privateNames/lib".Bar' has no exported member '#m'.
Copy link
Contributor

@nicolo-ribaudo nicolo-ribaudo Apr 20, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

This error seems to be wrong, since Bar is Foo and thus it has the #m static field.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can't tell if it's completely correct or not.
In this PR, I convert SyntaxError into TypeError.
Maybe it should be counted as "external" access.

According to the latest nightly for TS, import(..).X.#priv is technically syntactically valid, but it will never be semantically valid because it counts as "external" access to the private member, which is disallowed.
...

Originally posted by @bradzacher in babel/babel#14454 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will try to add more test cases to determine which case is more reasonable and self-consistent.

Copy link
Member

Choose a reason for hiding this comment

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

I think a comparable way to test this would be to stick private onto the non-# one and see what happens.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think a comparable way to test this would be to stick private onto the non-# one and see what happens.

I didn't manage to do it.
At first I tried to alias type Bar = typeof import("./lib").Bar, then I found I cannot use Bar.#m.
typeof import("./lib").Bar.#m does not imply (typeof import("./lib").Bar).#m.
it means typeof (import("./lib").Bar.#m).

What do you think about it?

Copy link
Member

Choose a reason for hiding this comment

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

For private, I just meant to write something like:

// @strict: true
// @target: esnext

// @filename: main.ts
export class Foo {
  private static m = 1;
  static #f(x: typeof import("./lib").Bar.m) {}
}

// @filename: lib.ts
export { Foo as Bar } from "./main";

Where the static thing is "private" via TS, not via ECMAScript's private identifiers. I was replying on my phone so couldn't write it myself, sorry about that.

Copy link
Member

Choose a reason for hiding this comment

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

Regarding the precedence, that's #19707 (mentioned in #48640). All of this is getting really hairy and inconsistent, though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The AST should be changed. This PR is blocked by #48640.

}

==== tests/cases/conformance/classes/members/privateNames/lib.ts (0 errors) ====
export { Foo as Bar } from "./main";

19 changes: 19 additions & 0 deletions tests/baselines/reference/privateNameInTypeQueryImport.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
//// [tests/cases/conformance/classes/members/privateNames/privateNameInTypeQueryImport.ts] ////

//// [main.ts]
export class Foo {
static #m = 1;
static #f(x: typeof import("./lib").Bar.#m) {}
}

//// [lib.ts]
export { Foo as Bar } from "./main";


//// [lib.js]
export { Foo as Bar } from "./main";
//// [main.js]
export class Foo {
static #m = 1;
static #f(x) { }
}
18 changes: 18 additions & 0 deletions tests/baselines/reference/privateNameInTypeQueryImport.symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
=== tests/cases/conformance/classes/members/privateNames/main.ts ===
export class Foo {
>Foo : Symbol(Foo, Decl(main.ts, 0, 0))

static #m = 1;
>#m : Symbol(Foo.#m, Decl(main.ts, 0, 18))

static #f(x: typeof import("./lib").Bar.#m) {}
>#f : Symbol(Foo.#f, Decl(main.ts, 1, 16))
>x : Symbol(x, Decl(main.ts, 2, 12))
>Bar : Symbol(Bar, Decl(lib.ts, 0, 8))
}

=== tests/cases/conformance/classes/members/privateNames/lib.ts ===
export { Foo as Bar } from "./main";
>Foo : Symbol(Foo, Decl(main.ts, 0, 0))
>Bar : Symbol(Bar, Decl(lib.ts, 0, 8))

19 changes: 19 additions & 0 deletions tests/baselines/reference/privateNameInTypeQueryImport.types
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
=== tests/cases/conformance/classes/members/privateNames/main.ts ===
export class Foo {
>Foo : Foo

static #m = 1;
>#m : number
>1 : 1

static #f(x: typeof import("./lib").Bar.#m) {}
>#f : (x: any) => void
>x : any
>Bar : any
}

=== tests/cases/conformance/classes/members/privateNames/lib.ts ===
export { Foo as Bar } from "./main";
>Foo : typeof import("tests/cases/conformance/classes/members/privateNames/main").Foo
>Bar : typeof import("tests/cases/conformance/classes/members/privateNames/main").Foo

11 changes: 11 additions & 0 deletions tests/cases/conformance/classes/members/privateNames/privateNameInTypeQueryImport.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// @strict: true
// @target: esnext

// @filename: main.ts
export class Foo {
static #m = 1;
static #f(x: typeof import("./lib").Bar.#m) {}
}

// @filename: lib.ts
export { Foo as Bar } from "./main";