You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@royale.apache.org by jo...@apache.org on 2020/08/11 17:03:16 UTC

[royale-compiler] branch develop updated: AccessorEmitter: static getters/setters are output in a way that allows them to be accessed with normal . member access instead of dynamic [] string member access.

This is an automated email from the ASF dual-hosted git repository.

joshtynjala pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/royale-compiler.git


The following commit(s) were added to refs/heads/develop by this push:
     new 77e5332  AccessorEmitter: static getters/setters are output in a way that allows them to be accessed with normal . member access instead of dynamic [] string member access.
77e5332 is described below

commit 77e53323c25a4f29d24cb89a2112752af3dac6bb
Author: Josh Tynjala <jo...@apache.org>
AuthorDate: Tue Aug 11 10:03:07 2020 -0700

    AccessorEmitter: static getters/setters are output in a way that allows them to be accessed with normal . member access instead of dynamic [] string member access.
    
    This also removes the requirement to add export annoations on all static accessors (previously, even private ones had to be exported to work properly).
    
    IdentifierEmitter: gets or sets of static getters/setters use normal . member access instead of [] string member access.
---
 .../internal/codegen/js/JSSessionModel.java        |  1 +
 .../internal/codegen/js/jx/AccessorEmitter.java    | 51 ++++++++++++++++++++--
 .../internal/codegen/js/jx/IdentifierEmitter.java  | 18 +-------
 .../js/royale/TestRoyaleAccessorMembers.java       | 10 +++--
 .../codegen/js/royale/TestRoyaleAccessors.java     | 14 +++---
 .../codegen/js/royale/TestRoyaleExpressions.java   | 10 ++---
 .../codegen/js/royale/TestRoyaleGlobalClasses.java |  4 +-
 7 files changed, 70 insertions(+), 38 deletions(-)

diff --git a/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/JSSessionModel.java b/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/JSSessionModel.java
index 86989f2..ce9d2c7 100644
--- a/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/JSSessionModel.java
+++ b/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/JSSessionModel.java
@@ -48,6 +48,7 @@ public class JSSessionModel
         public String originalName;
         public String uri;
         public boolean suppressExport;
+        public boolean preventRename;
     }
 
     public static class BindableVarInfo
diff --git a/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/AccessorEmitter.java b/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/AccessorEmitter.java
index 97b34bf..258bc8b 100644
--- a/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/AccessorEmitter.java
+++ b/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/AccessorEmitter.java
@@ -85,7 +85,7 @@ public class AccessorEmitter extends JSSubEmitter implements
         RoyaleJSProject project = (RoyaleJSProject)getWalker().getProject();
         boolean emitExports = true;
         if (project != null && project.config != null)
