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

Question: function_parameter_count rule behavior #2171

Closed
2 tasks done
varunpm1 opened this issue Apr 25, 2018 · 12 comments
Closed
2 tasks done

Question: function_parameter_count rule behavior #2171

varunpm1 opened this issue Apr 25, 2018 · 12 comments
Labels
question Question or doubts that needs discussion and clarification. Can become a bug or proposal.

Comments

@varunpm1
Copy link

New Issue Checklist

Possible Bug

Swift Lint rule - function_parameter_count

func test(a: Int, b: Int, c: Int, d: Int, e: Int, f: Int = 0, g: Int = 0)

The total parameters count for the above function is 7 (i.e., 5 compulsory parameters and 2 optional parameters). So I would like to know if the rule function_parameter_count should throw a warning for this function or not. Currently, this rule doesn't throw a warning for such functions.

func test(a: Int, b: Int, c: Int, d: Int, e: Int, f: Int, g: Int)

However, making all parameters as compulsory throws warning as expected.

  • Which Xcode version are you using (check xcode-select -p)? - XCode 9.3

As per my understanding, this should throw a warning in both the scenarios since total parameter count exceeds the limit. Any help/suggestions appreciated here.

Thanks, Varun

@marcelofabri marcelofabri added the question Question or doubts that needs discussion and clarification. Can become a bug or proposal. label Apr 25, 2018
@marcelofabri
Copy link
Collaborator

That's the expected behavior: we currently don't count the default parameters. If you feel like this is important, this could be a configuration option and PRs would be welcome.

@varunpm1
Copy link
Author

Understood. I think this could be added as a configuration option. Anyway, this is just my suggestion.

Thanks, Varun

@marcelofabri
Copy link
Collaborator

@varunpm1 would you want to implement it and contribute back?

@varunpm1
Copy link
Author

varunpm1 commented May 9, 2018

Hi @marcelofabri

I could try but I'm not sure on the steps to contributing.

@jpsim
Copy link
Collaborator

jpsim commented May 9, 2018

@varunpm1 thanks for your interest in contributing! Please read our CONTRIBUTING.md document and let us know if you have any questions.

@varunpm1
Copy link
Author

varunpm1 commented May 9, 2018

A small concern here -

When I tried to go through the steps as per CONTRIBUTING.md, I couldn't run the following command -
$ make docker_test

I'm receiving the following error -
/bin/sh: docker: command not found

Any help here would be appreciated.

Also, I'm assuming that this has to be a separate rule, correct?

@jpsim
Copy link
Collaborator

jpsim commented May 9, 2018

Are you sure you have docker installed? Instructions to do so here: https://docs.docker.com/install/

@jpsim
Copy link
Collaborator

jpsim commented May 9, 2018

And no, to configure the function_parameter_count rule to not ignore default parameters, I'd recommend you add an ignores_default_parameters configuration option to the rule, which would default to true but that you could get the behavior you want by setting it to false.

@varunpm1
Copy link
Author

Got it. Thanks for the help. Trying to implement the configuration.

@varunpm1
Copy link
Author

I have sent a PR to master branch with the required configuration. I have also added a test case for function_parameter_count rule. Let me know if anything else is needed here.

@jpsim
Copy link
Collaborator

jpsim commented May 10, 2018

Thanks for taking the initiative to do this! Myself or another contributor will hopefully have time to review your PR shortly.

@varunpm1
Copy link
Author

Closing this issue as the PR has been merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Question or doubts that needs discussion and clarification. Can become a bug or proposal.
Projects
None yet
Development

No branches or pull requests

3 participants