Issues in validation of fga.mod fields

[d-jeffery]

Schema could alternatively be a number.

https://github.com/openfga/language/blob/6de767e56acd7dce22385e5845f3c660c2dec978/pkg/js/transformer/modules/mod-to-json.ts#L113

Also realized since this is just JSON, i have no range to use for marking which contents in fga.mod when generating errors. The structure is missing line and character values, such that i'd also need to parse the YAML as well to get line numbers.

https://github.com/openfga/language/blob/6de767e56acd7dce22385e5845f3c660c2dec978/pkg/js/transformer/modules/mod-to-json.ts#L171

[ewanharris]

@d-jeffery given that the AuthorizationModel interface expects schema_version to be a string does it make sense to allow it be a number? It also creates potential differences between JS and Go parsing as I think we could only support one type there without some custom unmarshalling logic.

Do you think we should maybe change the ModFile interface to be a bit more AST-like where we return a Node for each field, something like the below? That would then improve generating the errors in consumers of language

interface Node<T> {
  value: T;

  line: {
    start: number;
    end: number;
  }

  column: {
    start: number;
    end: number;
  }
}

[d-jeffery]

@d-jeffery given that the AuthorizationModel interface expects schema_version to be a string does it make sense to allow it be a number? It also creates potential differences between JS and Go parsing as I think we could only support one type there without some custom unmarshalling logic.

Thats very fair. I think I was unnecessarily concerned here.

Do you think we should maybe change the ModFile interface to be a bit more AST-like where we return a Node for each field, something like the below? That would then improve generating the errors in consumers of language

interface Node<T> {
  value: T;

  line: {
    start: number;
    end: number;
  }

  column: {
    start: number;
    end: number;
  }
}

Yep, that is a good alternative. What the YAML structure provides is nice - it has both locations for both the key & the value, which supports maps & sequences. I'm hesitant about investing too much time here, but having that value location data makes marking validation errors super easy.

[ewanharris]

Closing out as #177 is merged