Remove unused variables potential dangerous behavior

[xczar0]

Delete unused variables do not take into consideration potential use of dynamically assigned variables.
Code will not analyze that scenario below (simpliest one)
DATA: lv_unused_b TYPE string.
assign ('lv_unused_b') to field-symbol(<test>).
In my opinion it should detect potential dynamic usage of variable and instead of deletion of variable it should add TODO.
I mean - it adds TODO, but for completely different reason - assigned, but not used variable

[fabianlupa]

Not sure about how the detection would reliably work? I am now also wondering how it works in ADT (and ATC).

DATA: lv_unused_b TYPE string.
DATA(lv_target) = 'LV_UNUSED_' && sy-abcde+1(1).
assign (lv_target) to field-symbol(<test>).

Also includes, enhancements and probably many more.

Edit: Todo instead of deletion seems better to me, then it doesn't have to be perfect.

[xczar0]

Hi!
I think this detection mechanism should look into ASSIGN ('varname') statement or ASSIGN (directVariable), if exists and check through local coding for already existing varname somewhere else.
If it was declared in local coding - then stop it from doing fancy stuff with that particular name.
Distinction between regular ASSIGN and dynamic assign is dynamic one contains () part, where in between you have variable name.
If you assign strings you simply do `assign '' to .
I Agree - doesn't have to be perfect, but on the other hand rule of refactoring is to not break, but improve :)
And also - i know dynamic coding is more problematic thing, because of course, it's dynamic.
If this cannot be fixed, maybe it should be mentioned in some "Potential Issues" document or in the check itself?

[jmgrassau]

Hi xczar0,

thanks for this finding! I agree, it's better to miss a few cases (simply keeping them as they are) than to break things, and as Fabian nicely pointed out, it would be impossible to reliably detect which variable is the target of the dynamic ASSIGN.

Therefore, I think it is best to simply deactivate the affected rules ("Report unused parameters", "Delete unused variables", "Use FINAL for immutable variables") for methods that contain such a dynamic ASSIGN. Probably (and hopefully!) this syntax is anyway not used very often… Will be fixed with the next release!

Kind regards,
Jörg-Michael

[jmgrassau]

Hi xczar0,

thanks again for pointing this out – version 1.13.1, which was just released, now blocks methods for "Delete unused variables" etc. if a dynamic ASSIGN is found (see details above)!

Kind regards,
Jörg-Michael