Skip to content

Commit 8a8048f

Browse files
jakebaileyjakebailey
authored
Rewrite logic for JSX attribute completion detection [4.5 cherry pick] (#47413)
1 parent 14016a7 commit 8a8048f

8 files changed

+343
-83
lines changed

Diff for: ‎src/services/completions.ts

+17-22
Original file line numberDiff line numberDiff line change
@@ -428,6 +428,7 @@ namespace ts.Completions {
428428
isJsxInitializer,
429429
isTypeOnlyLocation,
430430
isJsxIdentifierExpected,
431+
isRightOfOpenTag,
431432
importCompletionNode,
432433
insideJsDocTagTypeExpression,
433434
symbolToSortTextIdMap,
@@ -466,7 +467,9 @@ namespace ts.Completions {
466467
importCompletionNode,
467468
recommendedCompletion,
468469
symbolToOriginInfoMap,
469-
symbolToSortTextIdMap
470+
symbolToSortTextIdMap,
471+
isJsxIdentifierExpected,
472+
isRightOfOpenTag,
470473
);
471474
getJSCompletionEntries(sourceFile, location.pos, uniqueNames, getEmitScriptTarget(compilerOptions), entries); // TODO: GH#18217
472475
}
@@ -496,7 +499,9 @@ namespace ts.Completions {
496499
importCompletionNode,
497500
recommendedCompletion,
498501
symbolToOriginInfoMap,
499-
symbolToSortTextIdMap
502+
symbolToSortTextIdMap,
503+
isJsxIdentifierExpected,
504+
isRightOfOpenTag,
500505
);
501506
}
502507

@@ -638,6 +643,8 @@ namespace ts.Completions {
638643
options: CompilerOptions,
639644
preferences: UserPreferences,
640645
completionKind: CompletionKind,
646+
isJsxIdentifierExpected: boolean | undefined,
647+
isRightOfOpenTag: boolean | undefined,
641648
): CompletionEntry | undefined {
642649
let insertText: string | undefined;
643650
let replacementSpan = getReplacementSpanForContextToken(replacementToken);
@@ -713,25 +720,7 @@ namespace ts.Completions {
713720
}
714721
}
715722

