Pre-release of the AL compiler: AL0920 reports warnings on internal interfaces

[bjarkihall]

1. Describe the bug
We're getting reports in the pre-release of AL of the new/undocumented AL0920 warning.

The method 'Foo' cannot be used as the implementation for the interface method 'IFoo.Foo' because it is not public. This warning will become an error when targeting a runtime with version '19.0' or higher.

Interfaces can be declared as public or internal on object-level, e.g. for mocking/testing.
Interface methods themselves cannot be declared as internal.
A codeunit implementing the interface can be declared public or internal on object-level, and its procedures can be local/internal/public.

2. To Reproduce
Declare an internal interface and implement it using internal procedures for the signature and you'll get the warning (future error).

interface IFoo {
    Access = Internal;
    procedure Foo() // interface procedures can't be explicitly marked as internal, but the object itself is internal
}

codeunit 50000 CuFoo implements IFoo {
    internal procedure Foo() // matches the signature and access of IFoo.Foo
    begin
    end;
}

3. Expected behavior
We expect the warning not to apply to interfaces with Access = Internal, because their procedures aren't publicly available.
CuFoo.Foo isn't public, but so is IFoo.Foo, because IFoo isn't public.

4. Actual behavior
A warning is raised, indicating it'll be a future error.

5. Versions:

• AL Language: 18.0.2498801
• Visual Studio Code: 1.125.1
• Business Central: 28.0 , runtime 17.0
• List of Visual Studio Code extensions that you have installed: Pre-release of ms-dynamics-smb.al
• Operating System:
  • Windows
  • Linux
  • MacOS

Final Checklist

Please remember to do the following:

• Search the issue repository to ensure you are reporting a new issue

• Reproduce the issue after disabling all extensions except the AL Language extension

• Simplify your code around the issue to better isolate the problem

Internal work item: AB#641381

[bjarkihall]

Some additional context: we started seeing more than 2000 of these warnings in our codebase today, which makes the pre-release version of AL kind of unusable, we can either suppress this specific warning in app.json or simply switch to the release version of the AL extension.

[RadoArvay]

In this case the AL is right, I think.
Your interface and your implementation do not match. There is the "public" procedure definition in the interface, but "internal" procedure in the implementation.

We also have had a couple of these un-matched implementations and we corrected them.

[bjarkihall]

You can't declare an internal procedure on an interface, so what's there to "match"?
The signature itself is already fulfilled, name and arguments/return-value.
The interface object is internal and its procedure inherits that access.
So what's the error and why?
How did you correct the mismatch without making the procedures public?

[RadoArvay]

I think we shouldn't mix a scope of procedures and objects. I still believe that if the interface is internal, its procedures are public by default without explicit scope definition. And AL, I think, tries to also match scopes now.
So, as we have internal interfaces, we can also have internal implementations - by the object scope, not procedures one.

And the answer for your last question is: we had implementations internal (mostly), so to get rid of warnings was easy for us = just to remove internal scope from procedures.

[bjarkihall]

Access of codeunit procedures is inherited by their object definition - if we can't access the object we can't access the procedures, explicit marking doesn't change anything about that.

Now, we can't mark the public codeunit as internal (breaking change) and we can't mark procedures of interfaces (not allowed by the compiler).

So they're going to force thousands of errors if we don't make internal procedures public as a "way of working around it"? I don't think the compiler is ready to back this rule up.

[RadoArvay]

Yes, I agree - the access to procedures is a combination (I would not name it as an inheritance) of 2 parameters - object's access and procedure's scope.
Anyway, what options do you have in your situation? (because this is still warning-type issue, so you have got a time to solve it)

• is it an issue to have public procedures in your solutions? = just remove internal scope
• is it an issue to have internal codeunits? = just set obsoleting state for them and set the Access property "in the 2nd round"

[bjarkihall]

Yes thanks, we had considered both of these paths, but our solution is quite large, and we'd prefer to keep our internal procedures as internal.

We'd also like to keep our codeunit signatures as is, at least the publicly available ones instead of obsoleting them, but it seems like the only workaround currently is to create a new internal codeunit for each public codeunit that implements an internal interface (very common for mocking) and use that implementation instead and make sure our references point to those instead too.
This is quite some work to favor... what reason?

The rule says: "The method 'Foo' cannot be used as the implementation for the interface method 'IFoo.Foo' because it is not public", but IFoo isn't public either. So why do they want this to become an error?

I'd just like MS to see this ticket, justify the errors so we can understand the actual "why" from their pov, and hopefully offer a way of explicitly marking interface procedures as internal as a compromise, if object-level isn't enough, for some reason.

Currently, it still looks like the rule was added without considering internal interfaces, so to me it still looks like there's a bug in the validation and there's no favorable workaround.