Linting #359
Merged
Linting #359
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The linter checks if package imports are in a list of acceptable packages. The linter requires config file `.depguard.yml`. We don't need this at this moment.
The linter 'varcheck' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter. Replaced by unused. The linter 'structcheck' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter. Replaced by unused. The linter 'deadcode' is deprecated (since v1.49.0) due to: The owner seems to have abandoned the linter. Replaced by unused. The linter 'interfacer' is deprecated (since v1.38.0) due to: The repository of the linter has been archived by the owner. The linter 'scopelint' is deprecated (since v1.39.0) due to: The repository of the linter has been deprecated by the owner. Replaced by exportloopref. The linter 'golint' is deprecated (since v1.41.0) due to: The repository of the linter has been archived by the owner. Replaced by revive. The linter 'ifshort' is deprecated (since v1.48.0) due to: The repository of the linter has been deprecated by the owner.
Choraden
force-pushed
the
hg/linting
branch
3 times, most recently
from
7ad2fcd to
f615a6f
Compare
The quality of the linter is questionable. There are many false positives. For example: timakin/bodyclose#52
Choraden
force-pushed
the
hg/linting
branch
2 times, most recently
from
34a1488 to
2a6cb6d
Compare
The are many unchecked errors in the martian code. Most of are not dangerous e.g. come from messageview (printing http logs). Checking all these errors will increase the code complexity and decrease code readability.
Some of the issues need some discussion before the fix:
internal/martian/proxy.go: G402: TLS MinVersion too low. (gosec)
return &tls.Config{}
internal/martian/mitm/mitm.go: G505: Blocklisted import crypto/sha1: weak cryptographic primitive (gosec)
"crypto/sha1"
internal/martian/mitm/mitm.go: G401: Use of weak cryptographic primitive (gosec)
h := sha1.New()
Context key should have custom type to avoid collisions.
- increase number of func statements to 75 - increase allowed lines to 140
Function `func (a *adapter) Data(data []byte, streamEnded bool) error` from `martian/h2/grpc` has 49 complexity.
The linter wrongly interprets comments as code e.g. // GET http://example.com/ HTTP/1.1 // Host: example.com
Function `func (p *Proxy) handleConnectRequest` has nested if with complexity level 10 - `if p.mitm != nil {...}`.
- godot - misspell - dupword
After the golangci-lint version was bumped, the linter stopped working at the ci [1]. [1] golangci/golangci-lint-action#23
|
@mmatczuk I updated the linter and resolved its issues. I tried to split the work into specific commits in case we want to roll something back. If you feel the need to squash them somehow, let me know. Some of the linters were disabled as they need some discussion before fix, I believe. |
|
LGTM |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #340
Fixes #353