From 14d5e6de359be673d636d4ab348a8920ba503f58 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Sat, 1 Jun 2024 13:43:15 -0500 Subject: [PATCH] Rework jsinterop support for records to something lighter --- .../com/google/gwt/dev/jjs/ast/HasJsInfo.java | 3 + .../gwt/dev/jjs/impl/GwtAstBuilder.java | 51 ++++++++++------- .../google/gwt/dev/jjs/test/Java17Test.java | 55 +++++++++++++++---- 3 files changed, 79 insertions(+), 30 deletions(-) diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/HasJsInfo.java b/dev/core/src/com/google/gwt/dev/jjs/ast/HasJsInfo.java index 05a6f9e8dd..ec24041b3f 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/ast/HasJsInfo.java +++ b/dev/core/src/com/google/gwt/dev/jjs/ast/HasJsInfo.java @@ -60,6 +60,9 @@ public String computeName(JMember member) { @Override public String computeName(JMember member) { String methodName = member.getName(); + if (member.getEnclosingType() instanceof JRecordType) { + return methodName; + } if (startsWithCamelCase(methodName, "get")) { return Introspector.decapitalize(methodName.substring(3)); } 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 023b443ede..713c653108 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 @@ -222,6 +222,7 @@ import org.eclipse.jdt.internal.compiler.lookup.MethodScope; import org.eclipse.jdt.internal.compiler.lookup.MethodVerifier; import org.eclipse.jdt.internal.compiler.lookup.NestedTypeBinding; +import org.eclipse.jdt.internal.compiler.lookup.RecordComponentBinding; import org.eclipse.jdt.internal.compiler.lookup.ReferenceBinding; import org.eclipse.jdt.internal.compiler.lookup.Scope; import org.eclipse.jdt.internal.compiler.lookup.SourceTypeBinding; @@ -4113,8 +4114,14 @@ private void createField(FieldDeclaration x) { getFieldDisposition(binding), AccessModifier.fromFieldBinding(binding)); } enclosingType.addField(field); - JsInteropUtil.maybeSetJsInteropProperties(field, shouldExport(field), x.annotations); - processSuppressedWarnings(field, x.annotations); + if (x.isARecordComponent) { + // Skip setting jsinterop properties on record component fields + RecordComponentBinding component = ((SourceTypeBinding) binding.declaringClass).getRecordComponent(x.name); + processSuppressedWarnings(field, component.sourceRecordComponent().annotations); + } else { + JsInteropUtil.maybeSetJsInteropProperties(field, shouldExport(field), x.annotations); + processSuppressedWarnings(field, x.annotations); + } typeMap.setField(binding, field); } @@ -4183,9 +4190,14 @@ private void createMembers(TypeDeclaration x) { for (JField field : type.getFields()) { // Create a method binding that corresponds to the method we are creating, jdt won't // offer us one unless it was defined in source. + char[] fieldName = field.getName().toCharArray(); MethodBinding recordComponentAccessor = binding.getExactMethod( - field.getName().toCharArray(), new TypeBinding[0], curCud.scope); - typeMap.get(recordComponentAccessor); + fieldName, new TypeBinding[0], curCud.scope); + + // Get the record component, and pass on any annotations meant for the method + JMethod componentMethod = typeMap.get(recordComponentAccessor); + RecordComponentBinding component = binding.getRecordComponent(fieldName); + processAnnotations(component.sourceRecordComponent().annotations, componentMethod); } // At this time, we need to be sure a binding exists, either because the record declared @@ -4308,17 +4320,16 @@ private void createMethod(AbstractMethodDeclaration x) { } enclosingType.addMethod(method); - processAnnotations(x, method); + processAnnotations(x.annotations, method); typeMap.setMethod(b, method); } - private void processAnnotations(AbstractMethodDeclaration x, - JMethod method) { - maybeAddMethodSpecialization(x, method); - maybeSetInliningMode(x, method); - maybeSetHasNoSideEffects(x, method); - JsInteropUtil.maybeSetJsInteropProperties(method, shouldExport(method), x.annotations); - processSuppressedWarnings(method, x.annotations); + private void processAnnotations(Annotation[] annotations, JMethod method) { + maybeAddMethodSpecialization(annotations, method); + maybeSetInliningMode(annotations, method); + maybeSetHasNoSideEffects(annotations, method); + JsInteropUtil.maybeSetJsInteropProperties(method, shouldExport(method), annotations); + processSuppressedWarnings(method, annotations); } private void processAnnotations(JParameter parameter, Annotation... annotations) { @@ -4326,7 +4337,7 @@ private void processAnnotations(JParameter parameter, Annotation... annotations) processSuppressedWarnings(parameter, annotations); } - private void processSuppressedWarnings(CanHaveSuppressedWarnings x, Annotation... annotations) { + private static void processSuppressedWarnings(CanHaveSuppressedWarnings x, Annotation... annotations) { x.setSuppressedWarnings(JdtUtil.getSuppressedWarnings(annotations)); } @@ -4338,26 +4349,26 @@ private static boolean isUncheckedGenericMethodCall(MessageSend messageSend) { return false; } - private static void maybeSetInliningMode(AbstractMethodDeclaration x, JMethod method) { + private static void maybeSetInliningMode(Annotation[] annotations, JMethod method) { if (JdtUtil.getAnnotationByName( - x.annotations, "javaemul.internal.annotations.DoNotInline") != null) { + annotations, "javaemul.internal.annotations.DoNotInline") != null) { method.setInliningMode(InliningMode.DO_NOT_INLINE); } else if (JdtUtil.getAnnotationByName( - x.annotations, "javaemul.internal.annotations.ForceInline") != null) { + annotations, "javaemul.internal.annotations.ForceInline") != null) { method.setInliningMode(InliningMode.FORCE_INLINE); } } - private static void maybeSetHasNoSideEffects(AbstractMethodDeclaration x, JMethod method) { + private static void maybeSetHasNoSideEffects(Annotation[] annotations, JMethod method) { if (JdtUtil.getAnnotationByName( - x.annotations, "javaemul.internal.annotations.HasNoSideEffects") != null) { + annotations, "javaemul.internal.annotations.HasNoSideEffects") != null) { method.setHasSideEffects(false); } } - private void maybeAddMethodSpecialization(AbstractMethodDeclaration x, JMethod method) { + private void maybeAddMethodSpecialization(Annotation[] annotations, JMethod method) { AnnotationBinding specializeAnnotation = JdtUtil.getAnnotationByName( - x.annotations, "javaemul.internal.annotations.SpecializeMethod"); + annotations, "javaemul.internal.annotations.SpecializeMethod"); if (specializeAnnotation == null) { return; } diff --git a/user/test-super/com/google/gwt/dev/jjs/super/com/google/gwt/dev/jjs/test/Java17Test.java b/user/test-super/com/google/gwt/dev/jjs/super/com/google/gwt/dev/jjs/test/Java17Test.java index 63f8b1f57f..dfeb1104af 100644 --- a/user/test-super/com/google/gwt/dev/jjs/super/com/google/gwt/dev/jjs/test/Java17Test.java +++ b/user/test-super/com/google/gwt/dev/jjs/super/com/google/gwt/dev/jjs/test/Java17Test.java @@ -124,10 +124,10 @@ public String toString() { } /** - * Simple record with default exports. + * Simple record with one property accessor, one default method accessor */ @JsType(namespace = "java17") - public record JsRecord1(String name, int value) { } + public record JsRecord1(@JsProperty String name, int value) { } /** * Simple native type to verify JsRecord1. @@ -135,15 +135,16 @@ public record JsRecord1(String name, int value) { } @JsType(name = "JsRecord1", namespace = "java17", isNative = true) public static class JsObject1 { public String name; - public int value; + public native int value(); public JsObject1(String name, int value) { } } /** - * Record with explicit method accessors + * Record with explicit method accessor */ @JsType(namespace = "java17") - public record JsRecord2(@JsMethod String name, @JsMethod int value) { } + public record JsRecord2(@JsMethod String name, int value) { } + /** * Simple native type to verify JsRecord2. */ @@ -155,7 +156,32 @@ public JsObject2(String name, int value) { } public native int value(); } + /** + * Record with exported properties and methods. + */ + public record JsRecord3(String red, JsRecord1 green, JsRecord2 blue) { + @JsProperty + public String getFlavor() { + return "grape"; + } + @JsMethod + public int countBeans() { + return 7; + } + } + + /** + * Represented as an interface since there is no constructor to call or use to type check. + */ + @JsType(isNative = true) + public interface JsObject3 { + @JsProperty + String getFlavor(); + int countBeans(); + } + public void testJsTypeRecords() { + // Test with default accessor (method) and a property accessor JsRecord1 r1 = new JsRecord1("foo", 7); assertEquals("foo", r1.name()); assertEquals(7, r1.value()); @@ -163,12 +189,12 @@ public void testJsTypeRecords() { // Create an instance from JS, verify it is the same JsObject1 o1 = new JsObject1("foo", 7); - assertEquals("foo", r1.name); - assertEquals(7, r1.value); + assertEquals("foo", o1.name); + assertEquals(7, o1.value()); assertEquals(o1.toString(), r1.toString()); assertEquals(o1, r1); - // Repeat the test with methods for accessors + // Repeat the test with methods explicitly configured for accessors JsRecord2 r2 = new JsRecord2("foo", 7); assertEquals("foo", r2.name()); assertEquals(7, r2.value()); @@ -176,10 +202,19 @@ public void testJsTypeRecords() { // Create an instance from JS, verify it is the same JsObject2 o2 = new JsObject2("foo", 7); - assertEquals("foo", r2.name); - assertEquals(7, r2.value); + assertEquals("foo", o2.name()); + assertEquals(7, o2.value()); assertEquals(o2.toString(), r2.toString()); assertEquals(o2, r2); + // Test an object with exposed properties and methods + JsRecord3 r3 = new JsRecord3("fork", r1, r2); + assertEquals("grape", r3.getFlavor()); + assertEquals(7, r3.countBeans()); + + // Cast the instance to JS, verify it is the same + JsObject3 o3 = (JsObject3) (Object) r3; + assertEquals("grape", r3.getFlavor()); + assertEquals(7, r3.countBeans()); } }