royale-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alex Harui <aha...@adobe.com.INVALID>
Subject Re: Royale compiler not handling Date.fullYear etc
Date Mon, 02 Jul 2018 15:36:25 GMT
Hi Andrew,

I agree with your conclusions.  Upon further investigation, Google Closure just closed those
bugs as duplicates and the original issue remains open.  So adding some field to the ExternCConfig
sounds like the easiest path for now.

Thanks,
-Alex

´╗┐On 7/2/18, 2:14 AM, "Frost, Andrew" <Andrew.Frost@Harman.com> wrote:

    Hi
    
    Even if we get the latest version, they still don't support a "readonly" annotation:
    https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fgoogle%2Fclosure-compiler%2Fblob%2Fmaster%2Fsrc%2Fcom%2Fgoogle%2Fjavascript%2Fjscomp%2Fparsing%2FAnnotation.java&data=02%7C01%7Caharui%40adobe.com%7C0dc93c78a4d94e4d834208d5dffc3e41%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636661196836331609&sdata=BIj4sZ6G1RxdVqd2nGYA9do9hujoNGjCRrrIXqY%2Bw5Y%3D&reserved=0
    lists the ones that are supported.
    
    And I tried updating to the newest jar file, got some weird errors so I think that way
would lead to a lot more work.
    
    We do get the original Javadoc comment come into our parsing function, so we could do
a free-text search for "@readonly" within this? Might be the simplest route until(?) Google
add this support properly..
    
    Alternatively you suggest using ExternCConfiguration: having looked into this, I think
the process would be:
    1) update the externc-config.xml file to include something like <field-readonly><class>Date</class><name>timezoneOffset</name></field-readonly>
 (so a bit like the "exclude" list where we don't include things like Array.toSource etc)
    2) update the ExternCConfiguration.java file to handle this input field (like the "setExcludes"
handler for the "exclude" mapping..) and put in place a new "ReadOnlyMember" element and list
to hold this
    3) in the FieldReference.emit() method, check if this element is in the read-only property
list; if it is, treat it like an accessor but only with a 'get' method.
    
    So I'm thinking this latter approach is a nicer one and fits more with the rest of how
things are done... I'll update the pull requests later on after running through this and also
adding some tests...
    
    cheers
    
       Andrew
    
    
    
    -----Original Message-----
    From: Alex Harui [mailto:aharui@adobe.com.INVALID] 
    Sent: 30 June 2018 08:00
    To: dev@royale.apache.org
    Subject: [EXTERNAL] Re: Royale compiler not handling Date.fullYear etc
    
    Yeah, if @const becomes const in AS, that probably isn't right.
    
    I just found this:  https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fclicktime.symantec.com%2Fa%2F1%2Ft8Vt55bBn1paDRrO_tcbGDw3nOt-4_f5sEQDRaUr78g%3D%3Fd%3DlhuiLMs5TaNvmSSfvLo9lH6b9M276TThXO9G60OlPgOsY0f-pENIrkec8khCn7viL4JfEe1fvVJVaiQjQh6BhiYP7C_O9RJunWasASxHmucjtZZQBzeCviGxf9lrMUs0zxkkPNHBoa_rq8QP8-dOSlYcneP9T5WOgjLYBeKsizspqFWxP5Szt6O6GYS1QKNQtcPvwoh5NSOLYeB2K3j46FeJyZfM51xmX1vX4JN5xKWd1bY4mOAK6B9Kr7A2u6i65r0iSYuzL8ok9ybJXKGK43QxR_nahrgFSKD2Bg-0iMujUKi8uNlZ5kOGSzi3oE0ByPCbF-4QIZtYTa3V7zQ1AaG4rQnICykl0p0UohbhZZrVdA2536sB_xwCGEKvHXA0Hf5_evEzsPHoxiB5HeyKiYQW8vzNmmT3CSqOKfakgQLVd_4l5xMYQ7AeYmU%253D%26u%3Dhttps%253A%252F%252Fgithub.com%252Fgoogle%252Fclosure-compiler%252Fissues%252F139&data=02%7C01%7Caharui%40adobe.com%7C0dc93c78a4d94e4d834208d5dffc3e41%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636661196836331609&sdata=Q1nsKBTBxFETuFUUOXqMjx8v7E96Y0pZGBpQ%2BNWEwzI%3D&reserved=0
    I don't have time to investigate further right now, so if you have time that would be
great.  We might need to update which version of Google Closure we are using.
    
    Regarding the options you listed, I think goal is to generate the right AS classes with
