microsoft / AL

Home of the Dynamics 365 Business Central AL Language extension for Visual Studio Code. Used to track issues regarding the latest version of the AL compiler and developer tools available in the Visual Studio Code Marketplace or as part of the AL Developer Preview builds for Dynamics 365 Business Central.
MIT License
744 stars 245 forks source link

Warning AA0125 too draconian #5590

Closed rdebath closed 2 years ago

rdebath commented 4 years ago

No sure if you're interested in opinions like this one, but (unlike some of the other rules) I think this one could be useful if it were configurable or not quite so draconian.

Title Really poor rule.

Description The error as given below is not good

{
    "resource": "/c:/.../Customer_List.PageExt.al",
    "owner": "_generated_diagnostic_collection_name_#0",
    "code": "AA0215",
    "severity": 4,
    "message": "The file Customer_List.PageExt.al has an incorrect name. The valid name is CustomerList.PageExt.al.",
    "source": "AL",
    "startLineNumber": 1,
    "startColumn": 21,
    "endLineNumber": 1,
    "endColumn": 41
}

Reason for the rule You are requiring that the file names be made less readable.

Bad code sample

        {
            "id": "AA0215",
            "action": "None",
            "justification": "Too dumb to exist"
        },

Good code sample

Good and bad practices for fixing the rule I suggest either a method for specifying exactly the format allowed, or, perhaps, more reasonably accept files that match alphanumeric characters and ignore most other characters.

Remarks

jvincke commented 4 years ago

Rule AA0125 does exactly what it was created for. According to the description of the rule "Follow the best practices for naming" https://docs.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/analyzers/codecop-aa0215-followfilenameguide

The rule is part of the CodeCop. "CodeCop is an analyzer that enforces the official AL Coding Guidelines." https://docs.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/analyzers/codecop

I would suggest leaving this rule unchanged.

If you do not want to follow the official guidelines you have to disable the rule as you did in your 'Bad code sample'.

If you still want to archive your file name pattern you could use the VS Code extension 'waldo's CRS AL Language Extension', for example. Add

"CRS.FileNamePattern": "<ObjectName>.<ObjectTypeShortPascalCase>.al",
"CRS.FileNamePatternExtensions": "<ObjectName>.<ObjectTypeShort>Ext.al",
"CRS.FileNamePatternPageCustomizations": "<ObjectName>.<ObjectTypeShort>Cust.al",
"CRS.OnSaveAlFileAction": "Rename"

to your settings.json, and you are good to go. You have to name your objects accordingly, of course.

rdebath commented 4 years ago

Indeed. I would like to follow the intent of the rules, ie: Improve readability etc etc. And Waldo's extension is definitely a sufficient substitute.

But, when formulating this exact rule Microsoft only asked what pieces should be included (eg: object numbers) not minutia like the exact formatting of the name "Upd. Disc.% on Contract" hence my suggestion.

mjmatthiesen commented 4 years ago

I'm just here to say that the new "best" practise for file naming is nothing but horrendous. Makes file names unreadable, and harder to reference back to NAV version of the product.

Also, change the standard randomly is exceptionally infuriating.

Miki-84 commented 4 years ago

Hello,

This Rule used to work for us but today with the AL vscode estension 5.0 for BC 16, it requires to name the file with the company affix. If you check the styleguide, the nameing example with a Prefix on it omitt the prefix when naming the file. This case now gve you a wraning and i think it should not.

image

JulianTillmann commented 4 years ago

there's also suffix allowed in the naming conventions which isn't reflected in either the Warning AA0125 or the docs

https://docs.microsoft.com/en-us/dynamics365/business-central/dev-itpro/compliance/apptest-prefix-suffix

this rule has good intentions, but the execution could be improved

jpagessummar commented 3 years ago

What about when you have more than 1 object on the same file? It's not possible? For me it's very useful. For example, a table with enum and its page.

rdebath commented 3 years ago

@jpagessummar Microsoft are saying you should never do this.

Basically, if you have even a slightly larger than a trivial extension the "go-to XYZ" functions of the AL-extension get very, very, slow, so it's more effective to switch to the built in filename search of vscode. For this to work reasonably you have to name the files to match their contents. I know it would be nicer if Microsoft fixed the contextual searching in their AL extension, but I don't see them bothering to.

mazhelez commented 2 years ago

Thanks for reporting this issue. Sorry we haven’t completed it yet, but we’ve had to prioritize elsewhere. We’re planning to give the CodeCop engine and its rules an overhaul in a future major release. Thanks for your patience.

Going forward we will not track progress on CodeCop issues here on GitHub. Hence, we are closing this issue.