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

Revise funlen configurations #796

Closed
hollowaykeanho opened this issue Oct 8, 2019 · 4 comments
Closed

Revise funlen configurations #796

hollowaykeanho opened this issue Oct 8, 2019 · 4 comments
Labels
dependencies Relates to an upstream dependency false positive An error is reported when one does not exist

Comments

@hollowaykeanho
Copy link

hollowaykeanho commented Oct 8, 2019

Issue Requirements

Thank you for creating the issue!

Please include the following information:

  1. Version of golangci-lint: golangci-lint --version (or git commit if you don't use binary distribution)
    golangci-lint has version v1.19.1

  2. Config file: cat .golangci.yml
    Not applicable

  3. Go environment: go version && go env

GO111MODULE=""
GOARCH="amd64"
GOBIN="/home/u0/bin"
GOCACHE="/home/u0/.cache/go-build"
GOENV="/home/u0/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/u0"
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="/home/u0/Documents/gosandbox/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-build509950643=/tmp/go-build -gno-record-gcc-switches"
  1. Verbose output of running: golangci-lint run -v
    Not applicable

Problem

I getting a lot of funlen false positive and this linter's configurations make no sense. The command I use on daily basis is:

$ golangci-lint run --color never --enable-all "$dirpath"

Now I have to alter it to either:

$ golangci-lint run --color never --enable-all --disable=funlen "$dirpath"
OR
create a configuration file that set `funlen` instead of
using golang-lint off the shelves.
OR
commit new codes all over the places with //nolint:funlen

Reason for Changes

package float

//nolint:funlen
func testENDECScenarios() []testENDECScenario {
	return []testENDECScenario{
		{
			UID:      1,
			TestType: testToString,
			Description: `
Float.ToString is able to convert:
1. positive float32
2. positive accuracy
3. to Normal format
`,
			Switches: map[string]bool{
				useNormalFormat:     true,
				usePositiveFloat32:  true,
				usePositiveAccuracy: true,
			},
		}, {
			UID:      2,
			TestType: testToString,
			Description: `
Float.ToString is able to convert:
1. zero float32
2. positive accuracy
3. to Normal format
`,
			Switches: map[string]bool{
				useNormalFormat:     true,
				useZeroFloat32:      true,
				usePositiveAccuracy: true,
			},
		}, {
			UID:      3,
			TestType: testToString,
			Description: `
Float.ToString is able to convert:
1. negative good float32
2. positive accuracy
3. to Normal format
`,
			Switches: map[string]bool{
				useNormalFormat:     true,
				useNegativeFloat32:  true,
				usePositiveAccuracy: true,
			},
		}, {
			UID:      4,
			TestType: testToString,
			Description: `
Float.ToString is able to convert:
1. none
2. positive accuracy
3. to Normal format
`,
			Switches: map[string]bool{
				useNormalFormat:     true,
				usePositiveAccuracy: true,
			},
		}, {
			UID:      5,
			TestType: testToString,
			Description: `
Float.ToString is able to convert:
1. positive float32
2. zero accuracy
3. to Normal format
`,
			Switches: map[string]bool{
				useNormalFormat:    true,
				usePositiveFloat32: true,
				useZeroAccuracy:    true,
			},
		}, {
			UID:      6,
			TestType: testToString,
			Description: `
Float.ToString is able to convert:
1. positive float32
2. negative accuracy
3. to Normal format
`,
			Switches: map[string]bool{
				useNormalFormat:     true,
				usePositiveFloat32:  true,
				useNegativeAccuracy: true,
			},
		}, {
			UID:      7,
			TestType: testToString,
			Description: `
Float.ToString is able to convert:
1. positive float32
2. positive accuracy
3. to Scientific format
`,
			Switches: map[string]bool{
				useScientificFormat: true,
				usePositiveFloat32:  true,
				usePositiveAccuracy: true,
			},
		}, {
			UID:      8,
			TestType: testToString,
			Description: `
Float.ToString is able to convert:
1. zero float32
2. positive accuracy
3. to Scientific format
`,
			Switches: map[string]bool{
				useScientificFormat: true,
				useZeroFloat32:      true,
				usePositiveAccuracy: true,
			},
		}, {
			UID:      9,
			TestType: testToString,
			Description: `
Float.ToString is able to convert:
1. negative good float32
2. positive accuracy
3. to Scientific format
`,
			Switches: map[string]bool{
				useScientificFormat: true,
				useNegativeFloat32:  true,
				usePositiveAccuracy: true,
			},
...
}

This is a generator function that generates configurations data structure slices. It is obviously went over the limit of 60. Keeping it outside the function means the encouragement of using global variable, which is a bigger concern that keeping a function to a fix 60 length, 40 statements rule.

Some logical functions can easily go beyond 60 like https://github.com/kubernetes/kubernetes/blob/master/pkg/controller/garbagecollector/garbagecollector.go#L164.
By setting 60:40, the tool encourages programmers to spin multiple unnecessary small functions consisting of multiple functions OR forcing them not to tidy up their codes (E.g. braces management).

It takes a passionate Go programmer to the extend of altering the configurations so it will generate a lot of unnecessary noises between the new comers and seasoned developers.


Suggestion

  1. Do add heuristic analysis against the function natively OR
  2. Do not fix some brules to 60 lines, 40 statements without thorough research about practical meta-programming and take some actions like:
    2.1. Study and revise the brules with thorough discussions and data-driven.
    2.2. Setting the linter to only provide information, not warning / action-required.
@hollowaykeanho hollowaykeanho changed the title Increase funlen configurations Revise funlen configurations Oct 8, 2019
@jirfag
Copy link
Member

jirfag commented Oct 8, 2019

Thank you. I think this issue will be better to have in funlen repository

@hollowaykeanho
Copy link
Author

Issue transferred to this repo: ultraware/funlen#2.

@jirfag
Copy link
Member

jirfag commented Oct 8, 2019

Thank you

@tpounds tpounds added dependencies Relates to an upstream dependency false positive An error is reported when one does not exist labels Oct 8, 2019
@hollowaykeanho
Copy link
Author

hollowaykeanho commented Oct 9, 2019

Thank you

Welcome.

theckman pushed a commit to theckman/golangci-lint that referenced this issue May 3, 2020
$ git cherry --abbrev -v 0af0999fabfb ee9bf5809ead
+ abd8436 all: enable Go modules on CI (golangci#753)
+ 3c9d0fb checkers: recognize //line and //nolint in commentFormatting (golangci#756)
+ 0b517d7 checkers: extend deprecatedComment patterns (golangci#757)
+ 09100f6 checkers: use astcast package instead of coerce.go (golangci#758)
+ 2e9e97f checker: simplify boolExprSimplify (golangci#759)
+ 575701e make: add go-consistent to CI checks list (golangci#761)
+ b55f431 checkers: fix unlambda handling of builtins (golangci#763)
+ 5a7dee3 checker: handle lambdas properly in boolExprSimplify (golangci#765)
+ 5ce3939 checkers: teach boolExprSimplify a few new tricks (golangci#766)
+ 04d160f checkers: add new patterns to boolExprSimplify (golangci#768)
+ 09582e2 make: collect coverprofile separately from goveralls (golangci#769)
+ d8d0ee4 checkers: recognize NOTE pattern in deprecatedComment (golangci#770)
+ 12f0f85 Update copyright notice to 2019 (golangci#771)
+ f54bdb6 checkers: add stringXbytes checker
+ 170d65c checkers: followup for golangci#773 (golangci#774)
+ 84e9e83 checkers: make stringXbytes more linear (golangci#775)
+ a800815 checkers: add Depreacted typo pattern (golangci#776)
+ 6751be9 checkers: add hexLiterals (golangci#772)
+ ac61906 checkers: add typeAssertChain checker (golangci#782)
+ d19dbf1 checkers: add codegenComment checker (golangci#783)
+ d82b576 checkers: proper pkg/obj check for flagName (golangci#786)
+ dfcf754 ci: enable integration tests (golangci#787)
+ 5dafc45 checkers: fix equalFold false positive (golangci#788)
+ ed5e8e7 checkers: refactor and fix hexLiteral checker (golangci#789)
+ e704e07 checkers: add argOrder checker (golangci#790)
+ 34c1dc8 checkers: add Split handling to argOrder checker (golangci#791)
+ cbe095d checkers: add math.Max and math.Min to dupArg (golangci#792)
+ c986ee5 checkers: add checkers info fields test (golangci#794)
+ 66e5832 cmd/makedocs: use lintpack, fix build (golangci#793)
+ 6bce9d0 cmd/makedocs: add enabled/disabled by default info (golangci#795)
+ 4adbf9a checkers: simplify flagName (golangci#799)
+ 07de34a checkers: add octalLiteral checker (golangci#798)
+ 765907a cmd/makedocs: add checker param docs (golangci#796)
+ ee9bf58 cmd/makedocs: fix headers formatting (golangci#803)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Relates to an upstream dependency false positive An error is reported when one does not exist
Projects
None yet
Development

No branches or pull requests

3 participants