Fixes a bug that was exporting private constructors.
The access modifier for JConstructors was always being recorded as
public, which meant that private constructors used to not throw any
JsInterop validity check exceptions (which was good) but they were
being output (which was bad). With the new JsInteropRestrictionChecker
checking they started being reported as errors which is correct from
the perspective of blocking name collisions but is wrong from the
perspective that these methods shouldn't have been being exported
anyway.
Fixed the bug(s) by making sure that JConstructors are created with the
proper access level. As a result validity checks and output processes
are not running properly.
Change-Id: I561d227835238e4d8e714cd33e6a91b552b6eec7
diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JConstructor.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JConstructor.java
index f6aa5ad..2a036ae 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/ast/JConstructor.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JConstructor.java
@@ -37,7 +37,8 @@
}
private Object readResolve() {
- JConstructor result = new JConstructor(SourceOrigin.UNKNOWN, enclosingType);
+ JConstructor result =
+ new JConstructor(SourceOrigin.UNKNOWN, enclosingType, AccessModifier.PUBLIC);
result.signature = signature;
return result;
}
@@ -62,10 +63,10 @@
return defaultConstructor;
}
- public JConstructor(SourceInfo info, JClassType enclosingType) {
+ public JConstructor(SourceInfo info, JClassType enclosingType, AccessModifier access) {
// Access only matters for virtual methods, just use public.
super(info, enclosingType.getShortName(), enclosingType, JPrimitiveType.VOID, false, false,
- true, AccessModifier.PUBLIC);
+ true, access);
}
@Override
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/GwtAstBuilder.java b/dev/core/src/com/google/gwt/dev/jjs/impl/GwtAstBuilder.java
index 41fef2d..3a7b7ad 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/GwtAstBuilder.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/GwtAstBuilder.java
@@ -1229,7 +1229,7 @@
// class lambda$0$Type implements T {}
JClassType innerLambdaClass = createInnerClass(JdtUtil.asDottedString(x.binding.declaringClass.compoundName) +
"$" + new String(x.binding.selector), x, info, funcType);
- JConstructor ctor = new JConstructor(info, innerLambdaClass);
+ JConstructor ctor = new JConstructor(info, innerLambdaClass, AccessModifier.PRIVATE);
// locals captured by the lambda and saved as fields on the anonymous inner class
List<JField> locals = new ArrayList<JField>();
@@ -1742,7 +1742,7 @@
lambdaNameToInnerLambdaType.put(lambdaName, innerLambdaClass);
newTypes.add(innerLambdaClass);
- JConstructor ctor = new JConstructor(info, innerLambdaClass);
+ JConstructor ctor = new JConstructor(info, innerLambdaClass, AccessModifier.PRIVATE);
JMethodBody ctorBody = new JMethodBody(info);
JThisRef thisRef = new JThisRef(info, innerLambdaClass);
@@ -3972,7 +3972,8 @@
JMethod method;
boolean isNested = JdtUtil.isInnerClass(declaringClass);
if (x.isConstructor()) {
- method = new JConstructor(info, (JClassType) enclosingType);
+ method =
+ new JConstructor(info, (JClassType) enclosingType, AccessModifier.fromMethodBinding(b));
if (x.isDefaultConstructor()) {
((JConstructor) method).setDefaultConstructor();
}
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/ReferenceMapper.java b/dev/core/src/com/google/gwt/dev/jjs/impl/ReferenceMapper.java
index 49ad6b6..36cc34e 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/ReferenceMapper.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/ReferenceMapper.java
@@ -200,7 +200,8 @@
JMethod createConstructor(SourceInfo info, MethodBinding b) {
JDeclaredType enclosingType = (JDeclaredType) get(b.declaringClass);
- JMethod method = new JConstructor(info, (JClassType) enclosingType);
+ JMethod method =
+ new JConstructor(info, (JClassType) enclosingType, AccessModifier.fromMethodBinding(b));
enclosingType.addMethod(method);
/*
diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/JsInteropRestrictionCheckerTest.java b/dev/core/test/com/google/gwt/dev/jjs/impl/JsInteropRestrictionCheckerTest.java
index 64a7ac1..6553b15 100644
--- a/dev/core/test/com/google/gwt/dev/jjs/impl/JsInteropRestrictionCheckerTest.java
+++ b/dev/core/test/com/google/gwt/dev/jjs/impl/JsInteropRestrictionCheckerTest.java
@@ -112,6 +112,30 @@
assertCompileSucceeds();
}
+ public void testMultiplePrivateConstructorsExportSucceeds() throws Exception {
+ addSnippetImport("com.google.gwt.core.client.js.JsExport");
+ addSnippetClassDecl(
+ "@JsExport",
+ "public static class Buggy {",
+ " private Buggy() {}",
+ " private Buggy(int a) {}",
+ "}");
+
+ assertCompileSucceeds();
+ }
+
+ public void testMultiplePublicConstructorsExportFails() throws Exception {
+ addSnippetImport("com.google.gwt.core.client.js.JsExport");
+ addSnippetClassDecl(
+ "@JsExport",
+ "public static class Buggy {",
+ " public Buggy() {}",
+ " public Buggy(int a) {}",
+ "}");
+
+ assertCompileFails();
+ }
+
public void testSingleExportSucceeds() throws Exception {
addSnippetImport("com.google.gwt.core.client.js.JsExport");
addSnippetClassDecl(
@@ -149,7 +173,8 @@
optimize("void", "new Buggy();");
fail("JsInteropRestrictionCheckerTest should have prevented the name collision.");
} catch (Exception e) {
- assertTrue(e.getCause() instanceof UnableToCompleteException);
+ assertTrue(e.getCause() instanceof UnableToCompleteException
+ || e instanceof UnableToCompleteException);
}
}
diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/codesplitter/FragmentExtractorTest.java b/dev/core/test/com/google/gwt/dev/jjs/impl/codesplitter/FragmentExtractorTest.java
index 6f0a17a..778f064 100644
--- a/dev/core/test/com/google/gwt/dev/jjs/impl/codesplitter/FragmentExtractorTest.java
+++ b/dev/core/test/com/google/gwt/dev/jjs/impl/codesplitter/FragmentExtractorTest.java
@@ -18,6 +18,7 @@
import com.google.gwt.dev.jjs.CorrelationFactory;
import com.google.gwt.dev.jjs.SourceInfo;
import com.google.gwt.dev.jjs.SourceOrigin;
+import com.google.gwt.dev.jjs.ast.AccessModifier;
import com.google.gwt.dev.jjs.ast.JClassType;
import com.google.gwt.dev.jjs.ast.JConstructor;
import com.google.gwt.dev.jjs.ast.JDeclaredType;
@@ -181,7 +182,8 @@
SourceInfo nullSourceInfo = new MockSourceInfo();
final JClassType barType = new JClassType(nullSourceInfo, "Bar", false, false);
final JsName barConstructorName = new JsName(null, "Bar", "Bar");
- final JConstructor barConstructor = new JConstructor(nullSourceInfo, barType);
+ final JConstructor barConstructor =
+ new JConstructor(nullSourceInfo, barType, AccessModifier.PUBLIC);
Map<String, JsFunction> functionsByName = new HashMap<String, JsFunction>();
functionsByName.put("JavaClassHierarchySetupUtil.defineClass",
new JsFunction(nullSourceInfo, new JsRootScope(), DEFINE_CLASS_FUNCTION_NAME));
