Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[BUG] wrong usage of named arguments across the code #1864

Closed
martin-mat opened this issue Jan 25, 2024 · 2 comments
Closed

[BUG] wrong usage of named arguments across the code #1864

martin-mat opened this issue Jan 25, 2024 · 2 comments
Assignees
Labels
bug Something isn't working

Comments

@martin-mat
Copy link
Collaborator

Describe the bug
On many occurrence across the cnf-testsuite code, passing of named arguments are done in a wrong way.

Example1:
https://github.com/cncf/cnf-testsuite/blob/main/src/tasks/platform/observability.cr#L112
task_response = CNFManager::Task.task_runner(args, check_cnf_installed=false) do |args|

Example2:
https://github.com/cncf/cnf-testsuite/blob/main/src/tasks/utils/cnf_manager.cr#L269
cnf_configs = self.cnf_config_list(silent=true)

The right way how to pass named arguments in crystal is using ':'
https://crystal-lang.org/reference/1.11/syntax_and_semantics/default_and_named_arguments.html
like this:
task_response = CNFManager::Task.task_runner(args, check_cnf_installed: false) do |args|
cnf_configs = self.cnf_config_list(silent: true)

How and why it is working currently:
'check_cnf_installed=false'
is evaluated as an assignent of new variable named "check_cnf_installed" to "false". Then the whole assignment returns "false" and passes it as a positional argument.
Most such cases work just accidentally, also due to crystal's loose assignement of positional and named arguments. However when some changes further changes are done, it can stop working.

To Reproduce

  1. Look across the code
  2. Shake your head

Expected behavior
Always use the right way how to pass positional arguments.

Screenshots

Device (please complete the following information):

How will this be tested? aka Acceptance Criteria (optional)
Best if an intelligent grep command is developed to identify all such wrong assignments. Fix it, re-run the grep, it should return no occurrences after fixing.

@martin-mat martin-mat added the bug Something isn't working label Jan 25, 2024
@macaktom
Copy link
Contributor

I can look into it.

@HashNuke
Copy link
Collaborator

HashNuke commented Mar 5, 2024

Completed via PR. Closing issue.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working
Projects
Status: Done
Development

No branches or pull requests

5 participants