Skip to content

Avoid adding a dot when comment a property acccess's expression has trailing comments #28888

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
merged 8 commits into from
Dec 13, 2018
Merged

Avoid adding a dot when comment a property acccess's expression has trailing comments #28888

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
3fe7263
Added test case for trivia preceding dot in numeric LHS in property a…
DanielRosenwasser Dec 5, 2018
87e45f9
Accepted baselines.
DanielRosenwasser Dec 5, 2018
6fcc815
Check if the expression had any trailing comments.
DanielRosenwasser Dec 7, 2018
75c51f5
Accepted baselines.
DanielRosenwasser Dec 7, 2018
2e9be8c
Added more tests, including some in 'removeComments' mode.
DanielRosenwasser Dec 11, 2018
9a6d2ee
Accepted baselines.
DanielRosenwasser Dec 11, 2018
2999a1e
Merge remote-tracking branch 'origin/master' into numericPropertyAccess
DanielRosenwasser Dec 12, 2018
caa7441
Accepted baselines.
DanielRosenwasser Dec 12, 2018
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
23 changes: 16 additions & 7 deletions src/compiler/emitter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1651,11 +1651,17 @@ namespace ts {
function emitPropertyAccessExpression(node: PropertyAccessExpression) {
let indentBeforeDot = false;
let indentAfterDot = false;
const dotRangeFirstCommentStart = skipTrivia(
currentSourceFile!.text,
node.expression.end,
/*stopAfterLineBreak*/ false,
/*stopAtComments*/ true
);
const dotRangeStart = skipTrivia(currentSourceFile!.text, dotRangeFirstCommentStart);
const dotRangeEnd = dotRangeStart + 1;
if (!(getEmitFlags(node) & EmitFlags.NoIndentation)) {
const dotRangeStart = node.expression.end;
const dotRangeEnd = skipTrivia(currentSourceFile!.text, node.expression.end) + 1;
const dotToken = createToken(SyntaxKind.DotToken);
dotToken.pos = dotRangeStart;
dotToken.pos = node.expression.end;
dotToken.end = dotRangeEnd;
indentBeforeDot = needsIndentation(node, node.expression, dotToken);
indentAfterDot = needsIndentation(node, dotToken, node.name);
Expand All @@ -1664,7 +1670,8 @@ namespace ts {
emitExpression(node.expression);
increaseIndentIf(indentBeforeDot, /*writeSpaceIfNotIndenting*/ false);

const shouldEmitDotDot = !indentBeforeDot && needsDotDotForPropertyAccess(node.expression);
const dotHasCommentTrivia = dotRangeFirstCommentStart !== dotRangeStart;
const shouldEmitDotDot = !indentBeforeDot && needsDotDotForPropertyAccess(node.expression, dotHasCommentTrivia);
if (shouldEmitDotDot) {
writePunctuation(".");
}
Expand All @@ -1677,13 +1684,15 @@ namespace ts {

// 1..toString is a valid property access, emit a dot after the literal
// Also emit a dot if expression is a integer const enum value - it will appear in generated code as numeric literal
function needsDotDotForPropertyAccess(expression: Expression) {
function needsDotDotForPropertyAccess(expression: Expression, dotHasTrivia: boolean) {
expression = skipPartiallyEmittedExpressions(expression);
if (isNumericLiteral(expression)) {
// check if numeric literal is a decimal literal that was originally written with a dot
const text = getLiteralTextOfNode(<LiteralExpression>expression, /*neverAsciiEscape*/ true);
return !expression.numericLiteralFlags
&& !stringContains(text, tokenToString(SyntaxKind.DotToken)!);
// If he number will be printed verbatim and it doesn't already contain a dot, add one
// if the expression doesn't have any comments that will be emitted.
return !expression.numericLiteralFlags && !stringContains(text, tokenToString(SyntaxKind.DotToken)!) &&
(!dotHasTrivia || printerOptions.removeComments);
}
else if (isPropertyAccessExpression(expression) || isElementAccessExpression(expression)) {
// check if constant enum value is integer
Expand Down
25 changes: 21 additions & 4 deletions tests/baselines/reference/numericLiteralsWithTrailingDecimalPoints01.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ tests/cases/compiler/numericLiteralsWithTrailingDecimalPoints01.ts(9,24): error
!!! error TS1351: An identifier or keyword cannot immediately follow a numeric literal.
1.+2.0 + 3. ;

// Preserve whitespace where important for JS compatibility
// Preserve whitespace and comments where important for JS compatibility
var i: number = 1;
var test1 = i.toString();
var test1 = i.toString();
var test2 = 2.toString();
~~~~~~~~
!!! error TS1351: An identifier or keyword cannot immediately follow a numeric literal.
Expand All @@ -24,10 +24,27 @@ tests/cases/compiler/numericLiteralsWithTrailingDecimalPoints01.ts(9,24): error
!!! error TS1109: Expression expected.
var test3 = 3 .toString();
var test4 = 3 .toString();
var test5 = 3 .toString();
var test5 = 3 .toString();
var test6 = 3.['toString']();
var test7 = 3
.toString();
var test8 = new Number(4).toString();
var test9 = 3. + 3.
var test9 = 3. + 3.;
var test10 = 0 /* comment */.toString();
var test11 = 3. /* comment */ .toString();
var test12 = 3
/* comment */ .toString();
var test13 = 3.
/* comment */ .toString();
var test14 = 3
// comment
.toString();
var test15 = 3.
// comment
.toString();
var test16 = 3 // comment time
.toString();
var test17 = 3. // comment time again
.toString();


43 changes: 38 additions & 5 deletions tests/baselines/reference/numericLiteralsWithTrailingDecimalPoints01.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,35 @@
1.toString();
1.+2.0 + 3. ;

// Preserve whitespace where important for JS compatibility
// Preserve whitespace and comments where important for JS compatibility
var i: number = 1;
var test1 = i.toString();
var test1 = i.toString();
var test2 = 2.toString();
var test3 = 3 .toString();
var test4 = 3 .toString();
var test5 = 3 .toString();
var test5 = 3 .toString();
var test6 = 3.['toString']();
var test7 = 3
.toString();
var test8 = new Number(4).toString();
var test9 = 3. + 3.
var test9 = 3. + 3.;
var test10 = 0 /* comment */.toString();
var test11 = 3. /* comment */ .toString();
var test12 = 3
/* comment */ .toString();
var test13 = 3.
/* comment */ .toString();
var test14 = 3
// comment
.toString();
var test15 = 3.
// comment
.toString();
var test16 = 3 // comment time
.toString();
var test17 = 3. // comment time again
.toString();



//// [numericLiteralsWithTrailingDecimalPoints01.js]
Expand All @@ -24,7 +41,7 @@ var test9 = 3. + 3.
1.;
toString();
1. + 2.0 + 3.;
// Preserve whitespace where important for JS compatibility
// Preserve whitespace and comments where important for JS compatibility
var i = 1;
var test1 = i.toString();
var test2 = 2., toString;
Expand All @@ -37,3 +54,19 @@ var test7 = 3
.toString();
var test8 = new Number(4).toString();
var test9 = 3. + 3.;
var test10 = 0 /* comment */.toString();
var test11 = 3. /* comment */.toString();
var test12 = 3
/* comment */ .toString();
var test13 = 3.
/* comment */ .toString();
var test14 = 3
// comment
.toString();
var test15 = 3.
// comment
.toString();
var test16 = 3 // comment time
.toString();
var test17 = 3. // comment time again
.toString();
63 changes: 59 additions & 4 deletions tests/baselines/reference/numericLiteralsWithTrailingDecimalPoints01.symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@

1.+2.0 + 3. ;

// Preserve whitespace where important for JS compatibility
// Preserve whitespace and comments where important for JS compatibility
var i: number = 1;
>i : Symbol(i, Decl(numericLiteralsWithTrailingDecimalPoints01.ts, 6, 3))

var test1 = i.toString();
var test1 = i.toString();
>test1 : Symbol(test1, Decl(numericLiteralsWithTrailingDecimalPoints01.ts, 7, 3))
>i.toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --))
>i : Symbol(i, Decl(numericLiteralsWithTrailingDecimalPoints01.ts, 6, 3))
Expand All @@ -36,7 +36,7 @@ var test4 = 3 .toString();
>3 .toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --))
>toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --))

var test5 = 3 .toString();
var test5 = 3 .toString();
>test5 : Symbol(test5, Decl(numericLiteralsWithTrailingDecimalPoints01.ts, 11, 3))
>3 .toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --))
>toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --))
Expand All @@ -58,6 +58,61 @@ var test8 = new Number(4).toString();
>Number : Symbol(Number, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --))
>toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --))

