diff --git a/src/com/google/javascript/jscomp/ClosureRewriteModule.java b/src/com/google/javascript/jscomp/ClosureRewriteModule.java index 74e2771e686..c97c6973dee 100644 --- a/src/com/google/javascript/jscomp/ClosureRewriteModule.java +++ b/src/com/google/javascript/jscomp/ClosureRewriteModule.java @@ -198,7 +198,6 @@ final class ClosureRewriteModule implements HotSwapCompilerPass { private final AbstractCompiler compiler; private final PreprocessorSymbolTable preprocessorSymbolTable; - private final boolean preserveSugar; /** * Indicates where new nodes should be added in relation to some other node. @@ -647,7 +646,6 @@ void removeRoot(Node toRemove) { this.compiler = compiler; this.preprocessorSymbolTable = preprocessorSymbolTable; this.rewriteState = moduleRewriteState != null ? moduleRewriteState : new GlobalRewriteState(); - this.preserveSugar = compiler.getOptions().shouldPreserveGoogModule(); } private class UnwrapGoogLoadModule extends NodeTraversal.AbstractPreOrderCallback { @@ -995,7 +993,7 @@ private void updateGoogModule(Node call) { exportTheEmptyBinaryNamespaceAt(NodeUtil.getEnclosingStatement(call), AddAt.AFTER); } - if (!currentScript.declareLegacyNamespace && !preserveSugar) { + if (!currentScript.declareLegacyNamespace) { // Otherwise it's a regular module and the goog.module() line can be removed. compiler.reportChangeToEnclosingScope(call); NodeUtil.getEnclosingStatement(call).detach(); @@ -1058,11 +1056,9 @@ && isTopLevel(t, statementNode, ScopeType.EXEC_CONTEXT)) { if (currentScript.isModule || targetIsNonLegacyGoogModule) { if (isDestructuring) { - if (!preserveSugar) { - // Delete the goog.require() because we're going to inline its alias later. - compiler.reportChangeToEnclosingScope(statementNode); - statementNode.detach(); - } + // Delete the goog.require() because we're going to inline its alias later. + compiler.reportChangeToEnclosingScope(statementNode); + statementNode.detach(); } else if (targetIsNonLegacyGoogModule) { if (!isTopLevel(t, statementNode, ScopeType.EXEC_CONTEXT)) { // Rewrite @@ -1073,15 +1069,11 @@ && isTopLevel(t, statementNode, ScopeType.EXEC_CONTEXT)) { call.replaceWith(binaryNamespaceName); compiler.reportChangeToEnclosingScope(binaryNamespaceName); } else if (importHasAlias || !rewriteState.isLegacyModule(legacyNamespace)) { - if (!preserveSugar) { - // Delete the goog.require() because we're going to inline its alias later. - compiler.reportChangeToEnclosingScope(statementNode); - statementNode.detach(); - } + // Delete the goog.require() because we're going to inline its alias later. + compiler.reportChangeToEnclosingScope(statementNode); + statementNode.detach(); } } else { - // TODO(bangert): make this compatible with preserveSugar. const B = goog.require('b') runs - // into problems because the type checker cannot handle const. // Rewrite // "var B = goog.require('B');" to // "goog.require('B');" @@ -1091,7 +1083,7 @@ && isTopLevel(t, statementNode, ScopeType.EXEC_CONTEXT)) { statementNode.replaceWith(IR.exprResult(call)); compiler.reportChangeToEnclosingScope(call); } - if (targetIsNonLegacyGoogModule && !preserveSugar) { + if (targetIsNonLegacyGoogModule) { // Add goog.require() and namespace name to preprocessor table because they're removed // by current pass. If target is not a module then goog.require() is retained for // ProcessClosurePrimitives pass and symbols will be added there instead. @@ -1480,6 +1472,7 @@ private void maybeSplitMultiVar(Node rhsNode) { Node nameNode = rhsNode.getParent(); nameNode.detach(); rhsNode.detach(); + statementNode.getParent().addChildBefore(IR.var(nameNode, rhsNode), statementNode); } @@ -1536,9 +1529,7 @@ private void reportUnrecognizedRequires() { legacyNamespace)); // Remove the require node so this problem isn't reported all over again in // ProcessClosurePrimitives. - if (!preserveSugar) { - NodeUtil.getEnclosingStatement(requireNode).detach(); - } + NodeUtil.getEnclosingStatement(requireNode).detach(); continue; } diff --git a/src/com/google/javascript/jscomp/CompilerOptions.java b/src/com/google/javascript/jscomp/CompilerOptions.java index 143e935aaa0..491e56e80d6 100644 --- a/src/com/google/javascript/jscomp/CompilerOptions.java +++ b/src/com/google/javascript/jscomp/CompilerOptions.java @@ -755,7 +755,7 @@ public void setReplaceMessagesWithChromeI18n( public boolean closurePass; /** Do not strip goog.provide()/goog.require() calls from the code. */ - private boolean preserveClosurePrimitives; + private boolean preserveGoogProvidesAndRequires; /** Processes AngularJS-specific annotations */ boolean angularPass; @@ -1263,7 +1263,7 @@ public CompilerOptions() { locale = null; markAsCompiled = false; closurePass = false; - preserveClosurePrimitives = false; + preserveGoogProvidesAndRequires = false; angularPass = false; polymerVersion = null; dartPass = false; @@ -2426,27 +2426,12 @@ public void setClosurePass(boolean closurePass) { this.closurePass = closurePass; } - /** Preserve closure primitives. - * - * For now, this only preserves goog.provide(), goog.require() and goog.module() calls. - */ - public void setPreserveClosurePrimitives(boolean preserveClosurePrimitives) { - this.preserveClosurePrimitives = preserveClosurePrimitives; - } - - // TODO(bangert): Delete this alias once it has been deprecated for 3 months. - /** Preserve goog.provide(), goog.require() and goog.module() calls. */ - @Deprecated public void setPreserveGoogProvidesAndRequires(boolean preserveGoogProvidesAndRequires) { - setPreserveClosurePrimitives(preserveGoogProvidesAndRequires); + this.preserveGoogProvidesAndRequires = preserveGoogProvidesAndRequires; } public boolean shouldPreservesGoogProvidesAndRequires() { - return this.preserveClosurePrimitives || this.shouldGenerateTypedExterns(); - } - - public boolean shouldPreserveGoogModule() { - return this.preserveClosurePrimitives; + return this.preserveGoogProvidesAndRequires || this.shouldGenerateTypedExterns(); } public void setPreserveTypeAnnotations(boolean preserveTypeAnnotations) { @@ -2889,7 +2874,7 @@ public String toString() { .add("preferSingleQuotes", preferSingleQuotes) .add("preferStableNames", preferStableNames) .add("preserveDetailedSourceInfo", preservesDetailedSourceInfo()) - .add("preserveGoogProvidesAndRequires", preserveClosurePrimitives) + .add("preserveGoogProvidesAndRequires", preserveGoogProvidesAndRequires) .add("preserveTypeAnnotations", preserveTypeAnnotations) .add("prettyPrint", prettyPrint) .add("preventLibraryInjection", preventLibraryInjection) diff --git a/src/com/google/javascript/jscomp/ProcessClosurePrimitives.java b/src/com/google/javascript/jscomp/ProcessClosurePrimitives.java index c947ec5834c..e21329aec54 100644 --- a/src/com/google/javascript/jscomp/ProcessClosurePrimitives.java +++ b/src/com/google/javascript/jscomp/ProcessClosurePrimitives.java @@ -375,11 +375,8 @@ public void visit(NodeTraversal t, Node n, Node parent) { private boolean validPrimitiveCall(NodeTraversal t, Node n) { if (!n.getParent().isExprResult() || !t.inGlobalHoistScope()) { - // Ignore invalid primitives if we didn't strip module sugar. - if (!compiler.getOptions().shouldPreserveGoogModule()) { - compiler.report(t.makeError(n, INVALID_CLOSURE_CALL_ERROR)); - return false; - } + compiler.report(t.makeError(n, INVALID_CLOSURE_CALL_ERROR)); + return false; } return true; } diff --git a/src/com/google/javascript/jscomp/TemplateAstMatcher.java b/src/com/google/javascript/jscomp/TemplateAstMatcher.java index 00b002f19b8..3a7455fb6b1 100644 --- a/src/com/google/javascript/jscomp/TemplateAstMatcher.java +++ b/src/com/google/javascript/jscomp/TemplateAstMatcher.java @@ -440,9 +440,7 @@ private boolean matchesNode(Node template, Node ast) { // subsequent usages of the same named node are equivalent. return ast.getString().equals(this.localVarMatches.get(paramIndex)); } else { - String originalName = ast.getOriginalName(); - String name = (originalName != null) ? originalName : ast.getString(); - this.localVarMatches.set(paramIndex, name); + this.localVarMatches.set(paramIndex, ast.getString()); } } else if (isTemplateParameterStringLiteralNode(template)) { int paramIndex = (int) (template.getDouble()); diff --git a/src/com/google/javascript/refactoring/Matchers.java b/src/com/google/javascript/refactoring/Matchers.java index 702774a769d..d8ba521cc86 100644 --- a/src/com/google/javascript/refactoring/Matchers.java +++ b/src/com/google/javascript/refactoring/Matchers.java @@ -221,16 +221,12 @@ 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 googModuleOrProvide() { - return anyOf(googModule(), functionCall("goog.provide")); + return anyOf(functionCall("goog.module"), functionCall("goog.provide")); } /** diff --git a/src/com/google/javascript/refactoring/RefactoringDriver.java b/src/com/google/javascript/refactoring/RefactoringDriver.java index cff51322cf3..403dc9e1be9 100644 --- a/src/com/google/javascript/refactoring/RefactoringDriver.java +++ b/src/com/google/javascript/refactoring/RefactoringDriver.java @@ -106,13 +106,10 @@ public static CompilerOptions getCompilerOptions() { options.setCheckSuspiciousCode(true); options.setCheckSymbols(true); options.setCheckTypes(true); - options.setBrokenClosureRequiresLevel(CheckLevel.OFF); - // TODO(bangert): Remove this -- we want to rewrite code before closure syntax is removed. - // Unfortunately, setClosurePass is required, or code doesn't type check. - options.setClosurePass(true); + options.setClosurePass(true); options.setGenerateExports(true); - options.setPreserveClosurePrimitives(true); - + options.setPreserveGoogProvidesAndRequires(true); + options.setWarningLevel(DiagnosticGroups.STRICT_MISSING_REQUIRE, CheckLevel.WARNING); options.setWarningLevel(DiagnosticGroups.EXTRA_REQUIRE, CheckLevel.WARNING); options.setWarningLevel(DiagnosticGroups.LINT_CHECKS, CheckLevel.WARNING); diff --git a/src/com/google/javascript/refactoring/RefasterJsScanner.java b/src/com/google/javascript/refactoring/RefasterJsScanner.java index b426cb92b03..1fbfa5fee24 100644 --- a/src/com/google/javascript/refactoring/RefasterJsScanner.java +++ b/src/com/google/javascript/refactoring/RefasterJsScanner.java @@ -29,7 +29,6 @@ import com.google.javascript.jscomp.NodeUtil; import com.google.javascript.jscomp.SourceFile; import com.google.javascript.jscomp.TypeMatchingStrategy; -import com.google.javascript.rhino.IR; import com.google.javascript.rhino.JSDocInfo; import com.google.javascript.rhino.Node; import com.google.javascript.rhino.TypeIRegistry; @@ -141,18 +140,10 @@ public List processMatch(Match match) { .isEquivalentTo(matchedTemplate.afterTemplate.getLastChild())) { return ImmutableList.of(); } - - HashMap shortNames = new HashMap<>(); - for (String require : matchedTemplate.getGoogRequiresToAdd()) { - fix.addGoogRequire(match, require); - shortNames.put(require, fix.getRequireName(match, require)); - } - Node newNode = transformNode( matchedTemplate.afterTemplate.getLastChild(), - matchedTemplate.matcher.getTemplateNodeToMatchMap(), - shortNames); + matchedTemplate.matcher.getTemplateNodeToMatchMap()); Node nodeToReplace = match.getNode(); fix.attachMatchedNodeInfo(nodeToReplace, match.getMetadata().getCompiler()); fix.replace(nodeToReplace, newNode, match.getMetadata().getCompiler()); @@ -170,7 +161,10 @@ public List processMatch(Match match) { fix.delete(n); n = n.getNext(); } - + // Add/remove any goog.requires + for (String require : matchedTemplate.getGoogRequiresToAdd()) { + fix.addGoogRequire(match, require); + } for (String require : matchedTemplate.getGoogRequiresToRemove()) { fix.removeGoogRequire(match, require); } @@ -178,13 +172,10 @@ public List processMatch(Match match) { } /** - * Transforms the template node to a replacement node by mapping the template names to the ones - * that were matched against in the JsSourceMatcher. + * Transforms the template node to a replacement node by mapping the template names to + * the ones that were matched against in the JsSourceMatcher. */ - private Node transformNode( - Node templateNode, - Map templateNodeToMatchMap, - Map shortNames) { + private Node transformNode(Node templateNode, Map templateNodeToMatchMap) { Node clone = templateNode.cloneNode(); if (templateNode.isName()) { String name = templateNode.getString(); @@ -210,17 +201,8 @@ private Node transformNode( clone.putBooleanProp(Node.FREE_CALL, false); } } - if (templateNode.isQualifiedName()) { - String name = templateNode.getQualifiedName(); - if (shortNames.containsKey(name)) { - String shortName = shortNames.get(name); - if (!shortName.equals(name)) { - return IR.name(shortNames.get(name)); - } - } - } for (Node child : templateNode.children()) { - clone.addChildToBack(transformNode(child, templateNodeToMatchMap, shortNames)); + clone.addChildToBack(transformNode(child, templateNodeToMatchMap)); } return clone; } diff --git a/src/com/google/javascript/refactoring/SuggestedFix.java b/src/com/google/javascript/refactoring/SuggestedFix.java index 6917910c698..e740d30c7c9 100644 --- a/src/com/google/javascript/refactoring/SuggestedFix.java +++ b/src/com/google/javascript/refactoring/SuggestedFix.java @@ -27,8 +27,6 @@ import com.google.javascript.jscomp.AbstractCompiler; import com.google.javascript.jscomp.CodePrinter; import com.google.javascript.jscomp.CompilerOptions; -import com.google.javascript.jscomp.NodeTraversal; -import com.google.javascript.jscomp.NodeTraversal.AbstractPreOrderCallback; import com.google.javascript.jscomp.NodeUtil; import com.google.javascript.jscomp.parsing.JsDocInfoParser; import com.google.javascript.rhino.IR; @@ -101,7 +99,6 @@ public SetMultimap getReplacements() { return sb.toString(); } - // TODO(bangert): Find a non-conflicting name. static String getShortNameForRequire(String namespace) { int lastDot = namespace.lastIndexOf('.'); if (lastDot == -1) { @@ -204,8 +201,7 @@ public Builder insertBefore(Node nodeToInsertBefore, String content) { private Builder insertBefore(Node nodeToInsertBefore, String content, String sortKey) { int startPosition = nodeToInsertBefore.getSourceOffset(); JSDocInfo jsDoc = NodeUtil.getBestJSDocInfo(nodeToInsertBefore); - // ClosureRewriteModule adds jsDOC everywhere. - if (jsDoc != null && jsDoc.getOriginalCommentString() != null) { + if (jsDoc != null) { startPosition = jsDoc.getOriginalCommentPosition(); } Preconditions.checkNotNull(nodeToInsertBefore.getSourceFileName(), @@ -607,7 +603,7 @@ public Builder addLhsToGoogRequire(Match m, String namespace) { insertBefore(existingNode, "const " + shortName + " = "); return this; } - + /** * Adds a goog.require for the given namespace to the file if it does not * already exist. @@ -634,8 +630,8 @@ public Builder addGoogRequire(Match m, String namespace) { IR.string(namespace)); String shortName = getShortNameForRequire(namespace); - boolean useConstRequire = usesConstGoogRequires(metadata, script); - if (useConstRequire) { + + if (script.isModuleBody()) { googRequireNode = IR.constNode(IR.name(shortName), googRequireNode); } else { googRequireNode = IR.exprResult(googRequireNode); @@ -646,9 +642,6 @@ public Builder addGoogRequire(Match m, String namespace) { Node nodeToInsertBefore = null; Node child = script.getFirstChild(); while (child != null) { - if (Matchers.googModule().matches(child, metadata)) { - lastModuleOrProvideNode = child; - } if (NodeUtil.isExprCall(child)) { // TODO(mknichel): Replace this logic with a function argument // Matcher when it exists. @@ -666,13 +659,7 @@ public Builder addGoogRequire(Match m, String namespace) { } else if (NodeUtil.isNameDeclaration(child) && child.getFirstFirstChild() != null && Matchers.googRequire().matches(child.getFirstFirstChild(), metadata)) { - lastGoogRequireNode = child.getFirstFirstChild(); - String requireName = child.getFirstChild().getString(); - String originalName = child.getFirstChild().getOriginalName(); - if (originalName != null) { - requireName = originalName; - } - if (shortName.compareTo(requireName) < 0) { + if (shortName.compareTo(child.getFirstChild().getString()) < 0) { nodeToInsertBefore = child; break; } @@ -713,39 +700,6 @@ public Builder addGoogRequire(Match m, String namespace) { nodeToInsertBefore, googRequireNode, m.getMetadata().getCompiler(), namespace); } - /** - * If the namespace has a short name, return it. Otherwise return the full name. - * - *

Assumes {@link addGoogRequire} was already called. - */ - public String getRequireName(Match m, String namespace) { - Node existingNode = findGoogRequireNode(m.getNode(), m.getMetadata(), namespace); - if (existingNode != null && (existingNode.isConst() || existingNode.isVar())) { - Node lhsAssign = existingNode.getFirstChild(); - String originalName = lhsAssign.getOriginalName(); - if (originalName != null) { - return originalName; // The import was renamed inside a module. - } - return lhsAssign.getQualifiedName(); - } - Node script = NodeUtil.getEnclosingScript(m.getNode()); - - if (script != null && usesConstGoogRequires(m.getMetadata(), script)) { - return getShortNameForRequire(namespace); - } - return namespace; - } - - /** True if the file uses {@code const foo = goog.require('namespace.foo');} */ - private boolean usesConstGoogRequires(final NodeMetadata metadata, Node script) { - if (script.isModuleBody()) { - return true; - } - HasConstRequireOrModuleCallback callback = new HasConstRequireOrModuleCallback(metadata); - NodeTraversal.traverseEs6(metadata.getCompiler(), script, callback); - return callback.getUsesConstRequires(); - } - /** * Removes a goog.require for the given namespace to the file if it * already exists. @@ -872,34 +826,4 @@ public boolean isInClosurizedFile() { return isInClosurizedFile; } } - - /** Traverse an AST and find {@code goog.module} or {@code const X = goog.require('...');}. */ - private static class HasConstRequireOrModuleCallback extends AbstractPreOrderCallback { - private boolean usesConstRequires; - final NodeMetadata metadata; - - public HasConstRequireOrModuleCallback(NodeMetadata metadata) { - this.usesConstRequires = false; - this.metadata = metadata; - } - - boolean getUsesConstRequires() { - return usesConstRequires; - } - - @Override - public boolean shouldTraverse(NodeTraversal nodeTraversal, Node n, Node parent) { - if (Matchers.googModule().matches(n, metadata) || isConstRequire(n, metadata)) { - usesConstRequires = true; - return false; - } - return true; - } - - private static boolean isConstRequire(Node node, NodeMetadata metadata) { - return node.isConst() - && node.getFirstFirstChild() != null - && Matchers.googRequire().matches(node.getFirstFirstChild(), metadata); - } - } } diff --git a/test/com/google/javascript/jscomp/CodePrinterTest.java b/test/com/google/javascript/jscomp/CodePrinterTest.java index c1ad10524d7..087d19ed8cd 100644 --- a/test/com/google/javascript/jscomp/CodePrinterTest.java +++ b/test/com/google/javascript/jscomp/CodePrinterTest.java @@ -2670,7 +2670,6 @@ public void testEs6GoogModule() { + "}\n" + "exports.fn = fn;\n"; String expectedCode = "" - + "goog.module('foo.bar');\n" + "void 0;\n" + "var module$exports$foo$bar = {};\n" + "const STR = '3';\n" diff --git a/test/com/google/javascript/refactoring/ErrorToFixMapperTest.java b/test/com/google/javascript/refactoring/ErrorToFixMapperTest.java index 54d737f6b9f..a84017423e5 100644 --- a/test/com/google/javascript/refactoring/ErrorToFixMapperTest.java +++ b/test/com/google/javascript/refactoring/ErrorToFixMapperTest.java @@ -651,7 +651,7 @@ public void testMissingRequire_unsorted1() { @Test public void testMissingRequire_unsorted2() { // Both the fix for requires being unsorted, and the fix for the missing require, are applied. - // The end result is ordered. + // However, the end result is still out of order. assertChanges( LINE_JOINER.join( "goog.module('module');", @@ -664,10 +664,10 @@ public void testMissingRequire_unsorted2() { "alert(new DomHelper());"), LINE_JOINER.join( "goog.module('module');", + "const Xray = goog.require('goog.rays.Xray');", "", "const Anteater = goog.require('goog.Anteater');", "const DomHelper = goog.require('goog.dom.DomHelper');", - "const Xray = goog.require('goog.rays.Xray');", "", "alert(new Anteater());", "alert(new Xray());", diff --git a/test/com/google/javascript/refactoring/MatchersTest.java b/test/com/google/javascript/refactoring/MatchersTest.java index 972620c4a11..8dcc8d11847 100644 --- a/test/com/google/javascript/refactoring/MatchersTest.java +++ b/test/com/google/javascript/refactoring/MatchersTest.java @@ -208,17 +208,6 @@ public void testFunctionCall_prototype() { assertFalse(Matchers.functionCall("Foo.prototype.baz").matches(fnCall, metadata)); } - @Test - public void testGoogModule() { - String input = "goog.module('testcase');"; - Compiler compiler = getCompiler(input); - Node root = compileToScriptRoot(compiler); - Node fnCall = root.getFirstFirstChild(); - NodeMetadata metadata = new NodeMetadata(compiler); - assertTrue(Matchers.googModule().matches(fnCall, metadata)); - assertTrue(Matchers.googModuleOrProvide().matches(fnCall, metadata)); - } - @Test public void testEnum() { String input = "/** @enum {string} */ var foo = {BAR: 'baz'};"; diff --git a/test/com/google/javascript/refactoring/RefasterJsScannerTest.java b/test/com/google/javascript/refactoring/RefasterJsScannerTest.java index b1685e1b9f4..b3e75cd124a 100644 --- a/test/com/google/javascript/refactoring/RefasterJsScannerTest.java +++ b/test/com/google/javascript/refactoring/RefasterJsScannerTest.java @@ -19,7 +19,6 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; -import com.google.common.base.Joiner; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.javascript.jscomp.Compiler; @@ -33,6 +32,7 @@ import org.junit.runner.RunWith; import org.junit.runners.JUnit4; + /** * Unit tests for {RefasterJsScanner}. * @@ -727,95 +727,6 @@ public void test_unknownTemplateTypesNonNullable() throws Exception { assertChanges(externs, originalCode, null, template); } - @Test - public void test_importConstGoogRequire() throws Exception { - String externs = ""; - String originalCode = - Joiner.on('\n').join( - "goog.module('testcase');", - "", - "function f() { var loc = 'str'; }"); - String expectedCode = - Joiner.on('\n').join( - "goog.module('testcase');", - "const foo = goog.require('goog.foo');", - "", - "function f() { var loc = foo.f(); }"); - String template = - Joiner.on('\n').join( - "/**", - "* +require {goog.foo}", - "*/", - "function before_foo() {", - " var a = 'str';", - "};", - "function after_foo() {", - " var a = goog.foo.f();", - "}"); - assertChanges(externs, originalCode, expectedCode, template); - } - - @Test - public void test_importConstGoogRequireMultipleImports() throws Exception { - String externs = ""; - String originalCode = - Joiner.on('\n').join( - "goog.module('testcase');", - "const alpha = goog.require('goog.alpha');", - "const omega = goog.require('goog.omega');", - "", - "function f() { var loc = 'str'; }"); - String expectedCode = - Joiner.on('\n').join( - "goog.module('testcase');", - "const alpha = goog.require('goog.alpha');", - "const foo = goog.require('goog.foo');", - "const omega = goog.require('goog.omega');", - "", - "function f() { var loc = foo.f(); }"); - String template = - Joiner.on('\n').join( - "/**", - "* +require {goog.foo}", - "*/", - "function before_foo() {", - " var a = 'str';", - "};", - "function after_foo() {", - " var a = goog.foo.f();", - "}"); - assertChanges(externs, originalCode, expectedCode, template); - } - - @Test - public void test_importConstGoogRequireAlreadyNamed() throws Exception { - String externs = ""; - String originalCode = - Joiner.on('\n').join( - "goog.module('testcase');", - "const bar = goog.require('goog.foo');", - "", - "var loc = 'str';"); - String expectedCode = - Joiner.on('\n').join( - "goog.module('testcase');", - "const bar = goog.require('goog.foo');", - "", - "var loc = bar.f();"); - String template = - Joiner.on('\n').join( - "/**", - "* +require {goog.foo}", - "*/", - "function before_foo() {", - " var a = 'str';", - "};", - "function after_foo() {", - " var a = goog.foo.f();", - "}"); - assertChanges(externs, originalCode, expectedCode, template); - } - private static Compiler createCompiler() { return new Compiler(); } @@ -842,21 +753,10 @@ private static void assertChanges( RefasterJsScanner scanner = new RefasterJsScanner(); scanner.loadRefasterJsTemplateFromCode(refasterJsTemplate); - RefactoringDriver driver = - new RefactoringDriver.Builder(scanner) - .addExternsFromCode("function Symbol() {};" + externs) - .addInputsFromCode(originalCode) - .addInputsFromCode( - Joiner.on('\n').join( - "goog.module('goog.foo');", - "/** Trivial function. \n", - " * @return {string}", - " */", - "exports.f = function () { ", - " return 'str';", - "};"), - "foo.js") - .build(); + RefactoringDriver driver = new RefactoringDriver.Builder(scanner) + .addExternsFromCode("function Symbol() {};" + externs) + .addInputsFromCode(originalCode) + .build(); List fixes = driver.drive(); String newCode = ApplySuggestedFixes.applySuggestedFixesToCode( fixes, ImmutableMap.of("input", originalCode)).get("input"); diff --git a/test/com/google/javascript/refactoring/SuggestedFixTest.java b/test/com/google/javascript/refactoring/SuggestedFixTest.java index 2e289c19b51..d48c80577de 100644 --- a/test/com/google/javascript/refactoring/SuggestedFixTest.java +++ b/test/com/google/javascript/refactoring/SuggestedFixTest.java @@ -786,85 +786,6 @@ public void testAddGoogRequire_alreadyExists() { assertThat(replacementMap).isEmpty(); } - @Test - public void testAddRequireModule() { - String input = - Joiner.on('\n').join( - "goog.module('js.Foo');", - "", - "/** @private */", - "function foo_() {", - "}"); - String expected = - Joiner.on('\n').join( - "goog.module('js.Foo');", - "const safe = goog.require('goog.safe');", - "", - "/** @private */", - "function foo_() {", - "}"); - Compiler compiler = getCompiler(input); - Node root = compileToScriptRoot(compiler); - Match match = new Match(root.getFirstChild(), new NodeMetadata(compiler)); - SuggestedFix fix = new SuggestedFix.Builder().addGoogRequire(match, "goog.safe").build(); - assertChanges(fix, "", input, expected); - } - - @Test - public void testAddRequireConst() { - String input = - Joiner.on('\n').join( - "const bar = goog.require('goog.bar');", - "", - "/** @private */", - "function foo_() {};"); - String expected = - Joiner.on('\n').join( - "const bar = goog.require('goog.bar');", - "const safe = goog.require('goog.safe');", - "", - "/** @private */", - "function foo_() {};"); - Compiler compiler = getCompiler(input); - Node root = compileToScriptRoot(compiler); - Match match = new Match(root.getFirstChild(), new NodeMetadata(compiler)); - SuggestedFix fix = new SuggestedFix.Builder().addGoogRequire(match, "goog.safe").build(); - assertChanges(fix, "", input, expected); - } - - @Test - public void testAddRequireModuleUnchanged() { - String input = - Joiner.on('\n').join( - "goog.module('js.Foo');", - "const safe = goog.require('goog.safe');", - "", - "/** @private */", - "function foo_() {};"); - Compiler compiler = getCompiler(input); - Node root = compileToScriptRoot(compiler); - Match match = new Match(root.getFirstChild(), new NodeMetadata(compiler)); - SuggestedFix fix = new SuggestedFix.Builder().addGoogRequire(match, "goog.safe").build(); - assertThat(fix.getReplacements()).isEmpty(); - } - - @Test - public void testAddRequireModuleDifferentNameUnchanged() { - String input = - Joiner.on('\n').join( - "goog.module('js.Foo');", - "const googSafe = goog.require('goog.safe');", - "", - "/** @private */", - "function foo_() {};"); - Compiler compiler = getCompiler(input); - Node root = compileToScriptRoot(compiler); - Match match = new Match(root.getFirstChild(), new NodeMetadata(compiler)); - SuggestedFix.Builder fixBuilder = new SuggestedFix.Builder().addGoogRequire(match, "goog.safe"); - assertThat(fixBuilder.getRequireName(match, "goog.safe")).isEqualTo("googSafe"); - assertThat(fixBuilder.build().getReplacements()).isEmpty(); - } - private static void assertAddGoogRequire(String before, String after, String namespace) { Compiler compiler = getCompiler(before + after); Node root = compileToScriptRoot(compiler); diff --git a/test/com/google/javascript/refactoring/examples/NavigationalXssSinksRefactoringTest.java b/test/com/google/javascript/refactoring/examples/NavigationalXssSinksRefactoringTest.java index 7de58dc1b11..608b841a2d0 100644 --- a/test/com/google/javascript/refactoring/examples/NavigationalXssSinksRefactoringTest.java +++ b/test/com/google/javascript/refactoring/examples/NavigationalXssSinksRefactoringTest.java @@ -18,6 +18,7 @@ import static com.google.javascript.refactoring.testing.RefasterJsTestUtils.assertFileRefactoring; import com.google.common.collect.ImmutableList; + import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -45,14 +46,4 @@ public void test_refactorings() throws Exception { ImmutableList.of("goog_base.js"), "navigational_xss_sinks_test_out.js"); } - - @Test - public void testModuleRefactoring() throws Exception { - assertFileRefactoring( - NAVIGATIONAL_XSS_SINKS_TEMPLATE, - TESTDATA_DIR, - "navigational_xss_sinks_test_module_in.js", - ImmutableList.of("goog_base.js", "goog_foo.js"), - "navigational_xss_sinks_test_module_out.js"); - } } diff --git a/test/com/google/javascript/refactoring/examples/testdata/goog_base.js b/test/com/google/javascript/refactoring/examples/testdata/goog_base.js index df313374f3e..76cff422a94 100644 --- a/test/com/google/javascript/refactoring/examples/testdata/goog_base.js +++ b/test/com/google/javascript/refactoring/examples/testdata/goog_base.js @@ -24,6 +24,3 @@ goog.provide = function(name) {}; /** @param {string} name */ goog.require = function(name) {}; - -/** @param {string} name */ -goog.module = function(name) {}; diff --git a/test/com/google/javascript/refactoring/examples/testdata/goog_foo.js b/test/com/google/javascript/refactoring/examples/testdata/goog_foo.js deleted file mode 100644 index 2e75a157ba1..00000000000 --- a/test/com/google/javascript/refactoring/examples/testdata/goog_foo.js +++ /dev/null @@ -1,22 +0,0 @@ -/* - * Copyright 2016 The Closure Compiler Authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -/** @fileoverview Mock namespace . */ - -goog.module('goog.foo'); - -/** Does nothing. */ -exports.bar = function() {}; diff --git a/test/com/google/javascript/refactoring/examples/testdata/navigational_xss_sinks_test_module_in.js b/test/com/google/javascript/refactoring/examples/testdata/navigational_xss_sinks_test_module_in.js deleted file mode 100644 index 409b12e820a..00000000000 --- a/test/com/google/javascript/refactoring/examples/testdata/navigational_xss_sinks_test_module_in.js +++ /dev/null @@ -1,33 +0,0 @@ -/* - * Copyright 2016 The Closure Compiler Authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -/** - * @fileoverview Test cases for the navigational_xss_sinks.js RefasterJs - * template. - */ - -goog.module('refactoring_testcase'); -const foo = goog.require('goog.foo'); - -/** - * @param {!Location} target The target. - * @param {string} val The value. - */ -exports.nonnull_location_href = function(target, val) { - foo.bar(); - // Should match before_setLocationHref. - target.href = val; -}; diff --git a/test/com/google/javascript/refactoring/examples/testdata/navigational_xss_sinks_test_module_out.js b/test/com/google/javascript/refactoring/examples/testdata/navigational_xss_sinks_test_module_out.js deleted file mode 100644 index b8d6543b762..00000000000 --- a/test/com/google/javascript/refactoring/examples/testdata/navigational_xss_sinks_test_module_out.js +++ /dev/null @@ -1,34 +0,0 @@ -/* - * Copyright 2016 The Closure Compiler Authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -/** - * @fileoverview Test cases for the navigational_xss_sinks.js RefasterJs - * template. - */ - -goog.module('refactoring_testcase'); -const foo = goog.require('goog.foo'); -const safe = goog.require('goog.dom.safe'); - -/** - * @param {!Location} target The target. - * @param {string} val The value. - */ -exports.nonnull_location_href = function(target, val) { - foo.bar(); - // Should match before_setLocationHref. - safe.setLocationHref(target, val); -};