Al: The name of TableName is not valid. The name of variables and parameters must be suffixed with the type or object name.AL(AA0072)

Created on 2 Sep 2020  Â·  23Comments  Â·  Source: microsoft/AL

Describe the bug
Could this warning Ignore the Prefix of the Object please? It's just incredibly ugly to add the prefix to ObjectNames.

To Reproduce
Steps and to reproduce the behavior:
Add variables with a Prefix:

AL Code to reproduce the issue

var
        WebRequestLog: Record "ESC Web Request Log";
        StartProcessResponse: Codeunit "ESC Start Process Response";

Expected behavior
Don't trigger the warning if only the prefix is missing

Screenshots
If applicable, add screenshots to help explain your problem.
image

5. Versions:

  • AL Language: 6.0.324421
  • Business Central: Not related to a BC Version
CodeCop bug in-progress static-code-analysis

Most helpful comment

@MarcHansenMicrosoft thanks for the feedback. I'm curious though why MS feel the need to dictate any rules at all on this?

I've got a few concerns:

  • there will always be (what I would consider) false positives for sensible names e.g. WhseJnlLine is perfectly understandable and used in the base code but will be considered invalid
  • conversely, I sometimes have to abbreviate the object name due to the restriction on length and make my variable names more descriptive than the object name
  • at the moment our code analysis is so noisy with invalid variable warnings that we might miss something more important

I know I can just disable this rule but I'm interested why we've got it at all.

All 23 comments

Same would go for suffixes if they are also only used to mark fields as belonging to a co./app.

Agree with dtkb - we use suffixes for AppSource. Suddenly we've got hundreds of suggestions that our variable names aren't valid 😱

Two more problems with this CodeCop:

1) Problem in [EventSubscriber] procedure
[EventSubscriber(ObjectType::Table, Database::Contact, 'OnAfterDeleteEvent', '', false, false)]
local procedure DeleteLogOnAfterDelete(var Rec: Record Contact)
Cannot be changed because TableName must follow publisher.

2) Problem if object name contains dot
var
LineFeeNoteOnReportHist: Record "Line Fee Note on Report Hist.";
GetDocumentNoAndDate: Page "Get Document No. and Date";

Even common abbreviations are a problem
WhseJnlTemplate: Record "Warehouse Journal Template";
image

And when combined with the rule for temporary (Temp prefix) it gets very verbose. Any unique part of the name must be inside:
NewWarehouseJournalTemplateTemp: Record "Warehouse Journal Template" temporary; // Not allowed TempWarehouseJournalTemplateOld: Record "Warehouse Journal Template" temporary; // Not allowed TempNewWarehouseJournalTemplate, TempOldWarehouseJournalTemplate: Record "Warehouse Journal Template" temporary; // As required
I'm all for standards and verbose names, but this gets less readable. Yes it can be suppressed, but the general idea is good and I want the rule, it's just a little too hard.

How to correct name two variables the same record? Above that temporary?
var
Customer1: Record Customer;
Customer2: Record Customer;

It sounds like this feature was just nowhere near ready enough to be released. I bet running it over the standard codebase, which for better or worse is a fairly good predictor of a lot of end-user code out there, would have noisy enough results to make that clear.

Even after the bugs were fixed and convenience features added, how could we reconcile this with the typical idiom of abbreviating Journal to Jnl, Purchase to Purch, etc.? Either someone has to maintain a list of 'approved' abbreviations, or you basically say that we can't abbreviate anymore, and in the latter case readability arguably goes way down - and certainly a lot of version control diff noise results from 'correcting' the code (or we have yet another practically unusable warning to suppress).

I asked the same question about abbreviations a few months ago, but without an answer.
Therefore, we had to rename all the abbreviated variables in the whole application (hundreds of occurrences). It was hours of work and the readability of the code did not improve.

A similar question was if I had a Record variable (such as Customer) once as local, the second as global: it must not be named the same, but both must have the suffix TableName. Will we introduce the (previously rejected) prefixes Loc... and Glob...?

Not having Prefix and Suffix in the name should be allowed.
That is a bug.

EventSubcriber error is a bug as well.
. and other special chars is a bug as well.

We are working on the Temp issue.

Not allowing abbreviations are by design. We had a big internal discussion and landed on no abbreviations.

Not allowing 2 vars like customer1 and customer2 is also a bug.

Thanks for all the good feedbask.

I'm curious about whether Microsoft will also update the base app to apply to this rule. The reason I'm addressing this is because reports need to be copied to our own extensions to be customized. To apply to this rule we would now have to update the variable names in the copied report. When the report is updated later on in the base app, we would in many cases also like to update our copied report(s) which is an easier task when all variable names match.

It's good to hear that the Temp issue is worked on, I agree to the comment of @AskeHolst about the readability. Is it also possible to allow both 'Buffer' and 'Temp' for temporary records?

@MarcHansenMicrosoft thanks for the feedback. I'm curious though why MS feel the need to dictate any rules at all on this?

