Skip to content

Commit

Permalink
Fix compiler crash during a special case with multi-line template lit…
Browse files Browse the repository at this point in the history
…erals.

The error occurs when printing the code using the CompactCodePrinter, when the multi-line template literal falls on the last line. In the CompactCodePrinter, there is logic to combine the last line with the line above, but this breaks if the "last line" contains line breaks (within template literals). The CodePrinter will mistakenly think that the last line is longer than it actually is.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=220862628
  • Loading branch information
EatingW authored and blickly committed Nov 12, 2018
1 parent 73cea14 commit 96841ee
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 28 deletions.
80 changes: 52 additions & 28 deletions src/com/google/javascript/jscomp/CodePrinter.java
Expand Up @@ -145,43 +145,54 @@ void generateSourceMap(String code, SourceMap map) {
}

/**
* Reports to the code consumer that the given line has been cut at the
* given position, i.e. a \n has been inserted there. Or that a cut has
* been undone, i.e. a previously inserted \n has been removed.
* All mappings in the source maps after that position will be renormalized
* as needed.
* Reports to the code consumer that the given line has been cut at the given position, i.e. a
* \n has been inserted there. Or that a cut has been undone, i.e. a previously inserted \n has
* been removed. All mappings in the source maps after that position will be renormalized as
* needed.
*
* @param lineIndex The index of the line at which the newline was inserted/removed.
* @param charIndex The position on the line at which the newline was inserted./removed
* @param insertion True if a newline was inserted, false if a newline was removed.
* @param lastLineContainsLineBreaks True if the last line contains line breaks. This is useful
* when the last line is a template literal that spans multiple lines.
*/
void reportLineCut(int lineIndex, int charIndex, boolean insertion) {
void reportLineCut(
int lineIndex, int charIndex, boolean insertion, boolean lastLineContainsLineBreaks) {
if (createSrcMap) {
for (Mapping mapping : allMappings) {
mapping.start = convertPosition(mapping.start, lineIndex, charIndex,
insertion);
mapping.start =
convertPosition(
mapping.start, lineIndex, charIndex, insertion, lastLineContainsLineBreaks);

if (mapping.end != null) {
mapping.end = convertPosition(mapping.end, lineIndex, charIndex,
insertion);
mapping.end =
convertPosition(
mapping.end, lineIndex, charIndex, insertion, lastLineContainsLineBreaks);
}
}
}
}

/**
* Converts the given position by normalizing it against the insertion
* or removal of a newline at the given line and character position.
*
* @param position The existing position before the newline was inserted.
* @param lineIndex The index of the line at which the newline was inserted.
* @param characterPosition The position on the line at which the newline
* was inserted.
* @param insertion True if a newline was inserted, false if a newline was
* removed.
* Converts the given position by normalizing it against the insertion or removal of a newline
* at the given line and character position.
*
* @param position The existing position before the newline was inserted/removed.
* @param lineIndex The index of the line at which the newline was inserted/removed.
* @param characterPosition The position on the line at which the newline was inserted.
* @param insertion True if a newline was inserted, false if a newline was removed.
* @param lastLineContainsLineBreaks True if the last line contains line breaks. This is useful
* when the last line is a template literal that spans multiple lines.
* @return The normalized position.
* @throws IllegalStateException if an attempt to reverse a line cut is
* made on a previous line rather than the current line.
* @throws IllegalStateException if an attempt to reverse a line cut is made on a previous line
* rather than the current line.
*/
private static FilePosition convertPosition(FilePosition position, int lineIndex,
int characterPosition, boolean insertion) {
private static FilePosition convertPosition(
FilePosition position,
int lineIndex,
int characterPosition,
boolean insertion,
boolean lastLineContainsLineBreaks) {
int originalLine = position.getLine();
int originalChar = position.getColumn();
if (insertion) {
Expand All @@ -198,11 +209,14 @@ private static FilePosition convertPosition(FilePosition position, int lineIndex
return new FilePosition(
originalLine - 1, originalChar + characterPosition);
} else if (originalLine > lineIndex) {
if (lastLineContainsLineBreaks) {
return new FilePosition(originalLine - 1, originalChar);
} else {
// Not supported, can only undo a cut on the most recent line. To
// do this on a previous lines would require reevaluating the cut
// positions on all subsequent lines.
throw new IllegalStateException(
"Cannot undo line cut on a previous line.");
throw new IllegalStateException("Cannot undo line cut on a previous line.");
}
} else {
return position;
}
Expand Down Expand Up @@ -648,7 +662,7 @@ void maybeCutLine() {
int position = preferredBreakPosition;
code.insert(position, '\n');
prevCutPosition = position;
reportLineCut(lineIndex, position - lineStartPosition, true);
reportLineCut(lineIndex, position - lineStartPosition, true, false);
lineIndex++;
lineLength -= (position - lineStartPosition);
prevLineStartPosition = lineStartPosition;
Expand All @@ -675,15 +689,25 @@ void endFile() {
append(";");
startNewLine();
} else if (prevCutPosition > 0) {

// Shift the previous break to end of file by replacing it with a
// <space> and adding a new break at end of file. Adding the space
// handles cases like instanceof\nfoo. (it would be nice to avoid this)
code.setCharAt(prevCutPosition, ' ');
lineStartPosition = prevLineStartPosition;
lineLength = code.length() - lineStartPosition;
int cutLineIndex = lineIndex;
// We need +1 to account for the space added few lines above.
int prevLineEndPosition = prevCutPosition - prevLineStartPosition + 1;
reportLineCut(lineIndex, prevLineEndPosition, false);
if (code.indexOf("\n", prevCutPosition) != -1) {
// having "\n" in the code after prevCutPosition means that the original code had
// irremovable \n such as template literals with line breaks.
lineLength = code.length() - code.lastIndexOf("\n");
cutLineIndex = lineIndex - CharMatcher.is('\n').countIn(code.substring(prevCutPosition));
reportLineCut(cutLineIndex, prevLineEndPosition, false, true);
} else {
lineLength = code.length() - lineStartPosition;
reportLineCut(cutLineIndex, prevLineEndPosition, false, false);
}
lineIndex--;
prevCutPosition = 0;
prevLineStartPosition = 0;
Expand Down
23 changes: 23 additions & 0 deletions test/com/google/javascript/jscomp/CodePrinterTest.java
Expand Up @@ -23,6 +23,7 @@
import com.google.common.base.Joiner;
import com.google.common.collect.ImmutableList;
import com.google.javascript.jscomp.CompilerOptions.LanguageMode;
import com.google.javascript.jscomp.SourceMap.Format;
import com.google.javascript.rhino.IR;
import com.google.javascript.rhino.Node;
import com.google.javascript.rhino.Token;
Expand Down Expand Up @@ -3324,4 +3325,26 @@ private void checkWithOriginalName(
.build())
.isEqualTo(expectedCode);
}

@Test
public void testMultiLineTemplateLiteralInLastLine() {
// related to b/117613188
languageMode = LanguageMode.ECMASCRIPT_2015;
String code = "var x = 'this is a pretty long line'; var y = `hello\nworld\nfoo\nbar`;";
String expectedCode = "var x=\"this is a pretty long line\"; var y=`hello\nworld\nfoo\nbar`;\n";

CompilerOptions codePrinterOptions = new CompilerOptions();
// This needs to be smaller than the length of the first statement to force a line break
codePrinterOptions.setLineLengthThreshold(30);
// This needs to be true since `endFile()` of CompactCodePrinter would just return if false
codePrinterOptions.setPreferLineBreakAtEndOfFile(true);

assertThat(
new CodePrinter.Builder(parse(code))
.setCompilerOptions(codePrinterOptions)
.setPrettyPrint(false)
.setSourceMap(Format.DEFAULT.getInstance())
.build())
.isEqualTo(expectedCode);
}
}

0 comments on commit 96841ee

Please sign in to comment.