Skip to content

Commit

Permalink
Feature/error repeated (#59)
Browse files Browse the repository at this point in the history
* add err repeated when a set is repeated

* verify set value is not the same

* add docs of new cqllint detections
  • Loading branch information
FrancoLiberali authored Jan 13, 2024
1 parent d1cb4fb commit 426007c
Show file tree
Hide file tree
Showing 18 changed files with 314 additions and 50 deletions.
21 changes: 19 additions & 2 deletions condition/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,28 @@ import (

var (
// query

ErrFieldModelNotConcerned = errors.New("field's model is not concerned by the query (not joined)")
ErrJoinMustBeSelected = errors.New("field's model is joined more than once, select which one you want to use")
ErrFieldIsRepeated = errors.New("field is repeated")

// conditions
ErrEmptyConditions = errors.New("at least one condition is required")
ErrOnlyPreloadsAllowed = errors.New("only conditions that do a preload are allowed")

ErrEmptyConditions = errors.New("at least one condition is required")

// crud

ErrMoreThanOneObjectFound = errors.New("found more that one object that meet the requested conditions")
ErrObjectNotFound = errors.New("no object exists that meets the requested conditions")

// database

ErrUnsupportedByDatabase = errors.New("method not supported by database")
ErrOrderByMustBeCalled = errors.New("order by must be called before limit in an update statement")

// preload

ErrOnlyPreloadsAllowed = errors.New("only conditions that do a preload are allowed")
)

func methodError(err error, method string) error {
Expand All @@ -43,6 +52,14 @@ func joinMustBeSelectedError(field IField) error {
)
}

func fieldIsRepeatedError(field IField) error {
return fmt.Errorf("%w; field: %s.%s",
ErrFieldIsRepeated,
field.getModelType(),
field.fieldName(),
)
}

func preloadsInReturningNotAllowed(dialector sql.Dialector) error {
return fmt.Errorf("%w; preloads in returning are not allowed for database: %s",
ErrUnsupportedByDatabase,
Expand Down
5 changes: 5 additions & 0 deletions condition/gorm_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,11 @@ func getUpdateTablesAndValues(query *GormQuery, sets []ISet) (map[IField]TableAn
return nil, err
}

_, fieldAlreadyPresent := tables[field]
if fieldAlreadyPresent {
return nil, fieldIsRepeatedError(field)
}

tables[field] = TableAndValue{
table: table,
value: updateValue,
Expand Down
83 changes: 73 additions & 10 deletions cqllint/pkg/analyzer/analyzer.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,15 @@ var Analyzer = &analysis.Analyzer{
URL: "compiledquerylenguage.readthedocs.io",
Run: run,
Requires: []*analysis.Analyzer{inspect.Analyzer},
// Version: cql.Version,
}

var (
cqlMethods = []string{"Query", "Update", "Delete"}
cqlOrder = []string{"Descending", "Ascending"}
cqlSetMultiple = "SetMultiple"
cqlSet = "Set"
cqlSelectors = append(cqlOrder, cqlSetMultiple, cqlSet)
cqlSets = []string{cqlSetMultiple, "Set"}
cqlSelectors = append(cqlOrder, cqlSets...)
dynamicMethod = "Dynamic"
)

type Model struct {
Expand Down Expand Up @@ -66,7 +66,7 @@ func run(pass *analysis.Pass) (interface{}, error) {
if isSelector {
positionsToReport = findForSelector(callExpr, positionsToReport)
} else {
positionsToReport, _ = findForIndex(callExpr, positionsToReport)
positionsToReport, _ = findNotConcernedForIndex(callExpr, positionsToReport)
}
})

Expand All @@ -81,15 +81,22 @@ func run(pass *analysis.Pass) (interface{}, error) {
return nil, nil //nolint:nilnil // is necessary
}

// Finds errors in selector functions: Descending, Ascending, SetMultiple, Set
// Finds NotConcerned and Repeated errors in selector functions: Descending, Ascending, SetMultiple, Set
func findForSelector(callExpr *ast.CallExpr, positionsToReport []Model) []Model {
selectorExpr := callExpr.Fun.(*ast.SelectorExpr)

if !pie.Contains(cqlSelectors, selectorExpr.Sel.Name) {
return positionsToReport
}

_, models := findForIndex(selectorExpr.X.(*ast.CallExpr), positionsToReport)
findRepeatedFields(callExpr, selectorExpr)

return fieldNotConcerned(callExpr, selectorExpr, positionsToReport)
}

// Finds NotConcerned errors in selector functions: Descending, Ascending, SetMultiple, Set
func fieldNotConcerned(callExpr *ast.CallExpr, selectorExpr *ast.SelectorExpr, positionsToReport []Model) []Model {
_, models := findNotConcernedForIndex(selectorExpr.X.(*ast.CallExpr), positionsToReport)

for _, arg := range callExpr.Args {
var model Model
Expand All @@ -101,9 +108,10 @@ func findForSelector(callExpr *ast.CallExpr, positionsToReport []Model) []Model
positionsToReport = addPositionsToReport(positionsToReport, models, model)
} else {
argCallExpr := arg.(*ast.CallExpr)

setFunction := argCallExpr.Fun.(*ast.SelectorExpr).Sel.Name

if setFunction == "Dynamic" {
if setFunction == dynamicMethod {
model = getModel(argCallExpr.Args[0].(*ast.CallExpr).Fun.(*ast.SelectorExpr).X.(*ast.SelectorExpr).X.(*ast.SelectorExpr))
positionsToReport = addPositionsToReport(positionsToReport, models, model)
}
Expand All @@ -118,16 +126,71 @@ func findForSelector(callExpr *ast.CallExpr, positionsToReport []Model) []Model
return positionsToReport
}

// Finds errors in index functions: cql.Query, cql.Update, cql.Delete
func findForIndex(callExpr *ast.CallExpr, positionsToReport []Model) ([]Model, []string) {
func findRepeatedFields(call *ast.CallExpr, selectorExpr *ast.SelectorExpr) {
if !pie.Contains(cqlSets, selectorExpr.Sel.Name) {
return
}

fields := map[string][]token.Pos{}

for _, arg := range call.Args {
argCall := arg.(*ast.CallExpr)
argSelector := argCall.Fun.(*ast.SelectorExpr)
condition := argSelector.X.(*ast.CallExpr).Fun.(*ast.SelectorExpr).X.(*ast.SelectorExpr)

fieldName := getFieldName(condition)
fieldPos := condition.Sel.NamePos

_, isPresent := fields[fieldName]
if !isPresent {
fields[fieldName] = []token.Pos{fieldPos}
} else {
fields[fieldName] = append(fields[fieldName], fieldPos)
}

if argSelector.Sel.Name == dynamicMethod && len(argCall.Args) == 1 {
comparedField := argCall.Args[0].(*ast.CallExpr).Fun.(*ast.SelectorExpr).X.(*ast.SelectorExpr)
comparedFieldName := getFieldName(comparedField)

if comparedFieldName == fieldName {
passG.Reportf(
comparedField.Sel.NamePos,
"%s is set to itself",
comparedFieldName,
)
}
}
}

for fieldName, positions := range fields {
if len(positions) > 1 {
for _, pos := range positions {
passG.Reportf(
pos,
"%s is repeated",
fieldName,
)
}
}
}
}

func getFieldName(condition *ast.SelectorExpr) string {
conditionModel := condition.X.(*ast.SelectorExpr)

return conditionModel.X.(*ast.Ident).Name + "." + conditionModel.Sel.Name + "." + condition.Sel.Name
}

// Finds NotConcerned errors in index functions: cql.Query, cql.Update, cql.Delete
func findNotConcernedForIndex(callExpr *ast.CallExpr, positionsToReport []Model) ([]Model, []string) {
indexExpr, isIndex := callExpr.Fun.(*ast.IndexExpr)
if !isIndex {
// other functions may be between callExpr and the cql method, example: cql.Query(...).Limit(1).Descending
selectorExpr, isSelector := callExpr.Fun.(*ast.SelectorExpr)
if isSelector {
internalCallExpr, isCall := selectorExpr.X.(*ast.CallExpr)
if isCall {
return findForIndex(internalCallExpr, positionsToReport)
return findNotConcernedForIndex(internalCallExpr, positionsToReport)
}
}

Expand Down
9 changes: 7 additions & 2 deletions cqllint/pkg/analyzer/analyzer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,12 @@ import (
"github.com/FrancoLiberali/cql/cqllint/pkg/analyzer"
)

func TestAll(t *testing.T) {
func TestErrNotConcerned(t *testing.T) {
testdata := analysistest.TestData()
analysistest.Run(t, testdata, analyzer.Analyzer, "a")
analysistest.Run(t, testdata, analyzer.Analyzer, "not_concerned")
}

func TestErrRepeated(t *testing.T) {
testdata := analysistest.TestData()
analysistest.Run(t, testdata, analyzer.Analyzer, "repeated")
}
7 changes: 5 additions & 2 deletions cqllint/pkg/analyzer/testdata/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@ module github.com/FrancoLiberali/cql/cqllint/pkg/analyzer/testdata
go 1.18

require (
a v0.0.1
not_concerned v0.0.1
repeated v0.0.1
)

replace a => ./src/a
replace not_concerned => ./src/not_concerned

replace repeated => ./src/repeated
10 changes: 10 additions & 0 deletions cqllint/pkg/analyzer/testdata/src/not_concerned/go.mod
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
module github.com/FrancoLiberali/cql/cqllint/pkg/analyzer/testdata/src/not_concerned

go 1.18

require (
gorm.io/gorm v1.25.6
github.com/FrancoLiberali/cql v0.0.1
)

replace github.com/FrancoLiberali/cql => ./../../../../../..
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package a
package not_concerned

import (
"github.com/FrancoLiberali/cql"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package a
package not_concerned

import (
"gorm.io/gorm"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package a
package not_concerned

import (
"github.com/FrancoLiberali/cql"
Expand All @@ -23,26 +23,26 @@ func testSetDynamicNotJoinedInDifferentLines() {
}

func testSetDynamicMainModel() {
cql.Update[models.Brand](
cql.Update[models.Product](
db,
conditions.Brand.Name.IsDynamic().Eq(conditions.City.Name.Value()), // want "github.com/FrancoLiberali/cql/test/models.City is not joined by the query"
conditions.Product.String.IsDynamic().Eq(conditions.City.Name.Value()), // want "github.com/FrancoLiberali/cql/test/models.City is not joined by the query"
).Set(
conditions.Brand.Name.Set().Dynamic(
conditions.Brand.Name.Value(),
conditions.Product.Int.Set().Dynamic(
conditions.Product.IntPointer.Value(),
),
)
}

func testSetDynamicMainModelMultipleTimes() {
cql.Update[models.Brand](
cql.Update[models.Product](
db,
conditions.Brand.Name.IsDynamic().Eq(conditions.City.Name.Value()), // want "github.com/FrancoLiberali/cql/test/models.City is not joined by the query"
conditions.Product.String.IsDynamic().Eq(conditions.City.Name.Value()), // want "github.com/FrancoLiberali/cql/test/models.City is not joined by the query"
).Set(
conditions.Brand.Name.Set().Dynamic(
conditions.Brand.Name.Value(),
conditions.Product.String.Set().Dynamic(
conditions.Product.String2.Value(),
),
conditions.Brand.Name.Set().Dynamic(
conditions.Brand.Name.Value(),
conditions.Product.Int.Set().Dynamic(
conditions.Product.IntPointer.Value(),
),
)
}
Expand All @@ -54,7 +54,6 @@ func testSetDynamicJoinedModel() {
conditions.Phone.Name.IsDynamic().Eq(conditions.City.Name.Value()), // want "github.com/FrancoLiberali/cql/test/models.City is not joined by the query"
).Set(
conditions.Phone.Name.Set().Dynamic(conditions.Brand.Name.Value()),
conditions.Phone.Name.Set().Dynamic(conditions.Brand.Name.Value()),
)
}

Expand All @@ -67,7 +66,7 @@ func testSetDynamicNestedJoinedModel() {
conditions.Child.Name.IsDynamic().Eq(conditions.City.Name.Value()), // want "github.com/FrancoLiberali/cql/test/models.City is not joined by the query"
).Set(
conditions.Child.Name.Set().Dynamic(conditions.Parent1.Name.Value()),
conditions.Child.Name.Set().Dynamic(conditions.ParentParent.Name.Value()),
conditions.Child.Number.Set().Dynamic(conditions.ParentParent.Number.Value()),
)
}

Expand Down Expand Up @@ -97,12 +96,12 @@ func testSetMultipleMainModel() {
}

func testSetMultipleMainModelMultipleTimes() {
cql.Update[models.Brand](
cql.Update[models.Product](
db,
conditions.Brand.Name.IsDynamic().Eq(conditions.City.Name.Value()), // want "github.com/FrancoLiberali/cql/test/models.City is not joined by the query"
conditions.Product.String.IsDynamic().Eq(conditions.City.Name.Value()), // want "github.com/FrancoLiberali/cql/test/models.City is not joined by the query"
).SetMultiple(
conditions.Brand.Name.Set().Eq("asd"),
conditions.Brand.Name.Set().Eq("asd"),
conditions.Product.String.Set().Eq("asd"),
conditions.Product.Int.Set().Eq(1),
)
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
module github.com/FrancoLiberali/cql/cqllint/pkg/analyzer/testdata/src/a
module github.com/FrancoLiberali/cql/cqllint/pkg/analyzer/testdata/src/repeated

go 1.18

Expand Down
Loading

0 comments on commit 426007c

Please sign in to comment.