apache / royale-compiler

Apache Royale Compiler
https://royale.apache.org/
Apache License 2.0
98 stars 50 forks source link

Invalid Syntax Succeeds in Compilation, but Doesn't Load at Runtime #186

Open brianraymes opened 3 years ago

brianraymes commented 3 years ago

I've been finding random compilation issues that should be invalid, but seem to succeed.

Steps to Reproduce

Accidentally added two dots in a binding within MXML. In my case, I performed a refactor that mistakenly left two dots; an easy mistake to make.

"{someObject..someValue}"

Expected Results

Compilation should fail, or at least throw a warning.

Actual Results

Compiles successfully, but the resulting debug JS file is not usable at runtime without any warning or error. I'm not sure if it's failing to load, or some other issue.

Workaround

None that I know of at this time.

Comments

A mistake like this is likely an edge case, but is an easy mistake to make. The cost of running into it was rather large as it was hard to track down and took significant time to find/fix as there are no errors.

FYI, I am using vscode-as3mxml. I'm not sure if that is relevant or not with issue.

joshtynjala commented 3 years ago

I believe that the compiler is behaving as intended. I compared the following code with both the Royale compiler (as used inside vscode-as3mxml) versus compiling to SWF with the Flex SDK:

<s:Application xmlns:fx="http://ns.adobe.com/mxml/2009"
    xmlns:s="library://ns.adobe.com/flex/spark" xmlns:local="*">

    <fx:Declarations>
        <fx:String>{bindableObject..what}</fx:String>
        <fx:String>{bindableString..what}</fx:String>
    </fx:Declarations>

    <fx:Script>
        <![CDATA[
            [Bindable]
            public var bindableString:String = "Hello world";

            [Bindable]
            public var bindableObject:Object = {};
        ]]>
    </fx:Script>
</s:Application>

In both compilers, it results in one error only (for bindableString..what):

Error: Access of possibly undefined property what through a reference with static type String.

However, there is no error for bindableObject..what.

So, if the compiler knows the type, detecting an error seems to work correctly. If the type is Object (or *), the compiler doesn't know enough about the object to know whether there's an error or not. A variable typed as Object might be holding a value typed as XML, XMLList, or Proxy, and in that case .. could be a perfectly valid operator.

That's the thing about having things typed as Object, it basically reverts to ECMAScript semantics, which means detecting errors at runtime.

With that in mind, I would be opposed to adding an error. However, I think that a warning would be okay, as long as it only affects variables/properties typed as Object and * (and maybe other types that are dynamic, but definitely not for XML and XMLList).

brianraymes commented 3 years ago

Oh, I didn't realize my example was a bit ambiguous. "someObject" wasn't to mean the type Object, but just any object. In my case, it was strongly typed; there was no error or warning in my compilation output. That is unless I'm missing something. I am using maven to build my project.

joshtynjala commented 3 years ago

That's strange considering that I saw an error with String. It should be no different with any other non-dynamic type.

It's worth noting that even if you're always using classes, the variable type or function return type is what matters here.

Example:

var myObj:Object = new MyClass();

This variable is storing an instance of MyClass, but it is typed as Object, which means that the compiler won't even consider MyClass when it is checking for errors.

Similarly, something like array[0] or collection.getItemAt(0) will usually be typed as * by default. You need to cast it to be sure that the compiler sees the correct type.

There are a lot of ways in AS3 to end up falling back to Object or *.

brianraymes commented 3 years ago

As for runtime, I've noticed some strange issues with Royale that all seem related. Things stop working at some level of execution, but do not error or warn the user in any way. Perhaps that is as designed, but it has made some development a bit difficult. Here are a couple examples.

This scenario:

In this case, the file that contained the ".." was not initialized. Any declaration of the component was null at runtime. No error as to why, just simply didn't appear to load.

Another:

Hidden NPEs either in my code, or the asjs framework stop code execution within any component with no error. Here is an easy example that I recently ran into. The following code is within a simple MXML file.

private var _config:SomeTypedObject;

[Bindable]
public function get config():SomeTypedObject
{
    return this._config;
}

public function set config(SomeTypedObject):void
{
    this._config = value;

    // Do something that causes an NPE. In this case, someProperty is null.
    warning = config.someProperty.length == 0;
}

Within this file, config is bound to various fields. Because of the NPE, none of the bindings for this field fire, even though the value is set before the NPE. No error appears in the console or debugger. It's as if it's caught and suppressed at runtime.

I've been seeing stuff like this for a while now when developing with Royale. As for a framework example, a VirtualList will not render unless the ListPresentationModel is added with rowHeight set. This is because VirtualListVerticalLayout uses this value without checking for its existence. No error, no warning, no rendering of the list. Simply an empty component. Why are these NPE's swallowed? Is that as designed/expected?

Is any of this related to the runtime aspect of this issue? If not, I will create a new issue.

brianraymes commented 3 years ago

Similarly, something like array[0] or collection.getItemAt(0) will usually be typed as * by default. You need to cast it to be sure that the compiler sees the correct type.

There are a lot of ways in AS3 to end up falling back to Object or *.

My codebase is 99.999+% strongly typed. I would rather know before runtime that something is not correct. It's possible I may be running into cases where it's falling back for unknown reasons, and I don't have a way of seeing how or why. With the runtime suppression I've been running into, I'm not sure how to intuitively debug these issues.

joshtynjala commented 3 years ago

No error, no warning, no rendering of the list. Simply an empty component. Why are these NPE's swallowed? Is that as designed/expected?

I don't claim to speak for the framework developers because I work on the compiler only. However, as I understand it, this experience is a result of Royale's PAYG (Pay As You Go) philosophy. The idea is that your final app should contain basically everything it needs to function, and no more. In other words, no error checking for edge cases that you wouldn't encounter in a final app. The idea is that you're supposed to enable the edge case stuff and other developer niceties by manually adding the appropriate beads to your application or components (I'm not necessarily guaranteeing that those beads actually exist right in Royale now, but only that this the mechanism for how that sort of functionality would be enabled). Later, you can remove some of the development-only beads, if you're confident that everything is working as intended.

The "Basic" component set is the ultimate realization of the PAYG philosophy. It is really meant to be bare minimum by default, with beads to enable more and more. The "Express" component set is supposed to build on Basic by adding some of these things by default that improve the development experience, instead of being required to do it manually. However, I haven't seen anyone working on Express very much lately, so I can't say what state that is in. Jewel is sort of a separate set of components from Basic/Express, but I think the idea is that it would be a bit closer to Express than to Basic.

That's just kind of my impression as someone who doesn't work on or with the framework, but have been watching discussions closely enough to have a high level idea of what's going on. Maybe a framework dev can jump in and explain in more detail.

joshtynjala commented 3 years ago

It's possible I may be running into cases where it's falling back for unknown reasons, and I don't have a way of seeing how or why. With the runtime suppression I've been running into, I'm not sure how to intuitively debug these issues.

In the case of "{someObject..someValue}", hovering over any names before the .. (like someObject) in VSCode should at least tell you their types. If it comes up as something other than Object or *, that might give us a clue about an edge case where the compiler is failing to detect the same error that it produced for String in my example above.

greg-dove commented 3 years ago

I can try to look into this next week.