Disallow inherited constructors by default.

	Change on 2016/11/16 by tball <tball@google.com>

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=139360682

doc/man/j2objc.1

@@ -59,6 +59,9 @@ Print this message.
 
 .SH TRANSLATION OPTIONS
 .TP
+.BI \-\-allow\-inherited\-constructors
+Don't issue compiler warnings when native code accesses inherited constructors.
+.TP
 \fB\-batch-translate-max\fR\=\fIn\fR
 The maximum number of source files that are translated together. Batching
 speeds up translation, but requires more memory.
@@ -69,9 +72,6 @@ Translate dependent classes if out-of-date.
 .BI \-\-dead\-code\-report " file "
 Specify a ProGuard usage report for dead code elimination.
 .TP
-.BI \-\-disallow\-inherited\-constructors
-Issue compiler warnings when native code accesses inherited constructors.
-.TP
 .BI \-\-doc\-comments
 Translate Javadoc comments into Xcode-compatible comments.
 .TP

translator/src/main/java/com/google/devtools/j2objc/Options.java

@@ -78,7 +78,7 @@ public class Options {
   private boolean staticAccessorMethods = false;
   private int batchTranslateMaximum = -1;
   private String processors = null;
-  private boolean disallowInheritedConstructors = false;
+  private boolean disallowInheritedConstructors = true;
   private boolean swiftFriendly = false;
   private boolean nullability = false;
   private EnumSet<LintOption> lintOptions = EnumSet.noneOf(LintOption.class);
@@ -115,6 +115,7 @@ public class Options {
 
   // TODO(tball): remove obsolete flags once projects stop using them.
   private static final Set<String> obsoleteFlags = Sets.newHashSet(
+    "--disallow-inherited-constructors",
     "--final-methods-as-functions",
     "--no-final-methods-functions",
     "--hide-private-members",
@@ -493,8 +494,8 @@ private String[] loadInternal(String[] args) throws IOException {
           usage("-processor requires an argument");
         }
         processors = args[nArg];
-      } else if (arg.equals("--disallow-inherited-constructors")) {
-        disallowInheritedConstructors = true;
+      } else if (arg.equals("--allow-inherited-constructors")) {
+        disallowInheritedConstructors = false;
       } else if (arg.equals("--nullability")) {
         nullability = true;
       } else if (arg.startsWith("-Xlint")) {

translator/src/main/java/com/google/devtools/j2objc/gen/TypeDeclarationGenerator.java

@@ -34,17 +34,21 @@
 import com.google.devtools.j2objc.ast.VariableDeclarationFragment;
 import com.google.devtools.j2objc.jdt.BindingConverter;
 import com.google.devtools.j2objc.util.BindingUtil;
+import com.google.devtools.j2objc.util.ElementUtil;
 import com.google.devtools.j2objc.util.NameTable;
 import com.google.devtools.j2objc.util.TranslationUtil;
 import com.google.devtools.j2objc.util.TypeUtil;
 import com.google.devtools.j2objc.util.UnicodeUtils;
 import com.google.j2objc.annotations.Property;
-import java.util.ArrayList;
-import java.util.Collection;
+import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
+import javax.lang.model.element.ExecutableElement;
 import javax.lang.model.element.TypeElement;
+import javax.lang.model.element.VariableElement;
 import javax.lang.model.type.PrimitiveType;
 import org.eclipse.jdt.core.dom.IMethodBinding;
 import org.eclipse.jdt.core.dom.ITypeBinding;
@@ -666,7 +670,7 @@ private void printSortedDeclarations(
    * with a NS_UNAVAILABLE macro, so that clang will flag such access from native
    * code as an error.
    */
-  private void printDisallowedConstructors() {
+  protected void printDisallowedConstructors() {
     if (!Options.disallowInheritedConstructors()) {
       return;
     }
@@ -675,66 +679,70 @@ private void printDisallowedConstructors() {
         || BindingUtil.isAbstract(typeBinding)) {
       return;
     }
-    Set<IMethodBinding> constructors = BindingUtil.getDeclaredConstructors(typeBinding);
-    List<IMethodBinding> inheritedConstructors = new ArrayList<>();
+    Set<String> constructors = new HashSet<>();
+    for (IMethodBinding constructor : BindingUtil.getDeclaredConstructors(typeBinding)) {
+      constructors.add(nameTable.getMethodSelector(constructor));
+    }
+    Map<String, IMethodBinding> inheritedConstructors = new HashMap<>();
     ITypeBinding superType = typeBinding.getSuperclass();
     while (superType != null) {
       // Add super constructors that have unique parameters.
       for (IMethodBinding superC : BindingUtil.getDeclaredConstructors(superType)) {
-        if (!hasConstructor(constructors, superC)
-            && !hasConstructor(inheritedConstructors, superC)) {
-          inheritedConstructors.add(superC);
+        String selector = nameTable.getMethodSelector(superC);
+        if (!constructors.contains(selector)) {
+          inheritedConstructors.put(selector, superC);
         }
       }
       superType = superType.getSuperclass();
     }
     if (!inheritedConstructors.isEmpty()) {
       newline();
       println("// Disallowed inherited constructors, do not use.");
-      for (IMethodBinding constructor : inheritedConstructors) {
-        print(getConstructorSignature(constructor));
+      for (IMethodBinding constructor : inheritedConstructors.values()) {
+        print(getConstructorSignature(BindingConverter.getExecutableElement(constructor)));
         println(" NS_UNAVAILABLE;");
       }
     }
   }
 
-  // Returns true if a constructor has an overriding constructor in a specified set.
-  private static boolean hasConstructor(Collection<IMethodBinding> constructors,
-      IMethodBinding constructor) {
-    for (IMethodBinding c : constructors) {
-      if (constructor.isSubsignature(c)) {
-        return true;
-      }
-    }
-    return false;
-  }
-
   /**
    * Create an Objective-C constructor signature from a method binding.
    * This is similar to TypeGenerator.getMethodSignature(MethodDeclaration),
    * but works with constructors not declared by the type being generated.
    */
-  protected String getConstructorSignature(IMethodBinding m) {
+  private String getConstructorSignature(ExecutableElement element) {
     StringBuilder sb = new StringBuilder();
     sb.append("- (instancetype)");
-    ITypeBinding[] params = m.getParameterTypes();
-    String selector = nameTable.getMethodSelector(m);
-    String[] selParts = selector.split(":");
-    if (params.length == 0) {
-      assert selParts.length == 1 && !selector.endsWith(":");
-      sb.append(selParts[0]);
+    String selector = nameTable.getMethodSelector(element);
+    if (!selector.endsWith(":")) {
+      sb.append(selector);
     } else {
-      assert params.length == selParts.length;
+      String[] selParts = selector.split(":");
       int baseLength = sb.length() + selParts[0].length();
-      for (int i = 0; i < params.length; i++) {
-        if (i != 0) {
-          sb.append('\n');
-          sb.append(pad(baseLength - selParts[i].length()));
-        }
-        String typeName = nameTable.getObjCType(params[i]);
-        sb.append(UnicodeUtils.format("%s:(%s)arg%d", selParts[i], typeName, i));
+      TypeElement declaringClass = ElementUtil.getDeclaringClass(element);
+      int i = 0;
+      boolean first = true;
+      for (VariableElement param : env.captureInfo().getImplicitPrefixParams(declaringClass)) {
+        first = printParameter(sb, param, selParts, i++, baseLength, first);
+      }
+      for (VariableElement param : element.getParameters()) {
+        first = printParameter(sb, param, selParts, i++, baseLength, first);
+      }
+      for (VariableElement param : env.captureInfo().getImplicitPostfixParams(declaringClass)) {
+        first = printParameter(sb, param, selParts, i++, baseLength, first);
       }
     }
     return sb.toString();
   }
+
+  private boolean printParameter(StringBuilder sb, VariableElement param, String[] selParts,
+      int iArg, int baseLength, boolean first) {
+    if (!first) {
+      sb.append('\n');
+      sb.append(pad(baseLength - selParts[iArg].length()));
+    }
+    String paramType = nameTable.getObjCType(param.asType());
+    sb.append(UnicodeUtils.format("%s:(%s)arg%d", selParts[iArg], paramType, iArg));
+    return false;
+  }
 }

translator/src/main/java/com/google/devtools/j2objc/gen/TypePrivateDeclarationGenerator.java

@@ -103,4 +103,9 @@ protected void printFunctionDeclaration(FunctionDeclaration function) {
     }
     println(";");
   }
+
+  @Override
+  protected void printDisallowedConstructors() {
+    // Disallowed constructor declarations only needed in public headers.
+  }
 }

translator/src/main/java/com/google/devtools/j2objc/types/HeaderImportCollector.java

@@ -17,6 +17,7 @@
 package com.google.devtools.j2objc.types;
 
 import com.google.common.collect.Sets;
+import com.google.devtools.j2objc.Options;
 import com.google.devtools.j2objc.ast.AbstractTypeDeclaration;
 import com.google.devtools.j2objc.ast.AnnotationTypeDeclaration;
 import com.google.devtools.j2objc.ast.AnnotationTypeMemberDeclaration;
@@ -31,11 +32,14 @@
 import com.google.devtools.j2objc.ast.TypeDeclaration;
 import com.google.devtools.j2objc.ast.UnitTreeVisitor;
 import com.google.devtools.j2objc.jdt.BindingConverter;
+import com.google.devtools.j2objc.util.ElementUtil;
 import com.google.devtools.j2objc.util.TranslationUtil;
 import java.util.HashSet;
 import java.util.LinkedHashSet;
 import java.util.Set;
+import javax.lang.model.element.ExecutableElement;
 import javax.lang.model.element.TypeElement;
+import javax.lang.model.element.VariableElement;
 import org.eclipse.jdt.core.dom.IMethodBinding;
 import org.eclipse.jdt.core.dom.ITypeBinding;
 
@@ -104,6 +108,13 @@ private void addForwardDecl(ITypeBinding type) {
   private void addSuperType(TypeElement type) {
     if (type != null) {
       Import.addImports(type.asType(), superTypes, unit.getEnv());
+      if (Options.disallowInheritedConstructors()) {
+        for (ExecutableElement constructor : ElementUtil.getConstructors(type)) {
+          for (VariableElement param : constructor.getParameters()) {
+            addForwardDecl(BindingConverter.unwrapTypeMirrorIntoTypeBinding(param.asType()));
+          }
+        }
+      }
     }
   }
 

translator/src/main/resources/com/google/devtools/j2objc/J2ObjC.properties

@@ -40,13 +40,13 @@ Common options:\n\
   -Werror                      Make all warnings into errors.\n\
   -h, --help                   Print this message.\n\n\
 Other options:\n\
+  --allow-inherited-constructors Don't issue compiler warnings when native code accesses\
+  \n                               inherited constructors.\n\
   --batch-translate-max=<n>    The maximum number of source files that are translated.\
   \n                               together. Batching speeds up translation, but\
   \n                               requires more memory.\n\
   --build-closure              Translate dependent classes if out-of-date.\n\
   --dead-code-report <file>    Specify a ProGuard usage report for dead code elimination.\n\
-  --disallow-inherited-constructors Issue compiler warnings when native code accesses\
-  \n                               inherited constructors.\n\
   --doc-comments               Translate Javadoc comments into Xcode-compatible comments.\n\
   --no-extract-unsequenced     Don't rewrite expressions that would produce unsequenced\
   \n                               modification errors.\n\

translator/src/test/java/com/google/devtools/j2objc/gen/ObjectiveCSourceFileGeneratorTest.java

@@ -166,7 +166,7 @@ public void testOverriddenGenericConstructor() throws IOException {
     String translation = translateSourceFile(
         "class B extends A<String> { B(String s) { super(s); } }", "B", "B.h");
     assertTranslation(translation, "- (instancetype)initWithNSString:(NSString *)s;");
-    assertNotInTranslation(translation, "initWithId");
+    assertTranslation(translation, "initWithId:(id)arg0 NS_UNAVAILABLE;");
   }
 
   public void testPrivateMethodHiding() throws IOException {