Skip to content
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

Lexical classification for template strings #1744

Closed
wants to merge 7 commits into from
Closed

Lexical classification for template strings #1744

Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7bd35f7
Rudimentary, but imperfect, lexical classification for templates.
DanielRosenwasser Jan 20, 2015
796eeb5
Fixed issue with the kinds we check.
DanielRosenwasser Jan 20, 2015
a6e94fd
Addressed CR feedback.
DanielRosenwasser Jan 26, 2015
a7f35c5
classifyKeywordsInGenerics -> syntacticClassifierAbsent
DanielRosenwasser Jan 26, 2015
570a36b
Added tests, fixed order of emptying templateStack, unconditionally p…
DanielRosenwasser Jan 26, 2015
c470fcd
Fixed assertion
DanielRosenwasser Jan 26, 2015
31b0f66
Added comment for 'syntacticClassifierAbsent'.
DanielRosenwasser Jan 27, 2015
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
97 changes: 87 additions & 10 deletions src/services/services.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1451,6 +1451,9 @@ module ts {
InMultiLineCommentTrivia,
InSingleQuoteStringLiteral,
InDoubleQuoteStringLiteral,
InTemplateHeadOrNoSubstitutionTemplate,
InTemplateMiddleOrTail,
InTemplateSubstitutionPosition,
}

export enum TokenClass {
Expand Down Expand Up @@ -5671,12 +5674,12 @@ module ts {
// if there are more cases we want the classifier to be better at.
return true;
}

// 'classifyKeywordsInGenerics' should be 'true' when a syntactic classifier is not present.
function getClassificationsForLine(text: string, lexState: EndOfLineState, classifyKeywordsInGenerics?: boolean): ClassificationResult {

function getClassificationsForLine(text: string, lexState: EndOfLineState, syntacticClassifierAbsent?: boolean): ClassificationResult {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a comment on this function, syntacticClassifierAbsent is now more interesting. you want to add comments on what it does.

var offset = 0;
var token = SyntaxKind.Unknown;
var lastNonTriviaToken = SyntaxKind.Unknown;
var templateStack: SyntaxKind[];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just initialie this to []

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think one stack is sufficient. just make sure to clear it every call.


// If we're in a string literal, then prepend: "\
// (and a newline). That way when we lex we'll think we're still in a string literal.
Expand All @@ -5696,6 +5699,23 @@ module ts {
text = "/*\n" + text;
offset = 3;
break;
case EndOfLineState.InTemplateHeadOrNoSubstitutionTemplate:
if (syntacticClassifierAbsent) {
text = "`\n" + text;
offset = 2;
}
break;
case EndOfLineState.InTemplateMiddleOrTail:
if (syntacticClassifierAbsent) {
text = "}\n" + text;
offset = 2;
}
// fallthrough
case EndOfLineState.InTemplateSubstitutionPosition:
if (syntacticClassifierAbsent) {
templateStack = [SyntaxKind.TemplateHead];
}
break;
}

scanner.setText(text);
Expand Down Expand Up @@ -5757,16 +5777,54 @@ module ts {
angleBracketStack--;
}
else if (token === SyntaxKind.AnyKeyword ||
token === SyntaxKind.StringKeyword ||
token === SyntaxKind.NumberKeyword ||
token === SyntaxKind.BooleanKeyword) {
if (angleBracketStack > 0 && !classifyKeywordsInGenerics) {
token === SyntaxKind.StringKeyword ||
token === SyntaxKind.NumberKeyword ||
token === SyntaxKind.BooleanKeyword) {
if (angleBracketStack > 0 && !syntacticClassifierAbsent) {
// If it looks like we're could be in something generic, don't classify this
// as a keyword. We may just get overwritten by the syntactic classifier,
// causing a noisy experience for the user.
token = SyntaxKind.Identifier;
}
}
else if (token === SyntaxKind.TemplateHead && syntacticClassifierAbsent) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you wrap this in a big if (syntacticClassifierAbsent) clause

if (!templateStack) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you just initialize templateStack, then you don't need this check.

templateStack = [token];
}
else {
templateStack.push(token);
}
}
else if (token === SyntaxKind.OpenBraceToken && syntacticClassifierAbsent) {
// If we don't have anything on the template stack,
// then we aren't trying to keep track of a previously scanned template head.
if (templateStack && templateStack.length > 0) {
templateStack.push(token);
}
}
else if (token === SyntaxKind.CloseBraceToken && syntacticClassifierAbsent) {
// If we don't have anything on the template stack,
// then we aren't trying to keep track of a previously scanned template head.
if (templateStack && templateStack.length > 0) {
var lastTemplateStackToken = lastOrUndefined(templateStack);

if (lastTemplateStackToken === SyntaxKind.TemplateHead) {
token = scanner.reScanTemplateToken();

// Only pop on a TemplateTail; a TemplateMiddle indicates there is more for us.
if (token === SyntaxKind.TemplateTail) {
templateStack.pop();
}
else {
Debug.assert(token === SyntaxKind.TemplateMiddle, "Should have been a template middle. Was " + token);
}
}
else {
Debug.assert(token === SyntaxKind.CloseBraceToken, "Should have been an open brace. Was: " + token);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you mean OpenBranceToken. i do not think you ever push anything that is not a templateHead or an OpenBranceToken

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, maybe we can remove the assertion later on, but I'd feel more comfortable with it for now.

templateStack.pop();
}
}
}

lastNonTriviaToken = token;
}
Expand All @@ -5781,8 +5839,7 @@ module ts {
var start = scanner.getTokenPos();
var end = scanner.getTextPos();

// add the token
addResult(end - start, classFromKind(token));
addResult(end - start, classFromKind(token, syntacticClassifierAbsent));

if (end >= text.length) {
if (token === SyntaxKind.StringLiteral) {
Expand Down Expand Up @@ -5811,6 +5868,22 @@ module ts {
result.finalLexState = EndOfLineState.InMultiLineCommentTrivia;
}
}
else if (isTemplateLiteralKind(token) && syntacticClassifierAbsent) {
if (scanner.isUnterminated()) {
if (token === SyntaxKind.TemplateTail) {
result.finalLexState = EndOfLineState.InTemplateMiddleOrTail;
}
else if (token === SyntaxKind.NoSubstitutionTemplateLiteral) {
result.finalLexState = EndOfLineState.InTemplateHeadOrNoSubstitutionTemplate;
}
else {
Debug.fail("Only 'NoSubstitutionTemplateLiteral's and 'TemplateTail's can be unterminated; got SyntaxKind #" + token);
}
}
}
else if (templateStack && templateStack.length > 0 && lastOrUndefined(templateStack) === SyntaxKind.TemplateHead) {
result.finalLexState = EndOfLineState.InTemplateSubstitutionPosition;
}
}
}

Expand Down Expand Up @@ -5888,7 +5961,7 @@ module ts {
return token >= SyntaxKind.FirstKeyword && token <= SyntaxKind.LastKeyword;
}

function classFromKind(token: SyntaxKind) {
function classFromKind(token: SyntaxKind, syntacticClassifierAbsent?: boolean) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make this non-optional.

if (isKeyword(token)) {
return TokenClass.Keyword;
}
Expand All @@ -5913,6 +5986,10 @@ module ts {
return TokenClass.Whitespace;
case SyntaxKind.Identifier:
default:
// Only give a classification if nothing will more accurately classify.
if (syntacticClassifierAbsent && isTemplateLiteralKind(token)) {
return TokenClass.StringLiteral; // should make a TemplateLiteral
}
return TokenClass.Identifier;
}
}
Expand Down