You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@flex.apache.org by Greg Dove <gr...@apache.org> on 2017/03/03 05:09:41 UTC

[FlexJS] Working to fix a compiler bug - problem with the Problems

I have been looking into FLEX-35273 [1].

This is a compiler bug where it is possible to do things that don't make sense, like:

<js:initialView>
	<local:MyInitialView id="view1" />
	<local:MyInitialView id="view2" />
</js:initialView>

or even this :

<js:initialView>
	<js:SimpleCSSValuesImpl />
</js:initialView>

Neither of the above caused compile time errors.

I have a fix for both the above scenarios.

For the first one, I used MXMLDuplicateChildTagProblem which seems 'close enough'

"Child tag '${childTag}' bound to namespace '${childNamespace}' is already specified for element '${element}'. It will be ignored.";

This renders as :
Child tag 'MyInitialView' bound to namespace '*' is already specified for element 'js:initialView'. It will be ignored.
   <local:MyInitialView id="view2" />
    ^

in the first example above. The text does not feel entirely right, but seems close enough. "It will be ignored" sounds more like a warning (as opposed to an error/failure), which I think a) it should not be and b) it is not. 

For the second one, I could not find any relevant 'Problem' class (I may have missed one perhaps?) So I just made a new one. I don't really know what the rules are here.
These Problems seem to include an error code so I am unclear what to do if I add a new one. i.e. it is not clear what new error code I should apply (or even if the error code is important for something).
For now I just added an arbitrary errorCode = 1999 on MXMLBadChildTagPropertyAssignmentProblem.

The new problem renders out to :
In initializer for 'js:initialView', type org.apache.flex.core.SimpleCSSValuesImpl is not assignable to target type 'org.apache.flex.core.IApplicationView'. 

Alex, you may be able to advise here.... are there any rules I need to follow for the CompilerProblem classes (in particular, when adding a new class).

Assuming all is well above, I will commit this fix tomorrow. I was also trying to see if I could figure out how to get checkintests running, but I so far I did not. However the regular build tests are all fine with the changes I made for this.


1. https://issues.apache.org/jira/browse/FLEX-35273

Re: [FlexJS] Working to fix a compiler bug - problem with the Problems

Posted by Greg Dove <gr...@gmail.com>.

Thanks Alex, much appreciated


On Wed, Mar 8, 2017 at 7:24 PM, Alex Harui <ah...@adobe.com> wrote:

> OK.  I'll take care of it.
>
> On 3/7/17, 10:11 PM, "Greg Dove" <gr...@gmail.com> wrote:
>
> >Yes, I think they are errors with the tests.
> >
> >iirc it was
> >
> ><initialView>
> ><Label>
> ><initialView>
> >
> >or something like that. So I think the new code is picking up the right
> >things :)
> >
> >I am struggling for time right now, if you have time that would be
> >fantastic, otherwise I will try to get to it later tonight.
> >
> >
> >
> >
> >
> >
> >
> >On Wed, Mar 8, 2017 at 7:06 PM, Alex Harui <ah...@adobe.com> wrote:
> >
> >> Greg,
> >>
> >> I just realized that these might be legitimate errors.  I can fix them
> >>if
> >> you don't have time.
> >>
> >> -Alex
> >>
> >> On 3/7/17, 1:32 PM, "Alex Harui" <ah...@adobe.com> wrote:
> >>
> >> >
> >> >
> >> >On 3/7/17, 1:04 PM, "Greg Dove" <gr...@gmail.com> wrote:
> >> >
> >> >>I had never run those tests specifically - I was not even aware of
> >>those
> >> >>yet, the unit tests run during ant all build had proceeded without
> >> >>problems:
> >> >
> >> >There are three sets of unit tests.  Because the compiler is needed to
> >> >build the AS framework, we can't have the compiler tests require the AS
> >> >framework, so the set of tests that runs automatically don't have any
> >>AS
> >> >framework dependencies, then there is a set of tests with flex-sdk
> >> >dependencies and one with flex-asjs dependencies, which is what you
> >>just
> >> >ran.
> >> >
> >> >Ignore the "Unable to parse" for now.
> >> >
> >> >IMO, no need to revert the changes.  I've got plenty of other things to
> >> >do.
> >> >
> >> >Thanks for digging into it.
> >> >-Alex
> >> >
> >>
> >>
>
>

Re: [FlexJS] Working to fix a compiler bug - problem with the Problems

Posted by Alex Harui <ah...@adobe.com>.

OK.  I'll take care of it.

On 3/7/17, 10:11 PM, "Greg Dove" <gr...@gmail.com> wrote:

>Yes, I think they are errors with the tests.
>
>iirc it was
>
><initialView>
><Label>
><initialView>
>
>or something like that. So I think the new code is picking up the right
>things :)
>
>I am struggling for time right now, if you have time that would be
>fantastic, otherwise I will try to get to it later tonight.
>
>
>
>
>
>
>
>On Wed, Mar 8, 2017 at 7:06 PM, Alex Harui <ah...@adobe.com> wrote:
>
>> Greg,
>>
>> I just realized that these might be legitimate errors.  I can fix them
>>if
>> you don't have time.
>>
>> -Alex
>>
>> On 3/7/17, 1:32 PM, "Alex Harui" <ah...@adobe.com> wrote:
>>
>> >
>> >
>> >On 3/7/17, 1:04 PM, "Greg Dove" <gr...@gmail.com> wrote:
>> >
>> >>I had never run those tests specifically - I was not even aware of
>>those
>> >>yet, the unit tests run during ant all build had proceeded without
>> >>problems:
>> >
>> >There are three sets of unit tests.  Because the compiler is needed to
>> >build the AS framework, we can't have the compiler tests require the AS
>> >framework, so the set of tests that runs automatically don't have any
>>AS
>> >framework dependencies, then there is a set of tests with flex-sdk
>> >dependencies and one with flex-asjs dependencies, which is what you
>>just
>> >ran.
>> >
>> >Ignore the "Unable to parse" for now.
>> >
>> >IMO, no need to revert the changes.  I've got plenty of other things to
>> >do.
>> >
>> >Thanks for digging into it.
>> >-Alex
>> >
>>
>>

Re: [FlexJS] Working to fix a compiler bug - problem with the Problems

Posted by Greg Dove <gr...@gmail.com>.

Yes, I think they are errors with the tests.

iirc it was

<initialView>
<Label>
<initialView>

or something like that. So I think the new code is picking up the right
things :)

I am struggling for time right now, if you have time that would be
fantastic, otherwise I will try to get to it later tonight.







On Wed, Mar 8, 2017 at 7:06 PM, Alex Harui <ah...@adobe.com> wrote:

> Greg,
>
> I just realized that these might be legitimate errors.  I can fix them if
> you don't have time.
>
> -Alex
>
> On 3/7/17, 1:32 PM, "Alex Harui" <ah...@adobe.com> wrote:
>
> >
> >
> >On 3/7/17, 1:04 PM, "Greg Dove" <gr...@gmail.com> wrote:
> >
> >>I had never run those tests specifically - I was not even aware of those
> >>yet, the unit tests run during ant all build had proceeded without
> >>problems:
> >
> >There are three sets of unit tests.  Because the compiler is needed to
> >build the AS framework, we can't have the compiler tests require the AS
> >framework, so the set of tests that runs automatically don't have any AS
> >framework dependencies, then there is a set of tests with flex-sdk
> >dependencies and one with flex-asjs dependencies, which is what you just
> >ran.
> >
> >Ignore the "Unable to parse" for now.
> >
> >IMO, no need to revert the changes.  I've got plenty of other things to
> >do.
> >
> >Thanks for digging into it.
> >-Alex
> >
>
>

Re: [FlexJS] Working to fix a compiler bug - problem with the Problems

Posted by Alex Harui <ah...@adobe.com>.

Greg,

I just realized that these might be legitimate errors.  I can fix them if
you don't have time.

-Alex

On 3/7/17, 1:32 PM, "Alex Harui" <ah...@adobe.com> wrote:

>
>
>On 3/7/17, 1:04 PM, "Greg Dove" <gr...@gmail.com> wrote:
>
>>I had never run those tests specifically - I was not even aware of those
>>yet, the unit tests run during ant all build had proceeded without
>>problems:
>
>There are three sets of unit tests.  Because the compiler is needed to
>build the AS framework, we can't have the compiler tests require the AS
>framework, so the set of tests that runs automatically don't have any AS
>framework dependencies, then there is a set of tests with flex-sdk
>dependencies and one with flex-asjs dependencies, which is what you just
>ran.
>
>Ignore the "Unable to parse" for now.
>
>IMO, no need to revert the changes.  I've got plenty of other things to
>do.
>
>Thanks for digging into it.
>-Alex
>

Re: [FlexJS] Working to fix a compiler bug - problem with the Problems

Posted by Alex Harui <ah...@adobe.com>.

On 3/7/17, 1:04 PM, "Greg Dove" <gr...@gmail.com> wrote:

>I had never run those tests specifically - I was not even aware of those
>yet, the unit tests run during ant all build had proceeded without
>problems:

There are three sets of unit tests.  Because the compiler is needed to
build the AS framework, we can't have the compiler tests require the AS
framework, so the set of tests that runs automatically don't have any AS
framework dependencies, then there is a set of tests with flex-sdk
dependencies and one with flex-asjs dependencies, which is what you just
ran.

Ignore the "Unable to parse" for now.

IMO, no need to revert the changes.  I've got plenty of other things to do.

Thanks for digging into it.
-Alex

Re: [FlexJS] Working to fix a compiler bug - problem with the Problems

Posted by Greg Dove <gr...@gmail.com>.

I had never run those tests specifically - I was not even aware of those
yet, the unit tests run during ant all build had proceeded without problems:

I am seeing this:

    [junit] Running
org.apache.flex.compiler.internal.codegen.js.flexjs.TestFlexJSFile
    [junit] Unable to parse
D:\FLEXSDKS\_asf\flex-asjs\frameworks\as\basic-manifest.xml
    [junit] Tests run: 3, Failures: 0, Errors: 0, Skipped: 0, Time elapsed:
4.76 sec
    [junit] Running
org.apache.flex.compiler.internal.codegen.mxml.flexjs.TestFlexJSMXMLApplication
    [junit] Unable to parse
D:\FLEXSDKS\_asf\flex-asjs\frameworks\as\basic-manifest.xml
    [junit] In initializer for 'basic:initialView', type
org.apache.flex.html.Label is not assignable to target type
'org.apache.flex.core.IApplicationView'.
    [junit] Unable to parse
D:\FLEXSDKS\_asf\flex-asjs\frameworks\as\basic-manifest.xml
    [junit] Unable to parse
D:\FLEXSDKS\_asf\flex-asjs\frameworks\as\basic-manifest.xml
    [junit] In initializer for 'basic:initialView', type
org.apache.flex.html.Label is not assignable to target type
'org.apache.flex.core.IApplicationView'.
    [junit] Unable to parse
D:\FLEXSDKS\_asf\flex-asjs\frameworks\as\basic-manifest.xml
    [junit] In initializer for 'basic:initialView', type
org.apache.flex.html.Label is not assignable to target type
'org.apache.flex.core.IApplicationView'.
    [junit] Unable to parse
D:\FLEXSDKS\_asf\flex-asjs\frameworks\as\basic-manifest.xml
    [junit] In initializer for 'basic:initialView', type
org.apache.flex.html.Label is not assignable to target type
'org.apache.flex.core.IApplicationView'.

I am not sure what 'Unable to parse
D:\FLEXSDKS\_asf\flex-asjs\frameworks\as\basic-manifest.xml' is yet.

but the errors are related to my changes, but also seems like they found
something valid. I can look into this in a about 5-6 hours time. Do you
need me to revert the commit until I get a chance to address those tests?


On Wed, Mar 8, 2017 at 9:53 AM, Alex Harui <ah...@adobe.com> wrote:

>
>
> On 3/7/17, 9:41 AM, "Greg Dove" <gr...@gmail.com> wrote:
>
> >Anything else I should be doing?
>
> If you are using Ant to build flex-falcon, please run
>
>    ant flexjs.dependent.tests
>
> This may require setting up an ASJS_HOME environment variable or setting
> in env.properties to point to the flex-asjs repo that has previously been
> built so it as the SWCs.
>
> The flex-falcon build just failed.  Not sure if it is due to your changes.
>
> Thanks,
> -Alex
>
>

Re: [FlexJS] Working to fix a compiler bug - problem with the Problems

Posted by Alex Harui <ah...@adobe.com>.


On 3/7/17, 9:41 AM, "Greg Dove" <gr...@gmail.com> wrote:

>Anything else I should be doing?

If you are using Ant to build flex-falcon, please run

   ant flexjs.dependent.tests

This may require setting up an ASJS_HOME environment variable or setting
in env.properties to point to the flex-asjs repo that has previously been
built so it as the SWCs.

The flex-falcon build just failed.  Not sure if it is due to your changes.

Thanks,
-Alex

Re: [FlexJS] Working to fix a compiler bug - problem with the Problems

Posted by Greg Dove <gr...@gmail.com>.

Anything else I should be doing?

One thing I did not check (and generally have not checked specifically with
anything I did so far in FlexJS) is anything to do with inline
(fx:Component) mxml.

I *think* there are some unit tests for these, I would need to double
check.... but if you are using that anywhere it would just be helpful to
know that things work correctly inside that. I was intending to come back
and check this explicitly and also maybe add some new unit tests to the
compiler for these new errors.

Aside from that, no, there is nothing specific, apart from making sure that
your app compiles and runs as expected.
In theory, if you got errors on re-compile with the new code, it should
only be helping you. If you get no errors, then you can be more reassured
that your mxml is good (which I am sure it was anyway) :)



On Mon, Mar 6, 2017 at 10:07 PM, Harbs <ha...@gmail.com> wrote:

