From 95f6fed0d8479f7e6d565e21bd3f179ebc8efdb2 Mon Sep 17 00:00:00 2001 From: tjgq Date: Wed, 22 Aug 2018 11:06:35 -0700 Subject: [PATCH] Modify SourceFile to store whether a file is strong, weak or externs. This is necessary to support eliding weak sources passed under the --weakdep flag from the output. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=209794908 --- .../google/javascript/jscomp/Compiler.java | 4 +- .../javascript/jscomp/CompilerInput.java | 21 +++++++--- .../google/javascript/jscomp/SourceFile.java | 41 +++++++++++-------- .../javascript/jscomp/SyntheticAst.java | 5 ++- src/com/google/javascript/rhino/Node.java | 3 +- .../javascript/rhino/SimpleSourceFile.java | 10 ++--- .../javascript/rhino/StaticSourceFile.java | 32 +++++++++++++-- .../javascript/jscomp/AstValidatorTest.java | 7 ++-- .../javascript/jscomp/CompilerTestCase.java | 3 +- .../jscomp/parsing/AttachJsdocsTest.java | 10 ++--- .../jscomp/parsing/JsDocInfoParserTest.java | 8 ++-- .../javascript/jscomp/parsing/ParserTest.java | 14 ++++--- 12 files changed, 102 insertions(+), 56 deletions(-) diff --git a/src/com/google/javascript/jscomp/Compiler.java b/src/com/google/javascript/jscomp/Compiler.java index ead585e1b62..d4212cf82c2 100644 --- a/src/com/google/javascript/jscomp/Compiler.java +++ b/src/com/google/javascript/jscomp/Compiler.java @@ -1787,7 +1787,7 @@ Node parseInputs() { } if (NodeUtil.isFromTypeSummary(n)) { - input.setIsExtern(true); + input.setIsExtern(); externsRoot.addChildToBack(n); } else { jsRoot.addChildToBack(n); @@ -1991,7 +1991,7 @@ private boolean hoistIfExtern(CompilerInput input) { // If the input file is explicitly marked as an externs file, then move it out of the main // JS root and put it with the other externs. externsRoot.addChildToBack(n); - input.setIsExtern(true); + input.setIsExtern(); input.getModule().remove(input); diff --git a/src/com/google/javascript/jscomp/CompilerInput.java b/src/com/google/javascript/jscomp/CompilerInput.java index 087f6b8029c..518a2876c32 100644 --- a/src/com/google/javascript/jscomp/CompilerInput.java +++ b/src/com/google/javascript/jscomp/CompilerInput.java @@ -33,6 +33,7 @@ import com.google.javascript.jscomp.parsing.parser.FeatureSet; import com.google.javascript.rhino.InputId; import com.google.javascript.rhino.Node; +import com.google.javascript.rhino.StaticSourceFile.SourceKind; import java.io.IOException; import java.util.ArrayList; import java.util.List; @@ -70,6 +71,15 @@ public class CompilerInput extends DependencyInfo.Base implements SourceAst { private transient AbstractCompiler compiler; private transient ModulePath modulePath; + // TODO(tjgq): Whether a CompilerInput is an externs file is determined by the `isExtern` + // constructor argument and the `setIsExtern` method. Both are necessary because, while externs + // files passed under the --externs flag are not required to contain an @externs annotation, + // externs files passed under any other flag must have one, and the presence of the latter can + // only be known once the file has been parsed. To add to the confusion, note that CompilerInput + // doesn't actually store the extern bit itself, but instead mutates the SourceFile associated + // with the AST node. Once (when?) we enforce that extern files always contain an @externs + // annotation, we can store the extern bit in the AST node, and make SourceFile immutable. + public CompilerInput(SourceAst ast) { this(ast, ast.getSourceFile().getName(), false); } @@ -86,10 +96,8 @@ public CompilerInput(SourceAst ast, InputId inputId, boolean isExtern) { this.ast = ast; this.id = inputId; - // TODO(nicksantos): Add a precondition check here. People are passing - // in null, but they should not be. - if (ast != null && ast.getSourceFile() != null) { - ast.getSourceFile().setIsExtern(isExtern); + if (isExtern) { + setIsExtern(); } } @@ -498,11 +506,12 @@ public boolean isExtern() { return ast.getSourceFile().isExtern(); } - void setIsExtern(boolean isExtern) { + void setIsExtern() { + // TODO(tjgq): Add a precondition check here. People are passing in null, but they shouldn't be. if (ast == null || ast.getSourceFile() == null) { return; } - ast.getSourceFile().setIsExtern(isExtern); + ast.getSourceFile().setKind(SourceKind.EXTERN); } public int getLineOffset(int lineno) { diff --git a/src/com/google/javascript/jscomp/SourceFile.java b/src/com/google/javascript/jscomp/SourceFile.java index 5bf049f3f55..2e50436d697 100644 --- a/src/com/google/javascript/jscomp/SourceFile.java +++ b/src/com/google/javascript/jscomp/SourceFile.java @@ -25,6 +25,7 @@ import com.google.common.io.CharStreams; import com.google.common.io.Resources; import com.google.javascript.rhino.StaticSourceFile; +import com.google.javascript.rhino.StaticSourceFile.SourceKind; import java.io.File; import java.io.IOException; import java.io.InputStream; @@ -57,6 +58,7 @@ * @author nicksantos@google.com (Nick Santos) */ public class SourceFile implements StaticSourceFile, Serializable { + private static final long serialVersionUID = 1L; private static final String UTF8_BOM = "\uFEFF"; @@ -74,7 +76,7 @@ public interface Generator { private static final int SOURCE_EXCERPT_REGION_LENGTH = 5; private final String fileName; - private boolean isExternFile = false; + private SourceKind kind; // The fileName may not always identify the original file - for example, // supersourced Java inputs, or Java inputs that come from Jar files. This @@ -94,8 +96,9 @@ public interface Generator { * @param fileName The file name of the source file. It does not necessarily need to correspond to * a real path. But it should be unique. Will appear in warning messages emitted by the * compiler. + * @param kind The source kind. */ - public SourceFile(String fileName) { + public SourceFile(String fileName, SourceKind kind) { if (isNullOrEmpty(fileName)) { throw new IllegalArgumentException("a source must have a name"); } @@ -145,9 +148,6 @@ private void resetLineOffsets() { lineOffsets = null; } - ////////////////////////////////////////////////////////////////////////////// - // Implementation - /** * Gets all the code in this source file. * @throws IOException @@ -156,7 +156,6 @@ public String getCode() throws IOException { return code; } - /** * Gets a reader for the code in this source file. */ @@ -210,15 +209,21 @@ public String getName() { return fileName; } - /** Returns whether this is an extern. */ + /** Returns the source kind. */ @Override - public boolean isExtern() { - return isExternFile; + public SourceKind getKind() { + return kind; } - /** Sets that this is an extern. */ - void setIsExtern(boolean newVal) { - isExternFile = newVal; + /** + * Sets the source kind. + * + *

TODO(tjgq): Move the extern bit into the AST, so we can make the kind immutable. This is + * currently not possible because for some files the extern bit is not determined by the contents + * (e.g. files passed under the --externs flag and missing an externs annotation). + */ + void setKind(SourceKind kind) { + this.kind = kind; } @Override @@ -537,7 +542,7 @@ static class Preloaded extends SourceFile { private static final long serialVersionUID = 1L; Preloaded(String fileName, String originalPath, String code) { - super(fileName); + super(fileName, SourceKind.STRONG); super.setOriginalPath(originalPath); super.setCode(code); } @@ -556,7 +561,7 @@ static class Generated extends SourceFile { // Not private, so that LazyInput can extend it. Generated(String fileName, String originalPath, Generator generator) { - super(fileName); + super(fileName, SourceKind.STRONG); super.setOriginalPath(originalPath); this.generator = generator; } @@ -597,7 +602,7 @@ static class OnDisk extends SourceFile { private transient Charset inputCharset = UTF_8; OnDisk(Path path, String originalPath, Charset c) { - super(path.toString()); + super(path.toString(), SourceKind.STRONG); this.path = path; setOriginalPath(originalPath); if (c != null) { @@ -670,7 +675,7 @@ private void writeObject(java.io.ObjectOutputStream out) throws Exception { out.writeObject(inputCharset != null ? inputCharset.name() : null); out.writeObject(path != null ? path.toUri() : null); } - + @GwtIncompatible("ObjectInputStream") private void readObject(java.io.ObjectInputStream in) throws Exception { in.defaultReadObject(); @@ -702,9 +707,9 @@ static class AtUrl extends SourceFile { private String inputCharset = UTF_8.name(); AtUrl(URL url, String originalPath, Charset c) { - super(originalPath); - this.url = url; + super(originalPath, SourceKind.STRONG); super.setOriginalPath(originalPath); + this.url = url; if (c != null) { this.setCharset(c); } diff --git a/src/com/google/javascript/jscomp/SyntheticAst.java b/src/com/google/javascript/jscomp/SyntheticAst.java index 5ed2f79edc7..c127e9c8c33 100644 --- a/src/com/google/javascript/jscomp/SyntheticAst.java +++ b/src/com/google/javascript/jscomp/SyntheticAst.java @@ -21,6 +21,7 @@ import com.google.javascript.rhino.IR; import com.google.javascript.rhino.InputId; import com.google.javascript.rhino.Node; +import com.google.javascript.rhino.StaticSourceFile.SourceKind; /** * An AST generated totally by the compiler. @@ -37,13 +38,13 @@ public final class SyntheticAst implements SourceAst { SyntheticAst(String sourceName) { this.inputId = new InputId(sourceName); - this.sourceFile = new SourceFile(sourceName); + this.sourceFile = new SourceFile(sourceName, SourceKind.STRONG); clearAst(); } public SyntheticAst(Node root) { this.inputId = new InputId(root.getSourceFileName()); - this.sourceFile = new SourceFile(root.getSourceFileName()); + this.sourceFile = new SourceFile(root.getSourceFileName(), SourceKind.STRONG); this.root = checkNotNull(root); } diff --git a/src/com/google/javascript/rhino/Node.java b/src/com/google/javascript/rhino/Node.java index aa24847971d..6f7aa114d47 100644 --- a/src/com/google/javascript/rhino/Node.java +++ b/src/com/google/javascript/rhino/Node.java @@ -47,6 +47,7 @@ import com.google.common.annotations.GwtIncompatible; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Objects; +import com.google.javascript.rhino.StaticSourceFile.SourceKind; import com.google.javascript.rhino.jstype.JSType; import java.io.IOException; import java.io.ObjectInput; @@ -1334,7 +1335,7 @@ public final void setStaticSourceFile(@Nullable StaticSourceFile file) { /** Sets the source file to a non-extern file of the given name. */ public final void setSourceFileForTesting(String name) { - this.putProp(STATIC_SOURCE_FILE, new SimpleSourceFile(name, false)); + this.putProp(STATIC_SOURCE_FILE, new SimpleSourceFile(name, SourceKind.STRONG)); } // TODO(johnlenz): make this final diff --git a/src/com/google/javascript/rhino/SimpleSourceFile.java b/src/com/google/javascript/rhino/SimpleSourceFile.java index 8811be96ef7..5171cdb27ee 100644 --- a/src/com/google/javascript/rhino/SimpleSourceFile.java +++ b/src/com/google/javascript/rhino/SimpleSourceFile.java @@ -45,11 +45,11 @@ */ public final class SimpleSourceFile implements StaticSourceFile { private final String name; - private final boolean extern; + private final SourceKind kind; - public SimpleSourceFile(String name, boolean extern) { + public SimpleSourceFile(String name, SourceKind kind) { this.name = name; - this.extern = extern; + this.kind = kind; } @Override @@ -58,8 +58,8 @@ public String getName() { } @Override - public boolean isExtern() { - return extern; + public SourceKind getKind() { + return kind; } @Override diff --git a/src/com/google/javascript/rhino/StaticSourceFile.java b/src/com/google/javascript/rhino/StaticSourceFile.java index 6c5a78e144e..34f2f2c00c8 100644 --- a/src/com/google/javascript/rhino/StaticSourceFile.java +++ b/src/com/google/javascript/rhino/StaticSourceFile.java @@ -44,15 +44,39 @@ * @author nicksantos@google.com (Nick Santos) */ public interface StaticSourceFile { + + /** Source kinds. */ + public enum SourceKind { + /** A file whose contents are necessary both for type checking and emitting code. */ + STRONG, + /** A file whose contents are necessary for type checking only. */ + WEAK, + /** A file whose contents are extern declarations. */ + EXTERN + } + /** * The name of the file. Must be unique across all files in the compilation. */ String getName(); - /** - * Returns whether this is an externs file. - */ - boolean isExtern(); + /** The source kind. */ + SourceKind getKind(); + + /** Whether the source kind is STRONG. */ + default boolean isStrong() { + return getKind() == SourceKind.STRONG; + } + + /** Whether the source kind is WEAK. */ + default boolean isWeak() { + return getKind() == SourceKind.WEAK; + } + + /** Whether the source kind is EXTERN. */ + default boolean isExtern() { + return getKind() == SourceKind.EXTERN; + } /** * Returns the offset of the given line number relative to the file start. diff --git a/test/com/google/javascript/jscomp/AstValidatorTest.java b/test/com/google/javascript/jscomp/AstValidatorTest.java index 7a1807f2184..1d425e147aa 100644 --- a/test/com/google/javascript/jscomp/AstValidatorTest.java +++ b/test/com/google/javascript/jscomp/AstValidatorTest.java @@ -28,6 +28,7 @@ import com.google.javascript.rhino.JSTypeExpression; import com.google.javascript.rhino.Node; import com.google.javascript.rhino.SimpleSourceFile; +import com.google.javascript.rhino.StaticSourceFile.SourceKind; import com.google.javascript.rhino.Token; /** @@ -139,7 +140,7 @@ public void testValidScript() { Node n = new Node(Token.SCRIPT); expectInvalid(n, Check.SCRIPT); n.setInputId(new InputId("something_input")); - n.setStaticSourceFile(new SimpleSourceFile("something", false)); + n.setStaticSourceFile(new SimpleSourceFile("something", SourceKind.STRONG)); expectValid(n, Check.SCRIPT); expectInvalid(n, Check.STATEMENT); expectInvalid(n, Check.EXPRESSION); @@ -534,7 +535,7 @@ public void testValidFeatureInScript() { Node n = new Node(Token.SCRIPT); n.setInputId(new InputId("something_input")); - n.setStaticSourceFile(new SimpleSourceFile("something", false)); + n.setStaticSourceFile(new SimpleSourceFile("something", SourceKind.STRONG)); expectValid(n, Check.SCRIPT); n.addChildToFront(IR.let(IR.name("a"), IR.number(3))); @@ -604,7 +605,7 @@ private Node parseScriptWithoutCheckingLanguageLevel(String code) { Node script = n.getFirstChild(); assertNode(script).hasType(Token.SCRIPT); script.setInputId(new InputId("something_input")); - script.setStaticSourceFile(new SimpleSourceFile("something", false)); + script.setStaticSourceFile(new SimpleSourceFile("something", SourceKind.STRONG)); return script; } } diff --git a/test/com/google/javascript/jscomp/CompilerTestCase.java b/test/com/google/javascript/jscomp/CompilerTestCase.java index 04ca752bde4..8acb9568476 100644 --- a/test/com/google/javascript/jscomp/CompilerTestCase.java +++ b/test/com/google/javascript/jscomp/CompilerTestCase.java @@ -35,6 +35,7 @@ import com.google.javascript.jscomp.type.ReverseAbstractInterpreter; import com.google.javascript.jscomp.type.SemanticReverseAbstractInterpreter; import com.google.javascript.rhino.Node; +import com.google.javascript.rhino.StaticSourceFile.SourceKind; import com.google.javascript.rhino.testing.BaseJSTypeTestCase; import java.io.IOException; import java.io.InputStream; @@ -2316,7 +2317,7 @@ protected static final class Externs implements TestPart { externs = files; if (files != null) { for (SourceFile s : files) { - s.setIsExtern(true); + s.setKind(SourceKind.EXTERN); } } } diff --git a/test/com/google/javascript/jscomp/parsing/AttachJsdocsTest.java b/test/com/google/javascript/jscomp/parsing/AttachJsdocsTest.java index d92cfcd4561..422bc602492 100644 --- a/test/com/google/javascript/jscomp/parsing/AttachJsdocsTest.java +++ b/test/com/google/javascript/jscomp/parsing/AttachJsdocsTest.java @@ -23,6 +23,7 @@ import com.google.javascript.rhino.JSDocInfo; import com.google.javascript.rhino.Node; import com.google.javascript.rhino.SimpleSourceFile; +import com.google.javascript.rhino.StaticSourceFile.SourceKind; import com.google.javascript.rhino.Token; import com.google.javascript.rhino.testing.BaseJSTypeTestCase; import com.google.javascript.rhino.testing.TestErrorReporter; @@ -795,11 +796,10 @@ private Node parse(String source, String... warnings) { null, true, StrictMode.SLOPPY); - Node script = ParserRunner.parse( - new SimpleSourceFile("input", false), - source, - config, - testErrorReporter).ast; + Node script = + ParserRunner.parse( + new SimpleSourceFile("input", SourceKind.STRONG), source, config, testErrorReporter) + .ast; // verifying that all warnings were seen testErrorReporter.assertHasEncounteredAllErrors(); diff --git a/test/com/google/javascript/jscomp/parsing/JsDocInfoParserTest.java b/test/com/google/javascript/jscomp/parsing/JsDocInfoParserTest.java index a8e6046a9f0..33112fbbf78 100644 --- a/test/com/google/javascript/jscomp/parsing/JsDocInfoParserTest.java +++ b/test/com/google/javascript/jscomp/parsing/JsDocInfoParserTest.java @@ -38,6 +38,7 @@ import com.google.javascript.rhino.Node; import com.google.javascript.rhino.SimpleSourceFile; import com.google.javascript.rhino.StaticSourceFile; +import com.google.javascript.rhino.StaticSourceFile.SourceKind; import com.google.javascript.rhino.Token; import com.google.javascript.rhino.jstype.JSType; import com.google.javascript.rhino.jstype.ObjectType; @@ -4781,8 +4782,9 @@ private Node parseFull(String code, String... warnings) { .setStrictMode(StrictMode.SLOPPY) .build(); - ParseResult result = ParserRunner.parse( - new SimpleSourceFile("source", false), code, config, testErrorReporter); + ParseResult result = + ParserRunner.parse( + new SimpleSourceFile("source", SourceKind.STRONG), code, config, testErrorReporter); testErrorReporter.assertHasEncounteredAllWarnings(); return result.ast; @@ -4833,7 +4835,7 @@ private JSDocInfo parse(String comment, JsDocParsing parseDocumentation, .setStrictMode(Config.StrictMode.SLOPPY) .build(); - StaticSourceFile file = new SimpleSourceFile("testcode", false); + StaticSourceFile file = new SimpleSourceFile("testcode", SourceKind.STRONG); Node templateNode = IR.script(); templateNode.setStaticSourceFile(file); diff --git a/test/com/google/javascript/jscomp/parsing/ParserTest.java b/test/com/google/javascript/jscomp/parsing/ParserTest.java index 354d3c523dc..e8e8c671876 100644 --- a/test/com/google/javascript/jscomp/parsing/ParserTest.java +++ b/test/com/google/javascript/jscomp/parsing/ParserTest.java @@ -36,6 +36,7 @@ import com.google.javascript.rhino.Node; import com.google.javascript.rhino.SimpleSourceFile; import com.google.javascript.rhino.StaticSourceFile; +import com.google.javascript.rhino.StaticSourceFile.SourceKind; import com.google.javascript.rhino.Token; import com.google.javascript.rhino.jstype.JSType; import com.google.javascript.rhino.testing.BaseJSTypeTestCase; @@ -4639,11 +4640,12 @@ private static Node regex(String regex) { */ private Node parseError(String source, String... errors) { TestErrorReporter testErrorReporter = new TestErrorReporter(errors, null); - ParseResult result = ParserRunner.parse( - new SimpleSourceFile("input", false), - source, - createConfig(), - testErrorReporter); + ParseResult result = + ParserRunner.parse( + new SimpleSourceFile("input", SourceKind.STRONG), + source, + createConfig(), + testErrorReporter); Node script = result.ast; // check expected features if specified @@ -4666,7 +4668,7 @@ private Node parseWarning(String string, String... warnings) { private ParserRunner.ParseResult doParse(String string, String... warnings) { TestErrorReporter testErrorReporter = new TestErrorReporter(null, warnings); - StaticSourceFile file = new SimpleSourceFile("input", false); + StaticSourceFile file = new SimpleSourceFile("input", SourceKind.STRONG); ParserRunner.ParseResult result = ParserRunner.parse( file, string,