Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
urbite opened this issue Oct 22, 2024 · 5 comments
Open
Assignees

Comments

@urbite
Copy link

urbite commented Oct 22, 2024

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
Copy link
Contributor

JHertz5 commented Oct 22, 2024

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
Copy link
Author

urbite commented Oct 22, 2024

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
Copy link
Contributor

JHertz5 commented Oct 22, 2024

@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 rule imports token_case - this is the base rule that I mentioned earlier. Note that our new rule is created as a class called rule_002 which uses token_case as a parent class. We'll need to change the name of the class in our new rule to reflect the new rule number.
  • The token variable_declaration.variable_keyword is appended to lTokens. This is what causes this rule to act on that particular token, so we'll need to change that token in our new rule.
  • The docstring of the class rule_002 contains some documentation about what the rule does and gives an example of the rule's behaviour. This will need to be updated in our new rule.

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
Copy link
Author

urbite commented Oct 23, 2024

@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.

@JHertz5
Copy link
Contributor

JHertz5 commented Jan 19, 2025

Hi @urbite

It's been a few months, just wanted to check in. Do you see yourself ever doing this? No problem if not, just let us know and I can implement it instead.

Thanks,
Jukka

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In Progress
Development

No branches or pull requests

2 participants