From b9c348ba210e32a1d4fd5e36078742cab88f633a Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Tue, 26 May 2020 21:51:16 -0500 Subject: [PATCH 1/4] Support instance member overriding a static member with union types Fixes #46 --- .../visitor/MembersClassCleaner.java | 15 +- .../visitor/UnionTypeHelperTypeCreator.java | 12 ++ .../generator/externs/overloads/BUILD | 22 +++ .../generator/externs/overloads/Foo.java.txt | 157 ++++++++++++++++++ .../generator/externs/overloads/overloads.js | 31 ++++ 5 files changed, 230 insertions(+), 7 deletions(-) create mode 100644 javatests/jsinterop/generator/externs/overloads/BUILD create mode 100644 javatests/jsinterop/generator/externs/overloads/Foo.java.txt create mode 100644 javatests/jsinterop/generator/externs/overloads/overloads.js diff --git a/java/jsinterop/generator/visitor/MembersClassCleaner.java b/java/jsinterop/generator/visitor/MembersClassCleaner.java index 18cfd91..03cc6ce 100644 --- a/java/jsinterop/generator/visitor/MembersClassCleaner.java +++ b/java/jsinterop/generator/visitor/MembersClassCleaner.java @@ -17,8 +17,7 @@ package jsinterop.generator.visitor; -import static jsinterop.generator.model.AnnotationType.JS_METHOD; -import static jsinterop.generator.model.AnnotationType.JS_PROPERTY; +import static jsinterop.generator.model.AnnotationType.*; import static jsinterop.generator.model.EntityKind.METHOD; import java.util.ArrayList; @@ -163,12 +162,14 @@ private void renameSameEntities( String name = entity.getName(); entity.setName(name + "_STATIC"); - // TODO(dramaix): add a javadoc above the field explaining why it was renamed - AnnotationType annotationType = - entity.getKind() == METHOD ? JS_METHOD : JS_PROPERTY; + if (!entity.hasAnnotation(JS_OVERLAY)) { + // TODO(dramaix): add a javadoc above the field explaining why it was renamed + AnnotationType annotationType = + entity.getKind() == METHOD ? JS_METHOD : JS_PROPERTY; - ModelHelper.addAnnotationNameAttributeIfNotEmpty( - entity, name, annotationType, true); + ModelHelper.addAnnotationNameAttributeIfNotEmpty( + entity, name, annotationType, true); + } } } } diff --git a/java/jsinterop/generator/visitor/UnionTypeHelperTypeCreator.java b/java/jsinterop/generator/visitor/UnionTypeHelperTypeCreator.java index b72a70a..97178ba 100644 --- a/java/jsinterop/generator/visitor/UnionTypeHelperTypeCreator.java +++ b/java/jsinterop/generator/visitor/UnionTypeHelperTypeCreator.java @@ -80,12 +80,18 @@ public void applyTo(Program program) { @Override public boolean shouldProcessField(Field field) { + if (field.isStatic()) { + currentNameStack.push("Static"); + } currentNameStack.push(toCamelUpperCase(field.getName())); return true; } @Override public Field rewriteField(Field field) { + if (field.isStatic()) { + currentNameStack.pop(); + } currentNameStack.pop(); return field; } @@ -95,6 +101,9 @@ public boolean shouldProcessMethod(Method method) { // JsFunction type only have one method named onInvoke, don't use the method name // because it doesn't give us any much more information. if (!isCurrentTypeJsFunction()) { + if (method.isStatic()) { + currentNameStack.push("Static"); + } currentNameStack.push( method.getKind() == CONSTRUCTOR ? "Constructor" @@ -106,6 +115,9 @@ public boolean shouldProcessMethod(Method method) { @Override public Method rewriteMethod(Method method) { if (!isCurrentTypeJsFunction()) { + if (method.isStatic()) { + currentNameStack.pop(); + } currentNameStack.pop(); } diff --git a/javatests/jsinterop/generator/externs/overloads/BUILD b/javatests/jsinterop/generator/externs/overloads/BUILD new file mode 100644 index 0000000..bace774 --- /dev/null +++ b/javatests/jsinterop/generator/externs/overloads/BUILD @@ -0,0 +1,22 @@ +# Description: +# Tests conversion of "overloaded" members, static and non-static +# + +load( + "//javatests/jsinterop/generator:jsinterop_generator_test.bzl", + "jsinterop_generator_test", +) + +package( + default_visibility = ["//:__subpackages__"], + # Apache2 + licenses = ["notice"], +) + +jsinterop_generator_test( + name = "overloads", + srcs = ["overloads.js"], + expected_output = [ + "Foo.java.txt", + ], +) diff --git a/javatests/jsinterop/generator/externs/overloads/Foo.java.txt b/javatests/jsinterop/generator/externs/overloads/Foo.java.txt new file mode 100644 index 0000000..f4cf0c3 --- /dev/null +++ b/javatests/jsinterop/generator/externs/overloads/Foo.java.txt @@ -0,0 +1,157 @@ +package jsinterop.generator.externs.overloads; + +import jsinterop.annotations.JsMethod; +import jsinterop.annotations.JsOverlay; +import jsinterop.annotations.JsPackage; +import jsinterop.annotations.JsProperty; +import jsinterop.annotations.JsType; +import jsinterop.base.Js; + +@JsType(isNative = true, namespace = JsPackage.GLOBAL) +public class Foo { + @JsType(isNative = true, name = "?", namespace = JsPackage.GLOBAL) + public interface StaticUnionArgMethodArgUnionType { + @JsOverlay + static Foo.StaticUnionArgMethodArgUnionType of(Object o) { + return Js.cast(o); + } + + @JsOverlay + default double asDouble() { + return Js.asDouble(this); + } + + @JsOverlay + default String asString() { + return Js.asString(this); + } + + @JsOverlay + default boolean isDouble() { + return (Object) this instanceof Double; + } + + @JsOverlay + default boolean isString() { + return (Object) this instanceof String; + } + } + + @JsType(isNative = true, name = "?", namespace = JsPackage.GLOBAL) + public interface StaticUnionPropertyUnionType { + @JsOverlay + static Foo.StaticUnionPropertyUnionType of(Object o) { + return Js.cast(o); + } + + @JsOverlay + default double asDouble() { + return Js.asDouble(this); + } + + @JsOverlay + default String asString() { + return Js.asString(this); + } + + @JsOverlay + default boolean isDouble() { + return (Object) this instanceof Double; + } + + @JsOverlay + default boolean isString() { + return (Object) this instanceof String; + } + } + + @JsType(isNative = true, name = "?", namespace = JsPackage.GLOBAL) + public interface UnionArgMethodArgUnionType { + @JsOverlay + static Foo.UnionArgMethodArgUnionType of(Object o) { + return Js.cast(o); + } + + @JsOverlay + default double asDouble() { + return Js.asDouble(this); + } + + @JsOverlay + default String asString() { + return Js.asString(this); + } + + @JsOverlay + default boolean isDouble() { + return (Object) this instanceof Double; + } + + @JsOverlay + default boolean isString() { + return (Object) this instanceof String; + } + } + + @JsType(isNative = true, name = "?", namespace = JsPackage.GLOBAL) + public interface UnionPropertyUnionType { + @JsOverlay + static Foo.UnionPropertyUnionType of(Object o) { + return Js.cast(o); + } + + @JsOverlay + default double asDouble() { + return Js.asDouble(this); + } + + @JsOverlay + default String asString() { + return Js.asString(this); + } + + @JsOverlay + default boolean isDouble() { + return (Object) this instanceof Double; + } + + @JsOverlay + default boolean isString() { + return (Object) this instanceof String; + } + } + + @JsProperty(name = "unionProperty") + public static Foo.StaticUnionPropertyUnionType unionProperty_STATIC; + + @JsMethod(name = "noArgMethod") + public static native Object noArgMethod_STATIC(); + + public static native Object unionArgMethod(Foo.StaticUnionArgMethodArgUnionType arg); + + @JsOverlay + public static final Object unionArgMethod_STATIC(String arg) { + return unionArgMethod(Js.uncheckedCast(arg)); + } + + @JsOverlay + public static final Object unionArgMethod_STATIC(double arg) { + return unionArgMethod(Js.uncheckedCast(arg)); + } + + public Foo.UnionPropertyUnionType unionProperty; + + public native Object noArgMethod(); + + @JsOverlay + public final Object unionArgMethod(String arg) { + return unionArgMethod(Js.uncheckedCast(arg)); + } + + public native Object unionArgMethod(Foo.UnionArgMethodArgUnionType arg); + + @JsOverlay + public final Object unionArgMethod(double arg) { + return unionArgMethod(Js.uncheckedCast(arg)); + } +} diff --git a/javatests/jsinterop/generator/externs/overloads/overloads.js b/javatests/jsinterop/generator/externs/overloads/overloads.js new file mode 100644 index 0000000..7a82c44 --- /dev/null +++ b/javatests/jsinterop/generator/externs/overloads/overloads.js @@ -0,0 +1,31 @@ + +/** + * @fileoverview Test "overloaded" method conventions + * @externs + */ + +/** + * @constructor + */ +function Foo() {} + +Foo.noArgMethod = function() {}; +Foo.prototype.noArgMethod = function() {}; + +/** + * @param {string|number} arg + */ +Foo.unionArgMethod = function(arg) {}; +/** + * @param {string|number} arg + */ +Foo.prototype.unionArgMethod = function(arg) {}; + +/** + * @type {string|number} + */ +Foo.unionProperty; +/** + * @type {string|number} + */ +Foo.prototype.unionProperty; \ No newline at end of file From 3fa73e579f3d81ecaa575002c6ec02437ede3d70 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Wed, 27 May 2020 12:32:09 -0500 Subject: [PATCH 2/4] Also add a test for a function property and method arg --- .../generator/externs/overloads/Foo.java.txt | 15 +++++++++++++++ .../generator/externs/overloads/overloads.js | 12 +++++++++++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/javatests/jsinterop/generator/externs/overloads/Foo.java.txt b/javatests/jsinterop/generator/externs/overloads/Foo.java.txt index f4cf0c3..022926d 100644 --- a/javatests/jsinterop/generator/externs/overloads/Foo.java.txt +++ b/javatests/jsinterop/generator/externs/overloads/Foo.java.txt @@ -1,5 +1,6 @@ package jsinterop.generator.externs.overloads; +import jsinterop.annotations.JsFunction; import jsinterop.annotations.JsMethod; import jsinterop.annotations.JsOverlay; import jsinterop.annotations.JsPackage; @@ -9,6 +10,16 @@ import jsinterop.base.Js; @JsType(isNative = true, namespace = JsPackage.GLOBAL) public class Foo { + @JsFunction + public interface FunctionPropertyFn { + void onInvoke(); + } + + @JsFunction + public interface FunctionPropertyFn0 { + void onInvoke(); + } + @JsType(isNative = true, name = "?", namespace = JsPackage.GLOBAL) public interface StaticUnionArgMethodArgUnionType { @JsOverlay @@ -121,6 +132,9 @@ public class Foo { } } + @JsProperty(name = "functionProperty") + public static Foo.FunctionPropertyFn functionProperty_STATIC; + @JsProperty(name = "unionProperty") public static Foo.StaticUnionPropertyUnionType unionProperty_STATIC; @@ -139,6 +153,7 @@ public class Foo { return unionArgMethod(Js.uncheckedCast(arg)); } + public Foo.FunctionPropertyFn0 functionProperty; public Foo.UnionPropertyUnionType unionProperty; public native Object noArgMethod(); diff --git a/javatests/jsinterop/generator/externs/overloads/overloads.js b/javatests/jsinterop/generator/externs/overloads/overloads.js index 7a82c44..0f1fa43 100644 --- a/javatests/jsinterop/generator/externs/overloads/overloads.js +++ b/javatests/jsinterop/generator/externs/overloads/overloads.js @@ -28,4 +28,14 @@ Foo.unionProperty; /** * @type {string|number} */ -Foo.prototype.unionProperty; \ No newline at end of file +Foo.prototype.unionProperty; + +/** + * @type {function():void} + */ +Foo.functionProperty; + +/** + * @type {function():void} + */ +Foo.prototype.functionProperty; \ No newline at end of file From 91921918950e2e07c295bf76f616eeae2e6bdeb2 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Wed, 22 Jul 2020 13:47:10 -0500 Subject: [PATCH 3/4] Fix imports --- java/jsinterop/generator/visitor/MembersClassCleaner.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/java/jsinterop/generator/visitor/MembersClassCleaner.java b/java/jsinterop/generator/visitor/MembersClassCleaner.java index 03cc6ce..1e3f011 100644 --- a/java/jsinterop/generator/visitor/MembersClassCleaner.java +++ b/java/jsinterop/generator/visitor/MembersClassCleaner.java @@ -17,7 +17,9 @@ package jsinterop.generator.visitor; -import static jsinterop.generator.model.AnnotationType.*; +import static jsinterop.generator.model.AnnotationType.JS_METHOD; +import static jsinterop.generator.model.AnnotationType.JS_OVERLAY; +import static jsinterop.generator.model.AnnotationType.JS_PROPERTY; import static jsinterop.generator.model.EntityKind.METHOD; import java.util.ArrayList; From 98339c2ca208db64534a5bb5a74da7ee632ed0fa Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Wed, 22 Jul 2020 13:49:28 -0500 Subject: [PATCH 4/4] Draft at response to feedback, only adding static if there is an overload --- .../visitor/UnionTypeHelperTypeCreator.java | 23 +++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/java/jsinterop/generator/visitor/UnionTypeHelperTypeCreator.java b/java/jsinterop/generator/visitor/UnionTypeHelperTypeCreator.java index 97178ba..3b7c451 100644 --- a/java/jsinterop/generator/visitor/UnionTypeHelperTypeCreator.java +++ b/java/jsinterop/generator/visitor/UnionTypeHelperTypeCreator.java @@ -42,6 +42,7 @@ import jsinterop.generator.model.Annotation; import jsinterop.generator.model.ArrayTypeReference; import jsinterop.generator.model.CastExpression; +import jsinterop.generator.model.Entity; import jsinterop.generator.model.Field; import jsinterop.generator.model.InstanceOfExpression; import jsinterop.generator.model.JavaTypeReference; @@ -78,9 +79,23 @@ public void applyTo(Program program) { new AbstractRewriter() { private final Deque currentNameStack = new ArrayDeque<>(); + private final Deque currentTypeStack = new ArrayDeque<>(); + + @Override + public boolean shouldProcessType(Type node) { + currentTypeStack.push(node); + return super.shouldProcessType(node); + } + + @Override + public Type rewriteType(Type node) { + currentTypeStack.pop(); + return node; + } + @Override public boolean shouldProcessField(Field field) { - if (field.isStatic()) { + if (field.isStatic() && currentTypeStack.peek().getFields().stream().filter(f -> !f.equals(field)).anyMatch(f -> f.getName().equals(field.getName()))) { currentNameStack.push("Static"); } currentNameStack.push(toCamelUpperCase(field.getName())); @@ -89,7 +104,7 @@ public boolean shouldProcessField(Field field) { @Override public Field rewriteField(Field field) { - if (field.isStatic()) { + if (field.isStatic() && currentTypeStack.peek().getFields().stream().filter(f -> !f.equals(field)).anyMatch(f -> f.getName().equals(field.getName()))) { currentNameStack.pop(); } currentNameStack.pop(); @@ -101,7 +116,7 @@ public boolean shouldProcessMethod(Method method) { // JsFunction type only have one method named onInvoke, don't use the method name // because it doesn't give us any much more information. if (!isCurrentTypeJsFunction()) { - if (method.isStatic()) { + if (method.isStatic() && currentTypeStack.peek().getMethods().stream().filter(m -> !m.equals(method)).filter(m -> !m.hasAnnotation(JS_OVERLAY)).anyMatch(m -> m.getName().equals(method.getName()))) { currentNameStack.push("Static"); } currentNameStack.push( @@ -115,7 +130,7 @@ public boolean shouldProcessMethod(Method method) { @Override public Method rewriteMethod(Method method) { if (!isCurrentTypeJsFunction()) { - if (method.isStatic()) { + if (method.isStatic() && currentTypeStack.peek().getMethods().stream().filter(m -> !m.equals(method)).filter(m -> !m.hasAnnotation(JS_OVERLAY)).anyMatch(m -> m.getName().equals(method.getName()))) { currentNameStack.pop(); } currentNameStack.pop();