Indentation wrong for Functions/Procedures Without Arguments

[obruendl]

Environment Ubuntu 20.04

Describe the bug

VSG seems unable to handle procedure without arguments - indentation was messed up in this case. I worked around this by adding an ugly "dummy" parameter. Could you check if it can be configured to accept this case? Probably it is a problem with the indentation rules. Example here

It's of course possible that this somehow has to do with my indent configuration. You can find my config file here

To Reproduce

1. Checkout Open-Logic (develop branch)
2. remove the "dummy workaround" from olo_test_2c_vc.vhd
3. Run vsg on the file ("vsg -c ./lint/config/vsg_config.yml ./lint/config/vsg_config_overlay_vc.yml -f olo_test_2c_vc.vhd)

Regarding the "vsg_config_overlay_vc": This is required beause I have different coding rules for verification components and my main codebase. See Open-Logic documentation for details.

Expected behavior Indentation for the function without arguments should be "sane". However, I see that indentation after the function header is messed up (more inidentation than I would expect). Should be rather obvious - if not, feel free to check back with me.

Screenshots None

Additional context None

[JHertz5]

Hi @obruendl. Thanks for raising this issue. I will try to take a look soon. The config file that you've referenced is > 4k lines long. If you would be able to provide a minimal reproducible example, i.e. a minimal config file that demonstrates your problem, it would help us to investigate this more quickly.

[obruendl]

See attached example.

test_case.zip

[JHertz5]

Hi @obruendl. Thanks very much, that's helped a lot! With your MRE, I'm able to easily reproduce your problem.

Your snippet with default config ($ vsg -f test_pkg.vhd --fix)

package olo_base_pkg_math is

  function log2 return natural;

end package olo_base_pkg_math;

package body olo_base_pkg_math is

  function log2 return natural is
  begin

    return 3;

  end function log2;

end package body olo_base_pkg_math;

Your snippet with the custom config ($ vsg -f test_pkg.vhd -c vsg_test.yml --fix)

package olo_base_pkg_math is

  function log2 return natural;

  end package olo_base_pkg_math;

  package body olo_base_pkg_math is

    function log2 return natural is
      begin

        return 3;

      end function log2;

    end package body olo_base_pkg_math;

I can see that after the function specification with no parameter list, the indent level increases, and I can see that this does not occur if there is a parameter list in the function specification.

The relevant portion of the custom config file is as follows:

indent:
    tokens:
        function_specification:
            function_keyword:
                token: current
                after: "+2"
            close_parenthesis:
                token: "-1"
                after: "-1"

I've taken a look and I can see the source of this problem. As I'm sure you know, this config tells VSG how to indent different tokens in the code. In this instance, VSG is being told, when there is a function keyword within a function specification, change the indent level from the line after by +2, and when there is a ) within a function specification, change the indent level by -1 on the same line by -1 and by another -1 from the line after.

Therefore, if there is a function specification with a parameter list, the function keyword increases the indent level by 2, but the ) of the parameter list subtracts 2 and the indent level after the function specification is back to normal. However, if the parameter list is not there, the function keyword increases the indent level by 2, but there is no ) of the parameter list to counteract this, so the indent level after the function specification is changed. Here's an example:

  -- indent level X
  function log2 (value : integer) return natural;
  -- indent level X+2-1-1 = X

  -- indent level X
  function log2 return natural;
    --indent level X+2

So the problem is that there is a +2 indent, following by a token (which is optional) that is expected to reduce the indent back down.

If you use the default indent config:

indent:
    tokens:
        function_specification:
            function_keyword:
                token : current
                after : '+1'
            close_parenthesis:
                token : '-1'
                after : current

the indentation behaves much much better. I don't fully know what you are aiming for with your config, but if you are able to use the default config, I believe that you won't experience this problem. Otherwise, I suggest that you play around with your config more to see whether there's a way that you can use +1 rather than +2 for after the function keyword. I still have gaps in my knowledge on the operation of the indentation rules, but I can try to help if needed.

[obruendl]

Hi @JHertz5

The thing I want to achieve with the current configuration is that the function arguments are indented one level more than the variables and the like in the declarative part of the function, so they ar evisually set apart.

function howItShouldBe (
        someArg : integer);
    variable someVar : integer := 0;
begin
...

function howItShouldNotBe(
    someArg : integer);
    variable someVar : integer := 0;
begin
...

Do you have a suggestion how I can achieve this properly without relying on the closing parenthesis? I could fix this if something like "open_parenthesis" would be available but it is not.

[JHertz5]

Hi @obruendl.

I could fix this if something like "open_parenthesis" would be available but it is not.

Fortunately, that is easily resolved! The indent options, and their default configuration, are defined in the following file within VSG: vsg/vhdlFile/indent/indent_config.yaml. Within this file, we can add open_parenthesis (with some settings that do not modify any indentation by default), which opens it up to be modified by our custom config.

...
        function_specification:
            pure_keyword:
                token : current
                after : current
            impure_keyword:
                token : current
                after : current
            function_keyword:
                token : current
                after : '+1'
+           open_parenthesis:
+               token: current
+               after: current
            close_parenthesis:
                token : '-1'
                after : current
...

Now I can create a test config file as follows:

indent:
    tokens:
        function_specification:
            function_keyword:
                token: current
                after: "+1"
            open_parenthesis:
                token: "+1"
                after: "+1"
            close_parenthesis:
                token: "-1"
                after: "-1"

and it will produce the following output, which should match your indent requirements by the sounds of things.

package olo_base_pkg_math is

  function log2 return natural;

end package olo_base_pkg_math;

package body olo_base_pkg_math is

  function log2 return natural is
  begin

    return 3;

  end function log2;

  function howitshouldbe (
      someArg : integer
    ) return val is

    variable somevar : integer;

  begin

  end function howitshouldbe;

end package body olo_base_pkg_math;

When you have a chance, please give this a try and let me know whether it solves your problem. If so, I can raise a PR to add the open_parenthesis option to the main branch. If there are any other constructs for which a similar change is needed (I expect that you'd like the same for procedures), let me know and I can do them all in one go.

[obruendl]

@JHertz5

Cool. Is it possible to load a custom indent_config.yaml (or just modifications to the default one) instead of modifying the one in the installation location?

My point here is that I can't tell all people using Open Logic and contributing to it to manually modify the files installed with VSG. I mean I can - but this does not really sound like a good solution.

[JHertz5]

@obruendl

Cool. Is it possible to load a custom indent_config.yaml (or just modifications to the default one) instead of modifying the one in the installation location?

My point here is that I can't tell all people using Open Logic and contributing to it to manually modify the files installed with VSG. I mean I can - but this does not really sound like a good solution.

As I mentioned in my previous comment, if the indent_config.yaml modification solves your problem, I will raise a PR to add the modification to the main branch. Then there is no need for users to make the modification themselves.

[JHertz5]

Hi @obruendl. I've raised a PR (#1307) to resolve this, on the assumption that you're happy with the solution that I've mentioned above. If you have a chance to check it, please let me know whether you are happy that the PR addresses your problem.

[obruendl]

That's perfect. The PR resolves the problem.

[JHertz5]

Excellent, thanks very much for confirming!

[jeremiah-c-leary]

Evening @obruendl and @JHertz5 ,

At first I was wondering why we needed a pull request for this, but then I tried it to add indent myself and got an error message. I forgot a feature was added to check for undefined indents.

I will merge this PR to master.

--Jeremy

[JHertz5]

Thanks very much!