Skip to content

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