Skip to content

Commit 5813a35

Browse files
andrewbranchandrewbranch
authored
Allow moduleSymbolToValidIdentifier to be uppercase for JSX tags (microsoft#47625)
* Allow moduleSymbolToValidIdentifier to be uppercase for JSX tags * Cleaner way of getting the uppercase name when needed * Fix build errors, get rid of basically unnecessary ScriptTarget * More accurate name for parameter * Rename other parameter too * Fix failing test
1 parent 0d3ff0c commit 5813a35

File tree

7 files changed

+105
-26
lines changed

7 files changed

+105
-26
lines changed

‎src/services/codefixes/importFixes.ts‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ namespace ts.codefix {
7474
const symbolName = getNameForExportedSymbol(exportedSymbol, getEmitScriptTarget(compilerOptions));
7575
const checker = program.getTypeChecker();
7676
const symbol = checker.getMergedSymbol(skipAlias(exportedSymbol, checker));
77-
const exportInfos = getAllReExportingModules(sourceFile, symbol, moduleSymbol, symbolName, host, program, preferences, useAutoImportProvider);
77+
const exportInfos = getAllReExportingModules(sourceFile, symbol, moduleSymbol, symbolName, /*isJsxTagName*/ false, host, program, preferences, useAutoImportProvider);
7878
const useRequire = shouldUseRequire(sourceFile, program);
7979
const fix = getImportFixForSymbol(sourceFile, exportInfos, moduleSymbol, symbolName, program, /*position*/ undefined, !!isValidTypeOnlyUseSite, useRequire, host, preferences);
8080
if (fix) {
@@ -287,6 +287,7 @@ namespace ts.codefix {
287287
moduleSymbol: Symbol,
288288
sourceFile: SourceFile,
289289
symbolName: string,
290+
isJsxTagName: boolean,
290291
host: LanguageServiceHost,
291292
program: Program,
292293
formatContext: formatting.FormatContext,
@@ -296,7 +297,7 @@ namespace ts.codefix {
296297
const compilerOptions = program.getCompilerOptions();
297298
const exportInfos = pathIsBareSpecifier(stripQuotes(moduleSymbol.name))
298299
? [getSymbolExportInfoForSymbol(targetSymbol, moduleSymbol, program, host)]
299-
: getAllReExportingModules(sourceFile, targetSymbol, moduleSymbol, symbolName, host, program, preferences, /*useAutoImportProvider*/ true);
300+
: getAllReExportingModules(sourceFile, targetSymbol, moduleSymbol, symbolName, isJsxTagName, host, program, preferences, /*useAutoImportProvider*/ true);
300301
const useRequire = shouldUseRequire(sourceFile, program);
301302
const isValidTypeOnlyUseSite = isValidTypeOnlyAliasUseSite(getTokenAtPosition(sourceFile, position));
302303
const fix = Debug.checkDefined(getImportFixForSymbol(sourceFile, exportInfos, moduleSymbol, symbolName, program, position, isValidTypeOnlyUseSite, useRequire, host, preferences));
@@ -349,7 +350,7 @@ namespace ts.codefix {
349350
}
350351
}
351352

352-
function getAllReExportingModules(importingFile: SourceFile, targetSymbol: Symbol, exportingModuleSymbol: Symbol, symbolName: string, host: LanguageServiceHost, program: Program, preferences: UserPreferences, useAutoImportProvider: boolean): readonly SymbolExportInfo[] {
353+
function getAllReExportingModules(importingFile: SourceFile, targetSymbol: Symbol, exportingModuleSymbol: Symbol, symbolName: string, isJsxTagName: boolean, host: LanguageServiceHost, program: Program, preferences: UserPreferences, useAutoImportProvider: boolean): readonly SymbolExportInfo[] {
353354
const result: SymbolExportInfo[] = [];
354355
const compilerOptions = program.getCompilerOptions();
355356
const getModuleSpecifierResolutionHost = memoizeOne((isFromPackageJson: boolean) => {
@@ -364,7 +365,7 @@ namespace ts.codefix {
364365
}
365366

366367
const defaultInfo = getDefaultLikeExportInfo(moduleSymbol, checker, compilerOptions);
367-
if (defaultInfo && (defaultInfo.name === symbolName || moduleSymbolToValidIdentifier(moduleSymbol, getEmitScriptTarget(compilerOptions)) === symbolName) && skipAlias(defaultInfo.symbol, checker) === targetSymbol && isImportable(program, moduleFile, isFromPackageJson)) {
368+
if (defaultInfo && (defaultInfo.name === symbolName || moduleSymbolToValidIdentifier(moduleSymbol, getEmitScriptTarget(compilerOptions), isJsxTagName) === symbolName) && skipAlias(defaultInfo.symbol, checker) === targetSymbol && isImportable(program, moduleFile, isFromPackageJson)) {
368369
result.push({ symbol: defaultInfo.symbol, moduleSymbol, moduleFileName: moduleFile?.fileName, exportKind: defaultInfo.exportKind, targetFlags: skipAlias(defaultInfo.symbol, checker).flags, isFromPackageJson });
369370
}
370371

@@ -804,7 +805,7 @@ namespace ts.codefix {
804805

805806
const isValidTypeOnlyUseSite = isValidTypeOnlyAliasUseSite(symbolToken);
806807
const useRequire = shouldUseRequire(sourceFile, program);
807-
const exportInfos = getExportInfos(symbolName, getMeaningFromLocation(symbolToken), cancellationToken, sourceFile, program, useAutoImportProvider, host, preferences);
808+
const exportInfos = getExportInfos(symbolName, isJSXTagName(symbolToken), getMeaningFromLocation(symbolToken), cancellationToken, sourceFile, program, useAutoImportProvider, host, preferences);
808809
const fixes = arrayFrom(flatMapIterator(exportInfos.entries(), ([_, exportInfos]) =>
809810
getImportFixes(exportInfos, symbolName, symbolToken.getStart(sourceFile), isValidTypeOnlyUseSite, useRequire, program, sourceFile, host, preferences)));
810811
return { fixes, symbolName };
@@ -845,6 +846,7 @@ namespace ts.codefix {
845846
// Returns a map from an exported symbol's ID to a list of every way it's (re-)exported.
846847
function getExportInfos(
847848
symbolName: string,
849+
isJsxTagName: boolean,
848850
currentTokenMeaning: SemanticMeaning,
849851
cancellationToken: CancellationToken,
850852
fromFile: SourceFile,
@@ -876,7 +878,7 @@ namespace ts.codefix {
876878

877879
const compilerOptions = program.getCompilerOptions();
878880
const defaultInfo = getDefaultLikeExportInfo(moduleSymbol, checker, compilerOptions);
879-
if (defaultInfo && (defaultInfo.name === symbolName || moduleSymbolToValidIdentifier(moduleSymbol, getEmitScriptTarget(compilerOptions)) === symbolName) && symbolHasMeaning(defaultInfo.symbolForMeaning, currentTokenMeaning)) {
881+
if (defaultInfo && (defaultInfo.name === symbolName || moduleSymbolToValidIdentifier(moduleSymbol, getEmitScriptTarget(compilerOptions), isJsxTagName) === symbolName) && symbolHasMeaning(defaultInfo.symbolForMeaning, currentTokenMeaning)) {
880882
addSymbol(moduleSymbol, sourceFile, defaultInfo.symbol, defaultInfo.exportKind, program, isFromPackageJson);
881883
}
882884

@@ -1237,17 +1239,20 @@ namespace ts.codefix {
12371239
return some(declarations, decl => !!(getMeaningFromDeclaration(decl) & meaning));
12381240
}
12391241

1240-
export function moduleSymbolToValidIdentifier(moduleSymbol: Symbol, target: ScriptTarget | undefined): string {
1241-
return moduleSpecifierToValidIdentifier(removeFileExtension(stripQuotes(moduleSymbol.name)), target);
1242+
export function moduleSymbolToValidIdentifier(moduleSymbol: Symbol, target: ScriptTarget | undefined, forceCapitalize: boolean): string {
1243+
return moduleSpecifierToValidIdentifier(removeFileExtension(stripQuotes(moduleSymbol.name)), target, forceCapitalize);
12421244
}
12431245

1244-
export function moduleSpecifierToValidIdentifier(moduleSpecifier: string, target: ScriptTarget | undefined): string {
1246+
export function moduleSpecifierToValidIdentifier(moduleSpecifier: string, target: ScriptTarget | undefined, forceCapitalize?: boolean): string {
12451247
const baseName = getBaseFileName(removeSuffix(moduleSpecifier, "/index"));
12461248
let res = "";
12471249
let lastCharWasValid = true;
12481250
const firstCharCode = baseName.charCodeAt(0);
12491251
if (isIdentifierStart(firstCharCode, target)) {
12501252
res += String.fromCharCode(firstCharCode);
1253+
if (forceCapitalize) {
1254+
res = res.toUpperCase();
1255+
}
12511256
}
12521257
else {
12531258
lastCharWasValid = false;

‎src/services/completions.ts‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1520,7 +1520,6 @@ namespace ts.Completions {
15201520
source: string | undefined,
15211521
): CodeActionsAndSourceDisplay {
15221522
if (data?.moduleSpecifier) {
1523-
const { contextToken, previousToken } = getRelevantTokens(position, sourceFile);
15241523
if (previousToken && getImportStatementCompletionInfo(contextToken || previousToken).replacementNode) {
15251524
// Import statement completion: 'import c|'
15261525
return { codeActions: undefined, sourceDisplay: [textPart(data.moduleSpecifier)] };
@@ -1572,11 +1571,13 @@ namespace ts.Completions {
15721571
const checker = origin.isFromPackageJson ? host.getPackageJsonAutoImportProvider!()!.getTypeChecker() : program.getTypeChecker();
15731572
const { moduleSymbol } = origin;
15741573
const targetSymbol = checker.getMergedSymbol(skipAlias(symbol.exportSymbol || symbol, checker));
1574+
const isJsxOpeningTagName = contextToken?.kind === SyntaxKind.LessThanToken && isJsxOpeningLikeElement(contextToken.parent);
15751575
const { moduleSpecifier, codeAction } = codefix.getImportCompletionAction(
15761576
targetSymbol,
15771577
moduleSymbol,
15781578
sourceFile,
1579-
getNameForExportedSymbol(symbol, getEmitScriptTarget(compilerOptions)),
1579+
getNameForExportedSymbol(symbol, getEmitScriptTarget(compilerOptions), isJsxOpeningTagName),
1580+
isJsxOpeningTagName,
15801581
host,
15811582
program,
15821583
formatContext,
@@ -2486,12 +2487,17 @@ namespace ts.Completions {
24862487
preferences,
24872488
!!importCompletionNode,
24882489
context => {
2489-
exportInfo.forEach(sourceFile.path, (info, symbolName, isFromAmbientModule, exportMapKey) => {
2490+
exportInfo.forEach(sourceFile.path, (info, getSymbolName, isFromAmbientModule, exportMapKey) => {
2491+
const symbolName = getSymbolName(/*preferCapitalized*/ isRightOfOpenTag);
24902492
if (!isIdentifierText(symbolName, getEmitScriptTarget(host.getCompilationSettings()))) return;
24912493
if (!detailsEntryId && isStringANonContextualKeyword(symbolName)) return;
24922494
// `targetFlags` should be the same for each `info`
24932495
if (!isTypeOnlyLocation && !importCompletionNode && !(info[0].targetFlags & SymbolFlags.Value)) return;
24942496
if (isTypeOnlyLocation && !(info[0].targetFlags & (SymbolFlags.Module | SymbolFlags.Type))) return;
2497+
// Do not try to auto-import something with a lowercase first letter for a JSX tag
2498+
const firstChar = symbolName.charCodeAt(0);
2499+
if (isRightOfOpenTag && (firstChar < CharacterCodes.A || firstChar > CharacterCodes.Z)) return;
2500+
24952501
const isCompletionDetailsMatch = detailsEntryId && some(info, i => detailsEntryId.source === stripQuotes(i.moduleSymbol.name));
24962502
if (isCompletionDetailsMatch || !detailsEntryId && charactersFuzzyMatchInString(symbolName, lowerCaseTokenText)) {
24972503
const defaultExportInfo = find(info, isImportableExportInfo);
@@ -2504,6 +2510,7 @@ namespace ts.Completions {
25042510
const { exportInfo = defaultExportInfo, moduleSpecifier } = context.tryResolve(info, isFromAmbientModule) || {};
25052511
const isDefaultExport = exportInfo.exportKind === ExportKind.Default;
25062512
const symbol = isDefaultExport && getLocalSymbolForExportDefault(exportInfo.symbol) || exportInfo.symbol;
2513+
25072514
pushAutoImportSymbol(symbol, {
25082515
kind: moduleSpecifier ? SymbolOriginInfoKind.ResolvedExport : SymbolOriginInfoKind.Export,
25092516
moduleSpecifier,

‎src/services/exportInfoMap.ts‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,9 @@ namespace ts {
4545
export interface ExportInfoMap {
4646
isUsableByFile(importingFile: Path): boolean;
4747
clear(): void;
48-
add(importingFile: Path, symbol: Symbol, key: __String, moduleSymbol: Symbol, moduleFile: SourceFile | undefined, exportKind: ExportKind, isFromPackageJson: boolean, scriptTarget: ScriptTarget, checker: TypeChecker): void;
48+
add(importingFile: Path, symbol: Symbol, key: __String, moduleSymbol: Symbol, moduleFile: SourceFile | undefined, exportKind: ExportKind, isFromPackageJson: boolean, checker: TypeChecker): void;
4949
get(importingFile: Path, key: string): readonly SymbolExportInfo[] | undefined;
50-
forEach(importingFile: Path, action: (info: readonly SymbolExportInfo[], name: string, isFromAmbientModule: boolean, key: string) => void): void;
50+
forEach(importingFile: Path, action: (info: readonly SymbolExportInfo[], getSymbolName: (preferCapitalized?: boolean) => string, isFromAmbientModule: boolean, key: string) => void): void;
5151
releaseSymbols(): void;
5252
isEmpty(): boolean;
5353
/** @returns Whether the change resulted in the cache being cleared */
@@ -72,7 +72,7 @@ namespace ts {
7272
symbols.clear();
7373
usableByFileName = undefined;
7474
},
75-
add: (importingFile, symbol, symbolTableKey, moduleSymbol, moduleFile, exportKind, isFromPackageJson, scriptTarget, checker) => {
75+
add: (importingFile, symbol, symbolTableKey, moduleSymbol, moduleFile, exportKind, isFromPackageJson, checker) => {
7676
if (importingFile !== usableByFileName) {
7777
cache.clear();
7878
usableByFileName = importingFile;
@@ -88,7 +88,8 @@ namespace ts {
8888
// get a better name.
8989
const importedName = exportKind === ExportKind.Named || isExternalModuleSymbol(namedSymbol)
9090
? unescapeLeadingUnderscores(symbolTableKey)
91-
: getNameForExportedSymbol(namedSymbol, scriptTarget);
91+
: getNameForExportedSymbol(namedSymbol, /*scriptTarget*/ undefined);
92+
9293
const moduleName = stripQuotes(moduleSymbol.name);
9394
const id = exportInfoId++;
9495
const target = skipAlias(symbol, checker);
@@ -119,7 +120,18 @@ namespace ts {
119120
if (importingFile !== usableByFileName) return;
120121
exportInfo.forEach((info, key) => {
121122
const { symbolName, ambientModuleName } = parseKey(key);
122-
action(info.map(rehydrateCachedInfo), symbolName, !!ambientModuleName, key);
123+
const rehydrated = info.map(rehydrateCachedInfo);
124+
action(
125+
rehydrated,
126+
preferCapitalized => {
127+
const { symbol, exportKind } = rehydrated[0];
128+
const namedSymbol = exportKind === ExportKind.Default && getLocalSymbolForExportDefault(symbol) || symbol;
129+
return preferCapitalized
130+
? getNameForExportedSymbol(namedSymbol, /*scriptTarget*/ undefined, /*preferCapitalized*/ true)
131+
: symbolName;
132+
},
133+
!!ambientModuleName,
134+
key);
123135
});
124136
},
125137
releaseSymbols: () => {
@@ -321,7 +333,6 @@ namespace ts {
321333

322334
host.log?.("getExportInfoMap: cache miss or empty; calculating new results");
323335
const compilerOptions = program.getCompilerOptions();
324-
const scriptTarget = getEmitScriptTarget(compilerOptions);
325336
let moduleCount = 0;
326337
forEachExternalModuleToImportFrom(program, host, /*useAutoImportProvider*/ true, (moduleSymbol, moduleFile, program, isFromPackageJson) => {
327338
if (++moduleCount % 100 === 0) cancellationToken?.throwIfCancellationRequested();
@@ -339,7 +350,6 @@ namespace ts {
339350
moduleFile,
340351
defaultInfo.exportKind,
341352
isFromPackageJson,
342-
scriptTarget,
343353
checker);
344354
}
345355
checker.forEachExportAndPropertyOfModule(moduleSymbol, (exported, key) => {
@@ -352,7 +362,6 @@ namespace ts {
352362
moduleFile,
353363
ExportKind.Named,
354364
isFromPackageJson,
355-
scriptTarget,
356365
checker);
357366
}
358367
});

‎src/services/utilities.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3210,11 +3210,11 @@ namespace ts {
32103210
return isArray(valueOrArray) ? first(valueOrArray) : valueOrArray;
32113211
}
32123212

3213-
export function getNameForExportedSymbol(symbol: Symbol, scriptTarget: ScriptTarget | undefined) {
3213+
export function getNameForExportedSymbol(symbol: Symbol, scriptTarget: ScriptTarget | undefined, preferCapitalized?: boolean) {
32143214
if (!(symbol.flags & SymbolFlags.Transient) && (symbol.escapedName === InternalSymbolName.ExportEquals || symbol.escapedName === InternalSymbolName.Default)) {
32153215
// Name of "export default foo;" is "foo". Name of "export default 0" is the filename converted to camelCase.
32163216
return firstDefined(symbol.declarations, d => isExportAssignment(d) ? tryCast(skipOuterExpressions(d.expression), isIdentifier)?.text : undefined)
3217-
|| codefix.moduleSymbolToValidIdentifier(getSymbolParentOrFail(symbol), scriptTarget);
3217+
|| codefix.moduleSymbolToValidIdentifier(getSymbolParentOrFail(symbol), scriptTarget, !!preferCapitalized);
32183218
}
32193219
return symbol.name;
32203220
}

‎src/testRunner/unittests/tsserver/exportMapCache.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,8 @@ namespace ts.projectSystem {
8787
// transient symbols are recreated with every new checker.
8888
const programBefore = project.getCurrentProgram()!;
8989
let sigintPropBefore: readonly SymbolExportInfo[] | undefined;
90-
exportMapCache.forEach(bTs.path as Path, (info, name) => {
91-
if (name === "SIGINT") sigintPropBefore = info;
90+
exportMapCache.forEach(bTs.path as Path, (info, getSymbolName) => {
91+
if (getSymbolName() === "SIGINT") sigintPropBefore = info;
9292
});
9393
assert.ok(sigintPropBefore);
9494
assert.ok(sigintPropBefore![0].symbol.flags & SymbolFlags.Transient);
@@ -113,8 +113,8 @@ namespace ts.projectSystem {
113113

114114
// Get same info from cache again
115115
let sigintPropAfter: readonly SymbolExportInfo[] | undefined;
116-
exportMapCache.forEach(bTs.path as Path, (info, name) => {
117-
if (name === "SIGINT") sigintPropAfter = info;
116+
exportMapCache.forEach(bTs.path as Path, (info, getSymbolName) => {
117+
if (getSymbolName() === "SIGINT") sigintPropAfter = info;
118118
});
119119
assert.ok(sigintPropAfter);
120120
assert.notEqual(symbolIdBefore, getSymbolId(sigintPropAfter![0].symbol));

‎tests/cases/fourslash/completionsImport_jsxOpeningTagImportDefault.ts‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
// @module: commonjs
4+
// @jsx: react
5+
6+
// @Filename: /component.tsx
7+
//// export default function (props: any) {}
8+
9+
// @Filename: /index.tsx
10+
//// export function Index() {
11+
//// return <Component/**/
12+
//// }
13+
14+
goTo.marker("");
15+
verify.completions({
16+
marker: "",
17+
includes: {
18+
name: "Component",
19+
source: "/component",
20+
hasAction: true,
21+
sortText: completion.SortText.AutoImportSuggestions,
22+
},
23+
excludes: "component",
24+
preferences: {
25+
includeCompletionsForModuleExports: true,
26+
},
27+
});
28+
29+
verify.applyCodeActionFromCompletion("", {
30+
name: "Component",
31+
source: "/component",
32+
description: `Import default 'Component' from module "./component"`,
33+
newFileContent:
34+
`import Component from "./component";
35+
36+
export function Index() {
37+
return <Component
38+
}`,
39+
});

‎tests/cases/fourslash/importNameCodeFix_jsxOpeningTagImportDefault.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
// @module: commonjs
4+
// @jsx: react-jsx
5+
6+
// @Filename: /component.tsx
7+
//// export default function (props: any) {}
8+
9+
// @Filename: /index.tsx
10+
//// export function Index() {
11+
//// return <Component/**/ />;
12+
//// }
13+
14+
goTo.marker("");
15+
verify.importFixAtPosition([`import Component from "./component";
16+
17+
export function Index() {
18+
return <Component />;
19+
}`]);

0 commit comments

Comments
 (0)