Skip to content

Commit f8245bf

Browse files
ivanauthtstirrat15
andauthored
fix: correct error line numbers for non-standard YAML section ordering (#665)
Co-authored-by: Tanner Stirrat <tstirrat@gmail.com>
1 parent 7abd712 commit f8245bf

12 files changed

Lines changed: 187 additions & 65 deletions

go.mod

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ require (
1515
github.com/TylerBrock/colorjson v0.0.0-20200706003622-8a50f05110d2
1616
github.com/authzed/authzed-go v1.8.0
1717
github.com/authzed/grpcutil v0.0.0-20240123194739-2ea1e3d2d98b
18-
github.com/authzed/spicedb v1.51.1-0.20260331192321-82ad080e464b
18+
github.com/authzed/spicedb v1.51.1-0.20260408223255-9b516fc2731d
1919
github.com/brianvoe/gofakeit/v6 v6.28.0
2020
github.com/ccoveille/go-safecast/v2 v2.0.0
2121
github.com/cenkalti/backoff/v4 v4.3.0
@@ -192,7 +192,7 @@ require (
192192
github.com/go-toolsmith/astp v1.1.0 // indirect
193193
github.com/go-toolsmith/strparse v1.1.0 // indirect
194194
github.com/go-toolsmith/typep v1.1.0 // indirect
195-
github.com/go-viper/mapstructure/v2 v2.4.0 // indirect
195+
github.com/go-viper/mapstructure/v2 v2.5.0 // indirect
196196
github.com/go-xmlfmt/xmlfmt v1.1.3 // indirect
197197
github.com/gobwas/glob v0.2.3 // indirect
198198
github.com/godbus/dbus v0.0.0-20190726142602-4481cbc300e2 // indirect
@@ -241,7 +241,7 @@ require (
241241
github.com/jackc/pgpassfile v1.0.0 // indirect
242242
github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761 // indirect
243243
github.com/jackc/pgx-zerolog v0.0.0-20230315001418-f978528409eb // indirect
244-
github.com/jackc/pgx/v5 v5.7.6 // indirect
244+
github.com/jackc/pgx/v5 v5.9.1 // indirect
245245
github.com/jackc/puddle/v2 v2.2.2 // indirect
246246
github.com/jgautheron/goconst v1.8.2 // indirect
247247
github.com/jingyugao/rowserrcheck v1.1.1 // indirect

go.sum

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -736,8 +736,8 @@ github.com/authzed/grpcutil v0.0.0-20240123194739-2ea1e3d2d98b h1:wbh8IK+aMLTCey
736736
github.com/authzed/grpcutil v0.0.0-20240123194739-2ea1e3d2d98b/go.mod h1:s3qC7V7XIbiNWERv7Lfljy/Lx25/V1Qlexb0WJuA8uQ=
737737
github.com/authzed/jitterbug v0.0.0-20260128162915-e97d76daaa24 h1:BXaWSanmHFu3P0xWfTDPpwcJIQ/oSol29+CWe4lSGSU=
738738
github.com/authzed/jitterbug v0.0.0-20260128162915-e97d76daaa24/go.mod h1:WvEk4YHnUsmbUaWA/VseQty3X91f6/jEHek5mjYDZUg=
739-
github.com/authzed/spicedb v1.51.1-0.20260331192321-82ad080e464b h1:UanPsFaWbyhi/ih4d5pfL1sWjgNqBsYzNhx6feW9v+g=
740-
github.com/authzed/spicedb v1.51.1-0.20260331192321-82ad080e464b/go.mod h1:PaBZjCLOp/qUTuzMKSmIIxbSCZ/bte31p8RDS2R5I9U=
739+
github.com/authzed/spicedb v1.51.1-0.20260408223255-9b516fc2731d h1:0B5y5LRq/wjtxJ70lga60uV1qwSJ9J54aSdnv5t1iQQ=
740+
github.com/authzed/spicedb v1.51.1-0.20260408223255-9b516fc2731d/go.mod h1:K8qlhyXhF+MAqdb6poAw6PhFLMMGTjQr5UJZgZdo6/A=
741741
github.com/aws/aws-sdk-go-v2 v1.40.1 h1:difXb4maDZkRH0x//Qkwcfpdg1XQVXEAEs2DdXldFFc=
742742
github.com/aws/aws-sdk-go-v2 v1.40.1/go.mod h1:MayyLB8y+buD9hZqkCW3kX1AKq07Y5pXxtgB+rRFhz0=
743743
github.com/aws/aws-sdk-go-v2/config v1.32.3 h1:cpz7H2uMNTDa0h/5CYL5dLUEzPSLo2g0NkbxTRJtSSU=
@@ -1018,8 +1018,8 @@ github.com/go-toolsmith/strparse v1.1.0 h1:GAioeZUK9TGxnLS+qfdqNbA4z0SSm5zVNtCQi
10181018
github.com/go-toolsmith/strparse v1.1.0/go.mod h1:7ksGy58fsaQkGQlY8WVoBFNyEPMGuJin1rfoPS4lBSQ=
10191019
github.com/go-toolsmith/typep v1.1.0 h1:fIRYDyF+JywLfqzyhdiHzRop/GQDxxNhLGQ6gFUNHus=
10201020
github.com/go-toolsmith/typep v1.1.0/go.mod h1:fVIw+7zjdsMxDA3ITWnH1yOiw1rnTQKCsF/sk2H/qig=
1021-
github.com/go-viper/mapstructure/v2 v2.4.0 h1:EBsztssimR/CONLSZZ04E8qAkxNYq4Qp9LvH92wZUgs=
1022-
github.com/go-viper/mapstructure/v2 v2.4.0/go.mod h1:oJDH3BJKyqBA2TXFhDsKDGDTlndYOZ6rGS0BRZIxGhM=
1021+
github.com/go-viper/mapstructure/v2 v2.5.0 h1:vM5IJoUAy3d7zRSVtIwQgBj7BiWtMPfmPEgAXnvj1Ro=
1022+
github.com/go-viper/mapstructure/v2 v2.5.0/go.mod h1:oJDH3BJKyqBA2TXFhDsKDGDTlndYOZ6rGS0BRZIxGhM=
10231023
github.com/go-xmlfmt/xmlfmt v1.1.3 h1:t8Ey3Uy7jDSEisW2K3somuMKIpzktkWptA0iFCnRUWY=
10241024
github.com/go-xmlfmt/xmlfmt v1.1.3/go.mod h1:aUCEOzzezBEjDBbFBoSiya/gduyIiWYRP6CnSFIV8AM=
10251025
github.com/gobwas/glob v0.2.3 h1:A4xDbljILXROh+kObIiy5kIaPYD8e96x1tgBhUI5J+Y=
@@ -1256,8 +1256,8 @@ github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761 h1:iCEnooe7Ulw
12561256
github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761/go.mod h1:5TJZWKEWniPve33vlWYSoGYefn3gLQRzjfDlhSJ9ZKM=
12571257
github.com/jackc/pgx-zerolog v0.0.0-20230315001418-f978528409eb h1:pSv+zRVeAYjbXRFjyytFIMRBSKWVowCi7KbXSMR/+ug=
12581258
github.com/jackc/pgx-zerolog v0.0.0-20230315001418-f978528409eb/go.mod h1:CRUuPsmIajLt3dZIlJ5+O8IDSib6y8yrst8DkCthTa4=
1259-
github.com/jackc/pgx/v5 v5.7.6 h1:rWQc5FwZSPX58r1OQmkuaNicxdmExaEz5A2DO2hUuTk=
1260-
github.com/jackc/pgx/v5 v5.7.6/go.mod h1:aruU7o91Tc2q2cFp5h4uP3f6ztExVpyVv88Xl/8Vl8M=
1259+
github.com/jackc/pgx/v5 v5.9.1 h1:uwrxJXBnx76nyISkhr33kQLlUqjv7et7b9FjCen/tdc=
1260+
github.com/jackc/pgx/v5 v5.9.1/go.mod h1:mal1tBGAFfLHvZzaYh77YS/eC6IX9OWbRV1QIIM0Jn4=
12611261
github.com/jackc/puddle/v2 v2.2.2 h1:PR8nw+E/1w0GLuRFSmiioY6UooMp6KJv0/61nB7icHo=
12621262
github.com/jackc/puddle/v2 v2.2.2/go.mod h1:vriiEXHvEE654aYKXXjOvZM39qJ0q+azkZFrfEOc3H4=
12631263
github.com/jgautheron/goconst v1.8.2 h1:y0XF7X8CikZ93fSNT6WBTb/NElBu9IjaY7CCYQrCMX4=

internal/cmd/import.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ func importCmdFunc(cmd *cobra.Command, schemaClient v1.SchemaServiceClient, rela
8282
}
8383

8484
if cobrautil.MustGetBool(cmd, "schema") {
85-
if err := importSchema(cmd.Context(), schemaClient, p.Schema, prefix); err != nil {
85+
if err := importSchema(cmd.Context(), schemaClient, p.Schema.Schema, prefix); err != nil {
8686
return fmt.Errorf("error importing schema: %w", err)
8787
}
8888
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
relationships: >-
3+
document:1#viewer@user:tom
4+
assertions:
5+
assertTrue:
6+
- "document:1#viewer@user:maria"
7+
schema: |-
8+
definition user {}
9+
definition document {
10+
relation viewer: user
11+
permission view = viewer
12+
}
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
relationships: |-
3+
test:1#viewer@user:1
4+
test:2#viewer@user:2
5+
test:1#idontexisttt@user:1
6+
schema: |-
7+
definition user {}
8+
definition test {
9+
relation viewer: user
10+
permission view = viewer
11+
}
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
relationships: >-
3+
test:1#idontexisttt@user:1
4+
# notice how schema isn't the first section of the yaml
5+
schema: |-
6+
definition user {}
7+
definition test {
8+
relation viewer: user
9+
permission view = viewer
10+
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
assertions:
3+
assertTrue:
4+
- "test:1#view@user:1"
5+
relationships: >-
6+
test:1#viewer@user:1
7+
schema: |-
8+
definition user {}
9+
definition test {
10+
relation viewer: user
11+
permission view = nonexistent
12+
}
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
relationships: >-
3+
test:1#viewer@user:1
4+
schema: |-
5+
definition user {}
6+
definition test {
7+
relation viewer: user
8+
permission view = nonexistent
9+
}

internal/cmd/validate.go

Lines changed: 25 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import (
1515
"github.com/spf13/cobra"
1616

1717
"github.com/authzed/spicedb/pkg/development"
18-
core "github.com/authzed/spicedb/pkg/proto/core/v1"
1918
devinterface "github.com/authzed/spicedb/pkg/proto/developer/v1"
2019

2120
"github.com/authzed/zed/internal/commands"
@@ -161,34 +160,30 @@ func validateCmdFunc(cmd *cobra.Command, filenames []string) (string, bool, erro
161160

162161
filesystem := os.DirFS(parsed.RootSchemaDir)
163162

164-
// This logic will use the zero value of the struct, so we don't need
165-
// to do it conditionally.
166-
tuples := make([]*core.RelationTuple, 0)
167163
totalAssertions := 0
168164
totalRelationsValidated := 0
169165

170-
for _, rel := range parsed.Relationships.Relationships {
171-
tuples = append(tuples, rel.ToCoreTuple())
172-
}
173-
174-
// Create the development context for each run
166+
// Create the development context for each run. If a schemaFile was used,
167+
// the schema has already been inlined by ResolveSchemaFileIfPresent; clear
168+
// SchemaFile on the copy so the new API uses the inlined schema directly
169+
// rather than trying to re-read it via fs.FS (which doesn't support ../paths).
175170
ctx := cmd.Context()
176-
devCtx, devErrs, err := development.NewDevContext(ctx, &devinterface.RequestContext{
177-
Schema: parsed.Schema,
178-
Relationships: tuples,
179-
}, development.WithSourceFS(filesystem), development.WithRootFileName(parsed.SchemaFileName))
171+
vfForDev := *parsed.ValidationFile
172+
if vfForDev.SchemaFile != "" {
173+
vfForDev.SchemaFile = ""
174+
}
175+
yctx, devErrs, err := development.NewDevContextForValidationFile(ctx, &vfForDev,
176+
development.WithSourceFS(filesystem),
177+
development.WithRootFileName(parsed.SchemaFileName))
180178
if err != nil {
181179
return "", false, err
182180
}
183181
if devErrs != nil {
184-
schemaOffset := parsed.SchemaOffset.LineNumber
185-
186-
// Output errors
187-
outputDeveloperErrorsWithLineOffset(toPrint, parsed.DisplayContents, devErrs.InputErrors, schemaOffset, filesystem)
182+
outputDeveloperErrors(toPrint, parsed.DisplayContents, devErrs.InputErrors, filesystem)
188183
return toPrint.String(), true, nil
189184
}
190185
// Run assertions
191-
adevErrs, aerr := development.RunAllAssertions(devCtx, &parsed.Assertions)
186+
adevErrs, aerr := development.RunAllAssertions(yctx.DevContext, &yctx.Assertions)
192187
if aerr != nil {
193188
return "", false, aerr
194189
}
@@ -199,7 +194,7 @@ func validateCmdFunc(cmd *cobra.Command, filenames []string) (string, bool, erro
199194
successfullyValidatedFiles++
200195

201196
// Run expected relations for file
202-
_, erDevErrs, rerr := development.RunValidation(devCtx, &parsed.ExpectedRelations)
197+
_, erDevErrs, rerr := development.RunValidation(yctx.DevContext, &yctx.ExpectedRelations)
203198
if rerr != nil {
204199
return "", false, rerr
205200
}
@@ -208,7 +203,7 @@ func validateCmdFunc(cmd *cobra.Command, filenames []string) (string, bool, erro
208203
return toPrint.String(), true, nil
209204
}
210205
// Print out any warnings for file
211-
warnings, err := development.GetWarnings(ctx, devCtx)
206+
warnings, err := development.GetWarnings(ctx, yctx.DevContext)
212207
if err != nil {
213208
return "", false, err
214209
}
@@ -227,11 +222,11 @@ func validateCmdFunc(cmd *cobra.Command, filenames []string) (string, bool, erro
227222
} else {
228223
toPrint.WriteString(success())
229224
}
230-
totalAssertions += len(parsed.Assertions.AssertTrue) + len(parsed.Assertions.AssertFalse)
231-
totalRelationsValidated += len(parsed.ExpectedRelations.ValidationMap)
225+
totalAssertions += len(yctx.Assertions.AssertTrue) + len(yctx.Assertions.AssertFalse)
226+
totalRelationsValidated += len(yctx.ExpectedRelations.ValidationMap)
232227

233228
fmt.Fprintf(toPrint, " - %d relationships loaded, %d assertions run, %d expected relations validated\n",
234-
len(tuples),
229+
len(vfForDev.Relationships.Relationships),
235230
totalAssertions,
236231
totalRelationsValidated)
237232
}
@@ -266,31 +261,28 @@ func outputForLine(sb *strings.Builder, validateContents []byte, oneIndexedLineN
266261
}
267262

268263
func outputDeveloperErrors(sb *strings.Builder, validateContents []byte, devErrors []*devinterface.DeveloperError, sourceFS fs.FS) {
269-
outputDeveloperErrorsWithLineOffset(sb, validateContents, devErrors, 0, sourceFS)
270-
}
271-
272-
func outputDeveloperErrorsWithLineOffset(sb *strings.Builder, validateContents []byte, devErrors []*devinterface.DeveloperError, lineOffset int, sourceFS fs.FS) {
273264
for _, devErr := range devErrors {
274-
// If the error has a Path, read the contents from that file instead.
265+
contents := validateContents
275266
if len(devErr.Path) > 0 {
276267
fileContents, err := fs.ReadFile(sourceFS, devErr.Path[0])
277268
if err == nil {
278-
validateContents = fileContents
269+
contents = fileContents
279270
}
280271
}
281-
lines := strings.Split(string(validateContents), "\n")
282-
outputDeveloperError(sb, devErr, lines, lineOffset)
272+
273+
lines := strings.Split(string(contents), "\n")
274+
outputDeveloperError(sb, devErr, lines)
283275
}
284276
}
285277

286-
func outputDeveloperError(sb *strings.Builder, devError *devinterface.DeveloperError, lines []string, lineOffset int) {
278+
func outputDeveloperError(sb *strings.Builder, devError *devinterface.DeveloperError, lines []string) {
287279
errorSource := devError.Context
288280
if len(devError.Path) > 0 && devError.Path[0] != "" {
289281
errorSource = devError.Path[0]
290282
}
291283
lineContext := fmt.Sprintf("parse error in `%s`, line %d, column %d:", errorSource, devError.Line, devError.Column)
292284
fmt.Fprintf(sb, "%s%s %s\n", errorPrefix(), lineContext, errorMessageStyle().Render(devError.Message))
293-
errorLineNumber := int(devError.Line) - 1 + lineOffset // devError.Line is 1-indexed
285+
errorLineNumber := int(devError.Line) - 1 // devError.Line is 1-indexed
294286
for i := errorLineNumber - 3; i < errorLineNumber+3; i++ {
295287
if i == errorLineNumber {
296288
renderLine(sb, lines, i, devError.Context, errorLineNumber, -1)

internal/cmd/validate_test.go

Lines changed: 73 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ complete - 0 relationships loaded, 0 assertions run, 0 expected relations valida
205205
filepath.Join("validate-test", "missing-relation.yaml"),
206206
},
207207
expectNonZeroStatusCode: true,
208-
expectStr: "error: parse error in `missing-relation.yaml`, line 4, column 21: relation/permission `write` not found under definition `test` \n" +
208+
expectStr: "error: parse error in `missing-relation.yaml`, line 9, column 21: relation/permission `write` not found under definition `test` \n" +
209209
" 6 | definition user {}\n" +
210210
" 7 | definition test {\n" +
211211
" 8 | relation viewer: user\n" +
@@ -278,6 +278,78 @@ complete - 0 relationships loaded, 0 assertions run, 0 expected relations valida
278278
expectNonZeroStatusCode: true,
279279
expectStr: "error: parse error in `composable-schema-with-import-error-imported.zed`, line 5, column 23: relation/permission `unknownrel` not found under definition `group` \n 2 | \n 3 | definition group {\n 4 | relation member: user\n 5 > permission view = unknownrel\n > ^~~~~~~~~~\n 6 | }\n 7 | \n\n\n",
280280
},
281+
`relationship_error_points_at_correct_line_when_relationships_before_schema`: {
282+
files: []string{
283+
filepath.Join("validate-test", "relationship-error-before-schema.yaml"),
284+
},
285+
expectNonZeroStatusCode: true,
286+
expectStr: "error: parse error in `test:1#idontexisttt@user:1`, line 3, column 16: relation/permission `idontexisttt` not found under definition `test` \n" +
287+
" 1 | ---\n" +
288+
" 2 | relationships: >-\n" +
289+
" 3 > test:1#idontexisttt@user:1\n" +
290+
" > ^~~~~~~~~~~~~~~~~~~~~~~~~~\n" +
291+
" 4 | # notice how schema isn't the first section of the yaml\n" +
292+
" 5 | schema: |-\n\n\n",
293+
},
294+
`relationship_error_points_at_failing_line_among_multiple_relationships`: {
295+
files: []string{
296+
filepath.Join("validate-test", "relationship-error-before-schema-multi.yaml"),
297+
},
298+
expectNonZeroStatusCode: true,
299+
expectStr: "error: parse error in `test:1#idontexisttt@user:1`, line 5, column 16: relation/permission `idontexisttt` not found under definition `test` \n" +
300+
" 2 | relationships: |-\n" +
301+
" 3 | test:1#viewer@user:1\n" +
302+
" 4 | test:2#viewer@user:2\n" +
303+
" 5 > test:1#idontexisttt@user:1\n" +
304+
" > ^~~~~~~~~~~~~~~~~~~~~~~~~~\n" +
305+
" 6 | schema: |-\n" +
306+
" 7 | definition user {}\n\n\n",
307+
},
308+
`schema_error_points_at_correct_line_when_schema_after_relationships`: {
309+
files: []string{
310+
filepath.Join("validate-test", "schema-error-after-relationships.yaml"),
311+
},
312+
expectNonZeroStatusCode: true,
313+
expectStr: "error: parse error in `schema-error-after-relationships.yaml`, line 8, column 21: relation/permission `nonexistent` not found under definition `test` \n" +
314+
" 5 | definition user {}\n" +
315+
" 6 | definition test {\n" +
316+
" 7 | relation viewer: user\n" +
317+
" 8 > permission view = nonexistent\n" +
318+
" > ^~~~~~~~~~~\n" +
319+
" 9 | }\n" +
320+
" 10 | \n\n\n",
321+
},
322+
`assertion_error_points_at_correct_line_when_before_schema`: {
323+
files: []string{
324+
filepath.Join("validate-test", "assertion-error-before-schema.yaml"),
325+
},
326+
expectNonZeroStatusCode: true,
327+
expectStr: "error: parse error in `document:1#viewer@user:maria`, line 6, column 7: Expected relation or permission document:1#viewer@user:maria to exist \n" +
328+
" 3 | document:1#viewer@user:tom\n" +
329+
" 4 | assertions:\n" +
330+
" 5 | assertTrue:\n" +
331+
" 6 > - \"document:1#viewer@user:maria\"\n" +
332+
" > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~\n" +
333+
" 7 | schema: |-\n" +
334+
" 8 | definition user {}\n\n" +
335+
" Explanation:\n" +
336+
" ⨉ document:1 viewer (105.292µs)\n" +
337+
" \n\n\n",
338+
},
339+
`schema_error_points_at_correct_line_when_schema_after_assertions_and_relationships`: {
340+
files: []string{
341+
filepath.Join("validate-test", "schema-error-after-assertions.yaml"),
342+
},
343+
expectNonZeroStatusCode: true,
344+
expectStr: "error: parse error in `schema-error-after-assertions.yaml`, line 11, column 21: relation/permission `nonexistent` not found under definition `test` \n" +
345+
" 8 | definition user {}\n" +
346+
" 9 | definition test {\n" +
347+
" 10 | relation viewer: user\n" +
348+
" 11 > permission view = nonexistent\n" +
349+
" > ^~~~~~~~~~~\n" +
350+
" 12 | }\n" +
351+
" 13 | \n\n\n",
352+
},
281353
`yaml_content_with_zed_extension_gives_hint`: {
282354
files: []string{
283355
filepath.Join("validate-test", "yaml-with-zed-extension.zed"),

0 commit comments

Comments
 (0)