From 91f3ccad3fda06cba355d9cfdd669610e29f8a0a Mon Sep 17 00:00:00 2001 From: blickly Date: Wed, 17 Jan 2018 15:33:21 -0800 Subject: [PATCH] Group overloads together ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=182281037 --- .../jscomp/AbstractCommandLineRunner.java | 112 ++++++------ .../javascript/jscomp/CodeGenerator.java | 31 ++-- .../javascript/jscomp/CodingConventions.java | 8 +- .../javascript/jscomp/CompilerOptions.java | 22 +-- .../javascript/jscomp/ConformanceRules.java | 19 +- .../javascript/jscomp/DiagnosticGroup.java | 35 ++-- .../javascript/jscomp/DotFormatter.java | 124 ++++++------- .../jscomp/ExpressionDecomposer.java | 132 +++++++------- .../javascript/jscomp/GlobalNamespace.java | 44 ++--- src/com/google/javascript/jscomp/Linter.java | 62 ++++--- .../google/javascript/jscomp/NTIScope.java | 31 ++-- .../javascript/jscomp/NewTypeInference.java | 10 +- .../google/javascript/jscomp/NodeUtil.java | 57 +++--- .../google/javascript/jscomp/Reference.java | 14 +- .../jscomp/SideEffectsAnalysis.java | 21 +-- .../google/javascript/jscomp/SourceFile.java | 10 +- .../javascript/jscomp/deps/DepsGenerator.java | 9 +- .../javascript/jscomp/graph/DiGraph.java | 5 +- .../google/javascript/jscomp/graph/Graph.java | 18 +- .../jscomp/graph/LinkedDirectedGraph.java | 34 ++-- .../jscomp/graph/LinkedUndirectedGraph.java | 28 +-- .../javascript/jscomp/newtypes/JSType.java | 10 +- .../newtypes/JSTypeCreatorFromJSDoc.java | 17 +- .../javascript/jscomp/newtypes/JSTypes.java | 35 ++-- .../javascript/jscomp/parsing/IRFactory.java | 24 +-- .../jscomp/parsing/JsDocInfoParser.java | 106 ++++++----- .../parsing/parser/util/ErrorReporter.java | 4 +- .../javascript/jscomp/regex/RegExpTree.java | 21 ++- .../jscomp/transpile/TranspilerBuilder.java | 11 +- .../javascript/refactoring/Matchers.java | 44 ++--- .../refactoring/RefactoringDriver.java | 18 +- .../javascript/refactoring/SuggestedFix.java | 34 ++-- src/com/google/javascript/rhino/IR.java | 17 +- .../google/javascript/rhino/JSDocInfo.java | 53 +++--- .../javascript/rhino/JSDocInfoBuilder.java | 45 +++-- src/com/google/javascript/rhino/Node.java | 18 +- .../javascript/rhino/jstype/JSType.java | 45 ++--- .../rhino/jstype/JSTypeRegistry.java | 127 +++++++------- .../rhino/jstype/UnionTypeBuilder.java | 13 +- .../rhino/testing/BaseJSTypeTestCase.java | 16 +- .../sourcemap/SourceMapTestCase.java | 44 +++-- .../jscomp/CheckMissingReturnTest.java | 10 +- .../jscomp/ControlFlowAnalysisTest.java | 51 +++--- .../javascript/jscomp/GoldenFileComparer.java | 36 ++-- .../javascript/jscomp/IntegrationTest.java | 23 +-- .../jscomp/IntegrationTestCase.java | 40 +++-- .../jscomp/NewTypeInferenceTestBase.java | 57 +++--- .../javascript/jscomp/NodeUtilTest.java | 56 +++--- .../javascript/jscomp/TypeCheckTest.java | 166 +++++++++--------- .../javascript/jscomp/WarningsGuardTest.java | 21 ++- .../javascript/jscomp/parsing/ParserTest.java | 2 +- 51 files changed, 975 insertions(+), 1015 deletions(-) diff --git a/src/com/google/javascript/jscomp/AbstractCommandLineRunner.java b/src/com/google/javascript/jscomp/AbstractCommandLineRunner.java index 8f1a59e6f68..a293397aec8 100644 --- a/src/com/google/javascript/jscomp/AbstractCommandLineRunner.java +++ b/src/com/google/javascript/jscomp/AbstractCommandLineRunner.java @@ -590,63 +590,15 @@ protected List createInputs( return createInputs(files, jsonFiles, false, jsModuleSpecs); } - /** - * Check that relative paths inside zip files are unique, since multiple files with the same path - * inside different zips are considered duplicate inputs. Parameter {@code sourceFiles} may be - * modified if duplicates are removed. - */ - public static ImmutableList removeDuplicateZipEntries( - List sourceFiles, List jsModuleSpecs) throws IOException { - ImmutableList.Builder errors = ImmutableList.builder(); - Map sourceFilesByName = new HashMap<>(); - Iterator fileIterator = sourceFiles.iterator(); - int currentFileIndex = 0; - Iterator moduleIterator = jsModuleSpecs.iterator(); - // Tracks the total number of js files for current module and all the previous modules. - int cumulatedJsFileNum = 0; - JsModuleSpec currentModule = null; - while (fileIterator.hasNext()) { - SourceFile sourceFile = fileIterator.next(); - currentFileIndex++; - // Check whether we reached the next module. - if (moduleIterator.hasNext() && currentFileIndex > cumulatedJsFileNum) { - currentModule = moduleIterator.next(); - cumulatedJsFileNum += currentModule.numJsFiles; - } - String fullPath = sourceFile.getName(); - if (!fullPath.contains("!/")) { - // Not a zip file - continue; - } - String relativePath = fullPath.split("!")[1]; - if (!sourceFilesByName.containsKey(relativePath)) { - sourceFilesByName.put(relativePath, sourceFile); - } else { - SourceFile firstSourceFile = sourceFilesByName.get(relativePath); - if (firstSourceFile.getCode().equals(sourceFile.getCode())) { - fileIterator.remove(); - if (currentModule != null) { - currentModule.numJsFiles--; - } - } else { - errors.add(JSError.make( - CONFLICTING_DUPLICATE_ZIP_CONTENTS, firstSourceFile.getName(), sourceFile.getName())); - } - } - } - return errors.build(); - } - /** * Creates inputs from a list of source files, zips and json files. * - * Can be overridden by subclasses who want to pull files from different - * places. + *

Can be overridden by subclasses who want to pull files from different places. * * @param files A list of flag entries indicates js and zip file names * @param jsonFiles A list of json encoded files. - * @param allowStdIn Whether '-' is allowed appear as a filename to represent - * stdin. If true, '-' is only allowed to appear once. + * @param allowStdIn Whether '-' is allowed appear as a filename to represent stdin. If true, '-' + * is only allowed to appear once. * @param jsModuleSpecs A list js module specs. * @return An array of inputs */ @@ -696,12 +648,12 @@ protected List createInputs( } if (!config.outputManifests.isEmpty()) { - throw new FlagUsageException("Manifest files cannot be generated " + - "when the input is from stdin."); + throw new FlagUsageException( + "Manifest files cannot be generated when the input is from stdin."); } if (!config.outputBundles.isEmpty()) { - throw new FlagUsageException("Bundle files cannot be generated " + - "when the input is from stdin."); + throw new FlagUsageException( + "Bundle files cannot be generated when the input is from stdin."); } this.err.println(WAITING_FOR_INPUT_WARNING); @@ -727,6 +679,56 @@ protected List createInputs( return inputs; } + /** + * Check that relative paths inside zip files are unique, since multiple files with the same path + * inside different zips are considered duplicate inputs. Parameter {@code sourceFiles} may be + * modified if duplicates are removed. + */ + public static ImmutableList removeDuplicateZipEntries( + List sourceFiles, List jsModuleSpecs) throws IOException { + ImmutableList.Builder errors = ImmutableList.builder(); + Map sourceFilesByName = new HashMap<>(); + Iterator fileIterator = sourceFiles.iterator(); + int currentFileIndex = 0; + Iterator moduleIterator = jsModuleSpecs.iterator(); + // Tracks the total number of js files for current module and all the previous modules. + int cumulatedJsFileNum = 0; + JsModuleSpec currentModule = null; + while (fileIterator.hasNext()) { + SourceFile sourceFile = fileIterator.next(); + currentFileIndex++; + // Check whether we reached the next module. + if (moduleIterator.hasNext() && currentFileIndex > cumulatedJsFileNum) { + currentModule = moduleIterator.next(); + cumulatedJsFileNum += currentModule.numJsFiles; + } + String fullPath = sourceFile.getName(); + if (!fullPath.contains("!/")) { + // Not a zip file + continue; + } + String relativePath = fullPath.split("!")[1]; + if (!sourceFilesByName.containsKey(relativePath)) { + sourceFilesByName.put(relativePath, sourceFile); + } else { + SourceFile firstSourceFile = sourceFilesByName.get(relativePath); + if (firstSourceFile.getCode().equals(sourceFile.getCode())) { + fileIterator.remove(); + if (currentModule != null) { + currentModule.numJsFiles--; + } + } else { + errors.add( + JSError.make( + CONFLICTING_DUPLICATE_ZIP_CONTENTS, + firstSourceFile.getName(), + sourceFile.getName())); + } + } + } + return errors.build(); + } + /** * Creates JS source code inputs from a list of files. */ diff --git a/src/com/google/javascript/jscomp/CodeGenerator.java b/src/com/google/javascript/jscomp/CodeGenerator.java index 6f0b64da66b..0000b53a5b9 100644 --- a/src/com/google/javascript/jscomp/CodeGenerator.java +++ b/src/com/google/javascript/jscomp/CodeGenerator.java @@ -62,13 +62,7 @@ private CodeGenerator(CodeConsumer consumer) { this.jsDocInfoPrinter = new JSDocInfoPrinter(false); } - static CodeGenerator forCostEstimation(CodeConsumer consumer) { - return new CodeGenerator(consumer); - } - - protected CodeGenerator( - CodeConsumer consumer, - CompilerOptions options) { + protected CodeGenerator(CodeConsumer consumer, CompilerOptions options) { cc = consumer; this.outputCharsetEncoder = new OutputCharsetEncoder(options.getOutputCharset()); @@ -80,6 +74,10 @@ protected CodeGenerator( this.jsDocInfoPrinter = new JSDocInfoPrinter(useOriginalName); } + static CodeGenerator forCostEstimation(CodeConsumer consumer) { + return new CodeGenerator(consumer); + } + /** Insert a top-level identifying file as .i.js generated typing file. */ void tagAsTypeSummary() { add("/** @fileoverview @typeSummary */\n"); @@ -102,10 +100,6 @@ protected void add(String str) { cc.add(str); } - private void addIdentifier(String identifier) { - cc.addIdentifier(identifierEscape(identifier)); - } - protected void add(Node n) { add(n, Context.OTHER); } @@ -134,7 +128,8 @@ protected void add(Node n, Context context) { Preconditions.checkState( childCount == 2, "Bad binary operator \"%s\": expected 2 arguments but got %s", - opstr, childCount); + opstr, + childCount); int p = precedence(n); // For right-hand-side of operations, only pass context if it's @@ -641,9 +636,7 @@ protected void add(Node n, Context context) { boolean preferLineBreaks = type == Token.SCRIPT - || (type == Token.BLOCK - && !preserveBlock - && n.getParent().isScript()); + || (type == Token.BLOCK && !preserveBlock && n.getParent().isScript()); for (Node c = first; c != null; c = c.getNext()) { add(c, Context.STATEMENT); @@ -780,7 +773,9 @@ protected void add(Node n, Context context) { case GETELEM: Preconditions.checkState( childCount == 2, - "Bad GETELEM node: Expected 2 children but got %s. For node: %s", childCount, n); + "Bad GETELEM node: Expected 2 children but got %s. For node: %s", + childCount, + n); addExpr(first, NodeUtil.precedence(type), context); add("["); add(first.getNext()); @@ -1271,6 +1266,10 @@ protected void add(Node n, Context context) { cc.endSourceMapping(n); } + private void addIdentifier(String identifier) { + cc.addIdentifier(identifierEscape(identifier)); + } + private int precedence(Node n) { if (n.isCast()) { return precedence(n.getFirstChild()); diff --git a/src/com/google/javascript/jscomp/CodingConventions.java b/src/com/google/javascript/jscomp/CodingConventions.java index 11574f90ecd..bd3bfe38e75 100644 --- a/src/com/google/javascript/jscomp/CodingConventions.java +++ b/src/com/google/javascript/jscomp/CodingConventions.java @@ -128,13 +128,13 @@ public boolean isExported(String name, boolean local) { } @Override - public String getPackageName(StaticSourceFile source) { - return nextConvention.getPackageName(source); + public final boolean isExported(String name) { + return isExported(name, false) || isExported(name, true); } @Override - public final boolean isExported(String name) { - return isExported(name, false) || isExported(name, true); + public String getPackageName(StaticSourceFile source) { + return nextConvention.getPackageName(source); } @Override diff --git a/src/com/google/javascript/jscomp/CompilerOptions.java b/src/com/google/javascript/jscomp/CompilerOptions.java index 474e392e7dd..00f316bdfdb 100644 --- a/src/com/google/javascript/jscomp/CompilerOptions.java +++ b/src/com/google/javascript/jscomp/CompilerOptions.java @@ -1604,13 +1604,6 @@ public void setIdGenerators(Set idGenerators) { this.idGenerators = builder.build(); } - /** - * Sets the hash function to use for Xid - */ - public void setXidHashFunction(Xid.HashFunction xidHashFunction) { - this.xidHashFunction = xidHashFunction; - } - /** * Sets the id generators to replace. */ @@ -1627,6 +1620,13 @@ public void setIdGeneratorsMap(String previousMappings) { this.idGeneratorsMapSerialized = previousMappings; } + /** + * Sets the hash function to use for Xid + */ + public void setXidHashFunction(Xid.HashFunction xidHashFunction) { + this.xidHashFunction = xidHashFunction; + } + private Reach inlineFunctionsLevel; /** Use {@link #setInlineFunctions(Reach)} instead */ @@ -1654,6 +1654,10 @@ public void setMaxFunctionSizeAfterInlining(int funAstSize) { this.maxFunctionSizeAfterInlining = funAstSize; } + public void setInlineVariables(boolean inlineVariables) { + this.inlineVariables = inlineVariables; + } + /** * Set the variable inlining policy for the compiler. */ @@ -2244,10 +2248,6 @@ public void setCrossModuleMethodMotion(boolean crossModuleMethodMotion) { this.crossModuleMethodMotion = crossModuleMethodMotion; } - public void setInlineVariables(boolean inlineVariables) { - this.inlineVariables = inlineVariables; - } - public void setInlineLocalVariables(boolean inlineLocalVariables) { this.inlineLocalVariables = inlineLocalVariables; } diff --git a/src/com/google/javascript/jscomp/ConformanceRules.java b/src/com/google/javascript/jscomp/ConformanceRules.java index 9fcd0a5b585..b671c18b334 100644 --- a/src/com/google/javascript/jscomp/ConformanceRules.java +++ b/src/com/google/javascript/jscomp/ConformanceRules.java @@ -866,24 +866,17 @@ && matchesProp(n.getFirstChild(), r)) { return ConformanceResult.CONFORMANCE; } - private boolean matchesProp(Node n, Restriction r) { - return n.isGetProp() && n.getLastChild().getString().equals(r.property); - } - private ConformanceResult checkConformance( NodeTraversal t, Node n, Restriction r, boolean isCallInvocation) { TypeIRegistry registry = t.getCompiler().getTypeIRegistry(); TypeI methodClassType = registry.getType(r.type); - Node lhs = isCallInvocation - ? n.getFirstFirstChild() - : n.getFirstChild(); + Node lhs = isCallInvocation ? n.getFirstFirstChild() : n.getFirstChild(); if (methodClassType != null && lhs.getTypeI() != null) { TypeI targetType = lhs.getTypeI().restrictByNotNullOrUndefined(); if (targetType.isUnknownType() - || targetType.isUnresolved() - || targetType.isTop() - || targetType.isEquivalentTo( - registry.getNativeType(JSTypeNative.OBJECT_TYPE))) { + || targetType.isUnresolved() + || targetType.isTop() + || targetType.isEquivalentTo(registry.getNativeType(JSTypeNative.OBJECT_TYPE))) { if (reportLooseTypeViolations && !ConformanceUtil.validateCall( compiler, n.getParent(), r.restrictedCallType, isCallInvocation)) { @@ -899,6 +892,10 @@ private ConformanceResult checkConformance( return ConformanceResult.CONFORMANCE; } + private boolean matchesProp(Node n, Restriction r) { + return n.isGetProp() && n.getLastChild().getString().equals(r.property); + } + /** * From a provide name extract the method name. */ diff --git a/src/com/google/javascript/jscomp/DiagnosticGroup.java b/src/com/google/javascript/jscomp/DiagnosticGroup.java index be10cdd2994..8509debe1cd 100644 --- a/src/com/google/javascript/jscomp/DiagnosticGroup.java +++ b/src/com/google/javascript/jscomp/DiagnosticGroup.java @@ -60,29 +60,13 @@ private DiagnosticGroup(DiagnosticType type) { this.types = ImmutableSet.of(type); } - // DiagnosticGroups with only a single DiagnosticType. - private static final Map singletons = - new HashMap<>(); - - /** Create a diagnostic group that matches only the given type. */ - public static synchronized DiagnosticGroup forType(DiagnosticType type) { - if (!singletons.containsKey(type)) { - singletons.put(type, new DiagnosticGroup(type)); - } - return singletons.get(type); - } - - /** - * Create a composite group. - */ - public DiagnosticGroup(DiagnosticGroup ...groups) { + /** Create a composite group. */ + public DiagnosticGroup(DiagnosticGroup... groups) { this(null, groups); } - /** - * Create a composite group. - */ - public DiagnosticGroup(String name, DiagnosticGroup ...groups) { + /** Create a composite group. */ + public DiagnosticGroup(String name, DiagnosticGroup... groups) { ImmutableSet.Builder set = ImmutableSet.builder(); for (DiagnosticGroup group : groups) { @@ -93,6 +77,17 @@ public DiagnosticGroup(String name, DiagnosticGroup ...groups) { this.types = set.build(); } + // DiagnosticGroups with only a single DiagnosticType. + private static final Map singletons = new HashMap<>(); + + /** Create a diagnostic group that matches only the given type. */ + public static synchronized DiagnosticGroup forType(DiagnosticType type) { + if (!singletons.containsKey(type)) { + singletons.put(type, new DiagnosticGroup(type)); + } + return singletons.get(type); + } + /** * Returns whether the given error's type matches a type * in this group. diff --git a/src/com/google/javascript/jscomp/DotFormatter.java b/src/com/google/javascript/jscomp/DotFormatter.java index 03752be22ad..84947d164c9 100644 --- a/src/com/google/javascript/jscomp/DotFormatter.java +++ b/src/com/google/javascript/jscomp/DotFormatter.java @@ -24,7 +24,6 @@ import com.google.javascript.jscomp.graph.GraphvizGraph.GraphvizNode; import com.google.javascript.rhino.Node; import com.google.javascript.rhino.TypeI; - import java.io.IOException; import java.util.Arrays; import java.util.HashMap; @@ -98,6 +97,70 @@ static String toDot(Node n, ControlFlowGraph inCFG) return builder.toString(); } + /** + * Outputs a string in DOT format that presents the graph. + * + * @param graph Input graph. + * @return A string in Dot format that presents the graph. + */ + public static String toDot(GraphvizGraph graph) { + StringBuilder builder = new StringBuilder(); + builder.append(graph.isDirected() ? "digraph" : "graph"); + builder.append(INDENT); + builder.append(graph.getName()); + builder.append(" {\n"); + builder.append(INDENT); + builder.append("node [color=lightblue2, style=filled];\n"); + + final String edgeSymbol = graph.isDirected() ? ARROW : LINE; + + List nodes = graph.getGraphvizNodes(); + + String[] nodeNames = new String[nodes.size()]; + + for (int i = 0; i < nodeNames.length; i++) { + GraphvizNode gNode = nodes.get(i); + nodeNames[i] = + gNode.getId() + + " [label=\"" + + gNode.getLabel() + + "\" color=\"" + + gNode.getColor() + + "\"]"; + } + + // We sort the nodes so we get a deterministic output every time regardless + // of the implementation of the graph data structure. + Arrays.sort(nodeNames); + + for (String nodeName : nodeNames) { + builder.append(INDENT); + builder.append(nodeName); + builder.append(";\n"); + } + + List edges = graph.getGraphvizEdges(); + + String[] edgeNames = new String[edges.size()]; + + for (int i = 0; i < edgeNames.length; i++) { + GraphvizEdge edge = edges.get(i); + edgeNames[i] = edge.getNode1Id() + edgeSymbol + edge.getNode2Id(); + } + + // Again, we sort the edges as well. + Arrays.sort(edgeNames); + + for (String edgeName : edgeNames) { + builder.append(INDENT); + builder.append(edgeName); + builder.append(";\n"); + } + + builder.append("}\n"); + return builder.toString(); + } + /** * Converts an AST to dot representation and appends it to the given buffer. * @param n the root of the AST described in the dot formatted string @@ -208,63 +271,4 @@ private void formatPreamble() throws IOException { private void formatConclusion() throws IOException { builder.append("}\n"); } - - /** - * Outputs a string in DOT format that presents the graph. - * - * @param graph Input graph. - * @return A string in Dot format that presents the graph. - */ - public static String toDot(GraphvizGraph graph) { - StringBuilder builder = new StringBuilder (); - builder.append(graph.isDirected() ? "digraph" : "graph"); - builder.append(INDENT); - builder.append(graph.getName()); - builder.append(" {\n"); - builder.append(INDENT); - builder.append("node [color=lightblue2, style=filled];\n"); - - final String edgeSymbol = graph.isDirected() ? ARROW : LINE; - - List nodes = graph.getGraphvizNodes(); - - String[] nodeNames = new String[nodes.size()]; - - for (int i = 0; i < nodeNames.length; i++) { - GraphvizNode gNode = nodes.get(i); - nodeNames[i] = gNode.getId() + " [label=\"" + gNode.getLabel() + - "\" color=\"" + gNode.getColor() + "\"]"; - } - - // We sort the nodes so we get a deterministic output every time regardless - // of the implementation of the graph data structure. - Arrays.sort(nodeNames); - - for (String nodeName : nodeNames) { - builder.append(INDENT); - builder.append(nodeName); - builder.append(";\n"); - } - - List edges = graph.getGraphvizEdges(); - - String[] edgeNames = new String[edges.size()]; - - for (int i = 0; i < edgeNames.length; i++) { - GraphvizEdge edge = edges.get(i); - edgeNames[i] = edge.getNode1Id() + edgeSymbol + edge.getNode2Id(); - } - - // Again, we sort the edges as well. - Arrays.sort(edgeNames); - - for (String edgeName : edgeNames) { - builder.append(INDENT); - builder.append(edgeName); - builder.append(";\n"); - } - - builder.append("}\n"); - return builder.toString(); - } } diff --git a/src/com/google/javascript/jscomp/ExpressionDecomposer.java b/src/com/google/javascript/jscomp/ExpressionDecomposer.java index 47b7d1571a9..db97996cba5 100644 --- a/src/com/google/javascript/jscomp/ExpressionDecomposer.java +++ b/src/com/google/javascript/jscomp/ExpressionDecomposer.java @@ -117,54 +117,22 @@ void exposeExpression(Node expression) { compiler.reportChangeToEnclosingScope(change); } - // TODO(johnlenz): This is not currently used by the function inliner, - // as moving the call out of the expression before the actual function call - // causes additional variables to be introduced. As the variable - // inliner is improved, this might be a viable option. /** - * Extract the specified expression from its parent expression. - * @see #canExposeExpression - */ - void moveExpression(Node expression) { - String resultName = getResultValueName(); - Node injectionPoint = findInjectionPoint(expression); - checkNotNull(injectionPoint); - Node injectionPointParent = injectionPoint.getParent(); - checkNotNull(injectionPointParent); - checkState(NodeUtil.isStatementBlock(injectionPointParent)); - - // Replace the expression with a reference to the new name. - Node expressionParent = expression.getParent(); - expressionParent.replaceChild(expression, IR.name(resultName)); - - // Re-add the expression at the appropriate place. - Node newExpressionRoot = NodeUtil.newVarNode(resultName, expression); - injectionPointParent.addChildBefore(newExpressionRoot, injectionPoint); - - compiler.reportChangeToEnclosingScope(injectionPointParent); - } - - /** - * Rewrite the expression such that the sub-expression is in a movable - * expression statement while maintaining evaluation order. + * Rewrite the expression such that the sub-expression is in a movable expression statement while + * maintaining evaluation order. * - * Two types of subexpressions are extracted from the source expression: - * 1) subexpressions with side-effects. - * 2) conditional expressions, that contain the call, which are transformed - * into IF statements. - * - * The following terms are used: - * expressionRoot: The top-level node before which the any extracted - * expressions should be placed before. - * nonconditionalExpr: The node that will be extracted either express. + *

Two types of subexpressions are extracted from the source expression: 1) subexpressions with + * side-effects. 2) conditional expressions, that contain the call, which are transformed into IF + * statements. * + *

