jeremiah-c-leary / vhdl-style-guide

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

Rules to enforce case on the `shared` keyword in a shared variable declaration #1289

Open urbite opened 1 month ago

urbite commented 1 month ago

Is your feature request related to a problem? Please describe. I'd like a rule to enforce case on the shared keyword in a shared variable declaration. For example, I'd like:

  SHARED variable done_sh : flag_pt;

corrected to:

  shared variable done_sh : flag_pt;

@JHertz5 Notification alert

JHertz5 commented 1 month ago

Hi @urbite.

Thanks for raising this issue. The shared keyword is recognised by the parser already, so this should be a very simple rule to implement.

You mentioned at one point that you might like to make some contributions to VSG. Would you like me to give you some pointers on how you can raise a PR yourself to resolve this issue? No problem if not, I'll be able to raise a PR for this quite quickly.

Jukka

urbite commented 1 month ago

I would definitely be interested in trying to add some of these missing keywords to VSG. So some pointers on raising the PR and fixing it myself would be welcome.

I assume that many (or most) of these keyword case issues would be fixed in a similar manner. Then, some of your earlier fixes could be used as a template, in the same manner as raising an issue.

I read the docs on adding a rule and it doesn't seem too difficult. Especially if I can use your earlier case enhancements as examples.

Regarding the PRs and related flow, I commented on that here

JHertz5 commented 1 month ago

@urbite Excellent, I'd be glad to help with that.

That's right, many of the rules are extremely similar, so adding a rule that closely resembles another is not challenging. Similar rules are created as objects that inherit their functionality from a common "base rule", so when creating a rule, you can usually find another similar rule and use that as a template. In this case, variable_002 is a rule that enforces the case of the variable keyword, so this is an ideal rule to use as our template.

Creating the rule

The rule is at the path vsg/rules/variable/rule_002.py and if you open it up, you'll see the following

# -*- coding: utf-8 -*-

from vsg import token
from vsg.rules import token_case

lTokens = []
lTokens.append(token.variable_declaration.variable_keyword)

class rule_002(token_case):
    """
    This rule checks the **variable** keyword has proper case.

    |configuring_uppercase_and_lowercase_rules_link|

    **Violation**

    .. code-block:: vhdl

       VARIABLE count : integer;

    **Fix**

    .. code-block:: vhdl

       variable count : integer;
    """

    def __init__(self):
        super().__init__(lTokens)
        self.groups.append("case::keyword")

There are a few things to observe here:

The next step is to figure out what the name of the rule needs to be. There is documentation about the rule naming process here. In this case, it's quite straightforward. This will be another variable rule, so the new rule should be created in the same folder as our template rule. Case rule run in phase 6 (you can tell by running ./bin/vsg -rc variable_002 and observing the "phase" setting in the output), so the first digit in the number should be 5, and there are no other variable_5XX rules, so our new rule should be variable_500.

So with this as our template, we can create a copy of variable/rule_002.py called rule_500.py and update the contents accordingly. If you're not sure what the token should be, you can create a test VHDL file and run the tool bin/vsg_parser you should see something like this in the output

--------------------------------------------------------------------------------
3 |   shared variable done_sh : flag_pt;
<class 'vsg.token.variable_declaration.shared_keyword'>
<class 'vsg.token.variable_declaration.variable_keyword'>
<class 'vsg.token.variable_declaration.identifier'>
<class 'vsg.token.variable_declaration.colon'>
<class 'vsg.token.type_mark.name'>
<class 'vsg.token.variable_declaration.semicolon'>
--------------------------------------------------------------------------------

which tells us that our keyword uses the token object variable_declaration.shared_keyword. Important Once you have created the rule file, you will need to add it to the init.py file in the same directory. This is required for VSG to be able to access your new rule. Hopefully it'll be clear what I mean once you look at the init file.

Testing

You should again be able to copy and tweak the test files for variable_002, (tests/variable/test_rule_002.py). Hopefully, this step is straightforward to figure out. I don't know what code editor you're using, but VS code has integration for the Python testing framework which lets you run the tests easily from within the editor image

Documentation

To generate the documentation, you can use another helpful tool by running bin/vsg_rule_doc_gen -p docs. This uses the docstring we saw earlier to update the variable rule documentation. You'll also need to manually add the new rule's name to docs/rule_groups/case_keyword_rule_group.rst, docs/rule_groups/case_rule_group.rst, and docs/configuring_uppercase_and_lowercase_rules.rst. One of the unittests, rule_doc will fail to alert you if there are any problems.

PR

Once this is done, raise your PR and check that it passes the CI. Since this is your first PR, I think the repo owner needs to approve you or add you as a contributor or something before the full CI runs on your pull request, but you can run the checks locally to ensure that it will pass. Once the PR is ready, it is up to the repo owner to approve and merge.

Hopefully, this makes sense. If anything is not clear, feel free to ask.

urbite commented 1 month ago

@JHertz5 Excellent overview and instructions for adding a feature and creating a PR!

I have forked the project, but it may be a few days before I can add the case rule for shared due to other priorities.