-        	emitExports = project.config.getExportPublicSymbols();
+            emitExports = project.config.getExportPublicSymbols();
 
         if (!getModel().getPropertyMap().isEmpty())
         {
@@ -490,7 +490,7 @@ public class AccessorEmitter extends JSSubEmitter implements
                     writeNewline();
                     writeNewline();
                     writeNewline();
-                	writeNewline("/**");
+                    writeNewline("/**");
                     if (emitExports)
                     	writeNewline("  * @export");
                     if (p.type != null)
@@ -515,7 +515,41 @@ public class AccessorEmitter extends JSSubEmitter implements
                 }
                 else
                 {
-	                if (getterNode != null)
+                    // start by writing out the static accessors as regular variables
+                    // because Closure Compiler doesn't properly analyze calls to
+                    // defineProperties() alone.
+                    // since there's no analysis, Closure assumes that getters/setters
+                    // have no side effects, which results in important get/set calls
+                    // being removed as dead code.
+                    // defining the accessors as variables first convinces Closure to
+                    // handle them more intelligently while not preventing them from
+                    // being real accessors.
+                    // Source: https://developers.google.com/closure/compiler/docs/limitations
+                    writeNewline();
+                    writeNewline();
+                    writeNewline();
+                    writeNewline("/**");
+                    if (p.preventRename)
+                        writeNewline("  * @nocollapse");
+                    if (p.resolvedExport && !p.suppressExport)
+                        writeNewline("  * @export");
+                    if (p.type != null)
+                        writeNewline("  * @type {" + JSGoogDocEmitter.convertASTypeToJSType(p.type.getBaseName(), p.type.getPackageName()) + "}"); 
+                    writeNewline("  */");
+                    write(getEmitter().formatQualifiedName(qname));
+                    write(ASEmitterTokens.MEMBER_ACCESS);
+                    if (p.uri != null)
+                    {
+                        INamespaceDecorationNode ns = ((FunctionNode)getterNode).getActualNamespaceNode();
+                        INamespaceDefinition nsDef = (INamespaceDefinition)ns.resolve(project);
+                        fjs.formatQualifiedName(nsDef.getQualifiedName()); // register with used names
+                        write(JSRoyaleEmitter.formatNamespacedProperty(p.uri, baseName, false));
+                    }
+                    else
+                        write(baseName);
+                    write(ASEmitterTokens.SEMICOLON);
+
+                    if (getterNode != null)
 	                {
 	                    writeNewline();
 	                    writeNewline();
@@ -611,7 +645,7 @@ public class AccessorEmitter extends JSSubEmitter implements
                 ISetterNode setterNode = p.setter;
                 String baseName = p.name;
             	writeNewline("/**");
-                if (emitExports && !p.suppressExport)
+                if (p.resolvedExport && !p.suppressExport)
                     writeNewline("  * @export");
                 if (p.type != null)
                 	writeNewline("  * @type {" + JSGoogDocEmitter.convertASTypeToJSType(p.type.getBaseName(), p.type.getPackageName()) + "} */");
@@ -715,12 +749,18 @@ public class AccessorEmitter extends JSSubEmitter implements
         boolean emitExports = true;
         boolean exportProtected = false;
         boolean exportInternal = false;
+        boolean preventRenamePublicSymbols = true;
+        boolean preventRenameProtectedSymbols = true;
+        boolean preventRenameInternalSymbols = true;
         RoyaleJSProject project = (RoyaleJSProject) getWalker().getProject();
         if (project != null && project.config != null)
         {
             emitExports = project.config.getExportPublicSymbols();
             exportProtected = project.config.getExportProtectedSymbols();
             exportInternal = project.config.getExportInternalSymbols();
+            preventRenamePublicSymbols = project.config.getPreventRenamePublicSymbols();
+            preventRenameProtectedSymbols = project.config.getPreventRenameProtectedSymbols();
+            preventRenameInternalSymbols = project.config.getPreventRenameInternalSymbols();
         }
 		
 		PropertyNodes p = map.get(key);
@@ -736,14 +776,17 @@ public class AccessorEmitter extends JSSubEmitter implements
         if(uri != null || def.isPublic())
         {
             p.resolvedExport = p.resolvedExport || emitExports;
+            p.preventRename = p.preventRename || preventRenamePublicSymbols;
         }
         else if(def.isInternal())
         {
             p.resolvedExport = p.resolvedExport || exportInternal;
+            p.preventRename = p.preventRename || preventRenameInternalSymbols;
         }
         else if(def.isProtected())
         {
             p.resolvedExport = p.resolvedExport || exportProtected;
+            p.preventRename = p.preventRename || preventRenameProtectedSymbols;
         }
         p.getter = node;
 		if (!p.suppressExport) p.suppressExport = suppress;
diff --git a/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/IdentifierEmitter.java b/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/IdentifierEmitter.java
index 595a1e2..722625f 100644
--- a/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/IdentifierEmitter.java
+++ b/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/IdentifierEmitter.java
@@ -207,7 +207,7 @@ public class IdentifierEmitter extends JSSubEmitter implements
                     endMapping(prevSibling);
                     startMapping(parentNode, prevSibling);
                 }
-                if (!isCustomNamespace && (!(identifierIsAccessorFunction && isStatic))) {
+                if (!isCustomNamespace) {
                     write(ASEmitterTokens.MEMBER_ACCESS);
                     wroteMemberAccess = true;
                 }
@@ -358,14 +358,6 @@ public class IdentifierEmitter extends JSSubEmitter implements
                     	String ns = ((INamespaceResolvedReference)(nodeDef.getNamespaceReference())).resolveAETNamespace(getProject()).getName();
                     	write(JSRoyaleEmitter.formatNamespacedProperty(ns, qname, accessWithNS));
                     }
-                    else if (identifierIsAccessorFunction && isStatic)
-                    {
-                        write(ASEmitterTokens.SQUARE_OPEN);
-                        write(ASEmitterTokens.DOUBLE_QUOTE);
-                        write(node.getName());
-                        write(ASEmitterTokens.DOUBLE_QUOTE);
-                        write(ASEmitterTokens.SQUARE_CLOSE);
-                    }
                 	else
                 	{
                 	    if (!(nodeDef.getParent() instanceof IPackageDefinition)) {
@@ -396,14 +388,6 @@ public class IdentifierEmitter extends JSSubEmitter implements
                 	String ns = ((INamespaceResolvedReference)nodeDef.getNamespaceReference()).resolveAETNamespace(getProject()).getName();
                 	write(JSRoyaleEmitter.formatNamespacedProperty(ns, qname, accessWithNS));
                 }
-                else if (identifierIsAccessorFunction && isStatic)
-                {
-                    write(ASEmitterTokens.SQUARE_OPEN);
-                    write(ASEmitterTokens.DOUBLE_QUOTE);
-                	write(qname);
-                    write(ASEmitterTokens.DOUBLE_QUOTE);
-                    write(ASEmitterTokens.SQUARE_CLOSE);
-                }
                 else
                 {
                 	if (nodeDef != null && !isStatic && (nodeDef.getParent() instanceof ClassDefinition) && (!(nodeDef instanceof IParameterDefinition)) && nodeDef.isPrivate() && getProject().getAllowPrivateNameConflicts())
diff --git a/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleAccessorMembers.java b/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleAccessorMembers.java
index 079ff73..a8f5ea1 100644
--- a/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleAccessorMembers.java
+++ b/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleAccessorMembers.java
@@ -97,8 +97,9 @@ public class TestRoyaleAccessorMembers extends TestGoogAccessorMembers
         		IClassNode.class, WRAP_LEVEL_CLASS);
         asBlockWalker.visitClass(node);
         assertOut("/**\n * @constructor\n */\nRoyaleTest_A = function() {\n};\n\n\n" +
-				"RoyaleTest_A.get__foo = function() {\n  return -1;\n};\n\n\n" +
-        		"Object.defineProperties(RoyaleTest_A, /** @lends {RoyaleTest_A} */ {\n/**\n  * @export\n  * @type {number} */\nfoo: {\nget: RoyaleTest_A.get__foo}}\n);");
+          "/**\n  * @nocollapse\n  * @export\n  * @type {number}\n  */\nRoyaleTest_A.foo;\n\n\n" +
+          "RoyaleTest_A.get__foo = function() {\n  return -1;\n};\n\n\n" +
+          "Object.defineProperties(RoyaleTest_A, /** @lends {RoyaleTest_A} */ {\n/**\n  * @export\n  * @type {number} */\nfoo: {\nget: RoyaleTest_A.get__foo}}\n);");
     }
 
     @Override
