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

Rule AA0232 for Enums #6348

Closed
NKarolak opened this issue Dec 7, 2020 · 9 comments
Closed

Rule AA0232 for Enums #6348

NKarolak opened this issue Dec 7, 2020 · 9 comments
Labels
accepted bug Product bug CodeCop This is a specific static-code-analysis group (AA) in-progress static-code-analysis

Comments

@NKarolak
Copy link

NKarolak commented Dec 7, 2020

Describe the bug
The rule AA0232

The FlowField "My Enum" of "My New Table" should be added to the SIFT key.

is raised on Enum fields although they cannot be created as SIFT key.

To Reproduce
Enable the CodeCop in the settings.json

"al.enableCodeAnalysis": true,
"al.backgroundCodeAnalysis": true,
"al.codeAnalyzers": ["${CodeCop}"]

New Enum

enum 50100 "My Enum"
{
    value(0; Open)  { Caption = 'Open'; }
    value(1; Closed) { Caption = 'Closed'; }
}

New Table 1

table 50101 "My Table 1"
{
    Caption = 'My New Table';
    DataClassification = ToBeClassified;

    fields
    {
        field(1; "My PK"; Integer)
        {
            Caption = '';
            DataClassification = CustomerContent;
        }
        field(2; "My Enum"; Enum "My Enum")
        {
            Caption = 'My Enum';
            DataClassification = CustomerContent;
        }
    }
    keys
    {
        key(PK; "My PK")
        {
            Clustered = true;
        }
        key(MyEnum; "My Enum") { }
    }
}

New Table 2 with AA0232

table 50102 "My Table 2"
{
    Caption = 'My Table 2';
    DataClassification = ToBeClassified;

    fields
    {
        field(1; "My PK"; Integer)
        {
            Caption = 'My PK';
            DataClassification = CustomerContent;
        }
        field(2; "FlowField on Enum"; Enum "My Enum") // AA0232
        {
            Caption = 'FlowField on Enum';
            CalcFormula = min("My Table 1"."My Enum");
            Editable = false;
            FieldClass = FlowField;
        }
    }
    keys
    {
        key(PK; "My PK")
        {
            Clustered = true;
        }
    }
}

You cannot add the Enum field as SumIndexField in table 1:

 key(PK; "My PK")
        {
            Clustered = true;
            SumIndexFields = "My Enum" // AL0199
        }

This would raise AL0199:

The type of the sum index field '"My Enum"' must be numeric(Decimal, BigInteger, Integer, or Duration)

which is weird since Enums are numeric.

Expected behavior
Either the warning AA0232 shall not appear, or it must be possible to use Enums as SumIndexFields.

Versions:

  • AL Language: 6.1.362735
@thpeder thpeder added the bug Product bug label Dec 8, 2020
@qutreson qutreson added CodeCop This is a specific static-code-analysis group (AA) static-code-analysis labels Dec 8, 2020
@NKarolak
Copy link
Author

NKarolak commented Feb 1, 2021

When do you think this might get corrected?

@atoader
Copy link
Contributor

atoader commented Mar 7, 2021

@PaulIvan can you provide an ETA?

@NKarolak
Copy link
Author

Still no update :-(

@PaulIvan PaulIvan self-assigned this May 5, 2021
@PaulIvan
Copy link

PaulIvan commented May 5, 2021

Hi @NKarolak I agree this looks a bit weird but just because something is possible doesn't mean it should be used.

I'm just a bit confused as to why a lookup flowfield wouldn't be better in this case?

@dzzzb
Copy link

dzzzb commented May 5, 2021

I guess the example code is deliberately oversimplified, to the point of being nonsensical in practice, but it demonstrates the issue in theory.

That said, it seems brittle to rely on the numeric values of enums or options, and I never like it when I find code that does that.

Did adding to a SIFT key work with options? If not, I guess it was never intended to work.

I'd almost say that min/max should be disallowed on enums/options, for the reason above, but presumably that can't happen as it'd annoy too many people writing that kind of code ;-)

@NKarolak
Copy link
Author

NKarolak commented May 5, 2021

@DTKB
In reality, my enum flow field is a "Print Status" (1 = Open; ... 10 = Error; 20 = Printed) on a header record. It returns the minimum Print Status value from all related line records. Hence: only if all lines are printed (without errors), then the header record is displayed as printed.
It works like a charm. Only the rule AA0232 should not appear ;-)

@NKarolak
Copy link
Author

@PaulIvan Can you give an ETA?

@PaulIvan PaulIvan removed their assignment Dec 6, 2021
@NKarolak
Copy link
Author

@qutreson push ;-)

@mazhelez
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted bug Product bug CodeCop This is a specific static-code-analysis group (AA) in-progress static-code-analysis
Projects
None yet
Development

No branches or pull requests

7 participants