diff --git a/internal/cmd/validate-test/warnings-point-at-right-line.yaml b/internal/cmd/validate-test/warnings-point-at-right-line.yaml new file mode 100644 index 00000000..9b409a92 --- /dev/null +++ b/internal/cmd/validate-test/warnings-point-at-right-line.yaml @@ -0,0 +1,28 @@ +schema: | + definition user {} + + definition organization {} + + definition platform {} + + definition resource { + /** platform is the platform to which the resource belongs */ + relation platform: platform + + /** + * organization is the organization to which the resource belongs + */ + relation organization: organization + + /** admin is a user that can administer the resource */ + relation admin: user + + /** viewer is a read-only viewer of the resource */ + relation viewer: user + + /** can_admin allows a user to administer the resource */ + permission can_admin = admin + + /** delete_resource allows a user to delete the resource. */ + permission delete_resource = can_admin + } diff --git a/internal/cmd/validate-test/warnings-point-at-right-line.zed b/internal/cmd/validate-test/warnings-point-at-right-line.zed new file mode 100644 index 00000000..2c65af01 --- /dev/null +++ b/internal/cmd/validate-test/warnings-point-at-right-line.zed @@ -0,0 +1,27 @@ +definition user {} + +definition organization {} + +definition platform {} + +definition resource { + /** platform is the platform to which the resource belongs */ + relation platform: platform + + /** + * organization is the organization to which the resource belongs + */ + relation organization: organization + + /** admin is a user that can administer the resource */ + relation admin: user + + /** viewer is a read-only viewer of the resource */ + relation viewer: user + + /** can_admin allows a user to administer the resource */ + permission can_admin = admin + + /** delete_resource allows a user to delete the resource. */ + permission delete_resource = can_admin +} diff --git a/internal/cmd/validate.go b/internal/cmd/validate.go index 7885f0dc..4ee2818c 100644 --- a/internal/cmd/validate.go +++ b/internal/cmd/validate.go @@ -150,6 +150,7 @@ func validateCmdFunc(cmd *cobra.Command, filenames []string) (string, bool, erro } var parsed validationfile.ValidationFile + // the decoder is also where compilation happens. validateContents, isOnlySchema, err := decoder(&parsed) standardErrors, composableErrs, otherErrs := classifyErrors(err) @@ -208,6 +209,8 @@ func validateCmdFunc(cmd *cobra.Command, filenames []string) (string, bool, erro return "", false, err } if devErrs != nil { + // Calculate the schema offset, used for outputting errors and warnings + // and having them point to the right place regardless of zed vs yaml schemaOffset := parsed.Schema.SourcePosition.LineNumber if isOnlySchema { schemaOffset = 0 diff --git a/internal/cmd/validate_test.go b/internal/cmd/validate_test.go index ec3aaa12..20c1e5c6 100644 --- a/internal/cmd/validate_test.go +++ b/internal/cmd/validate_test.go @@ -215,7 +215,7 @@ complete - 0 relationships loaded, 0 assertions run, 0 expected relations valida filepath.Join("validate-test", "missing-relation.zed"), }, expectNonZeroStatusCode: true, - expectStr: "error: parse error in `write`, line 2, column 20: relation/permission `write` not found under definition `test` \n" + + expectStr: "error: parse error in `write`, line 2, column 21: relation/permission `write` not found under definition `test` \n" + " 1 | definition test {\n" + " 2 > permission view = write\n" + " > ^~~~~\n " + @@ -297,6 +297,18 @@ complete - 0 relationships loaded, 0 assertions run, 0 expected relations valida complete - 0 relationships loaded, 0 assertions run, 0 expected relations validated `, }, + `warnings_point_at_correct_line_in_zed`: { + files: []string{ + filepath.Join("validate-test", "warnings-point-at-right-line.zed"), + }, + expectStr: "warning: Permission \"delete_resource\" references parent type \"resource\" in its name; it is recommended to drop the suffix (relation-name-references-parent)\n 23 | permission can_admin = admin\n 24 | \n 25 | /** delete_resource allows a user to delete the resource. */\n 26 > permission delete_resource = can_admin\n > ^~~~~~~~~~~~~~~\n 27 | }\n 28 | \n\ncomplete - 0 relationships loaded, 0 assertions run, 0 expected relations validated\n", + }, + `warnings_point_at_correct_line_in_yaml`: { + files: []string{ + filepath.Join("validate-test", "warnings-point-at-right-line.yaml"), + }, + expectStr: "warning: Permission \"delete_resource\" references parent type \"resource\" in its name; it is recommended to drop the suffix (relation-name-references-parent)\n 23 | /** can_admin allows a user to administer the resource */\n 24 | permission can_admin = admin\n 25 | \n 26 > /** delete_resource allows a user to delete the resource. */\n > ^~~~~~~~~~~~~~~\n 27 | permission delete_resource = can_admin\n 28 | }\n\ncomplete - 0 relationships loaded, 0 assertions run, 0 expected relations validated\n", + }, } for name, tc := range testCases { @@ -306,7 +318,6 @@ complete - 0 relationships loaded, 0 assertions run, 0 expected relations valida require := require.New(t) cmd := zedtesting.CreateTestCobraCommandWithFlagValue(t, zedtesting.StringFlag{FlagName: "schema-type", FlagValue: tc.schemaTypeFlag}, - zedtesting.BoolFlag{FlagName: "force-color", FlagValue: false}, zedtesting.IntFlag{FlagName: "batch-size", FlagValue: 100}, zedtesting.IntFlag{FlagName: "workers", FlagValue: 1}, zedtesting.BoolFlag{FlagName: "fail-on-warn", FlagValue: false}, diff --git a/internal/decode/decoder.go b/internal/decode/decoder.go index 91ff32f1..2d30c862 100644 --- a/internal/decode/decoder.go +++ b/internal/decode/decoder.go @@ -200,7 +200,9 @@ func compileSchemaFromData(filename, schemaString string, out interface{}) error SchemaString: schemaString, }, composable.AllowUnprefixedObjectType(), composable.SourceFolder(inputSourceFolder)) - if composableCompileErr == nil { + // We'll only attempt to generate the composable schema string if we don't already + // have one from standard schema compilation + if composableCompileErr == nil && vfile.Schema.Schema == "" { compiledSchemaString, _, err := generator.GenerateSchema(composableCompiled.OrderedDefinitions) if err != nil { return fmt.Errorf("could not generate string schema: %w", err)