a getter and no setter.  If Google still doesn't handle @readonly, we could add a field to
the ExternCConfiguration where you specify which APIs should be read-only and correct the
output AS.
    
    HTH,
    -Alex
    
    On 6/29/18, 11:46 PM, "Frost, Andrew" <Andrew.Frost@Harman.com> wrote:
    
        Hi
        
        Well @const is at least supported by the Google classes; with a slight change in FieldReference.java
to actually set the internal flag ("this.isConst = comment.hasConstAnnotation();") then it
changes the ActionScript declaration so that it's now:
        public const timezoneOffset:Number = 13;
        
        And when you try to assign to it, you get 
        TestRoyale.mxml(38): col: 5 Error: Illegal assignment to a variable specified as constant.
        				date.timezoneOffset = 55;
        				^
        
        So it kind of works in flagging up a bad bit of code, but from an ActionScript perspective
it's not right, we should be getting an "AssignToReadOnlyPropertyProblem" rather than an "AssignToConstProblem".
        
        The options as I see it:
        1) live with this as a slightly incorrect warning, as it's very unlikely to happen
(shouldn't occur in the AS3 code to start with assuming that compiles already in Flex) and
it's the simplest/most elegant change
        2) have specific code in the SemanticUtils class which knows about this particular
Date property and is looking out for it by name ... not very efficient and something of a
hack!
        3) extend the closure compiler to support some of the other JSDoc annotations so that
we can generate property getters/setters and create read-only properties. Possibly the most
"correct" solution but not so good from a maintainability perspective if we have to change
the Google code...
        
        
        In terms of testing: as you said, the 'missing.js' in the royale-compiler folders
is for the compiler's testing, so if we add extra testing for the compiler with these new
properties then we need that file to also include those extra Date things. I guess it's not
a massive maintenance issue if these files are hardly ever changing.. I just wanted to be
sure I wasn't missing some step in the process that did an automatic sync from one to the
other. The same is true of the js.swc, it's being generated in the royale-typedefs folder
and currently I'm manually copying it to the royale-asjs folder... but for that one, there
must be something that copies it over, as that js/lib folder doesn't exist in the original
source!
        
        
        thanks
        
           Andrew
        
        
        
        -----Original Message-----
        From: Alex Harui [mailto:aharui@adobe.com.INVALID] 
        Sent: 30 June 2018 07:19
        To: dev@royale.apache.org
        Subject: [EXTERNAL] Re: Royale compiler not handling Date.fullYear etc
        
        Interesting.  In https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fclicktime.symantec.com%2Fa%2F1%2FJu25ASzndEjOnqYNDd7xiVeQME8ALGuo-URR6C3y6WI%3D%3Fd%3DlhuiLMs5TaNvmSSfvLo9lH6b9M276TThXO9G60OlPgOsY0f-pENIrkec8khCn7viL4JfEe1fvVJVaiQjQh6BhiYP7C_O9RJunWasASxHmucjtZZQBzeCviGxf9lrMUs0zxkkPNHBoa_rq8QP8-dOSlYcneP9T5WOgjLYBeKsizspqFWxP5Szt6O6GYS1QKNQtcPvwoh5NSOLYeB2K3j46FeJyZfM51xmX1vX4JN5xKWd1bY4mOAK6B9Kr7A2u6i65r0iSYuzL8ok9ybJXKGK43QxR_nahrgFSKD2Bg-0iMujUKi8uNlZ5kOGSzi3oE0ByPCbF-4QIZtYTa3V7zQ1AaG4rQnICykl0p0UohbhZZrVdA2536sB_xwCGEKvHXA0Hf5_evEzsPHoxiB5HeyKiYQW8vzNmmT3CSqOKfakgQLVd_4l5xMYQ7AeYmU%253D%26u%3Dhttps%253A%252F%252Fna01.safelinks.protection.outlook.com%252F%253Furl%253Dhttps%25253A%25252F%25252Fclicktime.symantec.com%25252Fa%25252F1%25252FuntftVdsWwPmiJWAVt3nm3wg6v4ZACZ9RDBNQjuszM0%25253D%25253Fd%25253DbbejT-O_-jFYytoEIpecgb-HW7JAfVy-JYJKJjpirj9WyJta8y-Vetrzg91hMyjxwIZDBbGoPRETuW8R-_GJ2QI3JFRNDooGe4nnEJmgsbOCgX9zvdpNOtRejsS_vQ96JFtVBei96NlGXnAeb9O-n2UPHrthFwLfNhxhivyLhutMuYZf1_Bwf9uhuogWi4XEGnREN0VeGK-7HR-0IXBlFkwvMeyJ_r7KS89xbvNmYhN1EFExUVrPWOSGUU7bDbqQGwx_iQnLVTX8Lj1IsNPJvd8qUgJnR5R6P-smt5q_FBaLNjsRWDWI0U_XMUyRIY_5-Kz1H2BKLxZppDcoEdbSVn_k9bD-Eo7722e3Jajt9nKt5EOvpU8kzNvIgbQxRNW4JbQ0gyaaZG-838aZUMmtuoW39NTiDdhoowZejUVmDmKstEs8NbBBtOnE3Ck%2525253D%252526u%25253Dhttps%2525253A%2525252F%2525252Fgithub.com%2525252Fgoogle%2525252Fclosure-compiler%2525252Fwiki%2525252FAnnotating-JavaScript-for-the-Closure-Compiler%2526data%253D02%25257C01%25257Caharui%252540adobe.com%25257Cb7acdaa31fda47dff0c508d5de553e2e%25257Cfa7b1b5a7b34438794aed2c178decee1%25257C0%25257C1%25257C636659380078411962%2526sdata%253DoCQLuHrDHlf9BSbfTC%25252F2UJgSlMLkKV2kNQ0z3FX%25252F%25252Bxk%25253D%2526reserved%253D0&data=02%7C01%7Caharui%40adobe.com%7C0dc93c78a4d94e4d834208d5dffc3e41%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636661196836331609&sdata=PBh%2FjWVqmzlXW4l%2FO5nWwXU3qReFcOr19%2BEPKoyXHuo%3D&reserved=0
