Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ linters:
enable:
- errcheck
- errname
- gochecksumtype
- govet
- ineffassign
- revive
Expand Down
8 changes: 7 additions & 1 deletion internal/schema/ast/ast.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ import (

// The human readable format is not 1-1 convertible with JSON. The JSON format
// is lossy. It loses formatting, such as comments, ordering of fields, etc...

//
//sumtype:decl
type Node interface {
isNode()
// Pos returns first token of the node
Expand Down Expand Up @@ -97,6 +98,7 @@ func (s *Schema) End() token.Position {
return token.Position{}
}

//sumtype:decl
type Declaration interface {
Node
isDecl()
Expand Down Expand Up @@ -165,6 +167,8 @@ func (t *CommonTypeDecl) End() token.Position {
// 1. A record type
// 2. A set type (Set<String>)
// 3. A path (Namespace::EntityType or String)
//
//sumtype:decl
type Type interface {
Node
isType()
Expand Down Expand Up @@ -377,6 +381,8 @@ func (r *Ref) End() token.Position {
}

// Name is an IDENT or STR
//
//sumtype:decl
type Name interface {
Node
isName()
Expand Down
49 changes: 0 additions & 49 deletions internal/schema/ast/ast_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,32 +116,6 @@ func Test_formatter_printInd_panic(t *testing.T) {
p.printInd("test")
}

type unknownNode struct {
Node // Embed Node interface to satisfy type checker
}

func Test_formatter_print_panic(t *testing.T) {
p := &formatter{
w: &bytes.Buffer{},
}

defer func() {
r := recover()
if r == nil {
t.Fatal("expected panic, got none")
}
msg, ok := r.(string)
if !ok {
t.Fatalf("expected string panic, got %T", r)
}
if !strings.Contains(msg, "unhandled node type") {
t.Errorf("expected panic message about unhandled type, got %q", msg)
}
}()

p.print(unknownNode{})
}

func Test_printBracketList_panic(t *testing.T) {
p := &formatter{
w: &bytes.Buffer{},
Expand All @@ -164,26 +138,3 @@ func Test_printBracketList_panic(t *testing.T) {
var emptyList []Node
printBracketList(p, emptyList, false)
}

type unknownType struct {
Type // Embed Type interface to satisfy it
}

func TestConvertType_Panic(t *testing.T) {
defer func() {
r := recover()
if r == nil {
t.Fatal("expected panic, got none")
}
msg, ok := r.(string)
if !ok {
t.Fatalf("expected string panic, got %T", r)
}
expected := "unknownType is not an AST type"
if !strings.Contains(msg, expected) {
t.Errorf("expected panic message to contain %q, got %q", expected, msg)
}
}()

convertType(unknownType{})
}
4 changes: 4 additions & 0 deletions internal/schema/ast/convert_human.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,10 @@ func convertNamespace(n *Namespace) *JSONNamespace {
commonType.Annotations[a.Key.String()] = a.Value.String()
}
jsNamespace.CommonTypes[astDecl.Name.String()] = commonType
case *CommentBlock:
// comment blocks intentionally dropped.
case *Namespace:
// namespaces intentionally not handled.
}
}
return jsNamespace
Expand Down
2 changes: 2 additions & 0 deletions internal/schema/ast/convert_json.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,8 @@ func convertJSONAppliesTo(appliesTo *JSONAppliesTo) *AppliesTo {
at.ContextRecord = t
case *Path:
at.ContextPath = t
case *SetType:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain why this needed to be added, etc? I see there is another case of panic in this file, which I'm not really excited about either ... Is there a way we can rephrase both to avoid those panics? I'd rather we use error handling in bad cases.

Either way, the PR can't be merged without 100% coverage, so this does need to be covered with a test to show when it happens.

panic("SetType not yet handled in convertJSONAppliesTo")
}
}

Expand Down
Loading