diff --git a/common/nullSafetyCheck.go b/common/nullSafetyCheck.go index ae9516f..1306589 100644 --- a/common/nullSafetyCheck.go +++ b/common/nullSafetyCheck.go @@ -2,9 +2,9 @@ package common import ( "errors" + "fmt" "github.com/fatih/structtag" "go/ast" - "golang.org/x/tools/go/analysis" ) func isPointerType(typeExpr ast.Expr) bool { @@ -30,7 +30,7 @@ func isGormValueNullable(tags *structtag.Tags) (*bool, error) { nullTagExist := gormTag.HasOption("null") notNullTagExist := gormTag.HasOption("not null") - if nullTagExist == notNullTagExist && nullTagExist == true { + if nullTagExist == notNullTagExist && nullTagExist { return nil, errors.New(`tags "null" and "not null" are specified at one field`) } @@ -43,26 +43,28 @@ func isGormValueNullable(tags *structtag.Tags) (*bool, error) { } } -func CheckFieldNullConsistency(pass analysis.Pass, field ast.Field, structName string, structTags string) { +func CheckFieldNullConsistency(field ast.Field, structName string, structTags string) error { tags, err := structtag.Parse(structTags) if err != nil { - pass.Reportf(field.Pos(), "Invalid structure tag: %s", err) + return errors.New(fmt.Sprintf("Invalid structure tag: %s", err)) + } if tags == nil { - return + return nil } isFieldNullable := isPointerType(field.Type) isColumnNullable, err := isGormValueNullable(tags) if err != nil { - pass.Reportf(field.Pos(), "Null safety error: %s", err) + return errors.New(fmt.Sprintf("Null safety error: %s", err)) } if isColumnNullable == nil { - return + return nil } if isFieldNullable != *isColumnNullable { - pass.Reportf(field.Pos(), "Null safety error in \"%s\" model, field \"%s\": column nullable policy doesn't match to tag nullable policy", structName, field.Names[0].Name) + return errors.New(fmt.Sprintf("Null safety error in \"%s\" model, field \"%s\": column nullable policy doesn't match to tag nullable policy", structName, field.Names[0].Name)) } + return nil } diff --git a/common/unnamedChecks.go b/common/unnamedChecks.go index 7c63ee7..953d266 100644 --- a/common/unnamedChecks.go +++ b/common/unnamedChecks.go @@ -1,18 +1,21 @@ package common import ( + "errors" + "fmt" "go/ast" - "golang.org/x/tools/go/analysis" ) -func CheckUnnamedModel(pass analysis.Pass, typeSpec ast.TypeSpec) { +func CheckUnnamedModel(typeSpec ast.TypeSpec) error { if typeSpec.Name == nil { - pass.Reportf(typeSpec.Pos(), "Unnamed model\n") + return errors.New(fmt.Sprintf("Unnamed model\n")) } + return nil } -func CheckUnnamedField(pass analysis.Pass, structName string, field ast.Field) { +func CheckUnnamedField(structName string, field ast.Field) error { if len(field.Names) == 0 { - pass.Reportf(field.Pos(), "Struct \"%s\" has unnamed field", structName) + return errors.New(fmt.Sprintf("Struct \"%s\" has unnamed field", structName)) } + return nil } diff --git a/go.mod b/go.mod index 32917e9..85b31d0 100644 --- a/go.mod +++ b/go.mod @@ -2,14 +2,12 @@ module gormlint go 1.23.2 -require golang.org/x/tools v0.28.0 +require ( + github.com/fatih/structtag v1.2.0 + golang.org/x/tools v0.28.0 +) require ( - github.com/fatih/color v1.18.0 // indirect - github.com/fatih/structtag v1.2.0 // indirect - github.com/mattn/go-colorable v0.1.13 // indirect - github.com/mattn/go-isatty v0.0.20 // indirect golang.org/x/mod v0.22.0 // indirect golang.org/x/sync v0.10.0 // indirect - golang.org/x/sys v0.28.0 // indirect ) diff --git a/go.sum b/go.sum index ae0c01c..14b23da 100644 --- a/go.sum +++ b/go.sum @@ -1,21 +1,10 @@ -github.com/fatih/color v1.18.0 h1:S8gINlzdQ840/4pfAwic/ZE0djQEH3wM94VfqLTZcOM= -github.com/fatih/color v1.18.0/go.mod h1:4FelSpRwEGDpQ12mAdzqdOukCy4u8WUtOY6lkT/6HfU= github.com/fatih/structtag v1.2.0 h1:/OdNE99OxoI/PqaW/SuSK9uxxT3f/tcSZgon/ssNSx4= github.com/fatih/structtag v1.2.0/go.mod h1:mBJUNpUnHmRKrKlQQlmCrh5PuhftFbNv8Ys4/aAZl94= github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= -github.com/mattn/go-colorable v0.1.13 h1:fFA4WZxdEF4tXPZVKMLwD8oUnCTTo08duU7wxecdEvA= -github.com/mattn/go-colorable v0.1.13/go.mod h1:7S9/ev0klgBDR4GtXTXX8a3vIGJpMovkB8vQcUbaXHg= -github.com/mattn/go-isatty v0.0.16/go.mod h1:kYGgaQfpe5nmfYZH+SKPsOc2e4SrIfOl2e/yFXSvRLM= -github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY= -github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= golang.org/x/mod v0.22.0 h1:D4nJWe9zXqHOmWqj4VMOJhvzj7bEZg4wEYa759z1pH4= golang.org/x/mod v0.22.0/go.mod h1:6SkKJ3Xj0I0BrPOZoBy3bdMptDDU9oJrpohJ3eWZ1fY= golang.org/x/sync v0.10.0 h1:3NQrjDixjgGwUOCaF8w2+VYHv0Ve/vGYSbdkTa98gmQ= golang.org/x/sync v0.10.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= -golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.28.0 h1:Fksou7UEQUWlKvIdsqzJmUmCX3cZuD2+P3XyyzwMhlA= -golang.org/x/sys v0.28.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/tools v0.28.0 h1:WuB6qZ4RPCQo5aP3WdKZS7i595EdWqWR8vqJTlwTVK8= golang.org/x/tools v0.28.0/go.mod h1:dcIOrVd3mfQKTgrDVQHqCPMWy6lnhfhtX3hLXYVLfRw= diff --git a/nullSafetyCheck/nullSafetyCheck.go b/nullSafetyCheck/nullSafetyCheck.go index a550ec1..163767b 100644 --- a/nullSafetyCheck/nullSafetyCheck.go +++ b/nullSafetyCheck/nullSafetyCheck.go @@ -25,17 +25,26 @@ func run(pass *analysis.Pass) (any, error) { return true } - common.CheckUnnamedModel(*pass, *typeSpec) + if err := common.CheckUnnamedModel(*typeSpec); err != nil { + pass.Reportf(structure.Pos(), err.Error()) + return false + } for _, field := range structure.Fields.List { - common.CheckUnnamedField(*pass, typeSpec.Name.Name, *field) + if err := common.CheckUnnamedField(typeSpec.Name.Name, *field); err != nil { + pass.Reportf(field.Pos(), err.Error()) + return false + } if field.Tag != nil { tagWithoutQuotes := field.Tag.Value[1 : len(field.Tag.Value)-1] tagWithoutSemicolons := strings.ReplaceAll(tagWithoutQuotes, ";", ",") - common.CheckFieldNullConsistency(*pass, *field, typeSpec.Name.Name, tagWithoutSemicolons) - } else { - // TODO: check necessary tags for some fields + err := common.CheckFieldNullConsistency(*field, typeSpec.Name.Name, tagWithoutSemicolons) + if err != nil { + pass.Reportf(field.Pos(), err.Error()) + return false + } } + // TODO: check necessary tags for some fields } return false }) diff --git a/models/testdata.go b/tests/models/testdata.go similarity index 72% rename from models/testdata.go rename to tests/models/testdata.go index da4b465..06f95e2 100644 --- a/models/testdata.go +++ b/tests/models/testdata.go @@ -7,7 +7,7 @@ type Order struct { ProductType ProductType ProductAmount uint Description string - CustomerId uint `gorm:"null;foreignKey:CustomerId;"` + CustomerId uint `gorm:"null;foreignKey:CustomerId;"` // want "Null safety error in \"Order\" model, field \"CustomerId\": column nullable policy doesn't match to tag nullable policy" Customer Customer CreatedAt int64 `gorm:"autoCreateTime"` DeadlineDate int64 diff --git a/tests/null_safety_test.go b/tests/null_safety_test.go new file mode 100644 index 0000000..4e6ac27 --- /dev/null +++ b/tests/null_safety_test.go @@ -0,0 +1,12 @@ +package tests + +import ( + "golang.org/x/tools/go/analysis/analysistest" + "gormlint/nullSafetyCheck" + "testing" +) + +func TestNullSafety(t *testing.T) { + t.Parallel() + analysistest.Run(t, analysistest.TestData(), nullSafetyCheck.NullSafetyAnalyzer, "null_safety") +} diff --git a/tests/testdata/src/null_safety/negative.go b/tests/testdata/src/null_safety/negative.go new file mode 100644 index 0000000..330865a --- /dev/null +++ b/tests/testdata/src/null_safety/negative.go @@ -0,0 +1,29 @@ +package null_safety + +type Order1 struct { + Id uint `gorm:"primaryKey"` + Description string + // not nullable - not nullable + CustomerId uint `gorm:"not null;foreignKey:CustomerId;"` +} + +type Order2 struct { + Id uint `gorm:"primaryKey"` + Description string + // nullable - nullable + CustomerId *uint `gorm:"null;foreignKey:CustomerId;"` +} + +type Order3 struct { + Id uint `gorm:"primaryKey"` + // nullable - unspecified + Status *string + Description string +} + +type Order4 struct { + Id uint `gorm:"primaryKey"` + // not nullable - unspecified + Status *string + Description string +} diff --git a/tests/testdata/src/null_safety/positive.go b/tests/testdata/src/null_safety/positive.go new file mode 100644 index 0000000..7d5f165 --- /dev/null +++ b/tests/testdata/src/null_safety/positive.go @@ -0,0 +1,29 @@ +package null_safety + +type Order5 struct { + Id uint `gorm:"primaryKey"` + Description string + // not nullable - nullable + CustomerId uint `gorm:"null;foreignKey:CustomerId;"` // want "Null safety error in \"Order5\" model, field \"CustomerId\": column nullable policy doesn't match to tag nullable policy" +} + +type Order6 struct { + Id uint `gorm:"primaryKey"` + Description string + // nullable - not nullable + CustomerId *uint `gorm:"not null;foreignKey:CustomerId;"` // want "Null safety error in \"Order6\" model, field \"CustomerId\": column nullable policy doesn't match to tag nullable policy" +} + +type Order7 struct { + Id uint `gorm:"primaryKey"` + Description string + // not nullable - not nullable, nullable + CustomerId uint `gorm:"not null;foreignKey:CustomerId;null;"` // want "Null safety error: tags \"null\" and \"not null\" are specified at one field" +} + +type Order8 struct { + Id uint `gorm:"primaryKey"` + Description string + // nullable - not nullable, nullable + CustomerId *uint `gorm:"not null;foreignKey:CustomerId;null;"` // want "Null safety error: tags \"null\" and \"not null\" are specified at one field" +}