development/vscode
development/vscode
1
0
Fork 0
mirror of https://github.com/Microsoft/vscode synced 2024-08-28 13:29:44 +00:00
Code Issues Packages Projects Releases Wiki Activity

Enforce ILocalizedString usage with f1 property via types (#162991)

Browse source 
* Enforce ILocalizedString usage with f1 property via types

* complete comment

* Visit all symbols when encountering a union type

* use ILocalizedString

* fix tests

Co-authored-by: Alex Dima <alexdima@microsoft.com>
This commit is contained in:
Tyler James Leonhardt 2022-10-11 21:24:43 -07:00 committed by GitHub
parent 24b8eb0d54
commit 3e4e351816
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 183 additions and 122 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

116
build/lib/treeshaking.js
View file

@ -490,54 +490,56 @@ function markNodes(ts, languageService, options) {
}
const nodeSourceFile = node.getSourceFile();
const loop = (node) => {
const [symbol, symbolImportNode] = getRealNodeSymbol(ts, checker, node);
if (symbolImportNode) {
setColor(symbolImportNode, 2 /* NodeColor.Black */);
const importDeclarationNode = findParentImportDeclaration(symbolImportNode);
if (importDeclarationNode && ts.isStringLiteral(importDeclarationNode.moduleSpecifier)) {
enqueueImport(importDeclarationNode, importDeclarationNode.moduleSpecifier.text);
const symbols = getRealNodeSymbol(ts, checker, node);
for (const { symbol, symbolImportNode } of symbols) {
if (symbolImportNode) {
setColor(symbolImportNode, 2 /* NodeColor.Black */);
const importDeclarationNode = findParentImportDeclaration(symbolImportNode);
if (importDeclarationNode && ts.isStringLiteral(importDeclarationNode.moduleSpecifier)) {
enqueueImport(importDeclarationNode, importDeclarationNode.moduleSpecifier.text);
}
}
}
if (isSymbolWithDeclarations(symbol) && !nodeIsInItsOwnDeclaration(nodeSourceFile, node, symbol)) {
for (let i = 0, len = symbol.declarations.length; i < len; i++) {
const declaration = symbol.declarations[i];
if (ts.isSourceFile(declaration)) {
// Do not enqueue full source files
// (they can be the declaration of a module import)
continue;
}
if (options.shakeLevel === 2 /* ShakeLevel.ClassMembers */ && (ts.isClassDeclaration(declaration) || ts.isInterfaceDeclaration(declaration)) && !isLocalCodeExtendingOrInheritingFromDefaultLibSymbol(ts, program, checker, declaration)) {
enqueue_black(declaration.name);
for (let j = 0; j < declaration.members.length; j++) {
const member = declaration.members[j];
const memberName = member.name ? member.name.getText() : null;
if (ts.isConstructorDeclaration(member)
|| ts.isConstructSignatureDeclaration(member)
|| ts.isIndexSignatureDeclaration(member)
|| ts.isCallSignatureDeclaration(member)
|| memberName === '[Symbol.iterator]'
|| memberName === '[Symbol.toStringTag]'
|| memberName === 'toJSON'
|| memberName === 'toString'
|| memberName === 'dispose' // TODO: keeping all `dispose` methods
|| /^_(.*)Brand$/.test(memberName || '') // TODO: keeping all members ending with `Brand`...
) {
enqueue_black(member);
if (isSymbolWithDeclarations(symbol) && !nodeIsInItsOwnDeclaration(nodeSourceFile, node, symbol)) {
for (let i = 0, len = symbol.declarations.length; i < len; i++) {
const declaration = symbol.declarations[i];
if (ts.isSourceFile(declaration)) {
// Do not enqueue full source files
// (they can be the declaration of a module import)
continue;
}
if (options.shakeLevel === 2 /* ShakeLevel.ClassMembers */ && (ts.isClassDeclaration(declaration) || ts.isInterfaceDeclaration(declaration)) && !isLocalCodeExtendingOrInheritingFromDefaultLibSymbol(ts, program, checker, declaration)) {
enqueue_black(declaration.name);
for (let j = 0; j < declaration.members.length; j++) {
const member = declaration.members[j];
const memberName = member.name ? member.name.getText() : null;
if (ts.isConstructorDeclaration(member)
|| ts.isConstructSignatureDeclaration(member)
|| ts.isIndexSignatureDeclaration(member)
|| ts.isCallSignatureDeclaration(member)
|| memberName === '[Symbol.iterator]'
|| memberName === '[Symbol.toStringTag]'
|| memberName === 'toJSON'
|| memberName === 'toString'
|| memberName === 'dispose' // TODO: keeping all `dispose` methods
|| /^_(.*)Brand$/.test(memberName || '') // TODO: keeping all members ending with `Brand`...
) {
enqueue_black(member);
}
if (isStaticMemberWithSideEffects(ts, member)) {
enqueue_black(member);
}
}
if (isStaticMemberWithSideEffects(ts, member)) {
enqueue_black(member);
// queue the heritage clauses
if (declaration.heritageClauses) {
for (const heritageClause of declaration.heritageClauses) {
enqueue_black(heritageClause);
}
}
}
// queue the heritage clauses
if (declaration.heritageClauses) {
for (const heritageClause of declaration.heritageClauses) {
enqueue_black(heritageClause);
}
else {
enqueue_black(declaration);
}
}
else {
enqueue_black(declaration);
}
}
}
node.forEachChild(loop);
@ -736,13 +738,20 @@ function findSymbolFromHeritageType(ts, checker, type) {
return findSymbolFromHeritageType(ts, checker, type.expression);
}
if (ts.isIdentifier(type)) {
return getRealNodeSymbol(ts, checker, type)[0];
const tmp = getRealNodeSymbol(ts, checker, type);
return (tmp.length > 0 ? tmp[0].symbol : null);
}
if (ts.isPropertyAccessExpression(type)) {
return findSymbolFromHeritageType(ts, checker, type.name);
}
return null;
}
class SymbolImportTuple {
constructor(symbol, symbolImportNode) {
this.symbol = symbol;
this.symbolImportNode = symbolImportNode;
}
}
/**
* Returns the node's symbol and the `import` node (if the symbol resolved from a different module)
*/
@ -774,7 +783,7 @@ function getRealNodeSymbol(ts, checker, node) {
}
if (!ts.isShorthandPropertyAssignment(node)) {
if (node.getChildCount() !== 0) {
return [null, null];
return [];
}
}
const { parent } = node;
@ -820,10 +829,7 @@ function getRealNodeSymbol(ts, checker, node) {
const type = checker.getTypeAtLocation(parent.parent);
if (name && type) {
if (type.isUnion()) {
const prop = type.types[0].getProperty(name);
if (prop) {
symbol = prop;
}
return generateMultipleSymbols(type, name, importNode);
}
else {
const prop = type.getProperty(name);
@ -854,9 +860,19 @@ function getRealNodeSymbol(ts, checker, node) {
}
}
if (symbol && symbol.declarations) {
return [symbol, importNode];
return [new SymbolImportTuple(symbol, importNode)];
}
return [];
function generateMultipleSymbols(type, name, importNode) {
const result = [];
for (const t of type.types) {
const prop = t.getProperty(name);
if (prop && prop.declarations) {
result.push(new SymbolImportTuple(prop, importNode));
}
}
return result;
}
return [null, null];
}
/** Get the token whose text contains the position */
function getTokenAtPosition(ts, sourceFile, position, allowPositionInLeadingTrivia, includeEndPosition) {

128
build/lib/treeshaking.ts
View file

@ -609,58 +609,60 @@ function markNodes(ts: typeof import('typescript'), languageService: ts.Language
const nodeSourceFile = node.getSourceFile();
const loop = (node: ts.Node) => {
const [symbol, symbolImportNode] = getRealNodeSymbol(ts, checker, node);
if (symbolImportNode) {
setColor(symbolImportNode, NodeColor.Black);
const importDeclarationNode = findParentImportDeclaration(symbolImportNode);
if (importDeclarationNode && ts.isStringLiteral(importDeclarationNode.moduleSpecifier)) {
enqueueImport(importDeclarationNode, importDeclarationNode.moduleSpecifier.text);
}
}
if (isSymbolWithDeclarations(symbol) && !nodeIsInItsOwnDeclaration(nodeSourceFile, node, symbol)) {
for (let i = 0, len = symbol.declarations.length; i < len; i++) {
const declaration = symbol.declarations[i];
if (ts.isSourceFile(declaration)) {
// Do not enqueue full source files
// (they can be the declaration of a module import)
continue;
const symbols = getRealNodeSymbol(ts, checker, node);
for (const { symbol, symbolImportNode } of symbols) {
if (symbolImportNode) {
setColor(symbolImportNode, NodeColor.Black);
const importDeclarationNode = findParentImportDeclaration(symbolImportNode);
if (importDeclarationNode && ts.isStringLiteral(importDeclarationNode.moduleSpecifier)) {
enqueueImport(importDeclarationNode, importDeclarationNode.moduleSpecifier.text);
}
}
if (options.shakeLevel === ShakeLevel.ClassMembers && (ts.isClassDeclaration(declaration) || ts.isInterfaceDeclaration(declaration)) && !isLocalCodeExtendingOrInheritingFromDefaultLibSymbol(ts, program, checker, declaration)) {
enqueue_black(declaration.name!);
for (let j = 0; j < declaration.members.length; j++) {
const member = declaration.members[j];
const memberName = member.name ? member.name.getText() : null;
if (
ts.isConstructorDeclaration(member)
|| ts.isConstructSignatureDeclaration(member)
|| ts.isIndexSignatureDeclaration(member)
|| ts.isCallSignatureDeclaration(member)
|| memberName === '[Symbol.iterator]'
|| memberName === '[Symbol.toStringTag]'
|| memberName === 'toJSON'
|| memberName === 'toString'
|| memberName === 'dispose'// TODO: keeping all `dispose` methods
|| /^_(.*)Brand$/.test(memberName || '') // TODO: keeping all members ending with `Brand`...
) {
enqueue_black(member);
}
if (isStaticMemberWithSideEffects(ts, member)) {
enqueue_black(member);
}
if (isSymbolWithDeclarations(symbol) && !nodeIsInItsOwnDeclaration(nodeSourceFile, node, symbol)) {
for (let i = 0, len = symbol.declarations.length; i < len; i++) {
const declaration = symbol.declarations[i];
if (ts.isSourceFile(declaration)) {
// Do not enqueue full source files
// (they can be the declaration of a module import)
continue;
}
// queue the heritage clauses
if (declaration.heritageClauses) {
for (const heritageClause of declaration.heritageClauses) {
enqueue_black(heritageClause);
if (options.shakeLevel === ShakeLevel.ClassMembers && (ts.isClassDeclaration(declaration) || ts.isInterfaceDeclaration(declaration)) && !isLocalCodeExtendingOrInheritingFromDefaultLibSymbol(ts, program, checker, declaration)) {
enqueue_black(declaration.name!);
for (let j = 0; j < declaration.members.length; j++) {
const member = declaration.members[j];
const memberName = member.name ? member.name.getText() : null;
if (
ts.isConstructorDeclaration(member)
|| ts.isConstructSignatureDeclaration(member)
|| ts.isIndexSignatureDeclaration(member)
|| ts.isCallSignatureDeclaration(member)
|| memberName === '[Symbol.iterator]'
|| memberName === '[Symbol.toStringTag]'
|| memberName === 'toJSON'
|| memberName === 'toString'
|| memberName === 'dispose'// TODO: keeping all `dispose` methods
|| /^_(.*)Brand$/.test(memberName || '') // TODO: keeping all members ending with `Brand`...
) {
enqueue_black(member);
}
if (isStaticMemberWithSideEffects(ts, member)) {
enqueue_black(member);
}
}
// queue the heritage clauses
if (declaration.heritageClauses) {
for (const heritageClause of declaration.heritageClauses) {
enqueue_black(heritageClause);
}
}
} else {
enqueue_black(declaration);
}
} else {
enqueue_black(declaration);
}
}
}
@ -879,7 +881,8 @@ function findSymbolFromHeritageType(ts: typeof import('typescript'), checker: ts
return findSymbolFromHeritageType(ts, checker, type.expression);
}
if (ts.isIdentifier(type)) {
return getRealNodeSymbol(ts, checker, type)[0];
const tmp = getRealNodeSymbol(ts, checker, type);
return (tmp.length > 0 ? tmp[0].symbol : null);
}
if (ts.isPropertyAccessExpression(type)) {
return findSymbolFromHeritageType(ts, checker, type.name);
@ -887,10 +890,17 @@ function findSymbolFromHeritageType(ts: typeof import('typescript'), checker: ts
return null;
}
class SymbolImportTuple {
constructor(
public readonly symbol: ts.Symbol | null,
public readonly symbolImportNode: ts.Declaration | null
) { }
}
/**
* Returns the node's symbol and the `import` node (if the symbol resolved from a different module)
*/
function getRealNodeSymbol(ts: typeof import('typescript'), checker: ts.TypeChecker, node: ts.Node): [ts.Symbol | null, ts.Declaration | null] {
function getRealNodeSymbol(ts: typeof import('typescript'), checker: ts.TypeChecker, node: ts.Node): SymbolImportTuple[] {
// Use some TypeScript internals to avoid code duplication
type ObjectLiteralElementWithName = ts.ObjectLiteralElement & { name: ts.PropertyName; parent: ts.ObjectLiteralExpression | ts.JsxAttributes };
@ -923,7 +933,7 @@ function getRealNodeSymbol(ts: typeof import('typescript'), checker: ts.TypeChec
if (!ts.isShorthandPropertyAssignment(node)) {
if (node.getChildCount() !== 0) {
return [null, null];
return [];
}
}
@ -976,10 +986,7 @@ function getRealNodeSymbol(ts: typeof import('typescript'), checker: ts.TypeChec
const type = checker.getTypeAtLocation(parent.parent);
if (name && type) {
if (type.isUnion()) {
const prop = type.types[0].getProperty(name);
if (prop) {
symbol = prop;
}
return generateMultipleSymbols(type, name, importNode);
} else {
const prop = type.getProperty(name);
if (prop) {
@ -1011,10 +1018,21 @@ function getRealNodeSymbol(ts: typeof import('typescript'), checker: ts.TypeChec
}
if (symbol && symbol.declarations) {
return [symbol, importNode];
return [new SymbolImportTuple(symbol, importNode)];
}
return [null, null];
return [];
function generateMultipleSymbols(type: ts.UnionType, name: string, importNode: ts.Declaration | null): SymbolImportTuple[] {
const result: SymbolImportTuple[] = [];
for (const t of type.types) {
const prop = t.getProperty(name);
if (prop && prop.declarations) {
result.push(new SymbolImportTuple(prop, importNode));
}
}
return result;
}
}
/** Get the token whose text contains the position */

43
src/vs/platform/actions/common/actions.ts
View file

@ -540,13 +540,7 @@ export class SyncActionDescriptor {
type OneOrN<T> = T | T[];
export interface IAction2Options extends ICommandAction {
/**
* Shorthand to add this command to the command palette
*/
f1?: boolean;
interface IAction2CommonOptions extends ICommandAction {
/**
* One or many menu items.
*/
@ -571,6 +565,41 @@ export interface IAction2Options extends ICommandAction {
_isFakeAction?: true;
}
interface IBaseAction2Options extends IAction2CommonOptions {
/**
* This type is used when an action is not going to show up in the command palette.
* In that case, it's able to use a string for the `title` and `category` properties.
*/
f1?: false;
}
interface ICommandPaletteOptions extends IAction2CommonOptions {
/**
* The title of the command that will be displayed in the command palette after the category.
* This overrides {@link ICommandAction.title} to ensure a string isn't used so that the title
* includes the localized value and the original value for users using language packs.
*/
title: ICommandActionTitle;
/**
* The category of the command that will be displayed in the command palette before the title suffixed.
* with a colon This overrides {@link ICommandAction.title} to ensure a string isn't used so that
* the title includes the localized value and the original value for users using language packs.
*/
category?: keyof typeof Categories | ILocalizedString;
/**
* Shorthand to add this command to the command palette. Note: this is not the only way to declare that
* a command should be in the command palette... however, enforcing ILocalizedString in the other scenarios
* is much more challenging and this gets us most of the way there.
*/
f1: true;
}
export type IAction2Options = ICommandPaletteOptions | IBaseAction2Options;
export interface IAction2F1RequiredOptions {
title: ICommandActionTitle;
category?: keyof typeof Categories | ILocalizedString;

8
src/vs/workbench/contrib/extensions/browser/extensions.contribution.ts
View file

@ -448,10 +448,10 @@ async function runAction(action: IAction): Promise<void> {
}
}
interface IExtensionActionOptions extends IAction2Options {
type IExtensionActionOptions = IAction2Options & {
menuTitles?: { [id: string]: string };
run(accessor: ServicesAccessor, ...args: any[]): Promise<any>;
}
};
class ExtensionsContributions extends Disposable implements IWorkbenchContribution {
@ -994,9 +994,8 @@ class ExtensionsContributions extends Disposable implements IWorkbenchContributi
title: { value: localize('extensionUpdates', "Show Extension Updates"), original: 'Show Extension Updates' },
category: ExtensionsLocalizedLabel,
precondition: CONTEXT_HAS_GALLERY,
f1: true,
menu: [{
id: MenuId.CommandPalette,
}, {
id: extensionsFilterSubMenu,
group: '3_installed',
when: CONTEXT_HAS_GALLERY,
@ -1049,7 +1048,6 @@ class ExtensionsContributions extends Disposable implements IWorkbenchContributi
id: 'workbench.extensions.action.showDisabledExtensions',
title: { value: localize('showDisabledExtensions', "Show Disabled Extensions"), original: 'Show Disabled Extensions' },
category: ExtensionsLocalizedLabel,
menu: [{
id: MenuId.CommandPalette,
when: ContextKeyExpr.or(CONTEXT_HAS_LOCAL_SERVER, CONTEXT_HAS_REMOTE_SERVER, CONTEXT_HAS_WEB_SERVER)

4
src/vs/workbench/contrib/snippets/browser/commands/abstractSnippetsActions.ts
View file

@ -7,12 +7,12 @@ import { EditorAction2 } from 'vs/editor/browser/editorExtensions';
import { localize } from 'vs/nls';
import { Action2, IAction2Options } from 'vs/platform/actions/common/actions';
const defaultOptions: Partial<IAction2Options> = {
const defaultOptions = {
category: {
value: localize('snippets', 'Snippets'),
original: 'Snippets'
},
};
} as const;
export abstract class SnippetsAction extends Action2 {

2
src/vs/workbench/contrib/snippets/browser/commands/configureSnippets.ts
View file

@ -214,8 +214,8 @@ export class ConfigureSnippets extends SnippetsAction {
mnemonicTitle: nls.localize({ key: 'miOpenSnippets', comment: ['&& denotes a mnemonic'] }, "User &&Snippets"),
original: 'User Snippets'
},
f1: true,
menu: [
{ id: MenuId.CommandPalette },
{ id: MenuId.MenubarPreferencesMenu, group: '3_snippets', order: 1 },
{ id: MenuId.GlobalActivity, group: '3_snippets', order: 1 },
]

2
src/vs/workbench/contrib/terminal/browser/terminalActions.ts
View file

@ -2234,7 +2234,7 @@ export function registerTerminalActions() {
id: TerminalCommandId.ShowTextureAtlas,
title: { value: localize('workbench.action.terminal.showTextureAtlas', "Show Terminal Texture Atlas"), original: 'Show Terminal Texture Atlas' },
f1: true,
category: Categories.Developer.value,
category: Categories.Developer,
precondition: ContextKeyExpr.or(TerminalContextKeys.isOpen)
});
}

2
src/vs/workbench/services/preferences/test/browser/keybindingsEditorModel.test.ts
View file

@ -672,7 +672,7 @@ suite('KeybindingsEditorModel', () => {
constructor() {
super({
id: command,
title,
title: { value: title, original: title },
f1: true
});
}