How to enforce snail_case for Function/Procedure arguments?

[obruendl]

Is it possible to enforce snail_case for VC procedure arguments? I configured them for "case: lower" - but this does not report errors if I name an argument in camelCase (starting lower-case but containing upper-case letters).

[JHertz5]

Hi @obruendl.

Lowercase should allow for snail_case. Are you asking for the tool to enforce that there are underscores between words? That is may be much more difficult.

With regards to the problem that you described, could you please provide example code and configuration to demonstrate your problem? This will make it easier to reproduce your problem and investigate.

[obruendl]

My wish would be for the tool to not allow uppercase letters.

Acceptable: some_parameter someparameter

Not acceptable: someParameter Someparameter SomeParameter

I created a test-case. Follow-the steps below:

1. Checkout Open-Logic (feature/linting-snailcase-arguments branch)
2. Run vsg on the test file (vsg -c ./lint/config/vsg_config.yml ./lint/config/vsg_config_overlay_vc.yml -f olo_test_axi_slave_vc.vhd)

I don't get any errors although the files contains non lower-case arguments:

Could it be that I missed some configuration value that would check procedure arguments? "vsg_config_overlay_vc.yml" just contains entries for all the case-rules I identified in the documentation. But after re-checking this I noticed that I couldn't find any rule that is clearly related to function/procedure arguments.

Which rule is responsible for checking procedure and function argument case?

[JHertz5]

@obruendl I've taken a look and also couldn't find any rules to enforce case on formal parameters in functions and procedures. I've done a test to check - 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

I'd have expected errors to be raised if there were rules that governed these things, so this confirms that there are no rules to enforce case on formal parameter names for functions and procedures. I suggest that we raise a request for such rules to be added.

[obruendl]

@JHertz5

Sounds sane. Please raise the related request and let me know once it is implemented, so I can add it to my config.

In the meantime I will have a special eye on those currently unchecked parameters in reviews.

[JHertz5]

Hi @obruendl. I've put something together on this branch. Feel free to give it a try and let me know whether it works for you. The branch currently works for function and procedure declarations, and for procedure calls, but not for function calls. This is because VSG's parser does not currently recognise function calls, so it will be a fair bit of work to get that implemented. I'll have a go, but it will take some time.

In the meantime, I noticed that your config lists case rules individually and sets them all to lower. I don't know if you're aware already, but in case you're not, you can use

rule:
  group:
    case:
      case: "lower"

to set all case rules (including future rules that haven't been added yet) to be lowercase. This means that you can have a smaller config file and don't have to update it to include new case rules. Just a suggestion :)

[JHertz5]

FYI, I've raised a PR, #1251, to address this issue.

[obruendl]

@JHertz5

I tried to simplify the ./lint/config/vsg_config_overlay_vc.yml file as you suggested:

rule:
    group:
        case:
            case: lower
    architecture_025:
        names: ["a"]
    constant_600:
        disable: true
    function_017:
        case: lower
        case_exceptions: []
    generic_600:
        disable: true
    length_002:
        disable: true # Better long VC files than many short ones
    variable_004:
        case: lower
        suffix_exceptions: []
    variable_600:
        disable: true

However, with this file VSG complains about many snail_case identifiers in the related code. To me it looks like this is not working correctly - at least if I use it as an overlay to a base config (./lint/config/vsg_config.yml) which defines all the rules separately.

Let me know my modification to the file is not matching what you suggested. Ohterwise, I guess this would be a new issue, right?

[obruendl]

@JHertz5

Regarding the snail_case checking of function/procedure arguments in the function/procedure definition: Which rule does check this? My gut-feeling would be that there should be a separate rule to check case of arguments. It would be well possible that somebody wants to enforce function names being PascalCase but arguments shall be camelCase.

[obruendl]

