jeremiah-c-leary / vhdl-style-guide

Style guide enforcement for VHDL
GNU General Public License v3.0
187 stars 38 forks source link

Capitalisation rules for type identifiers do not work on types that are declared in a different file. #1232

Open JHertz5 opened 5 days ago

JHertz5 commented 5 days ago

Environment VSG v3.26.0

Describe the bug The rule _type004 enforces capitalisation of type identifiers. This rule appears to be triggered by the type declaration. As a result, this means that types declared elsewhere, e.g. in a package that is a separate file, or that is part of the standard library, such as character are not corrected by this rule.

I suspect that the latter example is the reason that the rule ieee_500 was introduced. This rule solves the problem for a few predefined types, namely:

but it leaves out many more, inclding natural which is defined by the parser, but isn't included in the list of tokens that _ieee500 acts upon. There are also many other types like string, character, etc. that aren't included in the terms recognised by the parser. My point is that if we could try to add every predefined type, but that just adds more code and puts us at a greater risk of introducing human errors.

Instead, I suggest that, unless there is another reason that these keywords are treated as special, we treat them as any other type name and handle the capitalisation of type and subtype names in a more generic way..

One thing that we could do is replace rule _ieee005 with a single rule for _typemark tokens. That way, _type004 and _subtype002 will continue to handle types and subtype with local declarations, and the new rule, _type_mark500, (or perhaps a better name), could handle type marks for declared elsewhere, including VHDL's predefined types.

To Reproduce Steps to reproduce the behavior:

  1. Create a file "test.vhd" with the following contents:
    
    library ieee;
    use ieee.std_logic_1164.all;
    use ieee.numeric_std.all;
    use ieee.math_real.all;

entity test is end entity test;

architecture rtl of test is

type my_type is range 10 to 100;

subtype my_subtype is MY_TYPE range 10 to 20; subtype my_subtype_t is MY_EXTERNAL_TYPE range 10 to 20; subtype my_int is INTEGER range 10 to 100; subtype my_nat is NATURAL range 10 to 100; subtype my_pos is POSITIVE range 10 to 100; subtype my_char is CHARACTER range 'a' downto 'b';

signal s_my_subtype : MY_EXTERNAL_SUBTYPE; signal s_my_subtype : MY_SUBTYPE; signal s_my_subtype_t : MY_SUBTYPE_T; signal s_my_int : MY_INT; signal s_my_nat : MY_NAT; signal s_my_pos : MY_POS; signal s_my_char : MY_CHAR; signal s_my_type_t : MY_TYPE_T; signal s_int : INTEGER; signal s_nat : NATURAL; signal s_pos : POSITIVE; signal s_char : CHARACTER;

begin

end architecture rtl;

2. Run `vsg -f test.vhd --fix`
3. Observe test.vhd:
```vhdl
library ieee;
  use ieee.std_logic_1164.all;
  use ieee.numeric_std.all;
  use ieee.math_real.all;

entity test is
end entity test;

architecture rtl of test is

  type my_type is range 10 to 100;

  subtype my_subtype   is my_type range 10 to 20;
  subtype my_subtype_t is MY_EXTERNAL_TYPE range 10 to 20;
  subtype my_int       is integer range 10 to 100;
  subtype my_nat       is NATURAL range 10 to 100;
  subtype my_pos       is POSITIVE range 10 to 100;
  subtype my_char      is CHARACTER range 'a' downto 'b';

  signal s_my_subtype   : MY_EXTERNAL_SUBTYPE;
  signal s_my_subtype   : my_subtype;
  signal s_my_subtype_t : my_subtype_t;
  signal s_my_int       : my_int;
  signal s_my_nat       : my_nat;
  signal s_my_pos       : my_pos;
  signal s_my_char      : my_char;
  signal s_my_type_t    : MY_TYPE_T;
  signal s_int          : integer;
  signal s_nat          : NATURAL;
  signal s_pos          : POSITIVE;
  signal s_char         : CHARACTER;

begin

end architecture rtl;
  1. Note that MY_EXTERNAL_TYPE, MY_EXTERNAL_SUBTYPE, NATURAL, POSITIVE, and CHARACTER are still upper case. The latter three are all predefined VHDL types.

Expected behavior I expect the tool to enforce capitalisation on all type marks consistently.

JHertz5 commented 5 days ago

I will raise a PR with my suggested changes so that we can see what it looks like. I understand that this change in approach may not be agreed with (or may prove to have some errors that I have not forseen) so we can always reject the change if it doesn't work out.

JHertz5 commented 5 days ago

I've created a PR #1233 to resolve this. I'm open to any feedback/rejection. One potential issue to consider is overlap with _type014 and _subtype002 - maybe those rules can be set to be disabled by default if we prefer this approach?