HodorNV / ALOps

ALOps
59 stars 24 forks source link

[FEATURE REQUEST] Suppress pragma's #577

Closed Arthurvdv closed 8 months ago

Arthurvdv commented 1 year ago

Is your feature request related to a problem? Please describe. With more-and-more Code Cops it's sometimes tempting for the developer to add a pragma around the problem. Sometimes it's when a new Code Cop is introduced, sometimes it's a lack of knowledge howto fix the warning correct. Normally during the code review these are caught, but sometimes a few slips through.

Some warnings are easly solved, for example LC0013 from the BusinessCentral.LinterCop; LC0013 | Any table with a single field in the PK of type code or text, should have set NotBlank on the PK field.

EXAMPLE BAD CODE

#pragma warning disable LC0013
field(1; "Code"; Code[20])
{
    Caption = 'Code', comment = 'NLB="Code"';
}
#pragma warning restore LC0013

EXAMPLE GOOD CODE

field(1; "Code"; Code[20])
{
    Caption = 'Code', comment = 'NLB="Code"';
    NotBlank = false;
}

Adding the NotBlank-property is almost as much work as adding the pragma. So we would like to ignore the LC0013 pragma's. So even if there's a pragma still show a warning. There's a reportSuppressedDiagnostics parameter on the ALC.exe, maybe we can use this?

Describe the solution you'd like For the example case above it would be great to have an extra property on the compile(v1/v2)-step.

- task: ALOpsAppCompiler@2
  displayName: 'ALOps App Compiler'
  inputs:
    ignorepragma: 'LC0013,LC0014'
waldo1001 commented 1 year ago

Changing code is something we don't do - that's opening pandora's box .. ;-).

But we'll try to map the "reportSuppressedDiagnostics" to a new parameter. Good idea!

Arthurvdv commented 1 year ago

Changing code is something we don't do - that's opening pandora's box .. ;-).

Totally agree on this!

AdminHodor commented 1 year ago

Dear @Arthurvdv ,

The feature is implementen, although we still need to finetune a part of your request.

In the ALOpsAppCompile step you'll now find a parameter 'ignorepragmas', when any string is present all pragma's will be visible during compile.

We'll try to implement the list, ( 'LC0013,LC0014' ) in the next version, so you could already set ignore pragma's to a comma seperated list now. The question is, how do you want the list to react ?

Kind regards,

Arthurvdv commented 1 year ago

hi @AdminHodor,

I think that the second option, Only Error/Warning on the codes in ignorepragmas, set all others to "information", suits te best for the requirement. In that way, if a #pragma is set voor example the LC0013 the pipeline wil report a Info/Error/Warning.

I'm note sure if you can apply the ruleset also on this? I mean that if the LC0013 as default shows an Info, and in a custom ruleset the LC0013 is set as a warning, show this as a warning?

waldo1001 commented 1 year ago

We'll take this feedback into consideration!

Arthurvdv commented 11 months ago

Is there any update on this?

In the current form the feature is not usable, because when setting a value in the ignorepragmas all(!) pragma's are ignored and some of them throw an warning. As we have failonwarnings: True for all our pipelines, we can't set this feature at the moment, because we only want to have some of them ignored.

waldo1001 commented 11 months ago

part of next release:

- task: ALOpsAppCompiler@2
  inputs:
    artifacttype: 'Sandbox'
    versionselect: Weekly
    appversiontemplate: 'A.A.*.0'
    alcodeanalyzer: CodeCop, PerTenantExtensionCop
    ignorepragmas: 'AA0215,AA0137'

This will basically ignore those two pragmas, but "enable" all others. Come to think of it - quite a neat feature ;-).

Arthurvdv commented 10 months ago

Did this feature made it into the release of [v1.461] - 2023-11-20 ?

AdminHodor commented 10 months ago

Dear @Arthurvdv ,

Apologies for not updating the issue, this is indeed included in the v1.461 release. Once a value is in "ignorepragmas", we instruct the compiler to ignore all pragma's. The "ignorepragmas" parameter can contain an array (comma-seperated) of Pragma's, the codes in this array will be marked as "INFO" instead of WARNING or ERROR. The pragma codes in this array will thus be ignored.

Kind regards,