From 96841eed77c32db1f7b98ff0eb2794c2efbddc0b Mon Sep 17 00:00:00 2001 From: yitingwang Date: Fri, 9 Nov 2018 14:12:01 -0800 Subject: [PATCH] Fix compiler crash during a special case with multi-line template literals. 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 --- .../google/javascript/jscomp/CodePrinter.java | 80 ++++++++++++------- .../javascript/jscomp/CodePrinterTest.java | 23 ++++++ 2 files changed, 75 insertions(+), 28 deletions(-) diff --git a/src/com/google/javascript/jscomp/CodePrinter.java b/src/com/google/javascript/jscomp/CodePrinter.java index bbabe3af052..4b7d3f0e551 100644 --- a/src/com/google/javascript/jscomp/CodePrinter.java +++ b/src/com/google/javascript/jscomp/CodePrinter.java @@ -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) { @@ -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; } @@ -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; @@ -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 // 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; diff --git a/test/com/google/javascript/jscomp/CodePrinterTest.java b/test/com/google/javascript/jscomp/CodePrinterTest.java index 1ad5906069e..5228880d0a5 100644 --- a/test/com/google/javascript/jscomp/CodePrinterTest.java +++ b/test/com/google/javascript/jscomp/CodePrinterTest.java @@ -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; @@ -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); + } }