Hi @obruendl. I've put something together on this branch. Feel free to give it a try and let me know whether it works for you. The branch currently works for function and procedure declarations, and for procedure calls, but not for function calls. This is because VSG's parser does not currently recognise function calls, so it will be a fair bit of work to get that implemented. I'll have a go, but it will take some time.

I tested this branch but at least in the config I use in open-logic, it did not produce any errors or warnings on function/procedure arguments tarting with upper-case letters.

Let me know if I have to change anything on my config.

[JHertz5]

Hi @obruendl

However, with this file VSG complains about many snail_case identifiers in the related code.

I'm sorry to hear that it's not working. The config that you've posted looks like it should work fine. I tried your config on a small sample

entity test_ent is
  port (
    i_clk   : in    std_logic;
    i_reset : in    std_logic
  );
end entity test_ent;

architecture arch_rtl of test_ent is

begin

  my_signal_test <= assigning_value;

end architecture arch_rtl;

and it worked fine for me, no warnings raised. But I'm not using multiple configs, maybe that's part of the problem. The config files in the vsg call are processed from left to right; is your overlay config the second file listed (e.g. -c config/yml config_overlay.yml)? Yes, if you could create a new issue and provide a minimal reproducible example, I can take a deeper look.

Regarding the snail_case checking of function/procedure arguments in the function/procedure definition: Which rule does check this?

If I'm understanding correctly, your question is about whether there are separate rules for function names and function parameter names? If so, yes. At the moment, there is a rule called function_017 which enforces case on the function name at the start of the function declaration, and another rule called function_506 which enforces case on the function name and the end of the function declaration. The code

architecture rtl of test is

  function MY_FUNC return integer is

  begin

  end function MY_FUNC;

begin

end architecture rtl;

produces the following output

$ bin/vsg -f test.vhd 
================================================================================
File:  test.vhd
================================================================================
Phase 6 of 7... Reporting
Total Rules Checked: 785
Total Violations:    2
  Error   :     2
  Warning :     0
----------------------------+------------+------------+--------------------------------------
  Rule                      |  severity  |  line(s)   | Solution
----------------------------+------------+------------+--------------------------------------
  function_017              | Error      |          3 | Change "MY_FUNC" to "my_func"
  function_506              | Error      |          7 | Change "MY_FUNC" to "my_func"
----------------------------+------------+------------+--------------------------------------
NOTE: Refer to online documentation at https://vhdl-style-guide.readthedocs.io/en/latest/index.html for more information.

In the PR that I have raised, there are new rules, function_507 and function_508 which enforce the case of parameter names in function declarations and the consistent case in usage of those parameters respectively.

I tested this branch but at least in the config I use in open-logic, it did not produce any errors or warnings on function/procedure arguments tarting with upper-case letters.

Hmm that's strange. I'll have a look.

[JHertz5]

@obruendl I've downloaded https://github.com/open-logic/open-logic/blob/main/lint/config/vsg_config.yml, and pasted it into a file test.yml. I've then created a file called test.vhd with the following contents

architecture rtl of test is

    function myFunc (
            MY_PARAM : integer) return integer is
    begin

    end function;

begin

end architecture;

I then get the following output

$ bin/vsg -f test.vhd -c test.yml
================================================================================
File:  test.vhd
================================================================================
Phase 6 of 7... Reporting
Total Rules Checked: 750
Total Violations:    1
  Error   :     1
  Warning :     0
----------------------------+------------+------------+--------------------------------------
  Rule                      |  severity  |  line(s)   | Solution
----------------------------+------------+------------+--------------------------------------
  function_507              | Error      |          4 | Change "MY_PARAM" to "my_param"
----------------------------+------------+------------+--------------------------------------
NOTE: Refer to online documentation at https://vhdl-style-guide.readthedocs.io/en/latest/index.html for more information.

so it looks like the config is not the problem. Are you definitely on the right branch? Is it possible that you're using the globally installed vsg rather than the branch? What happens if you run bin/vsg -rc function_507? If you're on the right branch, you should see

