gocritic/ruleguard: MatchComment breaks ruleguard

[pierrre]

• 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).
• Yes, I've tried with the standalone linter if available. (https://golangci-lint.run/usage/linters/)

Description of the problem

I'm trying to use the ruleguard integration in gocritic.
When I try to use MatchComment, all other ruleguard rules stop working.

In the provided code, the call rule should report something, but it doesn't.
If you remove the comment rule, the call rule will work.

If I run the ruleguard command manually, it's working as expected.

ruleguard -rules="rules.go" .        
/home/pierre/gosrc/test/ruleguardcomment/rules.go:14:2: call: call (rules.go:14)
/home/pierre/gosrc/test/ruleguardcomment/rules.go:7:1: comment: foobar (rules.go:10)

I've also tried to run gocritic standalone linter, and it's working as expected:

gocritic check -enable='ruleguard' -@ruleguard.rules ./rules.go ./... 
./rules.go:14:2: ruleguard: call
./rules.go:7:1: ruleguard: foobar

Version of golangci-lint

$ golangci-lint --version
golangci-lint has version 1.39.0 built from 9aea4aee on 2021-03-26T08:02:53Z

Config file

$ cat .golangci.yml
linters:
  disable-all: true
  enable:
    - "gocritic"
linters-settings:
  gocritic:
    enabled-tags:
      - experimental
      - diagnostic
      - opinionated
      - performance
      - style
    settings:
      ruleguard:
        rules: "rules.go"

Go environment

$ go version && go env
go version go1.16.3 linux/amd64
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/pierre/.cache/go-build"
GOENV="/home/pierre/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/pierre/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/pierre/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/pierre/.gimme/versions/go1.16.3.src"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/pierre/.gimme/versions/go1.16.3.src/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.16.3"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/pierre/gosrc/test/ruleguardcomment/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 -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build909820084=/tmp/go-build -gno-record-gcc-switches"

Verbose output of running

$ golangci-lint cache clean
$ golangci-lint run -v
INFO [config_reader] Config search paths: [./ /home/pierre/gosrc/test/ruleguardcomment /home/pierre/gosrc/test /home/pierre/gosrc /home/pierre /home /] 
INFO [config_reader] Used config file .golangci.yml 
INFO [lintersdb] Active 1 linters: [gocritic]     
INFO [loader] Go packages loading at mode 575 (imports|deps|exports_file|files|name|types_sizes|compiled_files) took 11.270318ms 
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 77.739µs 
INFO [linters context/goanalysis] analyzers took 9.528607ms with top 10 stages: gocritic: 9.528607ms 
INFO [runner] processing took 2.242µs with stages: max_same_issues: 344ns, skip_dirs: 263ns, nolint: 199ns, max_from_linter: 194ns, path_prettifier: 157ns, cgo: 116ns, filename_unadjuster: 106ns, uniq_by_line: 106ns, skip_files: 106ns, diff: 104ns, identifier_marker: 102ns, autogenerated_exclude: 100ns, path_prefixer: 50ns, sort_results: 45ns, max_per_file_from_linter: 44ns, path_shortener: 44ns, source_code: 42ns, severity-rules: 42ns, exclude-rules: 40ns, exclude: 38ns 
INFO [runner] linters took 20.485539ms with stages: gocritic: 20.454889ms 
INFO File cache stats: 0 entries of total size 0B 
INFO Memory: 2 samples, avg is 72.5MB, max is 72.6MB 
INFO Execution took 35.349799ms

Code example or link to a public repository

package gorules

import (
	"github.com/quasilyte/go-ruleguard/dsl"
)

// foobar

func comment(m dsl.Matcher) {
	m.MatchComment(`// foobar`).Report(`foobar`)
}

func call(m dsl.Matcher) {
	m.Match(`m.Match`).Report(`call`)
}

[ldez]

Hello,

when I display the full go-critic output for your rule:

$ golangci-lint run
WARN [runner] Can't run linter gocritic: gocritic: ruleguard init error: ./rules.go:10: unexpected MatchComment method 
ERRO Running error: gocritic: ruleguard init error: ./rules.go:10: unexpected MatchComment method 

linters:
  disable-all: true
  enable:
    - "gocritic"

linters-settings:
  gocritic:
    enabled-tags:
      - experimental
      - diagnostic
      - opinionated
      - performance
      - style
    settings:
      ruleguard:
        rules: "./rules.go"
        failOnError: true

For now, I don't know if it's expected or not.

[ldez]

I found the problem it's because go-critic@v0.5.5 uses go-ruleguard@v0.3.1:
https://github.com/go-critic/go-critic/blob/v0.5.5/go.mod#L19

But MatchComment has been added in quasilyte/go-ruleguard#223 => v0.3.4

with the good version of go-ruleguard:

$ ./golangci-lint run 
rules.go:14:2: ruleguard: call (gocritic)
        m.Match(`m.Match`).Report(`call`)
        ^
rules.go:7:1: ruleguard: foobar (gocritic)
// foobar
^

[ldez]

@pierrre you can open a PR on https://github.com/go-critic/go-critic to update this dependency.

[pierrre]

Thank you, I didn't know that this feature was introduced recently.
I will send a PR.

[pierrre]

done go-critic/go-critic#1041

[quasilyte]

You can ping/CC me if you have some ruleguard or go-critic related problems by the way.

[pierrre]

https://github.com/go-critic/go-critic/releases/tag/v0.5.6 is released, I will try to update it in golangci-lint

[pierrre]

Here is the PR to update go-critic to v0.5.6
#1925

[pierrre]

Ooops

pkg/golinters/govet_test.go:21:15: appendAssign: append result not assigned to the same slice (gocritic)

What should I do ? Ignore it ?

[pierrre]

And after ignoring it 😆

Error: directive `//nolint:gocritic // Build a temporary list of analyzers.` is unused for linter "gocritic" (nolintlint)

[quasilyte]

I wouldn't ignore it as the fix is quite simple.
I don't know what golangci-lint maintainers would think though.