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