Fix other Go Report Card warnings #423
Conversation
Add field names to struct initializers, mostly these are our private key structs.
This is a neat tool that mostly catches unhandled errors (assignments to err variables that are never read). Add appropriate error handling to deal with these warnings. Pay attention to the subtle difference between = and := assignments.
Some functions already had it too high, some got it too high after additional error handling. We can't do anything about it other than split the big functions into smaller ones and introduce helpers to avoid too many conditionals in a single function.
|
|
||
| clientSession, endpoint := createSecureSession(inputBuffer) | ||
| if clientSession == nil { | ||
| return |
There was a problem hiding this comment.
maybe we should add os.Exit(1) and some message that will explain why nothing was done?
There was a problem hiding this comment.
We print an error message earlier, before returning nil here. I guess it will be enough to just exit with non-zero exit code to communicate a failure to the user: they will see the message and their terminal is likely to display a failure.
I'll add proper exit code here and in the Secure Message server example.
There was a problem hiding this comment.
@Lagovas done.
I've improved error forwarding a bit, moved the error message printing to main() and added exiting with non-zero status code. I've also done the same to Secure Message example for the sake of consistency.
| @@ -149,30 +149,34 @@ func New(key []byte, mode int) *SecureCell { | |||
| return &SecureCell{key, mode} | |||
| } | |||
|
|
|||
| func missing(data []byte) bool { | |||
| return data == nil || len(data) == 0 | |||
- Split the code in Secure Message example in the same way we do for Secure Session. - Improve the quit message a but. Without quotes it's not intuitive that you have to literally type in q-u-i-t to quit cleanly. - Unify error reporting in the code and output messages to stderr. - Exit with non-zero status code to indicate failure.
| fmt.Println("Type your settings from https://themis.cossacklabs.com/interactive-simulator/setup/") | ||
|
|
||
| fmt.Println("JSON endpoint: ") | ||
| endpoint, err := inputBuffer.ReadString('\n') | ||
| if err != nil { | ||
| fmt.Println("Failed to read endpoint URL:", err) | ||
| return nil, "" | ||
| err = fmt.Errorf("Failed to read endpoint URL:", err) |
There was a problem hiding this comment.
you use fmt.Errorf and don't use any pattern for formatting: https://play.golang.org/p/3z5WE3eymGs . Use URL: %s', err)
There was a problem hiding this comment.
I have to agree, it's pretty dumb of me to introduce new go vet warnings after getting rid of them :) Nice catch, thank you.
Make sure that errors are printed out nicely. Ironically, previous format strings introduced _new_ govet errors which we recently got rid of :)
Fix other warnings that we might run into on Go Report Card.
go vet: fix warnings about unkeyed field initializationAdd field names to struct initializers, mostly these are our private key structs.
go ineffassign: fix ineffective assignmentsThis is a neat tool that mostly catches unhandled errors (assignments to
errvariables that are never read). Add appropriate error handling to deal with these warnings. Pay attention to the subtle difference between=and:=assignments.go cyclo: reduce cyclomatic complexitySome functions already had it too high, some got it too high after additional error handling. We can't do anything about it other than split the big functions into smaller ones and introduce helpers to avoid too many conditionals in a single function.