Including Check Name in megacheck/staticcheck Output

[bflad]

Version

$ golangci-lint --version
golangci-lint has version 1.13.2 built from 7b2421d on 2019-01-26T16:37:12Z

Configuration

linters:
  disable-all: true
  enable:
    - staticcheck

Go Environment

$ go version
go version go1.11.5 darwin/amd64

`go env` output

$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/bflad/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/bflad/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/Cellar/go/1.11.5/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.11.5/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/bflad/src/github.com/terraform-providers/terraform-provider-aws/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/v0/_d108fkx1pbbg4_sh864_7740000gn/T/go-build938551488=/tmp/go-build -gno-record-gcc-switches -fno-common"

Description

Tools in the megacheck/staticcheck family have individual check names (e.g. SA1006, ST1005, etc.) that can be used for referencing the documentation at https://staticcheck.io/docs/checks as well as disabling them via a staticcheck.conf file. These check names are normally output by the underlying commands.

Using https://github.com/terraform-providers/terraform-provider-aws/ by example and removing the staticcheck.conf file so failures show, currently the golangci-lint output omits the check names which can make the remediation process seem more daunting than it actually is:

Current `golangci-lint` Output

$ golangci-lint run ./aws
aws/auth_helpers.go:209:37: session.New is deprecated: Use NewSession functions to create sessions instead. NewSession has the same functionality as New except an error can be returned when the func is called instead of waiting to receive an error until a request is made.  (staticcheck)
		metadataClient := ec2metadata.New(session.New(cfg))
		                                  ^
aws/auth_helpers.go:257:23: session.New is deprecated: Use NewSession functions to create sessions instead. NewSession has the same functionality as New except an error can be returned when the func is called instead of waiting to receive an error until a request is made.  (staticcheck)
	stsclient := sts.New(session.New(awsConfig))
	                     ^
aws/data_source_aws_db_event_categories_test.go:89:35: calling regexp.MatchString in a loop has poor performance, consider using regexp.Compile (staticcheck)
		matched, _ := regexp.MatchString("event_categories.[0-9]+", k)
		                                ^
aws/resource_aws_api_gateway_usage_plan.go:223:3: this value of `up` is never used (staticcheck)
		up, err = conn.UpdateUsagePlan(updateParameters)
		^
aws/resource_aws_cloudtrail.go:255:26: trail.SnsTopicName is deprecated: SnsTopicName has been deprecated  (staticcheck)
	d.Set("sns_topic_name", trail.SnsTopicName)
	                        ^
aws/resource_aws_efs_file_system.go:313:5: this value of `err` is never used (staticcheck)
	_, err := conn.DeleteFileSystem(&efs.DeleteFileSystemInput{
	   ^
aws/resource_aws_elastic_beanstalk_environment.go:927:33: calling regexp.MatchString in a loop has poor performance, consider using regexp.Compile (staticcheck)
		if ok, _ := regexp.MatchString(beanstalkSGRegexp, g); ok {
		                              ^
aws/resource_aws_glue_job.go:118:3: input.AllocatedCapacity is deprecated: This property is deprecated, use MaxCapacity instead.  (staticcheck)
		input.AllocatedCapacity = aws.Int64(int64(v.(int)))
		^
aws/resource_aws_glue_job.go:187:49: job.AllocatedCapacity is deprecated: This property is deprecated, use MaxCapacity instead.  (staticcheck)
	d.Set("allocated_capacity", int(aws.Int64Value(job.AllocatedCapacity)))
	                                               ^
aws/resource_aws_glue_job.go:222:3: jobUpdate.AllocatedCapacity is deprecated: This property is deprecated, use MaxCapacity instead.  (staticcheck)
		jobUpdate.AllocatedCapacity = aws.Int64(int64(v.(int)))
		^
aws/resource_aws_iam_user_login_profile_test.go:213:30: session.New is deprecated: Use NewSession functions to create sessions instead. NewSession has the same functionality as New except an error can be returned when the func is called instead of waiting to receive an error until a request is made.  (staticcheck)
		iamAsCreatedUserSession := session.New(&aws.Config{
		                           ^
aws/resource_aws_kinesis_firehose_delivery_stream.go:2059:4: createInput.S3DestinationConfiguration is deprecated: S3DestinationConfiguration has been deprecated  (staticcheck)
			createInput.S3DestinationConfiguration = s3Config
			^
aws/resource_aws_kinesis_firehose_delivery_stream.go:2193:4: updateInput.S3DestinationUpdate is deprecated: S3DestinationUpdate has been deprecated  (staticcheck)
			updateInput.S3DestinationUpdate = s3Config
			^
aws/resource_aws_kms_grant.go:545:16: infinite recursive call (staticcheck)
	return e.Error()
	              ^
aws/resource_aws_neptune_cluster_instance.go:340:31: db.PubliclyAccessible is deprecated: PubliclyAccessible has been deprecated  (staticcheck)
	d.Set("publicly_accessible", db.PubliclyAccessible)
	                             ^
aws/resource_aws_route.go:352:2: this value of `replaceOpts` is never used (staticcheck)
	replaceOpts := &ec2.ReplaceRouteInput{}
	^
aws/resource_aws_s3_bucket.go:1033:8: lifecycleRule.Prefix is deprecated: Prefix has been deprecated  (staticcheck)
				if lifecycleRule.Prefix != nil {
				   ^
aws/resource_aws_s3_bucket.go:1034:24: lifecycleRule.Prefix is deprecated: Prefix has been deprecated  (staticcheck)
					rule["prefix"] = *lifecycleRule.Prefix
					                  ^
aws/resource_aws_s3_bucket.go:1949:4: rcRule.Prefix is deprecated: Prefix has been deprecated  (staticcheck)
			rcRule.Prefix = aws.String(rr["prefix"].(string))
			^
aws/resource_aws_s3_bucket.go:2201:6: v.Prefix is deprecated: Prefix has been deprecated  (staticcheck)
		if v.Prefix != nil {
		   ^
aws/resource_aws_s3_bucket.go:2202:19: v.Prefix is deprecated: Prefix has been deprecated  (staticcheck)
			t["prefix"] = *v.Prefix
			               ^
aws/resource_aws_service_discovery_service.go:248:2: result.NamespaceId is deprecated: Top level attribute in request should be used to reference namespace-id  (staticcheck)
	result.NamespaceId = aws.String(configured["namespace_id"].(string))
	^
aws/resource_aws_service_discovery_service.go:270:28: config.NamespaceId is deprecated: Top level attribute in request should be used to reference namespace-id  (staticcheck)
	result["namespace_id"] = *config.NamespaceId
	                          ^
aws/resource_aws_spot_fleet_request.go:1155:2: empty branch (staticcheck)
	if err == nil && aws.BoolValue(resp.Return) {
	^

Proposal

It would be great if we could get the check names as well in the output. e.g.

Proposed `golangci-lint` Output

$ ~/src/github.com/golangci/golangci-lint/golangci-lint run ./aws
aws/auth_helpers.go:209:37: session.New is deprecated: Use NewSession functions to create sessions instead. NewSession has the same functionality as New except an error can be returned when the func is called instead of waiting to receive an error until a request is made.  (SA1019) (staticcheck)
		metadataClient := ec2metadata.New(session.New(cfg))
		                                  ^
aws/auth_helpers.go:257:23: session.New is deprecated: Use NewSession functions to create sessions instead. NewSession has the same functionality as New except an error can be returned when the func is called instead of waiting to receive an error until a request is made.  (SA1019) (staticcheck)
	stsclient := sts.New(session.New(awsConfig))
	                     ^
aws/data_source_aws_db_event_categories_test.go:89:35: calling regexp.MatchString in a loop has poor performance, consider using regexp.Compile (SA6000) (staticcheck)
		matched, _ := regexp.MatchString("event_categories.[0-9]+", k)
		                                ^
aws/resource_aws_api_gateway_usage_plan.go:223:3: this value of up is never used (SA4006) (staticcheck)
		up, err = conn.UpdateUsagePlan(updateParameters)
		^
aws/resource_aws_cloudtrail.go:255:26: trail.SnsTopicName is deprecated: SnsTopicName has been deprecated  (SA1019) (staticcheck)
	d.Set("sns_topic_name", trail.SnsTopicName)
	                        ^
aws/resource_aws_efs_file_system.go:313:5: this value of err is never used (SA4006) (staticcheck)
	_, err := conn.DeleteFileSystem(&efs.DeleteFileSystemInput{
	   ^
aws/resource_aws_elastic_beanstalk_environment.go:927:33: calling regexp.MatchString in a loop has poor performance, consider using regexp.Compile (SA6000) (staticcheck)
		if ok, _ := regexp.MatchString(beanstalkSGRegexp, g); ok {
		                              ^
aws/resource_aws_glue_job.go:118:3: input.AllocatedCapacity is deprecated: This property is deprecated, use MaxCapacity instead.  (SA1019) (staticcheck)
		input.AllocatedCapacity = aws.Int64(int64(v.(int)))
		^
aws/resource_aws_glue_job.go:187:49: job.AllocatedCapacity is deprecated: This property is deprecated, use MaxCapacity instead.  (SA1019) (staticcheck)
	d.Set("allocated_capacity", int(aws.Int64Value(job.AllocatedCapacity)))
	                                               ^
aws/resource_aws_glue_job.go:222:3: jobUpdate.AllocatedCapacity is deprecated: This property is deprecated, use MaxCapacity instead.  (SA1019) (staticcheck)
		jobUpdate.AllocatedCapacity = aws.Int64(int64(v.(int)))
		^
aws/resource_aws_iam_user_login_profile_test.go:213:30: session.New is deprecated: Use NewSession functions to create sessions instead. NewSession has the same functionality as New except an error can be returned when the func is called instead of waiting to receive an error until a request is made.  (SA1019) (staticcheck)
		iamAsCreatedUserSession := session.New(&aws.Config{
		                           ^
aws/resource_aws_kinesis_firehose_delivery_stream.go:2059:4: createInput.S3DestinationConfiguration is deprecated: S3DestinationConfiguration has been deprecated  (SA1019) (staticcheck)
			createInput.S3DestinationConfiguration = s3Config
			^
aws/resource_aws_kinesis_firehose_delivery_stream.go:2193:4: updateInput.S3DestinationUpdate is deprecated: S3DestinationUpdate has been deprecated  (SA1019) (staticcheck)
			updateInput.S3DestinationUpdate = s3Config
			^
aws/resource_aws_kms_grant.go:545:16: infinite recursive call (SA5007) (staticcheck)
	return e.Error()
	              ^
aws/resource_aws_neptune_cluster_instance.go:340:31: db.PubliclyAccessible is deprecated: PubliclyAccessible has been deprecated  (SA1019) (staticcheck)
	d.Set("publicly_accessible", db.PubliclyAccessible)
	                             ^
aws/resource_aws_route.go:352:2: this value of replaceOpts is never used (SA4006) (staticcheck)
	replaceOpts := &ec2.ReplaceRouteInput{}
	^
aws/resource_aws_s3_bucket.go:1033:8: lifecycleRule.Prefix is deprecated: Prefix has been deprecated  (SA1019) (staticcheck)
				if lifecycleRule.Prefix != nil {
				   ^
aws/resource_aws_s3_bucket.go:1034:24: lifecycleRule.Prefix is deprecated: Prefix has been deprecated  (SA1019) (staticcheck)
					rule["prefix"] = *lifecycleRule.Prefix
					                  ^
aws/resource_aws_s3_bucket.go:1949:4: rcRule.Prefix is deprecated: Prefix has been deprecated  (SA1019) (staticcheck)
			rcRule.Prefix = aws.String(rr["prefix"].(string))
			^
aws/resource_aws_s3_bucket.go:2201:6: v.Prefix is deprecated: Prefix has been deprecated  (SA1019) (staticcheck)
		if v.Prefix != nil {
		   ^
aws/resource_aws_s3_bucket.go:2202:19: v.Prefix is deprecated: Prefix has been deprecated  (SA1019) (staticcheck)
			t["prefix"] = *v.Prefix
			               ^
aws/resource_aws_service_discovery_service.go:248:2: result.NamespaceId is deprecated: Top level attribute in request should be used to reference namespace-id  (SA1019) (staticcheck)
	result.NamespaceId = aws.String(configured["namespace_id"].(string))
	^
aws/resource_aws_service_discovery_service.go:270:28: config.NamespaceId is deprecated: Top level attribute in request should be used to reference namespace-id  (SA1019) (staticcheck)
	result["namespace_id"] = *config.NamespaceId
	                          ^
aws/resource_aws_spot_fleet_request.go:1155:2: empty branch (SA9003) (staticcheck)
	if err == nil && aws.BoolValue(resp.Return) {
	^

The above was enabled with a change on this line:

golangci-lint/pkg/golinters/megacheck.go

Line 262 in 7b2421d

Text: markIdentifiers(i.Text),

From i.Text to i.String(). For completeness, I'm guessing this would also require updates to the following regular expression text replacement code to handle the new +\(\w+\) at the end:

golangci-lint/pkg/golinters/util.go

Lines 46 to 93 in 7b2421d

	var replacePatterns = []replacePattern{
	// unparam
	{`^(\S+) - (\S+) is unused$`, "`${1}` - `${2}` is unused"},
	{`^(\S+) - (\S+) always receives (\S+) \((.*)\)$`, "`${1}` - `${2}` always receives `${3}` (`${4}`)"},
	{`^(\S+) - (\S+) always receives (.*)$`, "`${1}` - `${2}` always receives `${3}`"},
	// interfacer
	{`^(\S+) can be (\S+)$`, "`${1}` can be `${2}`"},
	// govet
	{`^(\S+) arg list ends with redundant newline$`, "`${1}` arg list ends with redundant newline"},
	{`^(\S+) composite literal uses unkeyed fields$`, "`${1}` composite literal uses unkeyed fields"},
	// gosec
	{`^Blacklisted import (\S+): weak cryptographic primitive$`,
	"Blacklisted import `${1}`: weak cryptographic primitive"},
	{`^TLS InsecureSkipVerify set true.$`, "TLS `InsecureSkipVerify` set true."},
	// gosimple
	{`^should replace loop with (.*)$`, "should replace loop with `${1}`"},
	// megacheck
	{`^this value of (\S+) is never used$`, "this value of `${1}` is never used"},
	{`^should use time.Since instead of time.Now().Sub$`,
	"should use `time.Since` instead of `time.Now().Sub`"},
	{`^(func|const|field|type) (\S+) is unused$`, "${1} `${2}` is unused"},
	}
	func markIdentifiers(s string) string {
	replaceRegexpsOnce.Do(func() {
	for _, p := range replacePatterns {
	r := replaceRegexp{
	re: regexp.MustCompile(p.re),
	repl: p.repl,
	}
	replaceRegexps = append(replaceRegexps, r)
	}
	})
	for _, rr := range replaceRegexps {
	rs := rr.re.ReplaceAllString(s, rr.repl)
	if rs != s {
	return rs
	}
	}
	return s
	}

Theoretically, this new behavior could be behind a configuration flag if the output change is considered breaking.

If this change is acceptable (in any form), I can certainly submit the pull request, but wanted to verify that this would be welcome first. Thanks for your consideration and appreciate the nice tooling!

[jirfag]

hi!
thank you for the suggestion, I've prepended check names:

$ golangci-lint run test/testdata/gosimple.go
test/testdata/gosimple.go:7:2: S1017: should replace this `if` statement with an unconditional `strings.TrimPrefix` (gosimple)
        if strings.HasPrefix(id1, s1) { // ERROR "should replace.*with.*strings.TrimPrefix"
        ^

[bflad]

Thanks, @jirfag!