From 127ba79be5d9804540994804219b295af00bc73e Mon Sep 17 00:00:00 2001 From: kstanger Date: Thu, 3 Nov 2016 11:25:14 -0700 Subject: [PATCH] Adds Mappings class for managing class and method mappings. This makes NameTable.Factory no longer necessary. Change on 2016/11/03 by kstanger ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=138094849 --- translator/Makefile | 1 + .../com/google/devtools/j2objc/Options.java | 66 ++-------- .../devtools/j2objc/javac/JavacParser.java | 2 +- .../google/devtools/j2objc/jdt/JdtParser.java | 4 +- .../translate/JavaToIOSMethodTranslator.java | 3 +- .../google/devtools/j2objc/util/Mappings.java | 120 ++++++++++++++++++ .../devtools/j2objc/util/NameTable.java | 78 ++---------- .../google/devtools/j2objc/util/Parser.java | 3 +- .../j2objc/util/TranslationEnvironment.java | 6 +- .../devtools/j2objc/util/NameTableTest.java | 19 +-- 10 files changed, 154 insertions(+), 148 deletions(-) create mode 100644 translator/src/main/java/com/google/devtools/j2objc/util/Mappings.java diff --git a/translator/Makefile b/translator/Makefile index 468a5048ce..f985f5b7b2 100644 --- a/translator/Makefile +++ b/translator/Makefile @@ -298,6 +298,7 @@ JAVA_SOURCES = \ util/ErrorUtil.java \ util/FileUtil.java \ util/HeaderMap.java \ + util/Mappings.java \ util/NameTable.java \ util/PackageInfoLookup.java \ util/PackagePrefixes.java \ diff --git a/translator/src/main/java/com/google/devtools/j2objc/Options.java b/translator/src/main/java/com/google/devtools/j2objc/Options.java index 4585ee9960..6380da5724 100644 --- a/translator/src/main/java/com/google/devtools/j2objc/Options.java +++ b/translator/src/main/java/com/google/devtools/j2objc/Options.java @@ -20,11 +20,10 @@ import com.google.common.base.Preconditions; import com.google.common.base.Splitter; import com.google.common.collect.Lists; -import com.google.common.collect.Maps; import com.google.common.io.Resources; import com.google.devtools.j2objc.util.ErrorUtil; -import com.google.devtools.j2objc.util.FileUtil; import com.google.devtools.j2objc.util.HeaderMap; +import com.google.devtools.j2objc.util.Mappings; import com.google.devtools.j2objc.util.PackageInfoLookup; import com.google.devtools.j2objc.util.PackagePrefixes; import com.google.devtools.j2objc.util.Parser; @@ -32,15 +31,12 @@ import com.google.devtools.j2objc.util.Version; import java.io.File; import java.io.IOException; -import java.io.InputStream; import java.net.URL; import java.nio.charset.Charset; import java.nio.charset.UnsupportedCharsetException; import java.util.Collections; import java.util.EnumSet; -import java.util.Enumeration; import java.util.List; -import java.util.Map; import java.util.Properties; import java.util.logging.Handler; import java.util.logging.Level; @@ -73,8 +69,6 @@ public class Options { private boolean deprecatedDeclarations = false; private HeaderMap headerMap = new HeaderMap(); private File outputHeaderMappingFile = null; - private Map classMappings = Maps.newLinkedHashMap(); - private Map methodMappings = Maps.newLinkedHashMap(); private boolean stripGwtIncompatible = false; private boolean segmentedHeaders = true; private String fileEncoding = System.getProperty("file.encoding", "UTF-8"); @@ -98,6 +92,7 @@ public class Options { // TODO(tball): remove after front-end conversion is complete. private FrontEnd javaFrontEnd = FrontEnd.JDT; + private Mappings mappings = new Mappings(); private PackageInfoLookup packageInfoLookup = new PackageInfoLookup(); private PackagePrefixes packagePrefixes = new PackagePrefixes(packageInfoLookup); @@ -111,8 +106,6 @@ public class Options { public static final String DEFAULT_HEADER_MAPPING_FILE = "mappings.j2objc"; // Null if not set (means we use the default). Can be empty also (means we use no mapping files). - private static final String JRE_MAPPINGS_FILE = "JRE.mappings"; - private static String fileHeader; private static final String FILE_HEADER_KEY = "file-header"; private static String usageMessage; @@ -355,7 +348,7 @@ public static String[] load(String[] args) throws IOException { private String[] loadInternal(String[] args) throws IOException { setLogLevel(Level.INFO); - addJreMappings(); + mappings.addJreMappings(); // Create a temporary directory as the sourcepath's first entry, so that // modified sources will take precedence over regular files. @@ -393,7 +386,7 @@ private String[] loadInternal(String[] args) throws IOException { if (++nArg == args.length) { usage("--mapping requires an argument"); } - addMappingsFiles(args[nArg].split(",")); + mappings.addMappingsFiles(args[nArg].split(",")); } else if (arg.equals("--header-mapping")) { if (++nArg == args.length) { usage("--header-mapping requires an argument"); @@ -615,45 +608,6 @@ private void addPrefixOption(String arg) { packagePrefixes.addPrefix(arg.substring(0, i), arg.substring(i + 1)); } - private void addMappingsFiles(String[] filenames) throws IOException { - for (String filename : filenames) { - if (!filename.isEmpty()) { - addMappingsProperties(FileUtil.loadProperties(filename)); - } - } - } - - private void addJreMappings() throws IOException { - InputStream stream = J2ObjC.class.getResourceAsStream(JRE_MAPPINGS_FILE); - addMappingsProperties(FileUtil.loadProperties(stream)); - } - - private void addMappingsProperties(Properties mappings) { - Enumeration keyIterator = mappings.propertyNames(); - while (keyIterator.hasMoreElements()) { - String key = (String) keyIterator.nextElement(); - if (key.indexOf('(') > 0) { - // All method mappings have parentheses characters, classes don't. - String iosMethod = mappings.getProperty(key); - addMapping(methodMappings, key, iosMethod, "method mapping"); - } else { - String iosClass = mappings.getProperty(key); - addMapping(classMappings, key, iosClass, "class mapping"); - } - } - } - - /** - * Adds a class, method or package-prefix property to its map, reporting an error - * if that mapping was previously specified. - */ - private static void addMapping(Map map, String key, String value, String kind) { - String oldValue = map.put(key, value); - if (oldValue != null && !oldValue.equals(value)) { - ErrorUtil.error(kind + " redefined; was \"" + oldValue + ", now " + value); - } - } - /** * Check that the memory management option wasn't previously set to a * different value. If okay, then set the option. @@ -810,14 +764,6 @@ public static boolean generateDeprecatedDeclarations() { return instance.deprecatedDeclarations; } - public static Map getClassMappings() { - return instance.classMappings; - } - - public static Map getMethodMappings() { - return instance.methodMappings; - } - public static HeaderMap getHeaderMap() { return instance.headerMap; } @@ -872,6 +818,10 @@ public static List getBootClasspath() { return getPathArgument(bootclasspath); } + public static Mappings getMappings() { + return instance.mappings; + } + public static PackageInfoLookup getPackageInfoLookup() { return instance.packageInfoLookup; } diff --git a/translator/src/main/java/com/google/devtools/j2objc/javac/JavacParser.java b/translator/src/main/java/com/google/devtools/j2objc/javac/JavacParser.java index d334caeb1d..8d14070ff2 100644 --- a/translator/src/main/java/com/google/devtools/j2objc/javac/JavacParser.java +++ b/translator/src/main/java/com/google/devtools/j2objc/javac/JavacParser.java @@ -122,7 +122,7 @@ public void parseFiles(Collection paths, Handler handler, SourceVersion JavacTaskImpl task = (JavacTaskImpl) compiler.getTask( null, fileManager, diagnostics, javacOptions, null, fileObjects); JavacEnvironment parserEnv = new JavacEnvironment(task.getContext()); - TranslationEnvironment env = new TranslationEnvironment(nameTableFactory, parserEnv); + TranslationEnvironment env = new TranslationEnvironment(parserEnv); List units = new ArrayList<>(); try { diff --git a/translator/src/main/java/com/google/devtools/j2objc/jdt/JdtParser.java b/translator/src/main/java/com/google/devtools/j2objc/jdt/JdtParser.java index e189ea1075..c08abb2f84 100644 --- a/translator/src/main/java/com/google/devtools/j2objc/jdt/JdtParser.java +++ b/translator/src/main/java/com/google/devtools/j2objc/jdt/JdtParser.java @@ -134,7 +134,7 @@ public com.google.devtools.j2objc.ast.CompilationUnit parse(String mainTypeName, mainTypeName = FileUtil.getQualifiedMainTypeName(file, unit); } ParserEnvironment parserEnv = new JdtParserEnvironment(unit.getAST()); - TranslationEnvironment env = new TranslationEnvironment(nameTableFactory, parserEnv); + TranslationEnvironment env = new TranslationEnvironment(parserEnv); return TreeConverter.convertCompilationUnit(env, unit, path, mainTypeName, source); } @@ -163,7 +163,7 @@ public void acceptAST(String sourceFilePath, CompilationUnit ast) { try { String source = FileUtil.readFile(file); ParserEnvironment parserEnv = new JdtParserEnvironment(ast.getAST()); - TranslationEnvironment env = new TranslationEnvironment(nameTableFactory, parserEnv); + TranslationEnvironment env = new TranslationEnvironment(parserEnv); com.google.devtools.j2objc.ast.CompilationUnit unit = TreeConverter.convertCompilationUnit( env, ast, sourceFilePath, FileUtil.getMainTypeName(file), source); diff --git a/translator/src/main/java/com/google/devtools/j2objc/translate/JavaToIOSMethodTranslator.java b/translator/src/main/java/com/google/devtools/j2objc/translate/JavaToIOSMethodTranslator.java index 28d4c9e525..54e3171a45 100644 --- a/translator/src/main/java/com/google/devtools/j2objc/translate/JavaToIOSMethodTranslator.java +++ b/translator/src/main/java/com/google/devtools/j2objc/translate/JavaToIOSMethodTranslator.java @@ -35,6 +35,7 @@ import com.google.devtools.j2objc.types.IOSMethodBinding; import com.google.devtools.j2objc.util.BindingUtil; import com.google.devtools.j2objc.util.ErrorUtil; +import com.google.devtools.j2objc.util.Mappings; import com.google.devtools.j2objc.util.NameTable; import org.eclipse.jdt.core.dom.IMethodBinding; @@ -85,7 +86,7 @@ public boolean visit(ClassInstanceCreation node) { IMethodBinding binding = node.getMethodBinding(); String key = BindingUtil.getMethodKey(binding); - String selector = NameTable.STRING_CONSTRUCTOR_TO_METHOD_MAPPINGS.get(key); + String selector = Mappings.STRING_CONSTRUCTOR_TO_METHOD_MAPPINGS.get(key); if (selector != null) { assert !node.hasRetainedResult(); if (key.equals("java.lang.String.String(Ljava/lang/String;)V")) { diff --git a/translator/src/main/java/com/google/devtools/j2objc/util/Mappings.java b/translator/src/main/java/com/google/devtools/j2objc/util/Mappings.java new file mode 100644 index 0000000000..eee7ea7fea --- /dev/null +++ b/translator/src/main/java/com/google/devtools/j2objc/util/Mappings.java @@ -0,0 +1,120 @@ +/* + * 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. + */ + +package com.google.devtools.j2objc.util; + +import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.ImmutableMap; +import com.google.devtools.j2objc.J2ObjC; +import java.io.IOException; +import java.io.InputStream; +import java.util.Enumeration; +import java.util.HashMap; +import java.util.Map; +import java.util.Properties; + +/** + * Manages class and method name mappings. + */ +public class Mappings { + + /** + * We convert all the String constructor invocations to factory method + * invocations because we want to avoid calling [NSString alloc]. + */ + public static final Map STRING_CONSTRUCTOR_TO_METHOD_MAPPINGS = + ImmutableMap.builder() + .put("java.lang.String.String()V", "string") + .put("java.lang.String.String(Ljava/lang/String;)V", "stringWithString:") + .put("java.lang.String.String([B)V", "java_stringWithBytes:") + .put("java.lang.String.String([BLjava/lang/String;)V", "java_stringWithBytes:charsetName:") + .put("java.lang.String.String([BLjava/nio/charset/Charset;)V", + "java_stringWithBytes:charset:") + .put("java.lang.String.String([BI)V", "java_stringWithBytes:hibyte:") + .put("java.lang.String.String([BII)V", "java_stringWithBytes:offset:length:") + .put("java.lang.String.String([BIII)V", "java_stringWithBytes:hibyte:offset:length:") + .put("java.lang.String.String([BIILjava/lang/String;)V", + "java_stringWithBytes:offset:length:charsetName:") + .put("java.lang.String.String([BIILjava/nio/charset/Charset;)V", + "java_stringWithBytes:offset:length:charset:") + .put("java.lang.String.String([C)V", "java_stringWithCharacters:") + .put("java.lang.String.String([CII)V", "java_stringWithCharacters:offset:length:") + .put("java.lang.String.String([III)V", "java_stringWithInts:offset:length:") + .put("java.lang.String.String(II[C)V", "java_stringWithOffset:length:characters:") + .put("java.lang.String.String(Ljava/lang/StringBuffer;)V", + "java_stringWithJavaLangStringBuffer:") + .put("java.lang.String.String(Ljava/lang/StringBuilder;)V", + "java_stringWithJavaLangStringBuilder:") + .build(); + + private static final String JRE_MAPPINGS_FILE = "JRE.mappings"; + + private final Map classMappings = new HashMap<>(); + private final Map methodMappings = new HashMap<>(); + { + methodMappings.putAll(STRING_CONSTRUCTOR_TO_METHOD_MAPPINGS); + } + + public ImmutableMap getClassMappings() { + return ImmutableMap.copyOf(classMappings); + } + + public ImmutableMap getMethodMappings() { + return ImmutableMap.copyOf(methodMappings); + } + + @VisibleForTesting + void addClass(String key, String name) { + classMappings.put(key, name); + } + + public void addMappingsFiles(String[] filenames) throws IOException { + for (String filename : filenames) { + if (!filename.isEmpty()) { + addMappingsProperties(FileUtil.loadProperties(filename)); + } + } + } + + public void addJreMappings() throws IOException { + InputStream stream = J2ObjC.class.getResourceAsStream(JRE_MAPPINGS_FILE); + addMappingsProperties(FileUtil.loadProperties(stream)); + } + + private void addMappingsProperties(Properties mappings) { + Enumeration keyIterator = mappings.propertyNames(); + while (keyIterator.hasMoreElements()) { + String key = (String) keyIterator.nextElement(); + if (key.indexOf('(') > 0) { + // All method mappings have parentheses characters, classes don't. + String iosMethod = mappings.getProperty(key); + addMapping(methodMappings, key, iosMethod, "method mapping"); + } else { + String iosClass = mappings.getProperty(key); + addMapping(classMappings, key, iosClass, "class mapping"); + } + } + } + + /** + * Adds a class, method or package-prefix property to its map, reporting an error + * if that mapping was previously specified. + */ + private static void addMapping(Map map, String key, String value, String kind) { + String oldValue = map.put(key, value); + if (oldValue != null && !oldValue.equals(value)) { + ErrorUtil.error(kind + " redefined; was \"" + oldValue + ", now " + value); + } + } +} diff --git a/translator/src/main/java/com/google/devtools/j2objc/util/NameTable.java b/translator/src/main/java/com/google/devtools/j2objc/util/NameTable.java index 0007399cce..4688028252 100644 --- a/translator/src/main/java/com/google/devtools/j2objc/util/NameTable.java +++ b/translator/src/main/java/com/google/devtools/j2objc/util/NameTable.java @@ -273,35 +273,6 @@ public class NameTable { "superclass", "toManyRelationshipKeys", "toOneRelationshipKeys", "version"); - /** - * We convert all the String constructor invocations to factory method - * invocations because we want to avoid calling [NSString alloc]. - */ - public static final Map STRING_CONSTRUCTOR_TO_METHOD_MAPPINGS = - ImmutableMap.builder() - .put("java.lang.String.String()V", "string") - .put("java.lang.String.String(Ljava/lang/String;)V", "stringWithString:") - .put("java.lang.String.String([B)V", "java_stringWithBytes:") - .put("java.lang.String.String([BLjava/lang/String;)V", "java_stringWithBytes:charsetName:") - .put("java.lang.String.String([BLjava/nio/charset/Charset;)V", - "java_stringWithBytes:charset:") - .put("java.lang.String.String([BI)V", "java_stringWithBytes:hibyte:") - .put("java.lang.String.String([BII)V", "java_stringWithBytes:offset:length:") - .put("java.lang.String.String([BIII)V", "java_stringWithBytes:hibyte:offset:length:") - .put("java.lang.String.String([BIILjava/lang/String;)V", - "java_stringWithBytes:offset:length:charsetName:") - .put("java.lang.String.String([BIILjava/nio/charset/Charset;)V", - "java_stringWithBytes:offset:length:charset:") - .put("java.lang.String.String([C)V", "java_stringWithCharacters:") - .put("java.lang.String.String([CII)V", "java_stringWithCharacters:offset:length:") - .put("java.lang.String.String([III)V", "java_stringWithInts:offset:length:") - .put("java.lang.String.String(II[C)V", "java_stringWithOffset:length:characters:") - .put("java.lang.String.String(Ljava/lang/StringBuffer;)V", - "java_stringWithJavaLangStringBuffer:") - .put("java.lang.String.String(Ljava/lang/StringBuilder;)V", - "java_stringWithJavaLangStringBuilder:") - .build(); - /** * Map of package names to their specified prefixes. Multiple packages * can share a prefix; for example, the com.google.common packages in @@ -309,47 +280,16 @@ public class NameTable { */ private final PackagePrefixes prefixMap; - private final Map methodMappings; - - /** - * Factory class for creating new NameTable instances. - */ - public static class Factory { - - // Currently a shared map. - // TODO(kstanger): For thread safety this will either need to be a - // concurrent map, or make a copy for each NameTable. - private PackagePrefixes prefixMap = Options.getPackagePrefixes(); - - private final Map methodMappings = ImmutableMap.builder() - .putAll(Options.getMethodMappings()) - .putAll(STRING_CONSTRUCTOR_TO_METHOD_MAPPINGS) - .build(); - - private Factory() { - // Make sure the input mapping files have valid selectors. - for (String selector : methodMappings.values()) { - validateMethodSelector(selector); - } - } - - public NameTable newNameTable(Types typeEnv, ElementUtil elementUtil, CaptureInfo captureInfo) { - return new NameTable(typeEnv, elementUtil, captureInfo, prefixMap, methodMappings); - } - } - - public static Factory newFactory() { - return new Factory(); - } + private final ImmutableMap classMappings; + private final ImmutableMap methodMappings; - private NameTable( - Types typeEnv, ElementUtil elementUtil, CaptureInfo captureInfo, PackagePrefixes prefixMap, - Map methodMappings) { + public NameTable(Types typeEnv, ElementUtil elementUtil, CaptureInfo captureInfo) { this.typeEnv = typeEnv; this.elementUtil = elementUtil; this.captureInfo = captureInfo; - this.prefixMap = prefixMap; - this.methodMappings = methodMappings; + prefixMap = Options.getPackagePrefixes(); + classMappings = Options.getMappings().getClassMappings(); + methodMappings = Options.getMappings().getMethodMappings(); } public void setVariableName(IVariableBinding var, String name) { @@ -626,6 +566,7 @@ private String getRenamedMethodName(IMethodBinding method) { method = method.getMethodDeclaration(); String selector = methodMappings.get(BindingUtil.getMethodKey(method)); if (selector != null) { + validateMethodSelector(selector); return selector; } selector = getMethodNameFromAnnotation(method); @@ -931,8 +872,9 @@ public String getFullName(ITypeBinding binding) { String name = binding.getQualifiedName(); // Use mapping file entry, if it exists. - if (Options.getClassMappings().containsKey(name)) { - return Options.getClassMappings().get(name); + String mappedName = classMappings.get(name); + if (mappedName != null) { + return mappedName; } // Use camel-cased package+class name. diff --git a/translator/src/main/java/com/google/devtools/j2objc/util/Parser.java b/translator/src/main/java/com/google/devtools/j2objc/util/Parser.java index 370dee57dc..689eaa0117 100644 --- a/translator/src/main/java/com/google/devtools/j2objc/util/Parser.java +++ b/translator/src/main/java/com/google/devtools/j2objc/util/Parser.java @@ -30,7 +30,6 @@ public abstract class Parser { protected List sourcepathEntries = Lists.newArrayList(); protected String encoding = null; protected boolean includeRunningVMBootclasspath = true; - protected final NameTable.Factory nameTableFactory = NameTable.newFactory(); protected static final Splitter PATH_SPLITTER = Splitter.on(":").omitEmptyStrings(); @@ -132,4 +131,4 @@ public abstract void parseFiles( // Convert to return j2objc AST trees instead of JDT. public abstract org.eclipse.jdt.core.dom.CompilationUnit parseWithoutBindings( String unitName, String source); -} \ No newline at end of file +} diff --git a/translator/src/main/java/com/google/devtools/j2objc/util/TranslationEnvironment.java b/translator/src/main/java/com/google/devtools/j2objc/util/TranslationEnvironment.java index 6ecda59f80..cd7317277b 100644 --- a/translator/src/main/java/com/google/devtools/j2objc/util/TranslationEnvironment.java +++ b/translator/src/main/java/com/google/devtools/j2objc/util/TranslationEnvironment.java @@ -14,7 +14,6 @@ package com.google.devtools.j2objc.util; -import com.google.common.base.Preconditions; import com.google.devtools.j2objc.types.Types; /** @@ -29,13 +28,12 @@ public class TranslationEnvironment { private final NameTable nameTable; private final TranslationUtil translationUtil; - public TranslationEnvironment(NameTable.Factory nameTableFactory, ParserEnvironment parserEnv) { - Preconditions.checkNotNull(nameTableFactory); + public TranslationEnvironment(ParserEnvironment parserEnv) { elementUtil = new ElementUtil(parserEnv.elementUtilities()); typeUtil = new TypeUtil(parserEnv.typeUtilities(), elementUtil); typeEnv = new Types(parserEnv); captureInfo = new CaptureInfo(typeEnv, typeUtil); - nameTable = nameTableFactory.newNameTable(typeEnv, elementUtil, captureInfo); + nameTable = new NameTable(typeEnv, elementUtil, captureInfo); translationUtil = new TranslationUtil(typeEnv, nameTable); } diff --git a/translator/src/test/java/com/google/devtools/j2objc/util/NameTableTest.java b/translator/src/test/java/com/google/devtools/j2objc/util/NameTableTest.java index 186dd85c67..ed645d100c 100644 --- a/translator/src/test/java/com/google/devtools/j2objc/util/NameTableTest.java +++ b/translator/src/test/java/com/google/devtools/j2objc/util/NameTableTest.java @@ -146,18 +146,13 @@ public void testRenameClassAnnotation() throws IOException { } public void testRenameMapping() throws IOException { - Options.getClassMappings().put("foo.bar.A", "Test2Name"); - try { - addSourceFile("package foo.bar; public class A { static void test() {}}", "foo/bar/A.java"); - addSourceFile( - "package foo.bar; public class B { void test() { A.test(); }}", "foo/bar/B.java"); - String translation = translateSourceFile("foo.bar.A", "foo/bar/A.h"); - assertTranslation(translation, "@interface Test2Name : NSObject"); - translation = translateSourceFile("foo.bar.B", "foo/bar/B.m"); - assertTranslation(translation, "Test2Name_test();"); - } finally { - Options.getClassMappings().remove("foo.bar.A"); - } + Options.getMappings().addClass("foo.bar.A", "Test2Name"); + addSourceFile("package foo.bar; public class A { static void test() {}}", "foo/bar/A.java"); + addSourceFile("package foo.bar; public class B { void test() { A.test(); }}", "foo/bar/B.java"); + String translation = translateSourceFile("foo.bar.A", "foo/bar/A.h"); + assertTranslation(translation, "@interface Test2Name : NSObject"); + translation = translateSourceFile("foo.bar.B", "foo/bar/B.m"); + assertTranslation(translation, "Test2Name_test();"); } public void testRenameMethodAnnotation() throws IOException {