From 9dd129d8e6588c0a81d6e15496eba496dea7b8df Mon Sep 17 00:00:00 2001 From: Poovamraj T T Date: Mon, 19 Jan 2026 18:14:15 +0530 Subject: [PATCH 1/5] Supporting comments in DSL --- pkg/go/transformer/comments.go | 229 +++++++++++ pkg/go/transformer/comments_test.go | 381 ++++++++++++++++++ pkg/go/transformer/dsltojson.go | 257 +++++++++++- pkg/go/transformer/jsontodsl.go | 375 +++++++++++++++++ pkg/js/package-lock.json | 60 --- pkg/js/tests/comments.test.ts | 331 +++++++++++++++ pkg/js/transformer/dsltojson.ts | 357 +++++++++++++++- pkg/js/transformer/jsontodsl.ts | 252 ++++++++++++ .../authorization-model-with-comments.json | 67 +++ .../authorization-model.fga | 9 + .../authorization-model.json | 48 +++ 11 files changed, 2291 insertions(+), 75 deletions(-) create mode 100644 pkg/go/transformer/comments.go create mode 100644 pkg/go/transformer/comments_test.go create mode 100644 pkg/js/tests/comments.test.ts create mode 100644 tests/data/transformer/200-comment-preservation/authorization-model-with-comments.json create mode 100644 tests/data/transformer/200-comment-preservation/authorization-model.fga create mode 100644 tests/data/transformer/200-comment-preservation/authorization-model.json diff --git a/pkg/go/transformer/comments.go b/pkg/go/transformer/comments.go new file mode 100644 index 00000000..c8443cb6 --- /dev/null +++ b/pkg/go/transformer/comments.go @@ -0,0 +1,229 @@ +package transformer + +import ( + "strings" +) + +// CommentInfo holds comment information for a DSL element. +type CommentInfo struct { + // PrecedingLines contains comment lines that appear before the element (including the # prefix) + PrecedingLines []string `json:"preceding_lines,omitempty"` + // Inline contains an inline comment that appears on the same line as the element (including the # prefix) + Inline string `json:"inline,omitempty"` +} + +// CommentsMetadata holds comments for types, relations, and conditions. +type CommentsMetadata struct { + // Comments for the element itself + Comments *CommentInfo `json:"comments,omitempty"` + // RelationComments maps relation names to their comments + RelationComments map[string]*CommentInfo `json:"relation_comments,omitempty"` +} + +// ModelComments holds comments at the model level. +type ModelComments struct { + // PrecedingLines contains comment lines that appear before the model declaration + PrecedingLines []string `json:"preceding_lines,omitempty"` +} + +// CommentTracker tracks comments in DSL source and maps them to AST elements. +type CommentTracker struct { + lines []string + lineComments map[int]string // line number -> inline comment + modelComment *ModelComments // comments before the model declaration + typeComments map[string]*CommentsMetadata // type name -> comments metadata + condComments map[string]*CommentInfo // condition name -> comment info + cleanedToOriginal map[int]int // mapping from cleaned line numbers to original +} + +// NewCommentTracker creates a new CommentTracker from DSL source. +func NewCommentTracker(source string) *CommentTracker { + ct := &CommentTracker{ + lines: strings.Split(source, "\n"), + lineComments: make(map[int]string), + typeComments: make(map[string]*CommentsMetadata), + condComments: make(map[string]*CommentInfo), + cleanedToOriginal: nil, + } + ct.parseComments() + return ct +} + +// NewCommentTrackerWithMapping creates a new CommentTracker with line number mapping. +func NewCommentTrackerWithMapping(source string, cleanedToOriginal map[int]int) *CommentTracker { + ct := &CommentTracker{ + lines: strings.Split(source, "\n"), + lineComments: make(map[int]string), + typeComments: make(map[string]*CommentsMetadata), + condComments: make(map[string]*CommentInfo), + cleanedToOriginal: cleanedToOriginal, + } + ct.parseComments() + return ct +} + +// parseComments parses all comments from the source. +func (ct *CommentTracker) parseComments() { + for i, line := range ct.lines { + // Check for inline comment + if inlineIdx := strings.Index(line, " #"); inlineIdx != -1 { + // Make sure this isn't inside a condition expression or similar + // We only consider it an inline comment if there's actual code before it + beforeComment := strings.TrimSpace(line[:inlineIdx]) + if len(beforeComment) > 0 { + ct.lineComments[i] = strings.TrimSpace(line[inlineIdx+1:]) + } + } + } +} + +// GetPrecedingComments returns comments that immediately precede the given line number. +// Line numbers are 0-based. +func (ct *CommentTracker) GetPrecedingComments(lineNum int) []string { + if lineNum <= 0 || lineNum > len(ct.lines) { + return nil + } + + var comments []string + // Walk backwards from the line before the target + for i := lineNum - 1; i >= 0; i-- { + line := strings.TrimSpace(ct.lines[i]) + if len(line) == 0 { + // Empty line breaks the contiguous comment block + break + } + if strings.HasPrefix(line, "#") { + // Prepend to maintain order + comments = append([]string{line}, comments...) + } else { + // Non-comment, non-empty line breaks the block + break + } + } + + return comments +} + +// GetInlineComment returns the inline comment for the given line number. +// Line numbers are 0-based. +func (ct *CommentTracker) GetInlineComment(lineNum int) string { + if lineNum < 0 || lineNum >= len(ct.lines) { + return "" + } + + line := ct.lines[lineNum] + // Find inline comment + if inlineIdx := strings.Index(line, " #"); inlineIdx != -1 { + beforeComment := strings.TrimSpace(line[:inlineIdx]) + if len(beforeComment) > 0 { + return strings.TrimSpace(line[inlineIdx+1:]) + } + } + return "" +} + +// GetModelComments returns comments that appear before the model declaration. +func (ct *CommentTracker) GetModelComments() *ModelComments { + // Find the model declaration line + modelLine := -1 + for i, line := range ct.lines { + trimmed := strings.TrimSpace(line) + if strings.HasPrefix(trimmed, "model") { + modelLine = i + break + } + } + + if modelLine <= 0 { + return nil + } + + precedingComments := ct.GetPrecedingComments(modelLine) + if len(precedingComments) == 0 { + return nil + } + + return &ModelComments{ + PrecedingLines: precedingComments, + } +} + +// GetCommentInfoForLine creates a CommentInfo for elements at the given line. +// lineNum is expected to be 0-based and may be from cleaned source (if mapping exists). +func (ct *CommentTracker) GetCommentInfoForLine(lineNum int) *CommentInfo { + // If we have a line mapping, convert from cleaned line number to original + originalLineNum := lineNum + if ct.cleanedToOriginal != nil { + if mapped, ok := ct.cleanedToOriginal[lineNum]; ok { + originalLineNum = mapped + } + } + + preceding := ct.GetPrecedingComments(originalLineNum) + inline := ct.GetInlineComment(originalLineNum) + + if len(preceding) == 0 && inline == "" { + return nil + } + + return &CommentInfo{ + PrecedingLines: preceding, + Inline: inline, + } +} + +// SetTypeComments sets comments for a type at the given line. +func (ct *CommentTracker) SetTypeComments(typeName string, lineNum int) { + commentInfo := ct.GetCommentInfoForLine(lineNum) + if commentInfo == nil { + return + } + + if ct.typeComments[typeName] == nil { + ct.typeComments[typeName] = &CommentsMetadata{} + } + ct.typeComments[typeName].Comments = commentInfo +} + +// SetRelationComments sets comments for a relation within a type. +func (ct *CommentTracker) SetRelationComments(typeName, relationName string, lineNum int) { + commentInfo := ct.GetCommentInfoForLine(lineNum) + if commentInfo == nil { + return + } + + if ct.typeComments[typeName] == nil { + ct.typeComments[typeName] = &CommentsMetadata{} + } + if ct.typeComments[typeName].RelationComments == nil { + ct.typeComments[typeName].RelationComments = make(map[string]*CommentInfo) + } + ct.typeComments[typeName].RelationComments[relationName] = commentInfo +} + +// SetConditionComments sets comments for a condition. +func (ct *CommentTracker) SetConditionComments(conditionName string, lineNum int) { + commentInfo := ct.GetCommentInfoForLine(lineNum) + if commentInfo == nil { + return + } + ct.condComments[conditionName] = commentInfo +} + +// GetTypeComments returns the comments metadata for a type. +func (ct *CommentTracker) GetTypeComments(typeName string) *CommentsMetadata { + return ct.typeComments[typeName] +} + +// GetRelationComments returns comments for a relation. +func (ct *CommentTracker) GetRelationComments(typeName, relationName string) *CommentInfo { + if ct.typeComments[typeName] == nil { + return nil + } + return ct.typeComments[typeName].RelationComments[relationName] +} + +// GetConditionComments returns comments for a condition. +func (ct *CommentTracker) GetConditionComments(conditionName string) *CommentInfo { + return ct.condComments[conditionName] +} diff --git a/pkg/go/transformer/comments_test.go b/pkg/go/transformer/comments_test.go new file mode 100644 index 00000000..68fdce61 --- /dev/null +++ b/pkg/go/transformer/comments_test.go @@ -0,0 +1,381 @@ +package transformer_test + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/openfga/language/pkg/go/transformer" +) + +func TestCommentTracker(t *testing.T) { + t.Parallel() + + t.Run("extracts preceding comments", func(t *testing.T) { + t.Parallel() + + source := `# Comment 1 +# Comment 2 +model + schema 1.1` + + tracker := transformer.NewCommentTracker(source) + comments := tracker.GetPrecedingComments(2) + + require.Equal(t, []string{"# Comment 1", "# Comment 2"}, comments) + }) + + t.Run("extracts inline comments", func(t *testing.T) { + t.Parallel() + + source := `model + schema 1.1 + +type user # inline comment` + + tracker := transformer.NewCommentTracker(source) + inline := tracker.GetInlineComment(3) + + require.Equal(t, "# inline comment", inline) + }) + + t.Run("extracts model comments", func(t *testing.T) { + t.Parallel() + + source := `# Model comment +model + schema 1.1` + + tracker := transformer.NewCommentTracker(source) + modelComments := tracker.GetModelComments() + + require.NotNil(t, modelComments) + require.Equal(t, []string{"# Model comment"}, modelComments.PrecedingLines) + }) + + t.Run("handles no comments", func(t *testing.T) { + t.Parallel() + + source := `model + schema 1.1 + +type user` + + tracker := transformer.NewCommentTracker(source) + comments := tracker.GetPrecedingComments(3) + inline := tracker.GetInlineComment(3) + + require.Empty(t, comments) + require.Empty(t, inline) + }) +} + +func TestDSLToJSONWithComments(t *testing.T) { + t.Parallel() + + t.Run("preserves model comments", func(t *testing.T) { + t.Parallel() + + dsl := `# OpenFGA Model +# Version 1.0 +model + schema 1.1 + +type user` + + jsonStr, err := transformer.TransformDSLToJSONWithComments(dsl) + require.NoError(t, err) + + var result map[string]interface{} + err = json.Unmarshal([]byte(jsonStr), &result) + require.NoError(t, err) + + metadata, ok := result["metadata"].(map[string]interface{}) + require.True(t, ok, "metadata should exist") + + modelComments, ok := metadata["model_comments"].(map[string]interface{}) + require.True(t, ok, "model_comments should exist") + + precedingLines, ok := modelComments["preceding_lines"].([]interface{}) + require.True(t, ok, "preceding_lines should exist") + require.Len(t, precedingLines, 2) + require.Equal(t, "# OpenFGA Model", precedingLines[0]) + require.Equal(t, "# Version 1.0", precedingLines[1]) + }) + + t.Run("preserves type comments", func(t *testing.T) { + t.Parallel() + + dsl := `model + schema 1.1 + +# User type comment +type user` + + jsonStr, err := transformer.TransformDSLToJSONWithComments(dsl) + require.NoError(t, err) + + var result map[string]interface{} + err = json.Unmarshal([]byte(jsonStr), &result) + require.NoError(t, err) + + typeDefinitions, ok := result["type_definitions"].([]interface{}) + require.True(t, ok) + require.Len(t, typeDefinitions, 1) + + typeDef := typeDefinitions[0].(map[string]interface{}) + metadata, ok := typeDef["metadata"].(map[string]interface{}) + require.True(t, ok, "type metadata should exist") + + comments, ok := metadata["comments"].(map[string]interface{}) + require.True(t, ok, "comments should exist") + + precedingLines, ok := comments["preceding_lines"].([]interface{}) + require.True(t, ok) + require.Len(t, precedingLines, 1) + require.Equal(t, "# User type comment", precedingLines[0]) + }) + + t.Run("preserves relation comments", func(t *testing.T) { + t.Parallel() + + dsl := `model + schema 1.1 + +type document + relations + # Owner comment + define owner: [user]` + + jsonStr, err := transformer.TransformDSLToJSONWithComments(dsl) + require.NoError(t, err) + + var result map[string]interface{} + err = json.Unmarshal([]byte(jsonStr), &result) + require.NoError(t, err) + + typeDefinitions := result["type_definitions"].([]interface{}) + typeDef := typeDefinitions[0].(map[string]interface{}) + metadata := typeDef["metadata"].(map[string]interface{}) + relations := metadata["relations"].(map[string]interface{}) + ownerMeta := relations["owner"].(map[string]interface{}) + comments := ownerMeta["comments"].(map[string]interface{}) + precedingLines := comments["preceding_lines"].([]interface{}) + + require.Len(t, precedingLines, 1) + require.Equal(t, "# Owner comment", precedingLines[0]) + }) + + t.Run("preserves condition comments", func(t *testing.T) { + t.Parallel() + + dsl := `model + schema 1.1 + +type user + +# IP-based access control +condition ip_check(ip: string) { + ip == "127.0.0.1" +}` + + jsonStr, err := transformer.TransformDSLToJSONWithComments(dsl) + require.NoError(t, err) + + var result map[string]interface{} + err = json.Unmarshal([]byte(jsonStr), &result) + require.NoError(t, err) + + conditions := result["conditions"].(map[string]interface{}) + ipCheck := conditions["ip_check"].(map[string]interface{}) + metadata := ipCheck["metadata"].(map[string]interface{}) + comments := metadata["comments"].(map[string]interface{}) + precedingLines := comments["preceding_lines"].([]interface{}) + + require.Len(t, precedingLines, 1) + require.Equal(t, "# IP-based access control", precedingLines[0]) + }) +} + +func TestJSONToDSLWithComments(t *testing.T) { + t.Parallel() + + t.Run("emits model comments", func(t *testing.T) { + t.Parallel() + + jsonStr := `{ + "schema_version": "1.1", + "metadata": { + "model_comments": { + "preceding_lines": ["# Model comment"] + } + }, + "type_definitions": [{"type": "user"}] + }` + + dsl, err := transformer.TransformJSONStringToDSLWithComments(jsonStr) + require.NoError(t, err) + require.NotNil(t, dsl) + require.Contains(t, *dsl, "# Model comment\nmodel") + }) + + t.Run("emits type comments", func(t *testing.T) { + t.Parallel() + + jsonStr := `{ + "schema_version": "1.1", + "type_definitions": [{ + "type": "user", + "metadata": { + "comments": { + "preceding_lines": ["# User type"] + } + } + }] + }` + + dsl, err := transformer.TransformJSONStringToDSLWithComments(jsonStr) + require.NoError(t, err) + require.NotNil(t, dsl) + require.Contains(t, *dsl, "# User type\ntype user") + }) + + t.Run("emits relation comments", func(t *testing.T) { + t.Parallel() + + jsonStr := `{ + "schema_version": "1.1", + "type_definitions": [{ + "type": "document", + "relations": { + "owner": {"this": {}} + }, + "metadata": { + "relations": { + "owner": { + "directly_related_user_types": [{"type": "user"}], + "comments": { + "preceding_lines": ["# Owner relation"] + } + } + } + } + }] + }` + + dsl, err := transformer.TransformJSONStringToDSLWithComments(jsonStr) + require.NoError(t, err) + require.NotNil(t, dsl) + require.Contains(t, *dsl, "# Owner relation\n define owner:") + }) + + t.Run("emits condition comments", func(t *testing.T) { + t.Parallel() + + jsonStr := `{ + "schema_version": "1.1", + "type_definitions": [{"type": "user"}], + "conditions": { + "ip_check": { + "name": "ip_check", + "expression": "ip == \"127.0.0.1\"", + "parameters": { + "ip": {"type_name": "TYPE_NAME_STRING"} + }, + "metadata": { + "comments": { + "preceding_lines": ["# IP condition"] + } + } + } + } + }` + + dsl, err := transformer.TransformJSONStringToDSLWithComments(jsonStr) + require.NoError(t, err) + require.NotNil(t, dsl) + require.Contains(t, *dsl, "# IP condition\ncondition ip_check") + }) +} + +func TestRoundTripCommentPreservation(t *testing.T) { + t.Parallel() + + t.Run("round trip preserves comments", func(t *testing.T) { + t.Parallel() + + originalDSL := `# Model header comment +model + schema 1.1 + +# User type +type user + +# Document type +type document + relations + # Owner of document + define owner: [user] +` + + // DSL -> JSON + jsonStr, err := transformer.TransformDSLToJSONWithComments(originalDSL) + require.NoError(t, err) + + // JSON -> DSL + dsl, err := transformer.TransformJSONStringToDSLWithComments(jsonStr) + require.NoError(t, err) + require.NotNil(t, dsl) + + // Verify comments are preserved + require.Contains(t, *dsl, "# Model header comment") + require.Contains(t, *dsl, "# User type") + require.Contains(t, *dsl, "# Document type") + require.Contains(t, *dsl, "# Owner of document") + }) +} + +func TestBackwardCompatibility(t *testing.T) { + t.Parallel() + + t.Run("JSON without comments transforms correctly", func(t *testing.T) { + t.Parallel() + + jsonStr := `{ + "schema_version": "1.1", + "type_definitions": [{ + "type": "user" + }] + }` + + dsl, err := transformer.TransformJSONStringToDSLWithComments(jsonStr) + require.NoError(t, err) + require.NotNil(t, dsl) + require.Contains(t, *dsl, "type user") + }) + + t.Run("DSL without comments transforms correctly", func(t *testing.T) { + t.Parallel() + + dsl := `model + schema 1.1 + +type user` + + jsonStr, err := transformer.TransformDSLToJSONWithComments(dsl) + require.NoError(t, err) + require.NotEmpty(t, jsonStr) + + // Should not have model_comments if there were no comments + var result map[string]interface{} + err = json.Unmarshal([]byte(jsonStr), &result) + require.NoError(t, err) + + // Check metadata doesn't have model_comments if no comments + if metadata, ok := result["metadata"].(map[string]interface{}); ok { + _, hasModelComments := metadata["model_comments"] + require.False(t, hasModelComments) + } + }) +} diff --git a/pkg/go/transformer/dsltojson.go b/pkg/go/transformer/dsltojson.go index 6e7daa3a..895738fe 100644 --- a/pkg/go/transformer/dsltojson.go +++ b/pkg/go/transformer/dsltojson.go @@ -1,6 +1,7 @@ package transformer import ( + "encoding/json" "fmt" "strings" @@ -46,10 +47,19 @@ type OpenFgaDslListener struct { isModularModel bool moduleName string typeDefExtensions map[string]*openfgav1.TypeDefinition + + // Comment tracking + commentTracker *CommentTracker + modelComments *ModelComments + typeComments map[string]*CommentsMetadata + conditionComments map[string]*CommentInfo } func newOpenFgaDslListener() *OpenFgaDslListener { - return new(OpenFgaDslListener) + return &OpenFgaDslListener{ + typeComments: make(map[string]*CommentsMetadata), + conditionComments: make(map[string]*CommentInfo), + } } func ParseExpression(rewrites []*openfgav1.Userset, operator RelationDefinitionOperator) *openfgav1.Userset { @@ -127,8 +137,9 @@ func (l *OpenFgaDslListener) EnterTypeDef(ctx *parser.TypeDefContext) { ) } + typeName := ctx.GetTypeName().GetText() l.currentTypeDef = &openfgav1.TypeDefinition{ - Type: ctx.GetTypeName().GetText(), + Type: typeName, Relations: map[string]*openfgav1.Userset{}, Metadata: &openfgav1.Metadata{ Relations: map[string]*openfgav1.RelationMetadata{}, @@ -138,6 +149,19 @@ func (l *OpenFgaDslListener) EnterTypeDef(ctx *parser.TypeDefContext) { if l.isModularModel { l.currentTypeDef.Metadata.Module = l.moduleName } + + // Track type comments (line is 1-based from ANTLR, convert to 0-based) + // Use TYPE token's line number instead of GetStart() which may include preceding newlines + if l.commentTracker != nil && ctx.TYPE() != nil { + lineNum := ctx.TYPE().GetSymbol().GetLine() - 1 + commentInfo := l.commentTracker.GetCommentInfoForLine(lineNum) + if commentInfo != nil { + if l.typeComments[typeName] == nil { + l.typeComments[typeName] = &CommentsMetadata{} + } + l.typeComments[typeName].Comments = commentInfo + } + } } func (l *OpenFgaDslListener) EnterConditions(_ *parser.ConditionsContext) { @@ -168,6 +192,16 @@ func (l *OpenFgaDslListener) EnterCondition(ctx *parser.ConditionContext) { Module: l.moduleName, } } + + // Track condition comments (line is 1-based from ANTLR, convert to 0-based) + // Use CONDITION token's line number instead of GetStart() which may include preceding newlines + if l.commentTracker != nil && ctx.CONDITION() != nil { + lineNum := ctx.CONDITION().GetSymbol().GetLine() - 1 + commentInfo := l.commentTracker.GetCommentInfoForLine(lineNum) + if commentInfo != nil { + l.conditionComments[conditionName] = commentInfo + } + } } func (l *OpenFgaDslListener) ExitConditionParameter(ctx *parser.ConditionParameterContext) { @@ -298,6 +332,23 @@ func (l *OpenFgaDslListener) ExitRelationDeclaration(ctx *parser.RelationDeclara if l.isModularModel && isExtension { l.currentTypeDef.Metadata.Relations[relationName].Module = l.moduleName } + + // Track relation comments (line is 1-based from ANTLR, convert to 0-based) + // Use DEFINE token's line number instead of GetStart() which may include preceding newlines + if l.commentTracker != nil && l.currentTypeDef != nil && ctx.DEFINE() != nil { + typeName := l.currentTypeDef.GetType() + lineNum := ctx.DEFINE().GetSymbol().GetLine() - 1 + commentInfo := l.commentTracker.GetCommentInfoForLine(lineNum) + if commentInfo != nil { + if l.typeComments[typeName] == nil { + l.typeComments[typeName] = &CommentsMetadata{} + } + if l.typeComments[typeName].RelationComments == nil { + l.typeComments[typeName].RelationComments = make(map[string]*CommentInfo) + } + l.typeComments[typeName].RelationComments[relationName] = commentInfo + } + } } l.currentRelation = nil @@ -500,9 +551,19 @@ func (c *OpenFgaDslErrorListener) SyntaxError( /// func ParseDSL(data string) (*OpenFgaDslListener, *OpenFgaDslErrorListener) { + return ParseDSLWithOptions(data, true) +} + +// ParseDSLWithOptions parses DSL with optional comment preservation. +func ParseDSLWithOptions(data string, preserveComments bool) (*OpenFgaDslListener, *OpenFgaDslErrorListener) { + originalLines := strings.Split(data, "\n") cleanedLines := []string{} - for _, line := range strings.Split(data, "\n") { + // Build a mapping from cleaned line numbers to original line numbers + // (both 0-based indices) + cleanedToOriginal := make(map[int]int) + + for originalIdx, line := range originalLines { cleanedLine := "" switch { @@ -514,7 +575,15 @@ func ParseDSL(data string) (*OpenFgaDslListener, *OpenFgaDslErrorListener) { cleanedLine = strings.TrimRight(strings.Split(line, " #")[0], " ") } + cleanedIdx := len(cleanedLines) cleanedLines = append(cleanedLines, cleanedLine) + cleanedToOriginal[cleanedIdx] = originalIdx + } + + // Create comment tracker with original data and line mapping + var commentTracker *CommentTracker + if preserveComments { + commentTracker = NewCommentTrackerWithMapping(data, cleanedToOriginal) } cleanedData := strings.TrimRight(strings.Join(cleanedLines, "\n"), "\n") @@ -533,8 +602,15 @@ func ParseDSL(data string) (*OpenFgaDslListener, *OpenFgaDslErrorListener) { fgaParser.AddErrorListener(errorListener) listener := newOpenFgaDslListener() + listener.commentTracker = commentTracker + antlr.ParseTreeWalkerDefault.Walk(listener, fgaParser.Main()) + // Extract model comments after parsing + if preserveComments && commentTracker != nil { + listener.modelComments = commentTracker.GetModelComments() + } + return listener, errorListener } @@ -595,3 +671,178 @@ func TransformModularDSLToProto(data string) (*openfgav1.AuthorizationModel, map return &listener.authorizationModel, listener.typeDefExtensions, nil } + +// TransformResult contains the result of parsing DSL with comments preserved. +type TransformResult struct { + // AuthorizationModel is the parsed authorization model + AuthorizationModel *openfgav1.AuthorizationModel + // ModelComments contains comments at the model level + ModelComments *ModelComments + // TypeComments maps type names to their comments metadata + TypeComments map[string]*CommentsMetadata + // ConditionComments maps condition names to their comments + ConditionComments map[string]*CommentInfo +} + +// TransformDSLToProtoWithComments - Converts models authored in FGA DSL syntax to the OpenFGA Authorization Model +// Protobuf format while preserving comments metadata. +func TransformDSLToProtoWithComments(data string) (*TransformResult, error) { + listener, errorListener := ParseDSL(data) + + if errorListener.Errors != nil { + return nil, errorListener.Errors + } + + return &TransformResult{ + AuthorizationModel: &listener.authorizationModel, + ModelComments: listener.modelComments, + TypeComments: listener.typeComments, + ConditionComments: listener.conditionComments, + }, nil +} + +// TransformDSLToJSONWithComments - Converts models authored in FGA DSL syntax to the JSON syntax with comments +// preserved in the metadata. +func TransformDSLToJSONWithComments(data string) (string, error) { + result, err := TransformDSLToProtoWithComments(data) + if err != nil { + return "", err + } + + // First, marshal the proto to JSON + bytes, err := protojson.Marshal(result.AuthorizationModel) + if err != nil { + return "", fmt.Errorf("failed to marshal due to %w", err) + } + + // Parse into a generic map to inject comments + var jsonMap map[string]interface{} + if err := json.Unmarshal(bytes, &jsonMap); err != nil { + return "", fmt.Errorf("failed to unmarshal for comment injection: %w", err) + } + + // Inject model comments into top-level metadata + if result.ModelComments != nil && len(result.ModelComments.PrecedingLines) > 0 { + if jsonMap["metadata"] == nil { + jsonMap["metadata"] = make(map[string]interface{}) + } + metadata, ok := jsonMap["metadata"].(map[string]interface{}) + if ok { + metadata["model_comments"] = map[string]interface{}{ + "preceding_lines": result.ModelComments.PrecedingLines, + } + } + } + + // Inject type and relation comments + if typeDefinitions, ok := jsonMap["type_definitions"].([]interface{}); ok { + for _, td := range typeDefinitions { + typeDef, ok := td.(map[string]interface{}) + if !ok { + continue + } + + typeName, ok := typeDef["type"].(string) + if !ok { + continue + } + + typeComments := result.TypeComments[typeName] + if typeComments == nil { + continue + } + + // Ensure metadata exists + if typeDef["metadata"] == nil { + typeDef["metadata"] = make(map[string]interface{}) + } + metadata, ok := typeDef["metadata"].(map[string]interface{}) + if !ok { + continue + } + + // Add type-level comments + if typeComments.Comments != nil { + commentObj := make(map[string]interface{}) + if len(typeComments.Comments.PrecedingLines) > 0 { + commentObj["preceding_lines"] = typeComments.Comments.PrecedingLines + } + if typeComments.Comments.Inline != "" { + commentObj["inline"] = typeComments.Comments.Inline + } + if len(commentObj) > 0 { + metadata["comments"] = commentObj + } + } + + // Add relation comments + if len(typeComments.RelationComments) > 0 { + if metadata["relations"] == nil { + metadata["relations"] = make(map[string]interface{}) + } + relationsMetadata, ok := metadata["relations"].(map[string]interface{}) + if ok { + for relationName, relationComments := range typeComments.RelationComments { + // Ensure relation metadata exists + if relationsMetadata[relationName] == nil { + relationsMetadata[relationName] = make(map[string]interface{}) + } + relationMeta, ok := relationsMetadata[relationName].(map[string]interface{}) + if !ok { + continue + } + + commentObj := make(map[string]interface{}) + if len(relationComments.PrecedingLines) > 0 { + commentObj["preceding_lines"] = relationComments.PrecedingLines + } + if relationComments.Inline != "" { + commentObj["inline"] = relationComments.Inline + } + if len(commentObj) > 0 { + relationMeta["comments"] = commentObj + } + } + } + } + } + } + + // Inject condition comments + if conditions, ok := jsonMap["conditions"].(map[string]interface{}); ok { + for conditionName, condComments := range result.ConditionComments { + conditionData, ok := conditions[conditionName].(map[string]interface{}) + if !ok { + continue + } + + // Ensure metadata exists + if conditionData["metadata"] == nil { + conditionData["metadata"] = make(map[string]interface{}) + } + metadata, ok := conditionData["metadata"].(map[string]interface{}) + if !ok { + continue + } + + commentObj := make(map[string]interface{}) + if len(condComments.PrecedingLines) > 0 { + commentObj["preceding_lines"] = condComments.PrecedingLines + } + if condComments.Inline != "" { + commentObj["inline"] = condComments.Inline + } + if len(commentObj) > 0 { + metadata["comments"] = commentObj + } + } + } + + // Marshal back to JSON + finalBytes, err := json.Marshal(jsonMap) + if err != nil { + return "", fmt.Errorf("failed to marshal with comments: %w", err) + } + + return string(finalBytes), nil +} diff --git a/pkg/go/transformer/jsontodsl.go b/pkg/go/transformer/jsontodsl.go index e8b8bd5d..071eff1c 100644 --- a/pkg/go/transformer/jsontodsl.go +++ b/pkg/go/transformer/jsontodsl.go @@ -2,6 +2,7 @@ package transformer import ( "cmp" + "encoding/json" "fmt" "slices" "sort" @@ -14,6 +15,60 @@ import ( "github.com/openfga/language/pkg/go/errors" ) +// JSONModelWithComments represents the JSON structure with comments embedded. +type JSONModelWithComments struct { + SchemaVersion string `json:"schema_version"` + Metadata *JSONModelMetadata `json:"metadata,omitempty"` + TypeDefinitions []JSONTypeDefinitionWithComments `json:"type_definitions,omitempty"` + Conditions map[string]JSONConditionWithComments `json:"conditions,omitempty"` +} + +// JSONModelMetadata represents top-level model metadata including model comments. +type JSONModelMetadata struct { + ModelComments *JSONCommentBlock `json:"model_comments,omitempty"` +} + +// JSONCommentBlock represents a block of comments. +type JSONCommentBlock struct { + PrecedingLines []string `json:"preceding_lines,omitempty"` + Inline string `json:"inline,omitempty"` +} + +// JSONTypeDefinitionWithComments represents a type definition with comments. +type JSONTypeDefinitionWithComments struct { + Type string `json:"type"` + Relations map[string]json.RawMessage `json:"relations,omitempty"` + Metadata *JSONTypeMetadataWithComments `json:"metadata,omitempty"` +} + +// JSONTypeMetadataWithComments represents type metadata with comments. +type JSONTypeMetadataWithComments struct { + Module string `json:"module,omitempty"` + Comments *JSONCommentBlock `json:"comments,omitempty"` + Relations map[string]*JSONRelationMetadataWithComments `json:"relations,omitempty"` +} + +// JSONRelationMetadataWithComments represents relation metadata with comments. +type JSONRelationMetadataWithComments struct { + DirectlyRelatedUserTypes []json.RawMessage `json:"directly_related_user_types,omitempty"` + Module string `json:"module,omitempty"` + Comments *JSONCommentBlock `json:"comments,omitempty"` +} + +// JSONConditionWithComments represents a condition with comments. +type JSONConditionWithComments struct { + Name string `json:"name"` + Expression string `json:"expression"` + Parameters map[string]json.RawMessage `json:"parameters,omitempty"` + Metadata *JSONConditionMetadataWithComments `json:"metadata,omitempty"` +} + +// JSONConditionMetadataWithComments represents condition metadata with comments. +type JSONConditionMetadataWithComments struct { + Module string `json:"module,omitempty"` + Comments *JSONCommentBlock `json:"comments,omitempty"` +} + type DirectAssignmentValidator struct { occurred int } @@ -597,3 +652,323 @@ func sortByModule(aName, bName, aModule, bModule, aFile, bFile string) int { return cmp.Compare(aName, bName) } + +// formatCommentLines formats comment lines for DSL output. +func formatCommentLines(comments []string) string { + if len(comments) == 0 { + return "" + } + result := "" + for _, comment := range comments { + result += comment + "\n" + } + return result +} + +// formatInlineComment formats an inline comment for DSL output. +func formatInlineComment(comment string) string { + if comment == "" { + return "" + } + // Ensure the comment starts with # if it doesn't already + if !strings.HasPrefix(comment, "#") { + return " #" + comment + } + return " " + comment +} + +// TransformJSONStringToDSLWithComments - Converts models authored in OpenFGA JSON syntax to the DSL syntax, +// preserving any comments stored in the JSON metadata. +func TransformJSONStringToDSLWithComments(modelString string) (*string, error) { + // First try to parse as JSON with comments to extract comment metadata + var modelWithComments JSONModelWithComments + if err := json.Unmarshal([]byte(modelString), &modelWithComments); err != nil { + return nil, fmt.Errorf("failed to parse JSON: %w", err) + } + + // Also load via proto for the actual model transformation + model, err := LoadJSONStringToProto(modelString) + if err != nil { + return nil, err + } + + dsl, err := transformJSONProtoToDSLWithComments(model, &modelWithComments) + if err != nil { + return nil, err + } + + return &dsl, nil +} + +// transformJSONProtoToDSLWithComments converts the proto model to DSL while injecting comments. +func transformJSONProtoToDSLWithComments(model *openfgav1.AuthorizationModel, commentsModel *JSONModelWithComments) (string, error) { + schemaVersion := model.GetSchemaVersion() + + // Build model comments prefix + modelCommentsStr := "" + if commentsModel.Metadata != nil && commentsModel.Metadata.ModelComments != nil { + modelCommentsStr = formatCommentLines(commentsModel.Metadata.ModelComments.PrecedingLines) + } + + // Build type comments map + typeCommentsMap := make(map[string]*JSONTypeMetadataWithComments) + for _, typeDef := range commentsModel.TypeDefinitions { + if typeDef.Metadata != nil { + typeCommentsMap[typeDef.Type] = typeDef.Metadata + } + } + + typeDefinitions := []string{} + typeDefs := model.GetTypeDefinitions() + isModularModel := false + + for index := 0; index < len(typeDefs); index++ { + typeDef := typeDefs[index] + + if typeDef.GetMetadata().GetModule() != "" { + isModularModel = true + + break + } + } + + if isModularModel { + slices.SortStableFunc(typeDefs, func(a, b *openfgav1.TypeDefinition) int { + return sortByModule( + a.GetType(), b.GetType(), + a.GetMetadata().GetModule(), b.GetMetadata().GetModule(), + a.GetMetadata().GetSourceInfo().GetFile(), b.GetMetadata().GetSourceInfo().GetFile(), + ) + }) + } + + for index := 0; index < len(typeDefs); index++ { + typeDef := typeDefs[index] + + typeComments := typeCommentsMap[typeDef.GetType()] + parsedType, err := parseTypeWithComments(typeDef, isModularModel, false, typeComments) + if err != nil { + return "", err + } + + typeDefinitions = append(typeDefinitions, fmt.Sprintf("\n%v", parsedType)) + } + + typeDefsString := strings.Join(typeDefinitions, "\n") + if len(typeDefinitions) > 0 { + typeDefsString += "\n" + } + + parsedConditionsString, err := parseConditionsWithComments(model, false, commentsModel.Conditions) + if err != nil { + return "", err + } + + return fmt.Sprintf(`%vmodel + schema %v +%v%v`, modelCommentsStr, schemaVersion, typeDefsString, parsedConditionsString), nil +} + +// parseTypeWithComments parses a type definition including comments. +func parseTypeWithComments(typeDefinition *openfgav1.TypeDefinition, isModularModel, includeSourceInformation bool, typeComments *JSONTypeMetadataWithComments) (string, error) { + typeName := typeDefinition.GetType() + + // Build type comment prefix + typeCommentsStr := "" + typeInlineComment := "" + if typeComments != nil && typeComments.Comments != nil { + typeCommentsStr = formatCommentLines(typeComments.Comments.PrecedingLines) + typeInlineComment = formatInlineComment(typeComments.Comments.Inline) + } + + sourceString := constructSourceComment( + typeDefinition.GetMetadata().GetModule(), + typeDefinition.GetMetadata().GetSourceInfo().GetFile(), + "", + includeSourceInformation, + ) + + parsedTypeString := fmt.Sprintf(`%vtype %v%s%s`, typeCommentsStr, typeName, typeInlineComment, sourceString) + relations := typeDefinition.GetRelations() + metadata := typeDefinition.GetMetadata() + + if len(relations) > 0 { + parsedTypeString += "\n relations" + + relationsList := []string{} + for relation := range relations { + relationsList = append(relationsList, relation) + } + + // We are doing this in two loops (and sorting in between) + // to make sure we have a deterministic behaviour that matches the API + if isModularModel { + slices.SortStableFunc(relationsList, func(aName, bName string) int { + aMeta := metadata.GetRelations()[aName] + bMeta := metadata.GetRelations()[bName] + + return sortByModule( + aName, bName, + aMeta.GetModule(), bMeta.GetModule(), + aMeta.GetSourceInfo().GetFile(), bMeta.GetSourceInfo().GetFile(), + ) + }) + } else { + sort.Strings(relationsList) + } + + for index := 0; index < len(relationsList); index++ { + relationName := relationsList[index] + userset := relations[relationName] + meta := metadata.GetRelations()[relationName] + + // Get relation comments + var relationComments *JSONCommentBlock + if typeComments != nil && typeComments.Relations != nil { + if relMeta := typeComments.Relations[relationName]; relMeta != nil { + relationComments = relMeta.Comments + } + } + + parsedRelationString, err := parseRelationWithComments(typeName, relationName, userset, meta, includeSourceInformation, relationComments) + if err != nil { + return "", err + } + + parsedTypeString += fmt.Sprintf("\n%v", parsedRelationString) + } + } + + return parsedTypeString, nil +} + +// parseRelationWithComments parses a relation including comments. +func parseRelationWithComments( + typeName string, + relationName string, + relationDefinition *openfgav1.Userset, + relationMetadata *openfgav1.RelationMetadata, + includeSourceInformation bool, + comments *JSONCommentBlock, +) (string, error) { + validator := DirectAssignmentValidator{ + occurred: 0, + } + + // Build relation comment prefix + relationCommentsStr := "" + relationInlineComment := "" + if comments != nil { + for _, line := range comments.PrecedingLines { + relationCommentsStr += " " + line + "\n" + } + relationInlineComment = formatInlineComment(comments.Inline) + } + + sourceString := constructSourceComment( + relationMetadata.GetModule(), + relationMetadata.GetSourceInfo().GetFile(), + " extended by:", + includeSourceInformation, + ) + + typeRestrictions := relationMetadata.GetDirectlyRelatedUserTypes() + + parseFn := parseSubRelation + + switch { + case relationDefinition.GetDifference() != nil: + parseFn = parseDifference + case relationDefinition.GetUnion() != nil: + parseFn = parseUnion + case relationDefinition.GetIntersection() != nil: + parseFn = parseIntersection + } + + parsedRelationString, err := parseFn(typeName, relationName, relationDefinition, typeRestrictions, &validator) + if err != nil { + return "", err + } + + // Check if we have either no direct assignment, or we had exactly 1 direct assignment in the first position + if validator.occurrences() == 0 || (validator.occurrences() == 1 && validator.isFirstPosition(relationDefinition)) { + return fmt.Sprintf(`%s define %v: %v%s%s`, relationCommentsStr, relationName, parsedRelationString, relationInlineComment, sourceString), nil + } + + return "", errors.UnsupportedDSLNestingError(typeName, relationName) +} + +// parseConditionsWithComments parses conditions with comments. +func parseConditionsWithComments(model *openfgav1.AuthorizationModel, includeSourceInformation bool, conditionsWithComments map[string]JSONConditionWithComments) (string, error) { + conditionsMap := model.GetConditions() + if len(conditionsMap) == 0 { + return "", nil + } + + parsedConditionsString := "" + + conditionNames := []string{} + for conditionName := range conditionsMap { + conditionNames = append(conditionNames, conditionName) + } + + slices.SortStableFunc(conditionNames, func(aName, bName string) int { + aMeta := conditionsMap[aName].GetMetadata() + bMeta := conditionsMap[bName].GetMetadata() + + return sortByModule( + aName, bName, + aMeta.GetModule(), bMeta.GetModule(), + aMeta.GetSourceInfo().GetFile(), bMeta.GetSourceInfo().GetFile(), + ) + }) + + for index := 0; index < len(conditionNames); index++ { + conditionName := conditionNames[index] + condition := conditionsMap[conditionName] + + // Get condition comments + var comments *JSONCommentBlock + if condWithComments, ok := conditionsWithComments[conditionName]; ok && condWithComments.Metadata != nil { + comments = condWithComments.Metadata.Comments + } + + parsedConditionString, err := parseConditionWithComments(conditionName, condition, includeSourceInformation, comments) + if err != nil { + return "", err + } + + parsedConditionsString += fmt.Sprintf("\n%v", parsedConditionString) + } + + return parsedConditionsString, nil +} + +// parseConditionWithComments parses a condition with comments. +func parseConditionWithComments(conditionName string, conditionDef *openfgav1.Condition, includeSourceInformation bool, comments *JSONCommentBlock) (string, error) { + if conditionName != conditionDef.GetName() { + return "", errors.ConditionNameDoesntMatchError(conditionName, conditionDef.GetName()) + } + + // Build condition comment prefix + conditionCommentsStr := "" + if comments != nil { + conditionCommentsStr = formatCommentLines(comments.PrecedingLines) + } + + paramsString := parseConditionParams(conditionDef.GetParameters()) + sourceString := constructSourceComment( + conditionDef.GetMetadata().GetModule(), + conditionDef.GetMetadata().GetSourceInfo().GetFile(), + "", includeSourceInformation, + ) + + return fmt.Sprintf( + "%scondition %s(%s) {\n %s\n}%s\n", + conditionCommentsStr, + conditionDef.GetName(), + paramsString, + conditionDef.GetExpression(), + sourceString, + ), nil +} diff --git a/pkg/js/package-lock.json b/pkg/js/package-lock.json index d39e51f9..db1dcd00 100644 --- a/pkg/js/package-lock.json +++ b/pkg/js/package-lock.json @@ -3191,30 +3191,6 @@ "proxy-from-env": "^1.1.0" } }, - "node_modules/babel-jest": { - "version": "30.0.5", - "resolved": "https://registry.npmjs.org/babel-jest/-/babel-jest-30.0.5.tgz", - "integrity": "sha512-mRijnKimhGDMsizTvBTWotwNpzrkHr+VvZUQBof2AufXKB8NXrL1W69TG20EvOz7aevx6FTJIaBuBkYxS8zolg==", - "dev": true, - "license": "MIT", - "optional": true, - "peer": true, - "dependencies": { - "@jest/transform": "30.0.5", - "@types/babel__core": "^7.20.5", - "babel-plugin-istanbul": "^7.0.0", - "babel-preset-jest": "30.0.1", - "chalk": "^4.1.2", - "graceful-fs": "^4.2.11", - "slash": "^3.0.0" - }, - "engines": { - "node": "^18.14.0 || ^20.0.0 || ^22.0.0 || >=24.0.0" - }, - "peerDependencies": { - "@babel/core": "^7.11.0" - } - }, "node_modules/babel-plugin-istanbul": { "version": "7.0.0", "resolved": "https://registry.npmjs.org/babel-plugin-istanbul/-/babel-plugin-istanbul-7.0.0.tgz", @@ -3232,23 +3208,6 @@ "node": ">=12" } }, - "node_modules/babel-plugin-jest-hoist": { - "version": "30.0.1", - "resolved": "https://registry.npmjs.org/babel-plugin-jest-hoist/-/babel-plugin-jest-hoist-30.0.1.tgz", - "integrity": "sha512-zTPME3pI50NsFW8ZBaVIOeAxzEY7XHlmWeXXu9srI+9kNfzCUTy8MFan46xOGZY8NZThMqq+e3qZUKsvXbasnQ==", - "dev": true, - "license": "MIT", - "optional": true, - "peer": true, - "dependencies": { - "@babel/template": "^7.27.2", - "@babel/types": "^7.27.3", - "@types/babel__core": "^7.20.5" - }, - "engines": { - "node": "^18.14.0 || ^20.0.0 || ^22.0.0 || >=24.0.0" - } - }, "node_modules/babel-preset-current-node-syntax": { "version": "1.1.0", "resolved": "https://registry.npmjs.org/babel-preset-current-node-syntax/-/babel-preset-current-node-syntax-1.1.0.tgz", @@ -3276,25 +3235,6 @@ "@babel/core": "^7.0.0" } }, - "node_modules/babel-preset-jest": { - "version": "30.0.1", - "resolved": "https://registry.npmjs.org/babel-preset-jest/-/babel-preset-jest-30.0.1.tgz", - "integrity": "sha512-+YHejD5iTWI46cZmcc/YtX4gaKBtdqCHCVfuVinizVpbmyjO3zYmeuyFdfA8duRqQZfgCAMlsfmkVbJ+e2MAJw==", - "dev": true, - "license": "MIT", - "optional": true, - "peer": true, - "dependencies": { - "babel-plugin-jest-hoist": "30.0.1", - "babel-preset-current-node-syntax": "^1.1.0" - }, - "engines": { - "node": "^18.14.0 || ^20.0.0 || ^22.0.0 || >=24.0.0" - }, - "peerDependencies": { - "@babel/core": "^7.11.0" - } - }, "node_modules/balanced-match": { "version": "1.0.2", "resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-1.0.2.tgz", diff --git a/pkg/js/tests/comments.test.ts b/pkg/js/tests/comments.test.ts new file mode 100644 index 00000000..6090c59d --- /dev/null +++ b/pkg/js/tests/comments.test.ts @@ -0,0 +1,331 @@ +import { describe, expect, it } from "@jest/globals"; +import { + transformDSLToJSONWithComments, + transformDSLToJSONObjectWithComments, +} from "../transformer/dsltojson"; +import { + transformJSONStringToDSLWithComments, + transformJSONToDSLWithComments, +} from "../transformer/jsontodsl"; + +describe("Comment Preservation", () => { + describe("DSL to JSON with Comments", () => { + it("should preserve model comments", () => { + const dsl = `# OpenFGA Model +# Version 1.0 +model + schema 1.1 + +type user`; + + const result = transformDSLToJSONObjectWithComments(dsl); + + expect(result.modelComments).toBeDefined(); + expect(result.modelComments?.preceding_lines).toEqual([ + "# OpenFGA Model", + "# Version 1.0", + ]); + }); + + it("should preserve type comments", () => { + const dsl = `model + schema 1.1 + +# User type comment +type user`; + + const result = transformDSLToJSONObjectWithComments(dsl); + + expect(result.typeComments["user"]).toBeDefined(); + expect(result.typeComments["user"].comments?.preceding_lines).toEqual([ + "# User type comment", + ]); + }); + + it("should preserve relation comments", () => { + const dsl = `model + schema 1.1 + +type document + relations + # Owner comment + define owner: [user]`; + + const result = transformDSLToJSONObjectWithComments(dsl); + + expect(result.typeComments["document"]).toBeDefined(); + expect( + result.typeComments["document"].relation_comments?.["owner"] + ).toBeDefined(); + expect( + result.typeComments["document"].relation_comments?.["owner"] + .preceding_lines + ).toEqual(["# Owner comment"]); + }); + + it("should preserve condition comments", () => { + const dsl = `model + schema 1.1 + +type user + +# IP-based access control +condition ip_check(ip: string) { + ip == "127.0.0.1" +}`; + + const result = transformDSLToJSONObjectWithComments(dsl); + + expect(result.conditionComments["ip_check"]).toBeDefined(); + expect(result.conditionComments["ip_check"].preceding_lines).toEqual([ + "# IP-based access control", + ]); + }); + + it("should embed comments in JSON output", () => { + const dsl = `# Model comment +model + schema 1.1 + +# User type +type user`; + + const jsonStr = transformDSLToJSONWithComments(dsl); + const json = JSON.parse(jsonStr); + + expect(json.metadata?.model_comments?.preceding_lines).toEqual([ + "# Model comment", + ]); + expect(json.type_definitions?.[0].metadata?.comments?.preceding_lines).toEqual([ + "# User type", + ]); + }); + }); + + describe("JSON to DSL with Comments", () => { + it("should emit model comments", () => { + const json = { + schema_version: "1.1", + metadata: { + model_comments: { + preceding_lines: ["# Model comment"], + }, + }, + type_definitions: [{ type: "user" }], + }; + + const dsl = transformJSONToDSLWithComments(json); + + expect(dsl).toContain("# Model comment\nmodel"); + }); + + it("should emit type comments", () => { + const json = { + schema_version: "1.1", + type_definitions: [ + { + type: "user", + metadata: { + comments: { + preceding_lines: ["# User type"], + }, + }, + }, + ], + }; + + const dsl = transformJSONToDSLWithComments(json); + + expect(dsl).toContain("# User type\ntype user"); + }); + + it("should emit relation comments", () => { + const json = { + schema_version: "1.1", + type_definitions: [ + { + type: "document", + relations: { + owner: { this: {} }, + }, + metadata: { + relations: { + owner: { + directly_related_user_types: [{ type: "user" }], + comments: { + preceding_lines: ["# Owner relation"], + }, + }, + }, + }, + }, + ], + }; + + const dsl = transformJSONToDSLWithComments(json); + + expect(dsl).toContain("# Owner relation\n define owner:"); + }); + + it("should emit condition comments", () => { + const json = { + schema_version: "1.1", + type_definitions: [{ type: "user" }], + conditions: { + ip_check: { + name: "ip_check", + expression: 'ip == "127.0.0.1"', + parameters: { + ip: { type_name: "TYPE_NAME_STRING" as const }, + }, + metadata: { + comments: { + preceding_lines: ["# IP condition"], + }, + }, + }, + }, + }; + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const dsl = transformJSONToDSLWithComments(json as any); + + expect(dsl).toContain("# IP condition\ncondition ip_check"); + }); + + it("should parse JSON string with comments", () => { + const jsonStr = `{ + "schema_version": "1.1", + "metadata": { + "model_comments": { + "preceding_lines": ["# Model comment"] + } + }, + "type_definitions": [{"type": "user"}] + }`; + + const dsl = transformJSONStringToDSLWithComments(jsonStr); + + expect(dsl).toContain("# Model comment\nmodel"); + }); + }); + + describe("Round Trip Comment Preservation", () => { + it("should preserve comments through DSL -> JSON -> DSL", () => { + const originalDSL = `# Model header comment +model + schema 1.1 + +# User type +type user + +# Document type +type document + relations + # Owner of document + define owner: [user] +`; + + // DSL -> JSON + const jsonStr = transformDSLToJSONWithComments(originalDSL); + + // JSON -> DSL + const dsl = transformJSONStringToDSLWithComments(jsonStr); + + // Verify comments are preserved + expect(dsl).toContain("# Model header comment"); + expect(dsl).toContain("# User type"); + expect(dsl).toContain("# Document type"); + expect(dsl).toContain("# Owner of document"); + }); + + it("should handle multiple preceding comments", () => { + const originalDSL = `# Comment line 1 +# Comment line 2 +# Comment line 3 +model + schema 1.1 + +type user +`; + + const jsonStr = transformDSLToJSONWithComments(originalDSL); + const dsl = transformJSONStringToDSLWithComments(jsonStr); + + expect(dsl).toContain("# Comment line 1"); + expect(dsl).toContain("# Comment line 2"); + expect(dsl).toContain("# Comment line 3"); + }); + }); + + describe("Backward Compatibility", () => { + it("should handle JSON without comments", () => { + const json = { + schema_version: "1.1", + type_definitions: [{ type: "user" }], + }; + + const dsl = transformJSONToDSLWithComments(json); + + expect(dsl).toContain("type user"); + // Should not throw and should produce valid DSL + }); + + it("should handle DSL without comments", () => { + const dsl = `model + schema 1.1 + +type user`; + + const result = transformDSLToJSONObjectWithComments(dsl); + + // Should not have model comments + expect(result.modelComments).toBeUndefined(); + + // Type comments should be empty + expect(result.typeComments["user"]).toBeUndefined(); + }); + }); + + describe("Edge Cases", () => { + it("should handle empty lines between comments and elements", () => { + // Empty lines should break comment association + const dsl = `# This comment should not be associated + +model + schema 1.1 + +type user`; + + const result = transformDSLToJSONObjectWithComments(dsl); + + // The empty line breaks the association + expect(result.modelComments).toBeUndefined(); + }); + + it("should preserve inline comments", () => { + const dsl = `model + schema 1.1 + +type user # inline comment`; + + const result = transformDSLToJSONObjectWithComments(dsl); + + expect(result.typeComments["user"]).toBeDefined(); + expect(result.typeComments["user"].comments?.inline).toBe("# inline comment"); + }); + + it("should handle special characters in comments", () => { + const dsl = `# Comment with special chars: @#$%^&*() +model + schema 1.1 + +type user`; + + const jsonStr = transformDSLToJSONWithComments(dsl); + const dslResult = transformJSONStringToDSLWithComments(jsonStr); + + expect(dslResult).toContain("# Comment with special chars: @#$%^&*()"); + }); + }); +}); diff --git a/pkg/js/transformer/dsltojson.ts b/pkg/js/transformer/dsltojson.ts index 3c1c1408..4990441c 100644 --- a/pkg/js/transformer/dsltojson.ts +++ b/pkg/js/transformer/dsltojson.ts @@ -29,6 +29,124 @@ import OpenFGAParser, { import { DSLSyntaxError, DSLSyntaxSingleError } from "../errors"; import { TypeName } from "@openfga/sdk"; +// Comment tracking types +export interface CommentInfo { + preceding_lines?: string[]; + inline?: string; +} + +export interface CommentsMetadata { + comments?: CommentInfo; + relation_comments?: Record; +} + +export interface ModelComments { + preceding_lines?: string[]; +} + +// Comment Tracker class for extracting comments from DSL source +class CommentTracker { + private lines: string[]; + private cleanedToOriginal?: Record; + + constructor(source: string, cleanedToOriginal?: Record) { + this.lines = source.split("\n"); + this.cleanedToOriginal = cleanedToOriginal; + } + + // Get comments that immediately precede the given line number (0-based) + getPrecedingComments(lineNum: number): string[] { + if (lineNum <= 0 || lineNum > this.lines.length) { + return []; + } + + const comments: string[] = []; + // Walk backwards from the line before the target + for (let i = lineNum - 1; i >= 0; i--) { + const line = this.lines[i].trim(); + if (line.length === 0) { + // Empty line breaks the contiguous comment block + break; + } + if (line.startsWith("#")) { + // Prepend to maintain order + comments.unshift(line); + } else { + // Non-comment, non-empty line breaks the block + break; + } + } + + return comments; + } + + // Get the inline comment for the given line number (0-based) + getInlineComment(lineNum: number): string { + if (lineNum < 0 || lineNum >= this.lines.length) { + return ""; + } + + const line = this.lines[lineNum]; + // Find inline comment + const inlineIdx = line.indexOf(" #"); + if (inlineIdx !== -1) { + const beforeComment = line.slice(0, inlineIdx).trim(); + if (beforeComment.length > 0) { + return line.slice(inlineIdx + 1).trim(); + } + } + return ""; + } + + // Get comment info for an element at the given line + // lineNum is expected to be 0-based and may be from cleaned source (if mapping exists) + getCommentInfoForLine(lineNum: number): CommentInfo | undefined { + // If we have a line mapping, convert from cleaned line number to original + let originalLineNum = lineNum; + if (this.cleanedToOriginal !== undefined && lineNum in this.cleanedToOriginal) { + originalLineNum = this.cleanedToOriginal[lineNum]; + } + + const preceding = this.getPrecedingComments(originalLineNum); + const inline = this.getInlineComment(originalLineNum); + + if (preceding.length === 0 && !inline) { + return undefined; + } + + return { + preceding_lines: preceding.length > 0 ? preceding : undefined, + inline: inline || undefined, + }; + } + + // Get model comments (comments before the model declaration) + getModelComments(): ModelComments | undefined { + // Find the model declaration line + let modelLine = -1; + for (let i = 0; i < this.lines.length; i++) { + const trimmed = this.lines[i].trim(); + if (trimmed.startsWith("model")) { + modelLine = i; + break; + } + } + + if (modelLine <= 0) { + return undefined; + } + + const precedingComments = this.getPrecedingComments(modelLine); + if (precedingComments.length === 0) { + return undefined; + } + + return { + preceding_lines: precedingComments, + }; + } +} + enum RelationDefinitionOperator { RELATION_DEFINITION_OPERATOR_NONE = "", RELATION_DEFINITION_OPERATOR_OR = "or", @@ -91,7 +209,7 @@ interface StackRelation { } /** - * This Visitor walks the tree generated by parsers and produces Python code + * This Visitor walks the tree generated by parsers and produces the authorization model * * @returns {object} */ @@ -99,6 +217,12 @@ class OpenFgaDslListener extends OpenFGAListener { public authorizationModel: Partial = {}; public typeDefExtensions: Map = new Map(); + // Comment tracking + public commentTracker?: CommentTracker; + public modelComments?: ModelComments; + public typeComments: Record = {}; + public conditionComments: Record = {}; + private currentTypeDef: Partial | undefined; private currentRelation: Partial | undefined; private currentCondition: Condition | undefined; @@ -144,8 +268,9 @@ class OpenFgaDslListener extends OpenFGAListener { ctx.parser?.notifyErrorListeners("extend can only be used in a modular model", ctx._typeName.start, undefined); } + const typeName = ctx._typeName.getText(); this.currentTypeDef = { - type: ctx._typeName.getText(), + type: typeName, relations: {}, metadata: { relations: {} }, }; @@ -153,6 +278,19 @@ class OpenFgaDslListener extends OpenFGAListener { if (this.isModularModel) { this.currentTypeDef.metadata!.module = this.moduleName; } + + // Track type comments (line is 1-based from ANTLR, convert to 0-based) + // Use TYPE token's line number instead of ctx.start which may include preceding newlines + if (this.commentTracker && ctx.TYPE()) { + const lineNum = ctx.TYPE().symbol.line - 1; + const commentInfo = this.commentTracker.getCommentInfoForLine(lineNum); + if (commentInfo) { + if (!this.typeComments[typeName]) { + this.typeComments[typeName] = {}; + } + this.typeComments[typeName].comments = commentInfo; + } + } }; exitTypeDef = (ctx: TypeDefContext) => { @@ -225,6 +363,23 @@ class OpenFgaDslListener extends OpenFGAListener { if (this.isModularModel && (ctx.parentCtx as TypeDefContext).EXTEND()) { this.currentTypeDef!.metadata!.relations![relationName].module = this.moduleName; } + + // Track relation comments (line is 1-based from ANTLR, convert to 0-based) + // Use DEFINE token's line number instead of ctx.start which may include preceding newlines + if (this.commentTracker && this.currentTypeDef && ctx.DEFINE()) { + const typeName = this.currentTypeDef.type!; + const lineNum = ctx.DEFINE().symbol.line - 1; + const commentInfo = this.commentTracker.getCommentInfoForLine(lineNum); + if (commentInfo) { + if (!this.typeComments[typeName]) { + this.typeComments[typeName] = {}; + } + if (!this.typeComments[typeName].relation_comments) { + this.typeComments[typeName].relation_comments = {}; + } + this.typeComments[typeName].relation_comments![relationName] = commentInfo; + } + } } this.currentRelation = undefined; @@ -360,6 +515,16 @@ class OpenFgaDslListener extends OpenFGAListener { module: this.moduleName, }; } + + // Track condition comments (line is 1-based from ANTLR, convert to 0-based) + // Use CONDITION token's line number instead of ctx.start which may include preceding newlines + if (this.commentTracker && ctx.CONDITION()) { + const lineNum = ctx.CONDITION().symbol.line - 1; + const commentInfo = this.commentTracker.getCommentInfoForLine(lineNum); + if (commentInfo) { + this.conditionComments[conditionName] = commentInfo; + } + } }; exitConditionParameter = (ctx: ConditionParameterContext) => { @@ -441,20 +606,41 @@ class OpenFgaDslErrorListener extends ErrorListener { } } -export function parseDSL(data: string): { +export function parseDSL( + data: string, + preserveComments = true, +): { listener: OpenFgaDslListener; errorListener: OpenFgaDslErrorListener; } { - const cleanedData = data - .split("\n") - .map((line) => { - if (line.trimStart()[0] === "#") { - return ""; - } + const originalLines = data.split("\n"); - return line.split(" #")[0].trimEnd(); - }) - .join("\n"); + // Build a mapping from cleaned line numbers to original line numbers + // (both 0-based indices) + const cleanedToOriginal: Record = {}; + const cleanedLines: string[] = []; + + for (let originalIdx = 0; originalIdx < originalLines.length; originalIdx++) { + const line = originalLines[originalIdx]; + let cleanedLine = ""; + + if (line.trimStart().length === 0) { + // Empty line + } else if (line.trimStart()[0] === "#") { + cleanedLine = ""; + } else { + cleanedLine = line.split(" #")[0].trimEnd(); + } + + const cleanedIdx = cleanedLines.length; + cleanedLines.push(cleanedLine); + cleanedToOriginal[cleanedIdx] = originalIdx; + } + + // Create comment tracker with original data and line mapping + const commentTracker = preserveComments ? new CommentTracker(data, cleanedToOriginal) : undefined; + + const cleanedData = cleanedLines.join("\n"); const is = new antlr.InputStream(cleanedData); @@ -473,8 +659,15 @@ export function parseDSL(data: string): { // Finally parse the expression const listener = new OpenFgaDslListener(); + listener.commentTracker = commentTracker; + new antlr.ParseTreeWalker().walk(listener, parser.main()); + // Extract model comments after parsing + if (preserveComments && commentTracker) { + listener.modelComments = commentTracker.getModelComments(); + } + return { listener, errorListener }; } @@ -526,3 +719,143 @@ export function transformModularDSLToJSONObject(data: string): ModularDSLTransfo typeDefExtensions: listener.typeDefExtensions, }; } + +// Interface for DSL to JSON transformation result with comments +export interface TransformResultWithComments { + authorizationModel: Omit; + modelComments?: ModelComments; + typeComments: Record; + conditionComments: Record; +} + +/** + * transformDSLToJSONObjectWithComments - Converts models authored in FGA DSL syntax to the json syntax + * while preserving comments metadata. + * @param {string} data + * @returns {TransformResultWithComments} + */ +export function transformDSLToJSONObjectWithComments(data: string): TransformResultWithComments { + const { listener, errorListener } = parseDSL(data, true); + + if (errorListener.errors.length) { + throw new DSLSyntaxError(errorListener.errors); + } + + return { + authorizationModel: listener.authorizationModel as Omit, + modelComments: listener.modelComments, + typeComments: listener.typeComments, + conditionComments: listener.conditionComments, + }; +} + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +type JSONValue = string | number | boolean | null | JSONValue[] | { [key: string]: JSONValue }; + +/** + * transformDSLToJSONWithComments - Converts models authored in FGA DSL syntax to a stringified json representation + * with comments embedded in the metadata. + * @param {string} data + * @returns {string} + */ +export function transformDSLToJSONWithComments(data: string): string { + const result = transformDSLToJSONObjectWithComments(data); + + // Convert to JSON and inject comments + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const jsonObj: Record = result.authorizationModel as any; + + // Add model comments to top-level metadata + if (result.modelComments && result.modelComments.preceding_lines?.length) { + if (!jsonObj.metadata) { + jsonObj.metadata = {}; + } + (jsonObj.metadata as Record).model_comments = { + preceding_lines: result.modelComments.preceding_lines, + }; + } + + // Add type and relation comments + const typeDefinitions = jsonObj.type_definitions as Record[] | undefined; + if (typeDefinitions) { + for (const typeDef of typeDefinitions) { + const typeName = typeDef.type as string; + const typeComments = result.typeComments[typeName]; + if (!typeComments) continue; + + // Ensure metadata exists + if (!typeDef.metadata) { + typeDef.metadata = {}; + } + const metadata = typeDef.metadata as Record; + + // Add type-level comments + if (typeComments.comments) { + const commentObj: Record = {}; + if (typeComments.comments.preceding_lines?.length) { + commentObj.preceding_lines = typeComments.comments.preceding_lines; + } + if (typeComments.comments.inline) { + commentObj.inline = typeComments.comments.inline; + } + if (Object.keys(commentObj).length > 0) { + metadata.comments = commentObj; + } + } + + // Add relation comments + if (typeComments.relation_comments) { + if (!metadata.relations) { + metadata.relations = {}; + } + const relationsMetadata = metadata.relations as Record; + + for (const [relationName, relationComments] of Object.entries(typeComments.relation_comments)) { + if (!relationsMetadata[relationName]) { + relationsMetadata[relationName] = {}; + } + const relationMeta = relationsMetadata[relationName] as Record; + + const commentObj: Record = {}; + if (relationComments.preceding_lines?.length) { + commentObj.preceding_lines = relationComments.preceding_lines; + } + if (relationComments.inline) { + commentObj.inline = relationComments.inline; + } + if (Object.keys(commentObj).length > 0) { + relationMeta.comments = commentObj; + } + } + } + } + } + + // Add condition comments + const conditions = jsonObj.conditions as Record> | undefined; + if (conditions) { + for (const [conditionName, condComments] of Object.entries(result.conditionComments)) { + const conditionData = conditions[conditionName]; + if (!conditionData) continue; + + // Ensure metadata exists + if (!conditionData.metadata) { + conditionData.metadata = {}; + } + const metadata = conditionData.metadata as Record; + + const commentObj: Record = {}; + if (condComments.preceding_lines?.length) { + commentObj.preceding_lines = condComments.preceding_lines; + } + if (condComments.inline) { + commentObj.inline = condComments.inline; + } + if (Object.keys(commentObj).length > 0) { + metadata.comments = commentObj; + } + } + } + + return JSON.stringify(jsonObj); +} diff --git a/pkg/js/transformer/jsontodsl.ts b/pkg/js/transformer/jsontodsl.ts index 680ea84a..e9847d43 100644 --- a/pkg/js/transformer/jsontodsl.ts +++ b/pkg/js/transformer/jsontodsl.ts @@ -432,3 +432,255 @@ export function getModulesFromJSON(model: Omit): strin return Object.keys(modulesMap).sort(); } + +// Types for JSON with comments +interface CommentBlock { + preceding_lines?: string[]; + inline?: string; +} + +interface TypeMetadataWithComments extends Metadata { + comments?: CommentBlock; + relations?: Record< + string, + RelationMetadata & { + comments?: CommentBlock; + } + >; +} + +interface TypeDefinitionWithComments extends Omit { + metadata?: TypeMetadataWithComments; +} + +interface ConditionMetadataWithComments extends ConditionMetadata { + comments?: CommentBlock; +} + +interface ConditionWithComments extends Omit { + metadata?: ConditionMetadataWithComments; +} + +interface ModelMetadataWithComments { + model_comments?: { + preceding_lines?: string[]; + }; +} + +interface AuthorizationModelWithComments extends Omit { + metadata?: ModelMetadataWithComments; + type_definitions?: TypeDefinitionWithComments[]; + conditions?: Record; +} + +// Helper functions for formatting comments +function formatCommentLines(comments: string[] | undefined): string { + if (!comments?.length) { + return ""; + } + return comments.map((line) => line + "\n").join(""); +} + +function formatInlineComment(comment: string | undefined): string { + if (!comment) { + return ""; + } + // Ensure the comment starts with # if it doesn't already + if (!comment.startsWith("#")) { + return " #" + comment; + } + return " " + comment; +} + +// Parse type with comments +const parseTypeWithComments = ( + typeDef: TypeDefinitionWithComments, + isModularModel: boolean, + includeSourceInformation = false, +): string => { + const typeName = typeDef.type; + const metadata = typeDef.metadata as TypeMetadataWithComments | undefined; + + // Build type comment prefix + let typeCommentsStr = ""; + let typeInlineComment = ""; + if (metadata?.comments) { + typeCommentsStr = formatCommentLines(metadata.comments.preceding_lines); + typeInlineComment = formatInlineComment(metadata.comments.inline); + } + + const sourceString = constructSourceComment(typeDef.metadata, "", includeSourceInformation); + let parsedTypeString = `\n${typeCommentsStr}type ${typeName}${typeInlineComment}${sourceString}`; + + const relations = typeDef.relations || {}; + + if (Object.keys(relations)?.length) { + parsedTypeString += "\n relations"; + const sortedRelations = Object.entries(relations).sort(([aName], [bName]) => { + if (!isModularModel) { + return 0; + } + const aMetadata = typeDef.metadata?.relations?.[aName] || {}; + const bMetadata = typeDef.metadata?.relations?.[bName] || {}; + + return sortByModule(aName, bName, aMetadata as Metadata, bMetadata as Metadata); + }); + for (const [name, definition] of sortedRelations) { + const relationMeta = metadata?.relations?.[name]; + const relationComments = relationMeta?.comments; + + const parsedRelationString = parseRelationWithComments( + typeName, + name, + definition, + relationMeta as RelationMetadata, + includeSourceInformation, + relationComments, + ); + parsedTypeString += `\n${parsedRelationString}`; + } + } + + return parsedTypeString; +}; + +// Parse relation with comments +function parseRelationWithComments( + typeName: string, + relationName: string, + relationDefinition: Userset = {}, + relationMetadata: RelationMetadata = {}, + includeSourceInformation = false, + comments?: CommentBlock, +) { + const validator = new DirectAssignmentValidator(); + + // Build relation comment prefix + let relationCommentsStr = ""; + let relationInlineComment = ""; + if (comments) { + for (const line of comments.preceding_lines || []) { + relationCommentsStr += " " + line + "\n"; + } + relationInlineComment = formatInlineComment(comments.inline); + } + + let parsedRelationString = `${relationCommentsStr} define ${relationName}: `; + const typeRestrictions: RelationReference[] = relationMetadata.directly_related_user_types || []; + + if (relationDefinition.difference != null) { + parsedRelationString += parseDifference(typeName, relationName, relationDefinition, typeRestrictions, validator); + } else if (relationDefinition.union != null) { + parsedRelationString += parseUnion(typeName, relationName, relationDefinition, typeRestrictions, validator); + } else if (relationDefinition.intersection != null) { + parsedRelationString += parseIntersection(typeName, relationName, relationDefinition, typeRestrictions, validator); + } else { + parsedRelationString += parseSubRelation(typeName, relationName, relationDefinition, typeRestrictions, validator); + } + + parsedRelationString += relationInlineComment; + parsedRelationString += constructSourceComment(relationMetadata, " extended by:", includeSourceInformation); + + // Check if we have either no direct assignment, or we had exactly 1 direct assignment in the first position + if (!validator.occured || (validator.occured === 1 && validator.isFirstPosition(relationDefinition))) { + return parsedRelationString; + } + + throw new Error( + `the '${relationName}' relation definition under the '${typeName}' type is not supported by the OpenFGA DSL syntax yet`, + ); +} + +// Parse condition with comments +const parseConditionWithComments = ( + conditionName: string, + conditionDef: ConditionWithComments, + includeSourceInformation = false, +): string => { + if (conditionName != conditionDef.name) { + throw new ConditionNameDoesntMatchError(conditionName, conditionDef.name); + } + + // Build condition comment prefix + let conditionCommentsStr = ""; + if (conditionDef.metadata?.comments) { + conditionCommentsStr = formatCommentLines(conditionDef.metadata.comments.preceding_lines); + } + + const paramsString = parseConditionParams(conditionDef.parameters || {}); + const sourceString = constructSourceComment(conditionDef.metadata, "", includeSourceInformation); + + return `${conditionCommentsStr}condition ${conditionName}(${paramsString}) {\n ${conditionDef.expression}\n}${sourceString}\n`; +}; + +// Parse conditions with comments +const parseConditionsWithComments = ( + model: AuthorizationModelWithComments, + isModularModel: boolean, + includeSourceInformation = false, +): string => { + const conditionsMap = model.conditions || {}; + if (!Object.keys(conditionsMap).length) { + return ""; + } + + let parsedConditionsString = ""; + + Object.entries(conditionsMap) + .sort(([aName, aCondition], [bName, bCondition]) => { + if (!isModularModel) { + return aName.localeCompare(bName); + } + return sortByModule(aName, bName, aCondition.metadata as Metadata, bCondition.metadata as Metadata); + }) + .forEach(([conditionName, condition]) => { + const parsedConditionString = parseConditionWithComments(conditionName, condition, includeSourceInformation); + + parsedConditionsString += `\n${parsedConditionString}`; + }); + + return parsedConditionsString; +}; + +/** + * transformJSONToDSLWithComments - Converts models authored in OpenFGA JSON syntax to the DSL syntax, + * preserving any comments stored in the JSON metadata. + * @param {AuthorizationModelWithComments} model + * @returns {string} + */ +export const transformJSONToDSLWithComments = (model: AuthorizationModelWithComments): string => { + const schemaVersion = model?.schema_version || "1.1"; + const isModularModel = model.type_definitions?.some((typeDef) => typeDef.metadata?.module); + + // Build model comments prefix + let modelCommentsStr = ""; + if (model.metadata?.model_comments?.preceding_lines?.length) { + modelCommentsStr = formatCommentLines(model.metadata.model_comments.preceding_lines); + } + + const typeDefinitions = ( + isModularModel + ? model?.type_definitions?.sort((a, b) => + sortByModule(a.type, b.type, a.metadata as Metadata, b.metadata as Metadata), + ) + : model?.type_definitions + )?.map((typeDef) => parseTypeWithComments(typeDef, isModularModel ?? false, false)); + + const parsedConditionsString = parseConditionsWithComments(model, isModularModel ?? false, false); + + return `${modelCommentsStr}model + schema ${schemaVersion} +${typeDefinitions ? `${typeDefinitions.join("\n")}\n` : ""}${parsedConditionsString}`; +}; + +/** + * transformJSONStringToDSLWithComments - Converts models authored in OpenFGA JSON syntax (as string) + * to the DSL syntax, preserving any comments stored in the JSON metadata. + * @param {string} modelString + * @returns {string} + */ +export const transformJSONStringToDSLWithComments = (modelString: string): string => { + const model = JSON.parse(modelString) as AuthorizationModelWithComments; + + return transformJSONToDSLWithComments(model); +}; diff --git a/tests/data/transformer/200-comment-preservation/authorization-model-with-comments.json b/tests/data/transformer/200-comment-preservation/authorization-model-with-comments.json new file mode 100644 index 00000000..f50b3aae --- /dev/null +++ b/tests/data/transformer/200-comment-preservation/authorization-model-with-comments.json @@ -0,0 +1,67 @@ +{ + "schema_version": "1.1", + "metadata": { + "model_comments": { + "preceding_lines": ["# OpenFGA Authorization Model", "# Version 2.0"] + } + }, + "type_definitions": [ + { + "type": "user", + "metadata": { + "comments": { + "preceding_lines": ["# User type for representing users"] + } + } + }, + { + "type": "document", + "relations": { + "owner": { + "this": {} + }, + "viewer": { + "union": { + "child": [ + { + "this": {} + }, + { + "computedUserset": { + "relation": "owner" + } + } + ] + } + } + }, + "metadata": { + "comments": { + "preceding_lines": ["# Document type for file storage"] + }, + "relations": { + "owner": { + "directly_related_user_types": [ + { + "type": "user" + } + ], + "comments": { + "preceding_lines": ["# Owner of the document"] + } + }, + "viewer": { + "directly_related_user_types": [ + { + "type": "user" + } + ], + "comments": { + "preceding_lines": ["# Who can view this document"] + } + } + } + } + } + ] +} diff --git a/tests/data/transformer/200-comment-preservation/authorization-model.fga b/tests/data/transformer/200-comment-preservation/authorization-model.fga new file mode 100644 index 00000000..88827312 --- /dev/null +++ b/tests/data/transformer/200-comment-preservation/authorization-model.fga @@ -0,0 +1,9 @@ +model + schema 1.1 + +type user + +type document + relations + define owner: [user] + define viewer: [user] or owner diff --git a/tests/data/transformer/200-comment-preservation/authorization-model.json b/tests/data/transformer/200-comment-preservation/authorization-model.json new file mode 100644 index 00000000..de921516 --- /dev/null +++ b/tests/data/transformer/200-comment-preservation/authorization-model.json @@ -0,0 +1,48 @@ +{ + "schema_version": "1.1", + "type_definitions": [ + { + "type": "user" + }, + { + "type": "document", + "relations": { + "owner": { + "this": {} + }, + "viewer": { + "union": { + "child": [ + { + "this": {} + }, + { + "computedUserset": { + "relation": "owner" + } + } + ] + } + } + }, + "metadata": { + "relations": { + "owner": { + "directly_related_user_types": [ + { + "type": "user" + } + ] + }, + "viewer": { + "directly_related_user_types": [ + { + "type": "user" + } + ] + } + } + } + } + ] +} From 62916f3a4529e15f0da39eb9a6b948b1696fa43b Mon Sep 17 00:00:00 2001 From: Poovamraj T T Date: Mon, 19 Jan 2026 18:22:53 +0530 Subject: [PATCH 2/5] Fix tests/dsltojson.test.ts --- .../200-comment-preservation/authorization-model.json | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/data/transformer/200-comment-preservation/authorization-model.json b/tests/data/transformer/200-comment-preservation/authorization-model.json index de921516..994e9943 100644 --- a/tests/data/transformer/200-comment-preservation/authorization-model.json +++ b/tests/data/transformer/200-comment-preservation/authorization-model.json @@ -2,7 +2,9 @@ "schema_version": "1.1", "type_definitions": [ { - "type": "user" + "type": "user", + "relations": {}, + "metadata": null }, { "type": "document", From bf8e4992a13db0c9ab0d7d2ac2d77e3b2bcb3d89 Mon Sep 17 00:00:00 2001 From: Poovamraj T T Date: Mon, 19 Jan 2026 18:24:04 +0530 Subject: [PATCH 3/5] Fix linting error --- pkg/js/tests/comments.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/js/tests/comments.test.ts b/pkg/js/tests/comments.test.ts index 6090c59d..6b253606 100644 --- a/pkg/js/tests/comments.test.ts +++ b/pkg/js/tests/comments.test.ts @@ -174,7 +174,7 @@ type user`; conditions: { ip_check: { name: "ip_check", - expression: 'ip == "127.0.0.1"', + expression: "ip == \"127.0.0.1\"", parameters: { ip: { type_name: "TYPE_NAME_STRING" as const }, }, From 5016b905846f1e2884f1c7d361a29fc0afd43e53 Mon Sep 17 00:00:00 2001 From: Poovamraj T T Date: Mon, 19 Jan 2026 18:27:06 +0530 Subject: [PATCH 4/5] Fix linting issue --- pkg/js/errors.ts | 3 ++- pkg/js/transformer/jsontodsl.ts | 9 ++++++--- pkg/js/util/exceptions.ts | 6 ++++-- pkg/js/util/model_utils.ts | 3 ++- pkg/js/validator/validate-dsl.ts | 6 ++++-- pkg/js/validator/validate-store.ts | 6 ++++-- 6 files changed, 22 insertions(+), 11 deletions(-) diff --git a/pkg/js/errors.ts b/pkg/js/errors.ts index 12222c2a..c4124154 100644 --- a/pkg/js/errors.ts +++ b/pkg/js/errors.ts @@ -168,7 +168,8 @@ export class UnsupportedDSLNestingError extends Error { public relationName: string, ) { super( - `the '${relationName}' relation definition under the '${typeName}' type is not supported by the OpenFGA DSL syntax yet`, + `the '${relationName}' relation definition under the '${typeName}' type ` + + "is not supported by the OpenFGA DSL syntax yet", ); } } diff --git a/pkg/js/transformer/jsontodsl.ts b/pkg/js/transformer/jsontodsl.ts index e9847d43..1d744977 100644 --- a/pkg/js/transformer/jsontodsl.ts +++ b/pkg/js/transformer/jsontodsl.ts @@ -214,7 +214,8 @@ function parseRelation( } throw new Error( - `the '${relationName}' relation definition under the '${typeName}' type is not supported by the OpenFGA DSL syntax yet`, + `the '${relationName}' relation definition under the '${typeName}' type ` + + "is not supported by the OpenFGA DSL syntax yet", ); } @@ -587,7 +588,8 @@ function parseRelationWithComments( } throw new Error( - `the '${relationName}' relation definition under the '${typeName}' type is not supported by the OpenFGA DSL syntax yet`, + `the '${relationName}' relation definition under the '${typeName}' type ` + + "is not supported by the OpenFGA DSL syntax yet", ); } @@ -610,7 +612,8 @@ const parseConditionWithComments = ( const paramsString = parseConditionParams(conditionDef.parameters || {}); const sourceString = constructSourceComment(conditionDef.metadata, "", includeSourceInformation); - return `${conditionCommentsStr}condition ${conditionName}(${paramsString}) {\n ${conditionDef.expression}\n}${sourceString}\n`; + return `${conditionCommentsStr}condition ${conditionName}(${paramsString}) ` + + `{\n ${conditionDef.expression}\n}${sourceString}\n`; }; // Parse conditions with comments diff --git a/pkg/js/util/exceptions.ts b/pkg/js/util/exceptions.ts index ec8b9492..edfa9863 100644 --- a/pkg/js/util/exceptions.ts +++ b/pkg/js/util/exceptions.ts @@ -153,7 +153,8 @@ const createInvalidRelationOnTuplesetError = ( const { errors, lines, lineIndex, symbol, file, module } = props; errors.push( constructValidationError({ - message: `the \`${offendingRelation}\` relation definition on type \`${typeDef}\` is not valid: \`${offendingRelation}\` does not exist on \`${parent}\`, which is of type \`${typeName}\`.`, + message: `the \`${offendingRelation}\` relation definition on type \`${typeDef}\` is not valid: ` + + `\`${offendingRelation}\` does not exist on \`${parent}\`, which is of type \`${typeName}\`.`, lines, lineIndex, metadata: { @@ -414,7 +415,8 @@ function createMultipleModuleInSingleFileError(props: BaseProps, file: string, m const { errors } = props; errors.push( constructValidationError({ - message: `file ${file} would contain multiple module definitions (${modules.join(", ")}) when transforming to DSL. Only one module can be defined per file.`, + message: `file ${file} would contain multiple module definitions (${modules.join(", ")}) ` + + "when transforming to DSL. Only one module can be defined per file.", metadata: { symbol: file, errorType: ValidationError.MultipleModulesInFile }, }), ); diff --git a/pkg/js/util/model_utils.ts b/pkg/js/util/model_utils.ts index 48552998..154d6934 100644 --- a/pkg/js/util/model_utils.ts +++ b/pkg/js/util/model_utils.ts @@ -23,7 +23,8 @@ export function getModuleForObjectTypeRelation(typeDef: TypeDefinition, relation } /** - * isRelationAssignable - returns true if the relation is assignable, as in the relation definition has a key "this" or any of its children have a key "this". + * isRelationAssignable - returns true if the relation is assignable, as in the relation + * definition has a key "this" or any of its children have a key "this". * @param relDef - A Userset object representing a relation definition. * @return boolean - A boolean representing whether the relation definition has a key "this". */ diff --git a/pkg/js/validator/validate-dsl.ts b/pkg/js/validator/validate-dsl.ts index 22b927d7..4521bcc9 100644 --- a/pkg/js/validator/validate-dsl.ts +++ b/pkg/js/validator/validate-dsl.ts @@ -89,7 +89,8 @@ const deepCopy = (object: T): T => { return JSON.parse(JSON.stringify(object)); }; -// Ensure a relation is assignable, the rest of the checks are to ensure that no model has this as well as additional properties defined +// Ensure a relation is assignable, the rest of the checks are to ensure that +// no model has this as well as additional properties defined const relationIsSingle = (currentRelation: Userset): boolean => { return ( !Object.prototype.hasOwnProperty.call(currentRelation, RelationDefOperator.Union) && @@ -533,7 +534,8 @@ function childDefDefined( // a. directly assignable // b. not a rewrite (not union, intersection or exclusion) // c. none of the directly assignable types contains a wildcard or a relation - // d. on every valid assignable type, ensure that the computed relation (e.g. a in a from b) is a relation on those types + // d. on every valid assignable type, ensure that the computed relation + // (e.g. a in a from b) is a relation on those types const [fromTypes, isValid] = allowableTypes(typeMap, type, childDef.from); if (isValid && fromTypes.length) { const childRelationNotValid = []; diff --git a/pkg/js/validator/validate-store.ts b/pkg/js/validator/validate-store.ts index 15bdc458..0bbcb644 100644 --- a/pkg/js/validator/validate-store.ts +++ b/pkg/js/validator/validate-store.ts @@ -116,7 +116,8 @@ const invalidRelationUser = (relation: string, relations: string[], instancePath const nonMatchingRelationType = (relation: string, user: string, values: string[], instancePath: string) => { if (values.length) { return invalidStore( - `\`${relation}\` is not a relation on \`${user}\`, and does not exist in model - valid relations are [${values}].`, + `\`${relation}\` is not a relation on \`${user}\`, and does not exist in model ` + + `- valid relations are [${values}].`, instancePath, ); } @@ -137,7 +138,8 @@ const unidentifiedTestParam = (testParam: string, instancePath: string) => { const undefinedTypeTuple = (user: string, instancePath: string) => { return { keyword: "valid_store_warning", - message: `${user} does not match any existing tuples; the check is still valid - but double check to ensure this is intended.`, + message: `${user} does not match any existing tuples; the check is still valid ` + + "- but double check to ensure this is intended.", instancePath, }; }; From c975c8ce6ad246827fba4db146a4e5612823befe Mon Sep 17 00:00:00 2001 From: Poovamraj T T Date: Mon, 19 Jan 2026 18:31:51 +0530 Subject: [PATCH 5/5] Fix prettier issues --- pkg/js/tests/comments.test.ts | 43 ++++++++---------------------- pkg/js/transformer/jsontodsl.ts | 6 +++-- pkg/js/util/exceptions.ts | 6 +++-- pkg/js/validator/validate-store.ts | 3 ++- 4 files changed, 21 insertions(+), 37 deletions(-) diff --git a/pkg/js/tests/comments.test.ts b/pkg/js/tests/comments.test.ts index 6b253606..15e9446c 100644 --- a/pkg/js/tests/comments.test.ts +++ b/pkg/js/tests/comments.test.ts @@ -1,12 +1,6 @@ import { describe, expect, it } from "@jest/globals"; -import { - transformDSLToJSONWithComments, - transformDSLToJSONObjectWithComments, -} from "../transformer/dsltojson"; -import { - transformJSONStringToDSLWithComments, - transformJSONToDSLWithComments, -} from "../transformer/jsontodsl"; +import { transformDSLToJSONWithComments, transformDSLToJSONObjectWithComments } from "../transformer/dsltojson"; +import { transformJSONStringToDSLWithComments, transformJSONToDSLWithComments } from "../transformer/jsontodsl"; describe("Comment Preservation", () => { describe("DSL to JSON with Comments", () => { @@ -21,10 +15,7 @@ type user`; const result = transformDSLToJSONObjectWithComments(dsl); expect(result.modelComments).toBeDefined(); - expect(result.modelComments?.preceding_lines).toEqual([ - "# OpenFGA Model", - "# Version 1.0", - ]); + expect(result.modelComments?.preceding_lines).toEqual(["# OpenFGA Model", "# Version 1.0"]); }); it("should preserve type comments", () => { @@ -37,9 +28,7 @@ type user`; const result = transformDSLToJSONObjectWithComments(dsl); expect(result.typeComments["user"]).toBeDefined(); - expect(result.typeComments["user"].comments?.preceding_lines).toEqual([ - "# User type comment", - ]); + expect(result.typeComments["user"].comments?.preceding_lines).toEqual(["# User type comment"]); }); it("should preserve relation comments", () => { @@ -54,13 +43,8 @@ type document const result = transformDSLToJSONObjectWithComments(dsl); expect(result.typeComments["document"]).toBeDefined(); - expect( - result.typeComments["document"].relation_comments?.["owner"] - ).toBeDefined(); - expect( - result.typeComments["document"].relation_comments?.["owner"] - .preceding_lines - ).toEqual(["# Owner comment"]); + expect(result.typeComments["document"].relation_comments?.["owner"]).toBeDefined(); + expect(result.typeComments["document"].relation_comments?.["owner"].preceding_lines).toEqual(["# Owner comment"]); }); it("should preserve condition comments", () => { @@ -77,9 +61,7 @@ condition ip_check(ip: string) { const result = transformDSLToJSONObjectWithComments(dsl); expect(result.conditionComments["ip_check"]).toBeDefined(); - expect(result.conditionComments["ip_check"].preceding_lines).toEqual([ - "# IP-based access control", - ]); + expect(result.conditionComments["ip_check"].preceding_lines).toEqual(["# IP-based access control"]); }); it("should embed comments in JSON output", () => { @@ -93,12 +75,8 @@ type user`; const jsonStr = transformDSLToJSONWithComments(dsl); const json = JSON.parse(jsonStr); - expect(json.metadata?.model_comments?.preceding_lines).toEqual([ - "# Model comment", - ]); - expect(json.type_definitions?.[0].metadata?.comments?.preceding_lines).toEqual([ - "# User type", - ]); + expect(json.metadata?.model_comments?.preceding_lines).toEqual(["# Model comment"]); + expect(json.type_definitions?.[0].metadata?.comments?.preceding_lines).toEqual(["# User type"]); }); }); @@ -174,7 +152,8 @@ type user`; conditions: { ip_check: { name: "ip_check", - expression: "ip == \"127.0.0.1\"", + // eslint-disable-next-line quotes + expression: 'ip == "127.0.0.1"', parameters: { ip: { type_name: "TYPE_NAME_STRING" as const }, }, diff --git a/pkg/js/transformer/jsontodsl.ts b/pkg/js/transformer/jsontodsl.ts index 1d744977..bd4bb040 100644 --- a/pkg/js/transformer/jsontodsl.ts +++ b/pkg/js/transformer/jsontodsl.ts @@ -612,8 +612,10 @@ const parseConditionWithComments = ( const paramsString = parseConditionParams(conditionDef.parameters || {}); const sourceString = constructSourceComment(conditionDef.metadata, "", includeSourceInformation); - return `${conditionCommentsStr}condition ${conditionName}(${paramsString}) ` + - `{\n ${conditionDef.expression}\n}${sourceString}\n`; + return ( + `${conditionCommentsStr}condition ${conditionName}(${paramsString}) ` + + `{\n ${conditionDef.expression}\n}${sourceString}\n` + ); }; // Parse conditions with comments diff --git a/pkg/js/util/exceptions.ts b/pkg/js/util/exceptions.ts index edfa9863..1544c3ee 100644 --- a/pkg/js/util/exceptions.ts +++ b/pkg/js/util/exceptions.ts @@ -153,7 +153,8 @@ const createInvalidRelationOnTuplesetError = ( const { errors, lines, lineIndex, symbol, file, module } = props; errors.push( constructValidationError({ - message: `the \`${offendingRelation}\` relation definition on type \`${typeDef}\` is not valid: ` + + message: + `the \`${offendingRelation}\` relation definition on type \`${typeDef}\` is not valid: ` + `\`${offendingRelation}\` does not exist on \`${parent}\`, which is of type \`${typeName}\`.`, lines, lineIndex, @@ -415,7 +416,8 @@ function createMultipleModuleInSingleFileError(props: BaseProps, file: string, m const { errors } = props; errors.push( constructValidationError({ - message: `file ${file} would contain multiple module definitions (${modules.join(", ")}) ` + + message: + `file ${file} would contain multiple module definitions (${modules.join(", ")}) ` + "when transforming to DSL. Only one module can be defined per file.", metadata: { symbol: file, errorType: ValidationError.MultipleModulesInFile }, }), diff --git a/pkg/js/validator/validate-store.ts b/pkg/js/validator/validate-store.ts index 0bbcb644..09282d46 100644 --- a/pkg/js/validator/validate-store.ts +++ b/pkg/js/validator/validate-store.ts @@ -138,7 +138,8 @@ const unidentifiedTestParam = (testParam: string, instancePath: string) => { const undefinedTypeTuple = (user: string, instancePath: string) => { return { keyword: "valid_store_warning", - message: `${user} does not match any existing tuples; the check is still valid ` + + message: + `${user} does not match any existing tuples; the check is still valid ` + "- but double check to ensure this is intended.", instancePath, };