> I have noticed that MXML compiling does not produce the greatest error
> reports. I should have recorded issues as I saw them, but I didn’t. :-(
>
> These two cases make sense to me, and thanks for fixing them. I’ll be
> happy to update falcon and make sure that things still compile correctly in
> my apps. Anything else I should be doing?
>
> > On Mar 6, 2017, at 10:40 AM, Greg Dove <gr...@gmail.com> wrote:
> >
> > "It sound to me like you know what you’re talking about"
> >
> > Haha, Harbs that's probably more reassuring to me than it should be to
> you.
> > In terms of anything at all in the compiler, I am still LAYG (learning as
> > you go) here ;)
> > (btw, you can never have too many 'as you go' acronyms)
> >
> > I actually shared Alex's general concerns over making changes in the mxml
> > parsing, we don't have a large range of tests to cover all the potential
> > mxml variations (otherwise I expect this would have already been
> addressed).
> >
> > Harbs, I understand that you have a (quite possibly) large codebase so
> you
> > might provide the best real-world 'litmus test' in you have a large
> amount
> > of mxml.
> >
> >
> > In simple terms the changes will prevent the following 2 examples from
> > compiling, which currently compile fine in FlexJS, but should not:
> >
> > <js:initialView>
> >        <local:MyInitialView id="view1" />
> >        <local:MyInitialView id="view2" />
> > </js:initialView>
> >
> > in the above example the view2 instance was being created and assigned
> for
> > the initialView propertty, the first was ignored
> >
> > <js:initialView>
> >        <js:SimpleCSSValuesImpl />
> > </js:initialView>
> >
> > In this case the types are incompatible.
> >
> > I actually think the compiler errors for mxml could be more important
> than
> > a sesaoned Flex user would assume in terms of how new people will form
> > early impressions. When people who potentially don't know actionscript
> > (yet) take some FlexJS examples for a spin, they are probably going to
> look
> > at the mxml and think to themselves 'I can do that', so it will be the
> > first thing they mess with. Mxml-related errors might be the first thing
> > they see from the compiler after they start messing with stuff.
> > That's why I spent a bit of time discussing those...I have gone ahead and
> > gone with the two new ones as I suggested.
> >
> >
> >
> > On Mon, Mar 6, 2017 at 12:08 AM, Harbs <ha...@gmail.com> wrote:
> >
> >> I have to admit that you lost me pretty quickly… ;-)
> >>
> >> It sound to me like you know what you’re talking about, so I’m fine with
> >> whatever you did/want to do… :-)
> >>
> >> Harbs
> >>
> >>> On Mar 5, 2017, at 9:31 AM, Greg Dove <gr...@gmail.com> wrote:
> >>>
> >>> OK.... please brace yourself for an onslaught of text.
> >>>
> >>> I tested across Flex 4 and FlexJS through a bunch of variations. This
> >>> change passes all tests. In the end this is a very small and isolated
> >> code
> >>> change (I am almost certain it will not contribute to anyone's merge
> >>> conflicts) and I believe it gives greater compile-time type safety to
> >> mxml,
> >>> so I think it's necessary. But the error descriptions are open for more
> >>> input/review... I have favored a strong consistency with the Flex 4
> >>> descriptions, which were specific for mxml, over using the related
> >>> actionscript descriptions.
> >>>
> >>> BTW I had forgotten, but there was also a comment (it was not marked
> as a
> >>> todo) in the falcon MXMLPropertySpecifierNode class that indicated that
> >>> this type of work still needed to be done, so I expect it was an
> >> unfinished
> >>> implementation.
> >>>
> >>> If I hear nothing conflciting by EOD tomorrow I will commit what I
> have.
> >> It
> >>> can easily be reverted if anyone finds issues, or I am happy to address
> >> it
> >>> further if it is not correct for some cases that I did not anticipate.
> >>>
> >>> That's the end of the preamble.... best of luck getting through what
> >>> follows... it took me longer than the original coding part :)
> >>>
> >>> Test setup for Flex 4:
> >>> 'TestView' below is simply a Flex4 Group mxml component, like so:
> >>> <s:Group xmlns:fx="http://ns.adobe.com/mxml/2009"
> >>>        xmlns:s="library://ns.adobe.com/flex/spark"
> >>>        creationComplete="onCreated()">
> >>>   <fx:Script><![CDATA[
> >>>       import spark.components.Alert;
> >>>       private function onCreated():void{
> >>>         //check for actionscript version of error
> >>>         //this.number="something";
> >>>       }
> >>>       public var testArr:Array;
> >>>       public var number:Number;
> >>>       ]]></fx:Script>
> >>>   <s:layout>
> >>>       <s:VerticalLayout/>
> >>>   </s:layout>
> >>>   <s:Button click="test.text='testArr ['+testArr.toString()+'],\
> nnumber
> >>> val:'+number" width="100" height="50"/>
> >>>   <s:Label id="test"/>
> >>> </s:Group>
> >>>
> >>> Test setup for FlexJS:
> >>> 'TestView' below is simply a Flex4 Group like so:
> >>> <js:View xmlns:fx="http://ns.adobe.com/mxml/2009"
> >>> xmlns:js="library://ns.apache.org/flexjs/basic"
> >>> initComplete="onCreated()">
> >>>  <fx:Script><![CDATA[
> >>> import org.apache.flex.html.Alert;
> >>>       private function onCreated():void{
> >>>         //check for actionscript version of error
> >>>         //this.number="something";
> >>>       }
> >>>       public var testArr:Array;
> >>>       public var number:Number;
> >>>       ]]></fx:Script>
> >>> <js:Button click="Alert.show('testArr ['+testArr.toString()+'],\
> nnumber
> >>> val:'+number, this)" width="100" height="50"/>
> >>> </js:View>
> >>>
> >>>
> >>> Testing:
> >>>   <local:TestView id="test">
> >>>       <local:testArr>
> >>>           <fx:Number>23.56</fx:Number>
> >>>           <fx:Number>23.99</fx:Number>
> >>>           <fx:String>Something else</fx:String>
> >>>       </local:testArr>
> >>>       <local:number>
> >>>           <fx:Number>23.56</fx:Number>
> >>>       </local:number>
> >>>   </local:TestView>
> >>> The above compiles correctly and because the testArr property has an
> >>> 'Array' type, the children are inferred as array elements, it is the
> same
> >>> as specifying:
> >>>       <local:testArr>
> >>>           <fx:Array>
> >>>               <fx:Number>23.56</fx:Number>
> >>>               <fx:Number>23.99</fx:Number>
> >>>               <fx:String>Something else</fx:String>
> >>>           </fx:Array>
> >>>       </local:testArr>
> >>> Note: Using one vs. multiple children: the inference is the same - if
> the
> >>> property being initialized is of type Array, it infers correctly (and
> >>> performs the 'Array' wrapping if needed) irrespective of single or
> >> multiple
> >>> child tags.
> >>> NO BUGS FOR ARRAY (inferred or explicit)
> >>> This works in both Flex 4 and Falcon (before or after my local changes)
> >>> Multiple value children when the property being initialized is not
> >> 'Array'
> >>> type
> >>> ---------------------------------------------------------------------
> >>> the 'number' property on the other hand is of type Number, and changing
> >>> number assignment to multiple values, in this way:
> >>>  <local:number>
> >>>           <fx:Number>23.56</fx:Number>
> >>> <fx:Number>23.99</fx:Number>
> >>>       </local:number>
> >>> gives Flex 4 compiler errors like so:
> >>> Error:(20, 0) [flex4testforFlexJS]: In initializer for 'number',
> multiple
> >>> initializer values for target type Number.
> >>>   Error:(21, 0) [flex4testforFlexJS]: In initializer for 'number',
> >>> multiple initializer values for target type Number.
> >>> (adding more than 2 gives an individual error for each child)
> >>> <fx:Number>23.56</fx:Number>
> >>>               <fx:Number>23.99</fx:Number>
> >>>           <fx:String>Something else</fx:String>
> >>>       </local:number>
> >>> Swapping to one element with wrong type in multiple child assignment
> does
> >>> not change the error reported (it remains simply 'multiple values'):
> >>> Error:(20, 0) [flex4testforFlexJS]: In initializer for 'number',
> multiple
> >>> initializer values for target type Number.
> >>> Error:(21, 0) [flex4testforFlexJS]: In initializer for 'number',
> multiple
> >>> initializer values for target type Number.
> >>> CURRENT BUG
> >>> -----------
> >>> Currently in FlexJS, the following happens for either of the above
> >>> scenarios:
> >>> Only the last child tag is processed in multiple tags like this, the
> >>> earlier ones are ignored. It is processed as if it were the only child
> >> tag
> >>> (and it may be of an incompatible type).
> >>> PROPOSED FIX(DONE)
> >>> -----------------
> >>> Please let me know if you agree, or suggest alternative.
> >>> in the local fix for flexjs, I have now created a new Problem,
> >>> MXMLMultipleInitializersProblem, which follows the Flex 4 example
> >>> with outputs descriptions similar to:
> >>> GenericTests.mxml(57): col: 17 Error: In initializer for 'local:number'
> >>> multiple initializer values are not permitted for target type 'Number'.
> >>>               <fx:Number>23.99</fx:Number>
> >>>               ^
> >>> GenericTests.mxml(58): col: 13 Error: In initializer for 'local:number'
> >>> multiple initializer values are not permitted for target type 'Number'.
> >>>           <fx:String>Something else</fx:String>
> >>>           ^
> >>> Inappropriate single child when property being initialized is not
> 'Array'
> >>> type
> >>> ------------------------------------------------------------
> >> ----------------
> >>> <local:number>
> >>>       <fx:String>Something else</fx:String>
> >>>   </local:number>
> >>> in Flex 4, this gives:
> >>> Error:(22, 0) [flex4testforFlexJS]: In initializer for 'number', type
> >>> String is not assignable to target type 'Number'.
> >>> The corresponding Flex 4 actionscript error is
> >>> Error:(11, 0) [flex4testforFlexJS]: Error code: 1067: Implicit coercion
> >> of
> >>> a value of type String to an unrelated type Number.
> >>> If FlexJS, the actionscript error is the same
> >>> CURRENT BUG
> >>> -----------
> >>> in Falcon this currently does not cause a compiler error for mxml,
> >>> compilation completes normally, and in js the value of number will be
> the
> >>> "Something else", in swf it is NaN. IMO this is clearly wrong.
> >>>
> >>>
> >>> PROPOSED FIX (DONE)
> >>> -------------------
> >>> Please let me know if you agree, or suggest alternative.
> >>> I have added a new Problem to be closer to what was shown before in
> Flex
> >> 4:
> >>> This is the new one I mirrored from Flex 4 (with same description) in
> >> FlexJS
> >>>   GenericTests.mxml(56): col: 4 Error: In initializer for
> >> 'local:number',
> >>> type String is not assignable to target type 'Number'.
> >>>           <fx:String>Something else</fx:String>
> >>> ^
> >>> Multiple initializers
> >>> ---------------------
> >>>  <local:number>
> >>>           <fx:Number>23.56</fx:Number>
> >>>       </local:number>
> >>>       <local:number>
> >>>           <fx:Number>23.56</fx:Number>
> >>>       </local:number>
> >>> this gives the following in Flex 4:
> >>> Error:(24, 0) [flex4testforFlexJS]: Multiple initializers for property
> >>> 'number'.
> >>> In FlexJS (with or without the proposed fixes for the other issues
> above)
> >>> GenericTests.mxml(58): col: 9 Error: Child tag 'number' bound to
> >> namespace
> >>> '*' is already specified for element 'local:TestView'. It will be
> >> ignored.
> >>>       <local:number>
> >>>       ^
> >>> (In both Flex 4 and FlexJS the line number reported is for the
> beginning
> >> of
> >>> the 2nd initializer tag <local:number>, which is correct)
> >>> CURRENT BUG
> >>> -----------
> >>> I believe the error description should be improved - it currently says
> >> "It
> >>> will be ignored" and that is not what actually happens (not being
> ignored
> >>> is correct, IMO).
> >>> And (unless I am missing something important) the rest seems too
> verbose.
> >>> PROPOSED FIX (NOT DONE):
> >>> -----------------------
> >>> This is easy to change.... please let me know if you agree.
> >>> I suggest changing the FlexJS error description to be similar to Flex
> 4,
> >> to:
> >>> Multiple initializers for 'local:number' are not permitted for element
> >>> 'local:TestView'.
> >>>  <local:number>
> >>>           ^
> >>>
> >>>
> >>>
> >>>
> >>> On Sat, Mar 4, 2017 at 8:46 PM, Greg Dove <gr...@gmail.com> wrote:
> >>>
> >>>>
> >>>> I believe MXML is supposed to treat more than one child in a child tag
> >> as
> >>>> an array.  And thus, the equivalent AS code for:
> >>>>
> >>>> <js:initialView>
> >>>> <local:MyInitialView id="view1" />
> >>>> <local:MyOtherInitialView id="view2" />
> >>>> </js:initialView>
> >>>>
> >>>> is:
> >>>>
> >>>> initialView = [ new MyInitialView, new MyOtherInitialView];
> >>>>
> >>>>
> >>>>
> >>>> I understood the same, but I had thought it was supposed to be a bit
> >>>> smarter than that and only assume an implicit <fx:Array> wrapper if it
> >> is
> >>>> assigning to an Array typed property (maybe even 'Arraylike')
> property.
> >> I
> >>>> will check the behavior using Flex 4 - I assume we want to keep
> whatever
> >>>> 'smarts' were implemented there, or at least as close as makes sense?
> >> I'll
> >>>> also double check the compiler error for this same scenario in Flex 4,
> >>>> which I did not do yet.
> >>>>
> >>>> If you code that in AS, you will get an error (I hope), and I think
> the
> >>>> compiler should report the same error for this MXML scenario, since
> >>>> initialView is of type IApplicationView or something like that.
> >>>>
> >>>>
> >>>> There might arguably be some justification for having an 'mxml
> version'
> >> of
> >>>> an error, describing things more in mxml terms, particularly for new
> >> users
> >>>> as they come on board and perhaps play with mxml examples first before
> >>>> learning actionscript, but I will check both the actionscript error in
> >>>> FlexJS and the mxml error in Flex 4 (assuming there is one)  for this
> >>>> scenario and report back here for consensus.
> >>>>
> >>>> <js:initialView>
> >>>> <js:SimpleCSSValuesImpl />
> >>>> </js:initialView>
> >>>>
> >>>> Is the equivalent of:
> >>>> var initialView:IApplicationView = new SimpleCSSValuesImpl().
> >>>>
> >>>>
> >>>>
> >>>> Exactly, and in the fix, I have this currently being described as:
> >>>> In initializer for 'js:initialView', type
> org.apache.flex.core.SimpleCSS
> >> ValuesImpl
> >>>> is not assignable to target type 'org.apache.flex.core.IApplica
> >> tionView'.
> >>>>
> >>>> I will check the FlexJS actionscript error for this also, but I
> >> definitely
> >>>> pulled that one above as-is from Flex 4 for the same mxml problem
> >> (which is
> >>>> also correctly treated as a compile time error)
> >>>>
> >>>>
> >>>>
> >>>> On Fri, Mar 3, 2017 at 6:33 PM, Alex Harui <ah...@adobe.com> wrote:
> >>>>
> >>>>> Hi Greg,
> >>>>>
> >>>>> Thanks for digging into this.
> >>>>>
> >>>>> Without having dug into it myself, I would like to suggest returning
> a
> >>>>> type mismatch error of some sort.  IIRC, the DuplicateChildTagProblem
> >> is
> >>>>> for the following:
> >>>>>
> >>>>> <js:initialView>
> >>>>> <local:MyInitialView id="view1" />
> >>>>> </js:initialView>
> >>>>> <js:initialView>
> >>>>> <local:MyInitialView id="view2" />
> >>>>>
> >>>>> </js:initialView>
> >>>>>
> >>>>> The child tag (js:initialView) is really in there twice.
> >>>>>
> >>>>>
> >>>>> I believe MXML is supposed to treat more than one child in a child
> tag
> >> as
> >>>>> an array.  And thus, the equivalent AS code for:
> >>>>>
> >>>>> <js:initialView>
> >>>>> <local:MyInitialView id="view1" />
> >>>>> <local:MyOtherInitialView id="view2" />
> >>>>> </js:initialView>
> >>>>>
> >>>>> is:
> >>>>>
> >>>>> initialView = [ new MyInitialView, new MyOtherInitialView];
> >>>>>
> >>>>> If you code that in AS, you will get an error (I hope), and I think
> the
> >>>>> compiler should report the same error for this MXML scenario, since
> >>>>> initialView is of type IApplicationView or something like that.
> >>>>>
> >>>>> And same for the second scenario:
> >>>>>
> >>>>> <js:initialView>
> >>>>> <js:SimpleCSSValuesImpl />
> >>>>> </js:initialView>
> >>>>>
> >>>>> Is the equivalent of:
> >>>>>
> >>>>>
> >>>>> var initialView:IApplicationView = new SimpleCSSValuesImpl().
> >>>>>
> >>>>> My 2 cents,
> >>>>> -Alex
> >>>>>
> >>>>> On 3/2/17, 9:09 PM, "Greg Dove" <gr...@apache.org> wrote:
> >>>>>
> >>>>>> I have been looking into FLEX-35273 [1].
> >>>>>>
> >>>>>> This is a compiler bug where it is possible to do things that don't
> >> make
> >>>>>> sense, like:
> >>>>>>
> >>>>>> <js:initialView>
> >>>>>>     <local:MyInitialView id="view1" />
> >>>>>>     <local:MyInitialView id="view2" />
> >>>>>> </js:initialView>
> >>>>>>
> >>>>>> or even this :
> >>>>>>
> >>>>>> <js:initialView>
> >>>>>>     <js:SimpleCSSValuesImpl />
> >>>>>> </js:initialView>
> >>>>>>
> >>>>>> Neither of the above caused compile time errors.
> >>>>>>
> >>>>>> I have a fix for both the above scenarios.
> >>>>>>
> >>>>>> For the first one, I used MXMLDuplicateChildTagProblem which seems
> >> 'close
> >>>>>> enough'
> >>>>>>
> >>>>>> "Child tag '${childTag}' bound to namespace '${childNamespace}' is
> >>>>>> already specified for element '${element}'. It will be ignored.";
> >>>>>>
> >>>>>> This renders as :
> >>>>>> Child tag 'MyInitialView' bound to namespace '*' is already
> specified
> >> for
> >>>>>> element 'js:initialView'. It will be ignored.
> >>>>>> <local:MyInitialView id="view2" />
> >>>>>>  ^
> >>>>>>
> >>>>>> in the first example above. The text does not feel entirely right,
> but
> >>>>>> seems close enough. "It will be ignored" sounds more like a warning
> >> (as
> >>>>>> opposed to an error/failure), which I think a) it should not be and
> >> b) it
> >>>>>> is not.
> >>>>>>
> >>>>>> For the second one, I could not find any relevant 'Problem' class (I
> >> may
> >>>>>> have missed one perhaps?) So I just made a new one. I don't really
> >> know
> >>>>>> what the rules are here.
> >>>>>> These Problems seem to include an error code so I am unclear what to
> >> do
> >>>>>> if I add a new one. i.e. it is not clear what new error code I
> should
> >>>>>> apply (or even if the error code is important for something).
> >>>>>> For now I just added an arbitrary errorCode = 1999 on
> >>>>>> MXMLBadChildTagPropertyAssignmentProblem.
> >>>>>>
> >>>>>> The new problem renders out to :
> >>>>>> In initializer for 'js:initialView', type
> >>>>>> org.apache.flex.core.SimpleCSSValuesImpl is not assignable to
> target
> >>>>> type
> >>>>>> 'org.apache.flex.core.IApplicationView'.
> >>>>>>
> >>>>>> Alex, you may be able to advise here.... are there any rules I need
> to
> >>>>>> follow for the CompilerProblem classes (in particular, when adding a
> >> new
> >>>>>> class).
> >>>>>>
> >>>>>> Assuming all is well above, I will commit this fix tomorrow. I was
> >> also
> >>>>>> trying to see if I could figure out how to get checkintests running,
> >> but
> >>>>>> I so far I did not. However the regular build tests are all fine
> with
> >> the
> >>>>>> changes I made for this.
> >>>>>>
> >>>>>>
> >>>>>> 1. https://issues.apache.org/jira/browse/FLEX-35273
> >>>>>
> >>>>>
> >>>>
> >>
> >>
>
>

