Skip to content

Commit 64b1e77

Browse files
committed
overflow: add report tool and reduce overflows
1 parent e1b80d0 commit 64b1e77

12 files changed

Lines changed: 1621 additions & 161 deletions

cmd/llformat/main.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,8 @@ func runPrintLogCallsPatterns(w io.Writer, cfg formatter.PipelineConfig) int {
262262
selectorNames = dsl.LogPrintfSelectorNames()
263263
}
264264
fmt.Fprintf(
265-
w, "- printf-style selector names (effective): %s\n", strings.Join(
265+
w, "- printf-style selector names (effective): %s\n",
266+
strings.Join(
266267
selectorNames, ", ",
267268
),
268269
)

formatter/compact_call_formatter_test.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -135,17 +135,16 @@ func TestFormatCallPackedMultiLineNext_BreaksBeforeMultilineCallArg(
135135
out := FormatCallPackedMultiLineNext(call, "\t", 80, 8)
136136

137137
require.NotContains(
138-
t, out,
139-
"secondArgumentExtraYY, innerCallWithExtraLongName[",
138+
t, out, "secondArgumentExtraYY, innerCallWithExtraLongName[",
140139
)
141140

142141
for _, line := range strings.Split(out, "\n") {
143142
if line == "" {
144143
continue
145144
}
146145
require.LessOrEqual(
147-
t, llwidth.VisualLenWithTab(line, 8), 80,
148-
"line exceeds configured column limit: %q", line,
146+
t, llwidth.VisualLenWithTab(line, 8), 80, "line "+
147+
"exceeds configured column limit: %q", line,
149148
)
150149
}
151150
}

formatter/expression_formatter.go

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,9 @@ func formatKeyedCompositeLiteral(before, contIndent string,
6868
b.WriteString("{")
6969
b.WriteByte('\n')
7070
innerIndent := contIndent + "\t"
71+
if shouldOutdentCompositeElems(contIndent, innerIndent, elems) {
72+
innerIndent = contIndent
73+
}
7174

7275
// Keyed literals (map/struct): one entry per line with trailing comma.
7376
for _, e := range elems {
@@ -87,6 +90,52 @@ func formatKeyedCompositeLiteral(before, contIndent string,
8790
return b.String()
8891
}
8992

93+
func shouldOutdentCompositeElems(contIndent, innerIndent string,
94+
elems []string) bool {
95+
96+
if visualLen(contIndent) >= visualLen(innerIndent) {
97+
return false
98+
}
99+
maxInner := 0
100+
maxOutdent := 0
101+
for _, e := range elems {
102+
t := strings.TrimSpace(e)
103+
if t == "" {
104+
continue
105+
}
106+
if l := maxLineLenWithIndentAndComma(t, innerIndent); l > maxInner {
107+
maxInner = l
108+
}
109+
if l := maxLineLenWithIndentAndComma(t, contIndent); l > maxOutdent {
110+
maxOutdent = l
111+
}
112+
}
113+
114+
return maxInner > columnLimit && maxOutdent <= columnLimit
115+
}
116+
117+
func maxLineLenWithIndentAndComma(elem, indent string) int {
118+
lines := strings.Split(elem, "\n")
119+
if len(lines) == 0 {
120+
return visualLen(indent) + 1
121+
}
122+
maxLen := 0
123+
for i, line := range lines {
124+
lineLen := visualLen(line)
125+
if i == 0 {
126+
lineLen += visualLen(indent)
127+
}
128+
if i == len(lines)-1 {
129+
lineLen++
130+
}
131+
if lineLen > maxLen {
132+
maxLen = lineLen
133+
}
134+
}
135+
136+
return maxLen
137+
}
138+
90139
func formatSliceCompositeLiteral(arg, before, contIndent string,
91140
elems []string) (string, bool) {
92141

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
package formatter
2+
3+
import (
4+
"strings"
5+
"testing"
6+
7+
"github.com/stretchr/testify/require"
8+
)
9+
10+
func TestFormatCompositeLiteralArg_OutdentsWhenIndentOverflow(t *testing.T) {
11+
formatGlobalsMu.Lock()
12+
oldColumnLimit := columnLimit
13+
oldTabStop := tabStop
14+
defer func() {
15+
columnLimit = oldColumnLimit
16+
tabStop = oldTabStop
17+
formatGlobalsMu.Unlock()
18+
}()
19+
20+
columnLimit = 40
21+
tabStop = 8
22+
23+
arg := "Item{Index: int64(1)}"
24+
contIndent := "\t\t\t"
25+
out, ok := FormatCompositeLiteralArg(arg, contIndent)
26+
require.True(t, ok)
27+
require.Contains(t, out, "\n Index: "+
28+
"int64(1),\n }")
29+
require.NotContains(t, out, "\n\t\t\t\tIndex:")
30+
31+
for _, line := range strings.Split(out, "\n") {
32+
require.LessOrEqual(t, visualLen(line), columnLimit)
33+
}
34+
}

formatter/func_sig_formatter.go

Lines changed: 134 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,9 @@ func FormatFuncSignatureNext(signature, indent string, colLimit,
311311
formatted = collapseMultilineParenReturnListIfFits(
312312
formatted, colLimit, tabStop,
313313
)
314+
formatted = breakLongTypeArgListsIfNeeded(
315+
formatted, colLimit, tabStop,
316+
)
314317
needsBlank := hasNewlineOutsideBraces(formatted)
315318

316319
return formatted, needsBlank
@@ -405,6 +408,136 @@ func collapseMultilineParenReturnListIfFits(signature string, colLimit,
405408
return signature
406409
}
407410

411+
func breakLongTypeArgListsIfNeeded(signature string, colLimit,
412+
tabStop int) string {
413+
414+
for {
415+
lines := strings.Split(signature, "\n")
416+
changed := false
417+
418+
for i, line := range lines {
419+
if width.VisualLenWithTab(line, tabStop) <= colLimit {
420+
continue
421+
}
422+
423+
indent := leadingWhitespace(line)
424+
rewritten, ok := breakFirstTypeArgListInLine(
425+
line, indent,
426+
)
427+
if !ok {
428+
continue
429+
}
430+
431+
newLines := append([]string{}, lines[:i]...)
432+
newLines = append(
433+
newLines, strings.Split(rewritten, "\n")...,
434+
)
435+
newLines = append(newLines, lines[i+1:]...)
436+
437+
signature = strings.Join(newLines, "\n")
438+
changed = true
439+
break
440+
}
441+
442+
if !changed {
443+
return signature
444+
}
445+
}
446+
}
447+
448+
func breakFirstTypeArgListInLine(line, indent string) (string, bool) {
449+
b := []byte(line)
450+
451+
for i := 0; i < len(b); {
452+
switch {
453+
case scanner.IsStringStart(b, i):
454+
i = scanner.ScanString(b, i)
455+
456+
case scanner.IsLineCommentStart(b, i):
457+
return line, false
458+
459+
case scanner.IsBlockCommentStart(b, i):
460+
i = scanner.ScanBlockComment(b, i)
461+
462+
case b[i] == '[':
463+
if !isTypeArgListStart(line, i) {
464+
i++
465+
continue
466+
}
467+
end := scanner.ScanBalanced(b, i, '[', ']')
468+
if end == -1 {
469+
return line, false
470+
}
471+
472+
content := strings.TrimSpace(line[i+1 : end])
473+
parts := filterNonEmptyTrimmed(
474+
scanner.SplitTopLevelAny(content),
475+
)
476+
if len(parts) <= 1 {
477+
i++
478+
continue
479+
}
480+
481+
var out strings.Builder
482+
out.Grow(len(line) + len(parts)*4)
483+
out.WriteString(line[:i+1])
484+
out.WriteByte('\n')
485+
contIndent := indent + "\t"
486+
for _, part := range parts {
487+
out.WriteString(contIndent)
488+
out.WriteString(strings.TrimSpace(part))
489+
out.WriteString(",\n")
490+
}
491+
out.WriteString(indent)
492+
out.WriteByte(']')
493+
out.WriteString(line[end+1:])
494+
495+
return out.String(), true
496+
497+
default:
498+
i++
499+
}
500+
}
501+
502+
return line, false
503+
}
504+
505+
func isTypeArgListStart(line string, idx int) bool {
506+
if idx <= 0 || idx >= len(line) || line[idx] != '[' {
507+
return false
508+
}
509+
510+
j := idx - 1
511+
for j >= 0 && (line[j] == ' ' || line[j] == '\t') {
512+
j--
513+
}
514+
if j < 0 {
515+
return false
516+
}
517+
if line[j] == ']' {
518+
return true
519+
}
520+
if line[j] == '.' {
521+
k := j - 1
522+
for k >= 0 && isIdentChar(line[k]) {
523+
k--
524+
}
525+
526+
return k != j-1
527+
}
528+
if !isIdentChar(line[j]) {
529+
return false
530+
}
531+
532+
k := j
533+
for k >= 0 && isIdentChar(line[k]) {
534+
k--
535+
}
536+
ident := line[k+1 : j+1]
537+
538+
return ident != "map"
539+
}
540+
408541
func isFuncLitSignature(signature string) bool {
409542
trimmed := strings.TrimSpace(signature)
410543
// Signatures passed to the formatter may include non-whitespace
@@ -1558,7 +1691,7 @@ func (f *FuncSigFormatter) findMatchingParen(s string, start int) int {
15581691
// splitParams splits parameter list by commas, respecting nested
15591692
// parens/brackets.
15601693
func (f *FuncSigFormatter) splitParams(params string) []string {
1561-
return scanner.SplitTopLevel(params)
1694+
return scanner.SplitTopLevelAny(params)
15621695
}
15631696

15641697
func (f *FuncSigFormatter) splitFuncParamList(params string) []string {
Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
package formatter
2+
3+
import (
4+
"go/ast"
5+
"go/parser"
6+
"go/token"
7+
"strings"
8+
"testing"
9+
10+
"github.com/bhandras/llformat/width"
11+
"github.com/stretchr/testify/require"
12+
)
13+
14+
func TestFormatFuncSigs_GenericReturnListDoesNotOverflow(t *testing.T) {
15+
const src = `package p
16+
17+
type RecvXXXXXX[T1, T2 any] struct{}
18+
type RetXXXXXXXXXXXXXXXXXXXXXXXXXXX[T1, T2 any] interface{}
19+
type P interface{}
20+
21+
func (s *RecvXXXXXX[T1, T2]) Select(p P) (RetXXXXXXXXXXXXXXXXXXXXXXXXXXX[T1, T2], error) {
22+
return nil, nil
23+
}
24+
`
25+
26+
out, changed := FormatFuncSigsInSource([]byte(src), 80, 8)
27+
if !changed {
28+
out = []byte(src)
29+
}
30+
31+
for _, line := range strings.Split(string(out), "\n") {
32+
if line == "" {
33+
continue
34+
}
35+
require.LessOrEqual(
36+
t, width.VisualLenWithTab(line, 8), 80, "line "+
37+
"exceeds configured column limit: %q", line,
38+
)
39+
}
40+
}
41+
42+
func TestBreakLongTypeArgListsIfNeeded_BreaksReceiver(t *testing.T) {
43+
const sig = "func (r *ReceiverXXXXXXXXXXXXXXXXXXXXXXXXXXXX[T1, T2, " +
44+
"T3]) VeryLongMethodNameForSig() " +
45+
"ReturnXXXXXXXXXXXXXXXXXXXXXXXXXXXX[T1, T2, T3] {"
46+
47+
out := breakLongTypeArgListsIfNeeded(sig, 80, 8)
48+
49+
require.Contains(
50+
t, out, "ReceiverXXXXXXXXXXXXXXXXXXXXXXXXXXXX["+
51+
"\n T1,\n T2,\n T3,\n]",
52+
)
53+
}
54+
55+
func TestFormatFuncSignatureNext_BreaksGenericTypeArgs(t *testing.T) {
56+
const sig = "func (r *ReceiverXXXXXXXXXXXXXXXXXXXXXXXXXXXX[T1, T2, " +
57+
"T3]) VeryLongMethodNameForSig() " +
58+
"ReturnXXXXXXXXXXXXXXXXXXXXXXXXXXXX[T1, T2, T3] {"
59+
60+
out, _ := FormatFuncSignatureNext(sig, "", 80, 8)
61+
62+
require.Contains(
63+
t, out, "ReceiverXXXXXXXXXXXXXXXXXXXXXXXXXXXX["+
64+
"\n T1,\n T2,\n T3,\n]",
65+
)
66+
}
67+
68+
func TestFormatFuncSignatureNext_WithASTSignature(t *testing.T) {
69+
const src = `package p
70+
71+
type ReceiverXXXXXXXXXXXXXXXXXXXXXXXXXXXX[T1, T2, T3 any] struct{}
72+
type ReturnXXXXXXXXXXXXXXXXXXXXXXXXXXXX[T1, T2, T3 any] struct{}
73+
74+
func (r *ReceiverXXXXXXXXXXXXXXXXXXXXXXXXXXXX[T1, T2, T3]) VeryLongMethodNameForSig() ReturnXXXXXXXXXXXXXXXXXXXXXXXXXXXX[T1, T2, T3] {
75+
return ReturnXXXXXXXXXXXXXXXXXXXXXXXXXXXX[T1, T2, T3]{}
76+
}
77+
`
78+
79+
fset := token.NewFileSet()
80+
file, err := parser.ParseFile(fset, "in.go", src, parser.AllErrors)
81+
require.NoError(t, err)
82+
require.NotEmpty(t, file.Decls)
83+
84+
decl, ok := file.Decls[len(file.Decls)-1].(*ast.FuncDecl)
85+
require.True(t, ok)
86+
funcStart := fset.Position(decl.Pos()).Offset
87+
bracePos := fset.Position(decl.Body.Lbrace).Offset
88+
89+
signature := strings.TrimSpace(src[funcStart : bracePos+1])
90+
out, _ := FormatFuncSignatureNext(signature, "", 80, 8)
91+
92+
require.Contains(
93+
t, out, "ReceiverXXXXXXXXXXXXXXXXXXXXXXXXXXXX["+
94+
"\n T1,\n T2,\n T3,\n]",
95+
)
96+
}

formatter/pipeline_property_matrix_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,8 @@ func f() {
223223
// AST-based printing.
224224
if snippetIndex == 6 {
225225
require.Contains(
226-
t, string(
226+
t,
227+
string(
227228
out1,
228229
), "k"+
229230
"eep me",

0 commit comments

Comments
 (0)