I've got a few concerns:

  • there will always be (what I would consider) false positives for sensible names e.g. WhseJnlLine is perfectly understandable and used in the base code but will be considered invalid
  • conversely, I sometimes have to abbreviate the object name due to the restriction on length and make my variable names more descriptive than the object name
  • at the moment our code analysis is so noisy with invalid variable warnings that we might miss something more important

I know I can just disable this rule but I'm interested why we've got it at all.

I totally agree with @jimmymcp .

Just one more thought (sorry if already mentioned somewhere):
Since we (soon) need to replace all explicit "with"s in code with the full record name, we will be sometimes forced to use short or even extra short record variable names to keep the code readable.

It will be only Temp for now.

We will not force this rule for BaseApp only System and other extension.

For copied reports I would pragma ignore the specific rule.

I see all of your points in the abbreviations but for now we do not plan to change it.

First we will focus on the bugs.

I see there have been some new threads started, but I'm surprised there hasn't been a comment here in a month. I'll be the first to say that I am all for standards and nudging, sometimes forcing, developers in the direction of improving code quality. I absolutely love the majority of the rules. Unfortunately I feel that this one is forcing a reduction in quality.

I have to imagine the reason that there isn't a "Suggested Fix" for the warning is that Microsoft does not have the ability to define what "Readable" code actually is. The only fix that would work every time would make the code so unreadable it would destroy the intended results. So we're left with the developer making yet another choice on readability. Does anyone else see the irony here?

Variable naming is a standard, not a rule. We'll continue to educate developers on the standards for properly naming their variables, and they will adapt specific situations to write working code that is still readable. No one can put a firm rule on what readable code is. It's subjective. I haven't seen a rash of unreadable code lately, so unfortunately this is probably going to fall into the ever increasing set of ignores.

Could someone from Microsoft explain what root problem was occurring and why this was the solution? My mind could definitely be changed.

Some more issues:

  • RecRef: RecordRef; // using RecordRef as a name is weird, since it is highlighted as a reserved word
  • same with FieldRef and KeyRef

same with InStr and OutStr

I asked the same question about abbreviations a few months ago, but without an answer.
Therefore, we had to rename all the abbreviated variables in the whole application (hundreds of occurrences). It was hours of work and the readability of the code did not improve.

A similar question was if I had a Record variable (such as Customer) once as local, the second as global: it must not be named the same, but both must have the suffix TableName. Will we introduce the (previously rejected) prefixes Loc... and Glob...?

I am using a suffix 'G' for global variables. For 2 reasons: 1. make a distinction between a local var like Customer, and a global var Customer, and 2. to make very clear to any programmer that a suffix 'G' stands for global var (I only use the Hungarian notation for this case).
But since an update to v7.0.341259 I get the AA0072 info problem. Annoying.

"Not having Prefix and Suffix in the name should be allowed."
WHAT? We renamed hundreds of occurrences in accordance with this rule. Was it useless?

Please finalize this role ASAP. Abbreviations allowed yes/no. Prefixes/suffixes mandatory yes/no. Object name as suffix yes/no. Reserved words yes/no. Numbers in variable names yes/no. Disable in subscriber parameters.

Thanks.

To revisit my comments from last week, I am glad that this rule is in there as an option. That said, I think we've pretty much gotten to where we are going to add this to our ignore file and the more I think about it the more I think we might not ever take it out.

It's been this way for a while, and I do at least partly understand why, but the idea that these rules are not enforced in the main codebase just doesn't feel right to me. I get that every company has to make its own decisions about what quality means to them and what to prioritize. At the same time, combined with the suggestion to pragma ignore the warnings when copying from a base report instead of fixing them? And the insufficient testing before this rule was released? To me that just shows that this isn't a serious issue for Microsoft, and thus not a serious issue for me.

I really do not understand all restrictions.
But I use this Issue because its the same Error but not with Tables.

image

Whats with those Datatypes like Dialog or Variant? Its a bug or?

Is there a reason this rule is case sensitive? Nothing else in AL is...

Hi everyone, some changes were made to AA0072:
@BartPermentier
Affixes (such as ESC in your example, but applies to suffixes as well) no longer have to be included in the variable name. The examples you gave should work now

@IvoMol
The issue with EventSubscribers has been fixed.
The dot character does not trigger the rule, it's the fact that the rule was case sensitive. That has since been changed.
The rule was also relaxed to allow suffixing variables in order to differentiate between variables of the same record (such as CustomerNew & CustomerOld, or Customer1 & Customer2)

Thanks @dan-cris , from which AL Language version on is it available?

@NKarolak it should be available in the latest developer preview .

I will close this issue as it is diverging from the original. If you still have issues after trying the developer preview please make another issue and report it there 😀

Was this page helpful?
0 / 5 - 0 ratings

Related issues

dkatson picture dkatson  Â·  3Comments

kine picture kine  Â·  3Comments

ThomasBrodkorb picture ThomasBrodkorb  Â·  3Comments

worldofthenavcraft picture worldofthenavcraft  Â·  3Comments

TinaMenezes picture TinaMenezes  Â·  3Comments