Skip to content

Commit

Permalink
Remove disableScriptFeatureValidation() in all checks and transpilati…
Browse files Browse the repository at this point in the history
…on pass unit tests

The only places still disabling validation are optimization tests. I'm not sure it will be feasible to support this well in optimization passes, because optimizations can take an arbitrary expression or statement with arbitrary 'Features' and move it to a different file. (e.g. in CrossModuleCodeMotion and InlineFunctions)

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=202975908
  • Loading branch information
lauraharker committed Jul 2, 2018
1 parent e08eefc commit fc6fb89
Show file tree
Hide file tree
Showing 17 changed files with 40 additions and 25 deletions.
6 changes: 4 additions & 2 deletions src/com/google/javascript/jscomp/Es6RewriteArrowFunction.java
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ public void visit(NodeTraversal t, Node n, Node parent) {
visitArrowFunction(t, n, checkNotNull(context));
} else if (context != null && context.scopeBody == n) {
contextStack.pop();
addVarDeclarations(context);
addVarDeclarations(t, context);
}
}

Expand All @@ -125,12 +125,13 @@ private void visitArrowFunction(NodeTraversal t, Node n, ThisAndArgumentsContext
t.reportCodeChange();
}

private void addVarDeclarations(ThisAndArgumentsContext context) {
private void addVarDeclarations(NodeTraversal t, ThisAndArgumentsContext context) {
Node scopeBody = context.scopeBody;

if (context.needsThisVar) {
Node name = IR.name(THIS_VAR).setJSType(context.getThisType());
Node thisVar = IR.constNode(name, IR.thisNode().setJSType(context.getThisType()));
NodeUtil.addFeatureToScript(t.getCurrentFile(), Feature.CONST_DECLARATIONS);
thisVar.useSourceInfoIfMissingFromForTree(scopeBody);
makeTreeNonIndexable(thisVar);

Expand All @@ -149,6 +150,7 @@ private void addVarDeclarations(ThisAndArgumentsContext context) {
Node name = IR.name(ARGUMENTS_VAR).setJSType(context.getArgumentsType());
Node argumentsVar =
IR.constNode(name, IR.name("arguments").setJSType(context.getArgumentsType()));
NodeUtil.addFeatureToScript(t.getCurrentFile(), Feature.CONST_DECLARATIONS);
scopeBody.addChildToFront(argumentsVar);

JSDocInfoBuilder jsdoc = new JSDocInfoBuilder(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,19 +59,24 @@ public void visit(NodeTraversal t, Node n, Node parent) {
&& !parent.getParent().isFunction()) {
// Only consider declarations (all expressions have non-block parents) that are not directly
// within a function or top-level.
visitBlockScopedFunctionDeclaration(n, parent);
visitBlockScopedFunctionDeclaration(t, n, parent);
}
}

/**
* Rewrite the function declaration from:
*
* <pre>
* function f() {}
* FUNCTION
* NAME x
* PARAM_LIST
* BLOCK
* </pre> to <pre>
* </pre>
*
* to
*
* <pre>
* let f = function() {};
* LET
* NAME f
Expand All @@ -80,14 +85,16 @@ public void visit(NodeTraversal t, Node n, Node parent) {
* PARAM_LIST
* BLOCK
* </pre>
*
* This is similar to {@link Normalize.NormalizeStatements#rewriteFunctionDeclaration} but
* rewrites to "let" instead of "var".
*/
private void visitBlockScopedFunctionDeclaration(Node n, Node parent) {
private void visitBlockScopedFunctionDeclaration(NodeTraversal t, Node n, Node parent) {
// Prepare a spot for the function.
Node oldNameNode = n.getFirstChild();
Node fnNameNode = oldNameNode.cloneNode();
Node let = IR.declaration(fnNameNode, Token.LET).srcref(n);
NodeUtil.addFeatureToScript(t.getCurrentFile(), Feature.LET_DECLARATIONS);

// Prepare the function.
oldNameNode.setString("");
Expand Down
7 changes: 5 additions & 2 deletions src/com/google/javascript/jscomp/Es6RewriteDestructuring.java
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ private void visitPattern(NodeTraversal t, Node pattern, Node parent) {
} else if (NodeUtil.isEnhancedFor(parent) || NodeUtil.isEnhancedFor(parent.getParent())) {
visitDestructuringPatternInEnhancedFor(pattern);
} else if (parent.isCatch()) {
visitDestructuringPatternInCatch(pattern);
visitDestructuringPatternInCatch(t, pattern);
} else {
throw new IllegalStateException("unexpected parent");
}
Expand Down Expand Up @@ -437,11 +437,13 @@ private void wrapAssignmentInCallToArrow(NodeTraversal t, Node assignment) {
String tempVarName = DESTRUCTURING_TEMP_VAR + (destructuringVarCounter++);
Node rhs = assignment.getLastChild().detach();
Node newAssignment = IR.let(IR.name(tempVarName), rhs);
NodeUtil.addFeatureToScript(t.getCurrentFile(), Feature.LET_DECLARATIONS);
Node replacementExpr = IR.assign(assignment.getFirstChild().detach(), IR.name(tempVarName));
Node exprResult = IR.exprResult(replacementExpr);
Node returnNode = IR.returnNode(IR.name(tempVarName));
Node block = IR.block(newAssignment, exprResult, returnNode);
Node call = IR.call(IR.arrowFunction(IR.name(""), IR.paramList(), block));
NodeUtil.addFeatureToScript(t.getCurrentFile(), Feature.ARROW_FUNCTIONS);
call.useSourceInfoIfMissingFromForTree(assignment);
call.putBooleanProp(Node.FREE_CALL, true);
assignment.getParent().replaceChild(assignment, call);
Expand Down Expand Up @@ -482,12 +484,13 @@ private void visitDestructuringPatternInEnhancedFor(Node pattern) {
}
}

private void visitDestructuringPatternInCatch(Node pattern) {
private void visitDestructuringPatternInCatch(NodeTraversal t, Node pattern) {
String tempVarName = DESTRUCTURING_TEMP_VAR + (destructuringVarCounter++);
Node catchBlock = pattern.getNext();

pattern.replaceWith(IR.name(tempVarName));
catchBlock.addChildToFront(IR.declaration(pattern, IR.name(tempVarName), Token.LET));
NodeUtil.addFeatureToScript(t.getCurrentFile(), Feature.LET_DECLARATIONS);
}

/**
Expand Down
2 changes: 2 additions & 0 deletions src/com/google/javascript/jscomp/Es6RewriteModules.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import com.google.javascript.jscomp.NodeTraversal.AbstractPostOrderCallback;
import com.google.javascript.jscomp.deps.ModuleLoader;
import com.google.javascript.jscomp.deps.ModuleLoader.ResolutionMode;
import com.google.javascript.jscomp.parsing.parser.FeatureSet.Feature;
import com.google.javascript.rhino.IR;
import com.google.javascript.rhino.JSDocInfo;
import com.google.javascript.rhino.JSDocInfoBuilder;
Expand Down Expand Up @@ -535,6 +536,7 @@ private Node createExportsObject(NodeTraversal t, Node script) {
script.addChildToBack(exprResult);
} else if (mutated || importMap.containsKey(withSuffix)) {
addGetterExport(script, nodeForSourceInfo, objLit, exportedName, withSuffix);
NodeUtil.addFeatureToScript(t.getCurrentFile(), Feature.GETTER);
} else {
// This step is done before type checking and the type checker doesn't understand getters.
// However it does understand aliases. So if an export isn't mutated use an alias to make it
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,7 @@ private void visitExportDefault(NodeTraversal t, Node export, Node parent) {
// overwritten but they also act like a const for temporal dead-zone purposes.
Node var = IR.constNode(IR.name(name), export.removeFirstChild());
parent.replaceChild(export, var.useSourceInfoIfMissingFromForTree(export));
NodeUtil.addFeatureToScript(t.getCurrentFile(), Feature.CONST_DECLARATIONS);
}

exportedNameToLocalQName.put("default", new LocalQName(name, export));
Expand Down
2 changes: 2 additions & 0 deletions src/com/google/javascript/jscomp/Es6TypedToEs6Converter.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import com.google.common.collect.ImmutableList;
import com.google.javascript.jscomp.Es6RewriteClass.ClassDeclarationMetadata;
import com.google.javascript.jscomp.parsing.parser.FeatureSet.Feature;
import com.google.javascript.rhino.IR;
import com.google.javascript.rhino.JSDocInfo;
import com.google.javascript.rhino.JSDocInfo.Visibility;
Expand Down Expand Up @@ -323,6 +324,7 @@ private void visitInterface(NodeTraversal t, Node n, Node parent) {
Node empty = new Node(Token.EMPTY).useSourceInfoIfMissingFrom(n);
n.replaceChild(superTypes, empty);
members.setToken(Token.CLASS_MEMBERS);
NodeUtil.addFeatureToScript(t.getCurrentFile(), Feature.CLASSES);

maybeCreateQualifiedDeclaration(t, n, parent);
t.reportCodeChange();
Expand Down
24 changes: 16 additions & 8 deletions src/com/google/javascript/jscomp/EsNextToEs8Converter.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
* <p>Currently this class converts Object Rest/Spread properties as documented in tc39.
* https://github.com/tc39/proposal-object-rest-spread
*/
// TODO(lharker): object rest and spread are officially in ES2018/ES9, rename this pass.
public final class EsNextToEs8Converter implements NodeTraversal.Callback, HotSwapCompilerPass {
private final AbstractCompiler compiler;
private static final FeatureSet transpiledFeatures =
Expand Down Expand Up @@ -80,7 +81,7 @@ public void visit(NodeTraversal t, Node n, Node parent) {
break;
case OBJECT_PATTERN:
if (n.hasChildren() && n.getLastChild().isRest()) {
visitObjectPatternWithRest(n, parent);
visitObjectPatternWithRest(t, n, parent);
}
break;
default:
Expand Down Expand Up @@ -287,7 +288,7 @@ void insertBindings() {
* (a) the head: the pattern without the rest, whose value is the temporary variable.
* (b) the rest variable, whose value is the temporary variable after deletions.
*/
void prependDeclStatements(Token declType, Node block) {
void prependDeclStatements(NodeTraversal t, Token declType, Node block) {
List<Node> statements = new ArrayList<>();

for (ComputedPropertyName pair : this.computedProperties) {
Expand All @@ -296,6 +297,7 @@ void prependDeclStatements(Token declType, Node block) {

Node let = IR.let(IR.name(pair.varName()), pair.computation());
let.useSourceInfoIfMissingFromForTree(this.pattern);
NodeUtil.addFeatureToScript(t.getCurrentFile(), Feature.LET_DECLARATIONS);

statements.add(let);
}
Expand Down Expand Up @@ -378,7 +380,7 @@ private boolean canOmitResult(Node n) {
/*
* Handle object patterns with rest.
*/
private void visitObjectPatternWithRest(Node pattern, Node parent) {
private void visitObjectPatternWithRest(NodeTraversal t, Node pattern, Node parent) {
checkArgument(pattern.isObjectPattern(), pattern);

// A Builder object that will effect necessary changes to the syntax tree. The constructor
Expand All @@ -399,7 +401,8 @@ private void visitObjectPatternWithRest(Node pattern, Node parent) {
Node block = parent.getSecondChild();

// Use let so that the variables have block scope.
converter.prependDeclStatements(Token.LET, block); // Detaches the pattern from its parent.
converter.prependDeclStatements(t, Token.LET, block); // Detaches the pattern from its parent.
NodeUtil.addFeatureToScript(t.getCurrentFile(), Feature.LET_DECLARATIONS);

// Put the temp var in the catch, which was left empty by the removal of the pattern.
parent.addChildToFront(converter.newName(converter.rhsResultName));
Expand All @@ -422,7 +425,8 @@ private void visitObjectPatternWithRest(Node pattern, Node parent) {
Node body = parent.isParamList() ? parent.getNext() : grandparent.getNext();

// Use let so that the variables have function scope.
converter.prependDeclStatements(Token.LET, body); // Detaches the pattern from its parent.
converter.prependDeclStatements(t, Token.LET, body); // Detaches the pattern from its parent.
NodeUtil.addFeatureToScript(t.getCurrentFile(), Feature.LET_DECLARATIONS);

// Put the temp var in the param list (or default), which was left empty by the removal of the
// pattern.
Expand All @@ -443,7 +447,7 @@ private void visitObjectPatternWithRest(Node pattern, Node parent) {
Node enhancedFor = parent;

Node block = enhancedFor.getLastChild();
converter.prependDeclStatements(Token.ASSIGN, block);
converter.prependDeclStatements(t, Token.ASSIGN, block);

// Declare the deletion variable (will be assigned to later).
Node delVarDecl = IR.declaration(converter.newName(converter.restDeletionVarName), Token.LET);
Expand All @@ -456,6 +460,7 @@ private void visitObjectPatternWithRest(Node pattern, Node parent) {
enhancedFor.addChildToFront(let);

compiler.reportChangeToEnclosingScope(enhancedFor);
NodeUtil.addFeatureToScript(t.getCurrentFile(), Feature.LET_DECLARATIONS);
}

if (parent.isDestructuringLhs()) {
Expand All @@ -473,14 +478,15 @@ private void visitObjectPatternWithRest(Node pattern, Node parent) {
Node enhancedFor = grandparent.getParent();

Node block = enhancedFor.getLastChild();
converter.prependDeclStatements(grandparent.getToken(), block);
converter.prependDeclStatements(t, grandparent.getToken(), block);

// Replace the name declaration with a let for the temp variable.
Node let = new Node(Token.LET, converter.newName(converter.rhsResultName));
let.useSourceInfoIfMissingFrom(pattern);
enhancedFor.replaceChild(grandparent, let);

compiler.reportChangeToEnclosingScope(enhancedFor);
NodeUtil.addFeatureToScript(t.getCurrentFile(), Feature.LET_DECLARATIONS);
return;
} else {
/*
Expand Down Expand Up @@ -508,7 +514,7 @@ private void visitObjectPatternWithRest(Node pattern, Node parent) {
Node rhs = pattern.getNext();

Node body = IR.block();
converter.prependDeclStatements(Token.ASSIGN, body);
converter.prependDeclStatements(t, Token.ASSIGN, body);
if (!canOmitResult(parent)) {
// If the result is needed then we have to store and return a pristine copy whose
// properties are not deleted. This value is stored in the resultVar.
Expand All @@ -520,8 +526,10 @@ private void visitObjectPatternWithRest(Node pattern, Node parent) {
IR.declaration(converter.newName(converter.restDeletionVarName), Token.LET));
// Add the new let for the temp variable at the beginning of the body.
body.addChildToFront(IR.let(converter.newName(converter.rhsResultName), rhs.detach()));
NodeUtil.addFeatureToScript(t.getCurrentFile(), Feature.LET_DECLARATIONS);

Node call = IR.call(IR.arrowFunction(IR.name(""), IR.paramList(), body));
NodeUtil.addFeatureToScript(t.getCurrentFile(), Feature.ARROW_FUNCTIONS);
call.putBooleanProp(Node.FREE_CALL, true);
call.useSourceInfoIfMissingFromForTree(pattern);
NodeUtil.markNewScopesChanged(call, compiler);
Expand Down
1 change: 0 additions & 1 deletion test/com/google/javascript/jscomp/Es6ConvertSuperTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ protected void setUp() throws Exception {
setLanguageOut(LanguageMode.ECMASCRIPT5);
enableRunTypeCheckAfterProcessing();
disableTypeCheck();
disableScriptFeatureValidation();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ protected void setUp() throws Exception {

enableTypeInfoValidation();
enableTypeCheck();
disableScriptFeatureValidation();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ protected void setUp() throws Exception {
setAcceptedLanguage(LanguageMode.ECMASCRIPT_2015);
enableTypeCheck();
enableTypeInfoValidation();
disableScriptFeatureValidation();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ protected void setUp() throws Exception {
setAcceptedLanguage(LanguageMode.ECMASCRIPT_2015);
disableTypeCheck();
enableRunTypeCheckAfterProcessing();
disableScriptFeatureValidation();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ protected void setUp() throws Exception {
resolutionMode = ModuleLoader.ResolutionMode.BROWSER;
prefixReplacements = ImmutableMap.of();
pathEscaper = PathEscaper.ESCAPE;
disableScriptFeatureValidation();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ protected void setUp() throws Exception {
setLanguageOut(LanguageMode.ECMASCRIPT3);
enableTypeInfoValidation();
enableTypeCheck();
disableScriptFeatureValidation();
}

// Spreading into array literals.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ protected void setUp() throws Exception {
setLanguageOut(LanguageMode.ECMASCRIPT3);
enableRunTypeCheckAfterProcessing();
disableTypeCheck();
disableScriptFeatureValidation();
}

protected final PassFactory makePassFactory(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ protected void setUp() throws Exception {
setAcceptedLanguage(LanguageMode.ECMASCRIPT_NEXT);
setLanguageOut(LanguageMode.ECMASCRIPT_2017);
enableRunTypeCheckAfterProcessing();
disableScriptFeatureValidation();
}

@Override
Expand Down
1 change: 0 additions & 1 deletion test/com/google/javascript/jscomp/MultiPassTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ protected void setUp() throws Exception {
setAcceptedLanguage(LanguageMode.ECMASCRIPT5);
enableNormalize();
enableGatherExternProperties();
disableScriptFeatureValidation();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ protected void setUp() throws Exception {
setAcceptedLanguage(LanguageMode.ECMASCRIPT_NEXT);
setLanguageOut(LanguageMode.ECMASCRIPT3);
enableRunTypeCheckAfterProcessing();
enableScriptFeatureValidation();
}

@Override
Expand Down

0 comments on commit fc6fb89

Please sign in to comment.