diff --git a/README.md b/README.md index e53cc7d..19af89b 100644 --- a/README.md +++ b/README.md @@ -70,16 +70,17 @@ find . -name "*.thrift" | xargs -n 1 thriftls format -w Formatting flags: -| Flag | Meaning | -| ------------------- | -------------------------------------------------------------------------------------------------- | -| `-w` | Overwrite the file with the formatted result | -| `-d` | Print a diff instead of the formatted result | -| `--printWidth` | Target line width (default 80) | -| `--indent` | Indentation: a literal like `" "` or `"\t"`, a number like `8`, or a legacy spec like `"2spaces"` | -| `--align` | `field`, `assign`, or `disable` | -| `--fieldLineComma` | `add`, `remove`, or `disable` (keep as written) | -| `--config` | Path to a `thriftls.json` config file | -| `-I` | Additional include path, like the thrift compiler's `-I` (repeatable) | +| Flag | Meaning | +| --------------------- | -------------------------------------------------------------------------------------------------- | +| `-w` | Overwrite the file with the formatted result | +| `-d` | Print a diff instead of the formatted result | +| `--printWidth` | Target line width (default 80) | +| `--indent` | Indentation: a literal like `" "` or `"\t"`, a number like `8`, or a legacy spec like `"2spaces"` | +| `--align` | `field`, `assign`, or `disable` | +| `--fieldLineComma` | Struct/enum field separators: `add`, `remove`, `semicolon`, or `disable` (keep as written) | +| `--functionLineComma` | Service arg/throws separators: `add`, `remove`, `semicolon`, or `disable` (keep as written) | +| `--config` | Path to a `thriftls.json` config file | +| `-I` | Additional include path, like the thrift compiler's `-I` (repeatable) | Flags override the config file. @@ -132,12 +133,26 @@ Controls column alignment of struct/union/exception fields and enum values. ### fieldLineComma -Controls trailing separators on field lines. +Controls trailing separators on struct/union/exception fields and enum values. - `disable`: Keep as written (default) - `add`: Always add trailing commas +- `semicolon`: Always add trailing semicolons - `remove`: Remove trailing separators +### functionLineComma + +Controls trailing separators on service arguments and throws entries, +independently of `fieldLineComma`. + +- `disable`: Keep as written (default) +- `add`: Always add trailing commas +- `semicolon`: Always add trailing semicolons +- `remove`: Remove trailing separators + +Broken (multiline) argument and throws blocks are column-aligned like +struct fields, controlled by `align`. + ### includePaths List of additional paths to search for included thrift files. When a thrift diff --git a/formatter/body.go b/formatter/body.go index 714530d..4d8b282 100644 --- a/formatter/body.go +++ b/formatter/body.go @@ -143,10 +143,11 @@ func (f *formatter) functionHeader(v *syntax.Function) string { // or broken. The states are printed flat (or measured flat), so line docs // render as spaces; throwsBroken inserts hard lines to break the clause. func (f *formatter) functionFlat(v *syntax.Function, throwsBroken bool) doc.Doc { + args := f.flatFieldsJoin(v.Args) parts := []doc.Doc{ doc.Text(f.functionHeader(v)), doc.Text("("), - doc.Join(doc.Text(", "), f.flatFields(v.Args)), + args, doc.Text(")"), } if v.Throws != nil { @@ -155,7 +156,7 @@ func (f *formatter) functionFlat(v *syntax.Function, throwsBroken bool) doc.Doc } else { parts = append(parts, doc.Text(" throws ("), - doc.Join(doc.Text(", "), f.flatFields(v.Throws.Fields)), + f.flatFieldsJoin(v.Throws.Fields), doc.Text(")"), ) } @@ -164,6 +165,23 @@ func (f *formatter) functionFlat(v *syntax.Function, throwsBroken bool) doc.Doc return doc.Concat(parts) } +// flatFieldsJoin joins fields with their separators on one line: each +// field's own separator when preserving, or a single forced separator per +// the FunctionLineComma mode. +func (f *formatter) flatFieldsJoin(fields []*syntax.Field) doc.Doc { + parts := make([]doc.Doc, 0, len(fields)) + for i, field := range fields { + if i > 0 { + parts = append(parts, doc.Concat{ + doc.Text(sepText(fields[i-1].Sep, f.opts.FunctionLineComma)), + doc.Line, + }) + } + parts = append(parts, f.fieldContent(field, nil, false)) + } + return doc.Concat(parts) +} + // functionBrokenArgs renders the signature with arguments and throws both // broken, one per line. func (f *formatter) functionBrokenArgs(v *syntax.Function) doc.Doc { @@ -178,15 +196,6 @@ func (f *formatter) functionBrokenArgs(v *syntax.Function) doc.Doc { return doc.Concat(parts) } -// flatFields renders fields joined by ", " on one line, without comments. -func (f *formatter) flatFields(fields []*syntax.Field) []doc.Doc { - out := make([]doc.Doc, 0, len(fields)) - for _, field := range fields { - out = append(out, f.fieldContent(field, nil, false)) - } - return out -} - // brokenFields renders fields one per line, each with its trailing // separator per the FieldLineComma option. Comments and blank lines inside // the list are preserved. @@ -199,7 +208,10 @@ func (f *formatter) brokenFields(fields []*syntax.Field) doc.Doc { parts = append(parts, doc.HardLineNoBreak) } } - content := doc.Concat{f.fieldContent(field, nil, false), f.trailingSep(field.Sep)} + content := doc.Concat{ + f.fieldContent(field, f.alignmentFor(fields, i), true), + trailingSep(field.Sep, f.opts.FunctionLineComma), + } fieldDoc := append(f.leadingComments(field), content) fieldDoc = append(fieldDoc, f.trailingComments(field)...) parts = append(parts, doc.Concat(fieldDoc)) diff --git a/formatter/field.go b/formatter/field.go index b4b86af..e647b6f 100644 --- a/formatter/field.go +++ b/formatter/field.go @@ -7,18 +7,17 @@ import ( "github.com/karitham/thrift-ls/syntax" ) -// fieldList formats struct-like body fields. Fields are joined with ", " -// when the body stays on one line and with newlines otherwise; the comma in -// break mode comes from each field's own trailing separator, driven by the +// fieldList formats struct-like body fields. Fields are joined with their +// separator when the body stays on one line and with newlines otherwise; in +// break mode each field's own trailing separator is emitted, driven by the // FieldLineComma option. Blank lines between fields are preserved and force // the body to break. Column alignment applies per blank-line group, // matching the previous formatter. func (f *formatter) fieldList(fields []*syntax.Field, bodyID int) doc.Doc { - sep := doc.IfBreak(doc.Line, doc.Concat{doc.Text(","), doc.Line}) var parts []doc.Doc for i, field := range fields { if i > 0 { - parts = append(parts, sep) + parts = append(parts, fieldSep(fields[i-1].Sep, f.opts.FieldLineComma)) if f.blankBefore(field) >= 1 { parts = append(parts, doc.HardLine) } @@ -28,13 +27,41 @@ func (f *formatter) fieldList(fields []*syntax.Field, bodyID int) doc.Doc { return doc.Concat(parts) } +// fieldSep is the separator between two list items: a newline when the +// enclosing group breaks, otherwise the separator text — per-field when +// preserving (each item keeps its own trailing separator), or a single +// forced separator per the comma mode. +func fieldSep(prevSep syntax.TokenKind, mode CommaMode) doc.Doc { + return doc.IfBreak(doc.Line, doc.Concat{doc.Text(sepText(prevSep, mode)), doc.Line}) +} + +// sepText is the separator text between two flat list items: each item's +// own trailing separator when preserving, or a forced separator per the +// comma mode. +func sepText(prevSep syntax.TokenKind, mode CommaMode) string { + switch mode { + case CommaAdd: + return "," + case CommaSemicolon: + return ";" + case CommaRemove: + return "" + } + switch prevSep { + case syntax.TokenComma: + return "," + case syntax.TokenSemicolon: + return ";" + } + return "" +} + // enumValueList formats enum bodies with the same layout as fieldList. func (f *formatter) enumValueList(values []*syntax.EnumValue, bodyID int) doc.Doc { - sep := doc.IfBreak(doc.Line, doc.Concat{doc.Text(","), doc.Line}) var parts []doc.Doc for i, value := range values { if i > 0 { - parts = append(parts, sep) + parts = append(parts, fieldSep(values[i-1].Sep, f.opts.FieldLineComma)) if f.blankBefore(value) >= 1 { parts = append(parts, doc.HardLine) } @@ -131,7 +158,7 @@ func (f *formatter) field(v *syntax.Field, align *columnAlign, bodyID int) doc.D content := f.fieldContent(v, align, false) if bodyID != 0 { content = doc.IfBreakFor( - doc.Concat{f.fieldContent(v, align, true), f.trailingSep(v.Sep)}, + doc.Concat{f.fieldContent(v, align, true), trailingSep(v.Sep, f.opts.FieldLineComma)}, content, bodyID, ) @@ -200,7 +227,7 @@ func (f *formatter) enumValue(v *syntax.EnumValue, align *columnAlign, bodyID in content := f.enumValueContent(v, align, false) if bodyID != 0 { content = doc.IfBreakFor( - doc.Concat{f.enumValueContent(v, align, true), f.trailingSep(v.Sep)}, + doc.Concat{f.enumValueContent(v, align, true), trailingSep(v.Sep, f.opts.FieldLineComma)}, content, bodyID, ) @@ -228,18 +255,22 @@ func (f *formatter) enumValueContent(v *syntax.EnumValue, align *columnAlign, pa } // trailingSep returns the trailing separator for the given original -// separator, per the FieldLineComma option: always a comma when adding, +// separator, per the comma mode: always a comma or semicolon when forcing, // nothing when removing, the original separator when preserving. -func (f *formatter) trailingSep(sep syntax.TokenKind) doc.Doc { - switch f.opts.FieldLineComma { +func trailingSep(sep syntax.TokenKind, mode CommaMode) doc.Doc { + switch mode { case CommaAdd: return doc.Text(",") + case CommaSemicolon: + return doc.Text(";") case CommaRemove: return doc.Concat{} default: switch sep { - case syntax.TokenComma, syntax.TokenSemicolon: + case syntax.TokenComma: return doc.Text(",") + case syntax.TokenSemicolon: + return doc.Text(";") } return doc.Concat{} } diff --git a/formatter/format.go b/formatter/format.go index f26d26c..50b0e9f 100644 --- a/formatter/format.go +++ b/formatter/format.go @@ -34,10 +34,13 @@ const ( type CommaMode uint8 const ( - // CommaPreserve keeps the original separators (',', ';', or none). + // CommaPreserve keeps the original separators as written (',', ';', + // or none). CommaPreserve CommaMode = iota // CommaAdd adds a trailing comma everywhere. CommaAdd + // CommaSemicolon adds a trailing semicolon everywhere. + CommaSemicolon // CommaRemove removes all trailing separators. CommaRemove ) @@ -53,8 +56,13 @@ type Options struct { TabWidth int // Align controls column alignment (default AlignField). Align AlignMode - // FieldLineComma controls trailing separators (default CommaPreserve). + // FieldLineComma controls trailing separators after + // struct/union/exception fields and enum values (default + // CommaPreserve). FieldLineComma CommaMode + // FunctionLineComma controls trailing separators after service + // arguments and throws entries (default CommaPreserve). + FunctionLineComma CommaMode // NoTrailingNewline suppresses the final newline that is otherwise // appended to the formatted output. NoTrailingNewline bool diff --git a/formatter/format_test.go b/formatter/format_test.go index 95fe4cc..e75c52e 100644 --- a/formatter/format_test.go +++ b/formatter/format_test.go @@ -7,17 +7,21 @@ import ( "github.com/karitham/thrift-ls/syntax" ) -// fmtSrc formats src at the given width with the given options. It fails the -// test when parsing fails. -func fmtSrc(t *testing.T, src string, opts Options) string { - t.Helper() - doc, errs := syntax.Parse([]byte(src)) +// hasParseErrors reports whether any error is a hard parse error. +func hasParseErrors(errs []syntax.Error) bool { for _, err := range errs { if err.Severity == syntax.SeverityError { - t.Fatalf("parse errors: %v", errs) + return true } } - got, err := Format(doc, opts) + return false +} + +// fmtSrc formats src at the given width with the given options. It fails the +// test when parsing fails. +func fmtSrc(t *testing.T, src string, opts Options) string { + t.Helper() + got, err := Format(parseDoc(t, src), opts) if err != nil { t.Fatalf("Format: %v", err) } @@ -28,10 +32,8 @@ func fmtSrc(t *testing.T, src string, opts Options) string { func parseDoc(t *testing.T, src string) *syntax.Document { t.Helper() doc, errs := syntax.Parse([]byte(src)) - for _, err := range errs { - if err.Severity == syntax.SeverityError { - t.Fatalf("parse errors: %v", errs) - } + if hasParseErrors(errs) { + t.Fatalf("parse errors: %v", errs) } return doc } @@ -44,6 +46,13 @@ func testOpts(width int) Options { return o } +// commaOpts returns testOpts at width with the given FieldLineComma. +func commaOpts(width int, mode CommaMode) Options { + o := testOpts(width) + o.FieldLineComma = mode + return o +} + // runCase formats, checks idempotency, and re-parses the output. func runCase(t *testing.T, src string, opts Options, want string) { t.Helper() @@ -58,12 +67,30 @@ func runCase(t *testing.T, src string, opts Options, want string) { t.Errorf("not idempotent:\n first: %q\nsecond: %q", got, again) } // Self-validation: the output must parse cleanly. - _, errs := syntax.Parse([]byte(got)) - for _, err := range errs { - if err.Severity == syntax.SeverityError { - t.Errorf("formatted output does not parse: %v", errs) - break - } + if _, errs := syntax.Parse([]byte(got)); hasParseErrors(errs) { + t.Errorf("formatted output does not parse: %v", errs) + } +} + +// formatCase is one formatting test case. A zero width means the default 80. +type formatCase struct { + name string + src string + width int + want string +} + +// runFormatCases runs width-based table cases through runCase. +func runFormatCases(t *testing.T, cases []formatCase) { + t.Helper() + for _, tt := range cases { + t.Run(tt.name, func(t *testing.T) { + width := tt.width + if width == 0 { + width = 80 + } + runCase(t, tt.src, testOpts(width), tt.want) + }) } } @@ -80,7 +107,7 @@ func TestFormatAnnotations(t *testing.T) { }, { name: "empty annotation before an enum", - src: "@deprecation.Deprecated{}\nenum Status {\n A\n B\n}\n", + src: "@deprecation.Deprecated{}\nenum Status {\n A,\n B,\n}\n", want: "@deprecation.Deprecated{}\nenum Status { A, B }\n", }, { @@ -136,12 +163,7 @@ func TestFormatHeaders(t *testing.T) { } func TestFormatTypedefs(t *testing.T) { - tests := []struct { - name string - src string - width int - want string - }{ + tests := []formatCase{ { name: "simple", src: "typedef\ti64\tTimestamp", @@ -164,24 +186,11 @@ func TestFormatTypedefs(t *testing.T) { want: "typedef string Id (\n id_type = \"uuid\",\n long_annotation = \"some value\"\n)\n", }, } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - width := tt.width - if width == 0 { - width = 80 - } - runCase(t, tt.src, testOpts(width), tt.want) - }) - } + runFormatCases(t, tests) } func TestFormatConsts(t *testing.T) { - tests := []struct { - name string - src string - width int - want string - }{ + tests := []formatCase{ { name: "scalars", src: "const i32 a = 0xa1\nconst double b = -1.5e-3\nconst string c = \"x\"", @@ -233,12 +242,7 @@ func TestFormatConsts(t *testing.T) { } func TestFormatStructs(t *testing.T) { - tests := []struct { - name string - src string - width int - want string - }{ + tests := []formatCase{ { name: "empty struct", src: "struct Empty {}", @@ -309,29 +313,21 @@ func TestFormatStructs(t *testing.T) { want: "struct S {\n 1: i32 &parent\n}\n", }, { - name: "semicolon separators normalized", + name: "semicolon separators preserved", src: "struct S {\n 1: i32 a;\n 2: string b;\n}", - want: "struct S { 1: i32 a, 2: string b }\n", + want: "struct S { 1: i32 a; 2: string b }\n", + }, + { + name: "mixed separators preserved per field", + src: "struct S {\n 1: i32 a;\n 2: string b,\n 3: bool c\n}", + want: "struct S { 1: i32 a; 2: string b, 3: bool c }\n", }, } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - width := tt.width - if width == 0 { - width = 80 - } - runCase(t, tt.src, testOpts(width), tt.want) - }) - } + runFormatCases(t, tests) } func TestFormatEnums(t *testing.T) { - tests := []struct { - name string - src string - width int - want string - }{ + tests := []formatCase{ { name: "empty enum", src: "enum E {}", @@ -365,24 +361,11 @@ func TestFormatEnums(t *testing.T) { want: "enum E {\n A (\n a_anno = \"y\"\n )\n} (\n e_anno = \"x\"\n)\n", }, } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - width := tt.width - if width == 0 { - width = 80 - } - runCase(t, tt.src, testOpts(width), tt.want) - }) - } + runFormatCases(t, tests) } func TestFormatFunctions(t *testing.T) { - tests := []struct { - name string - src string - width int - want string - }{ + tests := []formatCase{ { name: "short signature stays flat", src: "service S {\n void ping()\n}", @@ -417,13 +400,13 @@ func TestFormatFunctions(t *testing.T) { name: "everything breaks when signature is long", src: "service S {\n i32 getUser(1: i64 id, 2: string name) throws (NotFound e)\n}", width: 45, - want: "service S {\n i32 getUser(\n 1: i64 id,\n 2: string name\n ) throws (\n NotFound e\n )\n}\n", + want: "service S {\n i32 getUser(\n 1: i64 id,\n 2: string name\n ) throws (\n NotFound e\n )\n}\n", }, { name: "args break without throws", src: "service S {\n i32 getUser(1: i64 id, 2: string name)\n}", width: 35, - want: "service S {\n i32 getUser(\n 1: i64 id,\n 2: string name\n )\n}\n", + want: "service S {\n i32 getUser(\n 1: i64 id,\n 2: string name\n )\n}\n", }, { name: "one arg stays flat", @@ -450,20 +433,11 @@ func TestFormatFunctions(t *testing.T) { want: "service Child extends Parent {\n void f()\n}\n", }, } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - runCase(t, tt.src, testOpts(tt.width), tt.want) - }) - } + runFormatCases(t, tests) } func TestFormatComments(t *testing.T) { - tests := []struct { - name string - src string - width int - want string - }{ + tests := []formatCase{ { name: "leading comments", src: "// before\nstruct S {\n 1: i32 a\n}", @@ -523,15 +497,7 @@ func TestFormatComments(t *testing.T) { want: "service S {\n void f(\n 1: i32 a // arg comment\n )\n}\n", }, } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - width := tt.width - if width == 0 { - width = 80 - } - runCase(t, tt.src, testOpts(width), tt.want) - }) - } + runFormatCases(t, tests) } func TestFormatOptions(t *testing.T) { @@ -543,21 +509,13 @@ func TestFormatOptions(t *testing.T) { }{ { name: "comma add", - opts: func() Options { - o := testOpts(30) - o.FieldLineComma = CommaAdd - return o - }(), + opts: commaOpts(30, CommaAdd), src: "struct S {\n 1: i32 a\n 2: string b\n}", want: "struct S {\n 1: i32 a,\n 2: string b,\n}\n", }, { name: "comma remove", - opts: func() Options { - o := testOpts(30) - o.FieldLineComma = CommaRemove - return o - }(), + opts: commaOpts(30, CommaRemove), src: "struct S {\n 1: i32 a,\n 2: string b,\n}", want: "struct S {\n 1: i32 a\n 2: string b\n}\n", }, @@ -676,3 +634,67 @@ struct User { 1: required i64 id } (tag = "x")` t.Errorf("got %q, want %q", got, want) } } + +func TestFormatSeparators(t *testing.T) { + tests := []struct { + name string + opts Options + src string + want string + }{ + { + name: "fields semicolon, functions comma", + opts: func() Options { + o := testOpts(30) + o.FieldLineComma = CommaSemicolon + o.FunctionLineComma = CommaAdd + return o + }(), + src: "struct S {\n 1: i32 a\n 2: string b\n}\n\nservice F {\n void go(1: i32 x) throws (\n 1: E err\n )\n}", + want: "struct S {\n 1: i32 a;\n 2: string b;\n}\n\nservice F {\n void go(1: i32 x) throws (\n 1: E err,\n )\n}\n", + }, + { + name: "semicolon mode flat", + opts: commaOpts(80, CommaSemicolon), + src: "struct S {\n 1: i32 a\n 2: string b\n}", + want: "struct S { 1: i32 a; 2: string b }\n", + }, + { + name: "preserve keeps per-field separators when broken", + opts: testOpts(30), + src: "struct S {\n 1: i32 a;\n 2: string b,\n 3: bool c\n}", + want: "struct S {\n 1: i32 a;\n 2: string b,\n 3: bool c\n}\n", + }, + { + name: "function preserve keeps argument separators", + opts: testOpts(30), + src: "service F {\n void go(1: i32 x; 2: string y)\n}", + want: "service F {\n void go(\n 1: i32 x;\n 2: string y\n )\n}\n", + }, + { + name: "function comma add forces commas on throws", + opts: func() Options { + o := testOpts(30) + o.FunctionLineComma = CommaAdd + return o + }(), + src: "service F {\n void go(1: i32 x) throws (\n 1: E err\n 2: F fail\n )\n}", + want: "service F {\n void go(1: i32 x) throws (\n 1: E err,\n 2: F fail,\n )\n}\n", + }, + { + name: "function comma remove drops argument separators", + opts: func() Options { + o := testOpts(30) + o.FunctionLineComma = CommaRemove + return o + }(), + src: "service F {\n void go(1: i32 x, 2: string y)\n}", + want: "service F {\n void go(\n 1: i32 x\n 2: string y\n )\n}\n", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + runCase(t, tt.src, tt.opts, tt.want) + }) + } +} diff --git a/main.go b/main.go index f891fbd..ddcfe02 100644 --- a/main.go +++ b/main.go @@ -95,7 +95,11 @@ func formatFlags() []cli.Flag { }, &cli.StringFlag{ Name: "fieldLineComma", - Usage: `trailing commas: "add", "remove", or "disable" to keep as written`, + Usage: `struct/union/exception field separators: "add", "remove", "semicolon", or "disable" to keep as written`, + }, + &cli.StringFlag{ + Name: "functionLineComma", + Usage: `service argument and throws separators: "add", "remove", "semicolon", or "disable" to keep as written`, }, &cli.StringFlag{ Name: "config", @@ -189,6 +193,10 @@ func formatPatch(cmd *cli.Command) (options.Patch, error) { v := cmd.String("fieldLineComma") p.FieldLineComma = &v } + if cmd.IsSet("functionLineComma") { + v := cmd.String("functionLineComma") + p.FunctionLineComma = &v + } if paths := cmd.StringSlice("I"); len(paths) > 0 { p.IncludePaths = &paths } diff --git a/options/options.go b/options/options.go index dec8bba..2533ae8 100644 --- a/options/options.go +++ b/options/options.go @@ -27,13 +27,14 @@ const ConfigFileName = "thriftls.json" // Patch is a partial set of options; nil fields are unset. type Patch struct { - PrintWidth *int `json:"printWidth"` - Indent *Indent `json:"indent"` - TabWidth *int `json:"tabWidth"` - Align *string `json:"align"` - FieldLineComma *string `json:"fieldLineComma"` - IncludePaths *[]string `json:"includePaths"` - LogLevel *int `json:"logLevel"` + PrintWidth *int `json:"printWidth"` + Indent *Indent `json:"indent"` + TabWidth *int `json:"tabWidth"` + Align *string `json:"align"` + FieldLineComma *string `json:"fieldLineComma"` + FunctionLineComma *string `json:"functionLineComma"` + IncludePaths *[]string `json:"includePaths"` + LogLevel *int `json:"logLevel"` } // Apply overlays p onto base: every set field of p replaces the @@ -55,6 +56,9 @@ func (p Patch) Apply(base Patch) Patch { if p.FieldLineComma != nil { out.FieldLineComma = p.FieldLineComma } + if p.FunctionLineComma != nil { + out.FunctionLineComma = p.FunctionLineComma + } if p.IncludePaths != nil { out.IncludePaths = p.IncludePaths } @@ -91,8 +95,11 @@ func (p Patch) Validate() error { if p.Align != nil && !oneOf(*p.Align, "field", "assign", "disable") { return fmt.Errorf("align must be one of \"field\", \"assign\", \"disable\", got %q", *p.Align) } - if p.FieldLineComma != nil && !oneOf(*p.FieldLineComma, "add", "remove", "disable", "preserve") { - return fmt.Errorf("fieldLineComma must be one of \"add\", \"remove\", \"disable\" (keep as written), got %q", *p.FieldLineComma) + if p.FieldLineComma != nil && !oneOf(*p.FieldLineComma, "add", "remove", "semicolon", "disable", "preserve") { + return fmt.Errorf("fieldLineComma must be one of \"add\", \"remove\", \"semicolon\", \"disable\" (keep as written), got %q", *p.FieldLineComma) + } + if p.FunctionLineComma != nil && !oneOf(*p.FunctionLineComma, "add", "remove", "semicolon", "disable", "preserve") { + return fmt.Errorf("functionLineComma must be one of \"add\", \"remove\", \"semicolon\", \"disable\" (keep as written), got %q", *p.FunctionLineComma) } if p.Indent != nil { if p.Indent.Width <= 0 || !isWhitespaceOnly(p.Indent.Value) { @@ -133,18 +140,29 @@ func (p Patch) Formatter() (formatter.Options, error) { } } if p.FieldLineComma != nil { - switch *p.FieldLineComma { - case "add": - o.FieldLineComma = formatter.CommaAdd - case "remove": - o.FieldLineComma = formatter.CommaRemove - case "disable", "preserve": - o.FieldLineComma = formatter.CommaPreserve - } + o.FieldLineComma = commaMode(*p.FieldLineComma) + } + if p.FunctionLineComma != nil { + o.FunctionLineComma = commaMode(*p.FunctionLineComma) } return o, nil } +// commaMode maps a config value to a formatter comma mode. The value is +// validated before this is called. +func commaMode(s string) formatter.CommaMode { + switch s { + case "add": + return formatter.CommaAdd + case "remove": + return formatter.CommaRemove + case "semicolon": + return formatter.CommaSemicolon + default: // "disable", "preserve" + return formatter.CommaPreserve + } +} + // Indent is a resolved indentation: the string emitted for one level and // its display width. It is set from a config value that may be a literal // string of spaces or tabs, a number of spaces, or a legacy spec like diff --git a/options/options_test.go b/options/options_test.go index a33fdd2..3c2cf7d 100644 --- a/options/options_test.go +++ b/options/options_test.go @@ -245,3 +245,41 @@ func TestLoadRejectsUnknownOverrideKeys(t *testing.T) { t.Fatal("Load accepted a config with an overrides key") } } + +// TestPatchCommaModes maps every config value to the formatter modes. +func TestPatchCommaModes(t *testing.T) { + tests := []struct { + value string + field formatter.CommaMode + function formatter.CommaMode + }{ + {"add", formatter.CommaAdd, formatter.CommaAdd}, + {"remove", formatter.CommaRemove, formatter.CommaRemove}, + {"semicolon", formatter.CommaSemicolon, formatter.CommaSemicolon}, + {"disable", formatter.CommaPreserve, formatter.CommaPreserve}, + {"preserve", formatter.CommaPreserve, formatter.CommaPreserve}, + } + for _, tt := range tests { + t.Run(tt.value, func(t *testing.T) { + p := Patch{FieldLineComma: &tt.value, FunctionLineComma: &tt.value} + o, err := p.Formatter() + if err != nil { + t.Fatalf("Formatter: %v", err) + } + if o.FieldLineComma != tt.field || o.FunctionLineComma != tt.function { + t.Errorf("value %q: field=%v function=%v", tt.value, o.FieldLineComma, o.FunctionLineComma) + } + }) + } + + // The two options map independently. + semicolon, add := "semicolon", "add" + p := Patch{FieldLineComma: &semicolon, FunctionLineComma: &add} + o, err := p.Formatter() + if err != nil { + t.Fatalf("Formatter: %v", err) + } + if o.FieldLineComma != formatter.CommaSemicolon || o.FunctionLineComma != formatter.CommaAdd { + t.Errorf("independent mapping failed: %+v", o) + } +}