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

Rule to remove commented out code #607

Open
imd1 opened this issue Jun 17, 2021 · 7 comments
Open

Rule to remove commented out code #607

imd1 opened this issue Jun 17, 2021 · 7 comments
Assignees

Comments

@imd1
Copy link
Contributor

imd1 commented Jun 17, 2021

Is your feature request related to a problem? Please describe.
Some people comment out code as they are changing it, and do not rely on source control to get back to the original. This leaves the code littered with "old" code as comments which looks very messy

Describe the solution you'd like
A rule that parses the comments to determine if there is valid VHDL code hidden inside, and remove the comments if so

Describe alternatives you've considered
Manual cleanup!

Additional context
Add any other context or screenshots about the feature request here.

@jeremiah-c-leary
Copy link
Owner

Hey @imd1 ,

This would be a killer feature if it could be implemented.

Do you have any thoughts on how we could differentiate between commented out code and comments?

@imd1
Copy link
Contributor Author

imd1 commented Jun 18, 2021

Hey @imd1 ,

This would be a killer feature if it could be implemented.

Do you have any thoughts on how we could differentiate between commented out code and comments?

remove the -- intelligently and check for parsable VHDL underneath?

@jeremiah-c-leary
Copy link
Owner

remove the -- intelligently and check for parsable VHDL underneath?

Using this as an example:

   if a = b then
  -- if a = '0' then
   -- Do something
      c <= 5;
   end if;

I could remove the second comment. So in this case it would look like:

   if a = b then
   if a = '0' then
   -- Do something
      c <= 5;
   end if;

which would fail to parse and could be flagged.

However, what about the second comment? If we remove the -- and attempt to parse:

   if a = b then
  -- if a = '0' then
    Do something
      c <= 5;
   end if;

That would clearly fail to parse, but it is not commented out code.

@jeremiah-c-leary
Copy link
Owner

Could we make the following assumptions about commented out code?

  1. Contains at least one VHDL keyword
  2. Contains at least one of the following: signal name, constant name, variable name, function name, etc...

So in my previous example, which I have expanded:

architecture rtl of fifo is

  constant b : integer;
  signal a : integer;
  signal c : integer;

begin

  process begin
  if a = b then
    -- if a = '0' then
     -- Do something
        c <= 5;
     end if;
  end process;
end architecture;

I could then follow these steps:

  1. Collect all the signal and constant names
  2. Check if comment includes a signal or constant name
  3. Check if comment includes a VHDL keyword

So in the example above, the first comment would be flagged because a exists and if exists.
The second comment would not be flagged because Do something is neither a signal/constant name nor a vhdl keyword.

I think this would only get you a certain class of commented out code.

@imd1
Copy link
Contributor Author

imd1 commented Sep 23, 2021

in your example, the if a = '0' then is valid VHDL even though it is incomplete, so I'd want this line removed...the -- Do something is clearly not.

So I would treat each comment line separately and somehow determine if it's valid VHDL or not

@jeremiah-c-leary
Copy link
Owner

So I would treat each comment line separately and somehow determine if it's valid VHDL or not

I was just thinking of the possible permutations for commented out code, and it seems endless. I doubt we can make this perfect, but if we could catch a certain percentage of commented out code it would be worth it.

Maybe there should be a series of commented out code rules. For example:

commented_out_001: checks for commented out library statements:

-- library ieee;

commented_out_002: check for commented out use statements:

library ieee;
-- use ieee.std_logic_1164.all;

or it is part of the library rules:

library_???: Checks for commented out library statements:

-- library ieee;

So instead of a single rule to check for commented out code, we have multiple rules.

@jeremiah-c-leary jeremiah-c-leary added this to Incomming in Issue Triage via automation Oct 29, 2021
@jeremiah-c-leary jeremiah-c-leary moved this from Incomming to User Feedback in Issue Triage Oct 29, 2021
@imd1
Copy link
Contributor Author

imd1 commented Dec 31, 2021

Separate rules makes sense...

The underlying principle should be whether each commented out line is valid VHDL syntax if the -- was removed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Deferred
Development

No branches or pull requests

2 participants