jeremiah-c-leary / vhdl-style-guide

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

Rules concurrent_012 and concurrent_401 interpreting simple assignments as array assignments #943

Closed flyinged closed 1 year ago

flyinged commented 1 year ago

Environment WSL2 on Windows 10 Enterprise vsg -h returns VHDL Style Guide (VSG) version: 3.14.0+zip.file

Describe the bug I had to disable rules concurrent_012 and concurrent_401 because they were messing up simple signal assignments

To Reproduce Use a test file (test.vhd in this example) containing the following code snippet:

new_phase <= (not ccd_i_init and init_r) or
             ccd_o_new_meas              or
             end_of_int                  or
             end_of_bin                  or
             end_of_ovr                  or
             end_of_rdout                or
             end_of_line                 or
             ccd_o_end_of_meas;

Running the command vsg -f test.vhd

I get the following output (line 91 is the 1st line of the snippet above):

================================================================================                           
File:  test.vhd                                                                                            
================================================================================                           
Phase 1 of 7... Reporting                                                                                  
Total Rules Checked: 153                                                                                   
Total Violations:    3                                                                                     
  Error   :     3                                                                                          
  Warning :     0                                                                                          
----------------------------+------------+------------+--------------------------------------              
  Rule                      |  severity  |  line(s)   | Solution                                           
----------------------------+------------+------------+--------------------------------------              
  concurrent_012            | Error      |         91 | Move parenthesis after assignment to the next line.
  concurrent_012            | Error      |         91 | Add carriage return after open parenthesis.        
  concurrent_012            | Error      |         91 | Move closing parenthesis to the next line.         
----------------------------+------------+------------+--------------------------------------              

According to the documentation, rule concurrent_012 should "check the structure of multiline concurrent simple signal assignments that contain arrays". Since the reference assignment does not contain an array, the rule should not apply.

If I the run: vsg -f test.vhd --fix

The output is:

================================================================================
File:  test.vhd
================================================================================
Phase 1 of 7... Reporting
Total Rules Checked: 153
Total Violations:    1
  Error   :     1
  Warning :     0
----------------------------+------------+------------+--------------------------------------
  Rule                      |  severity  |  line(s)   | Solution
----------------------------+------------+------------+--------------------------------------
  concurrent_011            | Error      |         89 | Move code after assignment to the same line as assignment.
----------------------------+------------+------------+--------------------------------------

The fixed code is:

  new_phase <=
  (
    not ccd_i_init and init_r
  ) or
    ccd_o_new_meas              or
    end_of_int                  or
    end_of_bin                  or
    end_of_ovr                  or
    end_of_rdout                or
    end_of_line                 or
    ccd_o_end_of_meas;

Given the code, the violation of concurrent_011 is expected. It's not clear anyway, why the code won't be fixed, since the rule is configured with fixable: true. The overall effect is that the unexpected application of rule concurrent_012seems to cause the described violation of rule concurrent_011.

If I run: vsg -f test.vhd -c test_config.yaml

with test_config.yaml containing a configuration that disables rule concurrent_012, then I get the following result:

================================================================================
File:  test.vhd
================================================================================
Phase 5 of 7... Reporting
Total Rules Checked: 520
Total Violations:    7
  Error   :     7
  Warning :     0
----------------------------+------------+------------+--------------------------------------
  Rule                      |  severity  |  line(s)   | Solution
----------------------------+------------+------------+--------------------------------------
  concurrent_003            | Error      |         90 | Indent with 15 space(s)
  concurrent_003            | Error      |         91 | Indent with 15 space(s)
  concurrent_003            | Error      |         92 | Indent with 15 space(s)
  concurrent_003            | Error      |         93 | Indent with 15 space(s)
  concurrent_003            | Error      |         94 | Indent with 15 space(s)
  concurrent_003            | Error      |         95 | Indent with 15 space(s)
  concurrent_003            | Error      |         96 | Indent with 15 space(s)
----------------------------+------------+------------+--------------------------------------

and the following fixed code (starting line: 89):

  new_phase <= (not ccd_i_init and init_r) or
  ccd_o_new_meas              or
  end_of_int                  or
  end_of_bin                  or
  end_of_ovr                  or
  end_of_rdout                or
  end_of_line                 or
  ccd_o_end_of_meas;

Given the code, the violation of concurrent_003 is also expected. Still not clear, why the code won't be fixed, since the rule is configured with fixable: true. The overall effect seems in this case caused by rule concurrent_401 that also interprets the simple assignment as an assignment containing an array.

If I run (on the original test.vhd file) vsg -f test.vhd -c test_config.yaml --fix

with test_config.yaml containing a configuration that disables both rule concurrent_012 and rule concurrent_401, then I get the following fixed code:

  new_phase <= (not ccd_i_init and init_r) or
               ccd_o_new_meas              or
               end_of_int                  or
               end_of_bin                  or
               end_of_ovr                  or
               end_of_rdout                or
               end_of_line                 or
               ccd_o_end_of_meas;

That's what I originally expected.

jeremiah-c-leary commented 1 year ago

Good Evening @flyinged,

I pushed an update for this to the issue-943 branch. When you get a chance could you check it out and let me know if it fixes the issue on your end.

Thanks,

--Jeremy

flyinged commented 1 year ago

Hi @jeremiah-c-leary

I can confirm you that the changes fix the issue also on my side.

Thanks Alessandro