airsdk / Adobe-Runtime-Support

Report, track and discuss issues in Adobe AIR. Monitored by Adobe - and HARMAN - and maintained by the AIR community.
206 stars 11 forks source link

Compiler: Type checks not performed on super calls #3513

Open Adolio opened 1 month ago

Adolio commented 1 month ago

Having this would prevent runtime errors by detecting issues at compile time.


This issue was initially reported in https://github.com/BowlerHatLLC/vscode-as3mxml/issues/770

Comment from @joshtynjala:

Interestingly, the ASC 2.0 compiler in the AIR SDK doesn't seem to perform type checks on super calls either. That's a bug that Harman should fix too. The Flex SDK compiler checks all super class properly, though.


Please consider adding those type checks in a future version of the SDK.

Thanks

joshtynjala commented 1 month ago

I'll just add the code examples from the vscode-as3mxml issue, so that the AIR SDK devs have more context without following the link to that issue.

In this example, there should be a compile-time error because 42 is not a valid value for the Point type:

import flash.geom.Point;

class A
{
    public function A(point:Point)
    {

    }
}

class B extends A
{
    public function B()
    {
        super(42);
    }
}

And this example should have a similar compile-time error:

override public function a(point:Point):void {
    super.a(42);
}

In the case of the AIR SDK compiler, I didn't specifically test whether other related checks are performed, but I'm guessing that they'll be missing similar to the Royale compiler, since they are related codebases. For example, it should also check that the number of arguments passed to the super method is correct. The Royale compiler didn't check that either.

ajwfrost commented 1 month ago

Thanks Josh! Although too late, I'd already followed the link :-) Presumably this has been an issue from the start!

The linked issue is on the vscode-as3mxml project - I assume that's not actually the component at fault, and the fixes would need to come from us and from the Apache royale-compiler project?

joshtynjala commented 1 month ago

the fixes would need to come from us and from the Apache royale-compiler project?

Correct.

For royale-compiler, I've already implemented the necessary fixes in a branch. We have a release in progress, so they'll go into the next version. The commits with fixes, if you're interested, are linked below:

https://github.com/apache/royale-compiler/commit/f0444c62522ecb700b9b7bd506749342b4fec5d3 https://github.com/apache/royale-compiler/commit/1b6a222357f69a9204e3fafaff90450d83b53282

joshtynjala commented 1 month ago

I should also mention that making this change may introduce new compiler errors in existing AS3 code that previously compiled without errors. Users may need some guidance. In most cases, they just need to add a cast to convert to the correct type. In some cases, it may actually be a bug in their code that was previously undiscovered. I think that undiscovered bugs are a bit less common with the AIR SDK than with Royale because AIR has the verifier performing extra runtime checks that are more likely to catch issues and throw runtime exceptions for them. Royale runs in JS, which is untyped and more lenient, so Royale apps can pass bad types further away from their source, and are more likely to appear to be working correctly.

Starling, Feathers, and other high-visibility projects shouldn't be affected because they're commonly compiled with both the AIR SDK and Flex SDK compilers, and the Flex SDK compiler doesn't have this bug.