|
| 1 | +# Refactor TODOs (Function Length & Complexity) |
| 2 | + |
| 3 | +This list prioritizes high-impact function length and nesting reductions while |
| 4 | +preserving behavior. Do not touch `testdata/**`. |
| 5 | + |
| 6 | +Generated from `golangci-lint run --enable-only funlen,nestif` analysis. |
| 7 | + |
| 8 | +## Agent Prompt (Refactor Workflow) |
| 9 | + |
| 10 | +Use this process when refactoring for length/complexity reduction: |
| 11 | + |
| 12 | +- Work from top priority to lower in this file. |
| 13 | +- Refactor one function at a time using helper extraction and early returns. |
| 14 | +- After each function refactor, run `make unit`. |
| 15 | +- Do not edit `testdata/**` or golden outputs. |
| 16 | +- If a behavior change is needed, add targeted tests (outside `testdata/**`). |
| 17 | +- Before committing: run `make fmt`, verify `git status` only has intended |
| 18 | + changes, then commit. |
| 19 | + |
| 20 | +Commit message format: |
| 21 | + |
| 22 | +- Subject: `package: short desc` (<= 64 chars), use `multi:` if needed. |
| 23 | +- Body: wrap at 72 chars. |
| 24 | + |
| 25 | +Example: |
| 26 | + |
| 27 | +``` |
| 28 | +multi: reduce function length in call formatting |
| 29 | +Extract helpers to reduce function length in call packing and signature |
| 30 | +handling without behavior changes. |
| 31 | +``` |
| 32 | + |
| 33 | +## Output Diff Check (Behavior Drift) |
| 34 | + |
| 35 | +Use `tools/plan_diff.sh` to compare actual formatted output against a baseline. |
| 36 | +The script automatically rebuilds llformat before each run. |
| 37 | + |
| 38 | +Examples: |
| 39 | + |
| 40 | +- Create baseline from ~/work/lnd-origin: |
| 41 | + `tools/plan_diff.sh --baseline /tmp/format-baseline --update ~/work/lnd-origin` |
| 42 | +- Compare current output against baseline: |
| 43 | + `tools/plan_diff.sh --baseline /tmp/format-baseline ~/work/lnd-origin` |
| 44 | +- Skip rebuild (when testing quickly): |
| 45 | + `tools/plan_diff.sh --skip-build --baseline /tmp/format-baseline ~/work/lnd-origin` |
| 46 | + |
| 47 | +--- |
| 48 | + |
| 49 | +# Deep Nesting Issues (nestif complexity >= 4) |
| 50 | + |
| 51 | +These blocks have nesting deeper than 3 levels (if-in-if, if-in-for, etc.). |
| 52 | +Use early returns, helper extraction, or state machines to flatten. |
| 53 | + |
| 54 | +## N0 (Critical: complexity >= 20) |
| 55 | + |
| 56 | +- `formatter/func_sig_formatter.go:1205` in `breakSignature` (complexity: 108!) |
| 57 | + - The `if returns != ""` block has extreme nesting depth. |
| 58 | + - Extract: `formatReturnsBlock`, `formatCanonicalReturns`, |
| 59 | + `formatLegacyReturns`, `handleMultipleReturns`. |
| 60 | +- `formatter/compact_call_formatter.go:1479` in `formatCallPackedMultiLineNext` |
| 61 | + (complexity: 24) |
| 62 | + - The `if llast.IsCallExpr(a)` block for nested call handling. |
| 63 | + - Extract: `handleNestedCallArg`, `formatCallInArg`. |
| 64 | +- `dsl/action.go:3659` in `formatSignatureSimple` (complexity: 23) |
| 65 | + - The `if hasReturns` block for return formatting. |
| 66 | + - Extract: `formatSimpleReturnsBlock`. |
| 67 | +- `dsl/action.go:4090` in `formatMethodSimple` (complexity: 23) |
| 68 | + - The `if hasReturns` block (duplicate logic with above). |
| 69 | + - Extract: `formatMethodReturnsBlock`. |
| 70 | +- `formatter/func_sig_formatter.go:1672` in `formatFuncTypeParam` (complexity: 22) |
| 71 | + - The width-check block for function type parameters. |
| 72 | + - Extract: `breakFuncTypeParam`, `formatFuncTypeMultiline`. |
| 73 | + |
| 74 | +## N1 (High: complexity 10-19) |
| 75 | + |
| 76 | +- `formatter/compact_call_formatter.go:1928` in `formatCallGreedyWithOptions` |
| 77 | + (complexity: 17) |
| 78 | + - The `if i > 0` block for argument processing. |
| 79 | + - Extract: `processGreedyArg`, `handleGreedyLineBreak`. |
| 80 | +- `formatter/func_sig_formatter.go:1127` in `breakSignature` (complexity: 15) |
| 81 | + - The `if needsFuncBreak` block. |
| 82 | + - Extract: `formatFuncTypeParamBreak`. |
| 83 | +- `formatter/func_sig_formatter.go:826` in `formatSignature` (complexity: 14) |
| 84 | + - The `if inlineBodyFound` block for inline function bodies. |
| 85 | + - Extract: `handleInlineBody`, `formatInlineReturn`. |
| 86 | +- `formatter/func_sig_formatter.go:2168` in `formatInterfaceReturns` |
| 87 | + (complexity: 10) |
| 88 | + - The `if strings.HasPrefix(returns, "(")` block. |
| 89 | + - Extract: `formatParenthesizedReturns`. |
| 90 | +- `formatter/compact_call_formatter.go:2317` in `formatCallGreedyWithOptions` |
| 91 | + (complexity: 10) |
| 92 | + - The `if cut <= 0` block for line cutting. |
| 93 | + - Extract: `handleGreedyCut`. |
| 94 | + |
| 95 | +## N2 (Medium: complexity 4-9) |
| 96 | + |
| 97 | +- `dsl/action.go:3618` in `formatSignatureSimple` (complexity: 6) |
| 98 | + - `if needMultiParams` block. |
| 99 | +- `dsl/action.go:4049` in `formatMethodSimple` (complexity: 6) |
| 100 | + - `if needMultiParams` block (duplicate). |
| 101 | +- `formatter/compact_call_formatter.go:2043` (complexity: 6) |
| 102 | + - `if a.kind == argExpr` block. |
| 103 | +- `formatter/compact_call_formatter.go:2282` (complexity: 6) |
| 104 | + - `if cut <= 0 && capCols <= 0` block. |
| 105 | +- `formatter/func_sig_formatter.go:265` (complexity: 5) |
| 106 | + - `if strings.Contains(signature, "\n")` block. |
| 107 | +- `formatter/func_sig_formatter.go:970` (complexity: 5) |
| 108 | + - `if strings.HasPrefix(afterParams, "(")` block. |
| 109 | +- `dsl/action.go:3953` (complexity: 4) |
| 110 | +- `dsl/action.go:4491` (complexity: 4) |
| 111 | +- `dsl/condition.go:1236` (complexity: 4) |
| 112 | +- `formatter/compact_call_formatter.go:2145` (complexity: 4) |
| 113 | +- `formatter/compact_call_formatter.go:3000` (complexity: 4) |
| 114 | +- `formatter/func_sig_formatter.go:415` (complexity: 4) |
| 115 | +- `formatter/func_sig_formatter.go:932` (complexity: 4) |
| 116 | +- `internal/compat/comment_formatter.go:501` (complexity: 4) |
| 117 | + |
| 118 | +--- |
| 119 | + |
| 120 | +# Function Length Issues (funlen) |
| 121 | + |
| 122 | +## Excluded Files (Intentionally Long) |
| 123 | + |
| 124 | +The following files contain declarative rule configurations that are naturally |
| 125 | +long. These are readable as-is and should NOT be refactored for funlen: |
| 126 | + |
| 127 | +- `dsl/rules.go` - Rule definitions are declarative data; splitting would |
| 128 | + reduce readability without improving maintainability. |
| 129 | + |
| 130 | +## P0 (Critical: >100 lines or >80 statements over limit) |
| 131 | + |
| 132 | +These are the largest functions and most urgent to split. |
| 133 | +- `formatter/compact_call_formatter.go:1872: formatCallGreedyWithOptions` |
| 134 | + (170 stmts, limit 40) |
| 135 | + - Extract: `greedyArgState`, `appendGreedyArg`, `emitGreedyLine`, |
| 136 | + `handleGreedyBreak`. |
| 137 | +- `formatter/func_sig_formatter.go:916: breakSignature` (120 stmts, limit 40) |
| 138 | + - Extract: `formatParams`, `formatReturns`, `buildSignature`, |
| 139 | + `handleMultilineReturn`. |
| 140 | +- `tools/fmt_repo/main.go:38: main` (111 lines, limit 60) |
| 141 | + - Extract: `parseFlags`, `collectFiles`, `processFiles`, `reportResults`. |
| 142 | +- `formatter/dsl_bundles_next.go:176: dslRulesForSignatures` (108 lines, |
| 143 | + limit 60) |
| 144 | + - Extract: `funcDeclSignatureRules`, `funcLitSignatureRules`, |
| 145 | + `interfaceMethodRules`. |
| 146 | +- `formatter/compact_call_formatter.go:1352: formatCallPackedMultiLineNext` |
| 147 | + (108 stmts, limit 40) |
| 148 | + - Extract: `packArgsNext`, `emitPackedNextLine`, `handleStringLitNext`. |
| 149 | + |
| 150 | +## P1 (High: 30-100 lines over limit) |
| 151 | + |
| 152 | +- `dsl/action.go:1165: BreakCallArgsLayoutAction.Execute` (95 lines, limit 60) |
| 153 | + - Extract: `resolveCallArgsTarget`, `buildCallArgsLayout`, |
| 154 | + `applyCallArgsEdits`. |
| 155 | +- `dsl/action.go:4199: BlankLinesBatchAction.Execute` (92 lines, limit 60) |
| 156 | + - Extract: `collectBlankLineEdits`, `applyBlankLineEdits`. |
| 157 | +- `formatter/pipeline.go:96: NewPipeline` (91 lines, limit 60) |
| 158 | + - Extract: `applyPipelineDefaults`, `buildStages`, `configureDSLOptions`. |
| 159 | +- `formatter/compact_call_formatter.go:794: formatCallPackedMultiLine` |
| 160 | + (90 stmts, limit 40) |
| 161 | + - Extract: `initPackedCallState`, `packArg`, `emitPackedLine`. |
| 162 | +- `dsl/layout/render.go:167: fitsAt` (90 lines, limit 60) |
| 163 | + - Extract: `fitsAtDoc`, `fitsAtBreak`, `fitsAtGroup`. |
| 164 | +- `formatter/func_sig_formatter.go:1729: formatFuncTypeParam` (88 stmts, |
| 165 | + limit 40) |
| 166 | + - Extract: `parseFuncType`, `formatFuncTypeParams`, `formatFuncTypeReturns`. |
| 167 | +- `dsl/expr_doc.go:467: methodChainSegmentDoc` (84 lines, limit 60) |
| 168 | + - Extract: `segmentCallDoc`, `segmentSelectorDoc`, `segmentBaseDoc`. |
| 169 | +- `dsl/rules.go:887: BlankLineRulesWithOptions` (82 lines, limit 60) |
| 170 | + - Extract: `returnBlankRules`, `caseClauseBlankRules`, `ifErrBlankRules`. |
| 171 | +- `dsl/action.go:3881: BreakInterfaceMethodAction.Execute` (82 lines, limit 60) |
| 172 | + - Extract: `resolveInterfaceMethod`, `formatInterfaceMethod`, |
| 173 | + `maybeInsertBlankAfterMethod`. |
| 174 | +- `cmd/llformat/main.go:29: run` (82 lines, limit 60) |
| 175 | + - Extract: `parseArgs`, `runFormat`, `runPrintPlan`. |
| 176 | +- `dsl/pattern.go:91: matchType` (81 stmts, limit 40) |
| 177 | + - Use a map[string]func(ast.Node)bool or type switch consolidation. |
| 178 | + |
| 179 | +## P2 (Medium: 10-30 lines over limit) |
| 180 | + |
| 181 | +- `cmd/llformat/main.go:223: runPrintPlan` (77 lines, limit 60) |
| 182 | + - Extract: `buildPlanConfig`, `printPlanStages`. |
| 183 | +- `dsl/expr_doc.go:1197: sliceExprDoc` (74 lines, limit 60) |
| 184 | + - Extract: `buildSliceDocs`, `maybeStructuredSliceDocs`. |
| 185 | +- `dsl/expr_doc.go:576: genericCallDoc` (73 lines, limit 60) |
| 186 | + - Extract: `buildCallDocs`, `formatCallLayout`. |
| 187 | +- `formatter/config_validation.go:16: ValidatePipelineConfig` (70 lines, |
| 188 | + limit 60) |
| 189 | + - Extract: `validateStyleOptions`, `validateDSLOptions`. |
| 190 | +- `dsl/rules.go:1298: PackedMultiLineOnlyRulesWithOptions` (69 lines, limit 60) |
| 191 | + - Extract: `packedOnlyCallRules`, `packedOnlyStringRules`. |
| 192 | +- `formatter/func_sig_formatter.go:2161: formatInterfaceReturns` (68 lines, |
| 193 | + limit 60) |
| 194 | + - Extract: `formatSingleReturn`, `formatMultiReturn`. |
| 195 | +- `dsl/action.go:2893: BreakFuncSignatureAction.Execute` (68 lines, limit 60) |
| 196 | + - Extract: `resolveFuncSigTarget`, `formatFuncSignature`, |
| 197 | + `shouldInsertBlankAfterSig`. |
| 198 | +- `formatter/legacy_call_selector.go:67: legacyCallSpansFromAST` (66 lines, |
| 199 | + limit 60) |
| 200 | + - Extract: `collectCallSpans`, `filterLegacySpans`. |
| 201 | +- `formatter/func_sig_formatter.go:1557: splitFuncParamList` (64 lines, |
| 202 | + limit 60) |
| 203 | + - Extract: `scanParamBoundaries`, `splitAtBoundaries`. |
| 204 | +- `dsl/rules.go:2119: MethodChainRules` (64 lines, limit 60) |
| 205 | + - Extract: `chainCallRule`, `chainSelectorRule`. |
| 206 | + |
| 207 | +## P3 (Low: <10 lines over limit or slightly over statement limit) |
| 208 | + |
| 209 | +- `formatter/func_sig_formatter.go:2290: canonicalizeInterfaceMethodParenReturnList` |
| 210 | + (63 lines, limit 60) |
| 211 | +- `internal/compat/comment_formatter.go:540: hoistInlineComments` (62 lines, |
| 212 | + limit 60) |
| 213 | + - Extract: `scanLineForInline`, `rewriteInlineComment`. |
| 214 | +- `internal/compat/comment_formatter.go:375: reflowBlockComment` (61 stmts, |
| 215 | + limit 40) |
| 216 | + - Extract: `blockCommentPrefix`, `reflowBlockWords`. |
| 217 | +- `dsl/action.go:2695: BreakFuncLitSignatureAction.Execute` (59 stmts, limit 40) |
| 218 | + - Extract: `resolveFuncLit`, `formatFuncLitSignature`. |
| 219 | +- `dsl/action.go:3565: formatSignatureSimple` (55 stmts, limit 40) |
| 220 | + - Extract: `formatSimpleParams`, `formatSimpleReturns`. |
| 221 | +- `internal/compat/comment_formatter.go:271: reflowLineCommentBlock` (54 stmts, |
| 222 | + limit 40) |
| 223 | + - Extract: `lineCommentPrefix`, `reflowLineWords`. |
| 224 | +- `formatter/func_sig_formatter.go:718: formatSignature` (51 stmts, limit 40) |
| 225 | + - Extract: `splitSignatureLines`, `formatSignatureParts`. |
| 226 | +- `formatter/compact_call_formatter.go:1716: buildSplitQuotedWithOptions` |
| 227 | + (51 stmts, limit 40) |
| 228 | + - Extract: `splitQuotedLine`, `emitQuotedSegment`. |
| 229 | +- `dsl/action.go:3987: formatMethodSimple` (51 stmts, limit 40) |
| 230 | + - Extract: `formatMethodParams`, `formatMethodReturns`. |
| 231 | +- `dsl/expr_doc.go:696: compositeLitDoc` (45 stmts, limit 40) |
| 232 | + - Extract: `buildCompositeLitDocs`, `formatCompositeLitLayout`. |
| 233 | +- `scanner/split.go:19: splitAtDepthZero` (45 stmts, limit 40) |
| 234 | + - Extract: `trackBracketDepth`, `splitAtZero`. |
| 235 | +- `dsl/action.go:4481: BreakMethodChainAction.Execute` (44 stmts, limit 40) |
| 236 | + - Extract: `resolveChainTarget`, `formatChainBreaks`. |
| 237 | +- `dsl/engine.go:79: Format` (44 stmts, limit 40) |
| 238 | + - Extract: `prepareContext`, `runIterations`, `finalizeFormat`. |
| 239 | +- `dsl/action.go:3291: formatSignatureCompat` (43 stmts, limit 40) |
| 240 | + - Extract: `formatCompatParams`, `formatCompatReturns`. |
| 241 | +- `formatter/dsl_bundles_next.go:102: dslRulesForMultiLineCalls` (41 stmts, |
| 242 | + limit 40) |
| 243 | +- `dsl/func_lit_expand_action.go:19: ExpandFuncLitBodyAction.Execute` |
| 244 | + (41 stmts, limit 40) |
| 245 | +- `formatter/func_sig_formatter.go:536: collapseSignatureWhitespace` |
| 246 | + (41 stmts, limit 40) |
| 247 | +- `dsl/layout/render.go:32: RenderAt` (46 stmts, limit 40) |
| 248 | + - Extract: `renderDoc`, `renderBreak`, `renderGroup`. |
0 commit comments