The following terms are used: expressionRoot: The top-level node before which the any + * extracted expressions should be placed before. nonconditionalExpr: The node that will be + * extracted either express. */ private void exposeExpression(Node expressionRoot, Node subExpression) { - Node nonconditionalExpr = findNonconditionalParent( - subExpression, expressionRoot); + Node nonconditionalExpr = findNonconditionalParent(subExpression, expressionRoot); // Before extraction, record whether there are side-effect - boolean hasFollowingSideEffects = NodeUtil.mayHaveSideEffects( - nonconditionalExpr, compiler); + boolean hasFollowingSideEffects = NodeUtil.mayHaveSideEffects(nonconditionalExpr, compiler); Node exprInjectionPoint = findInjectionPoint(nonconditionalExpr); DecompositionState state = new DecompositionState(); @@ -172,38 +140,34 @@ private void exposeExpression(Node expressionRoot, Node subExpression) { state.extractBeforeStatement = exprInjectionPoint; // Extract expressions in the reverse order of their evaluation. - for (Node grandchild = null, - child = nonconditionalExpr, - parent = child.getParent(); - parent != expressionRoot; - grandchild = child, - child = parent, - parent = child.getParent()) { + for (Node grandchild = null, child = nonconditionalExpr, parent = child.getParent(); + parent != expressionRoot; + grandchild = child, child = parent, parent = child.getParent()) { checkState(!isConditionalOp(parent) || child == parent.getFirstChild()); if (parent.isAssign()) { - if (isSafeAssign(parent, state.sideEffects)) { - // It is always safe to inline "foo()" for expressions such as - // "a = b = c = foo();" - // As the assignment is unaffected by side effect of "foo()" - // and the names assigned-to can not influence the state before - // the call to foo. - // - // This is not true of more complex LHS values, such as - // a.x = foo(); - // next().x = foo(); - // in these cases the checks below are necessary. - } else { - // Alias "next()" in "next().foo" - Node left = parent.getFirstChild(); - Token type = left.getToken(); - if (left != child) { - checkState(NodeUtil.isGet(left)); - if (type == Token.GETELEM) { - decomposeSubExpressions(left.getLastChild(), null, state); - } - decomposeSubExpressions(left.getFirstChild(), null, state); + if (isSafeAssign(parent, state.sideEffects)) { + // It is always safe to inline "foo()" for expressions such as + // "a = b = c = foo();" + // As the assignment is unaffected by side effect of "foo()" + // and the names assigned-to can not influence the state before + // the call to foo. + // + // This is not true of more complex LHS values, such as + // a.x = foo(); + // next().x = foo(); + // in these cases the checks below are necessary. + } else { + // Alias "next()" in "next().foo" + Node left = parent.getFirstChild(); + Token type = left.getToken(); + if (left != child) { + checkState(NodeUtil.isGet(left)); + if (type == Token.GETELEM) { + decomposeSubExpressions(left.getLastChild(), null, state); } + decomposeSubExpressions(left.getFirstChild(), null, state); } + } } else if (parent.isCall() && NodeUtil.isGet(parent.getFirstChild())) { Node functionExpression = parent.getFirstChild(); decomposeSubExpressions(functionExpression.getNext(), child, state); @@ -243,6 +207,34 @@ private void exposeExpression(Node expressionRoot, Node subExpression) { } } + // TODO(johnlenz): This is not currently used by the function inliner, + // as moving the call out of the expression before the actual function call + // causes additional variables to be introduced. As the variable + // inliner is improved, this might be a viable option. + /** + * Extract the specified expression from its parent expression. + * + * @see #canExposeExpression + */ + void moveExpression(Node expression) { + String resultName = getResultValueName(); + Node injectionPoint = findInjectionPoint(expression); + checkNotNull(injectionPoint); + Node injectionPointParent = injectionPoint.getParent(); + checkNotNull(injectionPointParent); + checkState(NodeUtil.isStatementBlock(injectionPointParent)); + + // Replace the expression with a reference to the new name. + Node expressionParent = expression.getParent(); + expressionParent.replaceChild(expression, IR.name(resultName)); + + // Re-add the expression at the appropriate place. + Node newExpressionRoot = NodeUtil.newVarNode(resultName, expression); + injectionPointParent.addChildBefore(newExpressionRoot, injectionPoint); + + compiler.reportChangeToEnclosingScope(injectionPointParent); + } + /** * @return "expression" or the node closest to "expression", that does not * have a conditional ancestor. diff --git a/src/com/google/javascript/jscomp/GlobalNamespace.java b/src/com/google/javascript/jscomp/GlobalNamespace.java index bf9b920d6cc..2436cc64308 100644 --- a/src/com/google/javascript/jscomp/GlobalNamespace.java +++ b/src/com/google/javascript/jscomp/GlobalNamespace.java @@ -795,6 +795,30 @@ void handleGet(JSModule module, Scope scope, handleGet(module, scope, n, parent, name, type, true); } + /** + * Updates our representation of the global namespace to reflect a read of a global name. + * + * @param module The current module + * @param scope The current scope + * @param n The node currently being visited + * @param parent {@code n}'s parent + * @param name The global name (e.g. "a" or "a.b.c.d") + * @param type The reference type + */ + void handleGet( + JSModule module, + Scope scope, + Node n, + Node parent, + String name, + Ref.Type type, + boolean shouldCreateProp) { + Name nameObj = getOrCreateName(name, shouldCreateProp); + + // No need to look up additional ancestors, since they won't be used. + nameObj.addRef(new Ref(module, scope, n, nameObj, type, currentPreOrderIndex++)); + } + private boolean isClassDefiningCall(Node callNode) { CodingConvention convention = compiler.getCodingConvention(); // Look for goog.inherits, goog.mixin @@ -873,26 +897,6 @@ Ref.Type determineGetTypeForHookOrBooleanExpr( return Ref.Type.ALIASING_GET; } - /** - * Updates our representation of the global namespace to reflect a read - * of a global name. - * - * @param module The current module - * @param scope The current scope - * @param n The node currently being visited - * @param parent {@code n}'s parent - * @param name The global name (e.g. "a" or "a.b.c.d") - * @param type The reference type - */ - void handleGet(JSModule module, Scope scope, Node n, Node parent, - String name, Ref.Type type, boolean shouldCreateProp) { - Name nameObj = getOrCreateName(name, shouldCreateProp); - - // No need to look up additional ancestors, since they won't be used. - nameObj.addRef( - new Ref(module, scope, n, nameObj, type, currentPreOrderIndex++)); - } - /** * Updates our representation of the global namespace to reflect a read * of a global name's longest prefix before the "prototype" property if the diff --git a/src/com/google/javascript/jscomp/Linter.java b/src/com/google/javascript/jscomp/Linter.java index ce3724b6ea4..49bf42e20ca 100644 --- a/src/com/google/javascript/jscomp/Linter.java +++ b/src/com/google/javascript/jscomp/Linter.java @@ -69,38 +69,7 @@ static void lint(String filename) throws IOException { lint(Paths.get(filename), new Compiler(System.out)); } - /** - * Keep applying fixes to the given file until no more fixes can be found, - * or until fixes have been applied {@code MAX_FIXES} times. - */ - static void fixRepeatedly(String filename) throws IOException { - for (int i = 0; i < MAX_FIXES; i++) { - if (!fix(filename)) { - break; - } - } - } - - /** - * @return Whether any fixes were applied. - */ - private static boolean fix(String filename) throws IOException { - Compiler compiler = new Compiler(System.out); - FixingErrorManager errorManager = new FixingErrorManager(); - compiler.setErrorManager(errorManager); - errorManager.setCompiler(compiler); - - lint(Paths.get(filename), compiler); - - Collection fixes = errorManager.getAllFixes(); - if (!fixes.isEmpty()) { - ApplySuggestedFixes.applySuggestedFixesToFiles(fixes); - return true; - } - return false; - } - - static void lint(Path path, Compiler compiler) throws IOException { + static void lint(Path path, Compiler compiler) throws IOException { SourceFile file = SourceFile.fromFile(path.toString()); CompilerOptions options = new CompilerOptions(); options.setLanguage(LanguageMode.ECMASCRIPT_NEXT); @@ -133,4 +102,33 @@ static void lint(Path path, Compiler compiler) throws IOException { SourceFile externs = SourceFile.fromCode("", ""); compiler.compile(ImmutableList.of(externs), ImmutableList.of(file), options); } + + /** + * Keep applying fixes to the given file until no more fixes can be found, or until fixes have + * been applied {@code MAX_FIXES} times. + */ + static void fixRepeatedly(String filename) throws IOException { + for (int i = 0; i < MAX_FIXES; i++) { + if (!fix(filename)) { + break; + } + } + } + + /** @return Whether any fixes were applied. */ + private static boolean fix(String filename) throws IOException { + Compiler compiler = new Compiler(System.out); + FixingErrorManager errorManager = new FixingErrorManager(); + compiler.setErrorManager(errorManager); + errorManager.setCompiler(compiler); + + lint(Paths.get(filename), compiler); + + Collection fixes = errorManager.getAllFixes(); + if (!fixes.isEmpty()) { + ApplySuggestedFixes.applySuggestedFixesToFiles(fixes); + return true; + } + return false; + } } diff --git a/src/com/google/javascript/jscomp/NTIScope.java b/src/com/google/javascript/jscomp/NTIScope.java index 74b0fe31684..f8c54946003 100644 --- a/src/com/google/javascript/jscomp/NTIScope.java +++ b/src/com/google/javascript/jscomp/NTIScope.java @@ -576,6 +576,13 @@ Namespace getNamespace(String name) { return decl == null ? null : decl.getNamespace(); } + Namespace getNamespace(QualifiedName qname) { + Namespace ns = getNamespace(qname.getLeftmostName()); + return (ns == null || qname.isIdentifier()) + ? ns + : ns.getSubnamespace(qname.getAllButLeftmost()); + } + void addFunNamespace(Node qnameNode) { if (qnameNode.isName()) { String varName = qnameNode.getString(); @@ -641,12 +648,6 @@ void addNamespace(QualifiedName qname, Node defSite, Namespace ns) { } } - Namespace getNamespace(QualifiedName qname) { - Namespace ns = getNamespace(qname.getLeftmostName()); - return (ns == null || qname.isIdentifier()) - ? ns : ns.getSubnamespace(qname.getAllButLeftmost()); - } - private Declaration getLocalDeclaration(String name, boolean includeTypes) { checkArgument(!name.contains(".")); if (!isDefinedLocally(name, includeTypes)) { @@ -706,6 +707,15 @@ public Declaration getDeclaration(QualifiedName qname, boolean includeTypes) { return decl != null ? decl : maybeGetForwardDeclaration(qname.toString()); } + public Declaration getDeclaration(String name, boolean includeTypes) { + checkArgument(!name.contains(".")); + Declaration decl = getLocalDeclaration(name, includeTypes); + if (decl != null) { + return decl; + } + return parent == null ? null : parent.getDeclaration(name, includeTypes); + } + private Declaration maybeGetForwardDeclaration(String qname) { NTIScope globalScope = this; while (globalScope.parent != null) { @@ -717,15 +727,6 @@ private Declaration maybeGetForwardDeclaration(String qname) { return null; } - public Declaration getDeclaration(String name, boolean includeTypes) { - checkArgument(!name.contains(".")); - Declaration decl = getLocalDeclaration(name, includeTypes); - if (decl != null) { - return decl; - } - return parent == null ? null : parent.getDeclaration(name, includeTypes); - } - private Namespace getNamespaceAfterFreezing(String typeName) { checkNotNull(preservedNamespaces, "Failed to preserve namespaces post-finalization"); QualifiedName qname = QualifiedName.fromQualifiedString(typeName); diff --git a/src/com/google/javascript/jscomp/NewTypeInference.java b/src/com/google/javascript/jscomp/NewTypeInference.java index b3161555775..b40e160b371 100644 --- a/src/com/google/javascript/jscomp/NewTypeInference.java +++ b/src/com/google/javascript/jscomp/NewTypeInference.java @@ -4836,11 +4836,6 @@ private static class LValueResultBwd { } } - private LValueResultBwd analyzeLValueBwd( - Node expr, TypeEnv outEnv, JSType type, boolean doSlicing) { - return analyzeLValueBwd(expr, outEnv, type, doSlicing, false); - } - /** * We use this to avoid putting global variables in type environments, because that * can cause crashes in TypeEnv#join. @@ -4849,6 +4844,11 @@ private boolean isGlobalVariable(Node maybeName, TypeEnv env) { return maybeName.isName() && envGetType(env, maybeName.getString()) == null; } + private LValueResultBwd + analyzeLValueBwd(Node expr, TypeEnv outEnv, JSType type, boolean doSlicing) { + return analyzeLValueBwd(expr, outEnv, type, doSlicing, false); + } + /** When {@code doSlicing} is set, remove the lvalue from the returned env */ private LValueResultBwd analyzeLValueBwd(Node expr, TypeEnv outEnv, JSType type, boolean doSlicing, boolean insideQualifiedName) { diff --git a/src/com/google/javascript/jscomp/NodeUtil.java b/src/com/google/javascript/jscomp/NodeUtil.java index d4c96d1674a..96af861ebf7 100644 --- a/src/com/google/javascript/jscomp/NodeUtil.java +++ b/src/com/google/javascript/jscomp/NodeUtil.java @@ -3766,7 +3766,8 @@ private static Node getAddingRoot(Node n) { * @param name A qualified name (e.g. "foo" or "foo.bar.baz") * @return A NAME or GETPROP node */ - public static Node newQName(AbstractCompiler compiler, String name) { + public static Node newQName( + AbstractCompiler compiler, String name) { int endPos = name.indexOf('.'); if (endPos == -1) { return newName(compiler, name); @@ -3798,6 +3799,33 @@ public static Node newQName(AbstractCompiler compiler, String name) { return node; } + /** + * Creates a node representing a qualified name, copying over the source + * location information from the basis node and assigning the given original + * name to the node. + * + * @param name A qualified name (e.g. "foo" or "foo.bar.baz") + * @param basisNode The node that represents the name as currently found in + * the AST. + * @param originalName The original name of the item being represented by the + * NAME node. Used for debugging information. + * + * @return A NAME or GETPROP node + */ + static Node newQName( + AbstractCompiler compiler, String name, Node basisNode, + String originalName) { + Node node = newQName(compiler, name); + useSourceInfoForNewQName(node, basisNode); + if (!originalName.equals(node.getOriginalName())) { + // If basisNode already had the correct original name, then it will already be set correctly. + // Setting it again will force the QName node to have a different property list from all of + // its children, causing greater memory consumption. + node.setOriginalName(originalName); + } + return node; + } + /** * Creates a property access on the {@code context} tree. */ @@ -3832,33 +3860,6 @@ public static Node newQNameDeclaration( return result; } - /** - * Creates a node representing a qualified name, copying over the source - * location information from the basis node and assigning the given original - * name to the node. - * - * @param name A qualified name (e.g. "foo" or "foo.bar.baz") - * @param basisNode The node that represents the name as currently found in - * the AST. - * @param originalName The original name of the item being represented by the - * NAME node. Used for debugging information. - * - * @return A NAME or GETPROP node - */ - static Node newQName( - AbstractCompiler compiler, String name, Node basisNode, - String originalName) { - Node node = newQName(compiler, name); - useSourceInfoForNewQName(node, basisNode); - if (!originalName.equals(node.getOriginalName())) { - // If basisNode already had the correct original name, then it will already be set correctly. - // Setting it again will force the QName node to have a different property list from all of - // its children, causing greater memory consumption. - node.setOriginalName(originalName); - } - return node; - } - /** * Custom update new QName node with source info from another node. * diff --git a/src/com/google/javascript/jscomp/Reference.java b/src/com/google/javascript/jscomp/Reference.java index 5281ac9c4d0..cb759594040 100644 --- a/src/com/google/javascript/jscomp/Reference.java +++ b/src/com/google/javascript/jscomp/Reference.java @@ -50,6 +50,13 @@ public final class Reference implements StaticRef, Serializable { this(nameNode, basicBlock, t.getScope(), t.getInput().getInputId()); } + private Reference(Node nameNode, BasicBlock basicBlock, Scope scope, InputId inputId) { + this.nameNode = nameNode; + this.basicBlock = basicBlock; + this.scope = scope; + this.inputId = inputId; + } + @Override public String toString() { return nameNode.toString(); @@ -65,13 +72,6 @@ static Reference createRefForTest(CompilerInput input) { return new Reference(new Node(Token.NAME), null, null, input.getInputId()); } - private Reference(Node nameNode, BasicBlock basicBlock, Scope scope, InputId inputId) { - this.nameNode = nameNode; - this.basicBlock = basicBlock; - this.scope = scope; - this.inputId = inputId; - } - /** Makes a copy of the current reference using a new Scope instance. */ Reference cloneWithNewScope(Scope newScope) { return new Reference(nameNode, basicBlock, newScope, inputId); diff --git a/src/com/google/javascript/jscomp/SideEffectsAnalysis.java b/src/com/google/javascript/jscomp/SideEffectsAnalysis.java index 4ede8a43ef5..71e7f05a36b 100644 --- a/src/com/google/javascript/jscomp/SideEffectsAnalysis.java +++ b/src/com/google/javascript/jscomp/SideEffectsAnalysis.java @@ -581,19 +581,7 @@ private abstract static class LocationAbstraction { */ abstract LocationSummary calculateLocationSummary(Node node); - /** - * Returns an abstraction-specific EffectLocation representing - * no location. - * - *

The bottom location joined with any location should return - * that location. - */ - abstract EffectLocation getBottomLocation(); - - /** - * Calculates the abstraction-specific side effects - * for the node. - */ + /** Calculates the abstraction-specific side effects for the node. */ public LocationSummary calculateLocationSummary(Set nodes) { EffectLocation modAccumulator = getBottomLocation(); EffectLocation refAccumulator = getBottomLocation(); @@ -607,6 +595,13 @@ public LocationSummary calculateLocationSummary(Set nodes) { return new LocationSummary(modAccumulator, refAccumulator); } + + /** + * Returns an abstraction-specific EffectLocation representing no location. + * + *

The bottom location joined with any location should return that location. + */ + abstract EffectLocation getBottomLocation(); } /** * A very imprecise location abstraction in which there are only two abstract diff --git a/src/com/google/javascript/jscomp/SourceFile.java b/src/com/google/javascript/jscomp/SourceFile.java index bb9faaef859..f33de7577d9 100644 --- a/src/com/google/javascript/jscomp/SourceFile.java +++ b/src/com/google/javascript/jscomp/SourceFile.java @@ -408,11 +408,6 @@ public static SourceFile fromFile(String fileName) { return fromFile(fileName, UTF_8); } - @GwtIncompatible("java.io.File") - public static SourceFile fromPath(Path path, Charset c) { - return builder().withCharset(c).buildFromPath(path); - } - /** @deprecated Use {@link SourceFile#fromPath(Path, Charset)} */ @Deprecated @GwtIncompatible("java.io.File") @@ -427,6 +422,11 @@ public static SourceFile fromFile(File file) { return fromFile(file, UTF_8); } + @GwtIncompatible("java.io.File") + public static SourceFile fromPath(Path path, Charset c) { + return builder().withCharset(c).buildFromPath(path); + } + public static SourceFile fromCode(String fileName, String code) { return builder().buildFromCode(fileName, code); } diff --git a/src/com/google/javascript/jscomp/deps/DepsGenerator.java b/src/com/google/javascript/jscomp/deps/DepsGenerator.java index 0fdc2f5807f..1de5dc28b3b 100644 --- a/src/com/google/javascript/jscomp/deps/DepsGenerator.java +++ b/src/com/google/javascript/jscomp/deps/DepsGenerator.java @@ -414,6 +414,10 @@ static List createSourceFilesFromPaths( return files; } + static List createSourceFilesFromPaths(String... paths) { + return createSourceFilesFromPaths(Arrays.asList(paths)); + } + static List createSourceFilesFromZipPaths( Collection paths) throws IOException { List zipSourceFiles = new ArrayList<>(); @@ -422,9 +426,4 @@ static List createSourceFilesFromZipPaths( } return zipSourceFiles; } - - static List createSourceFilesFromPaths( - String ... paths) { - return createSourceFilesFromPaths(Arrays.asList(paths)); - } } diff --git a/src/com/google/javascript/jscomp/graph/DiGraph.java b/src/com/google/javascript/jscomp/graph/DiGraph.java index e7dd133d885..7c6b75843fc 100644 --- a/src/com/google/javascript/jscomp/graph/DiGraph.java +++ b/src/com/google/javascript/jscomp/graph/DiGraph.java @@ -45,12 +45,11 @@ public abstract class DiGraph extends Graph { public abstract List> getDirectedPredNodes( DiGraphNode n); + public abstract List> getDirectedPredNodes(N nodeValue); + public abstract List> getDirectedSuccNodes( DiGraphNode n); - public abstract List> - getDirectedPredNodes(N nodeValue); - public abstract List> getDirectedSuccNodes(N nodeValue); diff --git a/src/com/google/javascript/jscomp/graph/Graph.java b/src/com/google/javascript/jscomp/graph/Graph.java index 08ffa2e343c..eb426d8e7e6 100644 --- a/src/com/google/javascript/jscomp/graph/Graph.java +++ b/src/com/google/javascript/jscomp/graph/Graph.java @@ -141,6 +141,15 @@ public final void connectIfNotFound(N n1, E edge, N n2) { /** Gets an immutable list of all edges. */ public abstract List> getEdges(); + /** + * Retrieves an edge from the graph. + * + * @param n1 Node one. + * @param n2 Node two. + * @return The list of edges between those two values in the graph. + */ + public abstract List> getEdges(N n1, N n2); + /** * Gets the degree of a node. * @@ -162,15 +171,6 @@ public int getWeight(N value) { */ public abstract List> getNeighborNodes(N value); - /** - * Retrieves an edge from the graph. - * - * @param n1 Node one. - * @param n2 Node two. - * @return The list of edges between those two values in the graph. - */ - public abstract List> getEdges(N n1, N n2); - /** * Retrieves any edge from the graph. * diff --git a/src/com/google/javascript/jscomp/graph/LinkedDirectedGraph.java b/src/com/google/javascript/jscomp/graph/LinkedDirectedGraph.java index 934a5d79af1..4cac89dc162 100644 --- a/src/com/google/javascript/jscomp/graph/LinkedDirectedGraph.java +++ b/src/com/google/javascript/jscomp/graph/LinkedDirectedGraph.java @@ -175,6 +175,15 @@ public List> getEdges(N n1, N n2) { return edges; } + @Override + public List> getEdges() { + List> result = new ArrayList<>(); + for (DiGraphNode node : nodes.values()) { + result.addAll(node.getOutEdges()); + } + return Collections.unmodifiableList(result); + } + @Override public GraphEdge getFirstEdge(N n1, N n2) { DiGraphNode dNode1 = getNodeOrFail(n1); @@ -277,22 +286,20 @@ public List> getDirectedPredNodes(N nodeValue) { } @Override - public List> getDirectedSuccNodes(N nodeValue) { - return getDirectedSuccNodes(nodes.get(nodeValue)); - } - - @Override - public List> getDirectedPredNodes( - DiGraphNode dNode) { + public List> getDirectedPredNodes(DiGraphNode dNode) { checkNotNull(dNode); - List> nodeList = - new ArrayList<>(dNode.getInEdges().size()); + List> nodeList = new ArrayList<>(dNode.getInEdges().size()); for (DiGraphEdge edge : dNode.getInEdges()) { nodeList.add(edge.getSource()); } return nodeList; } + @Override + public List> getDirectedSuccNodes(N nodeValue) { + return getDirectedSuccNodes(nodes.get(nodeValue)); + } + @Override public List> getDirectedSuccNodes( DiGraphNode dNode) { @@ -359,15 +366,6 @@ public List> getNeighborNodes(N value) { return result; } - @Override - public List> getEdges() { - List> result = new ArrayList<>(); - for (DiGraphNode node : nodes.values()) { - result.addAll(node.getOutEdges()); - } - return Collections.unmodifiableList(result); - } - @Override public int getNodeDegree(N value) { DiGraphNode node = getNodeOrFail(value); diff --git a/src/com/google/javascript/jscomp/graph/LinkedUndirectedGraph.java b/src/com/google/javascript/jscomp/graph/LinkedUndirectedGraph.java index e3867532165..dd59758d9fb 100644 --- a/src/com/google/javascript/jscomp/graph/LinkedUndirectedGraph.java +++ b/src/com/google/javascript/jscomp/graph/LinkedUndirectedGraph.java @@ -140,6 +140,20 @@ public List> getEdges(N n1, N n2) { getUndirectedGraphEdges(n1, n2)); } + @SuppressWarnings("unchecked") + @Override + public List> getEdges() { + List> result = new ArrayList<>(); + for (LinkedUndirectedGraphNode node : nodes.values()) { + for (UndiGraphEdge edge : node.getNeighborEdges()) { + if (edge.getNodeA() == node) { + result.add(edge); + } + } + } + return result; + } + @Override public GraphEdge getFirstEdge(N n1, N n2) { UndiGraphNode dNode1 = getNodeOrFail(n1); @@ -229,20 +243,6 @@ public int getNodeCount() { return nodes.size(); } - @SuppressWarnings("unchecked") - @Override - public List> getEdges() { - List> result = new ArrayList<>(); - for (LinkedUndirectedGraphNode node : nodes.values()) { - for (UndiGraphEdge edge : node.getNeighborEdges()) { - if (edge.getNodeA() == node) { - result.add(edge); - } - } - } - return result; - } - @Override public int getNodeDegree(N value) { UndiGraphNode uNode = getUndirectedGraphNode(value); diff --git a/src/com/google/javascript/jscomp/newtypes/JSType.java b/src/com/google/javascript/jscomp/newtypes/JSType.java index ae5d98dd280..1ffd2c75a40 100644 --- a/src/com/google/javascript/jscomp/newtypes/JSType.java +++ b/src/com/google/javascript/jscomp/newtypes/JSType.java @@ -1206,11 +1206,6 @@ public final boolean isNonLooseSubtypeOf(JSType other) { return isSubtypeOfHelper(false, other, SubtypeCache.create(), null); } - @Override - public final boolean isSubtypeOf(TypeI other) { - return isSubtypeOf(other, SubtypeCache.create()); - } - public static MismatchInfo whyNotSubtypeOf(JSType found, JSType expected) { if (found.isSingletonObj() && expected.isSingletonObj()) { MismatchInfo[] boxedInfo = new MismatchInfo[1]; @@ -1230,6 +1225,11 @@ public static MismatchInfo whyNotSubtypeOf(JSType found, JSType expected) { return null; } + @Override + public final boolean isSubtypeOf(TypeI other) { + return isSubtypeOf(other, SubtypeCache.create()); + } + final boolean isSubtypeOf(TypeI other, SubtypeCache subSuperMap) { if (this == other) { return true; diff --git a/src/com/google/javascript/jscomp/newtypes/JSTypeCreatorFromJSDoc.java b/src/com/google/javascript/jscomp/newtypes/JSTypeCreatorFromJSDoc.java index 5d2911e7eba..6247692a212 100644 --- a/src/com/google/javascript/jscomp/newtypes/JSTypeCreatorFromJSDoc.java +++ b/src/com/google/javascript/jscomp/newtypes/JSTypeCreatorFromJSDoc.java @@ -243,6 +243,14 @@ public JSType getDeclaredTypeOfNode(JSDocInfo jsdoc, RawNominalType ownerType, ? ImmutableList.of() : ownerType.getTypeParameters()); } + private JSType getDeclaredTypeOfNode( + JSDocInfo jsdoc, DeclaredTypeRegistry registry, ImmutableList typeParameters) { + if (jsdoc == null) { + return null; + } + return getTypeFromJSTypeExpression(jsdoc.getType(), registry, typeParameters); + } + public JSType getTypeOfCommentNode( Node n, RawNominalType ownerType, DeclaredTypeRegistry registry) { return getTypeFromComment( @@ -251,15 +259,6 @@ public JSType getTypeOfCommentNode( ownerType == null ? ImmutableList.of() : ownerType.getTypeParameters()); } - private JSType getDeclaredTypeOfNode(JSDocInfo jsdoc, - DeclaredTypeRegistry registry, ImmutableList typeParameters) { - if (jsdoc == null) { - return null; - } - return getTypeFromJSTypeExpression( - jsdoc.getType(), registry, typeParameters); - } - public Set getWarnings() { return warnings; } diff --git a/src/com/google/javascript/jscomp/newtypes/JSTypes.java b/src/com/google/javascript/jscomp/newtypes/JSTypes.java index 51cb092f073..d407ec50572 100644 --- a/src/com/google/javascript/jscomp/newtypes/JSTypes.java +++ b/src/com/google/javascript/jscomp/newtypes/JSTypes.java @@ -314,6 +314,20 @@ public JSType getArrayInstance() { return getArrayInstance(this.UNKNOWN); } + public JSType getArrayInstance(JSType t) { + if (arrayType == null) { + return this.UNKNOWN; + } + ImmutableList typeParams = arrayType.getTypeParameters(); + // typeParams can be != 1 in old externs files :-S + if (typeParams.size() == 1) { + return JSType.fromObjectType( + ObjectType.fromNominalType( + this.arrayType.getAsNominalType().instantiateGenerics(ImmutableList.of(t)))); + } + return arrayType.getInstanceAsJSType(); + } + public JSType getIArrayLikeInstance(JSType t) { return this.iArrayLike == null ? this.UNKNOWN @@ -384,19 +398,6 @@ public RawNominalType getIObjectType() { return this.iObject; } - public JSType getArrayInstance(JSType t) { - if (arrayType == null) { - return this.UNKNOWN; - } - ImmutableList typeParams = arrayType.getTypeParameters(); - // typeParams can be != 1 in old externs files :-S - if (typeParams.size() == 1) { - return JSType.fromObjectType(ObjectType.fromNominalType( - this.arrayType.getAsNominalType().instantiateGenerics(ImmutableList.of(t)))); - } - return arrayType.getInstanceAsJSType(); - } - public JSType getArgumentsArrayType(JSType t) { if (this.arguments == null) { return this.UNKNOWN; @@ -411,6 +412,10 @@ public JSType getArgumentsArrayType(JSType t) { return result; } + public JSType getArgumentsArrayType() { + return getArgumentsArrayType(this.UNKNOWN); + } + public JSType getRegexpType() { return regexpInstance != null ? regexpInstance : this.UNKNOWN; } @@ -443,10 +448,6 @@ ObjectType getStringInstanceObjType() { return stringInstanceObjtype != null ? stringInstanceObjtype : this.topObjectType; } - public JSType getArgumentsArrayType() { - return getArgumentsArrayType(this.UNKNOWN); - } - public JSType getITemplateArrayType() { return iTemplateArray != null ? iTemplateArray.getInstanceAsJSType() : UNKNOWN; } diff --git a/src/com/google/javascript/jscomp/parsing/IRFactory.java b/src/com/google/javascript/jscomp/parsing/IRFactory.java index 4f782f2ac8b..0bb255c7db4 100644 --- a/src/com/google/javascript/jscomp/parsing/IRFactory.java +++ b/src/com/google/javascript/jscomp/parsing/IRFactory.java @@ -713,6 +713,10 @@ private JSDocInfo handleJsDoc(ParseTree node) { return handleJsDoc(getJsDoc(node)); } + JSDocInfo handleJsDoc(com.google.javascript.jscomp.parsing.parser.Token token) { + return handleJsDoc(getJsDoc(token)); + } + private boolean shouldAttachJSDocHere(ParseTree tree) { switch (tree.type) { case EXPRESSION_STATEMENT: @@ -765,10 +769,6 @@ private static ParseTree findNearestNode(ParseTree tree) { } } - JSDocInfo handleJsDoc(com.google.javascript.jscomp.parsing.parser.Token token) { - return handleJsDoc(getJsDoc(token)); - } - Node transform(ParseTree tree) { JSDocInfo info = handleJsDoc(tree); Node node = transformDispatcher.process(tree); @@ -839,23 +839,23 @@ static int lineno(ParseTree node) { return lineno(node.location.start); } - static int charno(ParseTree node) { - return charno(node.location.start); - } - static int lineno(com.google.javascript.jscomp.parsing.parser.Token token) { return lineno(token.location.start); } - static int charno(com.google.javascript.jscomp.parsing.parser.Token token) { - return charno(token.location.start); - } - static int lineno(SourcePosition location) { // location lines start at zero, our AST starts at 1. return location.line + 1; } + static int charno(ParseTree node) { + return charno(node.location.start); + } + + static int charno(com.google.javascript.jscomp.parsing.parser.Token token) { + return charno(token.location.start); + } + static int charno(SourcePosition location) { return location.column; } diff --git a/src/com/google/javascript/jscomp/parsing/JsDocInfoParser.java b/src/com/google/javascript/jscomp/parsing/JsDocInfoParser.java index 3919cf7c656..ea4ac2f2b0d 100644 --- a/src/com/google/javascript/jscomp/parsing/JsDocInfoParser.java +++ b/src/com/google/javascript/jscomp/parsing/JsDocInfoParser.java @@ -1459,6 +1459,36 @@ Node parseAndRecordTypeNode(JsDocToken token) { token == JsDocToken.LEFT_CURLY, false); } + /** + * Looks for a parameter type expression at the current token and if found, returns it. Note that + * this method consumes input. + * + * @param token The current token. + * @param lineno The line of the type expression. + * @param startCharno The starting character position of the type expression. + * @param matchingLC Whether the type expression starts with a "{". + * @param onlyParseSimpleNames If true, only simple type names are parsed (via a call to + * parseTypeNameAnnotation instead of parseTypeExpressionAnnotation). + * @return The type expression found or null if none. + */ + private Node parseAndRecordTypeNode( + JsDocToken token, + int lineno, + int startCharno, + boolean matchingLC, + boolean onlyParseSimpleNames) { + Node typeNode; + + if (onlyParseSimpleNames) { + typeNode = parseTypeNameAnnotation(token); + } else { + typeNode = parseTypeExpressionAnnotation(token); + } + + recordTypeNode(lineno, startCharno, typeNode, matchingLC); + return typeNode; + } + /** * Looks for a type expression at the current token and if found, * returns it. Note that this method consumes input. @@ -1497,35 +1527,6 @@ private Node parseAndRecordParamTypeNode(JsDocToken token) { return typeNode; } - /** - * Looks for a parameter type expression at the current token and if found, - * returns it. Note that this method consumes input. - * - * @param token The current token. - * @param lineno The line of the type expression. - * @param startCharno The starting character position of the type expression. - * @param matchingLC Whether the type expression starts with a "{". - * @param onlyParseSimpleNames If true, only simple type names are parsed - * (via a call to parseTypeNameAnnotation instead of - * parseTypeExpressionAnnotation). - * @return The type expression found or null if none. - */ - private Node parseAndRecordTypeNode(JsDocToken token, int lineno, - int startCharno, - boolean matchingLC, - boolean onlyParseSimpleNames) { - Node typeNode; - - if (onlyParseSimpleNames) { - typeNode = parseTypeNameAnnotation(token); - } else { - typeNode = parseTypeExpressionAnnotation(token); - } - - recordTypeNode(lineno, startCharno, typeNode, matchingLC); - return typeNode; - } - /** * Converts a JSDoc token to its string representation. */ @@ -1659,6 +1660,27 @@ private ExtractionInfo extractMultilineTextualBlock(JsDocToken token) { token, getWhitespaceOption(WhitespaceOption.SINGLE_LINE), false); } + /** + * Extracts the text found on the current line and all subsequent until either an annotation, end + * of comment or end of file is reached. Note that if this method detects an end of line as the + * first token, it will quit immediately (indicating that there is no text where it was expected). + * Note that token = info.token; should be called after this method is used to update the token + * properly in the parser. + * + * @param token The start token. + * @param option How to handle whitespace. + * @param includeAnnotations Whether the extracted text may include annotations. If set to false, + * text extraction will stop on the first encountered annotation token. + * @return The extraction information. + */ + private ExtractionInfo extractMultilineTextualBlock( + JsDocToken token, WhitespaceOption option, boolean includeAnnotations) { + if (token == JsDocToken.EOC || token == JsDocToken.EOL || token == JsDocToken.EOF) { + return new ExtractionInfo("", token); + } + return extractMultilineComment(token, option, true, includeAnnotations); + } + private WhitespaceOption getWhitespaceOption(WhitespaceOption defaultValue) { return preserveWhitespace ? WhitespaceOption.PRESERVE : defaultValue; } @@ -1677,32 +1699,6 @@ private enum WhitespaceOption { SINGLE_LINE } - /** - * Extracts the text found on the current line and all subsequent - * until either an annotation, end of comment or end of file is reached. - * Note that if this method detects an end of line as the first token, it - * will quit immediately (indicating that there is no text where it was - * expected). Note that token = info.token; should be called after this - * method is used to update the token properly in the parser. - * - * @param token The start token. - * @param option How to handle whitespace. - * @param includeAnnotations Whether the extracted text may include - * annotations. If set to false, text extraction will stop on the first - * encountered annotation token. - * - * @return The extraction information. - */ - private ExtractionInfo extractMultilineTextualBlock(JsDocToken token, - WhitespaceOption option, - boolean includeAnnotations) { - if (token == JsDocToken.EOC || token == JsDocToken.EOL || - token == JsDocToken.EOF) { - return new ExtractionInfo("", token); - } - return extractMultilineComment(token, option, true, includeAnnotations); - } - /** * Extracts the top-level block comment from the JsDoc comment, if any. diff --git a/src/com/google/javascript/jscomp/parsing/parser/util/ErrorReporter.java b/src/com/google/javascript/jscomp/parsing/parser/util/ErrorReporter.java index 44d853f080c..71c2ae6191f 100644 --- a/src/com/google/javascript/jscomp/parsing/parser/util/ErrorReporter.java +++ b/src/com/google/javascript/jscomp/parsing/parser/util/ErrorReporter.java @@ -32,14 +32,14 @@ public final void reportError( reportError(location, message); } + protected abstract void reportError(SourcePosition location, String message); + @FormatMethod public final void reportWarning( SourcePosition location, @FormatString String format, Object... arguments) { String message = SimpleFormat.format(format, arguments); reportWarning(location, message); } - - protected abstract void reportError(SourcePosition location, String message); protected abstract void reportWarning(SourcePosition location, String message); public final boolean hadError() { diff --git a/src/com/google/javascript/jscomp/regex/RegExpTree.java b/src/com/google/javascript/jscomp/regex/RegExpTree.java index b6878f7ee40..4eab0be25fe 100644 --- a/src/com/google/javascript/jscomp/regex/RegExpTree.java +++ b/src/com/google/javascript/jscomp/regex/RegExpTree.java @@ -1460,15 +1460,6 @@ private DecomposedCharset decompose(CharRanges ranges, boolean inverted) { return new DecomposedCharset(inverted, ranges, namedGroups.toString()); } - @Override - protected void appendSourceCode(StringBuilder sb) { - if (DOT_CHARSET.ranges.equals(ranges)) { - sb.append('.'); - return; - } - decompose().appendSourceCode(sb); - } - DecomposedCharset decompose() { CharRanges negRanges = CharRanges.ALL_CODE_UNITS.difference(ranges); if (!ieExplicits.isEmpty()) { @@ -1480,8 +1471,16 @@ DecomposedCharset decompose() { } DecomposedCharset positive = decompose(ranges, false); DecomposedCharset negative = decompose(negRanges, true); - return positive.complexity() <= negative.complexity() - ? positive : negative; + return positive.complexity() <= negative.complexity() ? positive : negative; + } + + @Override + protected void appendSourceCode(StringBuilder sb) { + if (DOT_CHARSET.ranges.equals(ranges)) { + sb.append('.'); + return; + } + decompose().appendSourceCode(sb); } @Override diff --git a/src/com/google/javascript/jscomp/transpile/TranspilerBuilder.java b/src/com/google/javascript/jscomp/transpile/TranspilerBuilder.java index 3c85534fbbf..535bb4336a3 100644 --- a/src/com/google/javascript/jscomp/transpile/TranspilerBuilder.java +++ b/src/com/google/javascript/jscomp/transpile/TranspilerBuilder.java @@ -50,11 +50,10 @@ public static TranspilerBuilder toEs5() { public TranspilerBuilder caching() { return caching(DEFAULT_CACHE_SPEC); } - private static final String DEFAULT_CACHE_SPEC = "maximumSize=10000"; /** - * Returns a TranspilerBuilder with cached transpilations, using the given - * cache spec. Note that the builder itself is not changed. + * Returns a TranspilerBuilder with cached transpilations, using the given cache spec. Note that + * the builder itself is not changed. */ @CheckReturnValue public TranspilerBuilder caching(String spec) { @@ -62,14 +61,16 @@ public TranspilerBuilder caching(String spec) { } /** - * Returns a TranspilerBuilder with cached transpilations, using the given - * cache builder. Note that the builder itself is not changed. + * Returns a TranspilerBuilder with cached transpilations, using the given cache builder. Note + * that the builder itself is not changed. */ @CheckReturnValue public TranspilerBuilder caching(CacheBuilder builder) { return new TranspilerBuilder(new CachingTranspiler(transpiler, builder)); } + private static final String DEFAULT_CACHE_SPEC = "maximumSize=10000"; + /** * Returns the built Transpiler. */ diff --git a/src/com/google/javascript/refactoring/Matchers.java b/src/com/google/javascript/refactoring/Matchers.java index 702774a769d..3d4758f7438 100644 --- a/src/com/google/javascript/refactoring/Matchers.java +++ b/src/com/google/javascript/refactoring/Matchers.java @@ -167,6 +167,24 @@ public static Matcher functionCall() { return functionCall(null); } + /** + * Returns a Matcher that matches all nodes that are function calls that match the provided name. + * + * @param name The name of the function to match. For non-static functions, this must be the fully + * qualified name that includes the type of the object. For instance: {@code + * ns.AppContext.prototype.get} will match {@code appContext.get} and {@code this.get} when + * called from the AppContext class. + */ + public static Matcher functionCall(final String name) { + return new Matcher() { + @Override + public boolean matches(Node node, NodeMetadata metadata) { + // TODO(mknichel): Handle the case when functions are applied through .call or .apply. + return node.isCall() && propertyAccess(name).matches(node.getFirstChild(), metadata); + } + }; + } + /** * Returns a Matcher that matches any function call that has the given * number of arguments. @@ -192,24 +210,6 @@ public static Matcher functionCallWithNumArgs(final String name, final int numAr return allOf(functionCallWithNumArgs(numArgs), functionCall(name)); } - /** - * Returns a Matcher that matches all nodes that are function calls that match - * the provided name. - * @param name The name of the function to match. For non-static functions, - * this must be the fully qualified name that includes the type of the - * object. For instance: {@code ns.AppContext.prototype.get} will match - * {@code appContext.get} and {@code this.get} when called from the - * AppContext class. - */ - public static Matcher functionCall(final String name) { - return new Matcher() { - @Override public boolean matches(Node node, NodeMetadata metadata) { - // TODO(mknichel): Handle the case when functions are applied through .call or .apply. - return node.isCall() && propertyAccess(name).matches(node.getFirstChild(), metadata); - } - }; - } - public static Matcher googRequire(final String namespace) { return new Matcher() { @Override @@ -221,14 +221,14 @@ public boolean matches(Node node, NodeMetadata metadata) { }; } - public static Matcher googModule() { - return functionCall("goog.module"); - } - public static Matcher googRequire() { return functionCall("goog.require"); } + public static Matcher googModule() { + return functionCall("goog.module"); + } + public static Matcher googModuleOrProvide() { return anyOf(googModule(), functionCall("goog.provide")); } diff --git a/src/com/google/javascript/refactoring/RefactoringDriver.java b/src/com/google/javascript/refactoring/RefactoringDriver.java index 9d9bfa67990..5b509fbaa36 100644 --- a/src/com/google/javascript/refactoring/RefactoringDriver.java +++ b/src/com/google/javascript/refactoring/RefactoringDriver.java @@ -133,6 +133,11 @@ public Builder addExternsFromFile(String filename) { return this; } + public Builder addExternsFromFile(Iterable externs) { + this.externs.addAll(Lists.transform(ImmutableList.copyOf(externs), TO_SOURCE_FILE_FN)); + return this; + } + public Builder addExternsFromCode(String code) { externs.add(SourceFile.fromCode("externs", code)); return this; @@ -143,13 +148,13 @@ public Builder addExterns(Iterable externs) { return this; } - public Builder addExternsFromFile(Iterable externs) { - this.externs.addAll(Lists.transform(ImmutableList.copyOf(externs), TO_SOURCE_FILE_FN)); + public Builder addInputsFromFile(String filename) { + inputs.add(SourceFile.fromFile(filename)); return this; } - public Builder addInputsFromFile(String filename) { - inputs.add(SourceFile.fromFile(filename)); + public Builder addInputsFromFile(Iterable inputs) { + this.inputs.addAll(Lists.transform(ImmutableList.copyOf(inputs), TO_SOURCE_FILE_FN)); return this; } @@ -167,11 +172,6 @@ public Builder addInputs(Iterable inputs) { return this; } - public Builder addInputsFromFile(Iterable inputs) { - this.inputs.addAll(Lists.transform(ImmutableList.copyOf(inputs), TO_SOURCE_FILE_FN)); - return this; - } - public Builder withCompilerOptions(CompilerOptions compilerOptions) { this.compilerOptions = checkNotNull(compilerOptions); return this; diff --git a/src/com/google/javascript/refactoring/SuggestedFix.java b/src/com/google/javascript/refactoring/SuggestedFix.java index ae70b3dd78a..d6a65d277be 100644 --- a/src/com/google/javascript/refactoring/SuggestedFix.java +++ b/src/com/google/javascript/refactoring/SuggestedFix.java @@ -252,23 +252,7 @@ public Builder delete(Node n) { return delete(n, true); } - /** - * Deletes a node and its contents from the source file. - */ - public Builder deleteWithoutRemovingWhitespaceBefore(Node n) { - return delete(n, false); - } - - /** Deletes a node without touching any surrounding whitespace. */ - public Builder deleteWithoutRemovingWhitespace(Node n) { - replacements.put( - n.getSourceFileName(), CodeReplacement.create(n.getSourceOffset(), n.getLength(), "")); - return this; - } - - /** - * Deletes a node and its contents from the source file. - */ + /** Deletes a node and its contents from the source file. */ private Builder delete(Node n, boolean deleteWhitespaceBefore) { int startPosition = n.getSourceOffset(); int length; @@ -303,8 +287,8 @@ private Builder delete(Node n, boolean deleteWhitespaceBefore) { startPosition -= startPositionDiff; length += startPositionDiff; } else { - int startPositionDiff = startPosition - ( - previousSibling.getSourceOffset() + previousSibling.getLength()); + int startPositionDiff = + startPosition - (previousSibling.getSourceOffset() + previousSibling.getLength()); startPosition -= startPositionDiff; length += startPositionDiff; } @@ -327,6 +311,18 @@ private Builder delete(Node n, boolean deleteWhitespaceBefore) { return this; } + /** Deletes a node and its contents from the source file. */ + public Builder deleteWithoutRemovingWhitespaceBefore(Node n) { + return delete(n, false); + } + + /** Deletes a node without touching any surrounding whitespace. */ + public Builder deleteWithoutRemovingWhitespace(Node n) { + replacements.put( + n.getSourceFileName(), CodeReplacement.create(n.getSourceOffset(), n.getLength(), "")); + return this; + } + /** * Renames a given node to the provided name. * @param n The node to rename. diff --git a/src/com/google/javascript/rhino/IR.java b/src/com/google/javascript/rhino/IR.java index 9dd5d526fbe..51d1ac2fac3 100644 --- a/src/com/google/javascript/rhino/IR.java +++ b/src/com/google/javascript/rhino/IR.java @@ -45,6 +45,7 @@ /** * An AST construction helper class + * * @author johnlenz@google.com (John Lenz) */ public class IR { @@ -172,6 +173,10 @@ public static Node var(Node lhs, Node value) { return declaration(lhs, value, Token.VAR); } + public static Node var(Node lhs) { + return declaration(lhs, Token.VAR); + } + public static Node let(Node lhs, Node value) { return declaration(lhs, value, Token.LET); } @@ -180,10 +185,6 @@ public static Node constNode(Node lhs, Node value) { return declaration(lhs, value, Token.CONST); } - public static Node var(Node lhs) { - return declaration(lhs, Token.VAR); - } - public static Node declaration(Node lhs, Token type) { checkState(lhs.isName() || lhs.isDestructuringPattern() || lhs.isDestructuringLhs(), lhs); if (lhs.isDestructuringPattern()) { @@ -219,14 +220,14 @@ public static Node yield() { return new Node(Token.YIELD); } - public static Node await(Node expr) { + public static Node yield(Node expr) { checkState(mayBeExpression(expr)); - return new Node(Token.AWAIT, expr); + return new Node(Token.YIELD, expr); } - public static Node yield(Node expr) { + public static Node await(Node expr) { checkState(mayBeExpression(expr)); - return new Node(Token.YIELD, expr); + return new Node(Token.AWAIT, expr); } public static Node throwNode(Node expr) { diff --git a/src/com/google/javascript/rhino/JSDocInfo.java b/src/com/google/javascript/rhino/JSDocInfo.java index 6bc6d9bcc4f..95c0326f0bb 100644 --- a/src/com/google/javascript/rhino/JSDocInfo.java +++ b/src/com/google/javascript/rhino/JSDocInfo.java @@ -60,15 +60,14 @@ import java.util.Set; /** - *

JSDoc information describing JavaScript code. JSDoc is represented as a - * unified object with fields for each JSDoc annotation, even though some - * combinations are incorrect. For instance, if a JSDoc describes an enum, - * it cannot have information about a return type. This implementation - * takes advantage of such incompatibilities to reuse fields for multiple - * purposes, reducing memory consumption.

+ * JSDoc information describing JavaScript code. JSDoc is represented as a unified object with + * fields for each JSDoc annotation, even though some combinations are incorrect. For instance, if a + * JSDoc describes an enum, it cannot have information about a return type. This implementation + * takes advantage of such incompatibilities to reuse fields for multiple purposes, reducing memory + * consumption. * - *

Constructing {@link JSDocInfo} objects is simplified by - * {@link JSDocInfoBuilder} which provides early incompatibility detection.

+ *

Constructing {@link JSDocInfo} objects is simplified by {@link JSDocInfoBuilder} which + * provides early incompatibility detection. * */ public class JSDocInfo implements Serializable { @@ -1440,10 +1439,6 @@ public int getParameterCount() { return info.parameters.size(); } - void setType(JSTypeExpression type) { - setType(type, TYPEFIELD_TYPE); - } - void setInlineType() { this.inlineType = true; } @@ -1464,6 +1459,10 @@ boolean declareTypedefType(JSTypeExpression type) { return false; } + void setType(JSTypeExpression type) { + setType(type, TYPEFIELD_TYPE); + } + private void setType(JSTypeExpression type, int mask) { if ((bitset & MASK_TYPEFIELD) != 0) { throw new IllegalStateException( @@ -1495,14 +1494,6 @@ public String getThrowsDescriptionForType(JSTypeExpression type) { return documentation.throwsDescriptions.get(type); } - /** - * Returns whether a type, specified using the {@code @type} annotation, is - * present on this JSDoc. - */ - public boolean hasType() { - return hasType(TYPEFIELD_TYPE); - } - /** * Returns whether an enum parameter type, specified using the {@code @enum} * annotation, is present on this JSDoc. @@ -1527,6 +1518,14 @@ public boolean hasReturnType() { return hasType(TYPEFIELD_RETURN); } + /** + * Returns whether a type, specified using the {@code @type} annotation, is + * present on this JSDoc. + */ + public boolean hasType() { + return hasType(TYPEFIELD_TYPE); + } + private boolean hasType(int mask) { return (bitset & MASK_TYPEFIELD) == mask; } @@ -1535,13 +1534,6 @@ public boolean hasTypeInformation() { return (bitset & MASK_TYPEFIELD) != 0; } - /** - * Gets the type specified by the {@code @type} annotation. - */ - public JSTypeExpression getType() { - return getType(TYPEFIELD_TYPE); - } - /** * Returns whether the type annotation was inlined. */ @@ -1570,6 +1562,13 @@ public JSTypeExpression getTypedefType() { return getType(TYPEFIELD_TYPEDEF); } + /** + * Gets the type specified by the {@code @type} annotation. + */ + public JSTypeExpression getType() { + return getType(TYPEFIELD_TYPE); + } + private JSTypeExpression getType(int typefield) { if ((MASK_TYPEFIELD & bitset) == typefield) { return type; diff --git a/src/com/google/javascript/rhino/JSDocInfoBuilder.java b/src/com/google/javascript/rhino/JSDocInfoBuilder.java index a175e6a161d..58f00baa909 100644 --- a/src/com/google/javascript/rhino/JSDocInfoBuilder.java +++ b/src/com/google/javascript/rhino/JSDocInfoBuilder.java @@ -147,37 +147,16 @@ public boolean isDescriptionRecorded() { return currentInfo.getDescription() != null; } - /** - * Builds a {@link JSDocInfo} object based on the populated information and - * returns it. + * Builds a {@link JSDocInfo} object based on the populated information and returns it. * - * @return a {@link JSDocInfo} object populated with the values given to this - * builder. If no value was populated, this method simply returns - * {@code null} + * @return a {@link JSDocInfo} object populated with the values given to this builder. If no value + * was populated, this method simply returns {@code null} */ public JSDocInfo build() { return build(false); } - /** - * Builds a {@link JSDocInfo} object based on the populated information and - * returns it. Once this method is called, the builder can be reused to build - * another {@link JSDocInfo} object. - * - * @return a {@link JSDocInfo} object populated with the values given to this - * builder. If no value was populated, this method simply returns - * {@code null} - */ - public JSDocInfo buildAndReset() { - JSDocInfo info = build(false); - if (currentInfo == null) { - currentInfo = new JSDocInfo(parseDocumentation); - populated = false; - } - return info; - } - /** * Builds a {@link JSDocInfo} object based on the populated information and * returns it. @@ -200,6 +179,24 @@ public JSDocInfo build(boolean always) { } } + /** + * Builds a {@link JSDocInfo} object based on the populated information and + * returns it. Once this method is called, the builder can be reused to build + * another {@link JSDocInfo} object. + * + * @return a {@link JSDocInfo} object populated with the values given to this + * builder. If no value was populated, this method simply returns + * {@code null} + */ + public JSDocInfo buildAndReset() { + JSDocInfo info = build(false); + if (currentInfo == null) { + currentInfo = new JSDocInfo(parseDocumentation); + populated = false; + } + return info; + } + /** Generate defaults when certain parameters are not specified. */ private static void populateDefaults(JSDocInfo info) { if (info.getVisibility() == null) { diff --git a/src/com/google/javascript/rhino/Node.java b/src/com/google/javascript/rhino/Node.java index 3a49629e95e..aa81ec35446 100644 --- a/src/com/google/javascript/rhino/Node.java +++ b/src/com/google/javascript/rhino/Node.java @@ -941,6 +941,10 @@ public final Node clonePropsFrom(Node other) { return this; } + public final boolean hasProps() { + return propListHead != null; + } + public final void removeProp(byte propType) { PropListItem result = removeProp(propListHead, propType); if (result != propListHead) { @@ -948,10 +952,6 @@ public final void removeProp(byte propType) { } } - public final boolean hasProps() { - return propListHead != null; - } - /** * @param item The item to inspect * @param propType The property to look for @@ -1808,11 +1808,6 @@ private NodeMismatch checkTreeEqualsImpl(Node actual, boolean jsDoc) { return null; } - /** Returns true if this node is equivalent semantically to another */ - public final boolean isEquivalentTo(Node node) { - return isEquivalentTo(node, false, true, false, false); - } - /** Checks equivalence without going into child nodes */ public final boolean isEquivalentToShallow(Node node) { return isEquivalentTo(node, false, false, false, false); @@ -1835,6 +1830,11 @@ public final boolean isEquivalentToTyped(Node node) { return isEquivalentTo(node, true, true, true, false); } + /** Returns true if this node is equivalent semantically to another */ + public final boolean isEquivalentTo(Node node) { + return isEquivalentTo(node, false, true, false, false); + } + /** * @param compareType Whether to compare the JSTypes of the nodes. * @param recurse Whether to compare the children of the current node. If not, only the count of diff --git a/src/com/google/javascript/rhino/jstype/JSType.java b/src/com/google/javascript/rhino/jstype/JSType.java index 4f35bd6746e..55550c65391 100644 --- a/src/com/google/javascript/rhino/jstype/JSType.java +++ b/src/com/google/javascript/rhino/jstype/JSType.java @@ -54,18 +54,19 @@ import java.util.Objects; /** - * Represents JavaScript value types.

+ * Represents JavaScript value types. * - * Types are split into two separate families: value types and object types. + *

Types are split into two separate families: value types and object types. * - * A special {@link UnknownType} exists to represent a wildcard type on which - * no information can be gathered. In particular, it can assign to everyone, - * is a subtype of everyone (and everyone is a subtype of it).

+ *

A special {@link UnknownType} exists to represent a wildcard type on which no information can + * be gathered. In particular, it can assign to everyone, is a subtype of everyone (and everyone is + * a subtype of it). * - * If you remove the {@link UnknownType}, the set of types in the type system - * forms a lattice with the {@link #isSubtype} relation defining the partial - * order of types. All types are united at the top of the lattice by the - * {@link AllType} and at the bottom by the {@link NoType}.

+ *

If you remove the {@link UnknownType}, the set of types in the type system forms a lattice + * with the {@link #isSubtype} relation defining the partial order of types. All types are united at + * the top of the lattice by the {@link AllType} and at the bottom by the {@link NoType}. + * + *

* */ public abstract class JSType implements TypeI { @@ -1120,6 +1121,11 @@ static JSType getLeastSupertype(JSType thisType, JSType thatType) { thisType.registry.createUnionType(thisType, thatType)); } + @Override + public TypeI meetWith(TypeI that) { + return getGreatestSubtype(this, (JSType) that); + } + /** * Gets the greatest subtype of {@code this} and {@code that}. * The greatest subtype is the meet (∧) or infimum of both types in the @@ -1136,11 +1142,6 @@ public JSType getGreatestSubtype(JSType that) { return getGreatestSubtype(this, that); } - @Override - public TypeI meetWith(TypeI that) { - return getGreatestSubtype(this, (JSType) that); - } - /** * A generic implementation meant to be used as a helper for common * getGreatestSubtype implementations. @@ -1421,6 +1422,14 @@ public boolean isSubtypeWithoutStructuralTyping(TypeI that) { (JSType) that, ImplCache.createWithoutStructuralTyping(), SubtypingMode.NORMAL); } + /** + * In files translated from Java, we typecheck null and undefined loosely. + */ + public static enum SubtypingMode { + NORMAL, + IGNORE_NULL_UNDEFINED + } + /** * Checks whether {@code this} is a subtype of {@code that}.

* Note this function also returns true if this type structurally @@ -1455,14 +1464,6 @@ public boolean isSubtype(JSType that) { ImplCache.create(), SubtypingMode.NORMAL); } - /** - * In files translated from Java, we typecheck null and undefined loosely. - */ - public static enum SubtypingMode { - NORMAL, - IGNORE_NULL_UNDEFINED - } - public boolean isSubtype(JSType that, SubtypingMode mode) { return isSubtype(that, ImplCache.create(), mode); } diff --git a/src/com/google/javascript/rhino/jstype/JSTypeRegistry.java b/src/com/google/javascript/rhino/jstype/JSTypeRegistry.java index 580bb389153..8ab9b31e87e 100644 --- a/src/com/google/javascript/rhino/jstype/JSTypeRegistry.java +++ b/src/com/google/javascript/rhino/jstype/JSTypeRegistry.java @@ -1130,45 +1130,30 @@ public JSType getType(String jsTypeName) { return namesToTypes.get(jsTypeName); } - @SuppressWarnings("unchecked") - @Override - public JSType getNativeType(JSTypeNative typeId) { - return nativeTypes[typeId.ordinal()]; - } - - @SuppressWarnings("unchecked") - @Override - public ObjectType getNativeObjectType(JSTypeNative typeId) { - return (ObjectType) getNativeType(typeId); - } - - @SuppressWarnings("unchecked") - @Override - public FunctionType getNativeFunctionType(JSTypeNative typeId) { - return (FunctionType) getNativeType(typeId); - } - /** - * Looks up a type by name. To allow for forward references to types, an - * unrecognized string has to be bound to a NamedType object that will be - * resolved later. + * Looks up a type by name. To allow for forward references to types, an unrecognized string has + * to be bound to a NamedType object that will be resolved later. * * @param scope A scope for doing type name resolution. * @param jsTypeName The name string. * @param sourceName The name of the source file where this reference appears. * @param lineno The line number of the reference. - * @return a NamedType if the string argument is not one of the known types, - * otherwise the corresponding JSType object. + * @return a NamedType if the string argument is not one of the known types, otherwise the + * corresponding JSType object. */ - public JSType getType(StaticTypedScope scope, String jsTypeName, - String sourceName, int lineno, int charno) { + public JSType getType( + StaticTypedScope scope, + String jsTypeName, + String sourceName, + int lineno, + int charno) { return getType(scope, jsTypeName, sourceName, lineno, charno, true); } /** - * @param recordUnresolvedTypes record unresolved named types and resolve - * them later. Set to false if types should be ignored for backwards - * compatibility (i.e. previously unparsed template type args). + * @param recordUnresolvedTypes record unresolved named types and resolve them later. Set to false + * if types should be ignored for backwards compatibility (i.e. previously unparsed template + * type args). */ private JSType getType( StaticTypedScope scope, @@ -1215,6 +1200,24 @@ private JSType getType( return type; } + @SuppressWarnings("unchecked") + @Override + public JSType getNativeType(JSTypeNative typeId) { + return nativeTypes[typeId.ordinal()]; + } + + @SuppressWarnings("unchecked") + @Override + public ObjectType getNativeObjectType(JSTypeNative typeId) { + return (ObjectType) getNativeType(typeId); + } + + @SuppressWarnings("unchecked") + @Override + public FunctionType getNativeFunctionType(JSTypeNative typeId) { + return (FunctionType) getNativeType(typeId); + } + /** * Flushes out the current resolved and unresolved Named Types from * the type registry. This is intended to be used ONLY before a @@ -1371,6 +1374,16 @@ public FunctionType createFunctionType( return createFunctionType(returnType, createParameters(parameterTypes)); } + /** + * @param parameters the function's parameters or {@code null} to indicate that the parameter + * types are unknown. + * @param returnType the function's return type or {@code null} to indicate that the return type + * is unknown. + */ + public FunctionType createFunctionType(JSType returnType, Node parameters) { + return new FunctionBuilder(this).withParamsNode(parameters).withReturnType(returnType).build(); + } + /** * Creates a function type. The last parameter type of the function is * considered a variable length argument. @@ -1424,6 +1437,27 @@ public Node createParameters(JSType... parameterTypes) { return createParameters(false, parameterTypes); } + /** + * Creates a tree hierarchy representing a typed argument list. + * + * @param lastVarArgs whether the last type should considered as a variable length argument. + * @param parameterTypes the parameter types. The last element of this array is considered a + * variable length argument is {@code lastVarArgs} is {@code true}. + * @return a tree hierarchy representing a typed argument list + */ + private Node createParameters(boolean lastVarArgs, JSType... parameterTypes) { + FunctionParamBuilder builder = new FunctionParamBuilder(this); + int max = parameterTypes.length - 1; + for (int i = 0; i <= max; i++) { + if (lastVarArgs && i == max) { + builder.addVarArgs(parameterTypes[i]); + } else { + builder.addRequiredParams(parameterTypes[i]); + } + } + return builder.build(); + } + /** * Creates a tree hierarchy representing a typed argument list. The last * parameter type is considered a variable length argument. @@ -1446,29 +1480,6 @@ public Node createOptionalParameters(JSType... parameterTypes) { return builder.build(); } - /** - * Creates a tree hierarchy representing a typed argument list. - * - * @param lastVarArgs whether the last type should considered as a variable - * length argument. - * @param parameterTypes the parameter types. The last element of this array - * is considered a variable length argument is {@code lastVarArgs} is - * {@code true}. - * @return a tree hierarchy representing a typed argument list - */ - private Node createParameters(boolean lastVarArgs, JSType... parameterTypes) { - FunctionParamBuilder builder = new FunctionParamBuilder(this); - int max = parameterTypes.length - 1; - for (int i = 0; i <= max; i++) { - if (lastVarArgs && i == max) { - builder.addVarArgs(parameterTypes[i]); - } else { - builder.addRequiredParams(parameterTypes[i]); - } - } - return builder.build(); - } - /** * Creates a new function type based on an existing function type but * with a new return type. @@ -1483,20 +1494,6 @@ public FunctionType createFunctionTypeWithNewReturnType( .build(); } - /** - * @param parameters the function's parameters or {@code null} - * to indicate that the parameter types are unknown. - * @param returnType the function's return type or {@code null} to indicate - * that the return type is unknown. - */ - public FunctionType createFunctionType( - JSType returnType, Node parameters) { - return new FunctionBuilder(this) - .withParamsNode(parameters) - .withReturnType(returnType) - .build(); - } - private FunctionType createNativeFunctionType( JSType returnType, Node parameters) { return new FunctionBuilder(this) diff --git a/src/com/google/javascript/rhino/jstype/UnionTypeBuilder.java b/src/com/google/javascript/rhino/jstype/UnionTypeBuilder.java index bd11a5a9446..f7fab19b966 100644 --- a/src/com/google/javascript/rhino/jstype/UnionTypeBuilder.java +++ b/src/com/google/javascript/rhino/jstype/UnionTypeBuilder.java @@ -315,6 +315,11 @@ public UnionTypeBuilder addAlternate(JSType alternate, boolean isStructural) { return this; } + /** Adds an alternate to the union type under construction. Returns this for easy chaining. */ + public UnionTypeBuilder addAlternate(JSType alternate) { + return addAlternate(alternate, false); + } + private void mayRegisterDroppedProperties(JSType subtype, JSType supertype) { if (subtype.toMaybeRecordType() != null && supertype.toMaybeRecordType() != null) { this.registry.registerDroppedPropertiesInUnion( @@ -322,14 +327,6 @@ private void mayRegisterDroppedProperties(JSType subtype, JSType supertype) { } } - /** - * Adds an alternate to the union type under construction. Returns this - * for easy chaining. - */ - public UnionTypeBuilder addAlternate(JSType alternate) { - return addAlternate(alternate, false); - } - /** * Reduce the alternates into a non-union type. * If the alternates can't be accurately represented with a non-union diff --git a/src/com/google/javascript/rhino/testing/BaseJSTypeTestCase.java b/src/com/google/javascript/rhino/testing/BaseJSTypeTestCase.java index 7e0b6a0cd62..f57fe16ee78 100644 --- a/src/com/google/javascript/rhino/testing/BaseJSTypeTestCase.java +++ b/src/com/google/javascript/rhino/testing/BaseJSTypeTestCase.java @@ -440,6 +440,14 @@ protected void assertTypeEquals(JSType expected, JSTypeExpression actual) { assertEquals(expected, resolve(actual)); } + protected final void assertTypeEquals(JSType a, JSType b) { + Asserts.assertTypeEquals(a, b); + } + + protected final void assertTypeEquals(String msg, JSType a, JSType b) { + Asserts.assertTypeEquals(msg, a, b); + } + /** * Resolves a type expression, expecting the given warnings. */ @@ -596,14 +604,6 @@ protected JSType resolve(JSTypeExpression n, String... warnings) { + " */\n" + "function ActiveXObject(progId, opt_location) {}\n"; - protected final void assertTypeEquals(JSType a, JSType b) { - Asserts.assertTypeEquals(a, b); - } - - protected final void assertTypeEquals(String msg, JSType a, JSType b) { - Asserts.assertTypeEquals(msg, a, b); - } - protected final void assertTypeNotEquals(JSType a, JSType b) { Asserts.assertTypeNotEquals(a, b); } diff --git a/test/com/google/debugging/sourcemap/SourceMapTestCase.java b/test/com/google/debugging/sourcemap/SourceMapTestCase.java index 27ef454cf5e..8667b59f0f7 100644 --- a/test/com/google/debugging/sourcemap/SourceMapTestCase.java +++ b/test/com/google/debugging/sourcemap/SourceMapTestCase.java @@ -86,19 +86,18 @@ protected void checkSourceMap(String js, String expectedMap) checkSourceMap("testcode", js, expectedMap); } + protected void checkSourceMap(String fileName, String js, String expectedMap) throws IOException { + RunResult result = compile(js, fileName); + assertThat(result.sourceMapFileContent).isEqualTo(expectedMap); + assertThat(getSourceMap(result)).isEqualTo(result.sourceMapFileContent); + } + protected String getSourceMap(RunResult result) throws IOException { StringBuilder sb = new StringBuilder(); result.sourceMap.appendTo(sb, "testcode"); return sb.toString(); } - protected void checkSourceMap(String fileName, String js, String expectedMap) - throws IOException { - RunResult result = compile(js, fileName); - assertThat(result.sourceMapFileContent).isEqualTo(expectedMap); - assertThat(getSourceMap(result)).isEqualTo(result.sourceMapFileContent); - } - /** Finds the all the __XX__ tokens in the given JavaScript string. */ private static Map findTokens(Map inputs) { Map tokens = new LinkedHashMap<>(); @@ -261,30 +260,18 @@ protected RunResult compile(String js, String fileName) { return compile(js, fileName, null, null); } - protected CompilerOptions getCompilerOptions() { - CompilerOptions options = new CompilerOptions(); - options.setLanguageIn(LanguageMode.ECMASCRIPT3); - options.setSourceMapOutputPath("testcode_source_map.out"); - options.setSourceMapFormat(getSourceMapFormat()); - options.setSourceMapDetailLevel(detailLevel); - options.setSourceMapIncludeSourcesContent(sourceMapIncludeSourcesContent); - return options; - } - - protected RunResult compile( - String js1, String fileName1, String js2, String fileName2) { + protected RunResult compile(String js1, String fileName1, String js2, String fileName2) { Compiler compiler = new Compiler(); CompilerOptions options = getCompilerOptions(); options.setChecksOnly(true); - List inputs = - ImmutableList.of(SourceFile.fromCode(fileName1, js1)); + List inputs = ImmutableList.of(SourceFile.fromCode(fileName1, js1)); if (js2 != null && fileName2 != null) { - inputs = ImmutableList.of( - SourceFile.fromCode(fileName1, js1), - SourceFile.fromCode(fileName2, js2)); + inputs = + ImmutableList.of( + SourceFile.fromCode(fileName1, js1), SourceFile.fromCode(fileName2, js2)); } Result result = compiler.compile(EXTERNS, inputs, options); @@ -307,4 +294,13 @@ protected RunResult compile( return rr; } + protected CompilerOptions getCompilerOptions() { + CompilerOptions options = new CompilerOptions(); + options.setLanguageIn(LanguageMode.ECMASCRIPT3); + options.setSourceMapOutputPath("testcode_source_map.out"); + options.setSourceMapFormat(getSourceMapFormat()); + options.setSourceMapDetailLevel(detailLevel); + options.setSourceMapIncludeSourcesContent(sourceMapIncludeSourcesContent); + return options; + } } diff --git a/test/com/google/javascript/jscomp/CheckMissingReturnTest.java b/test/com/google/javascript/jscomp/CheckMissingReturnTest.java index 8f0f3df1500..d41342324ea 100644 --- a/test/com/google/javascript/jscomp/CheckMissingReturnTest.java +++ b/test/com/google/javascript/jscomp/CheckMissingReturnTest.java @@ -262,6 +262,11 @@ private void testMissing(String returnType, String body) { testMissingInShorthandFunction(returnType, body); } + /** Creates function with return type {number} */ + private void testMissing(String body) { + testMissing("number", body); + } + private void testNotMissing(String returnType, String body) { testNotMissingInTraditionalFunction(returnType, body); testNotMissingInShorthandFunction(returnType, body); @@ -272,11 +277,6 @@ private void testNotMissing(String body) { testNotMissing("number", body); } - /** Creates function with return type {number} */ - private void testMissing(String body) { - testMissing("number", body); - } - public void testArrowFunctions() { setAcceptedLanguage(LanguageMode.ECMASCRIPT_2015); diff --git a/test/com/google/javascript/jscomp/ControlFlowAnalysisTest.java b/test/com/google/javascript/jscomp/ControlFlowAnalysisTest.java index 3a4b57e9881..32c117f71b1 100644 --- a/test/com/google/javascript/jscomp/ControlFlowAnalysisTest.java +++ b/test/com/google/javascript/jscomp/ControlFlowAnalysisTest.java @@ -25,6 +25,7 @@ import com.google.javascript.jscomp.graph.DiGraph.DiGraphNode; import com.google.javascript.rhino.Node; import com.google.javascript.rhino.Token; +import java.io.IOException; import java.util.ArrayList; import java.util.Iterator; import java.util.List; @@ -48,6 +49,29 @@ private void testCfg(String input, String expected) { testCfg(input, expected, true); } + /** + * Given an input in JavaScript, test if the control flow analysis creates the proper control flow + * graph by comparing the expected Dot file output. + * + * @param input Input JavaScript. + * @param expected Expected Graphviz Dot file. + * @param shouldTraverseFunctions Whether to traverse functions when constructing the CFG (true by + * default). Passed in to the constructor of {@link ControlFlowAnalysis}. + */ + private void testCfg(String input, String expected, boolean shouldTraverseFunctions) { + Compiler compiler = new Compiler(); + ControlFlowAnalysis cfa = new ControlFlowAnalysis(compiler, shouldTraverseFunctions, true); + + Node root = compiler.parseSyntheticCode("cfgtest", input); + cfa.process(null, root); + ControlFlowGraph cfg = cfa.getCfg(); + try { + assertEquals(expected, DotFormatter.toDot(root, cfg)); + } catch (IOException e) { + fail("Tests failed with IOExceptions"); + } + } + /** * Gets all the edges of the graph. */ @@ -231,33 +255,6 @@ private ControlFlowGraph createCfg(String input) { return createCfg(input, false); } - /** - * Given an input in JavaScript, test if the control flow analysis - * creates the proper control flow graph by comparing the expected - * Dot file output. - * - * @param input Input JavaScript. - * @param expected Expected Graphviz Dot file. - * @param shouldTraverseFunctions Whether to traverse functions when - * constructing the CFG (true by default). Passed in to the - * constructor of {@link ControlFlowAnalysis}. - */ - private void testCfg(String input, String expected, - boolean shouldTraverseFunctions) { - Compiler compiler = new Compiler(); - ControlFlowAnalysis cfa = - new ControlFlowAnalysis(compiler, shouldTraverseFunctions, true); - - Node root = compiler.parseSyntheticCode("cfgtest", input); - cfa.process(null, root); - ControlFlowGraph cfg = cfa.getCfg(); - try { - assertEquals(expected, DotFormatter.toDot(root, cfg)); - } catch (java.io.IOException e) { - fail("Tests failed with IOExceptions"); - } - } - public void testSimpleStatements() { String src = "var a; a = a; a = a"; ControlFlowGraph cfg = createCfg(src); diff --git a/test/com/google/javascript/jscomp/GoldenFileComparer.java b/test/com/google/javascript/jscomp/GoldenFileComparer.java index 18883b60cd7..39bbf9974c2 100644 --- a/test/com/google/javascript/jscomp/GoldenFileComparer.java +++ b/test/com/google/javascript/jscomp/GoldenFileComparer.java @@ -104,31 +104,19 @@ private static void compileAndCompare( } /** - * Always use these options, they set --pretty_print option for easy verification. - */ - public static CompilerOptions options() { - CompilerOptions options = new CompilerOptions(); - options.setLanguageIn(LanguageMode.ECMASCRIPT3); - // Instrumentation done - options.setPrettyPrint(true); - return options; - } - - /** - * Compile one input file and throw if the result does not match golden. - * Pass options from this class, mutated with desired options + * Compile one input file and throw if the result does not match golden. Pass options from this + * class, mutated with desired options */ public static void compileAndCompare( String goldenFileName, CompilerOptions options, String sourceFileName) throws Exception { - List sourceFiles = - ImmutableList.of(readSource(sourceFileName)); + List sourceFiles = ImmutableList.of(readSource(sourceFileName)); List externsFiles = ImmutableList.of(); compileAndCompare(goldenFileName, options, sourceFiles, externsFiles); } /** - * Compile two input files and throw if the result does not match golden. - * Pass options from this class, mutated with desired options + * Compile two input files and throw if the result does not match golden. Pass options from this + * class, mutated with desired options */ public static void compileAndCompare( String goldenFileName, @@ -138,13 +126,21 @@ public static void compileAndCompare( String externsFileName) throws Exception { // Prepare sources - List sourceFiles = ImmutableList.of( - readSource(sourceFileName1), - readSource(sourceFileName2)); + List sourceFiles = + ImmutableList.of(readSource(sourceFileName1), readSource(sourceFileName2)); List externsFiles = ImmutableList.of(SourceFile.fromFile(toFullPath(externsFileName))); compileAndCompare(goldenFileName, options, sourceFiles, externsFiles); } + + /** Always use these options, they set --pretty_print option for easy verification. */ + public static CompilerOptions options() { + CompilerOptions options = new CompilerOptions(); + options.setLanguageIn(LanguageMode.ECMASCRIPT3); + // Instrumentation done + options.setPrettyPrint(true); + return options; + } } diff --git a/test/com/google/javascript/jscomp/IntegrationTest.java b/test/com/google/javascript/jscomp/IntegrationTest.java index 10765c504ac..31d95d6e359 100644 --- a/test/com/google/javascript/jscomp/IntegrationTest.java +++ b/test/com/google/javascript/jscomp/IntegrationTest.java @@ -4281,8 +4281,19 @@ public void testES5toES6() throws Exception { compile(options, code); } + // Due to JsFileParse not being supported in the JS version, the dependency parsing delegates to + // the {@link CompilerInput$DepsFinder} class which is incompatible with the + // DefaultCodingConvention due to it throwing on methods such as extractIsModuleFile which is + // needed in {@link CompilerInput$DepsFinder#visitSubtree}. Disable this test in the JsVersion. + // TODO(tdeegan): DepsFinder should error out early if run with DefaultCodingConvention. + @GwtIncompatible + public void testES6UnusedClassesAreRemovedDefaultCodingConvention() { + testES6UnusedClassesAreRemoved(CodingConventions.getDefault()); + } + // Tests that unused classes are removed, even if they are passed to $jscomp.inherits. - private void testES6UnusedClassesAreRemoved(CodingConvention convention) { + private void + testES6UnusedClassesAreRemoved(CodingConvention convention) { CompilerOptions options = createCompilerOptions(); options.setLanguageIn(LanguageMode.ECMASCRIPT_2015); options.setLanguageOut(LanguageMode.ECMASCRIPT3); @@ -4296,16 +4307,6 @@ private void testES6UnusedClassesAreRemoved(CodingConvention convention) { assertThat(result).isEqualTo("alert(1)"); } - // Due to JsFileParse not being supported in the JS version, the dependency parsing delegates to - // the {@link CompilerInput$DepsFinder} class which is incompatible with the - // DefaultCodingConvention due to it throwing on methods such as extractIsModuleFile which is - // needed in {@link CompilerInput$DepsFinder#visitSubtree}. Disable this test in the JsVersion. - // TODO(tdeegan): DepsFinder should error out early if run with DefaultCodingConvention. - @GwtIncompatible - public void testES6UnusedClassesAreRemovedDefaultCodingConvention() { - testES6UnusedClassesAreRemoved(CodingConventions.getDefault()); - } - public void testES6UnusedClassesAreRemoved() { testES6UnusedClassesAreRemoved(new ClosureCodingConvention()); testES6UnusedClassesAreRemoved(new GoogleCodingConvention()); diff --git a/test/com/google/javascript/jscomp/IntegrationTestCase.java b/test/com/google/javascript/jscomp/IntegrationTestCase.java index ee46a663073..19030a943f1 100644 --- a/test/com/google/javascript/jscomp/IntegrationTestCase.java +++ b/test/com/google/javascript/jscomp/IntegrationTestCase.java @@ -335,6 +335,27 @@ protected void test(CompilerOptions options, } } + /** Asserts that when compiling with the given compiler options, there is an error or warning. */ + protected void test( + CompilerOptions options, String[] original, String[] compiled, DiagnosticType[] warnings) { + Compiler compiler = compile(options, original); + checkUnexpectedErrorsOrWarnings(compiler, warnings.length); + + if (compiled != null) { + Node root = compiler.getRoot().getLastChild(); + Node expectedRoot = parseExpectedCode(compiled, options, normalizeResults); + String explanation = expectedRoot.checkTreeEquals(root); + assertNull( + "\nExpected: " + + compiler.toSource(expectedRoot) + + "\nResult: " + + compiler.toSource(root) + + "\n" + + explanation, + explanation); + } + } + /** * Asserts that there is at least one parse error. */ @@ -368,25 +389,6 @@ protected void testParseError(CompilerOptions options, } } - /** - * Asserts that when compiling with the given compiler options, - * there is an error or warning. - */ - protected void test(CompilerOptions options, - String[] original, String[] compiled, DiagnosticType[] warnings) { - Compiler compiler = compile(options, original); - checkUnexpectedErrorsOrWarnings(compiler, warnings.length); - - if (compiled != null) { - Node root = compiler.getRoot().getLastChild(); - Node expectedRoot = parseExpectedCode(compiled, options, normalizeResults); - String explanation = expectedRoot.checkTreeEquals(root); - assertNull("\nExpected: " + compiler.toSource(expectedRoot) + - "\nResult: " + compiler.toSource(root) + - "\n" + explanation, explanation); - } - } - protected void checkUnexpectedErrorsOrWarnings( Compiler compiler, int expected) { int actual = compiler.getErrors().length + compiler.getWarnings().length; diff --git a/test/com/google/javascript/jscomp/NewTypeInferenceTestBase.java b/test/com/google/javascript/jscomp/NewTypeInferenceTestBase.java index da90526129b..2014cc76e13 100644 --- a/test/com/google/javascript/jscomp/NewTypeInferenceTestBase.java +++ b/test/com/google/javascript/jscomp/NewTypeInferenceTestBase.java @@ -327,6 +327,36 @@ protected final void typeCheck(String js, Diagnostic... diagnostics) { } } + private void typeCheck(String externs, String js, DiagnosticType... warningKinds) { + parseAndTypeCheck(externs, js); + JSError[] warnings = compiler.getWarnings(); + JSError[] errors = compiler.getErrors(); + String errorMessage = + LINE_JOINER.join( + "Expected warning of type:", + "================================================================", + LINE_JOINER.join(warningKinds), + "================================================================", + "but found:", + "----------------------------------------------------------------", + LINE_JOINER.join(errors) + "\n" + LINE_JOINER.join(warnings), + "----------------------------------------------------------------\n"); + assertEquals( + errorMessage + "Warning count", warningKinds.length, warnings.length + errors.length); + for (JSError warning : warnings) { + assertWithMessage("Wrong warning type\n" + errorMessage) + .that(warningKinds) + .asList() + .contains(warning.getType()); + } + for (JSError error : errors) { + assertWithMessage("Wrong warning type\n" + errorMessage) + .that(warningKinds) + .asList() + .contains(error.getType()); + } + } + protected final void typeCheckCustomExterns( String externs, String js, DiagnosticType... warningKinds) { if (this.mode.checkNative()) { @@ -343,33 +373,6 @@ protected final void typeCheckCustomExterns( } } - private void typeCheck(String externs, String js, DiagnosticType... warningKinds) { - parseAndTypeCheck(externs, js); - JSError[] warnings = compiler.getWarnings(); - JSError[] errors = compiler.getErrors(); - String errorMessage = LINE_JOINER.join( - "Expected warning of type:", - "================================================================", - LINE_JOINER.join(warningKinds), - "================================================================", - "but found:", - "----------------------------------------------------------------", - LINE_JOINER.join(errors) + "\n" + LINE_JOINER.join(warnings), - "----------------------------------------------------------------\n"); - assertEquals( - errorMessage + "Warning count", - warningKinds.length, - warnings.length + errors.length); - for (JSError warning : warnings) { - assertWithMessage("Wrong warning type\n" + errorMessage) - .that(warningKinds).asList().contains(warning.getType()); - } - for (JSError error : errors) { - assertWithMessage("Wrong warning type\n" + errorMessage) - .that(warningKinds).asList().contains(error.getType()); - } - } - // Used only in the cases where we provide extra details in the error message. // Don't use in other cases. // It is deliberately less general; no custom externs and only a single diff --git a/test/com/google/javascript/jscomp/NodeUtilTest.java b/test/com/google/javascript/jscomp/NodeUtilTest.java index 118511e8d26..3941867f241 100644 --- a/test/com/google/javascript/jscomp/NodeUtilTest.java +++ b/test/com/google/javascript/jscomp/NodeUtilTest.java @@ -46,9 +46,7 @@ import javax.annotation.Nullable; import junit.framework.TestCase; -/** - * Tests for NodeUtil - */ +/** Tests for NodeUtil */ public final class NodeUtilTest extends TestCase { private static Node parse(String js) { @@ -74,6 +72,19 @@ private static Node getNode(String js) { return var.getFirstChild(); } + private static Node getNode(Node root, Token token) { + for (Node n : root.children()) { + if (n.getToken() == token) { + return n; + } + Node potentialMatch = getNode(n, token); + if (potentialMatch != null) { + return potentialMatch; + } + } + return null; + } + private static Node getYieldNode(String js) { return checkNotNull(getYieldNode(parse(js))); } @@ -90,19 +101,6 @@ private static Node getAwaitNode(Node root) { return getNode(root, Token.AWAIT); } - private static Node getNode(Node root, Token token) { - for (Node n : root.children()) { - if (n.getToken() == token) { - return n; - } - Node potentialMatch = getNode(n, token); - if (potentialMatch != null) { - return potentialMatch; - } - } - return null; - } - public void testGetNodeByLineCol_1() { Node root = parse("var x = 1;"); assertNull(NodeUtil.getNodeByLineCol(root, 1, 0)); @@ -3268,6 +3266,19 @@ private static Node getClassNode(String js) { return getClassNode(root); } + private static Node getClassNode(Node n) { + if (n.isClass()) { + return n; + } + for (Node c : n.children()) { + Node result = getClassNode(c); + if (result != null) { + return result; + } + } + return null; + } + /** * @param js JavaScript node to be passed to {@code NodeUtil.findLhsNodesInNode}. Must be either * an EXPR_RESULT containing an assignment operation (e.g. =, +=, /=, etc) @@ -3286,19 +3297,6 @@ private static Iterable findLhsNodesInNode(String js) { return NodeUtil.findLhsNodesInNode(root); } - private static Node getClassNode(Node n) { - if (n.isClass()) { - return n; - } - for (Node c : n.children()) { - Node result = getClassNode(c); - if (result != null) { - return result; - } - } - return null; - } - private static Node getCallNode(String js) { Node root = parse(js); return getCallNode(root); diff --git a/test/com/google/javascript/jscomp/TypeCheckTest.java b/test/com/google/javascript/jscomp/TypeCheckTest.java index de2cab79f79..5a0fefedc52 100644 --- a/test/com/google/javascript/jscomp/TypeCheckTest.java +++ b/test/com/google/javascript/jscomp/TypeCheckTest.java @@ -17820,6 +17820,89 @@ private void testTypes(String js, DiagnosticType type) { testTypes(js, type, false); } + void testTypes(String js, String description, boolean isError) { + testTypes(DEFAULT_EXTERNS, js, description, isError); + } + + void testTypes( + String externs, String js, String description, boolean isError) { + parseAndTypeCheck(externs, js); + + JSError[] errors = compiler.getErrors(); + if (description != null && isError) { + assertTrue("expected an error", errors.length > 0); + assertEquals(description, errors[0].description); + errors = Arrays.asList(errors).subList(1, errors.length).toArray( + new JSError[errors.length - 1]); + } + if (errors.length > 0) { + fail("unexpected error(s):\n" + LINE_JOINER.join(errors)); + } + + JSError[] warnings = compiler.getWarnings(); + if (description != null && !isError) { + assertTrue("expected a warning", warnings.length > 0); + assertEquals(description, warnings[0].description); + warnings = Arrays.asList(warnings).subList(1, warnings.length).toArray( + new JSError[warnings.length - 1]); + } + if (warnings.length > 0) { + fail("unexpected warnings(s):\n" + LINE_JOINER.join(warnings)); + } + } + + void testTypes(String js, DiagnosticType diagnosticType, boolean isError) { + testTypes(DEFAULT_EXTERNS, js, diagnosticType, isError); + } + + void testTypes(String externs, String js, DiagnosticType diagnosticType, + boolean isError) { + parseAndTypeCheck(externs, js); + + JSError[] errors = compiler.getErrors(); + if (diagnosticType != null && isError) { + assertTrue("expected an error", errors.length > 0); + assertEquals(diagnosticType, errors[0].getType()); + errors = Arrays.asList(errors).subList(1, errors.length).toArray( + new JSError[errors.length - 1]); + } + if (errors.length > 0) { + fail("unexpected error(s):\n" + LINE_JOINER.join(errors)); + } + + JSError[] warnings = compiler.getWarnings(); + if (diagnosticType != null && !isError) { + assertTrue("expected a warning", warnings.length > 0); + assertEquals(diagnosticType, warnings[0].getType()); + warnings = Arrays.asList(warnings).subList(1, warnings.length).toArray( + new JSError[warnings.length - 1]); + } + if (warnings.length > 0) { + fail("unexpected warnings(s):\n" + LINE_JOINER.join(warnings)); + } + } + + void testTypes(String js, String[] warnings) { + Node n = compiler.parseTestCode(js); + assertEquals(0, compiler.getErrorCount()); + Node externsNode = IR.root(); + // create a parent node for the extern and source blocks + IR.root(externsNode, n); + + makeTypeCheck().processForTesting(null, n); + assertEquals(0, compiler.getErrorCount()); + if (warnings != null) { + assertEquals(warnings.length, compiler.getWarningCount()); + JSError[] messages = compiler.getWarnings(); + for (int i = 0; i < warnings.length && i < compiler.getWarningCount(); + i++) { + assertEquals(warnings[i], messages[i].description); + } + } else { + assertEquals(0, compiler.getWarningCount()); + } + } + private void testClosureTypes(String js, String description) { testClosureTypesMultipleWarnings(js, description == null ? null : ImmutableList.of(description)); @@ -17870,41 +17953,6 @@ private void testClosureTypesMultipleWarnings( } } - void testTypes(String js, String description, boolean isError) { - testTypes(DEFAULT_EXTERNS, js, description, isError); - } - - void testTypes( - String externs, String js, String description, boolean isError) { - parseAndTypeCheck(externs, js); - - JSError[] errors = compiler.getErrors(); - if (description != null && isError) { - assertTrue("expected an error", errors.length > 0); - assertEquals(description, errors[0].description); - errors = Arrays.asList(errors).subList(1, errors.length).toArray( - new JSError[errors.length - 1]); - } - if (errors.length > 0) { - fail("unexpected error(s):\n" + LINE_JOINER.join(errors)); - } - - JSError[] warnings = compiler.getWarnings(); - if (description != null && !isError) { - assertTrue("expected a warning", warnings.length > 0); - assertEquals(description, warnings[0].description); - warnings = Arrays.asList(warnings).subList(1, warnings.length).toArray( - new JSError[warnings.length - 1]); - } - if (warnings.length > 0) { - fail("unexpected warnings(s):\n" + LINE_JOINER.join(warnings)); - } - } - - void testTypes(String js, DiagnosticType diagnosticType, boolean isError) { - testTypes(DEFAULT_EXTERNS, js, diagnosticType, isError); - } - void testTypesWithExterns(String externs, String js) { testTypes(externs, js, (String) null, false); } @@ -17918,33 +17966,6 @@ void testTypesWithExtraExterns( testTypes(DEFAULT_EXTERNS + "\n" + externs, js, description, false); } - void testTypes(String externs, String js, DiagnosticType diagnosticType, - boolean isError) { - parseAndTypeCheck(externs, js); - - JSError[] errors = compiler.getErrors(); - if (diagnosticType != null && isError) { - assertTrue("expected an error", errors.length > 0); - assertEquals(diagnosticType, errors[0].getType()); - errors = Arrays.asList(errors).subList(1, errors.length).toArray( - new JSError[errors.length - 1]); - } - if (errors.length > 0) { - fail("unexpected error(s):\n" + LINE_JOINER.join(errors)); - } - - JSError[] warnings = compiler.getWarnings(); - if (diagnosticType != null && !isError) { - assertTrue("expected a warning", warnings.length > 0); - assertEquals(diagnosticType, warnings[0].getType()); - warnings = Arrays.asList(warnings).subList(1, warnings.length).toArray( - new JSError[warnings.length - 1]); - } - if (warnings.length > 0) { - fail("unexpected warnings(s):\n" + LINE_JOINER.join(warnings)); - } - } - /** * Parses and type checks the JavaScript code. */ @@ -18015,27 +18036,6 @@ private TypeCheck makeTypeCheck() { return new TypeCheck(compiler, new SemanticReverseAbstractInterpreter(registry), registry); } - void testTypes(String js, String[] warnings) { - Node n = compiler.parseTestCode(js); - assertEquals(0, compiler.getErrorCount()); - Node externsNode = IR.root(); - // create a parent node for the extern and source blocks - IR.root(externsNode, n); - - makeTypeCheck().processForTesting(null, n); - assertEquals(0, compiler.getErrorCount()); - if (warnings != null) { - assertEquals(warnings.length, compiler.getWarningCount()); - JSError[] messages = compiler.getWarnings(); - for (int i = 0; i < warnings.length && i < compiler.getWarningCount(); - i++) { - assertEquals(warnings[i], messages[i].description); - } - } else { - assertEquals(0, compiler.getWarningCount()); - } - } - String suppressMissingProperty(String ... props) { String result = "function dummy(x) { "; for (String prop : props) { diff --git a/test/com/google/javascript/jscomp/WarningsGuardTest.java b/test/com/google/javascript/jscomp/WarningsGuardTest.java index 27053116b70..bd0cf877dd5 100644 --- a/test/com/google/javascript/jscomp/WarningsGuardTest.java +++ b/test/com/google/javascript/jscomp/WarningsGuardTest.java @@ -503,17 +503,20 @@ private static JSError makeError(String sourcePath, DiagnosticType type) { return JSError.make(n, type); } - private static JSError makeErrorWithType(DiagnosticType type) { + private static JSError makeError(String sourcePath, CheckLevel level) { Node n = new Node(Token.EMPTY); - n.setSourceFileForTesting("input"); - return JSError.make(n, type); + n.setSourceFileForTesting(sourcePath); + return JSError.make(n, DiagnosticType.make("FOO", level, "Foo description")); } - private static JSError makeError(String sourcePath, CheckLevel level) { + private static JSError makeError(String sourcePath, int lineno) { + return JSError.make(sourcePath, lineno, -1, BAR_WARNING); + } + + private static JSError makeErrorWithType(DiagnosticType type) { Node n = new Node(Token.EMPTY); - n.setSourceFileForTesting(sourcePath); - return JSError.make(n, - DiagnosticType.make("FOO", level, "Foo description")); + n.setSourceFileForTesting("input"); + return JSError.make(n, type); } private static JSError makeErrorWithLevel(CheckLevel level) { @@ -522,8 +525,4 @@ private static JSError makeErrorWithLevel(CheckLevel level) { return JSError.make(n, DiagnosticType.make("FOO", level, "Foo description")); } - - private static JSError makeError(String sourcePath, int lineno) { - return JSError.make(sourcePath, lineno, -1, BAR_WARNING); - } } diff --git a/test/com/google/javascript/jscomp/parsing/ParserTest.java b/test/com/google/javascript/jscomp/parsing/ParserTest.java index c8c259f448a..825597acdbd 100644 --- a/test/com/google/javascript/jscomp/parsing/ParserTest.java +++ b/test/com/google/javascript/jscomp/parsing/ParserTest.java @@ -2210,7 +2210,7 @@ public void testUseTemplateLiteral() { testTemplateLiteral("`hello ${name} ${world}`.length;"); } - public void testTemplateLiteral() { + public void testTemplateLiterals() { expectFeatures(Feature.TEMPLATE_LITERALS); testTemplateLiteral("``"); testTemplateLiteral("`\"`");