Oops, you already got there! Please ignore my previous comment :)

On Wed, Jan 6, 2021 at 12:39 PM <[email protected]> wrote:

> 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 084a639  AccessorEmitter: don't redeclare instance accessor as
> variable if it is an override
> 084a639 is described below
>
> commit 084a639759757024ea95db74b63ddaead33ee9d0
> Author: Josh Tynjala <[email protected]>
> AuthorDate: Tue Jan 5 15:39:36 2021 -0800
>
>     AccessorEmitter: don't redeclare instance accessor as variable if it
> is an override
>
>     Fixes commit 23e64f0e7297d9267cbd9b2d4d37fb60c6582552
> ---
>  .../internal/codegen/js/jx/AccessorEmitter.java    | 71
> +++++++++++-----------
>  .../js/royale/TestRoyaleAccessorMembers.java       | 12 ++--
>  .../codegen/js/royale/TestRoyaleClass.java         |  1 -
>  .../resources/royale/projects/super/Base_result.js |  8 ---
>  4 files changed, 41 insertions(+), 51 deletions(-)
>
> 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 771a56c..2c193bc 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
> @@ -128,42 +128,45 @@ public class AccessorEmitter extends JSSubEmitter
> implements
>                  }
>                  else
>                  {
> -                    // start by writing out the instance 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);
> -                    write(JSEmitterTokens.PROTOTYPE);
> -                    write(ASEmitterTokens.MEMBER_ACCESS);
> -                    if (p.uri != null)
> +                    IAccessorNode accessorNode = (getterNode != null) ?
> getterNode : setterNode;
> +                    if(!accessorNode.getDefinition().isOverride())
>                      {
> -                        IAccessorNode node = (getterNode != null) ?
> getterNode : setterNode;
> -                        INamespaceDecorationNode ns =
> ((FunctionNode)node).getActualNamespaceNode();
> -                        INamespaceDefinition nsDef =
> (INamespaceDefinition)ns.resolve(project);
> -
> fjs.formatQualifiedName(nsDef.getQualifiedName()); // register with used
> names
> -
> write(JSRoyaleEmitter.formatNamespacedProperty(p.uri, baseName, false));
> +                        // start by writing out the instance 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);
> +                        write(JSEmitterTokens.PROTOTYPE);
> +                        write(ASEmitterTokens.MEMBER_ACCESS);
> +                        if (p.uri != null)
> +                        {
> +                            INamespaceDecorationNode ns = ((FunctionNode)
> accessorNode).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);
>                      }
> -                    else
> -                        write(baseName);
> -                    write(ASEmitterTokens.SEMICOLON);
>
>                      if (getterNode != null)
>                         {
> 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 c8d4c3a..958902d 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
> @@ -77,8 +77,7 @@ public class TestRoyaleAccessorMembers extends
> TestGoogAccessorMembers
>                         IClassNode.class, WRAP_LEVEL_PACKAGE);
>          asBlockWalker.visitClass(node);
>          assertOut("/**\n * @constructor\n * @extends {A}\n */\nB =
> function() {\n  B.base(this, 'constructor');\n};\ngoog.inherits(B,
> A);\n\n\n" +
> -        "/**\n * @nocollapse\n * @export\n * @type {number}\n
> */\nB.prototype.foo;\n\n\n" +
> -                               "B.prototype.get__foo = function() {\n
> return B.superClass_.get__foo.apply(this);\n};\n\n\n" +
> +        "B.prototype.get__foo = function() {\n  return
> B.superClass_.get__foo.apply(this);\n};\n\n\n" +
>                         "Object.defineProperties(B.prototype, /** @lends
> {B.prototype} */ {\n/**\n * @type {number}\n */\nfoo: {\nget:
> B.prototype.get__foo}}\n);");
>      }
>
> @@ -89,8 +88,7 @@ public class TestRoyaleAccessorMembers extends
> TestGoogAccessorMembers
>                         IClassNode.class, WRAP_LEVEL_PACKAGE);
>          asBlockWalker.visitClass(node);
>          assertOut("/**\n * @constructor\n * @extends {A}\n */\nB =
> function() {\n  B.base(this, 'constructor');\n};\ngoog.inherits(B,
> A);\n\n\n" +
> -        "/**\n * @nocollapse\n * @export\n * @type {number}\n
> */\nB.prototype.foo;\n\n\n" +
> -                               "B.prototype.get__foo = function() {\n
> return B.superClass_.get__foo.apply(this);\n};\n\n\n" +
> +        "B.prototype.get__foo = function() {\n  return
> B.superClass_.get__foo.apply(this);\n};\n\n\n" +
>                         "Object.defineProperties(B.prototype, /** @lends
> {B.prototype} */ {\n/**\n * @type {number}\n */\nfoo: {\nget:
> B.prototype.get__foo,\nset: A.prototype.set__foo}}\n);");
>      }
>
> @@ -154,8 +152,7 @@ public class TestRoyaleAccessorMembers extends
> TestGoogAccessorMembers
>                         IClassNode.class, WRAP_LEVEL_PACKAGE);
>          asBlockWalker.visitClass(node);
>          assertOut("/**\n * @constructor\n * @extends {A}\n */\nB =
> function() {\n  B.base(this, 'constructor');\n};\ngoog.inherits(B,
> A);\n\n\n" +
> -        "/**\n * @nocollapse\n * @export\n * @type {number}\n
> */\nB.prototype.foo;\n\n\n" +
> -                               "B.prototype.set__foo = function(value)
> {\n  B.superClass_.set__foo.apply(this, [ value] );\n};\n\n\n" +
> +        "B.prototype.set__foo = function(value) {\n
> B.superClass_.set__foo.apply(this, [ value] );\n};\n\n\n" +
>                         "Object.defineProperties(B.prototype, /** @lends
> {B.prototype} */ {\n/**\n * @type {number}\n */\nfoo: {\nset:
> B.prototype.set__foo}}\n);");
>      }
>
> @@ -179,8 +176,7 @@ public class TestRoyaleAccessorMembers extends
> TestGoogAccessorMembers
>                         IClassNode.class, WRAP_LEVEL_PACKAGE);
>          asBlockWalker.visitClass(node);
>          assertOut("/**\n * @constructor\n * @extends {A}\n */\nB =
> function() {\n  B.base(this, 'constructor');\n};\ngoog.inherits(B,
> A);\n\n\n" +
> -        "/**\n * @nocollapse\n * @export\n * @type {number}\n
> */\nB.prototype.foo;\n\n\n" +
> -                               "B.prototype.set__foo = function(value)
> {\n  B.superClass_.set__foo.apply(this, [ value] );\n};\n\n\n" +
> +        "B.prototype.set__foo = function(value) {\n
> B.superClass_.set__foo.apply(this, [ value] );\n};\n\n\n" +
>                         "Object.defineProperties(B.prototype, /** @lends
> {B.prototype} */ {\n/**\n * @type {number}\n */\nfoo: {\nget:
> A.prototype.get__foo,\nset: B.prototype.set__foo}}\n);");
>      }
>
> diff --git
> a/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleClass.java
> b/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleClass.java
> index a30202b..595dbdd 100644
> ---
> a/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleClass.java
> +++
> b/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestRoyaleClass.java
> @@ -276,7 +276,6 @@ public class TestRoyaleClass extends TestGoogClass
>          IClassNode node = getClassNode("public class B extends A {public
> function B() {}; override public function set foo(value:Object):void
> {super.foo = value;};} class A {public function set foo(value:Object):void
> {}}");
>          asBlockWalker.visitClass(node);
>          String expected = "/**\n * @constructor\n * @extends
> {org.apache.royale.A}\n */\norg.apache.royale.B = function() {\n
> org.apache.royale.B.base(this,
> 'constructor');\n};\ngoog.inherits(org.apache.royale.B,
> org.apache.royale.A);\n\n\n" +
> -                "/**\n * @nocollapse\n * @export\n * @type {Object}\n
> */\norg.apache.royale.B.prototype.foo;\n\n\n" +
>                  "org.apache.royale.B.prototype.set__foo = function(value)
> {\n  org.apache.royale.B.superClass_.set__foo.apply(this, [ value]
> );\n};\n\n\n" +
>                  "Object.defineProperties(org.apache.royale.B.prototype,
> /** @lends {org.apache.royale.B.prototype} */ {\n/**\n * @type {Object}\n
> */\nfoo: {\nset: org.apache.royale.B.prototype.set__foo}}\n);";
>          assertOut(expected);
> diff --git
> a/compiler-jx/src/test/resources/royale/projects/super/Base_result.js
> b/compiler-jx/src/test/resources/royale/projects/super/Base_result.js
> index 169067a..fcaa48c 100644
> --- a/compiler-jx/src/test/resources/royale/projects/super/Base_result.js
> +++ b/compiler-jx/src/test/resources/royale/projects/super/Base_result.js
> @@ -35,14 +35,6 @@ Base = function() {
>  goog.inherits(Base, Super);
>
>
> -/**
> - * @nocollapse
> - * @export
> - * @type {string}
> - */
> -Base.prototype.text;
> -
> -
>  Base.prototype.get__text = function() {
>    return "A" + Base.superClass_.get__text.apply(this);
>  };
>
>

Reply via email to