Skip to content

fix: hyphened name not auto-completed by the ls#37455

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.

Sign up for GitHub

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
Orta Therox (orta) merged 3 commits into from
Mar 23, 2020
Merged

fix: hyphened name not auto-completed by the ls #37455

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f98172d
fix: hyphened name not auto-completed by the ls
Jack-Works Mar 18, 2020
5fbda85
fix: accept new baseline
Jack-Works Mar 19, 2020
bd9ba5e
Adds a test to validate the hypened identifiers in JSX
orta Mar 23, 2020
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
38 changes: 20 additions & 18 deletions src/compiler/scanner.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -308,14 +308,14 @@ namespace ts {
return languageVersion! >= ScriptTarget.ES2015 ?
lookupInUnicodeMap(code, unicodeESNextIdentifierStart) :
languageVersion! === ScriptTarget.ES5 ? lookupInUnicodeMap(code, unicodeES5IdentifierStart) :
lookupInUnicodeMap(code, unicodeES3IdentifierStart);
lookupInUnicodeMap(code, unicodeES3IdentifierStart);
}

function isUnicodeIdentifierPart(code: number, languageVersion: ScriptTarget | undefined) {
return languageVersion! >= ScriptTarget.ES2015 ?
lookupInUnicodeMap(code, unicodeESNextIdentifierPart) :
languageVersion! === ScriptTarget.ES5 ? lookupInUnicodeMap(code, unicodeES5IdentifierPart) :
lookupInUnicodeMap(code, unicodeES3IdentifierPart);
lookupInUnicodeMap(code, unicodeES3IdentifierPart);
}

