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

New rule for empty statements #717

Merged
merged 4 commits into from
Aug 20, 2024

Conversation

ans-bar-bm
Copy link
Contributor

Related discussion: #716

If there is further discussion needed / you do not want this rule please feel free to tell me / close this; i just had it implemented locally and figured may as well put it up as a PR here.

Currently reports a warning for all empty statements besides:

  • "if guards"
    if Customer.Get('10000') then; // <- empty, but avoids a runtime error
  • empty case lines
case EnumValue of 
    SomeEnum::First:
        ; // <- empty, but does not fall in else part
    SomeEnum::Second:
        Message('Some message.');
    else
        Error('Some error.');
end;

Not sure about any additional special cases, if you know any please feel free to add them / tell me.

Also not sure about all the necessary parts/steps for a new rule, i looked at a previous PR and copied from there.
If there is anything missing or something left to do i can help with please let me know.

@Arthurvdv
Copy link
Collaborator

Thank you for contributing this rule, this is a great rule to have in the LinterCop!

I've taken the liberty to renumber this from 0068 to 0069 and increase the default severity from Warning to Info. Personally I'm going to set this to Warning in our own pipelines, but I'm a bit hesitated to set this as the default where it could possible break other pipelines out there. At the moment I use the rule of thumb that new rules only get a default severity set to Warning if the impact is high, for example in the case of a runtime error.

@Arthurvdv Arthurvdv merged commit c0c0892 into StefanMaron:prerelease Aug 20, 2024
42 checks passed
@ans-bar-bm ans-bar-bm deleted the feature/emptyStatements branch August 20, 2024 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants