From 76f5aef81ac5a403bc7e6fbb609d3ed19a41b35c Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Tue, 1 Sep 2026 21:11:57 +0200 Subject: [PATCH] format: a CSV can be written in the dialect you were handed Adds delimiter, line_ending and header to the csv format. Separators are named rather than typed - comma, semicolon, tab and pipe - so tab and pipe need no escaping on a command line or in a recipe. The defaults are comma, lf and a header, which is what this tool has always written, so no existing file changes by a byte: checked against a binary built from main, four seeds by three sizes, twelve of twelve identical. These are the three ways a real CSV differs before its contents do. A European spreadsheet exports with semicolons, anything written on Windows ends its rows with CRLF, and a table dumped out of a database has no header. All three are CSV and all three break a reader that assumed the other thing. Two things are not obvious from the list of settings. The filler has to emit the SEPARATOR rather than a comma. The description is a quoted field and it is quoted only because it carries the separator, so a filler that kept dropping commas would leave a semicolon file looking perfect - right size, right separators between the fields, every row the same width - while never exercising the quoted path the setting exists to test. Nothing else here would have seen that, so the guard counts separators in the closing row: six fields need five, and any more are inside the quotes. And the dialect moves the floor. A CRLF row is a byte longer and a header is a whole line, so the four floors are 117, 119, 76 and 77 B rather than one number. The registry announces the floor for the settings left alone and Plan works out the real one, the same arrangement the log format uses. The structural checker is now TOLD the dialect rather than sniffing it, which needed Strict to take optional settings. A checker that guessed the separator would agree with a file that used the wrong one - it would split on whatever it found and report a tidy table either way. Whether the file uses the separator that was ordered is a question for a guard reading the manifest instead, and there is one. That the checker is not a rubber stamp is asked directly: a comma file handed to it as semicolon has to come back refused. Still to come for this format: the quote character and escaping, and the column count. A row count disagrees with the exact size contract for the reason an entry count does in a log, and encoding waits for the piece that does TXT, where it is the headline setting rather than one of several. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 25 +++ internal/format/csvfile/csv.go | 168 ++++++++++----- internal/format/csvfile/dialect.go | 168 +++++++++++++++ internal/guard/csvdialect_test.go | 336 +++++++++++++++++++++++++++++ internal/guard/parity_test.go | 3 + internal/oracle/oracle.go | 11 +- internal/oracle/strict.py | 78 +++++-- web/public/formats/index.html | 15 ++ web/public/pl/formaty/index.html | 15 ++ 9 files changed, 751 insertions(+), 68 deletions(-) create mode 100644 internal/format/csvfile/dialect.go create mode 100644 internal/guard/csvdialect_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index fda1b44..a4ec802 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -70,6 +70,31 @@ because it turns other people's test suites red. ### Added +- **A CSV can be written in the dialect you were handed.** `--set + delimiter=semicolon`, `--set line_ending=crlf` and `--set header=false` on a + `csv`, separately or together. Separators are named rather than typed, so + `tab` and `pipe` need no escaping: the four are `comma`, `semicolon`, `tab` + and `pipe`. + + These are the three ways a real CSV differs before its contents do. A + European spreadsheet exports with semicolons, anything written on Windows + ends its rows with CRLF, and a table dumped straight out of a database has no + header. All three are CSV and all three break a reader that assumed the other + thing. + + The description column keeps carrying the separator, so a semicolon file + still exercises quoted fields rather than quietly testing less than a comma + one does. + + Two things worth knowing. The smallest file changes with the dialect, because + a CRLF row is a byte longer and a header is a whole line - the tool tells you + the floor for the settings you gave it. And the manifest records the + separator as the character that is in the file, where the recipe names it as + a word. + + The defaults are `comma`, `lf` and a header, which is what this tool has + always written, so **no existing file changes by a byte**. + - **A log can be made quiet, or full of errors.** `--set level_mix=errors` on a `log`, with `realistic`, `quiet`, `errors` and `debug` to choose from. It decides which severities appear, the way `status_mix` already decides which diff --git a/internal/format/csvfile/csv.go b/internal/format/csvfile/csv.go index 77403d9..2b4bc40 100644 --- a/internal/format/csvfile/csv.go +++ b/internal/format/csvfile/csv.go @@ -35,7 +35,6 @@ import ( const ( generatorVersion = "1" - header = "id,name,email,amount,created,description\n" emailDomain = "@example.com" createdDate = "2026-08-01" @@ -43,9 +42,9 @@ const ( // drawn from below guarantees it. amountWidth = 9 - // rowTail is what follows the description: the closing quote and the - // newline. - rowTail = `"` + "\n" + // closingQuote ends the description. What follows it is the row ending, + // which the dialect decides, so the two are no longer one constant. + closingQuote = `"` // maxRowDigits bounds the width of the row number. A row is at least one // byte, so a file can never hold more rows than it has bytes, and a size is @@ -54,13 +53,24 @@ const ( // length whatever row number it lands on. maxRowDigits = 19 - // fixedWidth is every byte of a row except the row number, the name (which - // also forms the address) and the description. A constant expression, so it - // cannot drift away from the template above. - fixedWidth = 5 /* separators */ + len(emailDomain) + amountWidth + - len(createdDate) + 1 /* the opening quote */ + len(rowTail) + // fixedBeforeEnding is every byte of a row except the row number, the name + // (which also forms the address), the description and the row ending. A + // constant expression, so it cannot drift away from the template above. + // + // The five separators count one byte each, which is a fact about the + // separators offered rather than an assumption: every one of them is a + // single byte, and dialect.go says so where they are declared. + fixedBeforeEnding = 5 /* separators */ + len(emailDomain) + amountWidth + + len(createdDate) + 1 /* the opening quote */ + len(closingQuote) ) +// fixedWidth is fixedBeforeEnding plus the row ending, which the dialect +// decides. A CRLF row costs one byte more than an LF one, on every row, which +// is why the minimum moves with this setting. +func fixedWidth(d dialect) int64 { + return int64(fixedBeforeEnding + len(d.eol)) +} + func init() { format.Register(format.Descriptor{ ID: "csv", @@ -70,9 +80,14 @@ func init() { // A file with a header and no rows is legal CSV, and it is not something // anybody orders by naming a byte count - that is a shape request, and - // it arrives with the row count property. The minimum here is the header - // and one whole row. - MinBytes: minimumBytes(), + // it would arrive with a row count setting, which this format does not + // offer yet. + // + // The floor announced here is for the settings left alone. The dialect + // moves the real one - a CRLF row costs a byte more and a file with no + // header has one fewer line to pay for - so Plan works that one out and + // names it. The log format is arranged the same way. + MinBytes: minimumBytes(defaultDialect()), Padding: format.PaddingChannel{ Name: "the description field of the last row", @@ -85,9 +100,9 @@ func init() { // name and the manifest carry it instead. Label: format.LabelExternalOnly, Oracle: "python-csv", - // Separator, quoting, column count and column types come later. - // Declaring none now makes a recipe asking for them fail loudly. - Properties: nil, + // Quoting, column count and column types come later. Declaring none of + // them now makes a recipe asking for one fail loudly. + Properties: properties(), GeneratorVersion: generatorVersion, Generator: generator{}, }) @@ -95,16 +110,24 @@ func init() { type generator struct{} -type memo struct{ seed uint64 } +type memo struct { + seed uint64 + dia dialect +} func (generator) Plan(r format.Request) (format.Plan, error) { - min := minimumBytes() + d, err := parseDialect(r.Properties) + if err != nil { + return format.Plan{}, err + } + + min := minimumBytes(d) if r.Bytes < min { return format.Plan{}, &format.BelowMinimumError{ Format: "CSV", Requested: r.Bytes, Minimum: min, - Reason: "a table holds a header and whole rows, and one of each needs that much", + Reason: reasonForMinimum(d), Hint: fmt.Sprintf("Ask for %d B or more.", min), } } @@ -114,42 +137,62 @@ func (generator) Plan(r format.Request) (format.Plan, error) { Exact: true, Determinism: format.DeterminismByte, Properties: map[string]any{ - "encoding": "utf-8", - "line_ending": "lf", - "separator": ",", - "header": true, - "columns": 6, + "encoding": "utf-8", + // The manifest carries the separator as the CHARACTER, where the + // recipe names it as a word. That difference is deliberate and is + // the same one the contract already draws between size, which is an + // intention, and bytes, which is a fact. Changing it would break + // every script reading this field. + "line_ending": d.lineEndingID, + "separator": string(d.sep), + "header": d.header, + "columns": len(columnNames), // Stated even though it is always false here, so a test can assert // on it without knowing which formats carry a label internally. format.PropertyLabelEmbedded: false, }, - Memo: memo{seed: r.Seed}, + Memo: memo{seed: r.Seed, dia: d}, }, nil } +// reasonForMinimum says what the floor is made of, which changes with the +// dialect. A file with no header pays for rows alone, and saying "a header and +// whole rows" there would name something the file does not have. +func reasonForMinimum(d dialect) string { + if !d.header { + return "a table holds whole rows, and one of them needs that much" + } + return "a table holds a header and whole rows, and one of each needs that much" +} + func (generator) Write(ctx context.Context, w io.Writer, p format.Plan) error { m, ok := p.Memo.(memo) if !ok { return fmt.Errorf("csv: the plan was not produced by this generator") } - if err := core.WriteAll(w, []byte(header)); err != nil { - return err + if m.dia.header { + if err := core.WriteAll(w, []byte(m.dia.headerLine())); err != nil { + return err + } } rng := core.NewRand(m.seed) - return core.FillRecords(ctx, w, rng, p.Bytes-int64(len(header)), &rows{}) + return core.FillRecords(ctx, w, rng, p.Bytes-m.dia.headerBytes(), &rows{dia: m.dia}) } // rows builds the data rows. It carries the row number, so the id column counts // up the way a real export does. -type rows struct{ next int64 } +type rows struct { + next int64 + dia dialect +} // Shortest is the smallest row this builder can close a file with: the widest // row number, the longest name in both the name and the address, and an empty // description. It has to hold for every draw rather than for the lucky one. func (r *rows) Shortest() int64 { - return int64(maxRowDigits + 2*longestWord + fixedWidth) + return int64(maxRowDigits+2*longestWord) + fixedWidth(r.dia) } func (r *rows) Append(dst []byte, rng *rand.Rand) []byte { @@ -185,42 +228,51 @@ func (r *rows) append(dst []byte, rng *rand.Rand, want int64) []byte { whole := 100000 + rng.IntN(899999) cents := rng.IntN(100) + sep := r.dia.sep + dst = strconv.AppendInt(dst, r.next, 10) - dst = append(dst, ',') + dst = append(dst, sep) dst = append(dst, name...) - dst = append(dst, ',') + dst = append(dst, sep) dst = append(dst, name...) dst = append(dst, emailDomain...) - dst = append(dst, ',') + dst = append(dst, sep) dst = strconv.AppendInt(dst, int64(whole), 10) dst = append(dst, '.') if cents < 10 { dst = append(dst, '0') } dst = strconv.AppendInt(dst, int64(cents), 10) - dst = append(dst, ',') + dst = append(dst, sep) dst = append(dst, createdDate...) - dst = append(dst, ',', '"') + dst = append(dst, sep, '"') if want < 0 { - dst = appendPhrase(dst, rng, 3+rng.IntN(5)) + dst = appendPhrase(dst, rng, 3+rng.IntN(5), sep) } else { // Everything written so far, plus what still has to follow. - used := int64(len(dst)-start) + int64(len(rowTail)) - dst = appendFiller(dst, want-used) + used := int64(len(dst)-start) + int64(len(closingQuote)) + int64(len(r.dia.eol)) + dst = appendFiller(dst, want-used, sep) } - return append(dst, rowTail...) + dst = append(dst, closingQuote...) + return append(dst, r.dia.eol...) } -// appendPhrase writes a readable description. Every few words it drops a comma, -// which is the case a CSV reader has to get right and the reason the column is -// quoted at all. -func appendPhrase(dst []byte, rng *rand.Rand, n int) []byte { +// appendPhrase writes a readable description. Every few words it drops the +// SEPARATOR, which is the case a CSV reader has to get right and the reason the +// column is quoted at all. +// +// The separator rather than always a comma, and that is the point of the +// setting rather than a detail of it. A comma inside a semicolon separated file +// needs no quoting, so a description that kept dropping commas would leave a +// semicolon file never exercising the quoted path at all - the file would be +// the right size, parse everywhere, and quietly test less than the comma one. +func appendPhrase(dst []byte, rng *rand.Rand, n int, sep byte) []byte { for i := 0; i < n; i++ { if i > 0 { if i%3 == 0 { - dst = append(dst, ',') + dst = append(dst, sep) } dst = append(dst, ' ') } @@ -236,24 +288,32 @@ func appendPhrase(dst []byte, rng *rand.Rand, n int) []byte { // field early. // appendFiller stretches the description to the byte. // -// A comma every fourth word, unlike every other format here, and on purpose: -// the description is a quoted field, so the padding is what makes a long file -// keep exercising the quoting rather than turning into plain words. -func appendFiller(dst []byte, n int64) []byte { +// A separator every fourth word, unlike every other format here, and on +// purpose: the description is a quoted field, so the padding is what makes a +// long file keep exercising the quoting rather than turning into plain words. +// It follows the dialect for the reason appendPhrase gives. +func appendFiller(dst []byte, n int64, sep byte) []byte { + both := string(sep) + " " return core.AppendFiller(dst, words, n, func(i int) string { if i%4 == 0 { - return ", " + return both } return " " }) } -// minimumBytes is the header and one whole row, computed rather than written -// down so it cannot drift away from the template the way a number in a document -// would. -func minimumBytes() int64 { - var r rows - return int64(len(header)) + r.Shortest() +// minimumBytes is the header, when there is one, and one whole row. Computed +// rather than written down so it cannot drift away from the template the way a +// number in a document would. +// +// It takes the dialect because the floor moves with it: a CRLF row costs a byte +// more, and a file with no header has one fewer line to pay for. The registry +// announces the floor for the settings left alone, and Plan works out the real +// one for the settings that arrived - the same arrangement the log format uses, +// where the entry shape moves the floor too. +func minimumBytes(d dialect) int64 { + r := rows{dia: d} + return d.headerBytes() + r.Shortest() } // longestWord is the widest draw, because the minimum has to hold for every diff --git a/internal/format/csvfile/dialect.go b/internal/format/csvfile/dialect.go new file mode 100644 index 0000000..fa6bfa8 --- /dev/null +++ b/internal/format/csvfile/dialect.go @@ -0,0 +1,168 @@ +// The dialect: the three ways a CSV file differs before its contents do. +// +// A tester does not usually get to choose the CSV they are handed. A European +// export separates with a semicolon, anything written on Windows ends its rows +// with CRLF, and a feed dumped straight out of a database has no header row at +// all. All three parse as CSV and all three break readers that assumed the +// other thing, which is why they are settings rather than a fixed shape. +// +// Every value here leaves the file exactly the size that was asked for. What +// moves is the minimum: a CRLF row is a byte longer than an LF one, and a file +// with no header has one fewer line to pay for. +package csvfile + +import ( + "strings" + + "github.com/donislawdev/TestingFilesGenerator/internal/format" +) + +// Setting names. Public names, so they are spelled once. +const ( + Delimiter = "delimiter" + LineEnding = "line_ending" + Header = "header" +) + +// delimiters are the separators offered, by name rather than by character. +// +// By name because the alternative does not survive the trip. A tab cannot be +// typed as a flag value without an escape, a pipe is a shell metacharacter, and +// this repository has recorded more than a dozen occasions where a backslash +// went missing between a shell and a file. A word has none of those problems. +// +// Every one of them is a single byte, which is what lets the row arithmetic +// stay as it was. A separator of two bytes would move every width here. +var delimiters = map[string]byte{ + "comma": ',', + "semicolon": ';', + "tab": '\t', + "pipe": '|', +} + +// delimiterIDs is the closed set the registry offers, in one order so that +// every surface lists them the same way. +var delimiterIDs = []string{"comma", "semicolon", "tab", "pipe"} + +var lineEndings = map[string]string{ + "lf": "\n", + "crlf": "\r\n", +} + +// lineEndingIDs is the closed set, and it is deliberately the same vocabulary +// the log format uses. One setting under one name means one thing, whichever +// format offers it. +var lineEndingIDs = []string{"lf", "crlf"} + +// dialect is the settled form of the three settings: the names for the +// manifest, and the bytes for the writer. +type dialect struct { + delimiterID string + sep byte + + lineEndingID string + eol string + + header bool +} + +func defaultDialect() dialect { + return dialect{ + delimiterID: "comma", + sep: ',', + lineEndingID: "lf", + eol: "\n", + header: true, + } +} + +// parseDialect reads the three settings. +// +// A value that is not in the declared set has already been refused by the +// registry, which checks it against the declaration for every format at once. +// The refusals here catch what that check lets through: it compares without +// regard for case, so REALISTIC style spellings arrive here rather than being +// stopped there. Written up as O168 - the branches below are reachable through +// that door and only through it, so they are not dead code. +func parseDialect(props map[string]string) (dialect, error) { + d := defaultDialect() + + if v, ok := props[Delimiter]; ok && v != "" { + sep, known := delimiters[v] + if !known { + return dialect{}, badValue(Delimiter, v, + "it has to be "+strings.Join(delimiterIDs, ", ")) + } + d.delimiterID, d.sep = v, sep + } + + if v, ok := props[LineEnding]; ok && v != "" { + eol, known := lineEndings[v] + if !known { + return dialect{}, badValue(LineEnding, v, "it has to be lf or crlf") + } + d.lineEndingID, d.eol = v, eol + } + + if v, ok := props[Header]; ok && v != "" { + switch v { + case "true": + d.header = true + case "false": + d.header = false + default: + return dialect{}, badValue(Header, v, "it has to be true or false") + } + } + + return d, nil +} + +func badValue(key, val, why string) error { + return &format.PropertyValueError{Format: "csv", Key: key, Value: val, Reason: why} +} + +// columnNames are the columns this table has always had. The last one is the +// description, which is the field the closing row stretches to reach an exact +// length, so it stays last whatever else changes. +var columnNames = []string{"id", "name", "email", "amount", "created", "description"} + +// headerLine is the first row, built from the dialect rather than written out, +// so the separator in it cannot disagree with the separator in the rows below. +// That disagreement is the whole defect this format would have: a file whose +// header says one thing and whose rows do another still has the right size and +// still ends every line properly. +func (d dialect) headerLine() string { + return strings.Join(columnNames, string(d.sep)) + d.eol +} + +// headerBytes is what the header costs, which is nothing when there is none. +func (d dialect) headerBytes() int64 { + if !d.header { + return 0 + } + return int64(len(d.headerLine())) +} + +// properties is what the registry declares. Kept beside the settings rather +// than in the descriptor, so a value added to a set above cannot be forgotten +// in the declaration below. +func properties() []format.Property { + return []format.Property{ + { + Name: Delimiter, Kind: format.PropertyChoice, + Choices: delimiterIDs, Default: "comma", + Detail: "What separates the fields. Choose semicolon for the shape a European spreadsheet exports.", + }, + { + Name: LineEnding, Kind: format.PropertyChoice, + Choices: lineEndingIDs, Default: "lf", + Detail: "How each row ends. Choose crlf for the shape RFC 4180 asks for and Excel writes.", + }, + { + Name: Header, Kind: format.PropertyBool, + Default: "true", + Detail: "Whether the first row names the columns. Turn it off for a table dumped straight out of a database.", + }, + } +} diff --git a/internal/guard/csvdialect_test.go b/internal/guard/csvdialect_test.go new file mode 100644 index 0000000..a944e86 --- /dev/null +++ b/internal/guard/csvdialect_test.go @@ -0,0 +1,336 @@ +package guard + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/donislawdev/TestingFilesGenerator/internal/format" + _ "github.com/donislawdev/TestingFilesGenerator/internal/format/all" + "github.com/donislawdev/TestingFilesGenerator/internal/oracle" +) + +// csvDialects is every combination the registry offers, built from the +// declaration rather than written out. A list copied into a guard stops +// describing the thing it guards the moment somebody adds a value. +func csvDialects(t *testing.T) (delims, endings []string) { + t.Helper() + d, err := format.Get("csv") + if err != nil { + t.Fatal(err) + } + for _, p := range d.Properties { + switch p.Name { + case "delimiter": + delims = p.Choices + case "line_ending": + endings = p.Choices + } + } + if len(delims) == 0 || len(endings) == 0 { + t.Fatalf("csv declares %d separators and %d row endings, so this guard would walk nothing", + len(delims), len(endings)) + } + return delims, endings +} + +// writeCSV produces one file and hands back its bytes, failing loudly rather +// than returning an error nobody reads. +func writeCSV(t *testing.T, size int64, seed uint64, props map[string]string) []byte { + t.Helper() + d, err := format.Get("csv") + if err != nil { + t.Fatal(err) + } + p, err := d.Generator.Plan(format.Request{Bytes: size, Seed: seed, Label: true, Properties: props}) + if err != nil { + t.Fatalf("planning %d B with %v: %v", size, props, err) + } + var buf bytes.Buffer + if err := d.Generator.Write(context.Background(), &buf, p); err != nil { + t.Fatalf("writing %d B with %v: %v", size, props, err) + } + if int64(buf.Len()) != size { + t.Fatalf("%v: asked for %d B and got %d", props, size, buf.Len()) + } + return buf.Bytes() +} + +// csvFloor is the smallest file this dialect will take, asked of the format +// rather than worked out here. Repeating the arithmetic in the guard would only +// prove the guard agrees with itself. +func csvFloor(t *testing.T, props map[string]string) int64 { + t.Helper() + d, err := format.Get("csv") + if err != nil { + t.Fatal(err) + } + _, err = d.Generator.Plan(format.Request{Bytes: 1, Seed: 1, Label: true, Properties: props}) + var below *format.BelowMinimumError + if !errors.As(err, &below) { + t.Fatalf("%v took a one byte table, or refused it without saying what the floor is: %v", props, err) + } + return below.Minimum +} + +// The dialect asked for is the dialect in the file, and it moves the floor. +// +// Three settings, sixteen combinations, and each of them is a file somebody is +// really handed: a European export separates with a semicolon, anything written +// on Windows ends its rows with CRLF, and a feed dumped out of a database has +// no header. All three parse as CSV and all three break a reader that assumed +// the other thing. +// +// What could be wrong here divides in two, and the halves fail differently. +// +// The setting could be stored and then ignored - the file comes out the right +// size, every row still parses, and the manifest still says what was asked for. +// So the FILE is asked which separator is in it, not the manifest. +// +// Or the arithmetic could miss that the dialect moves the floor. A CRLF row +// costs a byte more than an LF one and a header is a whole line, so a floor +// worked out for one dialect is wrong for the others - and wrong in the safe +// direction for CRLF, which no size or determinism guard would ever see. That +// is asked by taking the floor the format announces and the byte below it. +func TestTheCSVDialectIsInTheFileAndMovesTheFloor(t *testing.T) { + delims, endings := csvDialects(t) + seen := map[int64]bool{} + + for _, delim := range delims { + for _, eol := range endings { + for _, header := range []string{"true", "false"} { + name := delim + "/" + eol + "/header=" + header + t.Run(name, func(t *testing.T) { + props := map[string]string{ + "delimiter": delim, "line_ending": eol, "header": header, + } + + n := csvFloor(t, props) + seen[n] = true + d, err := format.Get("csv") + if err != nil { + t.Fatal(err) + } + if _, err := d.Generator.Plan(format.Request{Bytes: n, Seed: 1, Label: true, + Properties: props}); err != nil { + t.Errorf("announces %d B as its floor and then refuses it: %v", n, err) + } + if _, err := d.Generator.Plan(format.Request{Bytes: n - 1, Seed: 1, Label: true, + Properties: props}); err == nil { + t.Errorf("took %d B, one below the %d B it calls its floor", n-1, n) + } + + // Exact to the byte, at seeds rather than at sizes somebody + // picked - the closing row is stretched to reach the length + // and its arithmetic is what the separator and the ending + // both feed into. + for seed := uint64(1); seed <= 6; seed++ { + for _, size := range []int64{n, n + 1, 1000, 8192} { + if size < n { + continue + } + writeCSV(t, size, seed, props) // fails inside on a miss + } + } + + body := writeCSV(t, 8192, 9, props) + wantEOL := map[string]string{"lf": "\n", "crlf": "\r\n"}[eol] + if !bytes.HasSuffix(body, []byte(wantEOL)) { + t.Errorf("the file does not end with %s, so the last row is unterminated", eol) + } + if eol == "lf" && bytes.Contains(body, []byte("\r")) { + t.Errorf("an lf file carries a carriage return, so some readers will see a different table") + } + + sep := map[string]string{ + "comma": ",", "semicolon": ";", "tab": "\t", "pipe": "|", + }[delim] + if sep == "" { + t.Fatalf("the registry offers %q and this guard does not know what it separates with. "+ + "Add it rather than deleting this check - a value nobody described is a value nobody verified.", delim) + } + lines := strings.Split(strings.TrimSuffix(string(body), wantEOL), wantEOL) + if len(lines) < 3 { + t.Fatalf("only %d rows, too few to say anything", len(lines)) + } + for i, line := range lines[:3] { + if !strings.Contains(line, sep) { + t.Errorf("row %d carries no %s, so the setting was stored and the writer ignored it: %s", + i+1, delim, line) + } + } + + hasHeader := strings.HasPrefix(lines[0], "id"+sep) + if hasHeader != (header == "true") { + t.Errorf("header=%s and the first row is %q", header, lines[0]) + } + + // The quoted field carries the separator, which is the only + // reason the column is quoted at all. + // + // This is the half a reader of the code would not think to + // ask. The description is padding, and padding that dropped + // commas whatever the dialect would leave a semicolon file + // looking perfect - right size, right separators between the + // fields, every row the same width - while quietly never + // exercising the quoted path that the separator setting + // exists to test. Nothing else here would see it. + // + // Asked by counting: a row has five separators between its + // six fields, so any row carrying more than five has them + // inside the quotes. + padded := lines[len(lines)-1] + if n := strings.Count(padded, sep); n <= 5 { + t.Errorf("the closing row carries %d %s and six fields need five of them, "+ + "so nothing sits inside the quotes and this dialect never exercises quoting: %.120s", + n, delim, padded) + } + }) + } + } + } + + // The floor is not one number wearing sixteen hats. Without this the whole + // test above would still pass on a build that ignored the dialect when + // working the floor out. + if len(seen) < 4 { + t.Errorf("sixteen dialects produced %d distinct floors. A header is a whole line and a CRLF row "+ + "is a byte longer than an LF one, so a floor that did not move was worked out for one dialect "+ + "and handed to the rest.", len(seen)) + } +} + +// Every dialect is well formed, judged by the checker rather than by us. +// +// This is the fidelity half, and it is separate on purpose. The guard above +// asks whether the bytes are what was ordered. This asks whether they are a +// CSV at all, and it asks something written in another language against the +// specification - because the settings above changed the very characters that +// decide where a field ends. +// +// The last case is the one that makes the rest mean anything. The checker is +// TOLD the dialect rather than sniffing it, so it could be a rubber stamp that +// agrees with whatever it is handed. Feeding it a file of one dialect while +// telling it another has to come back refused, or every pass above is worth +// nothing. +func TestEveryCSVDialectIsWellFormed(t *testing.T) { + delims, endings := csvDialects(t) + dir := t.TempDir() + + check := func(t *testing.T, body []byte, name string, settings ...string) oracle.Result { + t.Helper() + path := filepath.Join(dir, name+".csv") + if err := os.WriteFile(path, body, 0o600); err != nil { + t.Fatal(err) + } + return oracle.Strict("csv", path, settings...) + } + + ran := 0 + for _, delim := range delims { + for _, eol := range endings { + for _, header := range []string{"true", "false"} { + name := delim + "_" + eol + "_" + header + t.Run(name, func(t *testing.T) { + props := map[string]string{ + "delimiter": delim, "line_ending": eol, "header": header, + } + body := writeCSV(t, 8192, 9, props) + res := check(t, body, name, + "delimiter="+delim, "line_ending="+eol, "header="+header) + if !res.Available { + t.Skip("the structural check needs python") + } + ran++ + if res.Err != nil { + t.Errorf("%s is not well formed: %v", name, res.Err) + } + }) + } + } + } + + if ran == 0 { + t.Skip("the structural check never ran, so nothing here was judged") + } + if ran != len(delims)*len(endings)*2 { + t.Errorf("%d of %d dialects reached the checker", ran, len(delims)*len(endings)*2) + } + + // The checker is told, so it has to disagree when it is told wrong. + body := writeCSV(t, 8192, 4, map[string]string{"delimiter": "comma", "line_ending": "lf"}) + for _, wrong := range []struct{ what, setting string }{ + {"a comma file called semicolon", "delimiter=semicolon"}, + {"a comma file called tab", "delimiter=tab"}, + {"an lf file called crlf", "line_ending=crlf"}, + } { + res := check(t, body, "wrong_"+strings.ReplaceAll(wrong.setting, "=", "_"), wrong.setting) + if !res.Available { + t.Skip("the structural check needs python") + } + if res.Err == nil { + t.Errorf("%s passed the checker, so being told the dialect made it a rubber stamp: %s", + wrong.what, firstLineOf(res.Output)) + } + } +} + +// The manifest says which dialect the file is in, because that is the half of +// this tool a test suite reads rather than a person. +// +// The separator goes in as the CHARACTER while the recipe names it as a word, +// and that difference is deliberate: the recipe states an intention and the +// manifest records a fact, the same split the contract already draws between +// size and bytes. A script reading this field would break if it changed. +func TestTheCSVManifestRecordsTheDialectAsFacts(t *testing.T) { + d, err := format.Get("csv") + if err != nil { + t.Fatal(err) + } + cases := []struct { + props map[string]string + sep string + eol string + head bool + }{ + {map[string]string{}, ",", "lf", true}, + {map[string]string{"delimiter": "semicolon"}, ";", "lf", true}, + {map[string]string{"delimiter": "tab", "line_ending": "crlf"}, "\t", "crlf", true}, + {map[string]string{"delimiter": "pipe", "header": "false"}, "|", "lf", false}, + } + for _, c := range cases { + t.Run(fmt.Sprint(c.props), func(t *testing.T) { + p, err := d.Generator.Plan(format.Request{Bytes: 8192, Seed: 1, Label: true, Properties: c.props}) + if err != nil { + t.Fatal(err) + } + if got := p.Properties["separator"]; got != c.sep { + t.Errorf("separator is %q, wanted the character %q", got, c.sep) + } + if got := p.Properties["line_ending"]; got != c.eol { + t.Errorf("line_ending is %v, wanted %q", got, c.eol) + } + if got := p.Properties["header"]; got != c.head { + t.Errorf("header is %v, wanted %v", got, c.head) + } + // And it stays JSON a script can read, rather than something that + // only looks right in a Go printout. + if _, err := json.Marshal(p.Properties); err != nil { + t.Errorf("the properties do not survive being written out: %v", err) + } + }) + } +} + +func firstLineOf(s string) string { + if i := strings.IndexByte(s, '\n'); i >= 0 { + return s[:i] + } + return s +} diff --git a/internal/guard/parity_test.go b/internal/guard/parity_test.go index 73e39f3..ba55cb4 100644 --- a/internal/guard/parity_test.go +++ b/internal/guard/parity_test.go @@ -99,6 +99,9 @@ var reachableFromTheWindow = []string{ // gains a property gains its field with no window code. // TestTheWindowDrawsAFieldForEveryDeclaredProperty. "property:bmp.height", + "property:csv.delimiter", + "property:csv.header", + "property:csv.line_ending", "property:docx.paragraphs", "property:pptx.slides", "property:xlsx.columns", diff --git a/internal/oracle/oracle.go b/internal/oracle/oracle.go index 27e3c42..eb0de28 100644 --- a/internal/oracle/oracle.go +++ b/internal/oracle/oracle.go @@ -348,7 +348,14 @@ func sevenZip() (string, bool) { // // It is written in another language, to the specification, so it is not our // own code judging our own code. -func Strict(formatID, path string) Result { +// The optional settings are handed to the checker after the path, as +// key=value words. They exist for the formats whose file SHAPE is a setting - +// a CSV separated by semicolons is well formed and a checker told to split on +// commas would call it a single column table. The checker is told rather than +// left to work it out, because a checker that guesses the separator would +// happily agree with a file that used the wrong one, which is the question a +// guard has to answer instead. +func Strict(formatID, path string, settings ...string) Result { python, ok := inPath("python")() if !ok { return Result{Available: false, Tool: "strict structural check"} @@ -364,7 +371,7 @@ func Strict(formatID, path string) Result { //nolint:gosec // same as above - our own script, run against a file this // tool wrote a moment ago // nosemgrep: go.lang.security.audit.dangerous-exec-command.dangerous-exec-command - cmd := exec.CommandContext(ctx, python, script, formatID, path) + cmd := exec.CommandContext(ctx, python, append([]string{script, formatID, path}, settings...)...) var out, errOut strings.Builder cmd.Stdout = &out cmd.Stderr = &errOut diff --git a/internal/oracle/strict.py b/internal/oracle/strict.py index ceec166..ac44718 100644 --- a/internal/oracle/strict.py +++ b/internal/oracle/strict.py @@ -265,7 +265,11 @@ def check_log_line(line, number, shape): fail(f"line {number} has an octet above 255: {address}") -def check_csv(data): +CSV_DELIMITERS = {"comma": ",", "semicolon": ";", "tab": "\t", "pipe": "|"} +CSV_LINE_ENDINGS = {"lf": "\n", "crlf": "\r\n"} + + +def check_csv(data, settings=None): """Every row carries the same columns, written to RFC 4180 by hand. Not csv.reader. That module is the tolerant reader used as the reference @@ -278,16 +282,29 @@ def check_csv(data): 131 072 B. Padding pushed into one field instead of through rows would sail past every size and determinism guard and break the reader a tester has nearest. + + The dialect is TOLD rather than worked out. A checker that sniffed the + separator would agree with a file that used the wrong one - it would split + on whatever it found and report a tidy table either way - and whether the + file uses the separator that was ordered is a question for a guard reading + the manifest, not for this. Told, it can still catch the defect that + matters here: a header and its rows disagreeing about the separator. """ FIELD_CEILING = 131072 + settings = settings or {} + sep = CSV_DELIMITERS[settings.get("delimiter", "comma")] + eol = CSV_LINE_ENDINGS[settings.get("line_ending", "lf")] + has_header = settings.get("header", "true") == "true" + try: text = data.decode("utf-8") except UnicodeDecodeError as exc: fail(f"not valid UTF-8: {exc}") - if not text.endswith("\n"): - fail("the file does not end with a newline, so the last row is unterminated") + if not text.endswith(eol): + fail(f"the file does not end with {settings.get('line_ending', 'lf')}, " + "so the last row is unterminated") rows, field, row, quoted, i = [], [], [], False, 0 while i < len(text): @@ -307,14 +324,24 @@ def check_csv(data): if field: fail(f"row {len(rows) + 1} opens a quote in the middle of a field") quoted = True - elif ch == ",": + elif ch == sep: row.append("".join(field)) field = [] - elif ch == "\n": + elif text.startswith(eol, i): row.append("".join(field)) field = [] rows.append(row) row = [] + i += len(eol) + continue + elif ch in "\r\n": + # A bare terminator where the dialect says there should be another + # one. Caught here rather than swept into a field, because a lone CR + # inside a CRLF file is the shape a half converted writer produces + # and it splits rows for some readers and not others. + fail(f"row {len(rows) + 1} carries a bare " + f"{'CR' if ch == chr(13) else 'LF'} where the rows end with " + f"{settings.get('line_ending', 'lf')}") else: field.append(ch) i += 1 @@ -323,21 +350,28 @@ def check_csv(data): fail("a quoted field is never closed") if field or row: fail("the file ends in the middle of a row") - if len(rows) < 2: + # With a header there has to be something under it. Without one, a single + # row is the whole file and is legal - there is simply nothing to compare + # it against, which is a limit of this check rather than a fault in it. + least = 2 if has_header else 1 + if len(rows) < least: fail(f"the table holds {len(rows)} row(s), so there is no data to check") columns = len(rows[0]) if columns < 2: - fail(f"the header declares {columns} column(s)") + fail(f"the first row has {columns} column(s), so nothing is separated by " + f"the {settings.get('delimiter', 'comma')} this file is meant to use") + declares = "the header" if has_header else "the first row" for number, r in enumerate(rows, start=1): if len(r) != columns: - fail(f"row {number} has {len(r)} fields and the header declares {columns}") + fail(f"row {number} has {len(r)} fields and {declares} declares {columns}") for value in r: if len(value.encode("utf-8")) > FIELD_CEILING: fail(f"row {number} has a field of {len(value.encode('utf-8'))} B - " f"the default Python reader refuses anything above {FIELD_CEILING} B") - ok(f"{len(rows) - 1} data rows, {columns} columns each") + data = len(rows) - 1 if has_header else len(rows) + ok(f"{data} data rows, {columns} columns each") def check_json(data): @@ -1485,9 +1519,29 @@ def check_jxl(data): "tiff": check_tiff, "webp": check_webp, "avif": check_avif, "jxl": check_jxl, "docx": check_docx, "xlsx": check_xlsx, "pptx": check_pptx} +# Checks that take the shape of the file as well as its bytes. Everything else +# is handed the bytes alone, so adding a setting to one check cannot change how +# any other one is called. +TAKES_SETTINGS = {"csv"} + if __name__ == "__main__": - if len(sys.argv) != 3 or sys.argv[1] not in CHECKS: - print("FAIL usage: strict.py <" + "|".join(CHECKS) + "> ") + if len(sys.argv) < 3 or sys.argv[1] not in CHECKS: + print("FAIL usage: strict.py <" + "|".join(CHECKS) + "> [key=value ...]") + sys.exit(1) + kind = sys.argv[1] + extra = {} + for word in sys.argv[3:]: + if "=" not in word: + print(f"FAIL setting {word!r} is not key=value") + sys.exit(1) + key, _, value = word.partition("=") + extra[key] = value + if extra and kind not in TAKES_SETTINGS: + print(f"FAIL the {kind} check takes no settings, and was given {sorted(extra)}") sys.exit(1) with open(sys.argv[2], "rb") as handle: - CHECKS[sys.argv[1]](handle.read()) + body = handle.read() + if kind in TAKES_SETTINGS: + CHECKS[kind](body, extra) + else: + CHECKS[kind](body) diff --git a/web/public/formats/index.html b/web/public/formats/index.html index 01013f4..7fb5993 100644 --- a/web/public/formats/index.html +++ b/web/public/formats/index.html @@ -340,6 +340,21 @@

Settings each format accepts

height 1 - 20000 pixels + + csv + delimiter + comma, pipe, semicolon, tab + + + + line_ending + crlf, lf + + + + header + true or false + docx paragraphs diff --git a/web/public/pl/formaty/index.html b/web/public/pl/formaty/index.html index ce10c68..d4f1a83 100644 --- a/web/public/pl/formaty/index.html +++ b/web/public/pl/formaty/index.html @@ -340,6 +340,21 @@

Ustawienia, które przyjmuje każdy format

height 1 - 20000 pikseli + + csv + delimiter + comma, pipe, semicolon, tab + + + + line_ending + crlf, lf + + + + header + prawda albo fałsz + docx paragraphs