Re: [FlexJS] Working to fix a compiler bug - problem with the Problems

Posted by Harbs <ha...@gmail.com>.

I have noticed that MXML compiling does not produce the greatest error reports. I should have recorded issues as I saw them, but I didn’t. :-(

These two cases make sense to me, and thanks for fixing them. I’ll be happy to update falcon and make sure that things still compile correctly in my apps. Anything else I should be doing?

> On Mar 6, 2017, at 10:40 AM, Greg Dove <gr...@gmail.com> wrote:
> 
> "It sound to me like you know what you’re talking about"
> 
> Haha, Harbs that's probably more reassuring to me than it should be to you.
> In terms of anything at all in the compiler, I am still LAYG (learning as
> you go) here ;)
> (btw, you can never have too many 'as you go' acronyms)
> 
> I actually shared Alex's general concerns over making changes in the mxml
> parsing, we don't have a large range of tests to cover all the potential
> mxml variations (otherwise I expect this would have already been addressed).
> 
> Harbs, I understand that you have a (quite possibly) large codebase so you
> might provide the best real-world 'litmus test' in you have a large amount
> of mxml.
> 
> 
> In simple terms the changes will prevent the following 2 examples from
> compiling, which currently compile fine in FlexJS, but should not:
> 
> <js:initialView>
>        <local:MyInitialView id="view1" />
>        <local:MyInitialView id="view2" />
> </js:initialView>
> 
> in the above example the view2 instance was being created and assigned for
> the initialView propertty, the first was ignored
> 
> <js:initialView>
>        <js:SimpleCSSValuesImpl />
> </js:initialView>
> 
> In this case the types are incompatible.
> 
> I actually think the compiler errors for mxml could be more important than
> a sesaoned Flex user would assume in terms of how new people will form
> early impressions. When people who potentially don't know actionscript
> (yet) take some FlexJS examples for a spin, they are probably going to look
> at the mxml and think to themselves 'I can do that', so it will be the
> first thing they mess with. Mxml-related errors might be the first thing
> they see from the compiler after they start messing with stuff.
> That's why I spent a bit of time discussing those...I have gone ahead and
> gone with the two new ones as I suggested.
> 
> 
> 
> On Mon, Mar 6, 2017 at 12:08 AM, Harbs <ha...@gmail.com> wrote:
> 
>> I have to admit that you lost me pretty quickly… ;-)
>> 
>> It sound to me like you know what you’re talking about, so I’m fine with
>> whatever you did/want to do… :-)
>> 
>> Harbs
>> 
>>> On Mar 5, 2017, at 9:31 AM, Greg Dove <gr...@gmail.com> wrote:
>>> 
>>> OK.... please brace yourself for an onslaught of text.
>>> 
>>> I tested across Flex 4 and FlexJS through a bunch of variations. This
>>> change passes all tests. In the end this is a very small and isolated
>> code
>>> change (I am almost certain it will not contribute to anyone's merge
>>> conflicts) and I believe it gives greater compile-time type safety to
>> mxml,
>>> so I think it's necessary. But the error descriptions are open for more
>>> input/review... I have favored a strong consistency with the Flex 4
>>> descriptions, which were specific for mxml, over using the related
>>> actionscript descriptions.
>>> 
>>> BTW I had forgotten, but there was also a comment (it was not marked as a
>>> todo) in the falcon MXMLPropertySpecifierNode class that indicated that
>>> this type of work still needed to be done, so I expect it was an
>> unfinished
>>> implementation.
>>> 
>>> If I hear nothing conflciting by EOD tomorrow I will commit what I have.
>> It
>>> can easily be reverted if anyone finds issues, or I am happy to address
>> it
>>> further if it is not correct for some cases that I did not anticipate.
>>> 
>>> That's the end of the preamble.... best of luck getting through what
>>> follows... it took me longer than the original coding part :)
>>> 
>>> Test setup for Flex 4:
>>> 'TestView' below is simply a Flex4 Group mxml component, like so:
>>> <s:Group xmlns:fx="http://ns.adobe.com/mxml/2009"
>>>        xmlns:s="library://ns.adobe.com/flex/spark"
>>>        creationComplete="onCreated()">
>>>   <fx:Script><![CDATA[
>>>       import spark.components.Alert;
>>>       private function onCreated():void{
>>>         //check for actionscript version of error
>>>         //this.number="something";
>>>       }
>>>       public var testArr:Array;
>>>       public var number:Number;
>>>       ]]></fx:Script>
>>>   <s:layout>
>>>       <s:VerticalLayout/>
>>>   </s:layout>
>>>   <s:Button click="test.text='testArr ['+testArr.toString()+'],\nnumber
>>> val:'+number" width="100" height="50"/>
>>>   <s:Label id="test"/>
>>> </s:Group>
>>> 
>>> Test setup for FlexJS:
>>> 'TestView' below is simply a Flex4 Group like so:
>>> <js:View xmlns:fx="http://ns.adobe.com/mxml/2009"
>>> xmlns:js="library://ns.apache.org/flexjs/basic"
>>> initComplete="onCreated()">
>>>  <fx:Script><![CDATA[
>>> import org.apache.flex.html.Alert;
>>>       private function onCreated():void{
>>>         //check for actionscript version of error
>>>         //this.number="something";
>>>       }
>>>       public var testArr:Array;
>>>       public var number:Number;
>>>       ]]></fx:Script>
>>> <js:Button click="Alert.show('testArr ['+testArr.toString()+'],\nnumber
>>> val:'+number, this)" width="100" height="50"/>
>>> </js:View>
>>> 
>>> 
>>> Testing:
>>>   <local:TestView id="test">
>>>       <local:testArr>
>>>           <fx:Number>23.56</fx:Number>
>>>           <fx:Number>23.99</fx:Number>
>>>           <fx:String>Something else</fx:String>
>>>       </local:testArr>
>>>       <local:number>
>>>           <fx:Number>23.56</fx:Number>
>>>       </local:number>
>>>   </local:TestView>
>>> The above compiles correctly and because the testArr property has an
>>> 'Array' type, the children are inferred as array elements, it is the same
>>> as specifying:
>>>       <local:testArr>
>>>           <fx:Array>
>>>               <fx:Number>23.56</fx:Number>
>>>               <fx:Number>23.99</fx:Number>
>>>               <fx:String>Something else</fx:String>
>>>           </fx:Array>
>>>       </local:testArr>
>>> Note: Using one vs. multiple children: the inference is the same - if the
>>> property being initialized is of type Array, it infers correctly (and
>>> performs the 'Array' wrapping if needed) irrespective of single or
>> multiple
>>> child tags.
>>> NO BUGS FOR ARRAY (inferred or explicit)
>>> This works in both Flex 4 and Falcon (before or after my local changes)
>>> Multiple value children when the property being initialized is not
>> 'Array'
>>> type
>>> ---------------------------------------------------------------------
>>> the 'number' property on the other hand is of type Number, and changing
>>> number assignment to multiple values, in this way:
>>>  <local:number>
>>>           <fx:Number>23.56</fx:Number>
>>> <fx:Number>23.99</fx:Number>
>>>       </local:number>
>>> gives Flex 4 compiler errors like so:
>>> Error:(20, 0) [flex4testforFlexJS]: In initializer for 'number', multiple
>>> initializer values for target type Number.
>>>   Error:(21, 0) [flex4testforFlexJS]: In initializer for 'number',
>>> multiple initializer values for target type Number.
>>> (adding more than 2 gives an individual error for each child)
>>> <fx:Number>23.56</fx:Number>
>>>               <fx:Number>23.99</fx:Number>
>>>           <fx:String>Something else</fx:String>
>>>       </local:number>
>>> Swapping to one element with wrong type in multiple child assignment does
>>> not change the error reported (it remains simply 'multiple values'):
>>> Error:(20, 0) [flex4testforFlexJS]: In initializer for 'number', multiple
>>> initializer values for target type Number.
>>> Error:(21, 0) [flex4testforFlexJS]: In initializer for 'number', multiple
>>> initializer values for target type Number.
>>> CURRENT BUG
>>> -----------
>>> Currently in FlexJS, the following happens for either of the above
>>> scenarios:
>>> Only the last child tag is processed in multiple tags like this, the
>>> earlier ones are ignored. It is processed as if it were the only child
>> tag
>>> (and it may be of an incompatible type).
>>> PROPOSED FIX(DONE)
>>> -----------------
>>> Please let me know if you agree, or suggest alternative.
>>> in the local fix for flexjs, I have now created a new Problem,
>>> MXMLMultipleInitializersProblem, which follows the Flex 4 example
>>> with outputs descriptions similar to:
>>> GenericTests.mxml(57): col: 17 Error: In initializer for 'local:number'
>>> multiple initializer values are not permitted for target type 'Number'.
>>>               <fx:Number>23.99</fx:Number>
>>>               ^
>>> GenericTests.mxml(58): col: 13 Error: In initializer for 'local:number'
>>> multiple initializer values are not permitted for target type 'Number'.
>>>           <fx:String>Something else</fx:String>
>>>           ^
>>> Inappropriate single child when property being initialized is not 'Array'
>>> type
>>> ------------------------------------------------------------
>> ----------------
>>> <local:number>
>>>       <fx:String>Something else</fx:String>
>>>   </local:number>
>>> in Flex 4, this gives:
>>> Error:(22, 0) [flex4testforFlexJS]: In initializer for 'number', type
>>> String is not assignable to target type 'Number'.
>>> The corresponding Flex 4 actionscript error is
>>> Error:(11, 0) [flex4testforFlexJS]: Error code: 1067: Implicit coercion
>> of
>>> a value of type String to an unrelated type Number.
>>> If FlexJS, the actionscript error is the same
>>> CURRENT BUG
>>> -----------
>>> in Falcon this currently does not cause a compiler error for mxml,
>>> compilation completes normally, and in js the value of number will be the
>>> "Something else", in swf it is NaN. IMO this is clearly wrong.
>>> 
>>> 
>>> PROPOSED FIX (DONE)
>>> -------------------
>>> Please let me know if you agree, or suggest alternative.
>>> I have added a new Problem to be closer to what was shown before in Flex
>> 4:
>>> This is the new one I mirrored from Flex 4 (with same description) in
>> FlexJS
>>>   GenericTests.mxml(56): col: 4 Error: In initializer for
>> 'local:number',
>>> type String is not assignable to target type 'Number'.
>>>           <fx:String>Something else</fx:String>
>>> ^
>>> Multiple initializers
>>> ---------------------
>>>  <local:number>
>>>           <fx:Number>23.56</fx:Number>
>>>       </local:number>
>>>       <local:number>
>>>           <fx:Number>23.56</fx:Number>
>>>       </local:number>
>>> this gives the following in Flex 4:
>>> Error:(24, 0) [flex4testforFlexJS]: Multiple initializers for property
>>> 'number'.
>>> In FlexJS (with or without the proposed fixes for the other issues above)
>>> GenericTests.mxml(58): col: 9 Error: Child tag 'number' bound to
>> namespace
>>> '*' is already specified for element 'local:TestView'. It will be
>> ignored.
>>>       <local:number>
>>>       ^
>>> (In both Flex 4 and FlexJS the line number reported is for the beginning
>> of
>>> the 2nd initializer tag <local:number>, which is correct)
>>> CURRENT BUG
>>> -----------
>>> I believe the error description should be improved - it currently says
>> "It
>>> will be ignored" and that is not what actually happens (not being ignored
>>> is correct, IMO).
>>> And (unless I am missing something important) the rest seems too verbose.
>>> PROPOSED FIX (NOT DONE):
>>> -----------------------
>>> This is easy to change.... please let me know if you agree.
>>> I suggest changing the FlexJS error description to be similar to Flex 4,
>> to:
>>> Multiple initializers for 'local:number' are not permitted for element
>>> 'local:TestView'.
>>>  <local:number>
>>>           ^
>>> 
>>> 
>>> 
>>> 
>>> On Sat, Mar 4, 2017 at 8:46 PM, Greg Dove <gr...@gmail.com> wrote:
>>> 
>>>> 
>>>> I believe MXML is supposed to treat more than one child in a child tag
>> as
>>>> an array.  And thus, the equivalent AS code for:
>>>> 
>>>> <js:initialView>
>>>> <local:MyInitialView id="view1" />
>>>> <local:MyOtherInitialView id="view2" />
>>>> </js:initialView>
>>>> 
>>>> is:
>>>> 
>>>> initialView = [ new MyInitialView, new MyOtherInitialView];
>>>> 
>>>> 
>>>> 
>>>> I understood the same, but I had thought it was supposed to be a bit
>>>> smarter than that and only assume an implicit <fx:Array> wrapper if it
>> is
>>>> assigning to an Array typed property (maybe even 'Arraylike') property.
>> I
>>>> will check the behavior using Flex 4 - I assume we want to keep whatever
>>>> 'smarts' were implemented there, or at least as close as makes sense?
>> I'll
>>>> also double check the compiler error for this same scenario in Flex 4,
>>>> which I did not do yet.
>>>> 
>>>> If you code that in AS, you will get an error (I hope), and I think the
>>>> compiler should report the same error for this MXML scenario, since
>>>> initialView is of type IApplicationView or something like that.
>>>> 
>>>> 
>>>> There might arguably be some justification for having an 'mxml version'
>> of
>>>> an error, describing things more in mxml terms, particularly for new
>> users
>>>> as they come on board and perhaps play with mxml examples first before
>>>> learning actionscript, but I will check both the actionscript error in
>>>> FlexJS and the mxml error in Flex 4 (assuming there is one)  for this
>>>> scenario and report back here for consensus.
>>>> 
>>>> <js:initialView>
>>>> <js:SimpleCSSValuesImpl />
>>>> </js:initialView>
>>>> 
>>>> Is the equivalent of:
>>>> var initialView:IApplicationView = new SimpleCSSValuesImpl().
>>>> 
>>>> 
>>>> 
>>>> Exactly, and in the fix, I have this currently being described as:
>>>> In initializer for 'js:initialView', type org.apache.flex.core.SimpleCSS
>> ValuesImpl
>>>> is not assignable to target type 'org.apache.flex.core.IApplica
>> tionView'.
>>>> 
>>>> I will check the FlexJS actionscript error for this also, but I
>> definitely
>>>> pulled that one above as-is from Flex 4 for the same mxml problem
>> (which is
>>>> also correctly treated as a compile time error)
>>>> 
>>>> 
>>>> 
>>>> On Fri, Mar 3, 2017 at 6:33 PM, Alex Harui <ah...@adobe.com> wrote:
>>>> 
>>>>> Hi Greg,
>>>>> 
>>>>> Thanks for digging into this.
>>>>> 
>>>>> Without having dug into it myself, I would like to suggest returning a
>>>>> type mismatch error of some sort.  IIRC, the DuplicateChildTagProblem
>> is
>>>>> for the following:
>>>>> 
>>>>> <js:initialView>
>>>>> <local:MyInitialView id="view1" />
>>>>> </js:initialView>
>>>>> <js:initialView>
>>>>> <local:MyInitialView id="view2" />
>>>>> 
>>>>> </js:initialView>
>>>>> 
>>>>> The child tag (js:initialView) is really in there twice.
>>>>> 
>>>>> 
>>>>> I believe MXML is supposed to treat more than one child in a child tag
>> as
>>>>> an array.  And thus, the equivalent AS code for:
>>>>> 
>>>>> <js:initialView>
>>>>> <local:MyInitialView id="view1" />
>>>>> <local:MyOtherInitialView id="view2" />
>>>>> </js:initialView>
>>>>> 
>>>>> is:
>>>>> 
>>>>> initialView = [ new MyInitialView, new MyOtherInitialView];
>>>>> 
>>>>> If you code that in AS, you will get an error (I hope), and I think the
>>>>> compiler should report the same error for this MXML scenario, since
>>>>> initialView is of type IApplicationView or something like that.
>>>>> 
>>>>> And same for the second scenario:
>>>>> 
>>>>> <js:initialView>
>>>>> <js:SimpleCSSValuesImpl />
>>>>> </js:initialView>
>>>>> 
>>>>> Is the equivalent of:
>>>>> 
>>>>> 
>>>>> var initialView:IApplicationView = new SimpleCSSValuesImpl().
>>>>> 
>>>>> My 2 cents,
>>>>> -Alex
>>>>> 
>>>>> On 3/2/17, 9:09 PM, "Greg Dove" <gr...@apache.org> wrote:
>>>>> 
>>>>>> I have been looking into FLEX-35273 [1].
>>>>>> 
>>>>>> This is a compiler bug where it is possible to do things that don't
>> make
>>>>>> sense, like:
>>>>>> 
>>>>>> <js:initialView>
>>>>>>     <local:MyInitialView id="view1" />
>>>>>>     <local:MyInitialView id="view2" />
>>>>>> </js:initialView>
>>>>>> 
>>>>>> or even this :
>>>>>> 
>>>>>> <js:initialView>
>>>>>>     <js:SimpleCSSValuesImpl />
>>>>>> </js:initialView>
>>>>>> 
>>>>>> Neither of the above caused compile time errors.
>>>>>> 
>>>>>> I have a fix for both the above scenarios.
>>>>>> 
>>>>>> For the first one, I used MXMLDuplicateChildTagProblem which seems
>> 'close
>>>>>> enough'
>>>>>> 
>>>>>> "Child tag '${childTag}' bound to namespace '${childNamespace}' is
>>>>>> already specified for element '${element}'. It will be ignored.";
>>>>>> 
>>>>>> This renders as :
>>>>>> Child tag 'MyInitialView' bound to namespace '*' is already specified
>> for
>>>>>> element 'js:initialView'. It will be ignored.
>>>>>> <local:MyInitialView id="view2" />
>>>>>>  ^
>>>>>> 
>>>>>> in the first example above. The text does not feel entirely right, but
>>>>>> seems close enough. "It will be ignored" sounds more like a warning
>> (as
>>>>>> opposed to an error/failure), which I think a) it should not be and
>> b) it
>>>>>> is not.
>>>>>> 
>>>>>> For the second one, I could not find any relevant 'Problem' class (I
>> may
>>>>>> have missed one perhaps?) So I just made a new one. I don't really
>> know
>>>>>> what the rules are here.
>>>>>> These Problems seem to include an error code so I am unclear what to
>> do
>>>>>> if I add a new one. i.e. it is not clear what new error code I should
>>>>>> apply (or even if the error code is important for something).
>>>>>> For now I just added an arbitrary errorCode = 1999 on
>>>>>> MXMLBadChildTagPropertyAssignmentProblem.
>>>>>> 
>>>>>> The new problem renders out to :
>>>>>> In initializer for 'js:initialView', type
>>>>>> org.apache.flex.core.SimpleCSSValuesImpl is not assignable to target
>>>>> type
>>>>>> 'org.apache.flex.core.IApplicationView'.
>>>>>> 
>>>>>> Alex, you may be able to advise here.... are there any rules I need to
>>>>>> follow for the CompilerProblem classes (in particular, when adding a
>> new
>>>>>> class).
>>>>>> 
>>>>>> Assuming all is well above, I will commit this fix tomorrow. I was
>> also
>>>>>> trying to see if I could figure out how to get checkintests running,
>> but
>>>>>> I so far I did not. However the regular build tests are all fine with
>> the
>>>>>> changes I made for this.
>>>>>> 
>>>>>> 
>>>>>> 1. https://issues.apache.org/jira/browse/FLEX-35273
>>>>> 
>>>>> 
>>>> 
>> 
>>

