Duplicated rules enforcing indentation on null statements

[JHertz5]

Environment Latest main, git hash:

dd647776503a1a4bb0f4be6c844b8394e38dc106

Describe the bug I've discovered that we now have two rules governing the indentation of null statements, case_013 and null_statement_300. I didn't check thoroughly enough before implementing the latter rule🤦

To Reproduce Steps to reproduce the behavior:

1. Create file test_null.vhd

architecture rtl of fifo is

begin

my_proc : process is begin

case expression is

  when others =>

        null;

end case;

end process my_proc;

end architecture rtl;

2. Run `vsg -f test_null.vhd`
3. See output

$ bin/vsg -f test_null.vhd

File: test_null.vhd

Phase 4 of 7... Reporting Total Rules Checked: 499 Total Violations: 2 Error : 2 Warning : 0 ----------------------------+------------+------------+-------------------------------------- Rule | severity | line(s) | Solution ----------------------------+------------+------------+-------------------------------------- case_013 | Error | 13 | Use 8 spaces for indent null_statement_300 | Error | 13 | Use 8 spaces for indent ----------------------------+------------+------------+-------------------------------------- NOTE: Refer to online documentation at https://vhdl-style-guide.readthedocs.io/en/latest/index.html for more information.

[JHertz5]

Hi @jeremiah-c-leary

Apologies for causing this! I think I just assumed that, since there wasn't a rule for the case of null, there must not be one for the indent as well.

As I see it, there are two solutions that we could implement:

1. Walk back #1271, and replace the null_statement rules with specialised for the various cases in which null could appear.
2. Deprecate case_013 in favour of the more general null_statement_300.

Personally, I prefer the second option as this means fewer rules, less work, less complexity, etc. but I'm very willing to implement the first option if that's your preference. Any thoughts?

Thanks, Jukka

[jeremiah-c-leary]

Morning @JHertz5 ,

Apologies for causing this! I think I just assumed that, since there wasn't a rule for the case of null, there must not be one for the indent as well.

No need to apologize. This is the result of me not having a good game plan for implementing rules when I first started this project.

Personally, I prefer the second option as this means fewer rules, less work, less complexity, etc. but I'm very willing to implement the first option if that's your preference. Any thoughts?

I agree with you. Deprecating case_013 is keeping in the rule selection documentation of using production names for rules.

At one point I wanted to redefine all the rules according to the "new" method, but decided that would be too much chaos. Maybe for a 4.0 release where users will expect large changes.

--Jeremy

[JHertz5]

Hi @jeremiah-c-leary

I agree with you. Deprecating case_013 is keeping in the rule selection documentation of using production names for rules.

Excellent, I'll raise a PR to deprecate case_013.

At one point I wanted to redefine all the rules according to the "new" method, but decided that would be too much chaos. Maybe for a 4.0 release where users will expect large changes.

Yes, I think that's a good idea, it would likely result in a lot of broken configs. A major rework may be a good time to standardise all of the rule numbers to match their phase as well.

Thanks, Jukka

[JHertz5]

I've created PR #1284 to resolve this issue by deprecating case_013.

[jeremiah-c-leary]

Evening @JHertz5 ,

PR looks good. I will merge it to master.

--Jeremy

[JHertz5]

Thanks very much!