var test9 = 3. + 3.
var test9 = 3. + 3.;
>test9 : Symbol(test9, Decl(numericLiteralsWithTrailingDecimalPoints01.ts, 16, 3))

var test10 = 0 /* comment */.toString();
>test10 : Symbol(test10, Decl(numericLiteralsWithTrailingDecimalPoints01.ts, 17, 3))
>0 /* comment */.toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --))
>toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --))

var test11 = 3. /* comment */ .toString();
>test11 : Symbol(test11, Decl(numericLiteralsWithTrailingDecimalPoints01.ts, 18, 3))
>3. /* comment */ .toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --))
>toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --))

var test12 = 3
>test12 : Symbol(test12, Decl(numericLiteralsWithTrailingDecimalPoints01.ts, 19, 3))
>3 /* comment */ .toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --))

/* comment */ .toString();
>toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --))

var test13 = 3.
>test13 : Symbol(test13, Decl(numericLiteralsWithTrailingDecimalPoints01.ts, 21, 3))
>3. /* comment */ .toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --))

/* comment */ .toString();
>toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --))

var test14 = 3
>test14 : Symbol(test14, Decl(numericLiteralsWithTrailingDecimalPoints01.ts, 23, 3))
>3 // comment .toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --))

// comment
.toString();
>toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --))

var test15 = 3.
>test15 : Symbol(test15, Decl(numericLiteralsWithTrailingDecimalPoints01.ts, 26, 3))
>3. // comment .toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --))

// comment
.toString();
>toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --))

var test16 = 3 // comment time
>test16 : Symbol(test16, Decl(numericLiteralsWithTrailingDecimalPoints01.ts, 29, 3))
>3 // comment time .toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --))

.toString();
>toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --))

var test17 = 3. // comment time again
>test17 : Symbol(test17, Decl(numericLiteralsWithTrailingDecimalPoints01.ts, 31, 3))
>3. // comment time again .toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --))

.toString();
>toString : Symbol(Number.toString, Decl(lib.es5.d.ts, --, --))


79 changes: 75 additions & 4 deletions tests/baselines/reference/numericLiteralsWithTrailingDecimalPoints01.types
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,12 @@
>2.0 : 2
>3. : 3

// Preserve whitespace where important for JS compatibility
// Preserve whitespace and comments where important for JS compatibility
var i: number = 1;
>i : number
>1 : 1

var test1 = i.toString();
var test1 = i.toString();
>test1 : string
>i.toString() : string
>i.toString : (radix?: number) => string
Expand Down Expand Up @@ -56,7 +56,7 @@ var test4 = 3 .toString();
>3 : 3
>toString : (radix?: number) => string

var test5 = 3 .toString();
var test5 = 3 .toString();
>test5 : string
>3 .toString() : string
>3 .toString : (radix?: number) => string
Expand Down Expand Up @@ -88,9 +88,80 @@ var test8 = new Number(4).toString();
>4 : 4
>toString : (radix?: number) => string

var test9 = 3. + 3.
var test9 = 3. + 3.;
>test9 : number
>3. + 3. : number
>3. : 3
>3. : 3

var test10 = 0 /* comment */.toString();
>test10 : string
>0 /* comment */.toString() : string
>0 /* comment */.toString : (radix?: number) => string
>0 : 0
>toString : (radix?: number) => string

var test11 = 3. /* comment */ .toString();
>test11 : string
>3. /* comment */ .toString() : string
>3. /* comment */ .toString : (radix?: number) => string
>3. : 3
>toString : (radix?: number) => string

var test12 = 3
>test12 : string
>3 /* comment */ .toString() : string
>3 /* comment */ .toString : (radix?: number) => string
>3 : 3

/* comment */ .toString();
>toString : (radix?: number) => string

var test13 = 3.
>test13 : string
>3. /* comment */ .toString() : string
>3. /* comment */ .toString : (radix?: number) => string
>3. : 3

/* comment */ .toString();
>toString : (radix?: number) => string

var test14 = 3
>test14 : string
>3 // comment .toString() : string
>3 // comment .toString : (radix?: number) => string
>3 : 3

// comment
.toString();
>toString : (radix?: number) => string

var test15 = 3.
>test15 : string
>3. // comment .toString() : string
>3. // comment .toString : (radix?: number) => string
>3. : 3

// comment
.toString();
>toString : (radix?: number) => string

var test16 = 3 // comment time
>test16 : string
>3 // comment time .toString() : string
>3 // comment time .toString : (radix?: number) => string
>3 : 3

.toString();
>toString : (radix?: number) => string

var test17 = 3. // comment time again
>test17 : string
>3. // comment time again .toString() : string
>3. // comment time again .toString : (radix?: number) => string
>3. : 3

.toString();
>toString : (radix?: number) => string


Loading