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

Extend impure_functions list by socket functions #8835

Conversation

alies-dev
Copy link
Contributor

This prevents UnusedFunctionCall for some socket ext functions.

Result of all of these functions is boolean and can be ignored.

Note, socket errors can be fetched by:

  • socket_strerror
  • socket_last_error

Closes #8818

all of them returns boolean and can be ignored.
This prevents UnusedFunctionCall.
Note, socket errors can be fetched by:
 - socket_strerror
 - socket_last_error
@weirdan weirdan added the release:fix The PR will be included in 'Fixes' section of the release notes label Dec 4, 2022
@orklah orklah merged commit d2f7d86 into vimeo:master Dec 4, 2022
@orklah
Copy link
Collaborator

orklah commented Dec 4, 2022

Thanks!

@danog
Copy link
Collaborator

danog commented Dec 5, 2022

Hmm this does not feel like a good idea, generally you should always check if a network operation failed or not, are we sure this is a good idea?

@alies-dev alies-dev deleted the 8818-extend-impure_functions-by-socket-functions branch December 5, 2022 00:33
@alies-dev
Copy link
Contributor Author

@danog
This is opinionated, agree.

In this PR I simply continues on what we already do here. Is it good a bad - it's a separate topic.
IMO ideally it should throw a new type of error, e.g. UnusedOptionalFunctionCallResult, so a user can easily suppress any such error.

@danog
Copy link
Collaborator

danog commented Dec 5, 2022

I see now that a bunch of other socket functions were already included in the list, and that the list is actually indicating impure functions, so this PR actually makes sense, as it is (correctly) indicating that these functions have side effects.

However, ignoring the result of at least the write, read, connect and bind functions is still an antipattern, it might be worth introducing an UnusedImpureFunctionCallResult issue for this case, yes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:fix The PR will be included in 'Fixes' section of the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UnusedFunctionCall on socket* functions
4 participants