716-
// Before offering up a JSX attribute snippet, ensure that we aren't potentially completing
717-
// a tag name; this may appear as an attribute after the "<" when the tag has not yet been
718-
// closed, as in:
719-
//
720-
// return <>
721-
// foo <butto|
722-
// </>
723-
//
724-
// We can detect this case by checking if both:
725-
//
726-
// 1. The location is "<", so we are completing immediately after it.
727-
// 2. The "<" has the same position as its parent, so is not a binary expression.
728-
const kind = SymbolDisplay.getSymbolKind(typeChecker, symbol, location);
729-
if (
730-
kind === ScriptElementKind.jsxAttribute
731-
&& (location.kind !== SyntaxKind.LessThanToken || location.pos !== location.parent.pos)
732-
&& preferences.includeCompletionsWithSnippetText
733-
&& preferences.jsxAttributeCompletionStyle
734-
&& preferences.jsxAttributeCompletionStyle !== "none") {
723+
if (isJsxIdentifierExpected && !isRightOfOpenTag && preferences.includeCompletionsWithSnippetText && preferences.jsxAttributeCompletionStyle && preferences.jsxAttributeCompletionStyle !== "none") {
735724
let useBraces = preferences.jsxAttributeCompletionStyle === "braces";
736725
const type = typeChecker.getTypeOfSymbolAtLocation(symbol, location);
737726

@@ -776,7 +765,7 @@ namespace ts.Completions {
776765
// entries (like JavaScript identifier entries).
777766
return {
778767
name,
779-
kind,
768+
kind: SymbolDisplay.getSymbolKind(typeChecker, symbol, location),
780769
kindModifiers: SymbolDisplay.getSymbolModifiers(typeChecker, symbol),
781770
sortText,
782771
source,
@@ -1142,6 +1131,8 @@ namespace ts.Completions {
11421131
recommendedCompletion?: Symbol,
11431132
symbolToOriginInfoMap?: SymbolOriginInfoMap,
11441133
symbolToSortTextIdMap?: SymbolSortTextIdMap,
1134+
isJsxIdentifierExpected?: boolean,
1135+
isRightOfOpenTag?: boolean,
11451136
): UniqueNameSet {
11461137
const start = timestamp();
11471138
const variableDeclaration = getVariableDeclaration(location);
@@ -1183,6 +1174,8 @@ namespace ts.Completions {
11831174
compilerOptions,
11841175
preferences,
11851176
kind,
1177+
isJsxIdentifierExpected,
1178+
isRightOfOpenTag,
11861179
);
11871180
if (!entry) {
11881181
continue;
@@ -1534,6 +1527,7 @@ namespace ts.Completions {
15341527
readonly isTypeOnlyLocation: boolean;
15351528
/** In JSX tag name and attribute names, identifiers like "my-tag" or "aria-name" is valid identifier. */
15361529
readonly isJsxIdentifierExpected: boolean;
1530+
readonly isRightOfOpenTag: boolean;
15371531
readonly importCompletionNode?: Node;
15381532
readonly hasUnresolvedAutoImports?: boolean;
15391533
}
@@ -1941,6 +1935,7 @@ namespace ts.Completions {
19411935
symbolToSortTextIdMap,
19421936
isTypeOnlyLocation,
19431937
isJsxIdentifierExpected,
1938+
isRightOfOpenTag,
19441939
importCompletionNode,
19451940
hasUnresolvedAutoImports,
19461941
};

Diff for: ‎tests/cases/fourslash/jsxAttributeAsTagNameNoSnippet.ts

-61
This file was deleted.

Diff for: ‎tests/cases/fourslash/jsxAttributeSnippetCompletionClosed.ts

+74
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
/// <reference path="fourslash.ts" />
2+
//@Filename: file.tsx
3+
////interface NestedInterface {
4+
//// Foo: NestedInterface;
5+
//// (props: {className?: string}): any;
6+
////}
7+
////
8+
////declare const Foo: NestedInterface;
9+
////
10+
////function fn1() {
11+
//// return <Foo>
12+
//// <Foo /*1*/ />
13+
//// </Foo>
14+
////}
15+
////function fn2() {
16+
//// return <Foo>
17+
//// <Foo.Foo /*2*/ />
18+
//// </Foo>
19+
////}
20+
////function fn3() {
21+
//// return <Foo>
22+
//// <Foo.Foo cla/*3*/ />
23+
//// </Foo>
24+
////}
25+
////function fn4() {
26+
//// return <Foo>
27+
//// <Foo.Foo cla/*4*/ something />
28+
//// </Foo>
29+
////}
30+
////function fn5() {
31+
//// return <Foo>
32+
//// <Foo.Foo something /*5*/ />
33+
//// </Foo>
34+
////}
35+
////function fn6() {
36+
//// return <Foo>
37+
//// <Foo.Foo something cla/*6*/ />
38+
//// </Foo>
39+
////}
40+
////function fn7() {
41+
//// return <Foo /*7*/ />
42+
////}
43+
////function fn8() {
44+
//// return <Foo cla/*8*/ />
45+
////}
46+
////function fn9() {
47+
//// return <Foo cla/*9*/ something />
48+
////}
49+
////function fn10() {
50+
//// return <Foo something /*10*/ />
51+
////}
52+
////function fn11() {
53+
//// return <Foo something cla/*11*/ />
54+
////}
55+
56+
var preferences: FourSlashInterface.UserPreferences = {
57+
jsxAttributeCompletionStyle: "braces",
58+
includeCompletionsWithSnippetText: true,
59+
includeCompletionsWithInsertText: true,
60+
};
61+
62+
verify.completions(
63+
{ marker: "1", preferences, includes: { name: "className", insertText: "className={$1}", text: "(JSX attribute) className?: string", isSnippet: true, sortText: completion.SortText.OptionalMember } },
64+
{ marker: "2", preferences, includes: { name: "className", insertText: "className={$1}", text: "(JSX attribute) className?: string", isSnippet: true, sortText: completion.SortText.OptionalMember } },
65+
{ marker: "3", preferences, includes: { name: "className", insertText: "className={$1}", text: "(JSX attribute) className?: string", isSnippet: true, sortText: completion.SortText.OptionalMember } },
66+
{ marker: "4", preferences, includes: { name: "className", insertText: "className={$1}", text: "(JSX attribute) className?: string", isSnippet: true, sortText: completion.SortText.OptionalMember } },
67+
{ marker: "5", preferences, includes: { name: "className", insertText: "className={$1}", text: "(JSX attribute) className?: string", isSnippet: true, sortText: completion.SortText.OptionalMember } },
68+
{ marker: "6", preferences, includes: { name: "className", insertText: "className={$1}", text: "(JSX attribute) className?: string", isSnippet: true, sortText: completion.SortText.OptionalMember } },
69+
{ marker: "7", preferences, includes: { name: "className", insertText: "className={$1}", text: "(JSX attribute) className?: string", isSnippet: true, sortText: completion.SortText.OptionalMember } },
70+
{ marker: "8", preferences, includes: { name: "className", insertText: "className={$1}", text: "(JSX attribute) className?: string", isSnippet: true, sortText: completion.SortText.OptionalMember } },
71+
{ marker: "9", preferences, includes: { name: "className", insertText: "className={$1}", text: "(JSX attribute) className?: string", isSnippet: true, sortText: completion.SortText.OptionalMember } },
72+
{ marker: "10", preferences, includes: { name: "className", insertText: "className={$1}", text: "(JSX attribute) className?: string", isSnippet: true, sortText: completion.SortText.OptionalMember } },
73+
{ marker: "11", preferences, includes: { name: "className", insertText: "className={$1}", text: "(JSX attribute) className?: string", isSnippet: true, sortText: completion.SortText.OptionalMember } },
74+
)

Diff for: ‎tests/cases/fourslash/jsxAttributeSnippetCompletionUnclosed.ts

+74
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
/// <reference path="fourslash.ts" />
2+
//@Filename: file.tsx
3+
////interface NestedInterface {
4+
//// Foo: NestedInterface;
5+
//// (props: {className?: string}): any;
6+
////}
7+
////
8+
////declare const Foo: NestedInterface;
9+
////
10+
////function fn1() {
11+
//// return <Foo>
12+
//// <Foo /*1*/
13+
//// </Foo>
14+
////}
15+
////function fn2() {
16+
//// return <Foo>
17+
//// <Foo.Foo /*2*/
18+
//// </Foo>
19+
////}
20+
////function fn3() {
21+
//// return <Foo>
22+
//// <Foo.Foo cla/*3*/
23+
//// </Foo>
24+
////}
25+
////function fn4() {
26+
//// return <Foo>
27+
//// <Foo.Foo cla/*4*/ something
28+
//// </Foo>
29+
////}
30+
////function fn5() {
31+
//// return <Foo>
32+
//// <Foo.Foo something /*5*/
33+
//// </Foo>
34+
////}
35+
////function fn6() {
36+
//// return <Foo>
37+
//// <Foo.Foo something cla/*6*/
38+
//// </Foo>
39+
////}
40+
////function fn7() {
41+
//// return <Foo /*7*/
42+
////}
43+
////function fn8() {
44+
//// return <Foo cla/*8*/
45+
////}
46+
////function fn9() {
47+
//// return <Foo cla/*9*/ something
48+
////}
49+
////function fn10() {
50+
//// return <Foo something /*10*/
51+
////}
52+
////function fn11() {
53+
//// return <Foo something cla/*11*/
54+
////}
55+
56+
var preferences: FourSlashInterface.UserPreferences = {
57+
jsxAttributeCompletionStyle: "braces",
58+
includeCompletionsWithSnippetText: true,
59+
includeCompletionsWithInsertText: true,
60+
};
61+
62+
verify.completions(
63+
{ marker: "1", preferences, includes: { name: "className", insertText: "className={$1}", text: "(JSX attribute) className?: string", isSnippet: true, sortText: completion.SortText.OptionalMember } },
64+
{ marker: "2", preferences, includes: { name: "className", insertText: "className={$1}", text: "(JSX attribute) className?: string", isSnippet: true, sortText: completion.SortText.OptionalMember } },
65+
{ marker: "3", preferences, includes: { name: "className", insertText: "className={$1}", text: "(JSX attribute) className?: string", isSnippet: true, sortText: completion.SortText.OptionalMember } },
66+
{ marker: "4", preferences, includes: { name: "className", insertText: "className={$1}", text: "(JSX attribute) className?: string", isSnippet: true, sortText: completion.SortText.OptionalMember } },
67+
{ marker: "5", preferences, includes: { name: "className", insertText: "className={$1}", text: "(JSX attribute) className?: string", isSnippet: true, sortText: completion.SortText.OptionalMember } },
68+
{ marker: "6", preferences, includes: { name: "className", insertText: "className={$1}", text: "(JSX attribute) className?: string", isSnippet: true, sortText: completion.SortText.OptionalMember } },
69+
{ marker: "7", preferences, includes: { name: "className", insertText: "className={$1}", text: "(property) className?: string", isSnippet: true, sortText: completion.SortText.OptionalMember } },
70+
{ marker: "8", preferences, includes: { name: "className", insertText: "className={$1}", text: "(JSX attribute) className?: string", isSnippet: true, sortText: completion.SortText.OptionalMember } },
71+
{ marker: "9", preferences, includes: { name: "className", insertText: "className={$1}", text: "(JSX attribute) className?: string", isSnippet: true, sortText: completion.SortText.OptionalMember } },
72+
{ marker: "10", preferences, includes: { name: "className", insertText: "className={$1}", text: "(property) className?: string", isSnippet: true, sortText: completion.SortText.OptionalMember } },
73+
{ marker: "11", preferences, includes: { name: "className", insertText: "className={$1}", text: "(JSX attribute) className?: string", isSnippet: true, sortText: completion.SortText.OptionalMember } },
74+
)

Diff for: ‎tests/cases/fourslash/jsxTagNameCompletionClosed.ts

+54
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
/// <reference path="fourslash.ts" />
2+
//@Filename: file.tsx
3+
////interface NestedInterface {
4+
//// Foo: NestedInterface;
5+
//// (props: {}): any;
6+
////}
7+
////
8+
////declare const Foo: NestedInterface;
9+
////
10+
////function fn1() {
11+
//// return <Foo>
12+
//// </*1*/ />
13+
//// </Foo>
14+
////}
15+
////function fn2() {
16+
//// return <Foo>
17+
//// <Fo/*2*/ />
18+
//// </Foo>
19+
////}
20+
////function fn3() {
21+
//// return <Foo>
22+
//// <Foo./*3*/ />
23+
//// </Foo>
24+
////}
25+
////function fn4() {
26+
//// return <Foo>
27+
//// <Foo.F/*4*/ />
28+
//// </Foo>
29+
////}
30+
////function fn5() {
31+
//// return <Foo>
32+
//// <Foo.Foo./*5*/ />
33+
//// </Foo>
34+
////}
35+
////function fn6() {
36+
//// return <Foo>
37+
//// <Foo.Foo.F/*6*/ />
38+
//// </Foo>
39+
////}
40+
41+
var preferences: FourSlashInterface.UserPreferences = {
42+
jsxAttributeCompletionStyle: "braces",
43+
includeCompletionsWithSnippetText: true,
44+
includeCompletionsWithInsertText: true,
45+
};
46+
47+
verify.completions(
48+
{ marker: "1", preferences, includes: { name: "Foo", text: "const Foo: NestedInterface" } },
49+
{ marker: "2", preferences, includes: { name: "Foo", text: "const Foo: NestedInterface" } },
50+
{ marker: "3", preferences, includes: { name: "Foo", text: "(JSX attribute) NestedInterface.Foo: NestedInterface" } },
51+
{ marker: "4", preferences, includes: { name: "Foo", text: "(property) NestedInterface.Foo: NestedInterface" } },
52+
{ marker: "5", preferences, includes: { name: "Foo", text: "(JSX attribute) NestedInterface.Foo: NestedInterface" } },
53+
{ marker: "6", preferences, includes: { name: "Foo", text: "(property) NestedInterface.Foo: NestedInterface" } },
54+
)

0 commit comments

Comments
 (0)