I looked into this a bit and wastedassign feels like it is doing exactly what ineffassign does, except that it errs on the side of false positives instead of false negatives. The only failures in fiber today triggered by wastedassign are zero-initialization (eg foo := "" instead of var foo string)
So I might put my vote on disabling wastedassign. Dont need to decide that now though.

I'd prefer consistency here. For example, we use both var foo string and foo := "" in this codebase.

I believe wsl directly conflicts with gofumpt in some areas, so it is impossible to enable both with default configuration. We can probably add config to make them agree though.

does the above TODO also apply to these

No, it's an inline-comment, so it only applies that that one specified line.

Do you know which cases are not handled by errcheck?

I don't know off-head right now and don't have access to a Go system at the moment. Simply try it yourself and drop the line :D

In the current codebase it doesn't catch anything that other linters don't catch, but I'd like to see some cases that it does catch which the others dont.
Actually, I just remembered another reason why I disabled it - golangci-lint has a bug that prevents you from setting global ignores for this rule golangci/golangci-lint#3749
So until that get's fixed, you'd have to manually add ignores everywhere, which adds a lot of noise to the code.

is this relevant for us?

I think so, see their examples at timonwong/loggercheck@ef578c4#diff-dbd79b07e7f11adb8139d961bff8a7df5f2e3f331e92e9d13d8f34ff84595cdeR1

Ignoring the error returned by sess.byteBuffer.Write(raw) is risky, even if it's assumed that this operation will never fail. It's generally a good practice to handle or at least log errors to catch unexpected scenarios, especially in I/O operations. Can you please verify this assumption in the broader context of how sess.byteBuffer is used throughout the application? Additionally, considering the use of a mutex around this operation, ensure that the concurrency model doesn't introduce edge cases where the write operation might fail.

The depguard linter settings have been refined, including specifying packages to deny with descriptive reasons. This is a good practice for maintaining code quality and preventing the use of deprecated or undesirable packages. However, there are commented-out entries for the reflect and unsafe packages with a TODO note suggesting a need for justification when using these packages. It's important to decide whether to enforce these rules or not, as leaving commented-out code can lead to confusion.

Consider finalizing the decision on the reflect and unsafe packages and either enforce the rules or remove the commented-out code to avoid confusion.

The forbidigo linter settings include forbidding certain patterns like print, fmt.Print, and http.Default* with a TODO note to eventually enable more patterns. This approach helps maintain code quality by discouraging the use of certain functions. However, the commented-out patterns for panic and time.Sleep suggest an intention to further refine the rules, which should be addressed to avoid leaving unfinished configurations.

Finalize the decision on forbidding additional patterns like panic and time.Sleep to ensure the linter configuration is complete and reflects the project's coding standards.

The gci linter settings define a custom order for import sections, which is a good practice for maintaining consistency across the codebase. However, the alias section is commented out, suggesting uncertainty about its inclusion. It's important to clarify the intended import order and ensure it's consistently applied across the project.

Decide on the inclusion of the alias section in the import order and update the configuration accordingly to avoid ambiguity and ensure consistent import formatting.

The gocritic linter has several TODO comments related to enabling tags and disabling checks. While it's beneficial to customize the linter for specific needs, leaving TODO comments without addressing them can lead to confusion and an incomplete configuration.

Address the TODO comments in the gocritic linter settings by finalizing the decisions on enabled tags and disabled checks to ensure the configuration is clear and complete.

The gosec linter excludes the G104 check with a TODO note to enable it again. This exclusion might overlook unchecked errors, which can lead to potential security issues. It's crucial to assess the impact of this exclusion and consider enabling the G104 check to ensure comprehensive security analysis.

Re-evaluate the decision to exclude the G104 check from the gosec linter and consider enabling it to ensure thorough security analysis and maintain high code quality.

The linters section has been extensively modified, including enabling and disabling various linters. Some linters are marked as deprecated in favor of others, and there are several TODO comments suggesting further actions. It's essential to finalize decisions on deprecated linters and address TODO comments to ensure the linter configuration is up-to-date and reflects the project's standards.

Finalize decisions on deprecated linters and address TODO comments in the linters section to ensure the configuration is clear, complete, and aligned with best practices.

The run section has been updated with new settings including timeout, allow-serial-runners, and a modification to skip-dirs. The TODO comment on line 9 suggests an intention to revisit the decision to ignore the internal directory. It's important to ensure that this TODO is tracked and addressed to avoid unintentionally skipping important directories in the linting process.

Consider creating a tracking issue or plan to address the TODO regarding the internal directory to ensure it does not get overlooked.

MinVersion is missing from this TLS configuration. By default, TLS 1.2 is currently used as the minimum when acting as a client, and TLS 1.0 when acting as a server. General purpose web applications should default to TLS 1.3 with all other protocols disabled. Only where it is known that a web server must support legacy clients with unsupported and insecure browsers (such as Internet Explorer 10), it may be necessary to enable TLS 1.0 to provide support. Add MinVersion: tls.VersionTLS13 to the TLS configuration to bump the minimum version to TLS 1.3.

- config := &tls.Config{Certificates: []tls.Certificate{cer}}
+ config := &tls.Config{Certificates: []tls.Certificate{cer}, MinVersion: tls.VersionTLS13}

Detected a network listener listening on 0.0.0.0 or an empty string. This could unexpectedly expose the server publicly as it binds to all available interfaces. Instead, specify another IP address that is not 0.0.0.0 nor the empty string.

Consider specifying a more restrictive IP address to bind the server to, enhancing security by limiting the network interfaces the server is exposed on.

The addition of a comment regarding the use of the log package suggests the need to capture output and hints at a future change to use an internal log package instead. This is a good practice to ensure that logging is consistent and centralized within the project. Tracking this TODO for future implementation is recommended.

Would you like me to help with implementing the internal log package or create a tracking issue for this task?