unique symbol type imported in declaration files used as computed property key can not be named

[mohsen1]

TypeScript Version: 3.8 and 3.9.2

This is a regression from 3.7.

Search Terms:
unique symbol, index signature

Code
This is a distilled code from a bigger codebase. The Op is imported from Sequelize

import Op from './op';

export default function foo() {
  return {
    [Op.or]: [],
  };
}

// op.ts
declare const Op: {
  readonly or: unique symbol;
};

export default Op;

tsconfig.json

{
  "compilerOptions": {
    "outDir": "dist",
    "target": "es5",
    "module": "commonjs",
    "declaration": true,
    "strict": true,
    "esModuleInterop": true
  }
}

Expected behavior:
No Error
Actual behavior:

index.ts:3:25 - error TS4058: Return type of exported function has or is using name 'or' from external module "/Users/mohsen_azimi/Desktop/3.9-regression/op" but cannot be named.

3 export default function foo() {
                          ~~~

Related Issues:
#9944

[andrewbranch]

@weswigham, I could use your advice on this one. From what I can tell, it was a bizarre happy accident that this ever worked correctly, and it was “broken” by correctly setting context.enclosingDeclaration prior to looking up the symbol chain. (Previously, since enclosingDeclaration was undefined, the symbol accessibility check short-circuited to “yep I guess it’s accessible,” and then the type node generation fell back to an identifier with getNameOfSymbolAsWritten→ [Op.or], so the declaration emit AST was technically invalid, but emitted to something valid.)

I found that I can fix this particular example by considering not just exports, but also members, of the Op symbol in getCandidateListForSymbol:

function getCandidateListForSymbol(symbolFromSymbolTable: Symbol, resolvedImportedSymbol: Symbol, ignoreQualification: boolean | undefined) {
    if (isAccessible(symbolFromSymbolTable, resolvedImportedSymbol, ignoreQualification)) {
        return [symbolFromSymbolTable];
    }

    // Look in the exported members, if we can find accessibleSymbolChain, symbol is accessible using this chain
    // but only if the symbolFromSymbolTable can be qualified
    const candidateTable = getExportsOfSymbol(resolvedImportedSymbol);
    const accessibleSymbolsFromExports = candidateTable && getAccessibleSymbolChainFromSymbolTable(candidateTable, /*ignoreQualification*/ true);
    if (accessibleSymbolsFromExports && canQualifySymbol(symbolFromSymbolTable, getQualifiedLeftMeaning(meaning))) {
        return [symbolFromSymbolTable].concat(accessibleSymbolsFromExports);
    }

+    const type = getTypeOfSymbol(resolvedImportedSymbol);
+    if (type.flags & TypeFlags.Object) {
+        const membersCandidateTable = resolveStructuredTypeMembers(type as ObjectType).members;
+        const accessibleSymbolsFromMembers = getAccessibleSymbolChainFromSymbolTable(membersCandidateTable, /*ignoreQualification*/ true);
+        if (accessibleSymbolsFromMembers && canQualifySymbol(symbolFromSymbolTable, getQualifiedLeftMeaning(meaning))) {
+            return [symbolFromSymbolTable].concat(accessibleSymbolsFromMembers);
+        }
+    }
}

However, that breaks a bunch of tests by eagerly selecting a much worse serialization when a better one would have eventually been found in another symbol table. It seems like I want something similar to this only after other attempts to find an accessible symbol chain have failed. But all this symbol accessibility stuff is a little opaque to me.

[weswigham]

If you want to add a lower priority container, getAlternativeContainersOfSymbol is, iirc, a way to do it (I think that's the name). That's how we deprioritize an export assigned namespace vs the file being assigned to. (Priority is implicit in the order of the list of outputs)

You're absolutely right that symbol visibility is a bit of a mess, though! It's also not terribly performant, as it's an uncached exhaustive search of reachable symbols! But it's also quite hard to replace, what with all the visibility rules encoded into it :S