jeremiah-c-leary / vhdl-style-guide

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

Case rules for formal parameters in functions and procedures #1251

Closed JHertz5 closed 2 weeks ago

JHertz5 commented 1 month ago

Is your feature request related to a problem? Please describe. At the moment (as of v3.27.0), there doesn't appear to be any rule to enforce case for formal parameters in functions or procedures. After looking for such rules, I tested for their presence as follows:

Config: Setting all case rules to "lower" and making sure that none are disabled.

rule:
  group:
    case:
      case: "lower"
      disable: False

Code: The parameter names of the function and procedure are in uppercase.

architecture rtl of test is

  procedure my_proc (
    PARAM1 : in integer;
    PARAM2 : out integer
  ) is
  begin

  end procedure my_proc;

  function my_func (
    PARAM1 : in integer;
    PARAM2 : out integer
  ) return integer is
  begin

  end function my_func;

begin

  my_proc (
           PARAM1 => MY_PARAM1,
           PARAM2 => MY_PARAM2
         );

  my_sig <= my_func (
                     PARAM1 => MY_PARAM1,
                     PARAM2 => MY_PARAM2
                   );

end architecture rtl;

Output

$ vsg -f test.vhd -c test.yml --fix
================================================================================
File:  test.vhd
================================================================================
Phase 7 of 7... Reporting
Total Rules Checked: 769
Total Violations:    0
  Error   :     0
  Warning :     0

Describe the solution you'd like I'd like to have rules that would enforce the case of formal parameters, e.g. changing the code above to:

architecture rtl of test is

  procedure my_proc (
    param1 : in integer;
    param2 : out integer
  ) is
  begin

  end procedure my_proc;

  function my_func (
    param1 : in integer;
    param2 : out integer
  ) return integer is
  begin

  end function my_func;

begin

  my_proc (
           param1 => MY_PARAM1,
           param2 => MY_PARAM2
         );

  my_sig <= my_func (
                     param1 => MY_PARAM1,
                     param2 => MY_PARAM2
                   );

end architecture rtl;
JHertz5 commented 1 month ago

I'm planning to raise a PR to address this.

JHertz5 commented 1 month ago

This would also resolve issue #1249.

JHertz5 commented 1 month ago

Use port_010 as an example.

jeremiah-c-leary commented 1 month ago

Afternoon @JHertz5 ,

Could you add some nested functions and procedures to your function_508 and procedure_509 tests? It would ensure your implementation can handle names that are similar but formatted differently based on the hierarchy.

Thanks,

--Jeremy

JHertz5 commented 1 month ago

Hi @jeremiah-c-leary.

Yes absolutely! Part of the reason that the PR is not ready yet is actually because I've realised that the current implementation does not handle nested subprograms. I'm hoping to find some time to get the PR completed in the coming week.

Thanks, Jukka

JHertz5 commented 1 month ago

I've marked the PR as ready for review.

JHertz5 commented 3 weeks ago

I've discovered another problem in the PR 🤦 I'll mark it as a draft until it's been corrected. I've fixed that issue - the tool was crashing if there was a subprogram without an interface list. This problem has been corrected now and the PR has been marked as ready for review again.