function makeReverseMap(source: Map<number>): string[] {
Expand DownExpand Up@@ -349,7 +349,7 @@ namespace ts {
if (text.charCodeAt(pos) === CharacterCodes.lineFeed) {
pos++;
}
// falls through
// falls through
case CharacterCodes.lineFeed:
result.push(lineStart);
lineStart = pos;
Expand DownExpand Up@@ -503,8 +503,8 @@ namespace ts {
case CharacterCodes.formFeed:
case CharacterCodes.space:
case CharacterCodes.slash:
// starts of normal trivia
// falls through
// starts of normal trivia
// falls through
case CharacterCodes.lessThan:
case CharacterCodes.bar:
case CharacterCodes.equals:
Expand DownExpand Up@@ -533,7 +533,7 @@ namespace ts {
if (text.charCodeAt(pos + 1) === CharacterCodes.lineFeed) {
pos++;
}
// falls through
// falls through
case CharacterCodes.lineFeed:
pos++;
if (stopAfterLineBreak) {
Expand DownExpand Up@@ -715,7 +715,7 @@ namespace ts {
if (text.charCodeAt(pos + 1) === CharacterCodes.lineFeed) {
pos++;
}
// falls through
// falls through
case CharacterCodes.lineFeed:
pos++;
if (trailing) {
Expand DownExpand Up@@ -849,21 +849,23 @@ namespace ts {
ch > CharacterCodes.maxAsciiCharacter && isUnicodeIdentifierStart(ch, languageVersion);
}

export function isIdentifierPart(ch: number, languageVersion: ScriptTarget | undefined): boolean {
export function isIdentifierPart(ch: number, languageVersion: ScriptTarget | undefined, identifierVariant?: LanguageVariant): boolean {
return ch >= CharacterCodes.A && ch <= CharacterCodes.Z || ch >= CharacterCodes.a && ch <= CharacterCodes.z ||
ch >= CharacterCodes._0 && ch <= CharacterCodes._9 || ch === CharacterCodes.$ || ch === CharacterCodes._ ||
// "-" and ":" are valid in JSX Identifiers
(identifierVariant === LanguageVariant.JSX ? (ch === CharacterCodes.minus || ch === CharacterCodes.colon) : false) ||
ch > CharacterCodes.maxAsciiCharacter && isUnicodeIdentifierPart(ch, languageVersion);
}

/* @internal */
export function isIdentifierText(name: string, languageVersion: ScriptTarget | undefined): boolean {
export function isIdentifierText(name: string, languageVersion: ScriptTarget | undefined, identifierVariant?: LanguageVariant): boolean {
let ch = codePointAt(name, 0);
if (!isIdentifierStart(ch, languageVersion)) {
return false;
}

for (let i = charSize(ch); i < name.length; i += charSize(ch)) {
if (!isIdentifierPart(ch = codePointAt(name, i), languageVersion)) {
if (!isIdentifierPart(ch = codePointAt(name, i), languageVersion, identifierVariant)) {
return false;
}
}
Expand DownExpand Up@@ -1005,7 +1007,7 @@ namespace ts {
return result + text.substring(start, pos);
}

function scanNumber(): {type: SyntaxKind, value: string} {
function scanNumber(): {type: SyntaxKind, value: string} {
const start = pos;
const mainFragment = scanNumberFragment();
let decimalFragment: string | undefined;
Expand DownExpand Up@@ -1352,7 +1354,7 @@ namespace ts {
if (pos < end && text.charCodeAt(pos) === CharacterCodes.lineFeed) {
pos++;
}
// falls through
// falls through
case CharacterCodes.lineFeed:
case CharacterCodes.lineSeparator:
case CharacterCodes.paragraphSeparator:
Expand DownExpand Up@@ -1818,10 +1820,10 @@ namespace ts {
tokenFlags |= TokenFlags.Octal;
return token = SyntaxKind.NumericLiteral;
}
// This fall-through is a deviation from the EcmaScript grammar. The grammar says that a leading zero
// can only be followed by an octal digit, a dot, or the end of the number literal. However, we are being
// permissive and allowing decimal digits of the form 08* and 09* (which many browsers also do).
// falls through
// This fall-through is a deviation from the EcmaScript grammar. The grammar says that a leading zero
// can only be followed by an octal digit, a dot, or the end of the number literal. However, we are being
// permissive and allowing decimal digits of the form 08* and 09* (which many browsers also do).
// falls through
case CharacterCodes._1:
case CharacterCodes._2:
case CharacterCodes._3:
Expand DownExpand Up@@ -1860,8 +1862,8 @@ namespace ts {
return pos += 2, token = SyntaxKind.LessThanEqualsToken;
}
if (languageVariant === LanguageVariant.JSX &&
text.charCodeAt(pos + 1) === CharacterCodes.slash &&
text.charCodeAt(pos + 2) !== CharacterCodes.asterisk) {
text.charCodeAt(pos + 1) === CharacterCodes.slash &&
text.charCodeAt(pos + 2) !== CharacterCodes.asterisk) {
return pos += 2, token = SyntaxKind.LessThanSlashToken;
}
pos++;
Expand Down
30 changes: 20 additions & 10 deletions src/services/completions.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -232,6 +232,7 @@ namespace ts.Completions {
completionKind,
preferences,
propertyAccessToConvert,
completionData.isJsxIdentifierExpected,
isJsxInitializer,
recommendedCompletion,
symbolToOriginInfoMap,
Expand All@@ -255,6 +256,7 @@ namespace ts.Completions {
completionKind,
preferences,
propertyAccessToConvert,
completionData.isJsxIdentifierExpected,
isJsxInitializer,
recommendedCompletion,
symbolToOriginInfoMap,
Expand DownExpand Up@@ -441,6 +443,7 @@ namespace ts.Completions {
kind: CompletionKind,
preferences: UserPreferences,
propertyAccessToConvert?: PropertyAccessExpression,
jsxIdentifierExpected?: boolean,
isJsxInitializer?: IsJsxInitializer,
recommendedCompletion?: Symbol,
symbolToOriginInfoMap?: SymbolOriginInfoMap,
Expand All@@ -454,7 +457,7 @@ namespace ts.Completions {
const uniques = createMap<true>();
for (const symbol of symbols) {
const origin = symbolToOriginInfoMap ? symbolToOriginInfoMap[getSymbolId(symbol)] : undefined;
const info = getCompletionEntryDisplayNameForSymbol(symbol, target, origin, kind);
const info = getCompletionEntryDisplayNameForSymbol(symbol, target, origin, kind, !!jsxIdentifierExpected);
if (!info) {
continue;
}
Expand DownExpand Up@@ -564,7 +567,7 @@ namespace ts.Completions {
// completion entry.
return firstDefined(symbols, (symbol): SymbolCompletion | undefined => {
const origin = symbolToOriginInfoMap[getSymbolId(symbol)];
const info = getCompletionEntryDisplayNameForSymbol(symbol, compilerOptions.target!, origin, completionKind);
const info = getCompletionEntryDisplayNameForSymbol(symbol, compilerOptions.target!, origin, completionKind, completionData.isJsxIdentifierExpected);
return info && info.name === entryId.name && getSourceFromOrigin(origin) === entryId.source
? { type: "symbol" as const, symbol, location, symbolToOriginInfoMap, previousToken, isJsxInitializer, isTypeOnlyLocation }
: undefined;
Expand DownExpand Up@@ -716,6 +719,8 @@ namespace ts.Completions {
readonly insideJsDocTagTypeExpression: boolean;
readonly symbolToSortTextMap: SymbolSortTextMap;
readonly isTypeOnlyLocation: boolean;
/** In JSX tag name and attribute names, identifiers like "my-tag" or "aria-name" is valid identifier. */
readonly isJsxIdentifierExpected: boolean;
}
type Request = { readonly kind: CompletionDataKind.JsDocTagName | CompletionDataKind.JsDocTag } | { readonly kind: CompletionDataKind.JsDocParameterName, tag: JSDocParameterTag };

Expand DownExpand Up@@ -895,6 +900,7 @@ namespace ts.Completions {
let isRightOfOpenTag = false;
let isStartingCloseTag = false;
let isJsxInitializer: IsJsxInitializer = false;
let isJsxIdentifierExpected = false;

let location = getTouchingPropertyName(sourceFile, position);
if (contextToken) {
Expand DownExpand Up@@ -975,11 +981,12 @@ namespace ts.Completions {
if (!binaryExpressionMayBeOpenTag(parent as BinaryExpression)) {
break;
}
// falls through
// falls through

case SyntaxKind.JsxSelfClosingElement:
case SyntaxKind.JsxElement:
case SyntaxKind.JsxOpeningElement:
isJsxIdentifierExpected = true;
if (contextToken.kind === SyntaxKind.LessThanToken) {
isRightOfOpenTag = true;
location = contextToken;
Expand All@@ -992,6 +999,7 @@ namespace ts.Completions {
isJsxInitializer = true;
break;
case SyntaxKind.Identifier:
isJsxIdentifierExpected = true;
// For `<div x=[|f/**/|]`, `parent` will be `x` and `previousToken.parent` will be `f` (which is its own JsxAttribute)
// Note for `<div someBool f>` we don't want to treat this as a jsx inializer, instead it's the attribute name.
if (parent !== previousToken.parent &&
Expand DownExpand Up@@ -1066,7 +1074,8 @@ namespace ts.Completions {
isJsxInitializer,
insideJsDocTagTypeExpression,
symbolToSortTextMap,
isTypeOnlyLocation: isTypeOnly
isTypeOnlyLocation: isTypeOnly,
isJsxIdentifierExpected,
};

type JSDocTagWithTypeExpression = JSDocParameterTag | JSDocPropertyTag | JSDocReturnTag | JSDocTypeTag | JSDocTypedefTag;
Expand DownExpand Up@@ -1130,9 +1139,9 @@ namespace ts.Completions {
let insertQuestionDot = false;
if (type.isNullableType()) {
const canCorrectToQuestionDot =
isRightOfDot &&
!isRightOfQuestionDot &&
preferences.includeAutomaticOptionalChainCompletions !== false;
isRightOfDot &&
!isRightOfQuestionDot &&
preferences.includeAutomaticOptionalChainCompletions !== false;

if (canCorrectToQuestionDot || isRightOfQuestionDot) {
type = type.getNonNullableType();
Expand DownExpand Up@@ -1447,8 +1456,8 @@ namespace ts.Completions {
return insideJsDocTagTypeExpression
|| !isContextTokenValueLocation(contextToken) &&
(isPossiblyTypeArgumentPosition(contextToken, sourceFile, typeChecker)
|| isPartOfTypeNode(location)
|| isContextTokenTypeLocation(contextToken));
|| isPartOfTypeNode(location)
|| isContextTokenTypeLocation(contextToken));
}

function isContextTokenValueLocation(contextToken: Node) {
Expand DownExpand Up@@ -2399,6 +2408,7 @@ namespace ts.Completions {
target: ScriptTarget,
origin: SymbolOriginInfo | undefined,
kind: CompletionKind,
jsxIdentifierExpected: boolean,
): CompletionEntryDisplayNameForSymbol | undefined {
const name = originIsExport(origin) ? getNameForExportedSymbol(symbol, target) : symbol.name;
if (name === undefined
Expand All@@ -2411,7 +2421,7 @@ namespace ts.Completions {
}

const validNameResult: CompletionEntryDisplayNameForSymbol = { name, needsConvertPropertyAccess: false };
if (isIdentifierText(name, target) || symbol.valueDeclaration && isPrivateIdentifierPropertyDeclaration(symbol.valueDeclaration)) {
if (isIdentifierText(name, target, jsxIdentifierExpected ? LanguageVariant.JSX : LanguageVariant.Standard) || symbol.valueDeclaration && isPrivateIdentifierPropertyDeclaration(symbol.valueDeclaration)) {
return validNameResult;
}
switch (kind) {
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/api/tsserverlibrary.d.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -3394,7 +3394,7 @@ declare namespace ts {
/** Optionally, get the shebang */
function getShebang(text: string): string | undefined;
function isIdentifierStart(ch: number, languageVersion: ScriptTarget | undefined): boolean;
function isIdentifierPart(ch: number, languageVersion: ScriptTarget | undefined): boolean;
function isIdentifierPart(ch: number, languageVersion: ScriptTarget | undefined, identifierVariant?: LanguageVariant): boolean;
function createScanner(languageVersion: ScriptTarget, skipTrivia: boolean, languageVariant?: LanguageVariant, textInitial?: string, onError?: ErrorCallback, start?: number, length?: number): Scanner;
}
declare namespace ts {
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/api/typescript.d.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -3394,7 +3394,7 @@ declare namespace ts {
/** Optionally, get the shebang */
function getShebang(text: string): string | undefined;
function isIdentifierStart(ch: number, languageVersion: ScriptTarget | undefined): boolean;
function isIdentifierPart(ch: number, languageVersion: ScriptTarget | undefined): boolean;
function isIdentifierPart(ch: number, languageVersion: ScriptTarget | undefined, identifierVariant?: LanguageVariant): boolean;
function createScanner(languageVersion: ScriptTarget, skipTrivia: boolean, languageVariant?: LanguageVariant, textInitial?: string, onError?: ErrorCallback, start?: number, length?: number): Scanner;
}
declare namespace ts {
Expand Down
25 changes: 18 additions & 7 deletions tests/cases/fourslash/completionsInJsxTag.ts
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -9,6 +9,8 @@
//// div: {
//// /** Doc */
//// foo: string
//// /** Label docs */
//// "aria-label": string
//// }
//// }
////}
Expand All@@ -21,11 +23,20 @@

verify.completions({
marker: ["1", "2"],
exact: {
name: "foo",
text: "(JSX attribute) foo: string",
documentation: "Doc",
kind: "JSX attribute",
kindModifiers: "declare",
},
exact: [
{
name: "foo",
text: "(JSX attribute) foo: string",
documentation: "Doc",
kind: "JSX attribute",
kindModifiers: "declare",
},
{
name: "aria-label",
text: "(JSX attribute) \"aria-label\": string",
documentation: "Label docs",
kind: "JSX attribute",
kindModifiers: "declare",
},
],
});