diff --git a/cmd/gormlint/main.go b/cmd/gormlint/main.go index e2274b6..e50ccbe 100644 --- a/cmd/gormlint/main.go +++ b/cmd/gormlint/main.go @@ -2,17 +2,13 @@ package main import ( "golang.org/x/tools/go/analysis/multichecker" - "gormlint/foreignKeyCheck" "gormlint/nullSafetyCheck" - "gormlint/referencesCheck" "gormlint/relationsCheck" ) func main() { multichecker.Main( nullSafetyCheck.NullSafetyAnalyzer, - referencesCheck.ReferenceAnalyzer, - foreignKeyCheck.ForeignKeyCheck, relationsCheck.RelationsAnalyzer, ) } diff --git a/common/finders.go b/common/finders.go index f9fd4ac..cfd2bc6 100644 --- a/common/finders.go +++ b/common/finders.go @@ -1,6 +1,7 @@ package common import ( + "github.com/kuzgoga/fogg" "go/ast" "strings" ) @@ -32,32 +33,10 @@ func GetRelatedModel(modelName string, models map[string]Model) *Model { } } -func FindParamValue(paramName string, params []string) *string { - for _, rawParam := range params { - pair := strings.Split(rawParam, ":") - if len(pair) < 2 { - return nil - } - if strings.ToLower(pair[0]) == strings.ToLower(paramName) { - return &pair[1] - } - } - return nil -} - -func FindModelParam(paramName string, model Model) *Param { +func FindModelParam(paramName string, model Model) *fogg.TagParam { for _, field := range model.Fields { - for _, param := range field.Params { - pair := strings.Split(param, ":") - if len(pair) < 2 { - return nil - } - if strings.ToLower(pair[0]) == strings.ToLower(paramName) { - return &Param{ - Name: pair[0], - Value: pair[1], - } - } + if field.Tags.HasParam(paramName) { + return field.Tags.GetParam(paramName) } } return nil @@ -65,15 +44,13 @@ func FindModelParam(paramName string, model Model) *Param { func FindReferencesInM2M(m2mReference Field, relatedModel Model) *Field { /* Find `references` field in m2m relation */ - referencesTagValue := FindParamValue("references", m2mReference.Params) - if referencesTagValue != nil { - return GetModelField(&relatedModel, *referencesTagValue) + referencesTag := m2mReference.Tags.GetParam("references") + if referencesTag != nil { + return GetModelField(&relatedModel, referencesTag.Value) } else { for _, field := range relatedModel.Fields { - for _, opt := range field.Options { - if opt == "primaryKey" { - return &field - } + if field.Tags.HasOption("primaryKey") { + return &field } } for _, field := range relatedModel.Fields { @@ -87,7 +64,7 @@ func FindReferencesInM2M(m2mReference Field, relatedModel Model) *Field { func FindBackReferenceInM2M(relationName string, relatedModel Model) *Field { for _, field := range relatedModel.Fields { - m2mRelation := field.GetParam("many2many") + m2mRelation := field.Tags.GetParam("many2many") if m2mRelation != nil { if m2mRelation.Value == relationName { return &field diff --git a/common/model.go b/common/model.go index 8a67346..58cc8ac 100644 --- a/common/model.go +++ b/common/model.go @@ -1,19 +1,18 @@ package common import ( + "github.com/kuzgoga/fogg" "go/ast" "go/token" - "strings" ) type Field struct { Name string Type ast.Expr - Tags *string - Options []string // contains options like "autoCreateTime" or "null" - Params []string // contains params like "foreignKey:CustomerId" or "constrain:OnDelete:Cascade" + Tag *string Pos token.Pos Comment string + Tags fogg.Tag } type Model struct { @@ -27,45 +26,3 @@ type Param struct { Name string Value string } - -func (model *Model) GetParam(name string) *Param { - for _, field := range model.Fields { - for _, param := range field.Params { - pair := strings.SplitN(param, ":", 2) - if len(pair) != 2 { - return nil - } - if strings.ToLower(pair[0]) == strings.ToLower(name) { - return &Param{ - Name: pair[0], - Value: pair[1], - } - } - } - } - return nil -} - -func (model *Model) HasParam(name string) bool { - return model.GetParam(name) != nil -} - -func (field *Field) HasParam(name string) bool { - return field.GetParam(name) != nil -} - -func (field *Field) GetParam(name string) *Param { - for _, param := range field.Params { - pair := strings.SplitN(param, ":", 2) - if len(pair) != 2 { - return nil - } - if strings.ToLower(pair[0]) == strings.ToLower(name) { - return &Param{ - Name: pair[0], - Value: pair[1], - } - } - } - return nil -} diff --git a/common/model_methods.go b/common/model_methods.go new file mode 100644 index 0000000..1d60307 --- /dev/null +++ b/common/model_methods.go @@ -0,0 +1,10 @@ +package common + +func (model *Model) HasPrimaryKey() bool { + for _, field := range model.Fields { + if field.Tags.HasParam("primaryKey") || field.Tags.HasOption("primaryKey") { + return true + } + } + return false +} diff --git a/common/modelsParser.go b/common/modelsParser.go index 9a0dd70..88e2fcc 100644 --- a/common/modelsParser.go +++ b/common/modelsParser.go @@ -1,10 +1,9 @@ package common import ( - "github.com/fatih/structtag" + "github.com/kuzgoga/fogg" "go/ast" "golang.org/x/tools/go/analysis" - "strings" ) func ParseModels(pass *analysis.Pass, models *map[string]Model) { @@ -49,29 +48,17 @@ func ParseModels(pass *analysis.Pass, models *map[string]Model) { structField.Type = field.Type if field.Tag != nil { - structField.Tags = &field.Tag.Value - - tags, err := structtag.Parse(NormalizeStructTags(field.Tag.Value)) + structField.Tag = &field.Tag.Value + var structTag string + structTag = field.Tag.Value[1 : len(field.Tag.Value)-1] + tags, err := fogg.Parse(structTag) if err != nil { - pass.Reportf(field.Pos(), "Invalid structure tag: %s\n", err) + pass.Reportf(field.Pos(), "Invalid struct tag: %s\n", err) return false } - if tags != nil { - gormTag, parseErr := tags.Get("gorm") - if gormTag != nil && parseErr == nil { - gormTag.Options = append([]string{gormTag.Name}, gormTag.Options...) - for _, opt := range gormTag.Options { - if strings.Contains(opt, ":") { - structField.Params = append(structField.Params, opt) - } else { - structField.Options = append(structField.Options, opt) - } - } - } - if parseErr != nil { - pass.Reportf(field.Pos(), "Invalid structure tag: %s\n", parseErr) - return false - } + gormTag := tags.GetTag("gorm") + if gormTag != nil { + structField.Tags = *gormTag } } model.Fields[structField.Name] = structField diff --git a/common/normalizeStructTag.go b/common/normalizeStructTag.go deleted file mode 100644 index b825b49..0000000 --- a/common/normalizeStructTag.go +++ /dev/null @@ -1,10 +0,0 @@ -package common - -import "strings" - -func NormalizeStructTags(tags string) string { - // todo: process case with check with ';' literal - tagWithoutQuotes := tags[1 : len(tags)-1] - tagWithoutSemicolons := strings.ReplaceAll(tagWithoutQuotes, ";", ",") - return tagWithoutSemicolons -} diff --git a/foreignKeyCheck/foreignKeyCheck.go b/foreignKeyCheck/foreignKeyCheck.go deleted file mode 100644 index 02ba9e9..0000000 --- a/foreignKeyCheck/foreignKeyCheck.go +++ /dev/null @@ -1,49 +0,0 @@ -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/go.mod b/go.mod index 85b31d0..08d6edd 100644 --- a/go.mod +++ b/go.mod @@ -8,6 +8,7 @@ require ( ) require ( + github.com/kuzgoga/fogg v0.1.2 // indirect golang.org/x/mod v0.22.0 // indirect golang.org/x/sync v0.10.0 // indirect ) diff --git a/go.sum b/go.sum index 14b23da..c0e18c5 100644 --- a/go.sum +++ b/go.sum @@ -2,6 +2,8 @@ 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/kuzgoga/fogg v0.1.2 h1:zXOZEaSFNiC3xU/r10UYAQ9CpR2A2kx9F1bFC8O62AQ= +github.com/kuzgoga/fogg v0.1.2/go.mod h1:x0cKa6kIaweLKtzMWXw0xZZnl2PrLDpMmi+yL3xEIEg= 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= diff --git a/referencesCheck/referencesCheck.go b/referencesCheck/referencesCheck.go deleted file mode 100644 index 0724abf..0000000 --- a/referencesCheck/referencesCheck.go +++ /dev/null @@ -1,53 +0,0 @@ -package referencesCheck - -import ( - "golang.org/x/tools/go/analysis" - "gormlint/common" - "strings" -) - -// ReferenceAnalyzer todo: add URL for rule -var ReferenceAnalyzer = &analysis.Analyzer{ - Name: "GormReferencesCheck", - Doc: "report about invalid references in models", - Run: run, -} - -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 == "reference" { - pass.Reportf(field.Pos, "Typo in tag: \"reference\" instead of verb \"references\"") - } - if paramName == "references" { - fieldType := common.ResolveBaseType(field.Type) - - if fieldType == nil { - pass.Reportf(field.Pos, "Failed to process references check. Cannot resolve type \"%s\" in field \"%s\"", field.Type, field.Name) - return nil, nil - } - - relatedModel, modelExists := models[*fieldType] - - if modelExists { - _, fieldExists := relatedModel.Fields[paramValue] - if !fieldExists { - pass.Reportf(field.Pos, "Related field \"%s\" doesn't exist on model \"%s\"", paramValue, relatedModel.Name) - } - } else { - pass.Reportf(field.Pos, "Related model \"%s\" doesn't exist", *fieldType) - } - } - } - } - } - return nil, nil -} diff --git a/relationsCheck/relationsCheck.go b/relationsCheck/relationsCheck.go index cad6afa..0d8737a 100644 --- a/relationsCheck/relationsCheck.go +++ b/relationsCheck/relationsCheck.go @@ -9,8 +9,8 @@ import ( // RelationsAnalyzer todo: add URL for rule var RelationsAnalyzer = &analysis.Analyzer{ - Name: "GormReferencesCheck", - Doc: "report about invalid references in models", + Name: "GormRelationsCheck", + Doc: "report about invalid relations in models", Run: run, } @@ -50,7 +50,7 @@ func CheckMany2Many(pass *analysis.Pass, models map[string]common.Model) { var knownModels []string for _, model := range models { for _, field := range model.Fields { - m2mRelation := field.GetParam("many2many") + m2mRelation := field.Tags.GetParam("many2many") if m2mRelation != nil { relatedModel := common.GetModelFromType(field.Type, models) if relatedModel == nil { @@ -70,6 +70,15 @@ func CheckMany2Many(pass *analysis.Pass, models map[string]common.Model) { // TODO: check foreign key and references fmt.Printf("Found M2M relation between \"%s\" and \"%s\"\n", model.Name, relatedModel.Name) } else { + // Check self-reference + if model.Name == relatedModel.Name { + CheckTypesOfM2M(pass, model.Name, relatedModel.Name, m2mRelation.Value, field, field) + } else { + if !relatedModel.HasPrimaryKey() { + fmt.Printf("%#v\n", relatedModel) + pass.Reportf(field.Pos, "Can't build M2M relation `%s`, primary key on `%s` model is absont", m2mRelation.Value, relatedModel.Name) + } + } // Here you can forbid M2M relations without back-reference // TODO: process m2m without backref } diff --git a/tests/foreign_key_check_test.go b/tests/foreign_key_check_test.go deleted file mode 100644 index 0076dfe..0000000 --- a/tests/foreign_key_check_test.go +++ /dev/null @@ -1,12 +0,0 @@ -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/reference_check_test.go b/tests/reference_check_test.go deleted file mode 100644 index 72e103a..0000000 --- a/tests/reference_check_test.go +++ /dev/null @@ -1,12 +0,0 @@ -package tests - -import ( - "golang.org/x/tools/go/analysis/analysistest" - "gormlint/referencesCheck" - "testing" -) - -func TestReferenceCheck(t *testing.T) { - t.Parallel() - analysistest.Run(t, analysistest.TestData(), referencesCheck.ReferenceAnalyzer, "references_check") -} diff --git a/tests/testdata/src/foreign_key_check/negative.go b/tests/testdata/src/foreign_key_check/negative.go deleted file mode 100644 index 302afec..0000000 --- a/tests/testdata/src/foreign_key_check/negative.go +++ /dev/null @@ -1,12 +0,0 @@ -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 deleted file mode 100644 index 2b84f4c..0000000 --- a/tests/testdata/src/foreign_key_check/positive.go +++ /dev/null @@ -1,27 +0,0 @@ -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;"` -} diff --git a/tests/testdata/src/references_check/negative.go b/tests/testdata/src/references_check/negative.go deleted file mode 100644 index d7d338b..0000000 --- a/tests/testdata/src/references_check/negative.go +++ /dev/null @@ -1,26 +0,0 @@ -package references_check - -// TODO: add test with annotations on back-references - -type WorkArea struct { - Id uint `gorm:"primaryKey"` - Workshop Workshop `gorm:"foreignKey:WorkshopId;references:Id;"` - WorkshopId uint -} - -type Workshop struct { - Id uint `gorm:"primaryKey"` - Name string - WorkAreas []WorkArea `gorm:"constraint:OnDelete:CASCADE;"` -} - -type TeamType struct { - Code uint `gorm:"primaryKey"` - Name string `gorm:"not null"` -} - -type TeamTask struct { - Id uint `gorm:"primaryKey"` - TeamTypeId uint - TeamType TeamType `gorm:"references:Code;"` -} diff --git a/tests/testdata/src/references_check/positive.go b/tests/testdata/src/references_check/positive.go deleted file mode 100644 index 1a59dcb..0000000 --- a/tests/testdata/src/references_check/positive.go +++ /dev/null @@ -1,21 +0,0 @@ -package references_check - -// TODO: add test with annotations on back-references - -type User struct { - Name string - CompanyID string - Company Company `gorm:"references:code"` // want "Related field \"code\" doesn't exist on model \"Company\"" -} - -type Company struct { - ID int - Code string - Name string -} - -type Order struct { - Id uint `gorm:"primaryKey"` - CompanyID string `gorm:"references:Code"` // want "Related model \"string\" doesn't exist" - Company Company -}