Skip to content
This repository has been archived by the owner on Feb 23, 2022. It is now read-only.

Commit

Permalink
Fix internal error in CssCompiler caused by source-mapping of empty i…
Browse files Browse the repository at this point in the history
…mport block at start of file.

Trying to fix this by making the DefaultVisitController skip empty
import block nodes breaks another test, so a CL that fixes it by
tweaking the DefaultGssSourceMapGenerator is forthcoming.

It looks like the DefaultGssSourceMapGenerator can already generate
zero-length mappings, as when an empty import block is written after a
copyright header comment so clients should not be confused.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=126835623
  • Loading branch information
iflan committed Jul 12, 2016
1 parent 82c8a70 commit d674936
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,10 @@ public void startSourceMapping(CssNode node, int startLine, int startCharIndex)
public void endSourceMapping(CssNode node, int endLine, int endCharIndex) {
Preconditions.checkState(node != null);
Preconditions.checkState(endLine >= 0);
Preconditions.checkState(endCharIndex >= 0);
// -1 when a node contributes no content at the start of the buffer,
// as when a CssImportBlockNode is encountered, and there is no
// copyright comment.
Preconditions.checkState(endCharIndex >= -1);
if (!mappings.isEmpty() && mappings.peek().node == node) {
Mapping mapping = mappings.pop();
mapping.end = new FilePosition(endLine, endCharIndex);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ public interface GssSourceMapGenerator {
* @param node the {@link CssNode} to be processed
* @param endLine the last character's line number when it ends writing output
* @param endCharIndex the last character's character index when it ends writing output
* or one less than the corresponding {@link #startSourceMapping startCharIndex} if
* a source mapping is empty.
*/
public void endSourceMapping(CssNode node, int endLine, int endCharIndex);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,12 @@
import com.google.common.css.SourceCode;
import com.google.common.css.compiler.ast.ErrorManager;
import com.google.common.css.compiler.ast.testing.NewFunctionalTestBase;
import com.google.common.io.Files;

import junit.framework.TestCase;

import java.io.File;

public class ClosureCommandLineCompilerTest extends TestCase {

static final ExitCodeHandler EXIT_CODE_HANDLER =
Expand Down Expand Up @@ -64,4 +67,30 @@ public void testAllowDefPropagationDefaultsToTrue() throws Exception {
JobDescription jobDescription = flags.createJobDescription();
assertTrue(jobDescription.allowDefPropagation);
}


public void testEmptyImportBlocks() throws Exception {
// See b/29995881
ErrorManager errorManager = new NewFunctionalTestBase.TestErrorManager(new String[0]);

JobDescription job = new JobDescriptionBuilder()
.addInput(new SourceCode("main.css", "@import 'common.css';"))
.addInput(new SourceCode("common.css", "/* common */"))
.setOptimizeStrategy(JobDescription.OptimizeStrategy.SAFE)
.setCreateSourceMap(true)
.getJobDescription();

File outputDir = Files.createTempDir();
File sourceMapFile = new File(outputDir, "sourceMap");

String compiledCss = new ClosureCommandLineCompiler(
job, EXIT_CODE_HANDLER, errorManager)
.execute(null /*renameFile*/, sourceMapFile);

// The symptom was an IllegalStateException trapped by the compiler that
// resulted in the exit handler being called which causes fail(...),
// so if control reaches here, we're ok.

assertNotNull(compiledCss);
}
}

0 comments on commit d674936

Please sign in to comment.