@@ -157,8 +158,9 @@ public class TestRoyaleAccessorMembers extends TestGoogAccessorMembers
         		IClassNode.class, WRAP_LEVEL_CLASS);
         asBlockWalker.visitClass(node);
         assertOut("/**\n * @constructor\n */\nRoyaleTest_A = function() {\n};\n\n\n" +
-				"RoyaleTest_A.set__foo = function(value) {\n};\n\n\n" +
-        		"Object.defineProperties(RoyaleTest_A, /** @lends {RoyaleTest_A} */ {\n/**\n  * @export\n  * @type {number} */\nfoo: {\nset: RoyaleTest_A.set__foo}}\n);");
+          "/**\n  * @export\n  * @type {number}\n  */\nRoyaleTest_A.foo;\n\n\n" +
+          "RoyaleTest_A.set__foo = function(value) {\n};\n\n\n" +
+          "Object.defineProperties(RoyaleTest_A, /** @lends {RoyaleTest_A} */ {\n/**\n  * @export\n  * @type {number} */\nfoo: {\nset: RoyaleTest_A.set__foo}}\n);");
     }
 
     @Test
diff --git a/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleAccessors.java b/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleAccessors.java
index 002997f..5d0390f 100644
--- a/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleAccessors.java
+++ b/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleAccessors.java
@@ -135,9 +135,10 @@ public class TestRoyaleAccessors extends ASTestBase
                 IClassNode.class, WRAP_LEVEL_PACKAGE);
         asBlockWalker.visitClass(node);
         String expected = "/**\n * @constructor\n */\nB = function() {\n};\n\n\n/**\n */\nB.prototype.doStuff = function() {\n  var /** @type {string} */ theLabel = B.http_$$ns_apache_org$2017$custom$namespace__label;\n  B.http_$$ns_apache_org$2017$custom$namespace__label = theLabel;\n};\n\n\n/**\n * @private\n * @type {string}\n */\nB._label = null;\n\n\n" +