Re: [FlexJS] Working to fix a compiler bug - problem with the Problems

Posted by Greg Dove <gr...@gmail.com>.

"It sound to me like you know what you’re talking about"

Haha, Harbs that's probably more reassuring to me than it should be to you.
In terms of anything at all in the compiler, I am still LAYG (learning as
you go) here ;)
(btw, you can never have too many 'as you go' acronyms)

I actually shared Alex's general concerns over making changes in the mxml
parsing, we don't have a large range of tests to cover all the potential
mxml variations (otherwise I expect this would have already been addressed).

Harbs, I understand that you have a (quite possibly) large codebase so you
might provide the best real-world 'litmus test' in you have a large amount
of mxml.


In simple terms the changes will prevent the following 2 examples from
compiling, which currently compile fine in FlexJS, but should not:

<js:initialView>
        <local:MyInitialView id="view1" />
        <local:MyInitialView id="view2" />
</js:initialView>

in the above example the view2 instance was being created and assigned for
the initialView propertty, the first was ignored

<js:initialView>
        <js:SimpleCSSValuesImpl />
</js:initialView>

In this case the types are incompatible.

I actually think the compiler errors for mxml could be more important than
a sesaoned Flex user would assume in terms of how new people will form
early impressions. When people who potentially don't know actionscript
(yet) take some FlexJS examples for a spin, they are probably going to look
at the mxml and think to themselves 'I can do that', so it will be the
first thing they mess with. Mxml-related errors might be the first thing
they see from the compiler after they start messing with stuff.
That's why I spent a bit of time discussing those...I have gone ahead and
gone with the two new ones as I suggested.



On Mon, Mar 6, 2017 at 12:08 AM, Harbs <ha...@gmail.com> wrote:

