BusinessCentral.LinterCop icon indicating copy to clipboard operation
BusinessCentral.LinterCop copied to clipboard

LC0045 - Exceptions to the rule?

Open rvanbekkum opened this issue 1 year ago • 4 comments

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?

rvanbekkum avatar Jan 05 '24 07:01 rvanbekkum

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.

  • value(0; "") { }
  • value(0; "") { Caption = ''; }

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

  • value(0; "Undefined") { }
  • value(0; "None") { }
  • value(0; "") { Caption = 'Undefined'; }

Arthurvdv avatar Jan 09 '24 14:01 Arthurvdv

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? 😅

rvanbekkum avatar Jan 09 '24 14:01 rvanbekkum

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';
}

Arthurvdv avatar Jan 09 '24 16:01 Arthurvdv

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?

rvanbekkum avatar Jan 09 '24 17:01 rvanbekkum