Skip to content

Deduplicate protocol.ts content #57361

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 9 commits into from
Mar 4, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
109 changes: 90 additions & 19 deletions scripts/dtsBundler.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -214,28 +214,35 @@ function nodeToLocation(node) {

/**
* @param {ts.Node} node
* @param {boolean} needExportModifier
* @returns {ts.Node | undefined}
*/
function removeDeclareConstExport(node) {
function removeDeclareConstExport(node, needExportModifier) {
switch (node.kind) {
case ts.SyntaxKind.DeclareKeyword: // No need to emit this in d.ts files.
case ts.SyntaxKind.ConstKeyword: // Remove const from const enums.
case ts.SyntaxKind.ExportKeyword: // No export modifier; we are already in the namespace.
return undefined;
case ts.SyntaxKind.ExportKeyword: // No export modifier; we are already in the namespace.
if (!needExportModifier) {
return undefined;
}
}
return node;
}

/** @type {Map<string, ts.Symbol>[]} */
/** @type {{ locals: Map<string, { symbol: ts.Symbol, writeTarget: WriteTarget }>, exports: Map<string, ts.Symbol>}[]} */
const scopeStack = [];

/** @type {Map<ts.Symbol, string>} */
const symbolToNamespace = new Map();

/**
* @param {string} name
*/
function findInScope(name) {
for (let i = scopeStack.length - 1; i >= 0; i--) {
const scope = scopeStack[i];
const symbol = scope.get(name);
const symbol = scope.exports.get(name);
if (symbol) {
return symbol;
}
Expand Down Expand Up @@ -290,8 +297,9 @@ function symbolsConflict(s1, s2) {

/**
* @param {ts.Statement} decl
* @param {boolean} isInternal
*/
function verifyMatchingSymbols(decl) {
function verifyMatchingSymbols(decl, isInternal) {
ts.visitEachChild(decl, /** @type {(node: ts.Node) => ts.Node} */ function visit(node) {
if (ts.isIdentifier(node) && ts.isPartOfTypeNode(node)) {
if (ts.isQualifiedName(node.parent) && node !== node.parent.left) {
Expand All @@ -310,6 +318,10 @@ function verifyMatchingSymbols(decl) {
}
const symbolInScope = findInScope(symbolOfNode.name);
if (!symbolInScope) {
if (symbolOfNode.declarations?.every(d => isLocalDeclaration(d) && d.getSourceFile() === decl.getSourceFile()) && !isSelfReference(node, symbolOfNode)) {
// The symbol is a local that needs to be copied into the scope.
scopeStack[scopeStack.length - 1].locals.set(symbolOfNode.name, { symbol: symbolOfNode, writeTarget: isInternal ? WriteTarget.Internal : WriteTarget.Both });
Copy link
Member

Choose a reason for hiding this comment

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

It seems moderately weird for this verification function to modify the locals, but not sure what's better.

Copy link
Member Author

Choose a reason for hiding this comment

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

I feel like I considered either renaming the function or returning the set of new locals to be appended before I paused this work.

Copy link
Member Author

Choose a reason for hiding this comment

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

@jakebailey any more thoughts on this? I agree this code isn’t great. Otherwise I think the current state is ready for review.

Copy link
Member

Choose a reason for hiding this comment

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

No, not really; I think it's fine for now. Thankfully I don't think the bundler has that high of a cognitive load nor changes that much 😄

}
// We didn't find the symbol in scope at all. Just allow it and we'll fail at test time.
return node;
Comment on lines 325 to 326
Copy link
Member

Choose a reason for hiding this comment

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

Maybe not for this PR, but I do feel a bit like I should have originally just made this error.

}
Expand All @@ -323,39 +335,72 @@ function verifyMatchingSymbols(decl) {
}, /*context*/ undefined);
}

/**
* @param {ts.Declaration} decl
*/
function isLocalDeclaration(decl) {
return ts.canHaveModifiers(decl)
&& !ts.getModifiers(decl)?.some(m => m.kind === ts.SyntaxKind.ExportKeyword)
&& !!getDeclarationStatement(decl);
}

/**
* @param {ts.Node} reference
* @param {ts.Symbol} symbol
*/
function isSelfReference(reference, symbol) {
return symbol.declarations?.every(parent => ts.findAncestor(reference, p => p === parent));
}

/**
* @param {string} name
* @param {string} parent
* @param {boolean} needExportModifier
* @param {ts.Symbol} moduleSymbol
*/
function emitAsNamespace(name, moduleSymbol) {
function emitAsNamespace(name, parent, moduleSymbol, needExportModifier) {
assert(moduleSymbol.flags & ts.SymbolFlags.ValueModule, "moduleSymbol is not a module");

scopeStack.push(new Map());
const fullName = parent ? `${parent}.${name}` : name;

scopeStack.push({ locals: new Map(), exports: new Map() });
const currentScope = scopeStack[scopeStack.length - 1];

const target = containsPublicAPI(moduleSymbol) ? WriteTarget.Both : WriteTarget.Internal;

if (name === "ts") {
// We will write `export = ts` at the end.
assert(!needExportModifier, "ts namespace should not have an export modifier");
write(`declare namespace ${name} {`, target);
}
else {
// No export modifier; we are already in the namespace.
write(`namespace ${name} {`, target);
write(`${needExportModifier ? "export " : ""}namespace ${name} {`, target);
}
increaseIndent();

const moduleExports = typeChecker.getExportsOfModule(moduleSymbol);
for (const me of moduleExports) {
currentScope.set(me.name, me);
currentScope.exports.set(me.name, me);
symbolToNamespace.set(me, fullName);
}

/** @type {[ts.Statement, ts.SourceFile, WriteTarget][]} */
const exportedStatements = [];
/** @type {[name: string, fullName: string, moduleSymbol: ts.Symbol][]} */
const nestedNamespaces = [];
for (const me of moduleExports) {
assert(me.declarations?.length);

if (me.flags & ts.SymbolFlags.Alias) {
const resolved = typeChecker.getAliasedSymbol(me);
emitAsNamespace(me.name, resolved);
if (resolved.flags & ts.SymbolFlags.ValueModule) {
nestedNamespaces.push([me.name, fullName, resolved]);
}
else {
const namespaceName = symbolToNamespace.get(resolved);
assert(namespaceName, `Failed to find namespace for ${me.name} at ${nodeToLocation(me.declarations[0])}`);
write(`export import ${me.name} = ${namespaceName}.${me.name}`, target);
}
continue;
}

Expand All @@ -367,34 +412,60 @@ function emitAsNamespace(name, moduleSymbol) {
fail(`Unhandled declaration for ${me.name} at ${nodeToLocation(decl)}`);
}

verifyMatchingSymbols(statement);

const isInternal = ts.isInternalDeclaration(statement);
if (!ts.isModuleDeclaration(decl)) {
verifyMatchingSymbols(statement, isInternal);
}

if (!isInternal) {
const publicStatement = ts.visitEachChild(statement, node => {
// No @internal comments in the public API.
if (ts.isInternalDeclaration(node)) {
return undefined;
}
return removeDeclareConstExport(node);
return node;
}, /*context*/ undefined);

writeNode(publicStatement, sourceFile, WriteTarget.Public);
exportedStatements.push([publicStatement, sourceFile, WriteTarget.Public]);
}

const internalStatement = ts.visitEachChild(statement, removeDeclareConstExport, /*context*/ undefined);

writeNode(internalStatement, sourceFile, WriteTarget.Internal);
exportedStatements.push([statement, sourceFile, WriteTarget.Internal]);
}
}

const childrenNeedExportModifier = !!currentScope.locals.size;

nestedNamespaces.forEach(namespace => emitAsNamespace(...namespace, childrenNeedExportModifier));

currentScope.locals.forEach(({ symbol, writeTarget }) => {
symbol.declarations?.forEach(decl => {
// We already checked that getDeclarationStatement(decl) works for each declaration.
const statement = getDeclarationStatement(decl);
writeNode(/** @type {ts.Statement} */ (statement), decl.getSourceFile(), writeTarget);
});
});

exportedStatements.forEach(([statement, ...rest]) => {
let updated = ts.visitEachChild(statement, node => removeDeclareConstExport(node, childrenNeedExportModifier), /*context*/ undefined);
if (childrenNeedExportModifier && ts.canHaveModifiers(updated) && !updated.modifiers?.some(m => m.kind === ts.SyntaxKind.ExportKeyword)) {
updated = ts.factory.replaceModifiers(
updated,
[
ts.factory.createModifier(ts.SyntaxKind.ExportKeyword),
.../**@type {ts.NodeArray<ts.Modifier> | undefined}*/ (updated.modifiers) ?? [],
],
);
}
writeNode(updated, ...rest);
});

scopeStack.pop();

decreaseIndent();
write(`}`, target);
}

emitAsNamespace("ts", moduleSymbol);
emitAsNamespace("ts", "", moduleSymbol, /*needExportModifier*/ false);

write("export = ts;", WriteTarget.Both);

Expand Down
96 changes: 96 additions & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9845,13 +9845,49 @@ export interface CommentDirectivesMap {
export interface UserPreferences {
readonly disableSuggestions?: boolean;
readonly quotePreference?: "auto" | "double" | "single";
/**
* If enabled, TypeScript will search through all external modules' exports and add them to the completions list.
* This affects lone identifier completions but not completions on the right hand side of `obj.`.
*/
readonly includeCompletionsForModuleExports?: boolean;
/**
* Enables auto-import-style completions on partially-typed import statements. E.g., allows
* `import write|` to be completed to `import { writeFile } from "fs"`.
*/
readonly includeCompletionsForImportStatements?: boolean;
/**
* Allows completions to be formatted with snippet text, indicated by `CompletionItem["isSnippet"]`.
*/
readonly includeCompletionsWithSnippetText?: boolean;
/**
* Unless this option is `false`, or `includeCompletionsWithInsertText` is not enabled,
* member completion lists triggered with `.` will include entries on potentially-null and potentially-undefined
* values, with insertion text to replace preceding `.` tokens with `?.`.
*/
readonly includeAutomaticOptionalChainCompletions?: boolean;
/**
* If enabled, the completion list will include completions with invalid identifier names.
* For those entries, The `insertText` and `replacementSpan` properties will be set to change from `.x` property access to `["x"]`.
*/
readonly includeCompletionsWithInsertText?: boolean;
/**
* If enabled, completions for class members (e.g. methods and properties) will include
* a whole declaration for the member.
* E.g., `class A { f| }` could be completed to `class A { foo(): number {} }`, instead of
* `class A { foo }`.
*/
readonly includeCompletionsWithClassMemberSnippets?: boolean;
/**
* If enabled, object literal methods will have a method declaration completion entry in addition
* to the regular completion entry containing just the method name.
* E.g., `const objectLiteral: T = { f| }` could be completed to `const objectLiteral: T = { foo(): void {} }`,
* in addition to `const objectLiteral: T = { foo }`.
*/
readonly includeCompletionsWithObjectLiteralMethodSnippets?: boolean;
/**
* Indicates whether {@link CompletionEntry.labelDetails completion entry label details} are supported.
* If not, contents of `labelDetails` may be included in the {@link CompletionEntry.name} property.
*/
readonly useLabelDetailsInCompletionEntries?: boolean;
readonly allowIncompleteCompletions?: boolean;
readonly importModuleSpecifierPreference?: "shortest" | "project-relative" | "relative" | "non-relative";
Expand All @@ -9874,14 +9910,74 @@ export interface UserPreferences {
readonly allowRenameOfImportPath?: boolean;
readonly autoImportFileExcludePatterns?: string[];
readonly preferTypeOnlyAutoImports?: boolean;
/**
* Indicates whether imports should be organized in a case-insensitive manner.
*/
readonly organizeImportsIgnoreCase?: "auto" | boolean;
/**
* Indicates whether imports should be organized via an "ordinal" (binary) comparison using the numeric value
* of their code points, or via "unicode" collation (via the
* [Unicode Collation Algorithm](https://unicode.org/reports/tr10/#Scope)) using rules associated with the locale
* specified in {@link organizeImportsCollationLocale}.
*
* Default: `"ordinal"`.
*/
readonly organizeImportsCollation?: "ordinal" | "unicode";
/**
* Indicates the locale to use for "unicode" collation. If not specified, the locale `"en"` is used as an invariant
* for the sake of consistent sorting. Use `"auto"` to use the detected UI locale.
*
* This preference is ignored if {@link organizeImportsCollation} is not `"unicode"`.
*
* Default: `"en"`
*/
readonly organizeImportsLocale?: string;
/**
* Indicates whether numeric collation should be used for digit sequences in strings. When `true`, will collate
* strings such that `a1z < a2z < a100z`. When `false`, will collate strings such that `a1z < a100z < a2z`.
*
* This preference is ignored if {@link organizeImportsCollation} is not `"unicode"`.
*
* Default: `false`
*/
readonly organizeImportsNumericCollation?: boolean;
/**
* Indicates whether accents and other diacritic marks are considered unequal for the purpose of collation. When
* `true`, characters with accents and other diacritics will be collated in the order defined by the locale specified
* in {@link organizeImportsCollationLocale}.
*
* This preference is ignored if {@link organizeImportsCollation} is not `"unicode"`.
*
* Default: `true`
*/
readonly organizeImportsAccentCollation?: boolean;
/**
* Indicates whether upper case or lower case should sort first. When `false`, the default order for the locale
* specified in {@link organizeImportsCollationLocale} is used.
*
* This preference is ignored if {@link organizeImportsCollation} is not `"unicode"`. This preference is also
* ignored if we are using case-insensitive sorting, which occurs when {@link organizeImportsIgnoreCase} is `true`,
* or if {@link organizeImportsIgnoreCase} is `"auto"` and the auto-detected case sensitivity is determined to be
* case-insensitive.
*
* Default: `false`
*/
readonly organizeImportsCaseFirst?: "upper" | "lower" | false;
/**
* Indicates where named type-only imports should sort. "inline" sorts named imports without regard to if the import is
* type-only.
*
* Default: `last`
*/
readonly organizeImportsTypeOrder?: "first" | "last" | "inline";
/**
* Indicates whether to exclude standard library and node_modules file symbols from navTo results.
*/
readonly excludeLibrarySymbolsInNavTo?: boolean;
readonly lazyConfiguredProjectsFromExternalProject?: boolean;
readonly displayPartsForJSDoc?: boolean;
readonly generateReturnInDocTemplate?: boolean;
readonly disableLineTextInReferences?: boolean;
}

/** Represents a bigint literal value without requiring bigint support */
Expand Down
Loading