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

Add: Add Plugin for variable redefinition in foreach loop #704

Merged
merged 4 commits into from
Jun 11, 2024

Conversation

ghost
Copy link

@ghost ghost commented May 10, 2024

What

This pr adds a plugin to check if a variable is redefined by using the same variable name for the list and the element being iterated over.

foreach foo(foo){}

Why

Using the foreach for redefinition is valid code, but is more likely a typo than intended for normal VT development.
The foreach loop will also not throw an error if the list variable does not exist and is initialised as an empty list instead.
Should be reported:

urls = make_list( "/foo", "/bar" );
foreach url( url )

-> Typo in the second "url" variable

url1 = make_list( "foo", "foo2" );
url2 = make_list( "bar", "bar2" );
foreach url(make_list(url1 , url))

-> Probably a typo in the second "url"

url1 = make_list( "foo", "foo2" );
url2 = make_list( "bar", "bar2" );
foreach url(make_list_unique(url1 , url))

-> Probably a typo in the second "url"

References

Closes: Jira VTOPS-178

Checklist

nhargarter@gb-ho-10 ~/g/troubadix (variable_redefinition_in_foreach)> poetry run troubadix --include-tests check_variable_redefinition_in_foreach -d ~/gb/vulnerability-tests/nasl
ℹ Start linting 165532 files ... 
ℹ Time elapsed: 0:00:11.732734
  Plugin                                             Errors Warnings
  -------------------------------------------------------------------
  -------------------------------------------------------------------
ℹ sum                                                     0        0
  • Tests

@ghost ghost self-requested a review as a code owner May 10, 2024 12:57
Copy link

github-actions bot commented May 10, 2024

Conventional Commits Report

Type Number
Changed 2
Added 1

🚀 Conventional commits found.

Copy link

codecov bot commented May 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.28%. Comparing base (f2d067f) to head (0b1bc5e).
Report is 1 commits behind head on main.

Current head 0b1bc5e differs from pull request most recent head 3111bd8

Please upload reports for the commit 3111bd8 to get more accurate results.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #704      +/-   ##
==========================================
+ Coverage   79.15%   79.28%   +0.13%     
==========================================
  Files          84       85       +1     
  Lines        2811     2829      +18     
  Branches      595      599       +4     
==========================================
+ Hits         2225     2243      +18     
  Misses        443      443              
  Partials      143      143              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ghost ghost force-pushed the variable_redefinition_in_foreach branch from 02129ce to 5aa17ac Compare May 23, 2024 09:36
@ghost ghost force-pushed the variable_redefinition_in_foreach branch from 5aa17ac to 567b91c Compare June 3, 2024 08:57
@mbrinkhoff mbrinkhoff enabled auto-merge (rebase) June 11, 2024 12:03
@mbrinkhoff mbrinkhoff force-pushed the variable_redefinition_in_foreach branch from 0b1bc5e to 3111bd8 Compare June 11, 2024 12:03
@mbrinkhoff mbrinkhoff added the make release To trigger GitHub release action label Jun 11, 2024
@mbrinkhoff mbrinkhoff merged commit b9dc369 into main Jun 11, 2024
10 checks passed
@mbrinkhoff mbrinkhoff deleted the variable_redefinition_in_foreach branch June 11, 2024 12:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
make release To trigger GitHub release action
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants