From 03ccaff375cfbc84dd4c9fefcd1af3f8638f23c7 Mon Sep 17 00:00:00 2001 From: GogaCoder Date: Mon, 30 Dec 2024 23:44:24 +0700 Subject: [PATCH] feat: foreignKeys check +fix: "unnamed" field bug --- common/modelsParser.go | 14 ++++-- common/nullSafetyCheck.go | 4 +- common/unnamedChecks.go | 7 --- foreignKeyCheck/foreignKeyCheck.go | 48 +++++++++++++++++++ nullSafetyCheck/nullSafetyCheck.go | 16 +++++-- referencesCheck/referencesCheck.go | 4 +- tests/foreign_key_check_test.go | 12 +++++ ...fety_test.go => null_safety_check_test.go} | 0 .../src/foreign_key_check/negative.go | 12 +++++ .../src/foreign_key_check/positive.go | 27 +++++++++++ 10 files changed, 123 insertions(+), 21 deletions(-) create mode 100644 tests/foreign_key_check_test.go rename tests/{null_safety_test.go => null_safety_check_test.go} (100%) create mode 100644 tests/testdata/src/foreign_key_check/negative.go create mode 100644 tests/testdata/src/foreign_key_check/positive.go diff --git a/common/modelsParser.go b/common/modelsParser.go index 8f8a5d0..9a0dd70 100644 --- a/common/modelsParser.go +++ b/common/modelsParser.go @@ -32,12 +32,18 @@ func ParseModels(pass *analysis.Pass, models *map[string]Model) { for _, field := range structure.Fields.List { var structField Field - if err := CheckUnnamedField(typeSpec.Name.Name, *field); err != nil { - pass.Reportf(field.Pos(), err.Error()) - return false + + if len(field.Names) == 0 { + fieldType := ResolveBaseType(field.Type) + if fieldType == nil { + pass.Reportf(field.Pos(), "Failed to resolve model \"%s\" field type: %s", model.Name, field.Type) + } else { + structField.Name = *fieldType + } + } else { + structField.Name = field.Names[0].Name } - structField.Name = field.Names[0].Name structField.Pos = field.Pos() structField.Comment = field.Comment.Text() structField.Type = field.Type diff --git a/common/nullSafetyCheck.go b/common/nullSafetyCheck.go index 5838fe7..5a4bfd4 100644 --- a/common/nullSafetyCheck.go +++ b/common/nullSafetyCheck.go @@ -43,7 +43,7 @@ func isGormValueNullable(tags *structtag.Tags) (*bool, error) { } } -func CheckFieldNullConsistency(field ast.Field, structName string, structTags string) error { +func CheckFieldNullConsistency(field ast.Field, fieldName string, structName string, structTags string) error { tags, err := structtag.Parse(structTags) if err != nil { return errors.New(fmt.Sprintf("Invalid structure tag: %s", err)) @@ -64,7 +64,7 @@ func CheckFieldNullConsistency(field ast.Field, structName string, structTags st } if isFieldNullable != *isColumnNullable { - 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 errors.New(fmt.Sprintf("Null safety error in \"%s\" model, field \"%s\": column nullable policy doesn't match to tag nullable policy", structName, fieldName)) } return nil } diff --git a/common/unnamedChecks.go b/common/unnamedChecks.go index 953d266..8936857 100644 --- a/common/unnamedChecks.go +++ b/common/unnamedChecks.go @@ -12,10 +12,3 @@ func CheckUnnamedModel(typeSpec ast.TypeSpec) error { } return nil } - -func CheckUnnamedField(structName string, field ast.Field) error { - if len(field.Names) == 0 { - return errors.New(fmt.Sprintf("Struct \"%s\" has unnamed field", structName)) - } - return nil -} diff --git a/foreignKeyCheck/foreignKeyCheck.go b/foreignKeyCheck/foreignKeyCheck.go index f1a93f3..02ba9e9 100644 --- a/foreignKeyCheck/foreignKeyCheck.go +++ b/foreignKeyCheck/foreignKeyCheck.go @@ -1 +1,49 @@ package foreignKeyCheck + +import ( + "golang.org/x/tools/go/analysis" + "gormlint/common" + "strings" +) + +// ForeignKeyCheck todo: add URL for foreign key analyzer rules +var ForeignKeyCheck = &analysis.Analyzer{ + Name: "GormForeignKeyCheck", + Doc: "Check foreign key in gorm model struct tag", + Run: run, +} + +var models map[string]common.Model + +func run(pass *analysis.Pass) (any, error) { + models = make(map[string]common.Model) + common.ParseModels(pass, &models) + + for _, model := range models { + for _, field := range model.Fields { + for _, param := range field.Params { + pair := strings.Split(param, ":") + paramName := pair[0] + paramValue := pair[1] + if paramName == "foreignKey" { + foreignKey, fieldExist := model.Fields[paramValue] + + if !fieldExist { + pass.Reportf(field.Pos, "Foreign key \"%s\" mentioned in tag at field \"%s\" doesn't exist in model \"%s\"", paramValue, field.Name, model.Name) + } else { + foreignKeyType := common.ResolveBaseType(foreignKey.Type) + if foreignKeyType == nil { + pass.Reportf(foreignKey.Pos, "Failed to resolve type of foreign key field \"%s\": %s", field.Name, foreignKey.Type) + } else { + // TODO: handle all int types + if *foreignKeyType != "uint" && *foreignKeyType != "int" { + pass.Reportf(foreignKey.Pos, "Foreign key should have type like int, not \"%s\"", foreignKey.Type) + } + } + } + } + } + } + } + return nil, nil +} diff --git a/nullSafetyCheck/nullSafetyCheck.go b/nullSafetyCheck/nullSafetyCheck.go index 51991fb..7f43ff5 100644 --- a/nullSafetyCheck/nullSafetyCheck.go +++ b/nullSafetyCheck/nullSafetyCheck.go @@ -32,20 +32,26 @@ func run(pass *analysis.Pass) (any, error) { } for _, field := range structure.Fields.List { - if err := common.CheckUnnamedField(typeSpec.Name.Name, *field); err != nil { - pass.Reportf(field.Pos(), err.Error()) - return false + var structFieldName string + if len(field.Names) == 0 { + fieldType := common.ResolveBaseType(field.Type) + if fieldType == nil { + pass.Reportf(field.Pos(), "Failed to resolve model \"%s\" field type: %s", typeSpec.Name.Name, field.Type) + } else { + structFieldName = *fieldType + } + } else { + structFieldName = field.Names[0].Name } if field.Tag != nil { tagWithoutQuotes := field.Tag.Value[1 : len(field.Tag.Value)-1] tagWithoutSemicolons := strings.ReplaceAll(tagWithoutQuotes, ";", ",") - err := common.CheckFieldNullConsistency(*field, typeSpec.Name.Name, tagWithoutSemicolons) + err := common.CheckFieldNullConsistency(*field, structFieldName, 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/referencesCheck/referencesCheck.go b/referencesCheck/referencesCheck.go index f88de02..0724abf 100644 --- a/referencesCheck/referencesCheck.go +++ b/referencesCheck/referencesCheck.go @@ -13,10 +13,8 @@ var ReferenceAnalyzer = &analysis.Analyzer{ Run: run, } -var models map[string]common.Model - func run(pass *analysis.Pass) (any, error) { - models = make(map[string]common.Model) + models := make(map[string]common.Model) common.ParseModels(pass, &models) for _, model := range models { diff --git a/tests/foreign_key_check_test.go b/tests/foreign_key_check_test.go new file mode 100644 index 0000000..0076dfe --- /dev/null +++ b/tests/foreign_key_check_test.go @@ -0,0 +1,12 @@ +package tests + +import ( + "golang.org/x/tools/go/analysis/analysistest" + "gormlint/foreignKeyCheck" + "testing" +) + +func TestForeignKeyCheck(t *testing.T) { + t.Parallel() + analysistest.Run(t, analysistest.TestData(), foreignKeyCheck.ForeignKeyCheck, "foreign_key_check") +} diff --git a/tests/null_safety_test.go b/tests/null_safety_check_test.go similarity index 100% rename from tests/null_safety_test.go rename to tests/null_safety_check_test.go diff --git a/tests/testdata/src/foreign_key_check/negative.go b/tests/testdata/src/foreign_key_check/negative.go new file mode 100644 index 0000000..302afec --- /dev/null +++ b/tests/testdata/src/foreign_key_check/negative.go @@ -0,0 +1,12 @@ +package foreign_key_check + +type User struct { + Name string + CompanyRefer uint + Company Company `gorm:"foreignKey:CompanyRefer"` +} + +type Company struct { + ID int + Name string +} diff --git a/tests/testdata/src/foreign_key_check/positive.go b/tests/testdata/src/foreign_key_check/positive.go new file mode 100644 index 0000000..2b84f4c --- /dev/null +++ b/tests/testdata/src/foreign_key_check/positive.go @@ -0,0 +1,27 @@ +package foreign_key_check + +type PrepTask struct { + Id uint `gorm:"primaryKey"` + Description string + TaskId uint + WorkAreaId uint + WorkArea `gorm:"foreignKey:WorkAreaIds;constraint:OnDelete:CASCADE;"` // want "Foreign key \"WorkAreaIds\" mentioned in tag at field \"WorkArea\" doesn't exist in model \"PrepTask\"" + Deadline int64 +} + +type WorkArea struct { + Id uint `gorm:"primaryKey"` + Name string + Description string + Performance uint + PrepTasks []PrepTask `gorm:"constraint:OnDelete:CASCADE;"` +} + +type Shift struct { + Id uint `gorm:"primaryKey"` + Description string + ProductAmount uint + ShiftDate int64 + WorkAreaId string // want "Foreign key should have type like int, not \"string\"" + WorkArea WorkArea `gorm:"foreignKey:WorkAreaId;"` +}