diff --git a/src/main/java/com/google/javascript/clutz/AliasMapBuilder.java b/src/main/java/com/google/javascript/clutz/AliasMapBuilder.java index 139a0cae..b3105084 100644 --- a/src/main/java/com/google/javascript/clutz/AliasMapBuilder.java +++ b/src/main/java/com/google/javascript/clutz/AliasMapBuilder.java @@ -1,114 +1,112 @@ -package com.google.javascript.clutz; - -import com.google.javascript.rhino.Node; -import java.util.HashMap; -import java.util.Map; -import java.util.Set; - -/** - * Closure doesn't handle alias reexports like the following well: - * - *

`goog.module('bare.reexport'); const {Class} = goog.require('original.module'); exports = - * Class;` - * - *

It completely ignores the goog.require() and gives Class the unknown {?} type. - * - *

AliasMapBuilder looks for goog.require statements and generates mappings from the exported - * name of Class (here module$exports$bare$reexport) to the name of the imported symbol (here - * module$exports$original$module.Class), so if clutz finds a symbol that has no type, it can emit - * it as an alias to the correct symbol. - */ -public class AliasMapBuilder extends ImportBasedMapBuilder { - @Override - protected Map build( - String localModuleId, Node moduleBody, Set googProvides) { - Map aliasMap = new HashMap<>(); - if (localModuleId == null) { - //TODO(lucassloan): handle goog.module.get() - return aliasMap; - } - - // Loop over the statements, looking for import statements, and build a map from the local variable - // names to the original symbol name eg `const C = goog.require('a.b.c');` will result in the map - // containing 'C' -> 'module$exports$a$b$c' - Map localVariableToImportedSymbolNameMap = new HashMap<>(); - for (Node statement : moduleBody.children()) { - if (isImportAssignment(statement)) { - // `const C = goog.require()` or - // `const C = goog.module.get()` - String importedModuleId = statement.getFirstFirstChild().getChildAtIndex(1).getString(); - String localVariableName = statement.getFirstChild().getString(); - - String importedSymbolName = buildWholeModuleExportSymbolName(importedModuleId); - localVariableToImportedSymbolNameMap.put(localVariableName, importedSymbolName); - } else if (isImportDestructuringAssignment(statement)) { - // `const {C, Clazz: RenamedClazz} = goog.require()` or - // `const {C, Clazz: RenamedClazz} = goog.module.get()` - String importedModuleId = - statement.getFirstChild().getChildAtIndex(1).getChildAtIndex(1).getString(); - for (Node destructured : statement.getFirstFirstChild().children()) { - String originalName = destructured.getString(); - // Destructuring can use the original name `const {A} = goog.require("foo.a")` or rename - // it `const {A: RenamedA} = ...`, and closure uses whichever in the symbol name it - // generates, so we have to extract it. - String localVariableName; - if (destructured.getFirstChild() != null) { - // Renaming - localVariableName = destructured.getFirstChild().getString(); - } else { - // No rename - localVariableName = originalName; - } - - String importedSymbolName = buildNamedExportSymbolName(importedModuleId, originalName); - localVariableToImportedSymbolNameMap.put(localVariableName, importedSymbolName); - } - } - } - - // Loop over the statements, looking for export statements, and add mappings to the alias map - // if the export is of a variable that was imported - for (Node statement : moduleBody.children()) { - if (isWholeModuleExportAssignment(statement)) { - // `exports = foo` - String localVariableName = getExportsAssignmentRHS(statement); - - if (localVariableToImportedSymbolNameMap.containsKey(localVariableName)) { - aliasMap.put( - buildWholeModuleExportSymbolName(localModuleId), - localVariableToImportedSymbolNameMap.get(localVariableName)); - } - } else if (isNamedExportAssignment(statement)) { - // `exports.foo = foo;` - String localVariableName = getExportsAssignmentRHS(statement); - String exportName = getNamedExportName(statement); - - if (localVariableToImportedSymbolNameMap.containsKey(localVariableName)) { - aliasMap.put( - buildNamedExportSymbolName(localModuleId, exportName), - localVariableToImportedSymbolNameMap.get(localVariableName)); - } - } else if (isNamedExportPropAssignment(statement)) { - // `exports.foo = foo.bar;` - String localVariableName = getExportsAssignmentPropRootName(statement); - if (localVariableName.equals("exports")) { - // This is a "local" alias between two exports from the same module. - // There is no need to for clutz to special handle this, as the JS - // Compiler will resolve this properly. - continue; - } - String localPropName = getExportsAssignmentPropName(statement); - String exportName = getNamedExportName(statement); - String localNamespaceName = - localVariableToImportedSymbolNameMap.containsKey(localVariableName) - ? localVariableToImportedSymbolNameMap.get(localVariableName) - : localVariableName; - aliasMap.put( - buildNamedExportSymbolName(localModuleId, exportName), - localNamespaceName + "." + localPropName); - } - } - - return aliasMap; - } -} +package com.google.javascript.clutz; + +import com.google.javascript.rhino.Node; +import java.util.HashMap; +import java.util.Map; +import java.util.Set; + +/** + * Closure doesn't handle alias reexports like the following well: + * + *

`goog.module('bare.reexport'); const {Class} = goog.require('original.module'); exports = + * Class;` + * + *

It completely ignores the goog.require() and gives Class the unknown {?} type. + * + *

AliasMapBuilder looks for goog.require statements and generates mappings from the exported + * name of Class (here module$exports$bare$reexport) to the name of the imported symbol (here + * module$exports$original$module.Class), so if clutz finds a symbol that has no type, it can emit + * it as an alias to the correct symbol. + */ +public class AliasMapBuilder extends ImportBasedMapBuilder { + @Override + protected Map build( + String localModuleId, Node moduleBody, Set googProvides) { + Map aliasMap = new HashMap<>(); + if (localModuleId == null) { + // TODO(lucassloan): handle goog.module.get() + return aliasMap; + } + + // Loop over the statements, looking for import statements, and build a map from the local variable + // names to the original symbol name eg `const C = goog.require('a.b.c');` will result in the map + // containing 'C' -> 'module$exports$a$b$c' + Map localVariableToImportedSymbolNameMap = new HashMap<>(); + for (Node statement : moduleBody.children()) { + if (isImportAssignment(statement)) { + // `const C = goog.require()` or + // `const C = goog.module.get()` + String importedModuleId = statement.getFirstFirstChild().getSecondChild().getString(); + String localVariableName = statement.getFirstChild().getString(); + + String importedSymbolName = buildWholeModuleExportSymbolName(importedModuleId); + localVariableToImportedSymbolNameMap.put(localVariableName, importedSymbolName); + } else if (isImportDestructuringAssignment(statement)) { + // `const {C, Clazz: RenamedClazz} = goog.require()` or + // `const {C, Clazz: RenamedClazz} = goog.module.get()` + String importedModuleId = + statement.getFirstChild().getChildAtIndex(1).getSecondChild().getString(); + for (Node destructured : statement.getFirstFirstChild().children()) { + String originalName = destructured.getString(); + // Destructuring can use the original name `const {A} = goog.require("foo.a")` or rename + // it `const {A: RenamedA} = ...`, and closure uses whichever in the symbol name it + // generates, so we have to extract it. + String localVariableName; + if (destructured.hasChildren()) { + // Renaming + localVariableName = destructured.getFirstChild().getString(); + } else { + // No rename + localVariableName = originalName; + } + + String importedSymbolName = buildNamedExportSymbolName(importedModuleId, originalName); + localVariableToImportedSymbolNameMap.put(localVariableName, importedSymbolName); + } + } + } + + // Loop over the statements, looking for export statements, and add mappings to the alias map + // if the export is of a variable that was imported + for (Node statement : moduleBody.children()) { + if (isWholeModuleExportAssignment(statement)) { + // `exports = foo` + String localVariableName = getExportsAssignmentRHS(statement); + + if (localVariableToImportedSymbolNameMap.containsKey(localVariableName)) { + aliasMap.put( + buildWholeModuleExportSymbolName(localModuleId), + localVariableToImportedSymbolNameMap.get(localVariableName)); + } + } else if (isNamedExportAssignment(statement)) { + // `exports.foo = foo;` + String localVariableName = getExportsAssignmentRHS(statement); + String exportName = getNamedExportName(statement); + + if (localVariableToImportedSymbolNameMap.containsKey(localVariableName)) { + aliasMap.put( + buildNamedExportSymbolName(localModuleId, exportName), + localVariableToImportedSymbolNameMap.get(localVariableName)); + } + } else if (isNamedExportPropAssignment(statement)) { + // `exports.foo = foo.bar;` + String localVariableName = getExportsAssignmentPropRootName(statement); + if (localVariableName.equals("exports")) { + // This is a "local" alias between two exports from the same module. + // There is no need to for clutz to special handle this, as the JS + // Compiler will resolve this properly. + continue; + } + String localPropName = getExportsAssignmentPropName(statement); + String exportName = getNamedExportName(statement); + String localNamespaceName = + localVariableToImportedSymbolNameMap.getOrDefault(localVariableName, localVariableName); + aliasMap.put( + buildNamedExportSymbolName(localModuleId, exportName), + localNamespaceName + "." + localPropName); + } + } + + return aliasMap; + } +} diff --git a/src/main/java/com/google/javascript/clutz/DeclarationGenerator.java b/src/main/java/com/google/javascript/clutz/DeclarationGenerator.java index 436d90e2..93ce69c3 100644 --- a/src/main/java/com/google/javascript/clutz/DeclarationGenerator.java +++ b/src/main/java/com/google/javascript/clutz/DeclarationGenerator.java @@ -3,13 +3,13 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; +import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.collect.Iterables.transform; import static com.google.javascript.rhino.jstype.JSTypeNative.ALL_TYPE; import static com.google.javascript.rhino.jstype.JSTypeNative.ARRAY_TYPE; import static com.google.javascript.rhino.jstype.JSTypeNative.OBJECT_TYPE; import static com.google.javascript.rhino.jstype.JSTypeNative.STRING_TYPE; import static java.nio.charset.StandardCharsets.UTF_8; -import static com.google.common.collect.ImmutableList.toImmutableList; import static org.apache.commons.text.StringEscapeUtils.escapeEcmaScript; import com.google.common.base.CharMatcher; @@ -358,8 +358,7 @@ void generateDeclarations() { continue; } - getJsEntryPathsFromZip(source) - .stream() + getJsEntryPathsFromZip(source).stream() .map(p -> SourceFile.fromPath(p, UTF_8)) .forEach(sourceFiles::add); } @@ -392,8 +391,7 @@ void generateDeclarations() { */ static List getJsEntryPathsFromZip(String source) { try (ZipFile zipFile = new ZipFile(source)) { - return zipFile - .stream() + return zipFile.stream() .filter(e -> !e.isDirectory()) .filter(e -> e.getName().endsWith(".js")) .map(e -> source + "!/" + e.getName()) @@ -1022,13 +1020,6 @@ private String normalizeWindowGlobals(String name) { return name.replaceAll("^(window|this)\\.", ""); } - /** See the comment above on shouldAvoidGeneratingExterns. */ - private boolean shouldAvoidGeneratingExterns(ObjectType type) { - if (type.getConstructor() == null || type.getConstructor().getSource() == null) return false; - return shouldAvoidGeneratingExterns( - type.getConstructor().getSource().getSourceFileName(), type.getDisplayName()); - } - private void declareNamespace( String namespace, TypedVar symbol, @@ -1776,8 +1767,7 @@ private void visitClassOrInterface(String name, FunctionType ftype) { ObjectType superType = getSuperType(ftype); if (superType != null) { emit("extends"); - boolean emitInstanceForObject = !shouldAvoidGeneratingExterns(superType); - Visitor visitor = new ExtendsImplementsTypeVisitor(emitInstanceForObject); + Visitor visitor = new ExtendsImplementsTypeVisitor(); superType.visit(visitor); } @@ -1797,7 +1787,7 @@ private void emitCommaSeparatedInterfaces(Iterator it) { // TypeScript does not allow public APIs that expose non-exported/private types. emit(getGlobalSymbolNamespacePrefix() + "PrivateInterface"); } else { - ExtendsImplementsTypeVisitor visitor = new ExtendsImplementsTypeVisitor(false); + ExtendsImplementsTypeVisitor visitor = new ExtendsImplementsTypeVisitor(); type.visit(visitor); } if (it.hasNext()) { @@ -2225,7 +2215,7 @@ public Void caseSymbolType() { @Override public Void caseObjectType(ObjectType type) { - return emitObjectType(type, false, false); + return emitObjectType(type, false); } @Override @@ -2775,7 +2765,8 @@ private void maybeEmitSymbolIterator(JSType instanceType) { // Unfortunately, this method of detecting whether a class implements Iterator, is error-prone // in cases where the extension goes through another extends clause. Consider, C extends D, // and D implements Iterable. - // In such cases C doesn't need to emit anything new, because D's emit would have handled that. + // In such cases C doesn't need to emit anything new, because D's emit would have handled + // that. // Here we detect one such case - D being the always-present Array type and skip unnecessary // custom emit. if (instanceType.isSubtype(arrayType)) { @@ -3155,10 +3146,7 @@ private List getTemplateTypeNames(ObjectType objType) { if (objType.getTemplateTypeMap() == null) { return Collections.emptyList(); } - return objType - .getTemplateTypeMap() - .getTemplateKeys() - .stream() + return objType.getTemplateTypeMap().getTemplateKeys().stream() .map(jsType -> jsType != null ? jsType.getDisplayName() : "") .collect(toImmutableList()); } @@ -3463,8 +3451,7 @@ public void emitPrivateValue(String emitName) { emitBreak(); } - public Void emitObjectType( - ObjectType type, boolean extendingInstanceClass, boolean inExtendsImplementsPosition) { + public Void emitObjectType(ObjectType type, boolean inExtendsImplementsPosition) { // Closure doesn't require that all the type params be declared, but TS does if (!type.getTemplateTypeMap().isEmpty() && !typeRegistry.getNativeType(OBJECT_TYPE).equals(type)) { @@ -3526,15 +3513,9 @@ private String maybeRenameGlobalType(String name) { * any' is invalid, even though () => any is a valid type. */ class ExtendsImplementsTypeVisitor implements Visitor { - final boolean emitInstanceForObject; - - ExtendsImplementsTypeVisitor(boolean emitInstanceForObject) { - this.emitInstanceForObject = emitInstanceForObject; - } - @Override public Void caseObjectType(ObjectType type) { - emitObjectType(type, emitInstanceForObject, true); + emitObjectType(type, true); return null; } diff --git a/src/main/java/com/google/javascript/gents/Options.java b/src/main/java/com/google/javascript/gents/Options.java index ede47f1d..6007bc94 100644 --- a/src/main/java/com/google/javascript/gents/Options.java +++ b/src/main/java/com/google/javascript/gents/Options.java @@ -1,7 +1,5 @@ package com.google.javascript.gents; -import static com.google.common.base.Charsets.UTF_8; - import com.google.common.collect.ImmutableMap; import com.google.gson.Gson; import com.google.gson.reflect.TypeToken; @@ -13,6 +11,7 @@ import com.google.javascript.jscomp.parsing.Config; import java.io.IOException; import java.lang.reflect.Type; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Paths; import java.util.ArrayList; @@ -158,7 +157,8 @@ private Map getExternsMap() throws IOException { /* empty */ }.getType(); try (JsonReader reader = - new JsonReader(Files.newBufferedReader(Paths.get(externsMapFile), UTF_8))) { + new JsonReader( + Files.newBufferedReader(Paths.get(externsMapFile), StandardCharsets.UTF_8))) { return new Gson().fromJson(reader, mapType); } } else { @@ -183,7 +183,7 @@ private Map getExternsMap() throws IOException { if (sourcesManifest != null) { try { - filesToConvert = Files.readAllLines(Paths.get(sourcesManifest), UTF_8); + filesToConvert = Files.readAllLines(Paths.get(sourcesManifest), StandardCharsets.UTF_8); } catch (IOException e) { throw new CmdLineException( parser, "sources manifest file " + sourcesManifest + " not found.", e); @@ -194,7 +194,8 @@ private Map getExternsMap() throws IOException { srcFiles.addAll(arguments); } else { try { - srcFiles.addAll(Files.readAllLines(Paths.get(dependenciesManifest), UTF_8)); + srcFiles.addAll( + Files.readAllLines(Paths.get(dependenciesManifest), StandardCharsets.UTF_8)); } catch (IOException e) { throw new CmdLineException( parser, "dependencies manifest file " + dependenciesManifest + " not found.", e); diff --git a/src/test/java/com/google/javascript/clutz/DeclarationGeneratorTest.java b/src/test/java/com/google/javascript/clutz/DeclarationGeneratorTest.java index 4ed05973..98c2cf63 100644 --- a/src/test/java/com/google/javascript/clutz/DeclarationGeneratorTest.java +++ b/src/test/java/com/google/javascript/clutz/DeclarationGeneratorTest.java @@ -2,12 +2,12 @@ import static com.google.javascript.clutz.ProgramSubject.assertThatProgram; -import com.google.common.base.Charsets; import com.google.common.collect.Lists; import com.google.common.io.Files; import java.io.File; import java.io.FilenameFilter; import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.nio.file.FileSystems; import java.nio.file.Path; import java.util.Arrays; @@ -149,10 +149,10 @@ private static Path getTestDataFolderPath() { } static String getTestFileText(final File input) throws IOException { - String text = Files.asCharSource(input, Charsets.UTF_8).read(); + String text = Files.asCharSource(input, StandardCharsets.UTF_8).read(); if (input.getName().endsWith("_with_platform.d.ts")) { File platformGolden = getTestDataFolderPath().resolve("general_with_platform.d.ts").toFile(); - String platformGoldenText = Files.asCharSource(platformGolden, Charsets.UTF_8).read(); + String platformGoldenText = Files.asCharSource(platformGolden, StandardCharsets.UTF_8).read(); if (text.contains(PLATFORM_MARKER)) { text = text.replace(PLATFORM_MARKER, platformGoldenText); } else { diff --git a/src/test/java/com/google/javascript/clutz/DeclarationSyntaxTest.java b/src/test/java/com/google/javascript/clutz/DeclarationSyntaxTest.java index c0412134..59bb7bc8 100644 --- a/src/test/java/com/google/javascript/clutz/DeclarationSyntaxTest.java +++ b/src/test/java/com/google/javascript/clutz/DeclarationSyntaxTest.java @@ -4,7 +4,6 @@ import static com.google.javascript.clutz.DeclarationGeneratorTest.TS_SOURCES; import static org.junit.Assert.fail; -import com.google.common.base.Charsets; import com.google.common.base.Joiner; import com.google.common.collect.ImmutableList; import com.google.common.collect.Lists; @@ -12,6 +11,7 @@ import java.io.File; import java.io.FilenameFilter; import java.io.InputStreamReader; +import java.nio.charset.StandardCharsets; import java.nio.file.FileSystems; import java.nio.file.Path; import java.util.ArrayList; @@ -34,10 +34,6 @@ public class DeclarationSyntaxTest { (File dir, String name) -> JS_NO_EXTERNS.accept(dir, name) && dir.getName().equals("partialCrossModuleTypeImports"); - private static final FilenameFilter JS_ALIASED_INTERFACE = - (File dir, String name) -> - JS_NO_EXTERNS.accept(dir, name) && dir.getName().equals("aliasedInterface"); - public static final Path TSC = FileSystems.getDefault().getPath("node_modules", "typescript", "bin", "tsc"); @@ -130,7 +126,7 @@ public static void runChecked(final List command) throws Exception { tsc.destroyForcibly(); } if (tsc.exitValue() != 0) { - InputStreamReader isr = new InputStreamReader(tsc.getInputStream(), Charsets.UTF_8); + InputStreamReader isr = new InputStreamReader(tsc.getInputStream(), StandardCharsets.UTF_8); String consoleOut = CharStreams.toString(isr); fail("tsc exited abnormally with code " + tsc.exitValue() + "\n" + consoleOut); } diff --git a/src/test/java/com/google/javascript/clutz/ProgramSubject.java b/src/test/java/com/google/javascript/clutz/ProgramSubject.java index 11c365c9..c5565ff9 100644 --- a/src/test/java/com/google/javascript/clutz/ProgramSubject.java +++ b/src/test/java/com/google/javascript/clutz/ProgramSubject.java @@ -5,7 +5,6 @@ import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.Collections.singletonList; -import com.google.common.base.Charsets; import com.google.common.base.Joiner; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Lists; @@ -18,6 +17,7 @@ import java.io.File; import java.io.IOException; import java.io.PrintStream; +import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -108,7 +108,7 @@ void generatesDeclarations(File golden) throws IOException { if (System.getenv("UPDATE_GOLDENS") != null && !golden.getName().endsWith("_with_platform.d.ts") && expected.equals(expectedClean)) { - Files.asCharSink(golden, Charsets.UTF_8).write(stripped); + Files.asCharSink(golden, StandardCharsets.UTF_8).write(stripped); } else { check("generatedDeclarations()") .withMessage("Verifying %s", golden.getName()) diff --git a/src/test/java/com/google/javascript/gents/TypeScriptGeneratorTest.java b/src/test/java/com/google/javascript/gents/TypeScriptGeneratorTest.java index 6b2e33fe..d6b257e0 100644 --- a/src/test/java/com/google/javascript/gents/TypeScriptGeneratorTest.java +++ b/src/test/java/com/google/javascript/gents/TypeScriptGeneratorTest.java @@ -2,7 +2,6 @@ import static com.google.common.truth.Truth.assertThat; -import com.google.common.base.Charsets; import com.google.common.io.Files; import com.google.javascript.clutz.DeclarationGeneratorTest; import com.google.javascript.gents.TypeScriptGenerator.GentsResult; @@ -60,7 +59,7 @@ static Path getPackagePath() { } static String getFileText(final File input) throws IOException { - String text = Files.asCharSource(input, Charsets.UTF_8).read(); + String text = Files.asCharSource(input, StandardCharsets.UTF_8).read(); String cleanText = DeclarationGeneratorTest.GOLDEN_FILE_COMMENTS_REGEXP.matcher(text).replaceAll(""); return cleanText;