it mentions @const as the annotation.  That might already work and if not, we should probably
make it work.
        
        The missing.js in royale-compiler is just there to get the compiler's tests to run.
 The royale-typedefs repo has a dependency on royale-compiler, so we can't create a circular
dependency by having royale-compiler require royale-typedefs missing.js.  They don't need
to be kept in sync.  The royale-compiler version should be minimal.  The one in royale-typedefs
is intended to make a library with the right and complete Browser APIs.
        
        HTH,
        -Alex
        
        On 6/29/18, 3:55 PM, "Frost, Andrew" <Andrew.Frost@Harman.com> wrote:
        
            Hi
            
            Those date tests already test the mapping, and are running fine. They're not getting
stuck at the earlier stage which is where the original problem lay. So I'd been thinking of
adding a new test file under the below folder, where other AS-specific testing is happening:
            royale-compiler/compiler/src/test/java/as
            
            In terms of the read-only properties, I would have hoped that the definition in
missing.js could be written:
            /**
             * @type {number}
             * @property
             * @readonly
             */
            Date.prototype.timezoneOffset;
            
            but the JSDoc parser isn't able to pick up/report upon the 'property' or 'readonly'
usage. We could add support for these perhaps, manually within the FieldReference.java file
(which is where these properties are coming in currently) we could manually look for the "@property"
and/or "@readonly" tags within the comment.getOriginalCommentString() value; I would have
preferred to be able to call "comment.isReadOnly" or similar, but to get to that requires
changing Google's code..
            
            So yes, hold off doing anything with the pull requests for now, I'll see whether
I can get it to do things from the typedefs side of things...
            
            One extra note: I'm finding two "missing.js" files which aren't being kept in
sync at all (by the build tools); is this by design or should there be some kind of a link
between them?
            royale-typedefs\js\src\main\javascript\missing.js
            royale-compiler\compiler-externc\src\test\resources\typedefs\unit_tests\missing.js
            
            
            thanks
            
               Andrew
            
            
            
            -----Original Message-----
            From: Alex Harui [mailto:aharui@adobe.com.INVALID] 
            Sent: 29 June 2018 17:38
            To: dev@royale.apache.org
            Subject: [EXTERNAL] Re: Royale compiler not handling Date.fullYear etc
            
            There are Date tests in TestRoyaleGlobalClasses.java
            
            In this case, the issue may be in how to set up a copy of the tests to work with
js.swc instead of playerglobal.swc.
            
            Regarding read-only properties, I think the externc compiler might have a way
of doing that.  It would likely involve one of the JSDoc annotations or an interface.  And
the result should be a getter without a setter.  I don't have time to look for it right now.
 It would be best to deal with this in the typedefs instead of in the compiler, IMO.
            
            My 2 cents,
            -Alex
            
            On 6/29/18, 7:47 AM, "Frost, Andrew" <Andrew.Frost@Harman.com> wrote:
            
                ".. not yet" is probably the most appropriate response!!
                
                I had wondered whether it would need some formal self-tests adding, I'll have
a dig around to see how to do this bit :-)
                
                thanks
                
                   Andrew
                
                
                -----Original Message-----
                From: Harbs [mailto:harbs.lists@gmail.com] 
                Sent: 29 June 2018 13:35
                To: dev@royale.apache.org
                Subject: [EXTERNAL] Re: Royale compiler not handling Date.fullYear etc
                
                Cool. Are there compiler tests for these Date additions?
                
                
            
            
        
        
    
    

Mime
View raw message