StefanMaron / BusinessCentral.LinterCop

Community driven code linter for AL (MS Dynamics 365 Business Central)
https://stefanmaron.com
MIT License
76 stars 31 forks source link

LC0045 - Exceptions to the rule? #486

Open rvanbekkum opened 9 months ago

rvanbekkum commented 9 months ago

We already have some enum objects where ID 0 is used for values with the name Undefined, None, and/or with Caption = '', Locked = true; (with the latter resulting in the enum value not showing as a selectable option in the BC client).

Could it be considered that a LC0045 diagnostic is not raised in this case as the developer deliberately thought about the value with ID 0?

Arthurvdv commented 9 months ago

The examples you're referring to are indeed valid to assign to the zero ID. I could use your help on how we should change the logic of this rule to support these scenario's.

At the moment the rule will verify that the Name (and Caption) need to be Empty. Applies a .Trim() to allow a white spaces.

I'm unsure on howto allow cases like these below, without breaking the consistency of the rule.

rvanbekkum commented 9 months ago

I think something like

value(0; Undefined)
{
    Caption = '', Locked = true;
}

should be allowed.

I think for some of the checks you could reuse things that were done in https://github.com/StefanMaron/BusinessCentral.LinterCop/blob/master/Design/Rule0014PermissionSetCaptionLength.cs, like checking for Locked = true.

And/or for retrieving the Name of an enum value you could use IEnumValueSymbol.Name.

But maybe I am misunderstanding what you are asking? 😅

Arthurvdv commented 9 months ago

That was not what I meant 😅

I'm looking for what defines a Enum that is Undefined/None intent? At the moment the rule expects Name (and Caption) need to be Empty. That makes the rule easy to understand.

Do we hardcode that next to Empty also the values Undefined and None are acceptable? Not sure how long this list of "Allowed Names" is going to be and where we draw the line of acceptable values. 🤔

For example something like this;


value(0; Empty)
{
    Caption = 'Empty';
}
rvanbekkum commented 9 months ago

I think there could be 2 things that we could address:

  1. If the enum value has exactly Caption = '', Locked = true then the diagnostic should not be raised (because the value won't show up as a selectable option in the client)
  2. A list of Names that should be allowed None, Undefined and maybe others that could be configured in LinterCop.json.

What do you think?