-				"B.http_$$ns_apache_org$2017$custom$namespace__get__label = function() {\n  return B._label;\n};\n\n\n" +
-				"B.http_$$ns_apache_org$2017$custom$namespace__set__label = function(value) {\n  B._label = value;\n};\n\n\n" +
-        		"Object.defineProperties(B, /** @lends {B} */ {\n/**\n  * @export\n  * @type {string} */\nhttp_$$ns_apache_org$2017$custom$namespace__label: {\nget: B.http_$$ns_apache_org$2017$custom$namespace__get__label,\nset: B.http_$$ns_apache_org$2017$custom$namespace__set__label}}\n);";
+                "/**\n  * @nocollapse\n  * @export\n  * @type {string}\n  */\nB.http_$$ns_apache_org$2017$custom$namespace__label;\n\n\n" +
+                "B.http_$$ns_apache_org$2017$custom$namespace__get__label = function() {\n  return B._label;\n};\n\n\n" +
+                "B.http_$$ns_apache_org$2017$custom$namespace__set__label = function(value) {\n  B._label = value;\n};\n\n\n" +
+                "Object.defineProperties(B, /** @lends {B} */ {\n/**\n  * @export\n  * @type {string} */\nhttp_$$ns_apache_org$2017$custom$namespace__label: {\nget: B.http_$$ns_apache_org$2017$custom$namespace__get__label,\nset: B.http_$$ns_apache_org$2017$custom$namespace__set__label}}\n);";
         assertOut(expected);
     }
 
@@ -149,9 +150,10 @@ public class TestRoyaleAccessors extends ASTestBase
                 IClassNode.class, WRAP_LEVEL_PACKAGE);
         asBlockWalker.visitClass(node);
         String expected = "/**\n * @constructor\n */\nB = function() {\n};\n\n\n/**\n */\nB.prototype.doStuff = function() {\n  var /** @type {string} */ theLabel = B.http_$$ns_apache_org$2017$custom$namespace__label;\n  B.http_$$ns_apache_org$2017$custom$namespace__label = theLabel;\n};\n\n\n/**\n * @private\n * @type {string}\n */\nB._label = null;\n\n\n" +
-				"B.http_$$ns_apache_org$2017$custom$namespace__get__label = function() {\n  return B._label;\n};\n\n\n" +
-				"B.http_$$ns_apache_org$2017$custom$namespace__set__label = function(value) {\n  B._label = value;\n};\n\n\n" +
-        		"Object.defineProperties(B, /** @lends {B} */ {\n/**\n  * @export\n  * @type {string} */\nhttp_$$ns_apache_org$2017$custom$namespace__label: {\nget: B.http_$$ns_apache_org$2017$custom$namespace__get__label,\nset: B.http_$$ns_apache_org$2017$custom$namespace__set__label}}\n);";
+                "/**\n  * @nocollapse\n  * @export\n  * @type {string}\n  */\nB.http_$$ns_apache_org$2017$custom$namespace__label;\n\n\n" +        
+                "B.http_$$ns_apache_org$2017$custom$namespace__get__label = function() {\n  return B._label;\n};\n\n\n" +
+                "B.http_$$ns_apache_org$2017$custom$namespace__set__label = function(value) {\n  B._label = value;\n};\n\n\n" +
+                "Object.defineProperties(B, /** @lends {B} */ {\n/**\n  * @export\n  * @type {string} */\nhttp_$$ns_apache_org$2017$custom$namespace__label: {\nget: B.http_$$ns_apache_org$2017$custom$namespace__get__label,\nset: B.http_$$ns_apache_org$2017$custom$namespace__set__label}}\n);";
         assertOut(expected);
     }
 