> I have to admit that you lost me pretty quickly… ;-)
>
> It sound to me like you know what you’re talking about, so I’m fine with
> whatever you did/want to do… :-)
>
> Harbs
>
> > On Mar 5, 2017, at 9:31 AM, Greg Dove <gr...@gmail.com> wrote:
> >
> > OK.... please brace yourself for an onslaught of text.
> >
> > I tested across Flex 4 and FlexJS through a bunch of variations. This
> > change passes all tests. In the end this is a very small and isolated
> code
> > change (I am almost certain it will not contribute to anyone's merge
> > conflicts) and I believe it gives greater compile-time type safety to
> mxml,
> > so I think it's necessary. But the error descriptions are open for more
> > input/review... I have favored a strong consistency with the Flex 4
> > descriptions, which were specific for mxml, over using the related
> > actionscript descriptions.
> >
> > BTW I had forgotten, but there was also a comment (it was not marked as a
> > todo) in the falcon MXMLPropertySpecifierNode class that indicated that
> > this type of work still needed to be done, so I expect it was an
> unfinished
> > implementation.
> >
> > If I hear nothing conflciting by EOD tomorrow I will commit what I have.
> It
> > can easily be reverted if anyone finds issues, or I am happy to address
> it
> > further if it is not correct for some cases that I did not anticipate.
> >
> > That's the end of the preamble.... best of luck getting through what
> > follows... it took me longer than the original coding part :)
> >
> > Test setup for Flex 4:
> > 'TestView' below is simply a Flex4 Group mxml component, like so:
> > <s:Group xmlns:fx="http://ns.adobe.com/mxml/2009"
> >         xmlns:s="library://ns.adobe.com/flex/spark"
> >         creationComplete="onCreated()">
> >    <fx:Script><![CDATA[
> >        import spark.components.Alert;
> >        private function onCreated():void{
> >          //check for actionscript version of error
> >          //this.number="something";
> >        }
> >        public var testArr:Array;
> >        public var number:Number;
> >        ]]></fx:Script>
> >    <s:layout>
> >        <s:VerticalLayout/>
> >    </s:layout>
> >    <s:Button click="test.text='testArr ['+testArr.toString()+'],\nnumber
> > val:'+number" width="100" height="50"/>
> >    <s:Label id="test"/>
> > </s:Group>
> >
> > Test setup for FlexJS:
> > 'TestView' below is simply a Flex4 Group like so:
> > <js:View xmlns:fx="http://ns.adobe.com/mxml/2009"
> > xmlns:js="library://ns.apache.org/flexjs/basic"
> > initComplete="onCreated()">
> >   <fx:Script><![CDATA[
> > import org.apache.flex.html.Alert;
> >        private function onCreated():void{
> >          //check for actionscript version of error
> >          //this.number="something";
> >        }
> >        public var testArr:Array;
> >        public var number:Number;
> >        ]]></fx:Script>
> > <js:Button click="Alert.show('testArr ['+testArr.toString()+'],\nnumber
> > val:'+number, this)" width="100" height="50"/>
> > </js:View>
> >
> >
> > Testing:
> >    <local:TestView id="test">
> >        <local:testArr>
> >            <fx:Number>23.56</fx:Number>
> >            <fx:Number>23.99</fx:Number>
> >            <fx:String>Something else</fx:String>
> >        </local:testArr>
> >        <local:number>
> >            <fx:Number>23.56</fx:Number>
> >        </local:number>
> >    </local:TestView>
> > The above compiles correctly and because the testArr property has an
> > 'Array' type, the children are inferred as array elements, it is the same
> > as specifying:
> >        <local:testArr>
> >            <fx:Array>
> >                <fx:Number>23.56</fx:Number>
> >                <fx:Number>23.99</fx:Number>
> >                <fx:String>Something else</fx:String>
> >            </fx:Array>
> >        </local:testArr>
> > Note: Using one vs. multiple children: the inference is the same - if the
> > property being initialized is of type Array, it infers correctly (and
> > performs the 'Array' wrapping if needed) irrespective of single or
> multiple
> > child tags.
> > NO BUGS FOR ARRAY (inferred or explicit)
> > This works in both Flex 4 and Falcon (before or after my local changes)
> > Multiple value children when the property being initialized is not
> 'Array'
> > type
> > ---------------------------------------------------------------------
> > the 'number' property on the other hand is of type Number, and changing
> > number assignment to multiple values, in this way:
> >   <local:number>
> >            <fx:Number>23.56</fx:Number>
> > <fx:Number>23.99</fx:Number>
> >        </local:number>
> > gives Flex 4 compiler errors like so:
> > Error:(20, 0) [flex4testforFlexJS]: In initializer for 'number', multiple
> > initializer values for target type Number.
> >    Error:(21, 0) [flex4testforFlexJS]: In initializer for 'number',
> > multiple initializer values for target type Number.
> > (adding more than 2 gives an individual error for each child)
> > <fx:Number>23.56</fx:Number>
> >                <fx:Number>23.99</fx:Number>
> >            <fx:String>Something else</fx:String>
> >        </local:number>
> > Swapping to one element with wrong type in multiple child assignment does
> > not change the error reported (it remains simply 'multiple values'):
> > Error:(20, 0) [flex4testforFlexJS]: In initializer for 'number', multiple
> > initializer values for target type Number.
> > Error:(21, 0) [flex4testforFlexJS]: In initializer for 'number', multiple
> > initializer values for target type Number.
> > CURRENT BUG
> > -----------
> > Currently in FlexJS, the following happens for either of the above
> > scenarios:
> > Only the last child tag is processed in multiple tags like this, the
> > earlier ones are ignored. It is processed as if it were the only child
> tag
> > (and it may be of an incompatible type).
> > PROPOSED FIX(DONE)
> > -----------------
> > Please let me know if you agree, or suggest alternative.
> > in the local fix for flexjs, I have now created a new Problem,
> > MXMLMultipleInitializersProblem, which follows the Flex 4 example
> > with outputs descriptions similar to:
> > GenericTests.mxml(57): col: 17 Error: In initializer for 'local:number'
> > multiple initializer values are not permitted for target type 'Number'.
> >                <fx:Number>23.99</fx:Number>
> >                ^
> > GenericTests.mxml(58): col: 13 Error: In initializer for 'local:number'
> > multiple initializer values are not permitted for target type 'Number'.
> >            <fx:String>Something else</fx:String>
> >            ^
> > Inappropriate single child when property being initialized is not 'Array'
> > type
> > ------------------------------------------------------------
> ----------------
> > <local:number>
> >        <fx:String>Something else</fx:String>
> >    </local:number>
> > in Flex 4, this gives:
> > Error:(22, 0) [flex4testforFlexJS]: In initializer for 'number', type
> > String is not assignable to target type 'Number'.
> > The corresponding Flex 4 actionscript error is
> > Error:(11, 0) [flex4testforFlexJS]: Error code: 1067: Implicit coercion
> of
> > a value of type String to an unrelated type Number.
> > If FlexJS, the actionscript error is the same
> > CURRENT BUG
> > -----------
> > in Falcon this currently does not cause a compiler error for mxml,
> > compilation completes normally, and in js the value of number will be the
> > "Something else", in swf it is NaN. IMO this is clearly wrong.
> >
> >
> > PROPOSED FIX (DONE)
> > -------------------
> > Please let me know if you agree, or suggest alternative.
> > I have added a new Problem to be closer to what was shown before in Flex
> 4:
> > This is the new one I mirrored from Flex 4 (with same description) in
> FlexJS
> >    GenericTests.mxml(56): col: 4 Error: In initializer for
> 'local:number',
> > type String is not assignable to target type 'Number'.
> >            <fx:String>Something else</fx:String>
> > ^
> > Multiple initializers
> > ---------------------
> >   <local:number>
> >            <fx:Number>23.56</fx:Number>
> >        </local:number>
> >        <local:number>
> >            <fx:Number>23.56</fx:Number>
> >        </local:number>
> > this gives the following in Flex 4:
> > Error:(24, 0) [flex4testforFlexJS]: Multiple initializers for property
> > 'number'.
> > In FlexJS (with or without the proposed fixes for the other issues above)
> > GenericTests.mxml(58): col: 9 Error: Child tag 'number' bound to
> namespace
> > '*' is already specified for element 'local:TestView'. It will be
> ignored.
> >        <local:number>
> >        ^
> > (In both Flex 4 and FlexJS the line number reported is for the beginning
> of
> > the 2nd initializer tag <local:number>, which is correct)
> > CURRENT BUG
> > -----------
> > I believe the error description should be improved - it currently says
> "It
> > will be ignored" and that is not what actually happens (not being ignored
> > is correct, IMO).
> > And (unless I am missing something important) the rest seems too verbose.
> > PROPOSED FIX (NOT DONE):
> > -----------------------
> > This is easy to change.... please let me know if you agree.
> > I suggest changing the FlexJS error description to be similar to Flex 4,
> to:
> > Multiple initializers for 'local:number' are not permitted for element
> > 'local:TestView'.
> >   <local:number>
> >            ^
> >
> >
> >
> >
> > On Sat, Mar 4, 2017 at 8:46 PM, Greg Dove <gr...@gmail.com> wrote:
> >
> >>
> >> I believe MXML is supposed to treat more than one child in a child tag
> as
> >> an array.  And thus, the equivalent AS code for:
> >>
> >> <js:initialView>
> >>  <local:MyInitialView id="view1" />
> >>  <local:MyOtherInitialView id="view2" />
> >> </js:initialView>
> >>
> >> is:
> >>
> >>  initialView = [ new MyInitialView, new MyOtherInitialView];
> >>
> >>
> >>
> >> I understood the same, but I had thought it was supposed to be a bit
> >> smarter than that and only assume an implicit <fx:Array> wrapper if it
> is
> >> assigning to an Array typed property (maybe even 'Arraylike') property.
> I
> >> will check the behavior using Flex 4 - I assume we want to keep whatever
> >> 'smarts' were implemented there, or at least as close as makes sense?
> I'll
> >> also double check the compiler error for this same scenario in Flex 4,
> >> which I did not do yet.
> >>
> >> If you code that in AS, you will get an error (I hope), and I think the
> >> compiler should report the same error for this MXML scenario, since
> >> initialView is of type IApplicationView or something like that.
> >>
> >>
> >> There might arguably be some justification for having an 'mxml version'
> of
> >> an error, describing things more in mxml terms, particularly for new
> users
> >> as they come on board and perhaps play with mxml examples first before
> >> learning actionscript, but I will check both the actionscript error in
> >> FlexJS and the mxml error in Flex 4 (assuming there is one)  for this
> >> scenario and report back here for consensus.
> >>
> >> <js:initialView>
> >>  <js:SimpleCSSValuesImpl />
> >> </js:initialView>
> >>
> >> Is the equivalent of:
> >> var initialView:IApplicationView = new SimpleCSSValuesImpl().
> >>
> >>
> >>
> >> Exactly, and in the fix, I have this currently being described as:
> >> In initializer for 'js:initialView', type org.apache.flex.core.SimpleCSS
> ValuesImpl
> >> is not assignable to target type 'org.apache.flex.core.IApplica
> tionView'.
> >>
> >> I will check the FlexJS actionscript error for this also, but I
> definitely
> >> pulled that one above as-is from Flex 4 for the same mxml problem
> (which is
> >> also correctly treated as a compile time error)
> >>
> >>
> >>
> >> On Fri, Mar 3, 2017 at 6:33 PM, Alex Harui <ah...@adobe.com> wrote:
> >>
> >>> Hi Greg,
> >>>
> >>> Thanks for digging into this.
> >>>
> >>> Without having dug into it myself, I would like to suggest returning a
> >>> type mismatch error of some sort.  IIRC, the DuplicateChildTagProblem
> is
> >>> for the following:
> >>>
> >>> <js:initialView>
> >>>  <local:MyInitialView id="view1" />
> >>> </js:initialView>
> >>> <js:initialView>
> >>>  <local:MyInitialView id="view2" />
> >>>
> >>> </js:initialView>
> >>>
> >>> The child tag (js:initialView) is really in there twice.
> >>>
> >>>
> >>> I believe MXML is supposed to treat more than one child in a child tag
> as
> >>> an array.  And thus, the equivalent AS code for:
> >>>
> >>> <js:initialView>
> >>>  <local:MyInitialView id="view1" />
> >>>  <local:MyOtherInitialView id="view2" />
> >>> </js:initialView>
> >>>
> >>> is:
> >>>
> >>>  initialView = [ new MyInitialView, new MyOtherInitialView];
> >>>
> >>> If you code that in AS, you will get an error (I hope), and I think the
> >>> compiler should report the same error for this MXML scenario, since
> >>> initialView is of type IApplicationView or something like that.
> >>>
> >>> And same for the second scenario:
> >>>
> >>> <js:initialView>
> >>>  <js:SimpleCSSValuesImpl />
> >>> </js:initialView>
> >>>
> >>> Is the equivalent of:
> >>>
> >>>
> >>>  var initialView:IApplicationView = new SimpleCSSValuesImpl().
> >>>
> >>> My 2 cents,
> >>> -Alex
> >>>
> >>> On 3/2/17, 9:09 PM, "Greg Dove" <gr...@apache.org> wrote:
> >>>
> >>>> I have been looking into FLEX-35273 [1].
> >>>>
> >>>> This is a compiler bug where it is possible to do things that don't
> make
> >>>> sense, like:
> >>>>
> >>>> <js:initialView>
> >>>>      <local:MyInitialView id="view1" />
> >>>>      <local:MyInitialView id="view2" />
> >>>> </js:initialView>
> >>>>
> >>>> or even this :
> >>>>
> >>>> <js:initialView>
> >>>>      <js:SimpleCSSValuesImpl />
> >>>> </js:initialView>
> >>>>
> >>>> Neither of the above caused compile time errors.
> >>>>
> >>>> I have a fix for both the above scenarios.
> >>>>
> >>>> For the first one, I used MXMLDuplicateChildTagProblem which seems
> 'close
> >>>> enough'
> >>>>
> >>>> "Child tag '${childTag}' bound to namespace '${childNamespace}' is
> >>>> already specified for element '${element}'. It will be ignored.";
> >>>>
> >>>> This renders as :
> >>>> Child tag 'MyInitialView' bound to namespace '*' is already specified
> for
> >>>> element 'js:initialView'. It will be ignored.
> >>>>  <local:MyInitialView id="view2" />
> >>>>   ^
> >>>>
> >>>> in the first example above. The text does not feel entirely right, but
> >>>> seems close enough. "It will be ignored" sounds more like a warning
> (as
> >>>> opposed to an error/failure), which I think a) it should not be and
> b) it
> >>>> is not.
> >>>>
> >>>> For the second one, I could not find any relevant 'Problem' class (I
> may
> >>>> have missed one perhaps?) So I just made a new one. I don't really
> know
> >>>> what the rules are here.
> >>>> These Problems seem to include an error code so I am unclear what to
> do
> >>>> if I add a new one. i.e. it is not clear what new error code I should
> >>>> apply (or even if the error code is important for something).
> >>>> For now I just added an arbitrary errorCode = 1999 on
> >>>> MXMLBadChildTagPropertyAssignmentProblem.
> >>>>
> >>>> The new problem renders out to :
> >>>> In initializer for 'js:initialView', type
> >>>> org.apache.flex.core.SimpleCSSValuesImpl is not assignable to target
> >>> type
> >>>> 'org.apache.flex.core.IApplicationView'.
> >>>>
> >>>> Alex, you may be able to advise here.... are there any rules I need to
> >>>> follow for the CompilerProblem classes (in particular, when adding a
> new
> >>>> class).
> >>>>
> >>>> Assuming all is well above, I will commit this fix tomorrow. I was
> also
> >>>> trying to see if I could figure out how to get checkintests running,
> but
> >>>> I so far I did not. However the regular build tests are all fine with
> the
> >>>> changes I made for this.
> >>>>
> >>>>
> >>>> 1. https://issues.apache.org/jira/browse/FLEX-35273
> >>>
> >>>
> >>
>
>

Re: [FlexJS] Working to fix a compiler bug - problem with the Problems

Posted by Harbs <ha...@gmail.com>.

I have to admit that you lost me pretty quickly… ;-)

It sound to me like you know what you’re talking about, so I’m fine with whatever you did/want to do… :-)

Harbs

