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

Feature Request: Remove unneeded CLEAR on local variable at the end of a method #111

Closed
StefanRutzmoser opened this issue Sep 25, 2023 · 6 comments
Assignees

Comments

@StefanRutzmoser
Copy link

Hi,
I'd like to request an action to remove unnecessary CLEAR statements right before the end of a method if it's done on a locally defined variable. There is no reason why you should clear a variable that will reach it's "end of life" with the end of the method.

Example:

METHOD example.
  DATA lv_string type string.
  " [...]
  CLEAR lv_string.
ENDMETHOD.

Expected result:

METHOD example.
  DATA lv_string type string.
  " [...]
ENDMETHOD.
@StefanRutzmoser StefanRutzmoser changed the title Remove unneeded CLEAR on local variable at the end of a method Feature Request: Remove unneeded CLEAR on local variable at the end of a method Sep 25, 2023
@ConjuringCoffee
Copy link
Contributor

This would probably go along well with: #61

@jmgrassau
Copy link
Member

Hi Stefan and ConjuringCoffee,

thanks for the idea, I completely agree! Both unnecessary CLEAR of local variables at the beginning and at the end of a method will together make a very nice cleanup rule!

The only problem I'd see is that someone might be in the process of writing a method, so they could write the CLEAR statement and intend to add another block where this variable must be initial again. If they now press Ctrl+4 in between, the CLEAR would go away. On the other hand, in such a scenario, they would most likely be looking at this exact piece of code – it's the same for removing unused variables if you press Ctrl+4 after writing the DATA definition without using the variable already.

Kind regards,
Jörg-Michael

@jmgrassau
Copy link
Member

P.S.: What if CLEAR is not exactly at the end of the method, but still after the variable is last used (and outside of loops of course)? I guess in alignment with what has been discussed in #61 already, it should not be removed.

@ConjuringCoffee
Copy link
Contributor

The only problem I'd see is that someone might be in the process of writing a method, so they could write the CLEAR statement and intend to add another block where this variable must be initial again. If they now press Ctrl+4 in between, the CLEAR would go away. On the other hand, in such a scenario, they would most likely be looking at this exact piece of code – it's the same for removing unused variables if you press Ctrl+4 after writing the DATA definition without using the variable already.

For unused variables, I prefer using the option to add a TODO comment instead of removing the variable for exactly this reason. Maybe this could be an option for this new rule too?

@jmgrassau jmgrassau self-assigned this Oct 25, 2023
jmgrassau added a commit to jmgrassau/abap-cleaner that referenced this issue Oct 29, 2023
@jmgrassau
Copy link
Member

Hi Stefan and ConjuringCoffee,

thanks again for this inspiration! Version 1.9.0 was just released with the new cleanup rule "Remove needless CLEAR", which removes CLEAR of local variables from both the beginning (after declarations and asserts) and the end of a method. Default settings are as follows:

image

This will result in:

image

You can see in the comment in line 29-30 (right-hand side) why I thought different settings for method start and end make sense: Maybe someone just typed CLEAR lt_table at the end of the method and, out of a good habit, pressed Ctrl+4 before wanting to add more code to do something with lt_table! In such a case (after adding another executable line, so that CLEAR isn't at the end of the method anymore), ABAP cleaner would automatically remove the TODO comment again when Ctrl+4 is pressed the next time.

In the sample code I checked, I also found an example where I felt you wouldn't want to remove CLEAR, see lines 15–24 (right-hand side), hence the third option. And line 27 (right-hand side) shows why only "CLEAR any_identifier" is removed, but nothing that has a dereferencing operation, component selector, etc. to it. Hope I got everything right here!

Kind regards,
Jörg-Michael

@ConjuringCoffee
Copy link
Contributor

Very useful additions. I'll be using it with the default settings.

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

No branches or pull requests

3 participants