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

chore: enable more linters #5228

Merged
merged 5 commits into from
Sep 26, 2023
Merged

Conversation

DmitriyLewen
Copy link
Contributor

@DmitriyLewen DmitriyLewen commented Sep 22, 2023

Description

Disable:

  • goimports

Enable:

  • bodyclose
  • gci
  • gomodguard
  • tenv

Mage lint command is divided into run and fix:

  • mage lint:run - start checking linters
  • mage lint:fix - auto-correction of linter errors

Checklist

  • I've read the guidelines for contributing to this repository.
  • I've followed the conventions in the PR title.
  • I've added tests that prove my fix is effective or that my feature works.
  • I've updated the documentation with the relevant information (if needed).
  • I've added usage information (if the PR introduces new options)
  • I've included a "before" and "after" example to the description (if the PR is a user interface change).

@DmitriyLewen DmitriyLewen self-assigned this Sep 22, 2023
@DmitriyLewen DmitriyLewen marked this pull request as ready for review September 22, 2023 06:41
.golangci.yaml Outdated Show resolved Hide resolved
.golangci.yaml Outdated Show resolved Hide resolved
@simar7
Copy link
Member

simar7 commented Sep 25, 2023

I stumbled upon this PR as I was looking to upgrade the version of the linter in Trivy. Somewhat related to disabled goimports and adding gci, I think it would be useful to have checks that are verbose enough to action on. Here's an example of an issue which doesn't actually mention which file is affected:

image

You can see the run here https://pipelinesghubeus21.actions.githubusercontent.com/sW2xPNHBbgj3dOBFJ6FecL5Uu8FcYDm4N9DVQuSaj41Px47eWm/_apis/pipelines/1/runs/23914/signedlogcontent/6?urlExpires=2023-09-25T15%3A52%3A00.6167227Z&urlSigningMethod=HMACV1&urlSignature=NUHkY5lqKC1KDnzRQMPx1IFZxdlgnvzklXDHipfkYCs%3D

On that note, it's also strange that the linter has different behavior on different platforms. I'm unable to repro that issue on an ARM macOS machine.

@DmitriyLewen
Copy link
Contributor Author

Hello @simar7
Thanks for your information.

I tried to find way to show wrong file, but looks like there is no way for that.
--verbose flag doesn't show info about file in info log:

➜  golangci-lint run --out-format=github-actions --deadline=30m --verbose
INFO [config_reader] Config search paths: [./ /home/dmitriy/work/repositories/trivy /home/dmitriy/work/repositories /home/dmitriy/work /home/dmitriy /home /] 
INFO [config_reader] Used config file .golangci.yaml 
INFO [lintersdb] Active 12 linters: [gci goconst gocyclo gofmt gosec govet ineffassign misspell revive typecheck unconvert unused] 
INFO [loader] Go packages loading at mode 575 (files|imports|name|types_sizes|compiled_files|exports_file|deps) took 3m1.283557298s 
INFO [runner/filename_unadjuster] Pre-built 1 adjustments in 71.552866ms 
INFO [linters_context/goanalysis] analyzers took 19.309510247s with top 10 stages: inspect: 4.412298883s, unconvert: 2.631525211s, the_only_name: 2.426979618s, printf: 2.179944608s, ctrlflow: 2.083987258s, gci: 1.934062989s, gofmt: 950.554111ms, gosec: 627.907675ms, misspell: 450.407801ms, unused: 315.321992ms 
INFO [runner] Issues before processing: 417, after processing: 2 
INFO [runner] Processors filtering stat (out/in): filename_unadjuster: 417/417, path_prettifier: 417/417, nolint: 2/8, path_shortener: 2/2, source_code: 2/2, sort_results: 2/2, skip_dirs: 287/287, autogenerated_exclude: 65/287, exclude: 65/65, max_from_linter: 2/2, diff: 2/2, max_per_file_from_linter: 2/2, cgo: 417/417, identifier_marker: 65/65, exclude-rules: 8/65, uniq_by_line: 2/2, path_prefixer: 2/2, skip_files: 287/417, max_same_issues: 2/2, severity-rules: 2/2, fixer: 2/2 
INFO [runner] processing took 10.36779ms with stages: path_prettifier: 4.209024ms, nolint: 2.895032ms, skip_files: 1.018151ms, identifier_marker: 857.682µs, autogenerated_exclude: 792.373µs, skip_dirs: 363.502µs, exclude-rules: 122.851µs, filename_unadjuster: 33.728µs, source_code: 28.074µs, cgo: 27.567µs, exclude: 12.142µs, uniq_by_line: 2.797µs, max_from_linter: 1.276µs, max_same_issues: 1.117µs, path_shortener: 1.023µs, max_per_file_from_linter: 516ns, fixer: 229ns, diff: 207ns, sort_results: 203ns, severity-rules: 184ns, path_prefixer: 112ns 
INFO [runner] linters took 14.232190143s with stages: goanalysis_metalinter: 14.221720102s 
::error file=pkg/cache/remote.go,line=7::File is not `gci`-ed with --skip-generated -s standard,default,prefix(github.com/aquasecurity/),blank,dot (gci)
::error file=pkg/cache/remote.go,line=13::File is not `gci`-ed with --skip-generated -s standard,default,prefix(github.com/aquasecurity/),blank,dot (gci)
INFO File cache stats: 517 entries of total size 2.9MiB 
INFO Memory: 1935 samples, avg is 96.5MB, max is 2395.7MB 
INFO Execution took 3m15.596028158s         

Also i didn't find envDebug value for our case.

This looks more like a bug in golangci-lint/action or conflict in settings - #5218 (comment).

@simar7 simar7 self-requested a review September 26, 2023 06:19
@simar7
Copy link
Member

simar7 commented Sep 26, 2023

@knqyf263 I worked with @DmitriyLewen offline to get this resolved. I will approve and merge this so we can test out the behavior of the linter in other PRs like #5218

@simar7 simar7 added this pull request to the merge queue Sep 26, 2023
@knqyf263
Copy link
Collaborator

ok

Merged via the queue into aquasecurity:main with commit 559c0f3 Sep 26, 2023
16 checks passed
@DmitriyLewen DmitriyLewen deleted the chore/more-linters branch September 26, 2023 07:12
@knqyf263 knqyf263 mentioned this pull request Oct 1, 2023
6 tasks
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.

3 participants