allow unused private well-known symbol members

[Zzzen]

Fixes #42051

[andrewbranch]

It’s a little confusing to consider what it means for these members to be marked as private, but let’s think about the consequences:

1. Within the program, consumers can spread or for/of an instance of Unused with no problems, but they can’t directly access this member via element access.
2. Outside the program (i.e., consumers of the declaration emit as a library) can spread or for/of an instance of Unused, but they don’t know what type it yields. (The member declaration is preserved, but the type annotation is erased.)

I think because these members are to some degree usable by external consumers, we can’t ever mark them unused (at least, not if the class is exported or globally accessible).

[Zzzen]

2. Outside the program (i.e., consumers of the declaration emit as a library) can spread or for/of an instance of Unused, but they don’t know what type it yields. (The member declaration is preserved, but the type annotation is erased.)

🤔 Shouldn't symbol members behave like ordinary members? I don't know why not all following accesses report errors? Playground

class UsedOutsideClass {
    private *[Symbol.iterator](){ return ''; }
    private ['bar'] = () => {}
    private baz = () => {}
}

const foo = new UsedOutsideClass();
// No error ❓
foo[Symbol.iterator]()
// No error ❓
foo['bar']()
// Error ✅
foo.baz()

[andrewbranch]

Uhhh, that looks like a separate bug to me 😅

[andrewbranch]

The salient thing here isn’t the member declaration, it’s the access. foo.bar is an error but foo['bar'] is not. Anyway, don’t worry about that for this PR.

[andrewbranch]

#19335

[Zzzen]

😕 So we should simply ignore private Symbol.iterator/Symbol.asyncIterator when checking unused members?

[andrewbranch]

Yes, at least when the class is global or exported.

[Zzzen]

It occurs to me that maybe all well-known symbols should be ignored because they are designed to work implicitly and we have no practical way to track the usages.

class Foo {
  // Error: '[Symbol.toPrimitive]' is declared but its value is never read.
  private [Symbol.toPrimitive]() {
    return 1;
  }

  test() {
    // Logs 1
    console.log(String(this))
  }
}

[andrewbranch]

That seems plausible to me.

[andrewbranch]

This seems good to me. I’d like one more person to weigh in. @rbuckton @weswigham?

[weswigham]

I don't like this. We're able to do way better than syntactic specialization. Instead, check if the type of the name expression is the same as one of the members of the global SymbolConstructor interface.

[Zzzen]

We probably should not error here, but It is gonna take a lot of effort to fix.

[weswigham]

Actually, this should be easy to fix. Compare the type of the computer name, rather than the symbol. Every unique symbol has a unique type.

[Zzzen]

No, it is widened to an ordinary symbol.

https://ts-ast-viewer.com/#code/MYewdgzgLgBAllApgJwIZRMgygTwLYBGIANjALwy6EkB0CK6mA3AFCIAeADprMMahAgwAwjADeLGDE7I4AN3SIYAKgDa9NBmz4ixALoAKAJTiWkmAF8WFoA

[weswigham]

Hm, definitely a bug - I've opened #53276 to track it, since all members of the global SymbolConstructor are, in fact, declared as unique symbols nowadays.

[weswigham]

I think this is close to the right approach, with the changes I suggested, but is blocked on #53276 being fixed. I'd prefer we didn't have anything special for well-known symbols that doesn't apply to unique symbols in general, but relaxing usage checks for them, since they specify language-level protocol implementations, even when private, is a reasonable exception to the rule. (Though I could be wrong about that - someone may be equally sad that their private [myProtocolSymbol] method is marked as unused; it's just less common)

[typescript-bot]

With 6.0 out as the final release vehicle for this codebase, we're closing all PRs that don't fit the merge criteria for post-6.0 patches. If you think this was a mistake and this PR fits the post-6.0 patch criteria, please post to the 6.0 iteration issue with details (specifically, which PR and which patch criteria it satisfies).

Next steps for PRs:

• For crash bugfixes or language service improvements, PRs are currently accepted at the typescript-go repo
• Changes to type system behavior should wait until after 7.0, at which point mainline TypeScript development will resume in this repository with the Go codebase
• Library file updates (lib.d.ts etc) continue to live in this repo or the DOM Generator repo as appropriate