diff --git a/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleExpressions.java b/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleExpressions.java
index 39a1bac..085b5b6 100644
--- a/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleExpressions.java
+++ b/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleExpressions.java
@@ -894,7 +894,7 @@ public class TestRoyaleExpressions extends TestGoogExpressions
         IBinaryOperatorNode bnode = (IBinaryOperatorNode) findFirstDescendantOfType(
                 node, IBinaryOperatorNode.class);
         asBlockWalker.visitBinaryOperator(bnode);
-        assertOut("foo.bar.B[\"b\"] = 1");
+        assertOut("foo.bar.B.b = 1");
     }
 
     @Test
@@ -906,7 +906,7 @@ public class TestRoyaleExpressions extends TestGoogExpressions
         IBinaryOperatorNode bnode = (IBinaryOperatorNode) findFirstDescendantOfType(
                 node, IBinaryOperatorNode.class);
         asBlockWalker.visitBinaryOperator(bnode);
-        assertOut("foo.bar.B[\"b\"] = 1");
+        assertOut("foo.bar.B.b = 1");
     }
 
     @Test
@@ -918,7 +918,7 @@ public class TestRoyaleExpressions extends TestGoogExpressions
         IBinaryOperatorNode bnode = (IBinaryOperatorNode) findFirstDescendantOfType(
                 node, IBinaryOperatorNode.class);
         asBlockWalker.visitBinaryOperator(bnode);
-        assertOut("foo.bar.B[\"d\"].b = 1");
+        assertOut("foo.bar.B.d.b = 1");
     }
 
     @Test
@@ -942,7 +942,7 @@ public class TestRoyaleExpressions extends TestGoogExpressions
 
         ((JSRoyaleEmitter)asEmitter).getModel().setCurrentClass(def);
         asBlockWalker.visitBinaryOperator(bnode);
-        assertOut("foo.bar.B[\"d\"].b = 1");
+        assertOut("foo.bar.B.d.b = 1");
     }
 
     @Test
@@ -954,7 +954,7 @@ public class TestRoyaleExpressions extends TestGoogExpressions
         IBinaryOperatorNode bnode = (IBinaryOperatorNode) findFirstDescendantOfType(
                 node, IBinaryOperatorNode.class);
         asBlockWalker.visitBinaryOperator(bnode);
-        assertOut("foo.bar.B[\"b\"] = foo.bar.B[\"b\"] + 1");
+        assertOut("foo.bar.B.b = foo.bar.B.b + 1");
     }
 
     @Test
diff --git a/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleGlobalClasses.java b/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleGlobalClasses.java
index 6ef63d0..fc46038 100644
--- a/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleGlobalClasses.java
+++ b/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleGlobalClasses.java
@@ -303,7 +303,7 @@ public class TestRoyaleGlobalClasses extends TestGoogGlobalClasses
     {
         IVariableNode node = getVariable("var a:Number = Math.PI;");
         asBlockWalker.visitVariable(node);
-        assertOut("var /** @type {number} */ a = Math[\"PI\"]");
+        assertOut("var /** @type {number} */ a = Math.PI");
     }
     
     @Override
@@ -923,7 +923,7 @@ public class TestRoyaleGlobalClasses extends TestGoogGlobalClasses
         VariableNode node = (VariableNode)getNode("private static function get txtStr():String { return 'foo'; }; private function test() { var a:XML = <text><content>{txtStr}</content></text>;}",
         							 VariableNode.class, WRAP_LEVEL_CLASS);
         asBlockWalker.visitVariable(node);
-        assertOut("var /** @type {XML} */ a = new XML( '<text><content>' + RoyaleTest_A[\"txtStr\"] + '</content></text>')");
+        assertOut("var /** @type {XML} */ a = new XML( '<text><content>' + RoyaleTest_A.txtStr + '</content></text>')");
     }
     
     @Test