$ bin/vsg -rc function_507
{
  "rule": {
    "function_507": {
      "indent_style": "spaces",
      "indent_size": 2,
      "phase": 6,
      "disable": false,
      "fixable": true,
      "severity": "Error",
      "case": "lower",
      "prefix_exceptions": [],
      "suffix_exceptions": [],
      "case_exceptions": []
    }
  }
}

[obruendl]

@JHertz5

That I did not get any errors on wrong case for function argument makes sense - when I tested it, you didn't write me yet that you added a new rule function_507/function_508 so I didn't add them to my config and hence I didn't get errors on wrong case.

I tried again. I installed the branch you pointed me to. To make sure you can verify this was installed correctly, I paste the output to vsg --version below:

VHDL Style Guide (VSG) version: 3.13.1.post228+git.dd647776

The branch you pointed me to is: https://github.com/JHertz5/vhdl-style-guide/tree/issue-1251

I noticed that the branch is named 1251 but this issue is 1249. Was the reference possibly wrong?

I added the new rules to my config:

...
    function_507:
        case: lower
    function_508:
        case: lower
...

I then run linting:

vsg -c ./lint/config/vsg_config.yml ./lint/config/vsg_config_overlay_vc.yml -f ./test/tb/olo_test_axi_slave_vc.vhd 

But I get the error that function_507 is not known:

Error while processing ./test/tb/olo_test_axi_slave_vc.vhd: ERROR: Rule function_507 referenced in configuration could not be found

Why?

Screenshot summarizing the situation is attached:

Note: I am not blocked currently. But I would want to see that the modifications you made are working correctly, so I can integrate them after the next VSG release.

[obruendl]

Furthermore: Did you also implement the additional rules for procedures? Are those the same numbers (procedure_507/508)?

[JHertz5]

Hi @obruendl.

I noticed that the branch is named 1251 but this issue is 1249. Was the reference possibly wrong?

The reference to #1251 is intentional. That is an issue that I raised it because this issue has a specific name that only covers a subset of the problem, and is marked as a question. I wanted to state the required changes in very clear terms and mark it as a feature request.

But I get the error that function_507 is not known: Why?

I noticed that you used vsg as the command, rather than bin/vsg. It looks like you're using the globally installed version of VSG rather than the VSG from the branch. If you run which vsg, it will point you to the directory where VSG is installed, which I expect is not the version of VSG that we want to use when testing the changes on the branch.

In order to test these changes, you need to clone this repository (if you haven't already) and change it the branch mentioned above. In the clone of the repository, there will be a directory called bin, which contains a file called vsg - this is the entrypoint for running the branch's version of VSG. Changing the branch in a clone of the repo will not affect the installed version of VSG. If you look at my previous comments, you will see that I am using bin/vsg for this reason. Please try the same commands with the bin/vsg from your clone of VSG appended to the front of them and, hopefully, you should have more success. (bin/vsg is a relative path, so you may need to include the full path to your clone if you're trying to use the branch version of VSG in a different directory.)

Furthermore: Did you also implement the additional rules for procedures? Are those the same numbers (procedure_507/508)?

Unfortunately, procedure_507 is already taken, so the equivalent procedure rules are called procedure_508 and procedure_509. There is also a new rule called procedure_call_502, which checks for consistent case in procedure calls. All of these rules are enabled and set to lower case by default, so, as I demonstrated earlier, they should activate and work without any changes to your config YAML file (unless the YAML file has changed to override the defaults, e.g. disabling all rules that aren't explicitly enabled).

[obruendl]

@JHertz5

I can confirm the fix.

Running the command vsg was fine. I did run tox and then install the wheel in dist. I am not sure what went wrong before - probably I had an old *.whl file in the dist folder and accidentally installed this one. After clearing everything, building and installing, the new rules were found and work as expected.

Hence - this issue can be closed as soon as issue-1251 is merged into main.

[JHertz5]

Excellent, thanks very much for confirming.