jeremiah-c-leary / vhdl-style-guide

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

generic_map prefix (601) versus parentheses #1057

Closed SittingDuc closed 7 months ago

SittingDuc commented 7 months ago

Environment Ubuntu Linux 22.04 and VHDL Style Guide 3.18.0. Also seen with the issue_1053 branch

Describe the bug if generic_map_601 is enabled to require generics / generic_maps to have a prefix, and a given generic_map assigns only some bits of a vector, then multiple generic_map_601 violations are reported

To Reproduce Steps to reproduce the behavior:

  1. in config, enable generic_map_601
  2. attempt to parse code similar to shown
  3. observe three generic_map_601 violations on the highlighted line
    dut : entity work.ampersand
    generic map (
      g_FIRST     => c_FIRST,
      g_SECOND(0) => c_SECOND -- observe three violations here
    )
    port map (
      i_thing    => inthing,
      o_thing(0) => outthing
    );

output:

----------------------------+------------+------------+--------------------------------------
  Rule                      |  severity  |  line(s)   | Solution
----------------------------+------------+------------+--------------------------------------
  generic_map_601           | Error      |         19 | Prefix ( with one of the following: g_
  generic_map_601           | Error      |         19 | Prefix 0 with one of the following: g_
  generic_map_601           | Error      |         19 | Prefix ) with one of the following: g_
----------------------------+------------+------------+--------------------------------------

Expected behavior No violations on line 19

Comments Great Project, having fun trying to convince coworkers to move to it (hey, the benefits are obvious, guys)

SittingDuc commented 7 months ago

Because every bug is better with a testcase. Hey, I can attach json

vsg -c demo_1057.json -f generic_map_1057.vhd

demo_1057.json generic_map_1057.vhd.txt

jeremiah-c-leary commented 7 months ago

Good Afternoon @SittingDuc ,

I pushed an update for this to the issue-1057 branch. When you get a chance could you check it on your end.

Great Project, having fun trying to convince coworkers to move to it (hey, the benefits are obvious, guys)

There are groups in my company which are not interested. I can't figure out why. I agree though, the benefits are obvious. Especially during code reviews.

Regards,

--Jeremy

SittingDuc commented 7 months ago

On Topic: I have built branch-1057 and run it on my 'real' VHDL and the generic_map_601 violations are gone :+1: so I consider the update resolves the original bug. Many thanks and good luck with the linter work.

Wandering Off Topic: I had a theory that on matters of opinion (tab vs space, emacs vs vi, constants in uppercase or lower) where there is no clear winner (so not vi vs emacs :rofl:), the longer someone has been doing things "this way", the less interested they are in changing. Looking over my notes so far, a coworker has commented that the Linter can "come across as quite strict / inflexible", which I can see as offputting. And I have some files that vscode just paints orange as large sections of the code don't follow neither the new linter nor the paper "style guide" we give to new hires. But emacs doesn't complain, so it never got fixed... (the correct solution is obviously to migrate code we own to the output of the linter as being 'most correct', but that will take time too) So, one theory is Institutional Inertia, powered by grey-beards who have always done it this way and it worked so far. Style Guides are Aspirational, not Prescriptive. Etc.

(Great irony, I am the Grey-beard in our VHDL team and I am the one pushing for change..)

Thanks again,

jeremiah-c-leary commented 7 months ago

Evening @SittingDuc ,

On Topic: I have built branch-1057 and run it on my 'real' VHDL and the generic_map_601 violations are gone 👍 so I consider the update resolves the original bug. Many thanks and good luck with the linter work.

Sweet, I will get it merged to master. I am planning to do a release soon.

where there is no clear winner (so not vi vs emacs 🤣)

There is only one choice in the vi vs emacs debate. 😁

Looking over my notes so far, a coworker has commented that the Linter can "come across as quite strict / inflexible", which I can see as offputting.

The purpose of a linter is to be strict and inflexible. It's purpose is to enforce standards into the code and look for problems. If there is a published style guide given to new hires then there are standards. If a linter exists that can check the standards, then why not use one? One of the benefits of a linter is a tool is complaining there are problems with your code instead of a coworker.

So, one theory is Institutional Inertia, powered by grey-beards who have always done it this way and it worked so far.

Yeah, I have heard this before. There is a group I suggested they use VSG. I was told the people are set in the their ways and would not want to change.

I would like to think there are two value propositions for using VSG: 1) What is gained by having consistently formatted code? 2) Can we make it cheap to have consistently formatted code?

What is gained by having consistently formatted code?

I started VSG years ago after participating in a code review. I missed an error in a file that looked something like:

  elsif (a = '0' or a = '0')

I reviewed the file and put in findings for all the formatting issues. For some reason, that I still do not understand, I looked at the file the next day and found the above line. That is when I realized that inconsistent formatting was causing mental fatigue and prevented me from seeing the real problems. I suspect there were so many style issues that I ended up focusing on those instead of checking the code was correct. If I missed something as simple as the above example, what about other code issues which are harder to find?

Can we make it cheap to have consistently formatted code?

When I first started writing VSG it just flagged problems. Then someone at work suggested having VSG actually fix the problems. That was a genius idea, and so the --fix option was born. If you can get VSG configured to what you want, then it is trivial to enforce a coding style. You can easily convert entire code bases.

I want to believe VSG sells itself. Now I code lazily, not caring about indents or spacing etc... I just get the idea down and when I am thinking of the next thing to implement I run VSG on the code. It frees me from the formatting tedium, which I feel is a win. You do have to get used to the code not being in a "good" state for a short time though.

(Great irony, I am the Grey-beard in our VHDL team and I am the one pushing for change..)

Not all of us grey beards are content with our current tools or processes.

You should check out my ELFWS project. I am of the opinion that warnings are errors and ran into vendor tools which did not allow for suppression of warnings. So I wrote something that provides a consistent method for suppressing warnings regardless of the vendor.

Regards,

--Jeremy