> On Mar 5, 2017, at 9:31 AM, Greg Dove <gr...@gmail.com> wrote:
> 
> OK.... please brace yourself for an onslaught of text.
> 
> I tested across Flex 4 and FlexJS through a bunch of variations. This
> change passes all tests. In the end this is a very small and isolated code
> change (I am almost certain it will not contribute to anyone's merge
> conflicts) and I believe it gives greater compile-time type safety to mxml,
> so I think it's necessary. But the error descriptions are open for more
> input/review... I have favored a strong consistency with the Flex 4
> descriptions, which were specific for mxml, over using the related
> actionscript descriptions.
> 
> BTW I had forgotten, but there was also a comment (it was not marked as a
> todo) in the falcon MXMLPropertySpecifierNode class that indicated that
> this type of work still needed to be done, so I expect it was an unfinished
> implementation.
> 
> If I hear nothing conflciting by EOD tomorrow I will commit what I have. It
> can easily be reverted if anyone finds issues, or I am happy to address it
> further if it is not correct for some cases that I did not anticipate.
> 
> That's the end of the preamble.... best of luck getting through what
> follows... it took me longer than the original coding part :)
> 
> Test setup for Flex 4:
> 'TestView' below is simply a Flex4 Group mxml component, like so:
> <s:Group xmlns:fx="http://ns.adobe.com/mxml/2009"
>         xmlns:s="library://ns.adobe.com/flex/spark"
>         creationComplete="onCreated()">
>    <fx:Script><![CDATA[
>        import spark.components.Alert;
>        private function onCreated():void{
>          //check for actionscript version of error
>          //this.number="something";
>        }
>        public var testArr:Array;
>        public var number:Number;
>        ]]></fx:Script>
>    <s:layout>
>        <s:VerticalLayout/>
>    </s:layout>
>    <s:Button click="test.text='testArr ['+testArr.toString()+'],\nnumber
> val:'+number" width="100" height="50"/>
>    <s:Label id="test"/>
> </s:Group>
> 
> Test setup for FlexJS:
> 'TestView' below is simply a Flex4 Group like so:
> <js:View xmlns:fx="http://ns.adobe.com/mxml/2009"
> xmlns:js="library://ns.apache.org/flexjs/basic"
> initComplete="onCreated()">
>   <fx:Script><![CDATA[
> import org.apache.flex.html.Alert;
>        private function onCreated():void{
>          //check for actionscript version of error
>          //this.number="something";
>        }
>        public var testArr:Array;
>        public var number:Number;
>        ]]></fx:Script>
> <js:Button click="Alert.show('testArr ['+testArr.toString()+'],\nnumber
> val:'+number, this)" width="100" height="50"/>
> </js:View>
> 
> 
> Testing:
>    <local:TestView id="test">
>        <local:testArr>
>            <fx:Number>23.56</fx:Number>
>            <fx:Number>23.99</fx:Number>
>            <fx:String>Something else</fx:String>
>        </local:testArr>
>        <local:number>
>            <fx:Number>23.56</fx:Number>
>        </local:number>
>    </local:TestView>
> The above compiles correctly and because the testArr property has an
> 'Array' type, the children are inferred as array elements, it is the same
> as specifying:
>        <local:testArr>
>            <fx:Array>
>                <fx:Number>23.56</fx:Number>
>                <fx:Number>23.99</fx:Number>
>                <fx:String>Something else</fx:String>
>            </fx:Array>
>        </local:testArr>
> Note: Using one vs. multiple children: the inference is the same - if the
> property being initialized is of type Array, it infers correctly (and
> performs the 'Array' wrapping if needed) irrespective of single or multiple
> child tags.
> NO BUGS FOR ARRAY (inferred or explicit)
> This works in both Flex 4 and Falcon (before or after my local changes)
> Multiple value children when the property being initialized is not 'Array'
> type
> ---------------------------------------------------------------------
> the 'number' property on the other hand is of type Number, and changing
> number assignment to multiple values, in this way:
>   <local:number>
>            <fx:Number>23.56</fx:Number>
> <fx:Number>23.99</fx:Number>
>        </local:number>
> gives Flex 4 compiler errors like so:
> Error:(20, 0) [flex4testforFlexJS]: In initializer for 'number', multiple
> initializer values for target type Number.
>    Error:(21, 0) [flex4testforFlexJS]: In initializer for 'number',
> multiple initializer values for target type Number.
> (adding more than 2 gives an individual error for each child)
> <fx:Number>23.56</fx:Number>
>                <fx:Number>23.99</fx:Number>
>            <fx:String>Something else</fx:String>
>        </local:number>
> Swapping to one element with wrong type in multiple child assignment does
> not change the error reported (it remains simply 'multiple values'):
> Error:(20, 0) [flex4testforFlexJS]: In initializer for 'number', multiple
> initializer values for target type Number.
> Error:(21, 0) [flex4testforFlexJS]: In initializer for 'number', multiple
> initializer values for target type Number.
> CURRENT BUG
> -----------
> Currently in FlexJS, the following happens for either of the above
> scenarios:
> Only the last child tag is processed in multiple tags like this, the
> earlier ones are ignored. It is processed as if it were the only child tag
> (and it may be of an incompatible type).
> PROPOSED FIX(DONE)
> -----------------
> Please let me know if you agree, or suggest alternative.
> in the local fix for flexjs, I have now created a new Problem,
> MXMLMultipleInitializersProblem, which follows the Flex 4 example
> with outputs descriptions similar to:
> GenericTests.mxml(57): col: 17 Error: In initializer for 'local:number'
> multiple initializer values are not permitted for target type 'Number'.
>                <fx:Number>23.99</fx:Number>
>                ^
> GenericTests.mxml(58): col: 13 Error: In initializer for 'local:number'
> multiple initializer values are not permitted for target type 'Number'.
>            <fx:String>Something else</fx:String>
>            ^
> Inappropriate single child when property being initialized is not 'Array'
> type
> ----------------------------------------------------------------------------
> <local:number>
>        <fx:String>Something else</fx:String>
>    </local:number>
> in Flex 4, this gives:
> Error:(22, 0) [flex4testforFlexJS]: In initializer for 'number', type
> String is not assignable to target type 'Number'.
> The corresponding Flex 4 actionscript error is
> Error:(11, 0) [flex4testforFlexJS]: Error code: 1067: Implicit coercion of
> a value of type String to an unrelated type Number.
> If FlexJS, the actionscript error is the same
> CURRENT BUG
> -----------
> in Falcon this currently does not cause a compiler error for mxml,
> compilation completes normally, and in js the value of number will be the
> "Something else", in swf it is NaN. IMO this is clearly wrong.
> 
> 
> PROPOSED FIX (DONE)
> -------------------
> Please let me know if you agree, or suggest alternative.
> I have added a new Problem to be closer to what was shown before in Flex 4:
> This is the new one I mirrored from Flex 4 (with same description) in FlexJS
>    GenericTests.mxml(56): col: 4 Error: In initializer for 'local:number',
> type String is not assignable to target type 'Number'.
>            <fx:String>Something else</fx:String>
> ^
> Multiple initializers
> ---------------------
>   <local:number>
>            <fx:Number>23.56</fx:Number>
>        </local:number>
>        <local:number>
>            <fx:Number>23.56</fx:Number>
>        </local:number>
> this gives the following in Flex 4:
> Error:(24, 0) [flex4testforFlexJS]: Multiple initializers for property
> 'number'.
> In FlexJS (with or without the proposed fixes for the other issues above)
> GenericTests.mxml(58): col: 9 Error: Child tag 'number' bound to namespace
> '*' is already specified for element 'local:TestView'. It will be ignored.
>        <local:number>
>        ^
> (In both Flex 4 and FlexJS the line number reported is for the beginning of
> the 2nd initializer tag <local:number>, which is correct)
> CURRENT BUG
> -----------
> I believe the error description should be improved - it currently says "It
> will be ignored" and that is not what actually happens (not being ignored
> is correct, IMO).
> And (unless I am missing something important) the rest seems too verbose.
> PROPOSED FIX (NOT DONE):
> -----------------------
> This is easy to change.... please let me know if you agree.
> I suggest changing the FlexJS error description to be similar to Flex 4, to:
> Multiple initializers for 'local:number' are not permitted for element
> 'local:TestView'.
>   <local:number>
>            ^
> 
> 
> 
> 
> On Sat, Mar 4, 2017 at 8:46 PM, Greg Dove <gr...@gmail.com> wrote:
> 
>> 
>> I believe MXML is supposed to treat more than one child in a child tag as
>> an array.  And thus, the equivalent AS code for:
>> 
>> <js:initialView>
>>  <local:MyInitialView id="view1" />
>>  <local:MyOtherInitialView id="view2" />
>> </js:initialView>
>> 
>> is:
>> 
>>  initialView = [ new MyInitialView, new MyOtherInitialView];
>> 
>> 
>> 
>> I understood the same, but I had thought it was supposed to be a bit
>> smarter than that and only assume an implicit <fx:Array> wrapper if it is
>> assigning to an Array typed property (maybe even 'Arraylike') property. I
>> will check the behavior using Flex 4 - I assume we want to keep whatever
>> 'smarts' were implemented there, or at least as close as makes sense? I'll
>> also double check the compiler error for this same scenario in Flex 4,
>> which I did not do yet.
>> 
>> If you code that in AS, you will get an error (I hope), and I think the
>> compiler should report the same error for this MXML scenario, since
>> initialView is of type IApplicationView or something like that.
>> 
>> 
>> There might arguably be some justification for having an 'mxml version' of
>> an error, describing things more in mxml terms, particularly for new users
>> as they come on board and perhaps play with mxml examples first before
>> learning actionscript, but I will check both the actionscript error in
>> FlexJS and the mxml error in Flex 4 (assuming there is one)  for this
>> scenario and report back here for consensus.
>> 
>> <js:initialView>
>>  <js:SimpleCSSValuesImpl />
>> </js:initialView>
>> 
>> Is the equivalent of:
>> var initialView:IApplicationView = new SimpleCSSValuesImpl().
>> 
>> 
>> 
>> Exactly, and in the fix, I have this currently being described as:
>> In initializer for 'js:initialView', type org.apache.flex.core.SimpleCSSValuesImpl
>> is not assignable to target type 'org.apache.flex.core.IApplicationView'.
>> 
>> I will check the FlexJS actionscript error for this also, but I definitely
>> pulled that one above as-is from Flex 4 for the same mxml problem (which is
>> also correctly treated as a compile time error)
>> 
>> 
>> 
>> On Fri, Mar 3, 2017 at 6:33 PM, Alex Harui <ah...@adobe.com> wrote:
>> 
>>> Hi Greg,
>>> 
>>> Thanks for digging into this.
>>> 
>>> Without having dug into it myself, I would like to suggest returning a
>>> type mismatch error of some sort.  IIRC, the DuplicateChildTagProblem is
>>> for the following:
>>> 
>>> <js:initialView>
>>>  <local:MyInitialView id="view1" />
>>> </js:initialView>
>>> <js:initialView>
>>>  <local:MyInitialView id="view2" />
>>> 
>>> </js:initialView>
>>> 
>>> The child tag (js:initialView) is really in there twice.
>>> 
>>> 
>>> I believe MXML is supposed to treat more than one child in a child tag as
>>> an array.  And thus, the equivalent AS code for:
>>> 
>>> <js:initialView>
>>>  <local:MyInitialView id="view1" />
>>>  <local:MyOtherInitialView id="view2" />
>>> </js:initialView>
>>> 
>>> is:
>>> 
>>>  initialView = [ new MyInitialView, new MyOtherInitialView];
>>> 
>>> If you code that in AS, you will get an error (I hope), and I think the
>>> compiler should report the same error for this MXML scenario, since
>>> initialView is of type IApplicationView or something like that.
>>> 
>>> And same for the second scenario:
>>> 
>>> <js:initialView>
>>>  <js:SimpleCSSValuesImpl />
>>> </js:initialView>
>>> 
>>> Is the equivalent of:
>>> 
>>> 
>>>  var initialView:IApplicationView = new SimpleCSSValuesImpl().
>>> 
>>> My 2 cents,
>>> -Alex
>>> 
>>> On 3/2/17, 9:09 PM, "Greg Dove" <gr...@apache.org> wrote:
>>> 
>>>> I have been looking into FLEX-35273 [1].
>>>> 
>>>> This is a compiler bug where it is possible to do things that don't make
>>>> sense, like:
>>>> 
>>>> <js:initialView>
>>>>      <local:MyInitialView id="view1" />
>>>>      <local:MyInitialView id="view2" />
>>>> </js:initialView>
>>>> 
>>>> or even this :
>>>> 
>>>> <js:initialView>
>>>>      <js:SimpleCSSValuesImpl />
>>>> </js:initialView>
>>>> 
>>>> Neither of the above caused compile time errors.
>>>> 
>>>> I have a fix for both the above scenarios.
>>>> 
>>>> For the first one, I used MXMLDuplicateChildTagProblem which seems 'close
>>>> enough'
>>>> 
>>>> "Child tag '${childTag}' bound to namespace '${childNamespace}' is
>>>> already specified for element '${element}'. It will be ignored.";
>>>> 
>>>> This renders as :
>>>> Child tag 'MyInitialView' bound to namespace '*' is already specified for
>>>> element 'js:initialView'. It will be ignored.
>>>>  <local:MyInitialView id="view2" />
>>>>   ^
>>>> 
>>>> in the first example above. The text does not feel entirely right, but
>>>> seems close enough. "It will be ignored" sounds more like a warning (as
>>>> opposed to an error/failure), which I think a) it should not be and b) it
>>>> is not.
>>>> 
>>>> For the second one, I could not find any relevant 'Problem' class (I may
>>>> have missed one perhaps?) So I just made a new one. I don't really know
>>>> what the rules are here.
>>>> These Problems seem to include an error code so I am unclear what to do
>>>> if I add a new one. i.e. it is not clear what new error code I should
>>>> apply (or even if the error code is important for something).
>>>> For now I just added an arbitrary errorCode = 1999 on
>>>> MXMLBadChildTagPropertyAssignmentProblem.
>>>> 
>>>> The new problem renders out to :
>>>> In initializer for 'js:initialView', type
>>>> org.apache.flex.core.SimpleCSSValuesImpl is not assignable to target
>>> type
>>>> 'org.apache.flex.core.IApplicationView'.
>>>> 
>>>> Alex, you may be able to advise here.... are there any rules I need to
>>>> follow for the CompilerProblem classes (in particular, when adding a new
>>>> class).
>>>> 
>>>> Assuming all is well above, I will commit this fix tomorrow. I was also
>>>> trying to see if I could figure out how to get checkintests running, but
>>>> I so far I did not. However the regular build tests are all fine with the
>>>> changes I made for this.
>>>> 
>>>> 
>>>> 1. https://issues.apache.org/jira/browse/FLEX-35273
>>> 
>>> 
>>

Re: [FlexJS] Working to fix a compiler bug - problem with the Problems

Posted by Alex Harui <ah...@adobe.com>.

Hi Greg,

My main goal was to make sure the code that decides some set of child tags
is an array wasn't going to get messed up by trying to fix these bugs,
especially the multiple initializers being assigned to something that
isn't an Array.

Other than that, I'm really don't have any opinion on what the error
message really is.  So do what makes sense and folks who actually hit
these errors will tell us if they are confused.

Thanks for such detailed work,
-Alex


On 3/4/17, 11:31 PM, "Greg Dove" <gr...@gmail.com> wrote:

