Skip to content

Commit 5bdee45

Browse files
jakebaileyjakebailey
committed
Overhaul detection of JSX attributes and tag names
1 parent db9e007 commit 5bdee45

13 files changed

+353
-11
lines changed

Diff for: ‎src/services/completions.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -739,7 +739,7 @@ namespace ts.Completions {
739739
}
740740
}
741741

742-
const kind = SymbolDisplay.getSymbolKind(typeChecker, symbol, location);
742+
const kind = SymbolDisplay.getSymbolKind(typeChecker, symbol, location, contextToken);
743743
if (kind === ScriptElementKind.jsxAttribute && preferences.includeCompletionsWithSnippetText && preferences.jsxAttributeCompletionStyle && preferences.jsxAttributeCompletionStyle !== "none") {
744744
let useBraces = preferences.jsxAttributeCompletionStyle === "braces";
745745
const type = typeChecker.getTypeOfSymbolAtLocation(symbol, location);

Diff for: ‎src/services/symbolDisplay.ts

+61-3
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@ namespace ts.SymbolDisplay {
33
const symbolDisplayNodeBuilderFlags = NodeBuilderFlags.OmitParameterModifiers | NodeBuilderFlags.IgnoreErrors | NodeBuilderFlags.UseAliasDefinedOutsideCurrentScope;
44

55
// TODO(drosen): use contextual SemanticMeaning.
6-
export function getSymbolKind(typeChecker: TypeChecker, symbol: Symbol, location: Node): ScriptElementKind {
7-
const result = getSymbolKindOfConstructorPropertyMethodAccessorFunctionOrVar(typeChecker, symbol, location);
6+
export function getSymbolKind(typeChecker: TypeChecker, symbol: Symbol, location: Node, contextToken?: Node): ScriptElementKind {
7+
const result = getSymbolKindOfConstructorPropertyMethodAccessorFunctionOrVar(typeChecker, symbol, location, contextToken);
88
if (result !== ScriptElementKind.unknown) {
99
return result;
1010
}
@@ -25,7 +25,7 @@ namespace ts.SymbolDisplay {
2525
return result;
2626
}
2727

28-
function getSymbolKindOfConstructorPropertyMethodAccessorFunctionOrVar(typeChecker: TypeChecker, symbol: Symbol, location: Node): ScriptElementKind {
28+
function getSymbolKindOfConstructorPropertyMethodAccessorFunctionOrVar(typeChecker: TypeChecker, symbol: Symbol, location: Node, contextToken?: Node): ScriptElementKind {
2929
const roots = typeChecker.getRootSymbols(symbol);
3030
// If this is a method from a mapped type, leave as a method so long as it still has a call signature.
3131
if (roots.length === 1
@@ -83,6 +83,20 @@ namespace ts.SymbolDisplay {
8383
}
8484
return unionPropertyKind;
8585
}
86+
87+
if (contextToken) {
88+
const result = getSymbolKindOfJsxTagNameOrAttribute(location, contextToken);
89+
if (result !== ScriptElementKind.unknown) {
90+
return result;
91+
}
92+
}
93+
94+
if (isJsxAttribute(location) || location.parent && isJsxAttribute(location.parent) && location.parent.name === location) {
95+
return ScriptElementKind.jsxAttribute;
96+
}
97+
98+
// TODO(jakebailey): Delete the below code, once the edge cases it handles are handled above.
99+
86100
// If we requested completions after `x.` at the top-level, we may be at a source file location.
87101
switch (location.parent && location.parent.kind) {
88102
// If we've typed a character of the attribute name, will be 'JsxAttribute', else will be 'JsxOpeningElement'.
@@ -100,6 +114,50 @@ namespace ts.SymbolDisplay {
100114
return ScriptElementKind.unknown;
101115
}
102116

117+
function getSymbolKindOfJsxTagNameOrAttribute(location: Node, contextToken: Node): ScriptElementKind {
118+
const symbolKindFromContext = forEachAncestor(contextToken, (n) => {
119+
if (isJsxAttributeLike(n)) {
120+
return ScriptElementKind.jsxAttribute;
121+
}
122+
123+
if (isJsxFragment(n) || isJsxOpeningFragment(n) || isJsxClosingFragment(n)) {
124+
return "quit";
125+
}
126+
127+
if (isJsxOpeningElement(n) || isJsxSelfClosingElement(n) || isJsxClosingElement(n)) {
128+
if (contextToken.getEnd() <= n.tagName.getFullStart()) {
129+
// Definitely completing part of the tag name.
130+
return ScriptElementKind.jsxTagName;
131+
}
132+
133+
if (rangeContainsRange(n.tagName, contextToken)) {
134+
// We are to the right of the tag name, as the context is there.
135+
// figure out where we are based on where the location is.
136+
137+
// TODO(jakebailey): This seems hacky.
138+
if (contextToken.kind === SyntaxKind.DotToken || contextToken.kind === SyntaxKind.QuestionDotToken) {
139+
// Unfinished dotted tag name.
140+
return ScriptElementKind.jsxTagName;
141+
}
142+
143+
if (!rangeContainsRange(n, location)) {
144+
// Unclosed JSX element; location is entirely outside the element.
145+
return ScriptElementKind.jsxAttribute;
146+
}
147+
148+
if (n.tagName.getEnd() <= location.getFullStart()) {
149+
// After existing attributes, so is another attribute.
150+
return ScriptElementKind.jsxAttribute;
151+
}
152+
}
153+
154+
return "quit";
155+
}
156+
});
157+
158+
return symbolKindFromContext || ScriptElementKind.unknown;
159+
}
160+
103161
function getNormalizedSymbolModifiers(symbol: Symbol) {
104162
if (symbol.declarations && symbol.declarations.length) {
105163
const [declaration, ...declarations] = symbol.declarations;

Diff for: ‎src/services/types.ts

+2
Original file line numberDiff line numberDiff line change
@@ -1457,6 +1457,8 @@ namespace ts {
14571457
/**
14581458
* <JsxTagName attribute1 attribute2={0} />
14591459
*/
1460+
jsxTagName = "JSX tag name",
1461+
14601462
jsxAttribute = "JSX attribute",
14611463

14621464
/** String literal */

Diff for: ‎src/services/utilities.ts

+5-5
Original file line numberDiff line numberDiff line change
@@ -1106,16 +1106,16 @@ namespace ts {
11061106
* If position === end, returns the preceding token if includeItemAtEndPosition(previousToken) === true
11071107
*/
11081108
export function getTouchingToken(sourceFile: SourceFile, position: number, includePrecedingTokenAtEndPosition?: (n: Node) => boolean): Node {
1109-
return getTokenAtPositionWorker(sourceFile, position, /*allowPositionInLeadingTrivia*/ false, includePrecedingTokenAtEndPosition, /*includeEndPosition*/ false);
1109+
return getTokenAtPositionWorker(sourceFile, position, () => false, includePrecedingTokenAtEndPosition, /*includeEndPosition*/ false);
11101110
}
11111111

11121112
/** Returns a token if position is in [start-of-leading-trivia, end) */
11131113
export function getTokenAtPosition(sourceFile: SourceFile, position: number): Node {
1114-
return getTokenAtPositionWorker(sourceFile, position, /*allowPositionInLeadingTrivia*/ true, /*includePrecedingTokenAtEndPosition*/ undefined, /*includeEndPosition*/ false);
1114+
return getTokenAtPositionWorker(sourceFile, position, () => true, /*includePrecedingTokenAtEndPosition*/ undefined, /*includeEndPosition*/ false);
11151115
}
11161116

11171117
/** Get the token whose text contains the position */
1118-
function getTokenAtPositionWorker(sourceFile: SourceFile, position: number, allowPositionInLeadingTrivia: boolean, includePrecedingTokenAtEndPosition: ((n: Node) => boolean) | undefined, includeEndPosition: boolean): Node {
1118+
function getTokenAtPositionWorker(sourceFile: SourceFile, position: number, allowPositionInLeadingTrivia: (n: Node) => boolean, includePrecedingTokenAtEndPosition: ((n: Node) => boolean) | undefined, includeEndPosition: boolean): Node {
11191119
let current: Node = sourceFile;
11201120
let foundToken: Node | undefined;
11211121
outer: while (true) {
@@ -1145,7 +1145,7 @@ namespace ts {
11451145
// position and whose end is greater than the position.
11461146

11471147

1148-
const start = allowPositionInLeadingTrivia ? children[middle].getFullStart() : children[middle].getStart(sourceFile, /*includeJsDoc*/ true);
1148+
const start = allowPositionInLeadingTrivia(children[middle]) ? children[middle].getFullStart() : children[middle].getStart(sourceFile, /*includeJsDoc*/ true);
11491149
if (start > position) {
11501150
return Comparison.GreaterThan;
11511151
}
@@ -1180,7 +1180,7 @@ namespace ts {
11801180
}
11811181

11821182
function nodeContainsPosition(node: Node) {
1183-
const start = allowPositionInLeadingTrivia ? node.getFullStart() : node.getStart(sourceFile, /*includeJsDoc*/ true);
1183+
const start = allowPositionInLeadingTrivia(node) ? node.getFullStart() : node.getStart(sourceFile, /*includeJsDoc*/ true);
11841184
if (start > position) {
11851185
// If this child begins after position, then all subsequent children will as well.
11861186
return false;

Diff for: ‎tests/baselines/reference/api/tsserverlibrary.d.ts

+1
Original file line numberDiff line numberDiff line change
@@ -6594,6 +6594,7 @@ declare namespace ts {
65946594
/**
65956595
* <JsxTagName attribute1 attribute2={0} />
65966596
*/
6597+
jsxTagName = "JSX tag name",
65976598
jsxAttribute = "JSX attribute",
65986599
/** String literal */
65996600
string = "string",

Diff for: ‎tests/baselines/reference/api/typescript.d.ts

+1
Original file line numberDiff line numberDiff line change
@@ -6594,6 +6594,7 @@ declare namespace ts {
65946594
/**
65956595
* <JsxTagName attribute1 attribute2={0} />
65966596
*/
6597+
jsxTagName = "JSX tag name",
65976598
jsxAttribute = "JSX attribute",
65986599
/** String literal */
65996600
string = "string",

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

+69
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
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+
57+
verify.completions(
58+
{
59+
marker: test.markers(),
60+
includes: [
61+
{ name: "className", insertText: 'className={$1}', isSnippet: true, sortText: completion.SortText.OptionalMember }
62+
],
63+
preferences: {
64+
jsxAttributeCompletionStyle: "braces",
65+
includeCompletionsWithSnippetText: true,
66+
includeCompletionsWithInsertText: true,
67+
},
68+
}
69+
)

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

+68
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
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+
verify.completions(
57+
{
58+
marker: test.markers(),
59+
includes: [
60+
{ name: "className", insertText: 'className={$1}', isSnippet: true, sortText: completion.SortText.OptionalMember }
61+
],
62+
preferences: {
63+
jsxAttributeCompletionStyle: "braces",
64+
includeCompletionsWithSnippetText: true,
65+
includeCompletionsWithInsertText: true,
66+
},
67+
}
68+
)

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

+53
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
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+
verify.completions(
42+
{
43+
marker: test.markers(),
44+
includes: [
45+
{ name: "Foo", insertText: undefined, isSnippet: undefined }
46+
],
47+
preferences: {
48+
jsxAttributeCompletionStyle: "braces",
49+
includeCompletionsWithSnippetText: true,
50+
includeCompletionsWithInsertText: true,
51+
},
52+
}
53+
)

0 commit comments

Comments
 (0)