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

Custom Linter vs --fix #1728

Open
3 tasks done
MeenaAlfons opened this issue Feb 10, 2021 · 5 comments
Open
3 tasks done

Custom Linter vs --fix #1728

MeenaAlfons opened this issue Feb 10, 2021 · 5 comments
Labels
area: auto-fix enhancement New feature or improvement linter: custom About custom/private linters

Comments

@MeenaAlfons
Copy link

Thank you for creating the issue!

  • Yes, I'm using a binary release within 2 latest major releases. Only such installations are supported.
  • Yes, I've searched similar issues on GitHub and didn't find any.
  • Yes, I've included all information below (version, config, etc).

Please include the following information:

Version v1.33.0 of golangci-lint
$ golangci-lint --version
golangci-lint has version v1.33.0 built from (unknown, mod sum: "h1:/o4OtOR3Idim4FHKBJXcy+6ZjNDm82gwK/v6+gWyH9U=") on (unknown)
Config file
$ cat .golangci.yml
run:
  concurrency: 8
  timeout: 5m
  deadline: 5m
  skip-dirs:
    - vendor
  skip-files:
    - ".*?_with_.*?_gen\\.go"
    - "mock_.*?_gen\\.go"
linters:
  disable-all: true
  enable: # find all here: https://github.com/golangci/golangci-lint#enabled-by-default-linters
    # - unused # finds unused constants, variables, functions and types
    - deadcode # finds unused code
    # - godox # find TODO/FIXME messages
    - errcheck # finds unchecked errors
    - varcheck # finds unused global
    - ineffassign # Detects when assignments to existing variables are not used
    - unconvert # Remove unnecessary type conversions
    # - goconst # Finds repeated strings that could be replaced by a constant
    # - gocyclo # Computes and checks the cyclomatic complexity of functions
    - maligned # Tool to detect Go structs that would take less memory if their fields were sorted
    - prealloc # Finds slice declarations that could potentially be preallocated
    - govet # Vet examines Go source code and reports suspicious constructs
    - gosec # Inspects source code for security problems
    # - gosimple # Advocates simple code
    - lll # Max line length
    - depguard # Checks imports for packages not used
    - go-routine-recover

issues:
  max-issues-per-linter: 3
  max-same-issues: 3
  fix: true

linters-settings:
  lll:
    maxlength: 120
  depguard:
    list-type: blacklist
    include-go-root: true
    packages:
      - github.com/pkg/errors
      - log
      - go.uber.org/zap
      - go.uber.org/zap/zapcore
  custom:
    go-routine-recover:
      path: /golangci-lint-plugins/go-routine-recover.so
      description: Linter to find go-routines without recovery
      original-url: my.private.domain/path/to/go-routine-recover
