Go-tools: Investigate go-critic checks

[dominikh]

https://go-critic.github.io/overview
Last update (of this issue): Wed Jan 13 05:49:01 AM CET 2021

Consider implementing

• appendAssign
• appendCombine
• argOrder
• boolExprSimplify - quickfix, dup of #146
• captLocal - dup of #568
• deadCodeAfterLogFatal - dup of #111
• dupArg - staticcheck
• dupBranchBody - staticcheck, dup of #174
• dupCase - gosimple
• elseif - stylecheck or gosimple
• evalOrder - staticcheck, dup of #258
• exitAfterDefer - staticcheck
• flagDeref - staticcheck
• flagName - staticcheck
• hexLiteral - duplicate of #397
• ifElseChain - dup of #6
• importShadow - non-default stylecheck
• indexAlloc
• indexOnlyLoop - gosimple
• namedConst - stylecheck
• nilValReturn - staticcheck
• offBy1 - staticcheck
• ptrToRefParam
• regexpMust - stylecheck
• singleCaseSwitch - stylecheck or gosimple
• sloppyLen - staticcheck
• stringXbytes - staticcheck
• switchTrue - gosimple
• typeAssertChain
• underef - gosimple
• unlambda - dup of #123
• unnamedResult - interesting idea, but hard to implement as an automatic check without being noisy. might be better as an editor-only check
• unslice
• valSwap
• weakCond - staticcheck
• newDeref
• truncateCmp
• builtinShadowDecl
• deferUnlambda
• regexpSimplify
• sloppyTypeAssert

Already implemented

• caseOrder
• defaultCaseOrder
• typeSwitchVar
• yodaStyleExpr
• equalFold
• dupSubExpr
• badCall
• dupImport
• badLock

Not desirable

• builtinShadow
• initClause
• docStub
• emptyFallthrough
• emptyStringTest
• rangeExprCopy
• rangeValCopy
• octalLiteral
• commentFormatting
• commentedOutImport
• paramTypeCombine
• codegenComment
• commentedOutCode
• assignOp
• sloppyReassign
• wrapperFunc
• typeUnparen
• nestingReduce
• hugeParam
• badCond
• deprecatedComment
• methodExprCall
• unlabelStmt
• unnecessaryBlock
• regexpPattern
• whyNoLint
• mapKey
• badRegexp
• ruleguard
• sortSlice
• sqlQuery
• unnecessaryDefer

Undecided

• filepathJoin
• typeDefFirst

[quasilyte]

I think initClause fits stylecheck more, because it's not really a simplification, but rather an effort of avoiding code with surprises.

[quasilyte]

singleCaseSwitch can benefit from TODO comments checks.
Some switches with a single case have something like: "TODO: cover all things later".
These may be better left alone, at least by default.
This case is not handled by gocritic since it's not really convenient to match comments (non-doc comments) to arbitrary statement.

[quasilyte]

elseif finds many cases where code can be less beautiful when re-written.
See https://github.com/go-critic/go-critic/issues/420.
I don't know whether my approach gives good enough reduce in these false-positive like cases.
There can be issues in determining 100%-fine replacements.