Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AL0606: "The 'with' statement is deprecated and will be removed for cloud development in a future release. This warning will become an error in a future release." #5817

Closed
navdotnetreqs opened this issue Apr 17, 2020 · 24 comments
Assignees

Comments

@navdotnetreqs
Copy link

AL0606: "The 'with' statement is deprecated and will be removed for cloud development in a future release. This warning will become an error in a future release."

Why? Just.. why? Is there a reasonable explanation for this decision? Is there a reasonable replacement?

This will cause us massive amounts of work no one will ever pay for. We will have to manually rewrite hundreds of passages of codes in our codebase, just because Microsoft decided "with" will not be used any more, even if it's one of the most sensible things in (C/)AL.

@nicolassaleron
Copy link

Why not... why not... For what I have experienced, with causes ambiguities for developers, but this is not for me a good reason to prevent the usage of this keyword.

If with becomes forbidden, please: this has to come with a quick fix that can be applied on an entire project.

@esbenk
Copy link
Contributor

esbenk commented Apr 21, 2020

We know this is a controversial decision, but the with-statement (both explicit/implicit) is poison for SaaS development. We have already seen many cases where adding a new method or field breaks dependent-code because of the use of with-statements. This is not a "just because" thing, this is a real problem that makes it difficult for us to run a reliable cloud service.

The implicit with-statement is another flavor of this problem that we will also obsolete. Implicit with-statement are the with on the Rec in Codeunit.Run and the source table on a Page. They have al they same problems, but today you can't even avoid them.

Those type of breaks is preventing us from running BC online with continuous upgrades and added functionality.

The with keyword has always been controversial - not only in Pascal but also in JavaScript. The with-statement is less of a problem when you can fix code in the entire stack e.g. on-prem base-app development, but even in that case it is not without problems. If we add a new method to the Record type (a very likely scenario) we today risk breaking already deployed extensions (and on-prem baseapp code) when we upgrade the platform. This is limiting our ability to enhance our platform.

In the SaaS offering we have no choice to abandon the with-statement. It is important to stress that this is not only for our benefit, not being broken an prevented from being upgraded will definitely also benefit customers and partners. With Per-Tenant extensions in BC online this becomes even more of a problem because of the sheer number of individual extensions.

This warning is from a pre-release version, and hence there is no documentation for it yet. We will provide documentation with the release, but I hope that this gives a more clear picture of why we are doing this.

@nicolassaleron
Copy link

After these information, I think this is completely understandable, even the deletion of the implicit with on codeunits and tables.

You should also understand that it is going to create a high workload to update existing developments, for App Source apps and it might be a terrible workload for PTEs.

Could you consider adding a command in VSCode at some point do the job automatically?

@esbenk
Copy link
Contributor

esbenk commented Apr 21, 2020

We are adding fixers for both the explicit-with and for the implicit-with. You need to enable Code Actions in for the AL Language extension to activate them.

The scope of the fixers is yet to be defined. For the implicit-with they are currently instance and object-wide and for the explicit-with they are per instance. We may expand the scope, but maybe not in the first release dependent on time.

We are also going to support a gradual approach for the implicit-with where you can fix a single object at a time and switch the compilation for the specific object to not do implicit with.

@dzzzb
Copy link

dzzzb commented Apr 21, 2020

The implicit with-statement is another flavor of this problem that we will also obsolete. Implicit with-statement are the with on the Rec in Codeunit.Run and the source table on a Page. They have al they same problems, but today you can't even avoid them.

Will you also get rid of the implicit Rec. when writing code within a Table or TableExtension?

@esbenk
Copy link
Contributor

esbenk commented Apr 21, 2020

We are not getting rid of .Rec on tables and tables extensions. The .Rec is equivalent to the this in other languages. It is not a problem the same way as on codeunits and pages.

@navdotnetreqs
Copy link
Author

We know this is a controversial decision, but the with-statement (both explicit/implicit) is poison for SaaS development. We have already seen many cases where adding a new method or field breaks dependent-code because of the use of with-statements. This is not a "just because" thing, this is a real problem that makes it difficult for us to run a reliable cloud service.

I'm not sure I completely understand how and when this happens, some concrete examples would be nice, so we could better appreciate the reasons. Is the architecture really such that the with-statement is not retained within the scope where it is used, but instead it leaks globally into other areas causing havoc? Shouldn't that be fixed, instead of getting rid of a very useful command?

@nicolassaleron
Copy link

nicolassaleron commented Apr 21, 2020

@navdotnetreqs Let suppose you have written the code below in your extension and it works well:

codeunit 50000 "My Codeunit"
{

  procedure FillThingOnSalesLine(var SalesLine: Record "Sales Line")
  begin
    with SalesLine do begin
      //...
      "My Net Unit Price" := GetNetUnitPrice(SalesLine);
      //...
    end;
  end;

  procedure GetNetUnitPrice(SalesLine: Record "Sales Line"): Decimal
  begin
    exit(SalesLine."Unit Price" * (100 - SalesLine."Line Discount %") / 100);
  end;

}

Now, if in a future release, Microsoft decides to add a procedure GetNetUnitPrice in the Sales Line table, then, because of the with, the procedure in the codeunit is now hidden.

@navdotnetreqs
Copy link
Author

Thanks, I see how this could potentially by a problem, although doesn't the strict naming conventions (partner prefix/suffix) mitigate if not eliminate this, if used as prescribed?