Go environment
$ go version && go env
go version go1.15 darwin/amd64
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/username/Library/Caches/go-build"
GOENV="/Users/username/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/username/.gvm/pkgsets/go1.15/global/pkg/mod"
GONOPROXY="my.private.domain/*"
GONOSUMDB="my.private.domain/*"
GOOS="darwin"
GOPATH="/Users/username/.gvm/pkgsets/go1.15/global"
GOPRIVATE="my.private.domain/*"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/Users/username/.gvm/gos/go1.15"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/Users/username/.gvm/gos/go1.15/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/path/to/project/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/wh/4mp4d6g92tl09bxq79bxk_cw0000gp/T/go-build761519529=/tmp/go-build -gno-record-gcc-switches -fno-common"
Verbose output of running
$ golangci-lint cache clean
$ golangci-lint run -v
INFO [config_reader] Used config file build/configs/lint-required.yml 
INFO Loaded ../../path/to/go-routine-recover.so: go-routine-recover 
INFO [lintersdb] Active 12 linters: [deadcode depguard errcheck go-routine-recover gosec govet ineffassign lll maligned prealloc unconvert varcheck] 
INFO [loader] Go packages loading at mode 575 (compiled_files|deps|name|exports_file|files|imports|types_sizes) took 1.234498455s 
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 121.649298ms 
INFO [linters context/goanalysis] analyzers took 0s with no stages 
INFO [runner/max_same_issues] 16/19 issues with text "goroutinerecover: go-routine must contain a deferred call to r := recover()" were hidden, use --max-same-issues 
INFO [runner] Issues before processing: 5552, after processing: 3 
INFO [runner] Processors filtering stat (out/in): cgo: 5552/5552, exclude-rules: 352/429, max_from_linter: 3/3, diff: 19/19, skip_files: 4055/5552, skip_dirs: 4055/4055, exclude: 429/429, uniq_by_line: 19/19, source_code: 3/3, path_prefixer: 3/3, filename_unadjuster: 5552/5552, autogenerated_exclude: 429/4055, nolint: 19/352, max_per_file_from_linter: 19/19, severity-rules: 3/3, sort_results: 3/3, path_prettifier: 5552/5552, identifier_marker: 429/429, max_same_issues: 3/19, path_shortener: 3/3 
INFO [runner] processing took 80.277958ms with stages: nolint: 49.574238ms, path_prettifier: 12.55607ms, skip_files: 6.205368ms, identifier_marker: 3.48771ms, autogenerated_exclude: 3.461421ms, exclude-rules: 3.192881ms, skip_dirs: 887.802µs, cgo: 460.26µs, filename_unadjuster: 266.448µs, source_code: 142.412µs, max_same_issues: 29.346µs, uniq_by_line: 6.723µs, max_per_file_from_linter: 3.39µs, path_shortener: 1.356µs, max_from_linter: 1.282µs, diff: 330ns, exclude: 300ns, sort_results: 286ns, severity-rules: 212ns, path_prefixer: 123ns 
INFO [runner] linters took 247.044792ms with stages: goanalysis_metalinter: 166.628682ms 
INFO fixer took 0s with no stages                 
path/to/file1.go:41:2: goroutinerecover: go-routine must contain a deferred call to r := recover() (go-routine-recover)
        go func() {
        ^
path/to/file2.go:755:2: goroutinerecover: go-routine must contain a deferred call to r := recover() (go-routine-recover)
        go func() {
        ^
path/to/file3.go:204:2: goroutinerecover: go-routine must contain a deferred call to r := recover() (go-routine-recover)
        go func() {
        ^
INFO File cache stats: 3 entries of total size 48.3KiB 
INFO Memory: 18 samples, avg is 72.6MB, max is 74.0MB 
INFO Execution took 1.629181407s                  

We created a custom linter that works great. Right now we are trying to add suggested fixes to this linter. There is no example of such a thing in the New Linter tutorial on the website. However, we found SuggestedFix in the documentation of golang.org/x/tools/go/analysis. We used it but we still can't see any fixes happening.

		pass.Report(analysis.Diagnostic{
			Pos:     node.Pos(),
			End:     node.End(),
			Message: "go-routine must contain a deferred call to r := recover()",
			SuggestedFixes: []analysis.SuggestedFix{
				{
					Message: "add recover() with log message",
					TextEdits: []analysis.TextEdit{{
						Pos:     funcLit.Body.Lbrace,
						End:     funcLit.Body.Lbrace + 1,
						NewText: []byte(suggestedText),
					}},
				},
			},
		})

Is this the right way to support fixes in a custom linter ☝️ ? Or is there a different way we should use?

@MeenaAlfons MeenaAlfons added the bug Something isn't working label Feb 10, 2021
@boring-cyborg
Copy link

boring-cyborg bot commented Feb 10, 2021

Hey, thank you for opening your first Issue ! 🙂 If you would like to contribute we have a guide for contributors.

@ldez ldez added question Further information is requested linter: custom About custom/private linters and removed bug Something isn't working labels Feb 11, 2021
@ldez ldez added enhancement New feature or improvement and removed question Further information is requested labels Mar 24, 2021
@StevenACoffman
Copy link
Contributor

@MeenaAlfons If you figured this out, I would love to hear how!

@stale
Copy link

stale bot commented Aug 13, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale No recent correspondence or work activity label Aug 13, 2022
@ldez ldez removed the stale No recent correspondence or work activity label Aug 13, 2022
@polds
Copy link

polds commented May 20, 2024

Any progress in this area? I've run into this now where golangci-lint running a compiled .so cannot apply a fix, but crafting a custom main.go using singlechecker.Main() can run with fix:

package main

import (
	"fmt"

	"golang.org/x/tools/go/analysis/singlechecker"
	"mylinter"
)

func main() {
	p, err := mylinter.New(nil)
	if err != nil {
		fmt.Printf("Unable to initialize linter: %v\n", err)
		return
	}
	a, err := p.BuildAnalyzers()
	if err != nil {
		fmt.Printf("Unable to build analyzers: %v\n", err)
		return
	}
	singlechecker.Main(a[0])
}

go run cmd/mylinter/main.go -fix ./...

This works but a Plugin with a New func:

// New returns a new instance of the linter.
func New(any) ([]*analysis.Analyzer, error) {
	p, err := mylinter.New(nil)
	if err != nil {
		return nil, err
	}
	return p.BuildAnalyzers()
}

When I run:

GL_DEBUG="runner" golangci-lint run -v --config=.golangci.yaml --enable mylinter --max-same-issues 0 --fix

I see:

INFO [runner] fixer took 0s with no stages

@ldez
Copy link
Member

ldez commented May 20, 2024

This issue will be fixed when #1779 will be fixed.

I've started working on it again, and I hope to be able to finalize it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: auto-fix enhancement New feature or improvement linter: custom About custom/private linters
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants