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.

Sign up for GitHub

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

Jump to bottom

Missing output from some megacheck & go-critic checks #314

Closed
jakewarren opened this issue Dec 6, 2018 · 7 comments
Closed

Missing output from some megacheck & go-critic checks #314

jakewarren opened this issue Dec 6, 2018 · 7 comments
Labels
bug Something isn't working false negative An error is not reported when one exists

Comments

@jakewarren
Copy link

jakewarren commented Dec 6, 2018 •
edited
Loading

Hello, when running golangci-lint in Gitlab CI, golangci-lint is missing the output of a few checks from megacheck and go-critic.

I put together some sample code so hopefully this can be reproduced. Let me know if there is any further information I can provide.

Sample code

gocritic.go

package gocritic

func main() {

	//fmt.Println("This is commented out code")

	// crazy if-else chain to trip ifElseChain checker
	if 1 == 2 {

	} else if 2 == 3 {

	} else if 3 == 4 {

	} else {

	}
}

megacheck.go

package gocritic

import "regexp"

func megachecktest() {

	// generate SA6004
	regexp.MustCompile(`-String`)


	// generate 'omit comparison to bool constant' check
	testFlag := true

	if testFlag == true {

	}

}

Gitlab CI & golangci-lint config

.gitlab-ci.yml

image: golang:1.10

variables:
    REPO_NAME: gitlab.com/jake/gocritic-test/

lint:
  before_script:
  - shopt -s dotglob                                            # Ensure .git/ directory is moved as well
  - GO_FILES=$(find . -iname '*.go' -type f | grep -v /vendor/ | grep -v /banner/) # All the .go files, excluding vendor/
  - wget -O - -q https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | sh -s v1.12.3
  - mkdir -p $GOPATH/src/$REPO_NAME                             # move project into the GOPATH
  - mv $CI_PROJECT_DIR/* $GOPATH/src/$REPO_NAME
  - cd $GOPATH/src/$REPO_NAME

  script:
  - ./bin/golangci-lint --version
  - go version && go env
  - ./bin/golangci-lint run -v -c .golangci.yml


critic:
  allow_failure: true
  before_script:
    - shopt -s dotglob                                            # Ensure .git/ directory is moved as well
    - mkdir -p $GOPATH/src/$REPO_NAME                             # move project into the GOPATH
    - wget -O - -q https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | sh -s v1.12.3
    - mv $CI_PROJECT_DIR/* $GOPATH/src/$REPO_NAME
    - cd $GOPATH/src/$REPO_NAME
  script:
    - ./bin/golangci-lint --version
    - go version && go env
    - ./bin/golangci-lint run -v -c .golangci.critic.yml
.golangci.critic.yml

# This file contains all available configuration options
# with their default values.

# options for analysis running
run:
  # default concurrency is a available CPU number
  concurrency: 4

  # timeout for analysis, e.g. 30s, 5m, default is 1m
  deadline: 1m

  # exit code when at least one issue was found, default is 1
  issues-exit-code: 1

  # include test files or not, default is true
  tests: false

  # which dirs to skip: they won't be analyzed;
  # can use regexp here: generated.*, regexp is applied on full path;
  # default value is empty list, but next dirs are always skipped independently
  # from this option's value:
  #   	vendor$, third_party$, testdata$, examples$, Godeps$, builtin$
  skip-dirs:
    - vendor$


# all available settings of specific linters
linters-settings:
  govet:
    # report about shadowed variables
    check-shadowing: true
  golint:
    # minimalfor issues, default is 0.8
    min-: 0.8
  gofmt:
    # simplify code: gofmt with `-s` option, true by default
    simplify: true
  gocyclo:
    # minimal code complexity to report, 30 by default (but we recommend 10-20)
    min-complexity: 19
  maligned:
    # print struct with more effective memory layout or not, false by default
    suggest-new: true
  dupl:
    # tokens count to trigger issue, 150 by default
    threshold: 100
  goconst:
    # minimal length of string constant, 3 by default
    min-len: 3
    # minimal occurrences count to trigger, 3 by default
    min-occurrences: 3
  unused:
    # treat code as a program (not a library) and report unused exported identifiers; default is false.
    # XXX: if you enable this setting, unused will report a lot of false-positives in text editors:
    # if it's called for subdir of a project it can't find funcs usages. All text editor integrations
    # with golangci-lint call it on a directory with the changed file.
    check-exported: false
  unparam:
    # call graph construction algorithm (cha, rta). In general, use cha for libraries,
    # and rta for programs with main packages. Default is cha.
    algo: cha

    # Inspect exported functions, default is false. Set to true if no external program/library imports your code.
    # XXX: if you enable this setting, unparam will report a lot of false-positives in text editors:
    # if it's called for subdir of a project it can't find external interfaces. All text editor integrations
    # with golangci-lint call it on a directory with the changed file.
    check-exported: false
  nakedret:
    # make an issue if func has more lines of code than this setting and it has naked returns; default is 30
    max-func-lines: 30
  prealloc:
    # XXX: we don't recommend using this linter before doing performance profiling.
    # For most programs usage of prealloc will be a premature optimization.

    # Report preallocation suggestions only on simple loops that have no returns/breaks/continues/gotos in them.
    # True by default.
    simple: true
    range-loops: true # Report preallocation suggestions on range loops, true by default
    for-loops: false # Report preallocation suggestions on for loops, false by default
  gocritic:
    # which checks should be enabled; can't be combined with 'disabled-checks';
    # default are: [appendAssign assignOp caseOrder dupArg dupBranchBody dupCase flagDeref
    # ifElseChain regexpMust singleCaseSwitch sloppyLen switchTrue typeSwitchVar underef
    # unlambda unslice rangeValCopy defaultCaseOrder];
    # all checks list: https://github.com/go-critic/checkers
    enabled-checks:
      - appendAssign
      - appendCombine
      - assignOp
      - blankParam 
      - boolExprSimplify
      - boolFuncPrefix 
      - builtinShadow
      - busySelect
      - captLocal
      - caseOrder
      - commentedOutCode
      - deadCodeAfterLogFatal
      - defaultCaseOrder
      - deferInLoop
      - deprecatedComment
      - docStub
      - dupArg
      - dupBranchBody
      - dupCase
      - dupSubExpr
      - elseif 
      - emptyFallthrough
      - emptyFmt
      - evalOrder
      - flagDeref
      - flagName
      - floatCompare
      - hugeParam
      - ifElseChain
      - importPackageName
      - importShadow
      - indexAlloc
      - indexOnlyLoop
      - initClause
      - longChain 
      - methodExprCall 
      - namedConst
      - nestingReduce
      - nilValReturn
      - paramTypeCombine 
      - ptrToRefParam
      - rangeExprCopy
      - rangeValCopy
      - regexpMust
      - singleCaseSwitch
      - sloppyLen
      - sqlRowsClose
      - stdExpr
      - structLitKeyOrder 
      - switchTrue
      - typeSwitchVar
      - typeUnparen
      - underef
      - unexportedCall 
      - unlabelStmt
      - unlambda
      - unnamedResult
      - unnecessaryBlock
      - unslice
      - yodaStyleExpr 

    settings: # settings passed to gocritic
      captLocal: # must be valid enabled check name
        checkLocals: true
      rangeValCopy:
        sizeThreshold: 32

linters:
  disable-all: true
  enable:
    - gocritic
    - gosec
    - dupl
    - interfacer
    - goconst
    - golint
  



issues:
  # Independently from option `exclude` we use default exclude patterns,
  # it can be disabled by this option. To list all
  # excluded by default patterns execute `golangci-lint run --help`.
  # Default value for this option is true.
  exclude-use-default: false

  # Maximum issues count per one linter. Set to 0 to disable. Default is 50.
  max-per-linter: 0

  # Maximum count of issues with the same text. Set to 0 to disable. Default is 3.
  max-same-issues: 0
.golangci.yml

# This file contains all available configuration options
# with their default values.

# options for analysis running
run:
  # default concurrency is a available CPU number
  concurrency: 4

  # timeout for analysis, e.g. 30s, 5m, default is 1m
  deadline: 1m

  # exit code when at least one issue was found, default is 1
  issues-exit-code: 1

  # include test files or not, default is true
  tests: true

  # which dirs to skip: they won't be analyzed;
  # can use regexp here: generated.*, regexp is applied on full path;
  # default value is empty list, but next dirs are always skipped independently
  # from this option's value:
  #   	vendor$, third_party$, testdata$, examples$, Godeps$, builtin$
  skip-dirs:
    - vendor$


# all available settings of specific linters
linters-settings:
  govet:
    # report about shadowed variables
    check-shadowing: true
  golint:
    # minimalfor issues, default is 0.8
    min-: 0.8
  gofmt:
    # simplify code: gofmt with `-s` option, true by default
    simplify: true
  gocyclo:
    # minimal code complexity to report, 30 by default (but we recommend 10-20)
    min-complexity: 19
  maligned:
    # print struct with more effective memory layout or not, false by default
    suggest-new: true
  dupl:
    # tokens count to trigger issue, 150 by default
    threshold: 100
  goconst:
    # minimal length of string constant, 3 by default
    min-len: 3
    # minimal occurrences count to trigger, 3 by default
    min-occurrences: 3
  unused:
    # treat code as a program (not a library) and report unused exported identifiers; default is false.
    # XXX: if you enable this setting, unused will report a lot of false-positives in text editors:
    # if it's called for subdir of a project it can't find funcs usages. All text editor integrations
    # with golangci-lint call it on a directory with the changed file.
    check-exported: false
  unparam:
    # call graph construction algorithm (cha, rta). In general, use cha for libraries,
    # and rta for programs with main packages. Default is cha.
    algo: cha

    # Inspect exported functions, default is false. Set to true if no external program/library imports your code.
    # XXX: if you enable this setting, unparam will report a lot of false-positives in text editors:
    # if it's called for subdir of a project it can't find external interfaces. All text editor integrations
    # with golangci-lint call it on a directory with the changed file.
    check-exported: false
  nakedret:
    # make an issue if func has more lines of code than this setting and it has naked returns; default is 30
    max-func-lines: 30
  prealloc:
    # XXX: we don't recommend using this linter before doing performance profiling.
    # For most programs usage of prealloc will be a premature optimization.

    # Report preallocation suggestions only on simple loops that have no returns/breaks/continues/gotos in them.
    # True by default.
    simple: true
    range-loops: true # Report preallocation suggestions on range loops, true by default
    for-loops: false # Report preallocation suggestions on for loops, false by default


linters:
  enable:
    - gofmt
    - gocyclo


issues:
  # Independently from option `exclude` we use default exclude patterns,
  # it can be disabled by this option. To list all
  # excluded by default patterns execute `golangci-lint run --help`.
  # Default value for this option is true.
  exclude-use-default: false

  # Maximum issues count per one linter. Set to 0 to disable. Default is 50.
  max-per-linter: 0

  # Maximum count of issues with the same text. Set to 0 to disable. Default is 3.
  max-same-issues: 0

Actual Output

Gitlab CI output - lint job

Running with gitlab-runner 10.6.0 (a3543a27)
  on docker2 df4c59a3
Using Docker executor with image golang:1.10 ...
Pulling docker image golang:1.10 ...
Using docker image sha256:0a19f4d16598389f48e3f21d1e17e877c5f8f8e6f785d39c226db07ca908d07c for golang:1.10 ...
Running on runner-df4c59a3-project-205-concurrent-0 via thor...
Cloning repository...
Cloning into '/builds/jake/golangci-lint-test'...
Checking out d8959d94 as master...
Skipping Git submodules setup
$ shopt -s dotglob
$ GO_FILES=$(find . -iname '*.go' -type f | grep -v /vendor/ | grep -v /banner/)
$ wget -O - -q https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | sh -s v1.12.3
golangci/golangci-lint info checking GitHub for tag 'v1.12.3'
golangci/golangci-lint info found version: 1.12.3 for v1.12.3/linux/amd64
golangci/golangci-lint info installed ./bin/golangci-lint
$ mkdir -p $GOPATH/src/$REPO_NAME
$ mv $CI_PROJECT_DIR/* $GOPATH/src/$REPO_NAME
$ cd $GOPATH/src/$REPO_NAME
$ ./bin/golangci-lint --version
golangci-lint has version 1.12.3 built from 014a924 on 2018-11-24T08:55:55Z
$ go version && go env
go version go1.10.5 linux/amd64
GOARCH="amd64"
GOBIN=""
GOCACHE="/root/.cache/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/go"
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
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-build688588277=/tmp/go-build -gno-record-gcc-switches"
$ ./bin/golangci-lint run -v -c .golangci.yml
level=info msg="[config_reader] Used config file .golangci.yml"
level=info msg="Gocritic enabled checks: [appendAssign assignOp caseOrder dupArg dupBranchBody dupCase flagDeref ifElseChain regexpMust singleCaseSwitch sloppyLen switchTrue typeSwitchVar underef unlambda unslice defaultCaseOrder]"
level=info msg="[lintersdb] Active 10 linters: [deadcode errcheck gocyclo gofmt govet ineffassign megacheck structcheck typecheck varcheck]"
level=info msg="[loader] Go packages loading at mode load deps types and syntax took 1.86353391s"
level=info msg="[loader] SSA repr building timing: packages building 6.179381ms, total 575.780465ms"
level=info msg="[loader] SSA for megacheck repr building timing: packages building 7.834488ms, total 673.891341ms"
level=info msg="[runner] worker.2 took 535.975µs with stages: ineffassign: 266.699µs, errcheck: 80.736µs, gocyclo: 57.928µs, deadcode: 47.16µs, varcheck: 31.799µs, structcheck: 11.198µs, typecheck: 2.114µs"
level=info msg="[runner] worker.1 took 2.726412ms with stages: gofmt: 2.717638ms"
gocritic.go:3:6: `main` is unused (deadcode)
func main() {
     ^
megacheck.go:5:6: `megachecktest` is unused (deadcode)
func megachecktest() {
     ^
megacheck.go:10: File is not `gofmt`-ed with `-s` (gofmt)

level=info msg="[runner] worker.4 took 3.277516ms with stages: govet: 3.26455ms"
level=info msg="[runner] worker.3 took 119.794137ms with stages: megacheck: 119.74234ms"
level=info msg="[runner] Workers idle times: #1: 116.973808ms, #2: 117.179096ms, #4: 116.485316ms"
gocritic.go:12:9: empty branch (megacheck)
	} else if 3 == 4 {
	       ^
gocritic.go:14:9: empty branch (megacheck)
	} else {
	       ^
megacheck.go:14:2: empty branch (megacheck)
	if testFlag == true {
	^
level=info msg="[runner] processing took 715.595µs with stages: skip_dirs: 314.823µs, cgo: 128.225µs, source_code: 104.596µs, autogenerated_exclude: 51.196µs, path_prettifier: 50.777µs, nolint: 31.389µs, uniq_by_line: 11.36µs, max_from_linter: 7.997µs, max_per_file_from_linter: 5.576µs, path_shortener: 4.536µs, max_same_issues: 1.934µs, diff: 1.211µs, skip_files: 995ns, exclude: 980ns"
level=info msg="Memory: 32 samples, avg is 148.6MB, max is 271.2MB"
level=info msg="Execution took 3.316379246s"
ERROR: Job failed: exit code 1
Gitlab CI output - critic job

Running with gitlab-runner 10.6.0 (a3543a27)
  on docker 32c94010
Using Docker executor with image golang:1.10 ...
Pulling docker image golang:1.10 ...
Using docker image sha256:0a19f4d16598389f48e3f21d1e17e877c5f8f8e6f785d39c226db07ca908d07c for golang:1.10 ...
Running on runner-32c94010-project-205-concurrent-0 via thor...
Cloning repository...
Cloning into '/builds/jake/golangci-lint-test'...
Checking out d8959d94 as master...
Skipping Git submodules setup
$ shopt -s dotglob
$ mkdir -p $GOPATH/src/$REPO_NAME
$ wget -O - -q https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | sh -s v1.12.3
golangci/golangci-lint info checking GitHub for tag 'v1.12.3'
golangci/golangci-lint info found version: 1.12.3 for v1.12.3/linux/amd64
golangci/golangci-lint info installed ./bin/golangci-lint
$ mv $CI_PROJECT_DIR/* $GOPATH/src/$REPO_NAME
$ cd $GOPATH/src/$REPO_NAME
$ ./bin/golangci-lint --version
golangci-lint has version 1.12.3 built from 014a924 on 2018-11-24T08:55:55Z
$ go version && go env
go version go1.10.5 linux/amd64
GOARCH="amd64"
GOBIN=""
GOCACHE="/root/.cache/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/go"
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
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-build648059545=/tmp/go-build -gno-record-gcc-switches"
$ ./bin/golangci-lint run -v -c .golangci.critic.yml
level=info msg="[config_reader] Used config file .golangci.critic.yml"
level=info msg="Gocritic enabled checks: [appendAssign appendCombine assignOp blankParam boolExprSimplify boolFuncPrefix builtinShadow busySelect captLocal caseOrder commentedOutCode deadCodeAfterLogFatal defaultCaseOrder deferInLoop deprecatedComment docStub dupArg dupBranchBody dupCase dupSubExpr elseif emptyFallthrough emptyFmt evalOrder flagDeref flagName floatCompare hugeParam ifElseChain importPackageName importShadow indexAlloc indexOnlyLoop initClause longChain methodExprCall namedConst nestingReduce nilValReturn paramTypeCombine ptrToRefParam rangeExprCopy rangeValCopy regexpMust singleCaseSwitch sloppyLen sqlRowsClose stdExpr structLitKeyOrder switchTrue typeSwitchVar typeUnparen underef unexportedCall unlabelStmt unlambda unnamedResult unnecessaryBlock unslice yodaStyleExpr]"
level=info msg="[lintersdb] Active 6 linters: [dupl goconst gocritic golint gosec interfacer]"
level=info msg="[loader] Go packages loading at mode load deps types and syntax took 1.673211308s"
level=info msg="[loader] SSA repr building timing: packages building 6.713773ms, total 495.470632ms"
level=info msg="[loader] SSA for megacheck repr building timing: packages building 8.026521ms, total 675.817777ms"
level=info msg="[runner] worker.1 took 1.10048ms with stages: dupl: 582.846µs, gosec: 464.496µs, goconst: 21.752µs"
level=info msg="[runner] worker.3 took 4.517108ms with stages: gocritic: 4.503699ms"
level=info msg="[runner] worker.2 took 5.887639ms with stages: golint: 5.844073ms"
gocritic.go:12:9: dupBranchBody: both branches in if statement has same body (gocritic)
	} else if 3 == 4 {
	       ^
gocritic.go:8:2: ifElseChain: rewrite if-else to switch statement (gocritic)
	if 1 == 2 {
	^
level=info msg="[runner] worker.4 took 23.830539ms with stages: interfacer: 23.787918ms"
level=info msg="[runner] Workers idle times: #1: 21.004036ms, #2: 18.436737ms, #3: 19.735613ms"
level=info msg="[runner] processing took 375.458µs with stages: skip_dirs: 128.885µs, path_prettifier: 85.814µs, source_code: 63.645µs, autogenerated_exclude: 33.66µs, cgo: 23.506µs, nolint: 23.386µs, uniq_by_line: 4.218µs, path_shortener: 3.357µs, max_from_linter: 3.148µs, max_per_file_from_linter: 1.968µs, max_same_issues: 1.603µs, diff: 811ns, exclude: 749ns, skip_files: 708ns"
level=info msg="Memory: 26 samples, avg is 135.9MB, max is 270.9MB"
level=info msg="Execution took 2.950550172s"
ERROR: Job failed: exit code 1

Expected Output

Click to expand

❯ gocritic version
0.3.3
❯ gocritic check-project -withExperimental -withOpinionated .
/home/jake/golang/gocritic-test/gocritic.go:5:2: commentedOutCode: may want to remove commented-out code
/home/jake/golang/gocritic-test/gocritic.go:9:2: ifElseChain: should rewrite if-else to switch statement
/home/jake/golang/gocritic-test/gocritic.go:14:8: dupBranchBody: both branches in if statement has same body
lint error: exit status 1


❯ megacheck ./...
gocritic.go:12:9: empty branch (SA9003)
gocritic.go:14:9: empty branch (SA9003)
megacheck.go:8:20: regular expression does not contain any meta characters (SA6004)
megacheck.go:14:2: empty branch (SA9003)
megacheck.go:14:5: should omit comparison to bool constant, can be simplified to testFlag (S1002)
gocritic.go:3:6: func main is unused (U1000)
megacheck.go:5:6: func megachecktest is unused (U1000)

tldr

The following linter warnings are not showing up:

megacheck.go:8:20: regular expression does not contain any meta characters (SA6004)
megacheck.go:14:5: should omit comparison to bool constant, can be simplified to testFlag (S1002)
gocritic.go:5:2: commentedOutCode: may want to remove commented-out code
@lopezator
Copy link
Member

Same here, some gocritic checks doesn't appear to work.

Digging something I saw that some checks files are missing inside vendor/github.com/go-critic/checkers

Maybe this could be the cause?

This was referenced Dec 20, 2018
Closed
Closed
jirfag added a commit that referenced this issue Jan 8, 2019
@jirfag
Update megacheck to the latest version
4345bd2 
Also do following improvements:
  - show proper sublinter name for megacheck sublinters
  - refactor and make more simple and robust megacheck
  merging/optimizing
  - improve handling of unknown linter names in //nolint directives
  - minimize diff of our megacheck version from the upstream,
  golang/go#29612 blocks usage of the upstream
  version
  - support the new `stylecheck` linter
  - improve tests coverage for megacheck and nolint related cases
  - update and use upstream versions of unparam and interfacer instead of forked
  ones
  - don't use golangci/tools repo anymore
  - fix newly found issues after updating linters

Relates: #314
@jirfag jirfag mentioned this issue Jan 8, 2019
Merged
jirfag added a commit that referenced this issue Jan 8, 2019
@jirfag
Update megacheck to the latest version
37603c1 
Also do following improvements:
  - show proper sublinter name for megacheck sublinters
  - refactor and make more simple and robust megacheck
  merging/optimizing
  - improve handling of unknown linter names in //nolint directives
  - minimize diff of our megacheck version from the upstream,
  golang/go#29612 blocks usage of the upstream
  version
  - support the new `stylecheck` linter
  - improve tests coverage for megacheck and nolint related cases
  - update and use upstream versions of unparam and interfacer instead of forked
  ones
  - don't use golangci/tools repo anymore
  - fix newly found issues after updating linters

Also should be noted that megacheck works much faster in the newest
release, therefore golangci-lint works noticeably faster for large
repos.

Relates: #314
jirfag added a commit that referenced this issue Jan 8, 2019
@jirfag
Update megacheck to the latest version
14afb84 
Also do following improvements:
  - show proper sublinter name for megacheck sublinters
  - refactor and make more simple and robust megacheck
  merging/optimizing
  - improve handling of unknown linter names in //nolint directives
  - minimize diff of our megacheck version from the upstream,
  golang/go#29612 blocks usage of the upstream
  version
  - support the new `stylecheck` linter
  - improve tests coverage for megacheck and nolint related cases
  - update and use upstream versions of unparam and interfacer instead of forked
  ones
  - don't use golangci/tools repo anymore
  - fix newly found issues after updating linters

Also should be noted that megacheck works much faster in the newest
release, therefore golangci-lint works noticeably faster for large
repos.

Relates: #314
jirfag added a commit that referenced this issue Jan 8, 2019
@jirfag
Update megacheck to the latest version
a9d458e 
Also do following improvements:
  - show proper sublinter name for megacheck sublinters
  - refactor and make more simple and robust megacheck
  merging/optimizing
  - improve handling of unknown linter names in //nolint directives
  - minimize diff of our megacheck version from the upstream,
  golang/go#29612 blocks usage of the upstream
  version
  - support the new `stylecheck` linter
  - improve tests coverage for megacheck and nolint related cases
  - update and use upstream versions of unparam and interfacer instead of forked
  ones
  - don't use golangci/tools repo anymore
  - fix newly found issues after updating linters

Also should be noted that megacheck works much faster and consumes less
memory in the newest release, therefore golangci-lint works noticeably
faster and consumes less memory for large repos.

Relates: #314
jirfag added a commit that referenced this issue Jan 8, 2019
@jirfag
Update megacheck to the latest version
420fb26 
Also do following improvements:
  - show proper sublinter name for megacheck sublinters
  - refactor and make more simple and robust megacheck
  merging/optimizing
  - improve handling of unknown linter names in //nolint directives
  - minimize diff of our megacheck version from the upstream,
  golang/go#29612 blocks usage of the upstream
  version
  - support the new `stylecheck` linter
  - improve tests coverage for megacheck and nolint related cases
  - update and use upstream versions of unparam and interfacer instead of forked
  ones
  - don't use golangci/tools repo anymore
  - fix newly found issues after updating linters
  - megacheck is able to handle partially compiling programs

Also should be noted that megacheck works much faster and consumes less
memory in the newest release, therefore golangci-lint works noticeably
faster and consumes less memory for large repos.

Relates: #314
jirfag added a commit that referenced this issue Jan 8, 2019
@jirfag
Update megacheck to the latest version
ff372f7 
Also do following improvements:
  - show proper sublinter name for megacheck sublinters
  - refactor and make more simple and robust megacheck
  merging/optimizing
  - improve handling of unknown linter names in //nolint directives
  - minimize diff of our megacheck version from the upstream,
  golang/go#29612 blocks usage of the upstream
  version
  - support the new `stylecheck` linter
  - improve tests coverage for megacheck and nolint related cases
  - update and use upstream versions of unparam and interfacer instead of forked
  ones
  - don't use golangci/tools repo anymore
  - fix newly found issues after updating linters

Also should be noted that megacheck works much faster and consumes less
memory in the newest release, therefore golangci-lint works noticeably
faster and consumes less memory for large repos.

Relates: #314
jirfag added a commit that referenced this issue Jan 8, 2019
@jirfag
Update megacheck to the latest version
7705f82 
Also do following improvements:
  - show proper sublinter name for megacheck sublinters
  - refactor and make more simple and robust megacheck
  merging/optimizing
  - improve handling of unknown linter names in //nolint directives
  - minimize diff of our megacheck version from the upstream,
  golang/go#29612 blocks usage of the upstream
  version
  - support the new `stylecheck` linter
  - improve tests coverage for megacheck and nolint related cases
  - update and use upstream versions of unparam and interfacer instead of forked
  ones
  - don't use golangci/tools repo anymore
  - fix newly found issues after updating linters

Also should be noted that megacheck works much faster and consumes less
memory in the newest release, therefore golangci-lint works noticeably
faster and consumes less memory for large repos.

Relates: #314
jirfag added a commit that referenced this issue Jan 8, 2019
@jirfag
Update gocritic
dba8bdc 
Fix #324, relates #314

1. Update gocritic to the latest version
2. Use proper gocritic checkers repo, old repo was archived
3. Get enabled by default gocritic checks in sync with go-critic: don't
enable performance, experimental and opinionated checks by default
4. Support of `enabled-tags` options for gocritic
5. Enable almost all gocritic checks for the project
6. Make rich debugging for gocritic
7. Meticulously validate gocritic checks config
@jirfag jirfag mentioned this issue Jan 8, 2019
Merged
jirfag added a commit that referenced this issue Jan 9, 2019
@jirfag
Update gocritic
4e3255d 
Fix #324, relates #314

1. Update gocritic to the latest version
2. Use proper gocritic checkers repo, old repo was archived
3. Get enabled by default gocritic checks in sync with go-critic: don't
enable performance, experimental and opinionated checks by default
4. Support of `enabled-tags` options for gocritic
5. Enable almost all gocritic checks for the project
6. Make rich debugging for gocritic
7. Meticulously validate gocritic checks config
jirfag added a commit that referenced this issue Jan 9, 2019
@jirfag
Update gocritic
749cdb2 
Fix #324, relates #314

1. Update gocritic to the latest version
2. Use proper gocritic checkers repo, old repo was archived
3. Get enabled by default gocritic checks in sync with go-critic: don't
enable performance, experimental and opinionated checks by default
4. Support of `enabled-tags` options for gocritic
5. Enable almost all gocritic checks for the project
6. Make rich debugging for gocritic
7. Meticulously validate gocritic checks config
jirfag added a commit that referenced this issue Jan 9, 2019
@jirfag
Update gocritic
87aae77 
Fix #324, relates #314

1. Update gocritic to the latest version
2. Use proper gocritic checkers repo, old repo was archived
3. Get enabled by default gocritic checks in sync with go-critic: don't
enable performance, experimental and opinionated checks by default
4. Support of `enabled-tags` options for gocritic
5. Enable almost all gocritic checks for the project
6. Make rich debugging for gocritic
7. Meticulously validate gocritic checks config
@jirfag
Copy link
Member

jirfag commented Jan 9, 2019

hi! thank you guys, can you check it in the latest master version? I've updated megacheck and gocritic

@jakewarren
Copy link
Author

Hi @jirfag thank you for working on it! I re-ran the tests with my little test repo and it looks like the gocritic checks are working properly now. I'm still not seeing the megacheck warnings displayed (SA6004 and S1002) though.

@lopezator lopezator mentioned this issue Jan 16, 2019
Closed
@jirfag
Copy link
Member

jirfag commented Jan 20, 2019

SA6004 was disabled in dominikh/go-tools@dc1aef7,
S1002 was changed in dominikh/go-tools@c98cc07,

please, check it on the latest version of staticcheck (megacheck)

@jakewarren
Copy link
Author

Good catch! Yes, can confirm that SA6004 no longer appears with the latest stable release of staticcheck.

Still seeing S1002 when running against the sample code.

❯ /tmp/staticcheck_linux_amd64 -version
staticcheck_linux_amd64 2019.1
❯ /tmp/staticcheck_linux_amd64 .
gocritic.go:3:6: func main is unused (U1000)
gocritic.go:12:9: empty branch (SA9003)
gocritic.go:14:9: empty branch (SA9003)
megacheck.go:5:6: func megachecktest is unused (U1000)
megacheck.go:14:2: empty branch (SA9003)
megacheck.go:14:5: should omit comparison to bool constant, can be simplified to testFlag (S1002)

Version info from the CI output using v1.13:

$ ./bin/golangci-lint --version
golangci-lint has version 1.13 built from 2192097 on 2019-01-21T07:58:29Z
Full CI output 
$ wget -O - -q https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | sh -s v1.13
golangci/golangci-lint info checking GitHub for tag 'v1.13'
golangci/golangci-lint info found version: 1.13 for v1.13/linux/amd64
golangci/golangci-lint info installed ./bin/golangci-lint
$ mkdir -p $GOPATH/src/$REPO_NAME
$ mv $CI_PROJECT_DIR/* $GOPATH/src/$REPO_NAME
$ cd $GOPATH/src/$REPO_NAME
$ ./bin/golangci-lint --version
golangci-lint has version 1.13 built from 2192097 on 2019-01-21T07:58:29Z
$ go version && go env
go version go1.10.7 linux/amd64
GOARCH="amd64"
GOBIN=""
GOCACHE="/root/.cache/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/go"
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
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-build466756837=/tmp/go-build -gno-record-gcc-switches"
$ ./bin/golangci-lint run -v -c .golangci.yml
level=info msg="[config_reader] Used config file .golangci.yml"
level=info msg="[lintersdb] Active 12 linters: [deadcode errcheck gocyclo gofmt gosimple govet ineffassign staticcheck structcheck typecheck unused varcheck]"
level=info msg="[lintersdb] Optimized sublinters [staticcheck gosimple unused] into metalinter megacheck"
level=info msg="[loader] Go packages loading at mode load deps types and syntax took 1.627738567s"
level=info msg="[loader] SSA repr building timing: packages building 7.370708ms, total 258.565576ms"
level=info msg="[runner] worker.4 took 10.488339ms with stages: govet: 10.087872ms, ineffassign: 189.821µs, errcheck: 106.505µs, deadcode: 46.65µs, varcheck: 18.442µs, structcheck: 10.548µs, typecheck: 2.899µs"
level=info msg="[runner] worker.2 took 11.453747ms with stages: gofmt: 11.446615ms"
megacheck.go:5:6: `megachecktest` is unused (deadcode)
func megachecktest() {
   ^
gocritic.go:3:6: `main` is unused (deadcode)
func main() {
   ^
megacheck.go:10: File is not `gofmt`-ed with `-s` (gofmt)

level=info msg="[runner] worker.3 took 25.875192ms with stages: gocyclo: 25.8638ms"
level=info msg="[runner] worker.1 took 803.506129ms with stages: megacheck: 803.474327ms"
level=info msg="[runner] Workers idle times: #2: 789.914941ms, #3: 775.440337ms, #4: 790.913636ms"
level=info msg="[runner] Issues before processing: 9, after processing: 6"
level=info msg="[runner] processing took 438.92µs with stages: cgo: 106.898µs, skip_dirs: 78.001µs, source_code: 76.815µs, autogenerated_exclude: 66.623µs, path_prettifier: 48.496µs, nolint: 31.674µs, uniq_by_line: 7.861µs, max_from_linter: 6.838µs, path_shortener: 5.54µs, max_per_file_from_linter: 5.414µs, max_same_issues: 1.692µs, diff: 1.183µs, exclude: 1.007µs, skip_files: 878ns"
gocritic.go:12:9: empty branch (staticcheck)
  } else if 3 == 4 {
         ^
gocritic.go:14:9: empty branch (staticcheck)
  } else {
         ^
megacheck.go:14:2: empty branch (staticcheck)
  if testFlag == true {
  ^
level=info msg="Memory: 27 samples, avg is 125.9MB, max is 203.6MB"
level=info msg="Execution took 2.768947083s"
ERROR: Job failed: exit code 1

@jirfag
Copy link
Member

jirfag commented Jan 22, 2019

thank you, I will take a look

@tpounds tpounds added bug Something isn't working false negative An error is not reported when one exists labels Oct 1, 2019
@jirfag
Copy link
Member

jirfag commented Oct 15, 2019

go-critic and staticcheck were updated many times after that

@jirfag jirfag closed this as completed Oct 15, 2019
@snyk-bot snyk-bot mentioned this issue Dec 19, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working false negative An error is not reported when one exists
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@tpounds @jirfag @lopezator @jakewarren