Config v1beta5 + static formatter (#137)

* namespace version v1beta5

* conditionally statically format

go.mod

@@ -9,7 +9,7 @@ require (
 	github.com/cli/browser v1.0.0
 	github.com/go-git/go-billy/v5 v5.3.1
 	github.com/go-git/go-git/v5 v5.4.2
-	github.com/lekkodev/rules v1.4.1-0.20230316183402-c634bcfe3adf
+	github.com/lekkodev/rules v1.5.1
 	github.com/mitchellh/go-homedir v1.1.0
 	github.com/spf13/cobra v1.5.0
 	github.com/stretchr/testify v1.7.0

go.sum

@@ -212,8 +212,8 @@ github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
 github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
 github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
 github.com/kylelemons/godebug v0.0.0-20170820004349-d65d576e9348/go.mod h1:B69LEHPfb2qLo0BaaOLcbitczOKLWTsrBG9LczfCD4k=
-github.com/lekkodev/rules v1.4.1-0.20230316183402-c634bcfe3adf h1:wO8Nxokdz3cdqIla/R3KILA5a/7KTyQRg8/nrNM5PRQ=
-github.com/lekkodev/rules v1.4.1-0.20230316183402-c634bcfe3adf/go.mod h1:Htxk+nSZEY46blL2nniZdmbrVffAfFAF5JfoNAIFulQ=
+github.com/lekkodev/rules v1.5.1 h1:B5UjaCxaiVmDa8CgjxbNVuYQMT6sxGwLs0jwav8F8nw=
+github.com/lekkodev/rules v1.5.1/go.mod h1:iiZ5wDiwbOL0wnpZp+UWG7M4P2tnHoAv4owHc9nXxxk=
 github.com/mailru/easyjson v0.0.0-20190614124828-94de47d64c63/go.mod h1:C1wdFJiN94OJF2b5HbByQZoLdCWB1Yqtg26g4irojpc=
 github.com/mailru/easyjson v0.0.0-20190626092158-b2ccc519800e/go.mod h1:C1wdFJiN94OJF2b5HbByQZoLdCWB1Yqtg26g4irojpc=
 github.com/mailru/easyjson v0.7.6 h1:8yTIVnZgCoiM1TgqoeTl+LfU5Jg6/xL3QhGQnimLYnA=

pkg/feature/version.go

@@ -27,6 +27,8 @@ const (
 	NamespaceVersionV1Beta3 NamespaceVersion = "v1beta3"
 	// Supports generating n-ary rules AST
 	NamespaceVersionV1Beta4 NamespaceVersion = "v1beta4"
+	// Supports != operator
+	NamespaceVersionV1Beta5 NamespaceVersion = "v1beta5"
 )
 
 var (
@@ -49,6 +51,7 @@ func AllNamespaceVersions() []NamespaceVersion {
 		NamespaceVersionV1Beta2,
 		NamespaceVersionV1Beta3,
 		NamespaceVersionV1Beta4,
+		NamespaceVersionV1Beta5,
 	}
 }
 
@@ -57,7 +60,7 @@ func SupportedNamespaceVersions() []NamespaceVersion {
 	all := AllNamespaceVersions()
 	start := 0
 	for i, v := range all {
-		if v == NamespaceVersionV1Beta3 { // Last supported version
+		if v == NamespaceVersionV1Beta3 { // First supported version
 			start = i
 			break
 		}

pkg/feature/version_test.go

@@ -26,6 +26,7 @@ func TestPriorVersionsSupported(t *testing.T) {
 	assert.NotContains(t, supported, NamespaceVersionV1Beta2)
 	assert.Contains(t, supported, NamespaceVersionV1Beta3)
 	assert.Contains(t, supported, NamespaceVersionV1Beta4)
+	assert.Contains(t, supported, NamespaceVersionV1Beta5)
 }
 
 func TestVersionOrder(t *testing.T) {

pkg/rules/v1beta3.go

@@ -79,6 +79,12 @@ func (v1b3 *v1beta3) evaluateRule(rule *rulesv1beta3.Rule, context map[string]in
 		switch r.Atom.ComparisonOperator {
 		case rulesv1beta3.ComparisonOperator_COMPARISON_OPERATOR_EQUALS:
 			return v1b3.evaluateEquals(r.Atom.GetComparisonValue(), runtimeCtxVal)
+		case rulesv1beta3.ComparisonOperator_COMPARISON_OPERATOR_NOT_EQUALS:
+			b, err := v1b3.evaluateEquals(r.Atom.GetComparisonValue(), runtimeCtxVal)
+			if err != nil {
+				return false, err
+			}
+			return !b, nil
 		case rulesv1beta3.ComparisonOperator_COMPARISON_OPERATOR_LESS_THAN:
 			return v1b3.evaluateNumberComparator(r.Atom.ComparisonOperator, r.Atom.GetComparisonValue(), runtimeCtxVal)
 		case rulesv1beta3.ComparisonOperator_COMPARISON_OPERATOR_LESS_THAN_OR_EQUALS:

pkg/rules/v1beta3_fixtures.go

@@ -26,6 +26,11 @@ func AgeEqualsV3(age float64) *rulesv1beta3.Atom {
 	return AgeV3("==", age)
 }
 
+// age !=
+func AgeNotEqualsV3(age float64) *rulesv1beta3.Atom {
+	return AgeV3("!=", age)
+}
+
 // city ==
 func CityEqualsV3(city string) *rulesv1beta3.Atom {
 	return CityV3("==", city)
@@ -52,6 +57,8 @@ func atomV3(key string, op string, value *structpb.Value) *rulesv1beta3.Atom {
 	switch op {
 	case "==":
 		protoOp = rulesv1beta3.ComparisonOperator_COMPARISON_OPERATOR_EQUALS
+	case "!=":
+		protoOp = rulesv1beta3.ComparisonOperator_COMPARISON_OPERATOR_NOT_EQUALS
 	case "<":
 		protoOp = rulesv1beta3.ComparisonOperator_COMPARISON_OPERATOR_LESS_THAN
 	case ">":

pkg/rules/v1beta3_test.go

@@ -127,6 +127,16 @@ func TestEqualsV3(t *testing.T) {
 			context:  CtxBuilder().Age("not a number").B(),
 			hasError: true,
 		},
+		{
+			atom:     AgeNotEqualsV3(12),
+			context:  CtxBuilder().Age(25).B(),
+			expected: true,
+		},
+		{
+			atom:     AgeNotEqualsV3(12),
+			context:  CtxBuilder().Age(12).B(),
+			expected: false,
+		},
 		{
 			atom:     AgeEqualsV3(12),
 			context:  nil, // not present

pkg/star/formatter.go

@@ -21,13 +21,10 @@ import (
 	"github.com/bazelbuild/buildtools/build"
 	butils "github.com/bazelbuild/buildtools/buildifier/utils"
 	"github.com/lekkodev/cli/pkg/fs"
+	"github.com/lekkodev/cli/pkg/star/static"
 	"github.com/pkg/errors"
 )
 
-const (
-	InputTypeAuto string = "auto"
-)
-
 type Formatter interface {
 	Format(ctx context.Context) (persisted, diffExists bool, err error)
 }
@@ -53,12 +50,10 @@ func (f *formatter) Format(ctx context.Context) (persisted, diffExists bool, err
 	if err != nil {
 		return false, false, errors.Wrapf(err, "failed to read file %s", f.filePath)
 	}
-	parser := butils.GetParser(InputTypeAuto)
-	bfile, err := parser(f.filePath, data)
+	ndata, err := f.format(data)
 	if err != nil {
-		return false, false, errors.Wrap(err, "bparse")
+		return false, false, errors.Wrap(err, "static parser format")
 	}
-	ndata := build.Format(bfile)
 	diffExists = !bytes.Equal(data, ndata)
 	if !diffExists {
 		return false, false, nil
@@ -70,3 +65,20 @@ func (f *formatter) Format(ctx context.Context) (persisted, diffExists bool, err
 	}
 	return !f.dryRun, diffExists, nil
 }
+
+func (f *formatter) format(data []byte) ([]byte, error) {
+	// first try static walking
+	ndata, err := static.NewWalker(f.filePath, data).Format()
+	if err != nil && !errors.Is(err, static.ErrUnsupportedStaticParsing) {
+		return nil, errors.Wrap(err, "static parser format")
+	} else if err == nil {
+		return ndata, nil
+	}
+	// raw formatting without static parsing
+	parser := butils.GetParser(static.InputTypeAuto)
+	bfile, err := parser(f.filePath, data)
+	if err != nil {
+		return nil, errors.Wrap(err, "bparse")
+	}
+	return build.Format(bfile), nil
+}

pkg/star/static/traverse.go

@@ -18,8 +18,17 @@ import (
 	"fmt"
 
 	"github.com/bazelbuild/buildtools/build"
-	"github.com/lekkodev/cli/pkg/star"
 	"github.com/pkg/errors"
+	"go.starlark.net/starlark"
+)
+
+const (
+	FeatureConstructor   starlark.String = "feature"
+	FeatureVariableName  string          = "result"
+	DefaultValueAttrName string          = "default"
+	DescriptionAttrName  string          = "description"
+	RulesAttrName        string          = "rules"
+	InputTypeAuto        string          = "auto"
 )
 
 func defaultNoop(v *build.Expr) error           { return nil }
@@ -70,14 +79,14 @@ func (t *traverser) traverse() error {
 	if err != nil {
 		return err
 	}
-	defaultExpr, err := ast.get(star.DefaultValueAttrName)
+	defaultExpr, err := ast.get(DefaultValueAttrName)
 	if err != nil {
 		return err
 	}
 	if err := t.defaultFn(defaultExpr); err != nil {
 		return errors.Wrap(err, "default fn")
 	}
-	descriptionExprPtr, err := ast.get(star.DescriptionAttrName)
+	descriptionExprPtr, err := ast.get(DescriptionAttrName)
 	if err != nil {
 		return err
 	}
@@ -150,7 +159,7 @@ func (ast *starFeatureAST) unset(key string) {
 
 func (ast *starFeatureAST) parseRules(fn rulesFn) error {
 	rulesW := &rulesWrapper{}
-	rulesExprPtr, err := ast.get(star.RulesAttrName)
+	rulesExprPtr, err := ast.get(RulesAttrName)
 	if err == nil { // extract existing rules
 		v := *rulesExprPtr
 		listV, ok := v.(*build.ListExpr)
@@ -169,14 +178,14 @@ func (ast *starFeatureAST) parseRules(fn rulesFn) error {
 		return err
 	}
 	if len(rulesW.rules) == 0 {
-		ast.unset(star.RulesAttrName)
+		ast.unset(RulesAttrName)
 		return nil
 	}
 	var newList []build.Expr
 	for _, rule := range rulesW.rules {
 		newList = append(newList, rule.toExpr())
 	}
-	ast.set(star.RulesAttrName, &build.ListExpr{
+	ast.set(RulesAttrName, &build.ListExpr{
 		List:           newList,
 		ForceMultiLine: true,
 	})
@@ -189,11 +198,11 @@ func (t *traverser) getFeatureAST() (*starFeatureAST, error) {
 		switch t := expr.(type) {
 		case *build.AssignExpr:
 			lhs, ok := t.LHS.(*build.Ident)
-			if ok && lhs.Name == star.FeatureVariableName {
+			if ok && lhs.Name == FeatureVariableName {
 				rhs, ok := t.RHS.(*build.CallExpr)
 				if ok && len(rhs.List) > 0 {
 					structName, ok := rhs.X.(*build.Ident)
-					if ok && structName.Name == star.FeatureConstructor.GoString() {
+					if ok && structName.Name == FeatureConstructor.GoString() {
 						return &starFeatureAST{rhs}, nil
 					}
 				}

pkg/star/static/traverse_test.go

@@ -20,14 +20,13 @@ import (
 	"github.com/bazelbuild/buildtools/build"
 	butils "github.com/bazelbuild/buildtools/buildifier/utils"
 	"github.com/lekkodev/cli/pkg/feature"
-	"github.com/lekkodev/cli/pkg/star"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
 func testFile(t *testing.T) *build.File {
 	_, _, starBytes := testStar(t, feature.FeatureTypeBool)
-	p := butils.GetParser(star.InputTypeAuto)
+	p := butils.GetParser(InputTypeAuto)
 	file, err := p("test.star", starBytes)
 	require.NoError(t, err, "failed to parse test star file")
 	return file

pkg/star/static/walk.go

@@ -24,7 +24,6 @@ import (
 	"github.com/bazelbuild/buildtools/build"
 	butils "github.com/bazelbuild/buildtools/buildifier/utils"
 	"github.com/lekkodev/cli/pkg/feature"
-	"github.com/lekkodev/cli/pkg/star"
 	"github.com/lekkodev/rules/pkg/parser"
 	"github.com/pkg/errors"
 	"go.starlark.net/starlark"
@@ -39,7 +38,12 @@ var (
 // that hold lekko features.
 type Walker interface {
 	Build() (*feature.Feature, error)
+	// Note: this method will perform mutations based on the V3 ruleslang
+	// AST if it exists. If not, it will fall back to the ruleString
+	// provided in the feature.
 	Mutate(f *feature.Feature) ([]byte, error)
+	// Returns the formatted bytes.
+	Format() ([]byte, error)
 }
 
 type walker struct {
@@ -87,8 +91,16 @@ func (w *walker) Mutate(f *feature.Feature) ([]byte, error) {
 	return t.format(), nil
 }
 
+func (w *walker) Format() ([]byte, error) {
+	f, err := w.Build()
+	if err != nil {
+		return nil, errors.Wrap(err, "build")
+	}
+	return w.Mutate(f)
+}
+
 func (w *walker) genAST() (*build.File, error) {
-	parser := butils.GetParser(star.InputTypeAuto)
+	parser := butils.GetParser(InputTypeAuto)
 	file, err := parser(w.filename, w.starBytes)
 	if err != nil {
 		return nil, errors.Wrap(err, "parse")
@@ -150,10 +162,6 @@ func (w *walker) extractValue(vPtr *build.Expr) (interface{}, feature.FeatureTyp
 			if err != nil {
 				return nil, "", errors.Wrap(err, "extract struct elem value")
 			}
-			// structValueVar, ok := vVar.(structpb.Value)
-			// if ok {
-			// 	vVar = structValueVar.Kind
-			// }
 			keyVals[key] = vVar
 		}
 		return keyVals, feature.FeatureTypeJSON, nil
@@ -378,9 +386,16 @@ func (w *walker) mutateRulesFn(f *feature.Feature) rulesFn {
 			if err != nil {
 				return errors.Wrapf(err, "rule %d: gen value", i)
 			}
+			newRuleString := r.Condition
+			if r.ConditionASTV3 != nil {
+				newRuleString, err = parser.RuleToString(r.ConditionASTV3)
+				if err != nil {
+					return errors.Wrap(err, "error attempting to parse v3 rule ast to string")
+				}
+			}
 			newRule := rule{
 				conditionV: &build.StringExpr{
-					Value: r.Condition,
+					Value: newRuleString,
 				},
 				v: gen,
 			}

pkg/star/static/walk_test.go

@@ -69,7 +69,7 @@ func testStar(t *testing.T, ft feature.FeatureType) (testVal, testVal, []byte) {
     default = %s,
     rules = [
         ("age == 10", %s),
-        ("city IN [\"Rome\", \"Milan\"]", %s),
+        ("city in [\"Rome\",\"Milan\"]", %s),
     ],
 )
 `, val.starRepr, ruleVal.starRepr, ruleVal.starRepr))
@@ -147,13 +147,29 @@ func TestWalkerMutateModifyRuleCondition(t *testing.T) {
 	require.NoError(t, err)
 	require.NotNil(t, f)
 
+	f.Rules[0].ConditionASTV3 = nil // set to nil so that mutation favors the raw string
 	f.Rules[0].Condition = "age == 12"
 
 	bytes, err := b.Mutate(f)
 	require.NoError(t, err)
 	assert.Contains(t, string(bytes), "age == 12")
 }
 
+func TestWalkerMutateModifyRuleConditionV3(t *testing.T) {
+	_, _, starBytes := testStar(t, feature.FeatureTypeBool)
+	b := testWalker(starBytes)
+	f, err := b.Build()
+	require.NoError(t, err)
+	require.NotNil(t, f)
+
+	// the AST takes precedence over the condition string
+	f.Rules[0].ConditionASTV3.GetAtom().ComparisonValue = structpb.NewNumberValue(12)
+
+	bytes, err := b.Mutate(f)
+	require.NoError(t, err)
+	assert.Contains(t, string(bytes), "age == 12")
+}
+
 func TestWalkerMutateAddRule(t *testing.T) {
 	_, _, starBytes := testStar(t, feature.FeatureTypeBool)
 	b := testWalker(starBytes)