From e6d71aec7f10ce49286f7a80909bb73070d9d507 Mon Sep 17 00:00:00 2001 From: Moises Vega Date: Mon, 10 Oct 2022 10:30:58 -0700 Subject: [PATCH] Add support for changed types in thriftbreak (#559) * Add check for changed types * Update error message * Update struct name --- internal/compare/compare.go | 17 +++++++++++++++++ internal/compare/compare_test.go | 26 ++++++++++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/internal/compare/compare.go b/internal/compare/compare.go index e40c8d66..8dd0ce06 100644 --- a/internal/compare/compare.go +++ b/internal/compare/compare.go @@ -99,6 +99,22 @@ func (p *Pass) requiredField(fromField, toField *compile.FieldSpec, to *compile. } } +func (p *Pass) changedTypes(fromField, toField *compile.FieldSpec, to *compile.StructSpec, file string) { + if fromField.Type == nil || toField.Type == nil { + return + } + + if fromField.Type.ThriftName() != toField.Type.ThriftName() { + p.Report(Diagnostic{ + FilePath: file, + Message: fmt.Sprintf( + "changing type of field %q in struct %q from %q to %q", + toField.ThriftName(), to.ThriftName(), fromField.Type.ThriftName(), + toField.Type.ThriftName()), + }) + } +} + // StructSpecs compares two structs defined in a Thrift file. func (p *Pass) structSpecs(from, to *compile.StructSpec, file string) { fields := make(map[int16]*compile.FieldSpec, len(from.Fields)) @@ -109,6 +125,7 @@ func (p *Pass) structSpecs(from, to *compile.StructSpec, file string) { for _, toField := range to.Fields { if fromField, ok := fields[toField.ID]; ok { p.requiredField(fromField, toField, to, file) + p.changedTypes(fromField, toField, to, file) } else if toField.Required { p.Report(Diagnostic{ FilePath: file, diff --git a/internal/compare/compare_test.go b/internal/compare/compare_test.go index be13eb41..4292f492 100644 --- a/internal/compare/compare_test.go +++ b/internal/compare/compare_test.go @@ -101,6 +101,32 @@ func TestErrorRequiredCase(t *testing.T) { wantError: `foo.thrift:changing an optional field "fieldA" in "structA" to required` + "\n" + `foo.thrift:changing an optional field "fieldB" in "structA" to required`, }, + { + desc: "found a type changed", + fromStruct: &compile.StructSpec{ + Name: "structA", + Fields: compile.FieldGroup{ + &compile.FieldSpec{ + Name: "fieldA", + Type: &compile.BinarySpec{}, + }, + }, + }, + toStruct: &compile.StructSpec{ + Name: "structA", + Fields: compile.FieldGroup{ + &compile.FieldSpec{ + Name: "fieldA", + Type: &compile.BoolSpec{}, + }, + &compile.FieldSpec{ + Name: "fieldB", + Type: &compile.BinarySpec{}, + }, + }, + }, + wantError: `foo.thrift:changing type of field "fieldA" in struct "structA" from "binary" to "bool"`, + }, } for _, tt := range tests {