>OK.... please brace yourself for an onslaught of text.
>
>I tested across Flex 4 and FlexJS through a bunch of variations. This
>change passes all tests. In the end this is a very small and isolated code
>change (I am almost certain it will not contribute to anyone's merge
>conflicts) and I believe it gives greater compile-time type safety to
>mxml,
>so I think it's necessary. But the error descriptions are open for more
>input/review... I have favored a strong consistency with the Flex 4
>descriptions, which were specific for mxml, over using the related
>actionscript descriptions.
>
>BTW I had forgotten, but there was also a comment (it was not marked as a
>todo) in the falcon MXMLPropertySpecifierNode class that indicated that
>this type of work still needed to be done, so I expect it was an
>unfinished
>implementation.
>
>If I hear nothing conflciting by EOD tomorrow I will commit what I have.
>It
>can easily be reverted if anyone finds issues, or I am happy to address it
>further if it is not correct for some cases that I did not anticipate.
>
>That's the end of the preamble.... best of luck getting through what
>follows... it took me longer than the original coding part :)
>
>Test setup for Flex 4:
>'TestView' below is simply a Flex4 Group mxml component, like so:
><s:Group xmlns:fx="http://ns.adobe.com/mxml/2009"
>         xmlns:s="library://ns.adobe.com/flex/spark"
>         creationComplete="onCreated()">
>    <fx:Script><![CDATA[
>        import spark.components.Alert;
>        private function onCreated():void{
>          //check for actionscript version of error
>          //this.number="something";
>        }
>        public var testArr:Array;
>        public var number:Number;
>        ]]></fx:Script>
>    <s:layout>
>        <s:VerticalLayout/>
>    </s:layout>
>    <s:Button click="test.text='testArr ['+testArr.toString()+'],\nnumber
>val:'+number" width="100" height="50"/>
>    <s:Label id="test"/>
></s:Group>
>
>Test setup for FlexJS:
>'TestView' below is simply a Flex4 Group like so:
><js:View xmlns:fx="http://ns.adobe.com/mxml/2009"
>xmlns:js="library://ns.apache.org/flexjs/basic"
>initComplete="onCreated()">
>   <fx:Script><![CDATA[
>import org.apache.flex.html.Alert;
>        private function onCreated():void{
>          //check for actionscript version of error
>          //this.number="something";
>        }
>        public var testArr:Array;
>        public var number:Number;
>        ]]></fx:Script>
><js:Button click="Alert.show('testArr ['+testArr.toString()+'],\nnumber
>val:'+number, this)" width="100" height="50"/>
></js:View>
>
>
>Testing:
>    <local:TestView id="test">
>        <local:testArr>
>            <fx:Number>23.56</fx:Number>
>            <fx:Number>23.99</fx:Number>
>            <fx:String>Something else</fx:String>
>        </local:testArr>
>        <local:number>
>            <fx:Number>23.56</fx:Number>
>        </local:number>
>    </local:TestView>
>The above compiles correctly and because the testArr property has an
>'Array' type, the children are inferred as array elements, it is the same
>as specifying:
>        <local:testArr>
>            <fx:Array>
>                <fx:Number>23.56</fx:Number>
>                <fx:Number>23.99</fx:Number>
>                <fx:String>Something else</fx:String>
>            </fx:Array>
>        </local:testArr>
>Note: Using one vs. multiple children: the inference is the same - if the
>property being initialized is of type Array, it infers correctly (and
>performs the 'Array' wrapping if needed) irrespective of single or
>multiple
>child tags.
>NO BUGS FOR ARRAY (inferred or explicit)
>This works in both Flex 4 and Falcon (before or after my local changes)
>Multiple value children when the property being initialized is not 'Array'
>type
>---------------------------------------------------------------------
>the 'number' property on the other hand is of type Number, and changing
>number assignment to multiple values, in this way:
>   <local:number>
>            <fx:Number>23.56</fx:Number>
><fx:Number>23.99</fx:Number>
>        </local:number>
>gives Flex 4 compiler errors like so:
>Error:(20, 0) [flex4testforFlexJS]: In initializer for 'number', multiple
>initializer values for target type Number.
>    Error:(21, 0) [flex4testforFlexJS]: In initializer for 'number',
>multiple initializer values for target type Number.
>(adding more than 2 gives an individual error for each child)
><fx:Number>23.56</fx:Number>
>                <fx:Number>23.99</fx:Number>
>            <fx:String>Something else</fx:String>
>        </local:number>
>Swapping to one element with wrong type in multiple child assignment does
>not change the error reported (it remains simply 'multiple values'):
>Error:(20, 0) [flex4testforFlexJS]: In initializer for 'number', multiple
>initializer values for target type Number.
>Error:(21, 0) [flex4testforFlexJS]: In initializer for 'number', multiple
>initializer values for target type Number.
>CURRENT BUG
>-----------
>Currently in FlexJS, the following happens for either of the above
>scenarios:
>Only the last child tag is processed in multiple tags like this, the
>earlier ones are ignored. It is processed as if it were the only child tag
>(and it may be of an incompatible type).
>PROPOSED FIX(DONE)
>-----------------
>Please let me know if you agree, or suggest alternative.
>in the local fix for flexjs, I have now created a new Problem,
>MXMLMultipleInitializersProblem, which follows the Flex 4 example
>with outputs descriptions similar to:
>GenericTests.mxml(57): col: 17 Error: In initializer for 'local:number'
>multiple initializer values are not permitted for target type 'Number'.
>                <fx:Number>23.99</fx:Number>
>                ^
>GenericTests.mxml(58): col: 13 Error: In initializer for 'local:number'
>multiple initializer values are not permitted for target type 'Number'.
>            <fx:String>Something else</fx:String>
>            ^
>Inappropriate single child when property being initialized is not 'Array'
>type
>--------------------------------------------------------------------------
>--
><local:number>
>        <fx:String>Something else</fx:String>
>    </local:number>
>in Flex 4, this gives:
>Error:(22, 0) [flex4testforFlexJS]: In initializer for 'number', type
>String is not assignable to target type 'Number'.
>The corresponding Flex 4 actionscript error is
>Error:(11, 0) [flex4testforFlexJS]: Error code: 1067: Implicit coercion of
>a value of type String to an unrelated type Number.
>If FlexJS, the actionscript error is the same
>CURRENT BUG
>-----------
>in Falcon this currently does not cause a compiler error for mxml,
>compilation completes normally, and in js the value of number will be the
>"Something else", in swf it is NaN. IMO this is clearly wrong.
>
>
>PROPOSED FIX (DONE)
>-------------------
>Please let me know if you agree, or suggest alternative.
>I have added a new Problem to be closer to what was shown before in Flex
>4:
>This is the new one I mirrored from Flex 4 (with same description) in
>FlexJS
>    GenericTests.mxml(56): col: 4 Error: In initializer for
>'local:number',
>type String is not assignable to target type 'Number'.
>            <fx:String>Something else</fx:String>
>^
>Multiple initializers
>---------------------
>   <local:number>
>            <fx:Number>23.56</fx:Number>
>        </local:number>
>        <local:number>
>            <fx:Number>23.56</fx:Number>
>        </local:number>
>this gives the following in Flex 4:
>Error:(24, 0) [flex4testforFlexJS]: Multiple initializers for property
>'number'.
>In FlexJS (with or without the proposed fixes for the other issues above)
>GenericTests.mxml(58): col: 9 Error: Child tag 'number' bound to namespace
>'*' is already specified for element 'local:TestView'. It will be ignored.
>        <local:number>
>        ^
>(In both Flex 4 and FlexJS the line number reported is for the beginning
>of
>the 2nd initializer tag <local:number>, which is correct)
>CURRENT BUG
>-----------
>I believe the error description should be improved - it currently says "It
>will be ignored" and that is not what actually happens (not being ignored
>is correct, IMO).
>And (unless I am missing something important) the rest seems too verbose.
>PROPOSED FIX (NOT DONE):
>-----------------------
>This is easy to change.... please let me know if you agree.
>I suggest changing the FlexJS error description to be similar to Flex 4,
>to:
>Multiple initializers for 'local:number' are not permitted for element
>'local:TestView'.
>   <local:number>
>            ^
>
>
>
>
>On Sat, Mar 4, 2017 at 8:46 PM, Greg Dove <gr...@gmail.com> wrote:
>
>>
>> I believe MXML is supposed to treat more than one child in a child tag
>>as
>> an array.  And thus, the equivalent AS code for:
>>
>> <js:initialView>
>>   <local:MyInitialView id="view1" />
>>   <local:MyOtherInitialView id="view2" />
>> </js:initialView>
>>
>> is:
>>
>>   initialView = [ new MyInitialView, new MyOtherInitialView];
>>
>>
>>
>> I understood the same, but I had thought it was supposed to be a bit
>> smarter than that and only assume an implicit <fx:Array> wrapper if it
>>is
>> assigning to an Array typed property (maybe even 'Arraylike') property.
>>I
>> will check the behavior using Flex 4 - I assume we want to keep whatever
>> 'smarts' were implemented there, or at least as close as makes sense?
>>I'll
>> also double check the compiler error for this same scenario in Flex 4,
>> which I did not do yet.
>>
>> If you code that in AS, you will get an error (I hope), and I think the
>> compiler should report the same error for this MXML scenario, since
>> initialView is of type IApplicationView or something like that.
>>
>>
>> There might arguably be some justification for having an 'mxml version'
>>of
>> an error, describing things more in mxml terms, particularly for new
>>users
>> as they come on board and perhaps play with mxml examples first before
>> learning actionscript, but I will check both the actionscript error in
>> FlexJS and the mxml error in Flex 4 (assuming there is one)  for this
>> scenario and report back here for consensus.
>>
>> <js:initialView>
>>   <js:SimpleCSSValuesImpl />
>> </js:initialView>
>>
>> Is the equivalent of:
>> var initialView:IApplicationView = new SimpleCSSValuesImpl().
>>
>>
>>
>> Exactly, and in the fix, I have this currently being described as:
>> In initializer for 'js:initialView', type
>>org.apache.flex.core.SimpleCSSValuesImpl
>> is not assignable to target type
>>'org.apache.flex.core.IApplicationView'.
>>
>> I will check the FlexJS actionscript error for this also, but I
>>definitely
>> pulled that one above as-is from Flex 4 for the same mxml problem
>>(which is
>> also correctly treated as a compile time error)
>>
>>
>>
>> On Fri, Mar 3, 2017 at 6:33 PM, Alex Harui <ah...@adobe.com> wrote:
>>
>>> Hi Greg,
>>>
>>> Thanks for digging into this.
>>>
>>> Without having dug into it myself, I would like to suggest returning a
>>> type mismatch error of some sort.  IIRC, the DuplicateChildTagProblem
>>>is
>>> for the following:
>>>
>>> <js:initialView>
>>>   <local:MyInitialView id="view1" />
>>> </js:initialView>
>>> <js:initialView>
>>>   <local:MyInitialView id="view2" />
>>>
>>> </js:initialView>
>>>
>>> The child tag (js:initialView) is really in there twice.
>>>
>>>
>>> I believe MXML is supposed to treat more than one child in a child tag
>>>as
>>> an array.  And thus, the equivalent AS code for:
>>>
>>> <js:initialView>
>>>   <local:MyInitialView id="view1" />
>>>   <local:MyOtherInitialView id="view2" />
>>> </js:initialView>
>>>
>>> is:
>>>
>>>   initialView = [ new MyInitialView, new MyOtherInitialView];
>>>
>>> If you code that in AS, you will get an error (I hope), and I think the
>>> compiler should report the same error for this MXML scenario, since
>>> initialView is of type IApplicationView or something like that.
>>>
>>> And same for the second scenario:
>>>
>>> <js:initialView>
>>>   <js:SimpleCSSValuesImpl />
>>> </js:initialView>
>>>
>>> Is the equivalent of:
>>>
>>>
>>>   var initialView:IApplicationView = new SimpleCSSValuesImpl().
>>>
>>> My 2 cents,
>>> -Alex
>>>
>>> On 3/2/17, 9:09 PM, "Greg Dove" <gr...@apache.org> wrote:
>>>
>>> >I have been looking into FLEX-35273 [1].
>>> >
>>> >This is a compiler bug where it is possible to do things that don't
>>>make
>>> >sense, like:
>>> >
>>> ><js:initialView>
>>> >       <local:MyInitialView id="view1" />
>>> >       <local:MyInitialView id="view2" />
>>> ></js:initialView>
>>> >
>>> >or even this :
>>> >
>>> ><js:initialView>
>>> >       <js:SimpleCSSValuesImpl />
>>> ></js:initialView>
>>> >
>>> >Neither of the above caused compile time errors.
>>> >
>>> >I have a fix for both the above scenarios.
>>> >
>>> >For the first one, I used MXMLDuplicateChildTagProblem which seems
>>>'close
>>> >enough'
>>> >
>>> >"Child tag '${childTag}' bound to namespace '${childNamespace}' is
>>> >already specified for element '${element}'. It will be ignored.";
>>> >
>>> >This renders as :
>>> >Child tag 'MyInitialView' bound to namespace '*' is already specified
>>>for
>>> >element 'js:initialView'. It will be ignored.
>>> >   <local:MyInitialView id="view2" />
>>> >    ^
>>> >
>>> >in the first example above. The text does not feel entirely right, but
>>> >seems close enough. "It will be ignored" sounds more like a warning
>>>(as
>>> >opposed to an error/failure), which I think a) it should not be and
>>>b) it
>>> >is not.
>>> >
>>> >For the second one, I could not find any relevant 'Problem' class (I
>>>may
>>> >have missed one perhaps?) So I just made a new one. I don't really
>>>know
>>> >what the rules are here.
>>> >These Problems seem to include an error code so I am unclear what to
>>>do
>>> >if I add a new one. i.e. it is not clear what new error code I should
>>> >apply (or even if the error code is important for something).
>>> >For now I just added an arbitrary errorCode = 1999 on
>>> >MXMLBadChildTagPropertyAssignmentProblem.
>>> >
>>> >The new problem renders out to :
>>> >In initializer for 'js:initialView', type
>>> >org.apache.flex.core.SimpleCSSValuesImpl is not assignable to target
>>> type
>>> >'org.apache.flex.core.IApplicationView'.
>>> >
>>> >Alex, you may be able to advise here.... are there any rules I need to
>>> >follow for the CompilerProblem classes (in particular, when adding a
>>>new
>>> >class).
>>> >
>>> >Assuming all is well above, I will commit this fix tomorrow. I was
>>>also
>>> >trying to see if I could figure out how to get checkintests running,
>>>but
>>> >I so far I did not. However the regular build tests are all fine with
>>>the
>>> >changes I made for this.
>>> >
>>> >
>>> >1. https://issues.apache.org/jira/browse/FLEX-35273
>>>
>>>
>>

Re: [FlexJS] Working to fix a compiler bug - problem with the Problems

Posted by Greg Dove <gr...@gmail.com>.

OK.... please brace yourself for an onslaught of text.

I tested across Flex 4 and FlexJS through a bunch of variations. This
change passes all tests. In the end this is a very small and isolated code
change (I am almost certain it will not contribute to anyone's merge
conflicts) and I believe it gives greater compile-time type safety to mxml,
so I think it's necessary. But the error descriptions are open for more
input/review... I have favored a strong consistency with the Flex 4
descriptions, which were specific for mxml, over using the related
actionscript descriptions.

BTW I had forgotten, but there was also a comment (it was not marked as a
todo) in the falcon MXMLPropertySpecifierNode class that indicated that
this type of work still needed to be done, so I expect it was an unfinished
implementation.

If I hear nothing conflciting by EOD tomorrow I will commit what I have. It
can easily be reverted if anyone finds issues, or I am happy to address it
further if it is not correct for some cases that I did not anticipate.

That's the end of the preamble.... best of luck getting through what
follows... it took me longer than the original coding part :)

Test setup for Flex 4:
'TestView' below is simply a Flex4 Group mxml component, like so:
<s:Group xmlns:fx="http://ns.adobe.com/mxml/2009"
         xmlns:s="library://ns.adobe.com/flex/spark"
         creationComplete="onCreated()">
    <fx:Script><![CDATA[
        import spark.components.Alert;
        private function onCreated():void{
          //check for actionscript version of error
          //this.number="something";
        }
        public var testArr:Array;
        public var number:Number;
        ]]></fx:Script>
    <s:layout>
        <s:VerticalLayout/>
    </s:layout>
    <s:Button click="test.text='testArr ['+testArr.toString()+'],\nnumber
val:'+number" width="100" height="50"/>
    <s:Label id="test"/>
</s:Group>

Test setup for FlexJS:
'TestView' below is simply a Flex4 Group like so:
<js:View xmlns:fx="http://ns.adobe.com/mxml/2009"
xmlns:js="library://ns.apache.org/flexjs/basic"
initComplete="onCreated()">
   <fx:Script><![CDATA[
import org.apache.flex.html.Alert;
        private function onCreated():void{
          //check for actionscript version of error
          //this.number="something";
        }
        public var testArr:Array;
        public var number:Number;
        ]]></fx:Script>
<js:Button click="Alert.show('testArr ['+testArr.toString()+'],\nnumber
val:'+number, this)" width="100" height="50"/>
</js:View>


Testing:
    <local:TestView id="test">
        <local:testArr>
            <fx:Number>23.56</fx:Number>
            <fx:Number>23.99</fx:Number>
            <fx:String>Something else</fx:String>
        </local:testArr>
        <local:number>
            <fx:Number>23.56</fx:Number>
        </local:number>
    </local:TestView>
The above compiles correctly and because the testArr property has an
'Array' type, the children are inferred as array elements, it is the same
as specifying:
        <local:testArr>
            <fx:Array>
                <fx:Number>23.56</fx:Number>
                <fx:Number>23.99</fx:Number>
                <fx:String>Something else</fx:String>
            </fx:Array>
        </local:testArr>
Note: Using one vs. multiple children: the inference is the same - if the
property being initialized is of type Array, it infers correctly (and
performs the 'Array' wrapping if needed) irrespective of single or multiple
child tags.
NO BUGS FOR ARRAY (inferred or explicit)
This works in both Flex 4 and Falcon (before or after my local changes)
Multiple value children when the property being initialized is not 'Array'
type
---------------------------------------------------------------------
the 'number' property on the other hand is of type Number, and changing
number assignment to multiple values, in this way:
   <local:number>
            <fx:Number>23.56</fx:Number>
<fx:Number>23.99</fx:Number>
        </local:number>
gives Flex 4 compiler errors like so:
Error:(20, 0) [flex4testforFlexJS]: In initializer for 'number', multiple
initializer values for target type Number.
    Error:(21, 0) [flex4testforFlexJS]: In initializer for 'number',
multiple initializer values for target type Number.
(adding more than 2 gives an individual error for each child)
<fx:Number>23.56</fx:Number>
                <fx:Number>23.99</fx:Number>
            <fx:String>Something else</fx:String>
        </local:number>
Swapping to one element with wrong type in multiple child assignment does
not change the error reported (it remains simply 'multiple values'):
Error:(20, 0) [flex4testforFlexJS]: In initializer for 'number', multiple
initializer values for target type Number.
Error:(21, 0) [flex4testforFlexJS]: In initializer for 'number', multiple
initializer values for target type Number.
CURRENT BUG
-----------
Currently in FlexJS, the following happens for either of the above
scenarios:
Only the last child tag is processed in multiple tags like this, the
earlier ones are ignored. It is processed as if it were the only child tag
(and it may be of an incompatible type).
PROPOSED FIX(DONE)
-----------------
Please let me know if you agree, or suggest alternative.
in the local fix for flexjs, I have now created a new Problem,
MXMLMultipleInitializersProblem, which follows the Flex 4 example
with outputs descriptions similar to:
GenericTests.mxml(57): col: 17 Error: In initializer for 'local:number'
multiple initializer values are not permitted for target type 'Number'.
                <fx:Number>23.99</fx:Number>
                ^
GenericTests.mxml(58): col: 13 Error: In initializer for 'local:number'
multiple initializer values are not permitted for target type 'Number'.
            <fx:String>Something else</fx:String>
            ^
Inappropriate single child when property being initialized is not 'Array'
type
----------------------------------------------------------------------------
<local:number>
        <fx:String>Something else</fx:String>
    </local:number>
in Flex 4, this gives:
Error:(22, 0) [flex4testforFlexJS]: In initializer for 'number', type
String is not assignable to target type 'Number'.
The corresponding Flex 4 actionscript error is
Error:(11, 0) [flex4testforFlexJS]: Error code: 1067: Implicit coercion of
a value of type String to an unrelated type Number.
If FlexJS, the actionscript error is the same
CURRENT BUG
-----------
in Falcon this currently does not cause a compiler error for mxml,
compilation completes normally, and in js the value of number will be the
"Something else", in swf it is NaN. IMO this is clearly wrong.


PROPOSED FIX (DONE)
-------------------
Please let me know if you agree, or suggest alternative.
I have added a new Problem to be closer to what was shown before in Flex 4:
This is the new one I mirrored from Flex 4 (with same description) in FlexJS
    GenericTests.mxml(56): col: 4 Error: In initializer for 'local:number',
type String is not assignable to target type 'Number'.
            <fx:String>Something else</fx:String>
^
Multiple initializers
---------------------
   <local:number>
            <fx:Number>23.56</fx:Number>
        </local:number>
        <local:number>
            <fx:Number>23.56</fx:Number>
        </local:number>
this gives the following in Flex 4:
Error:(24, 0) [flex4testforFlexJS]: Multiple initializers for property
'number'.
In FlexJS (with or without the proposed fixes for the other issues above)
GenericTests.mxml(58): col: 9 Error: Child tag 'number' bound to namespace
'*' is already specified for element 'local:TestView'. It will be ignored.
        <local:number>
        ^
