Skip to content

Commit

Permalink
Migrate from goog.module.declareNamespace to goog.declareModuleId.
Browse files Browse the repository at this point in the history 
We're just renaming the function to make it clearer.
For now both functions are supported until goog.module.declareNamespace is no longer used.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=214629511
  • Loading branch information
@brad4d
johnplaisted authored and brad4d committed Sep 27, 2018
1 parent c74d06c commit bb05a1c
Show file tree
Hide file tree
Showing 14 changed files with 228 additions and 26 deletions.
4 changes: 3 additions & 1 deletion src/com/google/javascript/jscomp/CheckMissingAndExtraRequires.java
View file
Expand Up @@ -318,7 +318,9 @@ private boolean isMissingRequire(String namespace, Node node) {
// check for them explicitly.
|| namespace.equals("goog.module.get")
|| namespace.equals("goog.module.declareLegacyNamespace")
|| namespace.equals("goog.module.declareNamespace")) {
// TODO(johnplaisted): Consolidate on declareModuleId.
|| namespace.equals("goog.module.declareNamespace")
|| namespace.equals("goog.declareModuleId")) {
return false;
}

Expand Down
4 changes: 3 additions & 1 deletion src/com/google/javascript/jscomp/CompilerInput.java
View file
Expand Up @@ -401,7 +401,9 @@ void visitSubtree(Node n, Node parent) {
return;
}
} else if (parent.isGetProp()
&& parent.matchesQualifiedName("goog.module.declareNamespace")
// TODO(johnplaisted): Consolidate on declareModuleId
&& (parent.matchesQualifiedName("goog.module.declareNamespace")
|| parent.matchesQualifiedName("goog.declareModuleId"))
&& parent.getParent().isCall()) {
Node argument = parent.getParent().getSecondChild();
if (!argument.isString()) {
Expand Down
4 changes: 3 additions & 1 deletion src/com/google/javascript/jscomp/Es6RewriteModules.java
View file
Expand Up @@ -338,7 +338,9 @@ public void visit(NodeTraversal t, Node n, Node parent) {
scriptNodeCount++;
visitScript(t, n);
} else if (n.isCall()) {
if (n.getFirstChild().matchesQualifiedName("goog.module.declareNamespace")) {
// TODO(johnplaisted): Consolidate on declareModuleId.
if (n.getFirstChild().matchesQualifiedName("goog.module.declareNamespace")
|| n.getFirstChild().matchesQualifiedName("goog.declareModuleId")) {
n.getParent().detach();
}
}
Expand Down
34 changes: 19 additions & 15 deletions src/com/google/javascript/jscomp/GatherModuleMetadata.java
View file
Expand Up @@ -41,20 +41,20 @@ public final class GatherModuleMetadata implements CompilerPass {
static final DiagnosticType MIXED_MODULE_TYPE =
DiagnosticType.error("JSC_MIXED_MODULE_TYPE", "A file cannot be both {0} and {1}.");

static final DiagnosticType INVALID_DECLARE_NAMESPACE_CALL =
static final DiagnosticType INVALID_DECLARE_MODULE_ID_CALL =
DiagnosticType.error(
"JSC_INVALID_DECLARE_NAMESPACE_CALL",
"goog.module.declareNamespace parameter must be a string literal.");
"goog.declareModuleId parameter must be a string literal.");

static final DiagnosticType DECLARE_MODULE_NAMESPACE_OUTSIDE_ES6_MODULE =
static final DiagnosticType DECLARE_MODULE_ID_OUTSIDE_ES6_MODULE =
DiagnosticType.error(
"JSC_DECLARE_MODULE_NAMESPACE_OUTSIDE_ES6_MODULE",
"goog.module.declareNamespace can only be called within ES6 modules.");
"goog.declareModuleId can only be called within ES6 modules.");

static final DiagnosticType MULTIPLE_DECLARE_MODULE_NAMESPACE =
DiagnosticType.error(
"JSC_MULTIPLE_DECLARE_MODULE_NAMESPACE",
"goog.module.declareNamespace can only be called once per ES6 module.");
"goog.declareModuleId can only be called once per ES6 module.");

static final DiagnosticType INVALID_REQUIRE_TYPE =
DiagnosticType.error(
Expand All @@ -74,6 +74,10 @@ public final class GatherModuleMetadata implements CompilerPass {
IR.getprop(IR.name("goog"), IR.string("setTestOnly"));
private static final Node GOOG_MODULE_DECLARELEGACYNAMESPACE =
IR.getprop(GOOG_MODULE.cloneTree(), IR.string("declareLegacyNamespace"));
private static final Node GOOG_DECLARE_MODULE_ID =
IR.getprop(IR.name("goog"), IR.string("declareModuleId"));

// TODO(johnplaisted): Remove once clients have migrated to declareModuleId
private static final Node GOOG_MODULE_DECLARNAMESPACE =
IR.getprop(GOOG_MODULE.cloneTree(), IR.string("declareNamespace"));

Expand Down Expand Up @@ -117,7 +121,7 @@ public GatherModuleMetadata(

private class ModuleMetadataBuilder {
private boolean ambiguous;
private Node declaresNamespace;
private Node declaredModuleId;
private Node declaresLegacyNamespace;
private final Node rootNode;
final ModuleMetadata.Builder metadataBuilder;
Expand Down Expand Up @@ -145,8 +149,8 @@ void moduleType(ModuleType type, NodeTraversal t, Node n) {
t.report(n, MIXED_MODULE_TYPE, metadataBuilder.moduleType().description, type.description);
}

void recordDeclareNamespace(Node declaresNamespace) {
this.declaresNamespace = declaresNamespace;
void recordDeclareModuleId(Node declaredModuleId) {
this.declaredModuleId = declaredModuleId;
}

void recordDeclareLegacyNamespace(Node declaresLegacyNamespace) {
Expand All @@ -160,9 +164,8 @@ boolean isScript() {
ModuleMetadata build() {
metadataBuilder.googNamespacesBuilder().addAll(googNamespaces);
if (!ambiguous) {
if (declaresNamespace != null && metadataBuilder.moduleType() != ModuleType.ES6_MODULE) {
compiler.report(
JSError.make(declaresNamespace, DECLARE_MODULE_NAMESPACE_OUTSIDE_ES6_MODULE));
if (declaredModuleId != null && metadataBuilder.moduleType() != ModuleType.ES6_MODULE) {
compiler.report(JSError.make(declaredModuleId, DECLARE_MODULE_ID_OUTSIDE_ES6_MODULE));
}

if (declaresLegacyNamespace != null) {
Expand Down Expand Up @@ -340,16 +343,17 @@ private void visitGoogCall(NodeTraversal t, Node n) {
}
} else if (getprop.matchesQualifiedName(GOOG_MODULE_DECLARELEGACYNAMESPACE)) {
currentModule.recordDeclareLegacyNamespace(n);
} else if (getprop.matchesQualifiedName(GOOG_MODULE_DECLARNAMESPACE)) {
if (currentModule.declaresNamespace != null) {
} else if (getprop.matchesQualifiedName(GOOG_DECLARE_MODULE_ID)
|| getprop.matchesQualifiedName(GOOG_MODULE_DECLARNAMESPACE)) {
if (currentModule.declaredModuleId != null) {
t.report(n, MULTIPLE_DECLARE_MODULE_NAMESPACE);
}
if (n.hasTwoChildren() && n.getLastChild().isString()) {
currentModule.recordDeclareNamespace(n);
currentModule.recordDeclareModuleId(n);
String namespace = n.getLastChild().getString();
addNamespace(currentModule, namespace, t, n);
} else {
t.report(n, INVALID_DECLARE_NAMESPACE_CALL);
t.report(n, INVALID_DECLARE_MODULE_ID_CALL);
}
} else if (getprop.matchesQualifiedName(GOOG_REQUIRE)) {
if (n.hasTwoChildren() && n.getLastChild().isString()) {
Expand Down
2 changes: 1 addition & 1 deletion src/com/google/javascript/jscomp/ModuleMetadataMap.java
View file
Expand Up @@ -118,7 +118,7 @@ public boolean isScript() {

/**
* Closure namespaces that this file is associated with. Created by goog.provide, goog.module,
* and goog.module.declareNamespace.
* and goog.declareModuleId.
*/
public abstract ImmutableMultiset<String> googNamespaces();

Expand Down
9 changes: 6 additions & 3 deletions src/com/google/javascript/jscomp/deps/JsFileParser.java
View file
Expand Up @@ -52,7 +52,8 @@ public final class JsFileParser extends JsFileLineParser {
// but fails to match without "use strict"; since we look for semicolon, not open brace.
Pattern.compile(
"(?:^|;)(?:[a-zA-Z0-9$_,:{}\\s]+=)?\\s*"
+ "goog\\.(?<func>provide|module|require|requireType|addDependency)"
+ "goog\\.(?<func>provide|module|require|requireType|addDependency|declareModuleId)"
// TODO(johnplaisted): Remove declareNamespace.
+ "(?<subfunc>\\.declareNamespace)?\\s*\\((?<args>.*?)\\)");

/**
Expand Down Expand Up @@ -260,7 +261,8 @@ protected boolean parseLine(String line) throws ParseException {
if (line.contains("provide")
|| line.contains("require")
|| line.contains("module")
|| line.contains("addDependency")) {
|| line.contains("addDependency")
|| line.contains("declareModuleId")) {
// Iterate over the provides/requires.
googMatcher.reset(line);
while (googMatcher.find()) {
Expand All @@ -274,7 +276,8 @@ protected boolean parseLine(String line) throws ParseException {
// See if it's a require or provide.
String methodName = googMatcher.group("func");
char firstChar = methodName.charAt(0);
boolean isDeclareModuleNamespace = firstChar == 'm' && googMatcher.group("subfunc") != null;
boolean isDeclareModuleNamespace =
firstChar == 'd' || (firstChar == 'm' && googMatcher.group("subfunc") != null);
boolean isModule = !isDeclareModuleNamespace && firstChar == 'm';
boolean isProvide = firstChar == 'p';
boolean providesNamespace = isProvide || isModule || isDeclareModuleNamespace;
Expand Down
2 changes: 2 additions & 0 deletions src/com/google/javascript/jscomp/ijs/ConvertToTypedInterface.java
View file
Expand Up @@ -75,6 +75,8 @@ public class ConvertToTypedInterface implements CompilerPass {
"goog.forwardDeclare",
"goog.module",
"goog.module.declareLegacyNamespace",
// TODO(johnplaisted): Consolidate on declareModuleId / delete declareNamespace.
"goog.declareModuleId",
"goog.module.declareNamespace",
"goog.provide",
"goog.require");
Expand Down
8 changes: 5 additions & 3 deletions src/com/google/javascript/jscomp/lint/CheckEs6ModuleFileStructure.java
View file
Expand Up @@ -37,7 +37,7 @@ public final class CheckEs6ModuleFileStructure extends AbstractPreOrderCallback
/** Statements that must appear in a certain order within ES6 modules (for the sake of style). */
private enum OrderedStatement {
IMPORT("import statements"),
DECLARE_NAMESPACE("a call to goog.module.declareNamespace()"),
DECLARE_MODULE_ID("a call to goog.declareModuleId()"),
GOOG_REQUIRE("calls to goog.require()"),
OTHER("other statements");

Expand Down Expand Up @@ -98,8 +98,10 @@ private void checkOrder(NodeTraversal t, Node n, OrderedStatement statement) {
private boolean visitExprResult(NodeTraversal t, Node exprResult, Node parent) {
if (parent.isModuleBody() && exprResult.getFirstChild().isCall()) {
Node call = exprResult.getFirstChild();
if (call.getFirstChild().matchesQualifiedName("goog.module.declareNamespace")) {
checkOrder(t, call, OrderedStatement.DECLARE_NAMESPACE);
// TODO(johnplaisted): Remove declareNamespace.
if (call.getFirstChild().matchesQualifiedName("goog.module.declareNamespace")
|| call.getFirstChild().matchesQualifiedName("goog.declareModuleId")) {
checkOrder(t, call, OrderedStatement.DECLARE_MODULE_ID);
return false;
} else if (call.getFirstChild().matchesQualifiedName("goog.require")) {
checkOrder(t, call, OrderedStatement.GOOG_REQUIRE);
Expand Down
96 changes: 96 additions & 0 deletions test/com/google/javascript/jscomp/Es6RewriteModulesWithGoogInteropTest.java
View file
Expand Up @@ -206,6 +206,20 @@ public void testGoogModuleGetForLegacyGoogModule() {

@Test
public void testGoogModuleGetForEs6Module() {
test(
srcs(
SourceFile.fromCode("es6.js", "export{}; goog.declareModuleId('es6');"),
SourceFile.fromCode(
"closure.js",
"goog.module('my.module'); function f() { const y = goog.module.get('es6'); }")),
expected(
SourceFile.fromCode("es6.js", "/** @const */ var module$es6 = {};"),
SourceFile.fromCode(
"closure.js", "goog.module('my.module'); function f() { const y = module$es6; }")));
}

@Test
public void testGoogModuleGetForEs6ModuleDeclaresNamespace() {
test(
srcs(
SourceFile.fromCode("es6.js", "export{}; goog.module.declareNamespace('es6');"),
Expand All @@ -218,6 +232,21 @@ public void testGoogModuleGetForEs6Module() {
"closure.js", "goog.module('my.module'); function f() { const y = module$es6; }")));
}

@Test
public void testDeclareModuleId() {
test(
srcs(
SourceFile.fromCode("es6.js", "export var x; goog.declareModuleId('my.es6');"),
SourceFile.fromCode("goog.js", "const es6 = goog.require('my.es6'); use(es6, es6.x);")),
expected(
SourceFile.fromCode(
"es6.js",
lines(
"var x$$module$es6;/** @const */ var module$es6={};",
"/** @const */ module$es6.x=x$$module$es6;")),
SourceFile.fromCode("goog.js", "const es6 = module$es6; use(es6, es6.x);")));
}

@Test
public void testDeclareNamespace() {
test(
Expand All @@ -233,6 +262,25 @@ public void testDeclareNamespace() {
SourceFile.fromCode("goog.js", "const es6 = module$es6; use(es6, es6.x);")));
}

@Test
public void testDestructureDeclareModuleId() {
test(
srcs(
SourceFile.fromCode("es6.js", "export var x, z; goog.declareModuleId('my.es6');"),
SourceFile.fromCode("goog.js", "const {x, z: y} = goog.require('my.es6'); use(x, y);")),
expected(
SourceFile.fromCode(
"es6.js",
lines(
"var x$$module$es6, z$$module$es6;",
"/** @const */ var module$es6={};",
"/** @const */ module$es6.x=x$$module$es6;",
"/** @const */ module$es6.z=z$$module$es6;")),
SourceFile.fromCode(
"goog.js",
lines("const x = module$es6.x;", "const y = module$es6.z;", "use(x, y);"))));
}

@Test
public void testDestructureDeclareNamespace() {
test(
Expand Down Expand Up @@ -382,7 +430,55 @@ public void testCorrectTypeNodeIsRenamed() {
"/** @const */ module$testcode.value = value$$module$testcode;"));
}

@Test
public void testForwardDeclareEs6Module() {
test(
srcs(
SourceFile.fromCode("es6.js", "export{}; goog.declareModuleId('es6');"),
SourceFile.fromCode(
"closure.js",
lines(
"goog.module('my.module');",
"const alias = goog.forwardDeclare('es6');",
"let /** !alias.Type */ x;",
"alias;"))),
expected(
SourceFile.fromCode("es6.js", "/** @const */ var module$es6 = {};"),
SourceFile.fromCode(
"closure.js",
lines("goog.module('my.module');", "let /** !module$es6.Type */ x;", "alias;"))));

test(
srcs(
SourceFile.fromCode("es6.js", "export{}; goog.declareModuleId('es6');"),
SourceFile.fromCode(
"closure.js",
lines(
"goog.module('my.module');",
"goog.forwardDeclare('es6');",
"let /** !es6.Type */ x;",
"es6;"))),
expected(
SourceFile.fromCode("es6.js", "/** @const */ var module$es6 = {};"),
SourceFile.fromCode(
"closure.js",
lines("goog.module('my.module');", "let /** !module$es6.Type */ x;", "es6;"))));

testError(
ImmutableList.of(
SourceFile.fromCode("es6.js", "export{}; goog.declareModuleId('es6');"),
SourceFile.fromCode(
"closure.js",
lines(
"goog.module('my.module');",
"let alias = goog.forwardDeclare('es6');",
"let /** !alias.Type */ x;",
"alias = goog.modle.get('es6');"))),
FORWARD_DECLARE_IN_ES6_SHOULD_BE_CONST);
}

@Test
public void testForwardDeclareEs6ModuleDeclareNamespace() {
test(
srcs(
SourceFile.fromCode("es6.js", "export{}; goog.module.declareNamespace('es6');"),
Expand Down
10 changes: 10 additions & 0 deletions test/com/google/javascript/jscomp/GatherModuleMetadataTest.java
View file
Expand Up @@ -155,6 +155,16 @@ public void testEs6Module() {
assertThat(m.isEs6Module()).isTrue();
}

@Test
public void testEs6ModuleDeclareModuleId() {
testSame("export var x; goog.declareModuleId('my.module');");
assertThat(metadataMap().getModulesByGoogNamespace().keySet()).containsExactly("my.module");
ModuleMetadata m = metadataMap().getModulesByGoogNamespace().get("my.module");
assertThat(m.googNamespaces()).containsExactly("my.module");
assertThat(m.isEs6Module()).isTrue();
assertThat(m.isGoogModule()).isFalse();
}

@Test
public void testEs6ModuleDeclareNamespace() {
testSame("export var x; goog.module.declareNamespace('my.module');");
Expand Down
44 changes: 44 additions & 0 deletions test/com/google/javascript/jscomp/deps/DepsGeneratorTest.java
View file
Expand Up @@ -126,6 +126,50 @@ public void testEs6Modules() throws Exception {
assertEquals(expected, output);
}

@Test
public void testEs6ModuleDeclareModuleId() throws Exception {
List<SourceFile> srcs = new ArrayList<>();
srcs.add(
SourceFile.fromCode(
"/base/javascript/foo/foo.js", "goog.declareModuleId('my.namespace');\nexport {};"));
srcs.add(
SourceFile.fromCode(
"/base/javascript/closure/goog/googmodule.js",
LINE_JOINER.join(
"goog.module('my.goog.module');",
"const namespace = goog.require('my.namespace');")));
DepsGenerator depsGenerator =
new DepsGenerator(
ImmutableList.of(),
srcs,
DepsGenerator.InclusionStrategy.ALWAYS,
"/base/javascript/closure",
errorManager,
new ModuleLoader(
null,
ImmutableList.of("/base/"),
ImmutableList.of(),
BrowserModuleResolver.FACTORY,
ModuleLoader.PathResolver.ABSOLUTE));
String output = depsGenerator.computeDependencyCalls();

assertNoWarnings();

// Write the output.
assertWithMessage("There should be output").that(output).isNotEmpty();

// Write the expected output.
String expected =
LINE_JOINER.join(
"goog.addDependency('../foo/foo.js', ['my.namespace'], "
+ "[], {'lang': 'es6', 'module': 'es6'});",
"goog.addDependency('goog/googmodule.js', ['my.goog.module'], ['my.namespace'], "
+ "{'lang': 'es6', 'module': 'goog'});",
"");

assertEquals(expected, output);
}

@Test
public void testEs6ModuleDeclareNamespace() throws Exception {
List<SourceFile> srcs = new ArrayList<>();
Expand Down

0 comments on commit bb05a1c

Please sign in to comment.