@esbenk
Copy link
Contributor

esbenk commented Apr 22, 2020

Strict-naming convention could mitigate this, but it would require enforcement of strict unique naming across all extensions (including a very large number of Per-Tenant-Extensions). We don't believe that is a viable solution to solve this.

@navdotnetreqs
Copy link
Author

Strict-naming convention could mitigate this, but it would require enforcement of strict unique naming across all extensions (including a very large number of Per-Tenant-Extensions). We don't believe that is a viable solution to solve this.

How about allowing usage of with for all custom objects, but not for objects of the base application (eg. sales line as in your example)? Probably a more difficult rule to make, but would lessen the burden of restructure and allow for the usage of neater code in at least some cases.

This change will most likely lead to people using single character record variable names because who wants to clutter one screen of source code with a hundred TempATOTrackingSpecification.fuu, TempATOTrackingSpecification.bar. One used to be able to use with, now we go back to a.fuu and b.fuu.

@nicolassaleron
Copy link

@esbenk About my example and the fact you mentioned that with is causing problems during upgrades:

  • if I upgrade BC with my example, without recompiling the app, are we agree that my app is still calling GetNetUnitPrice from the codeunit? I mean with is just some syntax sugar: the role of the compiler is to delete the with keyword and translate each call to a fully qualified reference (in my case CurrCodeunit.GetNetUnitPrice (by the way CurrCodeunit is missing in AL :) ).

  • if I recompile my example, then I could simply have an error message that says "There might be an ambiguity because the same symbol GetNetUnitPrice exists at multiple scope levels (CurrCodeunit.GetNetUnitPrice and SalesLine.GetNetUnitPrice). Please use an explicit syntax.". And then it might be useful to get be able to reference something like CurrCodeunit.

@esbenk
Copy link
Contributor

esbenk commented Apr 22, 2020

We always recompile during upgrade. We have to do that to ensure consistency, but also to make sure that the emitted code matches the new runtime.

You are right that the with statement is essentially a syntactic sugar and that is also why we can create a "fixer" that expands it. The problem is the your source can fail because of changes in either the platform and/or dependencies.

We don't want the language to have this kind of built-in pitfalls. It hurts us, it hurts the customer, but it certainly also hurts you as partners when a customer can't be upgraded. I don't think that saving of characters when writing code justifies that cost.

@esbenk esbenk closed this as completed May 27, 2020
@dzzzb
Copy link

dzzzb commented May 27, 2020

We are not getting rid of .Rec on tables and tables extensions. The .Rec is equivalent to the this in other languages. It is not a problem the same way as on codeunits and pages.

I still see plenty potential for ambiguity with the implicit Rec. on Table objects. Can you elaborate on why it is considered so much more OK? I guess for Tables, you can see the fields in the same object/file, so it's less easy to accidentally shadow/hide them. But TableExtensions don't have that locality. Personally I'd just make Rec. needed here too. I'm not sure I'm a huge fan of implicit this situations in general.

@esbenk
Copy link
Contributor

esbenk commented May 27, 2020

It is not an implicit with. The Rec. is essentially the same as this in C#.

@nicolassaleron
Copy link

nicolassaleron commented May 27, 2020

Rec is not really the same as this in C# because Microsoft recompiles the AL code when upgrading to newer version. It is not really the case in .Net when you run an app against a newer library or a newer framework.

AL packages should contain some ALIL: like MSIL in .Net, an intermediate language that removes any ambiguity coming from the AL syntax and keep the intent/context of the initial compilation. During upgrades, the recompilation should occur on this ALIL and not the AL code.
Only then, the recompilation will be safe with implicit Rec.

@esbenk
Copy link
Contributor

esbenk commented May 27, 2020

Please provide an example where code without the Rec. on a record can be broken by an upstream change.

@hhfiddelke
Copy link

What happens if there is a local function with name "abc" and a new field called also "abc", or a field in an extension that is now hidden by a function or vice versa.

@esbenk
Copy link
Contributor

esbenk commented Jun 3, 2020

Some samples and more explanatio here https://github.com/microsoft/BCTech/tree/master/samples/WithOrWithout

@rdebath
Copy link

rdebath commented Oct 23, 2020

I'm so disappointed you didn't take the suggestion of running your with removing code during the creation of the .app file and having ambiguity errors/warnings in vscode.

That would fix all the upgrade issues without forcing the partners to uglify their code.

@navdotnetreqs
Copy link
Author

navdotnetreqs commented Oct 26, 2020

I'm so disappointed you didn't take the suggestion of running your with removing code during the creation of the .app file and having ambiguity errors/warnings in vscode.

That would fix all the upgrade issues without forcing the partners to uglify their code.

Well, to be fair, as with all the other constant breaking changes, this decision is perfectly aligned with Microsoft's tendency of forcing partners to shuffle their manure.

@stevelinz
Copy link

stevelinz commented Jul 11, 2022

How would I rewrite this procedure to not use 'with'?

code.txt

@stevelinz
Copy link

I think I figured it out.
code2.txt

@rdebath
Copy link

rdebath commented Jul 12, 2022

@stevelinz Don't bother doing this manually. the extension "AZ AL Dev tools" has the fix that Microsoft haven't bothered to do.

It's "Remove 'with' usage" commands reliably do the simple syntax switch that Microsoft have fobbed of on us rather than doing it when the "compile" the code into the app file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants