jeremiah-c-leary / vhdl-style-guide

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

Array Multiline Structure Rules modify parenthesis that are not indicating sub-arrays within the array assignment #1034

Closed JHertz5 closed 10 months ago

JHertz5 commented 10 months ago

Environment master branch

VHDL Style Guide (VSG) version: 3.17.0.dev19
Git commit SHA: 92503eee

Describe the bug When presented with an aggregate assignment in which an array element is referenced by an expression that include parenthesis (e.g. a value of another array), the rule modifies those parentheis, even though they do no indicate a sub-array. This results in behaviour that is likely not desired by the user.

To Reproduce Steps to reproduce the behavior:

  1. Create a file called test.vhd with the following contents:

    
    architecture rtl of test is
    
    constant c_constant : t_array :=
    (
    c_enum_list(ENUM_LITERAL_1) => 1,
    others => 'X'
    );
    
    signal data_concurrent  : t_array;
    signal data_sequuential : t_array;

begin

data_concurrent <= ( c_enum_list(ENUM_LITERAL_1) => 1, others => 'X' );

proc_test : process (all) is

variable v_variable : t_array;

begin

v_variable :=
(
  c_enum_list(ENUM_LITERAL_1) => 1,
  others => 'X'
);

data_sequuential <=
(
  c_enum_list(ENUM_LITERAL_1) => v_array(1),
  others       => 'X'
);

end process proc_test;

end architecture rtl;

2. Run `vsg -f ./test.vhd` and observe the following output:

================================================================================ File: ./test.vhd

Phase 1 of 7... Reporting Total Rules Checked: 157 Total Violations: 6 Error : 6 Warning : 0 ----------------------------+------------+------------+-------------------------------------- Rule | severity | line(s) | Solution ----------------------------+------------+------------+-------------------------------------- concurrent_012 | Error | 16 | Add carriage return after open parenthesis. concurrent_012 | Error | 16 | Move closing parenthesis to the next line. variable_assignment_008 | Error | 28 | Add carriage return after open parenthesis. variable_assignment_008 | Error | 28 | Move closing parenthesis to the next line. sequential_009 | Error | 34 | Add carriage return after open parenthesis. sequential_009 | Error | 34 | Move closing parenthesis to the next line. ----------------------------+------------+------------+-------------------------------------- NOTE: Refer to online documentation at https://vhdl-style-guide.readthedocs.io/en/latest/index.html for more information.

5. Run `vsg -f ./test.vhd --fix` and observe the following changes to test.vhd:
```vhdl
architecture rtl of test is

  constant c_constant : t_array :=
  (
    c_enum_list(
                 ENUM_LITERAL_1
               ) => 1,
    others => 'X'
  );

  signal data_concurrent  : t_array;
  signal data_sequuential : t_array;

begin

  data_concurrent <=
  (
    c_enum_list(
                 ENUM_LITERAL_1
               ) => 1,
    others       => 'X'
  );

  proc_test : process (all) is

    variable v_variable : t_array;

  begin

    v_variable :=
    (
      c_enum_list(
                   ENUM_LITERAL_1
                 ) => 1,
      others => 'X'
    );

    data_sequuential <=
    (
      c_enum_list(
                   ENUM_LITERAL_1
                 ) => v_array(1),
      others       => 'X'
    );

  end process proc_test;

end architecture rtl;

Expected behavior I would expect the expression c_enum_list(ENUM_LITERAL_1) to be unaffected by these rules since these parenthesis do not create a sub-array within the array.

jeremiah-c-leary commented 10 months ago

Good Evening @JHertz5 ,

I pushed an update to the issue-1034 branch. When you get a chance could you check it out and let me know if there are any other issues.

Thanks,

--Jeremy

JHertz5 commented 10 months ago

Hi @jeremiah-c-leary. Thanks so much for taking a look at this. I have run your branch and it looks perfect!

jeremiah-c-leary commented 10 months ago

Morning @JHertz5 ,

Awesome, I will merge this into master.

--Jeremy