(In both Flex 4 and FlexJS the line number reported is for the beginning of
the 2nd initializer tag <local:number>, which is correct)
CURRENT BUG
-----------
I believe the error description should be improved - it currently says "It
will be ignored" and that is not what actually happens (not being ignored
is correct, IMO).
And (unless I am missing something important) the rest seems too verbose.
PROPOSED FIX (NOT DONE):
-----------------------
This is easy to change.... please let me know if you agree.
I suggest changing the FlexJS error description to be similar to Flex 4, to:
Multiple initializers for 'local:number' are not permitted for element
'local:TestView'.
   <local:number>
            ^




On Sat, Mar 4, 2017 at 8:46 PM, Greg Dove <gr...@gmail.com> wrote:

>
> I believe MXML is supposed to treat more than one child in a child tag as
> an array.  And thus, the equivalent AS code for:
>
> <js:initialView>
>   <local:MyInitialView id="view1" />
>   <local:MyOtherInitialView id="view2" />
> </js:initialView>
>
> is:
>
>   initialView = [ new MyInitialView, new MyOtherInitialView];
>
>
>
> I understood the same, but I had thought it was supposed to be a bit
> smarter than that and only assume an implicit <fx:Array> wrapper if it is
> assigning to an Array typed property (maybe even 'Arraylike') property. I
> will check the behavior using Flex 4 - I assume we want to keep whatever
> 'smarts' were implemented there, or at least as close as makes sense? I'll
> also double check the compiler error for this same scenario in Flex 4,
> which I did not do yet.
>
> If you code that in AS, you will get an error (I hope), and I think the
> compiler should report the same error for this MXML scenario, since
> initialView is of type IApplicationView or something like that.
>
>
> There might arguably be some justification for having an 'mxml version' of
> an error, describing things more in mxml terms, particularly for new users
> as they come on board and perhaps play with mxml examples first before
> learning actionscript, but I will check both the actionscript error in
> FlexJS and the mxml error in Flex 4 (assuming there is one)  for this
> scenario and report back here for consensus.
>
> <js:initialView>
>   <js:SimpleCSSValuesImpl />
> </js:initialView>
>
> Is the equivalent of:
> var initialView:IApplicationView = new SimpleCSSValuesImpl().
>
>
>
> Exactly, and in the fix, I have this currently being described as:
> In initializer for 'js:initialView', type org.apache.flex.core.SimpleCSSValuesImpl
> is not assignable to target type 'org.apache.flex.core.IApplicationView'.
>
> I will check the FlexJS actionscript error for this also, but I definitely
> pulled that one above as-is from Flex 4 for the same mxml problem (which is
> also correctly treated as a compile time error)
>
>
>
> On Fri, Mar 3, 2017 at 6:33 PM, Alex Harui <ah...@adobe.com> wrote:
>
>> Hi Greg,
>>
>> Thanks for digging into this.
>>
>> Without having dug into it myself, I would like to suggest returning a
>> type mismatch error of some sort.  IIRC, the DuplicateChildTagProblem is
>> for the following:
>>
>> <js:initialView>
>>   <local:MyInitialView id="view1" />
>> </js:initialView>
>> <js:initialView>
>>   <local:MyInitialView id="view2" />
>>
>> </js:initialView>
>>
>> The child tag (js:initialView) is really in there twice.
>>
>>
>> I believe MXML is supposed to treat more than one child in a child tag as
>> an array.  And thus, the equivalent AS code for:
>>
>> <js:initialView>
>>   <local:MyInitialView id="view1" />
>>   <local:MyOtherInitialView id="view2" />
>> </js:initialView>
>>
>> is:
>>
>>   initialView = [ new MyInitialView, new MyOtherInitialView];
>>
>> If you code that in AS, you will get an error (I hope), and I think the
>> compiler should report the same error for this MXML scenario, since
>> initialView is of type IApplicationView or something like that.
>>
>> And same for the second scenario:
>>
>> <js:initialView>
>>   <js:SimpleCSSValuesImpl />
>> </js:initialView>
>>
>> Is the equivalent of:
>>
>>
>>   var initialView:IApplicationView = new SimpleCSSValuesImpl().
>>
>> My 2 cents,
>> -Alex
>>
>> On 3/2/17, 9:09 PM, "Greg Dove" <gr...@apache.org> wrote:
>>
>> >I have been looking into FLEX-35273 [1].
>> >
>> >This is a compiler bug where it is possible to do things that don't make
>> >sense, like:
>> >
>> ><js:initialView>
>> >       <local:MyInitialView id="view1" />
>> >       <local:MyInitialView id="view2" />
>> ></js:initialView>
>> >
>> >or even this :
>> >
>> ><js:initialView>
>> >       <js:SimpleCSSValuesImpl />
>> ></js:initialView>
>> >
>> >Neither of the above caused compile time errors.
>> >
>> >I have a fix for both the above scenarios.
>> >
>> >For the first one, I used MXMLDuplicateChildTagProblem which seems 'close
>> >enough'
>> >
>> >"Child tag '${childTag}' bound to namespace '${childNamespace}' is
>> >already specified for element '${element}'. It will be ignored.";
>> >
>> >This renders as :
>> >Child tag 'MyInitialView' bound to namespace '*' is already specified for
>> >element 'js:initialView'. It will be ignored.
>> >   <local:MyInitialView id="view2" />
>> >    ^
>> >
>> >in the first example above. The text does not feel entirely right, but
>> >seems close enough. "It will be ignored" sounds more like a warning (as
>> >opposed to an error/failure), which I think a) it should not be and b) it
>> >is not.
>> >
>> >For the second one, I could not find any relevant 'Problem' class (I may
>> >have missed one perhaps?) So I just made a new one. I don't really know
>> >what the rules are here.
>> >These Problems seem to include an error code so I am unclear what to do
>> >if I add a new one. i.e. it is not clear what new error code I should
>> >apply (or even if the error code is important for something).
>> >For now I just added an arbitrary errorCode = 1999 on
>> >MXMLBadChildTagPropertyAssignmentProblem.
>> >
>> >The new problem renders out to :
>> >In initializer for 'js:initialView', type
>> >org.apache.flex.core.SimpleCSSValuesImpl is not assignable to target
>> type
>> >'org.apache.flex.core.IApplicationView'.
>> >
>> >Alex, you may be able to advise here.... are there any rules I need to
>> >follow for the CompilerProblem classes (in particular, when adding a new
>> >class).
>> >
>> >Assuming all is well above, I will commit this fix tomorrow. I was also
>> >trying to see if I could figure out how to get checkintests running, but
>> >I so far I did not. However the regular build tests are all fine with the
>> >changes I made for this.
>> >
>> >
>> >1. https://issues.apache.org/jira/browse/FLEX-35273
>>
>>
>

Re: [FlexJS] Working to fix a compiler bug - problem with the Problems

Posted by Greg Dove <gr...@gmail.com>.

I believe MXML is supposed to treat more than one child in a child tag as
an array.  And thus, the equivalent AS code for:

<js:initialView>
  <local:MyInitialView id="view1" />
  <local:MyOtherInitialView id="view2" />
</js:initialView>

is:

  initialView = [ new MyInitialView, new MyOtherInitialView];



I understood the same, but I had thought it was supposed to be a bit
smarter than that and only assume an implicit <fx:Array> wrapper if it is
assigning to an Array typed property (maybe even 'Arraylike') property. I
will check the behavior using Flex 4 - I assume we want to keep whatever
'smarts' were implemented there, or at least as close as makes sense? I'll
also double check the compiler error for this same scenario in Flex 4,
which I did not do yet.

If you code that in AS, you will get an error (I hope), and I think the
compiler should report the same error for this MXML scenario, since
initialView is of type IApplicationView or something like that.


There might arguably be some justification for having an 'mxml version' of
an error, describing things more in mxml terms, particularly for new users
as they come on board and perhaps play with mxml examples first before
learning actionscript, but I will check both the actionscript error in
FlexJS and the mxml error in Flex 4 (assuming there is one)  for this
scenario and report back here for consensus.

<js:initialView>
  <js:SimpleCSSValuesImpl />
</js:initialView>

Is the equivalent of:
var initialView:IApplicationView = new SimpleCSSValuesImpl().



Exactly, and in the fix, I have this currently being described as:
In initializer for 'js:initialView', type
org.apache.flex.core.SimpleCSSValuesImpl
is not assignable to target type 'org.apache.flex.core.IApplicationView'.

I will check the FlexJS actionscript error for this also, but I definitely
pulled that one above as-is from Flex 4 for the same mxml problem (which is
also correctly treated as a compile time error)



On Fri, Mar 3, 2017 at 6:33 PM, Alex Harui <ah...@adobe.com> wrote:

> Hi Greg,
>
> Thanks for digging into this.
>
> Without having dug into it myself, I would like to suggest returning a
> type mismatch error of some sort.  IIRC, the DuplicateChildTagProblem is
> for the following:
>
> <js:initialView>
>   <local:MyInitialView id="view1" />
> </js:initialView>
> <js:initialView>
>   <local:MyInitialView id="view2" />
>
> </js:initialView>
>
> The child tag (js:initialView) is really in there twice.
>
>
> I believe MXML is supposed to treat more than one child in a child tag as
> an array.  And thus, the equivalent AS code for:
>
> <js:initialView>
>   <local:MyInitialView id="view1" />
>   <local:MyOtherInitialView id="view2" />
> </js:initialView>
>
> is:
>
>   initialView = [ new MyInitialView, new MyOtherInitialView];
>
> If you code that in AS, you will get an error (I hope), and I think the
> compiler should report the same error for this MXML scenario, since
> initialView is of type IApplicationView or something like that.
>
> And same for the second scenario:
>
> <js:initialView>
>   <js:SimpleCSSValuesImpl />
> </js:initialView>
>
> Is the equivalent of:
>
>
>   var initialView:IApplicationView = new SimpleCSSValuesImpl().
>
> My 2 cents,
> -Alex
>
> On 3/2/17, 9:09 PM, "Greg Dove" <gr...@apache.org> wrote:
>
> >I have been looking into FLEX-35273 [1].
> >
> >This is a compiler bug where it is possible to do things that don't make
> >sense, like:
> >
> ><js:initialView>
> >       <local:MyInitialView id="view1" />
> >       <local:MyInitialView id="view2" />
> ></js:initialView>
> >
> >or even this :
> >
> ><js:initialView>
> >       <js:SimpleCSSValuesImpl />
> ></js:initialView>
> >
> >Neither of the above caused compile time errors.
> >
> >I have a fix for both the above scenarios.
> >
> >For the first one, I used MXMLDuplicateChildTagProblem which seems 'close
> >enough'
> >
> >"Child tag '${childTag}' bound to namespace '${childNamespace}' is
> >already specified for element '${element}'. It will be ignored.";
> >
> >This renders as :
> >Child tag 'MyInitialView' bound to namespace '*' is already specified for
> >element 'js:initialView'. It will be ignored.
> >   <local:MyInitialView id="view2" />
> >    ^
> >
> >in the first example above. The text does not feel entirely right, but
> >seems close enough. "It will be ignored" sounds more like a warning (as
> >opposed to an error/failure), which I think a) it should not be and b) it
> >is not.
> >
> >For the second one, I could not find any relevant 'Problem' class (I may
> >have missed one perhaps?) So I just made a new one. I don't really know
> >what the rules are here.
> >These Problems seem to include an error code so I am unclear what to do
> >if I add a new one. i.e. it is not clear what new error code I should
> >apply (or even if the error code is important for something).
> >For now I just added an arbitrary errorCode = 1999 on
> >MXMLBadChildTagPropertyAssignmentProblem.
> >
> >The new problem renders out to :
> >In initializer for 'js:initialView', type
> >org.apache.flex.core.SimpleCSSValuesImpl is not assignable to target type
> >'org.apache.flex.core.IApplicationView'.
> >
> >Alex, you may be able to advise here.... are there any rules I need to
> >follow for the CompilerProblem classes (in particular, when adding a new
> >class).
> >
> >Assuming all is well above, I will commit this fix tomorrow. I was also
> >trying to see if I could figure out how to get checkintests running, but
> >I so far I did not. However the regular build tests are all fine with the
> >changes I made for this.
> >
> >
> >1. https://issues.apache.org/jira/browse/FLEX-35273
>
>

Re: [FlexJS] Working to fix a compiler bug - problem with the Problems

Posted by Alex Harui <ah...@adobe.com>.

Hi Greg,

Thanks for digging into this.

Without having dug into it myself, I would like to suggest returning a
type mismatch error of some sort.  IIRC, the DuplicateChildTagProblem is
for the following:

<js:initialView>
  <local:MyInitialView id="view1" />
</js:initialView>
<js:initialView>
  <local:MyInitialView id="view2" />

</js:initialView>

The child tag (js:initialView) is really in there twice.


I believe MXML is supposed to treat more than one child in a child tag as
an array.  And thus, the equivalent AS code for:

<js:initialView>
  <local:MyInitialView id="view1" />
  <local:MyOtherInitialView id="view2" />
</js:initialView>

is:

  initialView = [ new MyInitialView, new MyOtherInitialView];

If you code that in AS, you will get an error (I hope), and I think the
compiler should report the same error for this MXML scenario, since
initialView is of type IApplicationView or something like that.

And same for the second scenario:

<js:initialView>
  <js:SimpleCSSValuesImpl />
</js:initialView>

Is the equivalent of:


  var initialView:IApplicationView = new SimpleCSSValuesImpl().

My 2 cents,
-Alex

On 3/2/17, 9:09 PM, "Greg Dove" <gr...@apache.org> wrote:

>I have been looking into FLEX-35273 [1].
>
>This is a compiler bug where it is possible to do things that don't make
>sense, like:
>
><js:initialView>
>	<local:MyInitialView id="view1" />
>	<local:MyInitialView id="view2" />
></js:initialView>
>
>or even this :
>
><js:initialView>
>	<js:SimpleCSSValuesImpl />
></js:initialView>
>
>Neither of the above caused compile time errors.
>
>I have a fix for both the above scenarios.
>
>For the first one, I used MXMLDuplicateChildTagProblem which seems 'close
>enough'
>
>"Child tag '${childTag}' bound to namespace '${childNamespace}' is
>already specified for element '${element}'. It will be ignored.";
>
>This renders as :
>Child tag 'MyInitialView' bound to namespace '*' is already specified for
>element 'js:initialView'. It will be ignored.
>   <local:MyInitialView id="view2" />
>    ^
>
>in the first example above. The text does not feel entirely right, but
>seems close enough. "It will be ignored" sounds more like a warning (as
>opposed to an error/failure), which I think a) it should not be and b) it
>is not. 
>
>For the second one, I could not find any relevant 'Problem' class (I may
>have missed one perhaps?) So I just made a new one. I don't really know
>what the rules are here.
>These Problems seem to include an error code so I am unclear what to do
>if I add a new one. i.e. it is not clear what new error code I should
>apply (or even if the error code is important for something).
>For now I just added an arbitrary errorCode = 1999 on
>MXMLBadChildTagPropertyAssignmentProblem.
>
>The new problem renders out to :
>In initializer for 'js:initialView', type
>org.apache.flex.core.SimpleCSSValuesImpl is not assignable to target type
>'org.apache.flex.core.IApplicationView'.
>
>Alex, you may be able to advise here.... are there any rules I need to
>follow for the CompilerProblem classes (in particular, when adding a new
>class).
>
>Assuming all is well above, I will commit this fix tomorrow. I was also
>trying to see if I could figure out how to get checkintests running, but
>I so far I did not. However the regular build tests are all fine with the
>changes I made for this.
>
>
>1. https://issues.apache.org/jira/browse/FLEX-35273