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

LC0045 - Exceptions to the rule? #486

Open
rvanbekkum opened this issue Jan 5, 2024 · 4 comments
Open

LC0045 - Exceptions to the rule? #486

rvanbekkum opened this issue Jan 5, 2024 · 4 comments
Labels
enhancement New feature or request

Comments

@rvanbekkum
Copy link
Contributor

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 Arthurvdv added the enhancement New feature or request label Jan 5, 2024
@Arthurvdv
Copy link
Collaborator

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

@rvanbekkum
Copy link
Contributor Author

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
Copy link
Collaborator

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
Copy link
Contributor Author

rvanbekkum commented Jan 9, 2024

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants