Syntax errors in gocritic ruleguard file are silently ignored

[sebastien-rosset]

golangci-run silently ignores the following gocritic configuration issues:

1. The value of the ruleguard/rules setting is an inexistent path or the file is not readable. For example, the value is ruleguard/rules.go, but that file does not exist
2. The content of the ruleguard rules has at least one syntax error. All rules in the rules.go file are silently ignored.

This is unlike the gocritic and ruleguard behavior which print an error message and exit with error status.

For example, below in the ruleguard Where clause, it should be m["x"], not m[x], but any syntax error will be silently ignored.

// +build ignore
package gorules
import (
  "github.com/quasilyte/go-ruleguard/dsl/fluent"
)
func _(m fluent.Matcher) {
  m.Match(`$x.Do()`).Where(m[x].Type.Implements("MyInterface")).Report(`something`)
}

The workaround is to test the ruleguard file with gocritic and ruleguard, but there can be subtle problems such as when the version of gocritic and ruleguard included with golangci does not match the standalone version of gocritic and ruleguard.

• 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 of golangci-lint

$ golangci-lint --version
golangci-lint has version 1.32.2 built from 52d26a3 on 2020-11-03T01:15:38Z

Also tried golangci-lint built from local workspace with master branch as of 12/17/2020:

golangci-lint has version (devel) built from (unknown, mod sum: "") on (unknown)

Config file

$ cat .golangci.yml
linters:
  enable-all: false
  disable-all: false
  enable:
    - gocritic
  fast: false

linters-settings:
  gocritic:
    enabled-checks:
      - ruleguard
    settings:
      ruleguard:
        rules: gorules/rules.go

Go environment

$ go version && go env
go version go1.15.5 linux/amd64
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/serosset/.cache/go-build"
GOENV="/home/serosset/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/serosset/goroot/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/serosset/goroot"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD=""
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-build049545731=/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/serosset/goroot/src/bitbucket-eng-sjc1.cisco.com/an/build-tools /home/serosset/goroot/src/bitbucket-eng-sjc1.cisco.com/an /home/serosset/goroot/src/bitbucket-eng-sjc1.cisco.com /home/serosset/goroot/src /home/serosset/goroot /home/serosset /home /] 
INFO [config_reader] Used config file .golangci.yml 
INFO [lintersdb] Active 11 linters: [deadcode errcheck gocritic gosimple govet ineffassign staticcheck structcheck typecheck unused varcheck] 
INFO [loader] Go packages loading at mode 575 (exports_file|imports|name|compiled_files|deps|files|types_sizes) took 294.117245ms 
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 28.905626ms 
INFO [linters context/goanalysis] analyzers took 0s with no stages 
INFO [linters context/goanalysis] analyzers took 0s with no stages 
INFO [runner] Issues before processing: 2, after processing: 0 
INFO [runner] Processors filtering stat (out/in): autogenerated_exclude: 2/2, exclude: 0/2, cgo: 2/2, skip_dirs: 2/2, path_prettifier: 2/2, skip_files: 2/2, filename_unadjuster: 2/2, identifier_marker: 2/2 
INFO [runner] processing took 921.389µs with stages: path_prettifier: 553.306µs, autogenerated_exclude: 125.562µs, exclude: 107.269µs, identifier_marker: 67.213µs, skip_dirs: 27.277µs, nolint: 25.947µs, cgo: 6.77µs, max_same_issues: 1.92µs, filename_unadjuster: 1.493µs, uniq_by_line: 1.111µs, diff: 648ns, max_from_linter: 482ns, path_shortener: 458ns, skip_files: 413ns, source_code: 367ns, sort_results: 345ns, exclude-rules: 232ns, severity-rules: 215ns, path_prefixer: 191ns, max_per_file_from_linter: 170ns 
INFO [runner] linters took 93.278296ms with stages: goanalysis_metalinter: 91.24029ms, unused: 923.158µs 
INFO File cache stats: 0 entries of total size 0B 
INFO Memory: 6 samples, avg is 71.3MB, max is 71.8MB 
INFO Execution took 432.993189ms             

[sebastien-rosset]

When gocritic is invoked directly, an error is returned as shown below. I'm not sure if that helps, but in that case the exit status is 0; this is unlike the scenario when gocritic is reporting at least one issue.

gocritic check  -@ruleguard.rules gorules/rules.go -enable='ruleguard' .
ruleguard init error: typechecker error: gorules/rules.go:7:30: undeclared name: x

produces the following output:

// +build ignore
package gorules
import (
  "github.com/quasilyte/go-ruleguard/dsl/fluent"
)
func _(m fluent.Matcher) {
  m.Match(`$x.Do()`).Where(m[x].Type.Implements("MyInterface")).Report(`something`)
}

After some troubleshooting, it appears the root cause is in the gocritic source code:
https://github.com/go-critic/go-critic/blob/master/checkers/ruleguard_checker.go#L52-L74

[sebastien-rosset]

This should be fixed by PR go-critic/go-critic#1015 in go-critic. This issue can be closed when a new version of go-critic is released and the go-critic version is bumped in golangci-lint.

[sebastien-rosset]

The issue was "fixed" in go-critic/go-critic#1015, however, the solution does not work when invoked from golangci-lint. The problem is that all go-critic output seems to be redirected to /dev/null by golangci-lint.

What is the idiomatic way in golangci-lint to exit with error status when a linter (go-critic + ruleguard) is unable to initialize itself? Specifically in the case of go-critic, the user can write their own ruleguard static analysis rules which are parsed by go-critic + ruleguard. Since the user may inadvertently include syntax errors in the ruleguard files, it would be good to exit with non-zero status.

Update: Never mind, the fix is not part of go-critic@v0.5.3.