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

Some small fixes for staticcheck #1565

Merged
merged 3 commits into from
Jan 30, 2024

Conversation

RebeccaMahany
Copy link
Contributor

Addresses some, but not all, occurrences of staticcheck violations in the following categories:

Code simplifications:

  • S1000 - Use plain channel send or receive instead of single-case select
  • S1005 - Drop unnecessary use of the blank identifier
  • S1023 - Omit redundant control flow
  • S1030 - Use bytes.Buffer.String or bytes.Buffer.Bytes
  • S1038 - Unnecessarily complex way of printing formatted string
  • S1039 - Unnecessary use of fmt.Sprint

Misuse of standard library:

  • SA1016 - Trapping a signal that cannot be trapped
  • SA1019 - Using a deprecated function, variable, constant or field

Dubious code constructs that have a high probability of being wrong:

  • SA9004 - Only the first constant has an explicit type

Stylistic issues:

  • ST1005 - Incorrectly formatted error string

@@ -59,7 +59,7 @@ func runSocket(args []string) error {

// Wait forever
sig := make(chan os.Signal, 1)
signal.Notify(sig, os.Interrupt, os.Kill, syscall.Signal(15))
signal.Notify(sig, os.Interrupt, syscall.SIGTERM, syscall.Signal(15))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

os.Kill cannot be trapped (did you mean syscall.SIGTERM?)

@@ -118,7 +118,7 @@ func WithDebugLogging() FlattenOpts {
// re-writing arrays into maps, and for filtering. See "Query
// Specification" for docs.
func WithQuery(q []string) FlattenOpts {
if q == nil || len(q) == 0 || (len(q) == 1 && q[0] == "") {
if len(q) == 0 || (len(q) == 1 && q[0] == "") {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should omit nil check; len() for []string is defined as zero

"tuf-server.example.com": resolution{ips: []string{"34.149.84.181"}, err: nil},
"google.com": resolution{ips: []string{"2620:149:af0::10", "17.253.144.10"}, err: nil},
"apple.com": resolution{ips: []string{"2607:f8b0:4009:808::200e", "142.250.65.206"}, err: nil},
"kolide-server.example.com": {ips: []string{"2607:f8b0:4009:808::200e", "142.250.65.206"}, err: nil},
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

simplifycompositelit

@@ -20,8 +19,6 @@ import (
func TestTufReleaseVersionTable(t *testing.T) {
t.Parallel()

rand.Seed(time.Now().UnixNano())
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rand.Seed has been deprecated since Go 1.20 and an alternative has been available since Go 1.0: As of Go 1.20 there is no reason to call Seed with a random value. Programs that call Seed with a known value to get a specific sequence of results should use New(NewSource(seed)) to obtain a local random generator.

ErrorControlNormal = "normal"
ErrorControlCritical = "critical"
ErrorControlNormal ErrorControlType = "normal"
ErrorControlCritical ErrorControlType = "critical"
)

type StartType string
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't make this same fix for the StartType constants below because it causes a compilation error (!!) and I didn't want to make too many changes in this PR

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reasonable! As long as it's not nolint we can fix it in a followup

@RebeccaMahany RebeccaMahany marked this pull request as ready for review January 30, 2024 16:08
James-Pickett
James-Pickett previously approved these changes Jan 30, 2024
Copy link
Contributor

@James-Pickett James-Pickett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this!

Copy link
Contributor

@zackattack01 zackattack01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔥 thank you!

@RebeccaMahany RebeccaMahany added this pull request to the merge queue Jan 30, 2024
Merged via the queue into kolide:main with commit fd73a70 Jan 30, 2024
26 checks passed
@RebeccaMahany RebeccaMahany deleted the becca/staticcheck-fixes branch January 30, 2024 17:10
Copy link
Contributor

@directionless directionless left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yay

ErrorControlNormal = "normal"
ErrorControlCritical = "critical"
ErrorControlNormal ErrorControlType = "normal"
ErrorControlCritical ErrorControlType = "critical"
)

type StartType string
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reasonable! As long as it's not nolint we can fix it in a followup

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants