From b209c4e97d686b9a97c67dceab492279639ede2e Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Wed, 18 Mar 2020 22:05:47 -0400 Subject: [PATCH] New Checks: S035 through S037 for invalid attribute references (#105) Reference: https://github.com/bflad/tfproviderlint/issues/104 --- CHANGELOG.md | 12 +++ README.md | 3 + helper/analysisutils/schema_analyzers.go | 29 +++++++ helper/analysisutils/schema_runners.go | 50 ++++++++++++ helper/astutils/expr.go | 40 ++++++++++ helper/astutils/function_parameters.go | 40 ---------- .../terraformtype/helper/schema/attributes.go | 61 +++++++++++++++ passes/S015/S015.go | 5 +- passes/S035/README.md | 32 ++++++++ passes/S035/S035.go | 8 ++ passes/S035/S035_test.go | 12 +++ passes/S035/testdata/src/a/alias.go | 21 +++++ passes/S035/testdata/src/a/main.go | 78 +++++++++++++++++++ passes/S035/testdata/src/a/outside_package.go | 21 +++++ passes/S035/testdata/src/a/schema/schema.go | 5 ++ passes/S035/testdata/src/a/vendor | 1 + passes/S036/README.md | 32 ++++++++ passes/S036/S036.go | 8 ++ passes/S036/S036_test.go | 12 +++ passes/S036/testdata/src/a/alias.go | 21 +++++ passes/S036/testdata/src/a/main.go | 78 +++++++++++++++++++ passes/S036/testdata/src/a/outside_package.go | 21 +++++ passes/S036/testdata/src/a/schema/schema.go | 5 ++ passes/S036/testdata/src/a/vendor | 1 + passes/S037/README.md | 32 ++++++++ passes/S037/S037.go | 8 ++ passes/S037/S037_test.go | 12 +++ passes/S037/testdata/src/a/alias.go | 21 +++++ passes/S037/testdata/src/a/main.go | 78 +++++++++++++++++++ passes/S037/testdata/src/a/outside_package.go | 21 +++++ passes/S037/testdata/src/a/schema/schema.go | 5 ++ passes/S037/testdata/src/a/vendor | 1 + passes/checks.go | 6 ++ .../schema/crudfuncinfo/crudfuncinfo.go | 4 +- .../schemavalidatefuncinfo.go | 8 +- 35 files changed, 742 insertions(+), 50 deletions(-) create mode 100644 helper/analysisutils/schema_analyzers.go create mode 100644 helper/analysisutils/schema_runners.go create mode 100644 helper/astutils/expr.go delete mode 100644 helper/astutils/function_parameters.go create mode 100644 helper/terraformtype/helper/schema/attributes.go create mode 100644 passes/S035/README.md create mode 100644 passes/S035/S035.go create mode 100644 passes/S035/S035_test.go create mode 100644 passes/S035/testdata/src/a/alias.go create mode 100644 passes/S035/testdata/src/a/main.go create mode 100644 passes/S035/testdata/src/a/outside_package.go create mode 100644 passes/S035/testdata/src/a/schema/schema.go create mode 120000 passes/S035/testdata/src/a/vendor create mode 100644 passes/S036/README.md create mode 100644 passes/S036/S036.go create mode 100644 passes/S036/S036_test.go create mode 100644 passes/S036/testdata/src/a/alias.go create mode 100644 passes/S036/testdata/src/a/main.go create mode 100644 passes/S036/testdata/src/a/outside_package.go create mode 100644 passes/S036/testdata/src/a/schema/schema.go create mode 120000 passes/S036/testdata/src/a/vendor create mode 100644 passes/S037/README.md create mode 100644 passes/S037/S037.go create mode 100644 passes/S037/S037_test.go create mode 100644 passes/S037/testdata/src/a/alias.go create mode 100644 passes/S037/testdata/src/a/main.go create mode 100644 passes/S037/testdata/src/a/outside_package.go create mode 100644 passes/S037/testdata/src/a/schema/schema.go create mode 120000 passes/S037/testdata/src/a/vendor diff --git a/CHANGELOG.md b/CHANGELOG.md index 45ac1055..dfccd2d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,15 @@ +# v0.12.0 + +BREAKING CHANGES + +* helper/astutils: The `IsFunctionParameterType*` functions have been renamed `IsExprType*` to better denote their purpose. + +FEATURES + +* **New Check:** `S035`: check for `Schema` with invalid `AtLeastOneOf` attribute references +* **New Check:** `S036`: check for `Schema` with invalid `ConflictsWith` attribute references +* **New Check:** `S037`: check for `Schema` with invalid `ExactlyOneOf` attribute references + # v0.11.0 ENHANCEMENTS diff --git a/README.md b/README.md index a192c854..7265ec55 100644 --- a/README.md +++ b/README.md @@ -140,6 +140,9 @@ Standard lint checks are enabled by default in the `tfproviderlint` tool. Opt-in | [S032](passes/S032/README.md) | check for `Schema` of `Computed` only with `MinItems` configured | AST | | [S033](passes/S033/README.md) | check for `Schema` of `Computed` only with `StateFunc` configured | AST | | [S034](passes/S034/README.md) | check for `Schema` that configure `PromoteSingle` | AST | +| [S035](passes/S035/README.md) | check for `Schema` with invalid `AtLeastOneOf` attribute references | AST | +| [S036](passes/S036/README.md) | check for `Schema` with invalid `ConflictsWith` attribute references | AST | +| [S037](passes/S037/README.md) | check for `Schema` with invalid `ExactlyOneOf` attribute references | AST | ### Standard Validation Checks diff --git a/helper/analysisutils/schema_analyzers.go b/helper/analysisutils/schema_analyzers.go new file mode 100644 index 00000000..396148ce --- /dev/null +++ b/helper/analysisutils/schema_analyzers.go @@ -0,0 +1,29 @@ +package analysisutils + +import ( + "fmt" + + "github.com/bflad/tfproviderlint/passes/commentignore" + "github.com/bflad/tfproviderlint/passes/helper/schema/schemainfo" + "golang.org/x/tools/go/analysis" +) + +// SchemaAttributeReferencesAnalyzer returns an Analyzer for fields that use schema attribute references +func SchemaAttributeReferencesAnalyzer(analyzerName string, fieldName string) *analysis.Analyzer { + doc := fmt.Sprintf(`check for Schema with invalid %[2]s references + +The %[1]s analyzer ensures schema attribute references in the Schema %[2]s +field use valid syntax. The Terraform Plugin SDK can unit test attribute +references to verify the references against the full schema. +`, analyzerName, fieldName) + + return &analysis.Analyzer{ + Name: analyzerName, + Doc: doc, + Requires: []*analysis.Analyzer{ + commentignore.Analyzer, + schemainfo.Analyzer, + }, + Run: SchemaAttributeReferencesRunner(analyzerName, fieldName), + } +} diff --git a/helper/analysisutils/schema_runners.go b/helper/analysisutils/schema_runners.go new file mode 100644 index 00000000..fde4b9ab --- /dev/null +++ b/helper/analysisutils/schema_runners.go @@ -0,0 +1,50 @@ +package analysisutils + +import ( + "go/ast" + + "github.com/bflad/tfproviderlint/helper/astutils" + "github.com/bflad/tfproviderlint/helper/terraformtype/helper/schema" + "github.com/bflad/tfproviderlint/passes/commentignore" + "github.com/bflad/tfproviderlint/passes/helper/schema/schemainfo" + "golang.org/x/tools/go/analysis" +) + +// SchemaAttributeReferencesRunner returns an Analyzer runner for fields that use schema attribute references +func SchemaAttributeReferencesRunner(analyzerName string, fieldName string) func(*analysis.Pass) (interface{}, error) { + return func(pass *analysis.Pass) (interface{}, error) { + ignorer := pass.ResultOf[commentignore.Analyzer].(*commentignore.Ignorer) + schemaInfos := pass.ResultOf[schemainfo.Analyzer].([]*schema.SchemaInfo) + + for _, schemaInfo := range schemaInfos { + if ignorer.ShouldIgnore(analyzerName, schemaInfo.AstCompositeLit) { + continue + } + + if !schemaInfo.DeclaresField(fieldName) { + continue + } + + switch value := schemaInfo.Fields[fieldName].Value.(type) { + case *ast.CompositeLit: + if !astutils.IsExprTypeArrayString(value.Type) { + continue + } + + for _, elt := range value.Elts { + attributeReference := astutils.ExprStringValue(elt) + + if attributeReference == nil { + continue + } + + if _, err := schema.ParseAttributeReference(*attributeReference); err != nil { + pass.Reportf(elt.Pos(), "%s: invalid %s attribute reference: %s", analyzerName, fieldName, err) + } + } + } + } + + return nil, nil + } +} diff --git a/helper/astutils/expr.go b/helper/astutils/expr.go new file mode 100644 index 00000000..ebd012d0 --- /dev/null +++ b/helper/astutils/expr.go @@ -0,0 +1,40 @@ +package astutils + +import ( + "go/ast" +) + +// IsExprTypeArrayString returns true if the expression matches []string +func IsExprTypeArrayString(e ast.Expr) bool { + arrayType, ok := e.(*ast.ArrayType) + + return ok && IsExprTypeString(arrayType.Elt) +} + +// IsExprTypeArrayError returns true if the expression matches []error +func IsExprTypeArrayError(e ast.Expr) bool { + arrayType, ok := e.(*ast.ArrayType) + + return ok && IsExprTypeError(arrayType.Elt) +} + +// IsExprTypeError returns true if the expression matches string +func IsExprTypeError(e ast.Expr) bool { + ident, ok := e.(*ast.Ident) + + return ok && ident.Name == "error" +} + +// IsExprTypeInterface returns true if the expression matches interface{} +func IsExprTypeInterface(e ast.Expr) bool { + _, ok := e.(*ast.InterfaceType) + + return ok +} + +// IsExprTypeString returns true if the expression matches string +func IsExprTypeString(e ast.Expr) bool { + ident, ok := e.(*ast.Ident) + + return ok && ident.Name == "string" +} diff --git a/helper/astutils/function_parameters.go b/helper/astutils/function_parameters.go deleted file mode 100644 index 941b88dc..00000000 --- a/helper/astutils/function_parameters.go +++ /dev/null @@ -1,40 +0,0 @@ -package astutils - -import ( - "go/ast" -) - -// IsFunctionParameterTypeArrayString returns true if the expression matches []string -func IsFunctionParameterTypeArrayString(e ast.Expr) bool { - arrayType, ok := e.(*ast.ArrayType) - - return ok && IsFunctionParameterTypeString(arrayType.Elt) -} - -// IsFunctionParameterTypeArrayError returns true if the expression matches []error -func IsFunctionParameterTypeArrayError(e ast.Expr) bool { - arrayType, ok := e.(*ast.ArrayType) - - return ok && IsFunctionParameterTypeError(arrayType.Elt) -} - -// IsFunctionParameterTypeError returns true if the expression matches string -func IsFunctionParameterTypeError(e ast.Expr) bool { - ident, ok := e.(*ast.Ident) - - return ok && ident.Name == "error" -} - -// IsFunctionParameterTypeInterface returns true if the expression matches interface{} -func IsFunctionParameterTypeInterface(e ast.Expr) bool { - _, ok := e.(*ast.InterfaceType) - - return ok -} - -// IsFunctionParameterTypeString returns true if the expression matches string -func IsFunctionParameterTypeString(e ast.Expr) bool { - ident, ok := e.(*ast.Ident) - - return ok && ident.Name == "string" -} diff --git a/helper/terraformtype/helper/schema/attributes.go b/helper/terraformtype/helper/schema/attributes.go new file mode 100644 index 00000000..8888276a --- /dev/null +++ b/helper/terraformtype/helper/schema/attributes.go @@ -0,0 +1,61 @@ +package schema + +import ( + "fmt" + "math" + "regexp" + "strconv" + "strings" +) + +const ( + // Pattern for schema attribute names + AttributeNameRegexpPattern = `^[a-z0-9_]+$` + + // Pattern for schema references to attributes, such as ConflictsWith values + AttributeReferenceRegexpPattern = `^[a-z0-9_]+(\.[a-z0-9_]+)*$` +) + +var ( + AttributeNameRegexp = regexp.MustCompile(AttributeNameRegexpPattern) + AttributeReferenceRegexp = regexp.MustCompile(AttributeReferenceRegexpPattern) +) + +// ParseAttributeReference validates and returns the split representation of schema attribute reference. +// Attribute references are used in Schema fields such as AtLeastOneOf, ConflictsWith, and ExactlyOneOf. +func ParseAttributeReference(reference string) ([]string, error) { + if !AttributeReferenceRegexp.MatchString(reference) { + return nil, fmt.Errorf("%q must contain only valid attribute names, separated by periods", reference) + } + + attributeReferenceParts := strings.Split(reference, ".") + + if len(attributeReferenceParts) == 1 { + return attributeReferenceParts, nil + } + + configurationBlockReferenceErr := fmt.Errorf("%q configuration block attribute references are only valid for TypeList and MaxItems: 1 attributes and nested attributes must be separated by .0.", reference) + + if math.Mod(float64(len(attributeReferenceParts)), 2) == 0 { + return attributeReferenceParts, configurationBlockReferenceErr + } + + // All even parts of an attribute reference must be 0 + for idx, attributeReferencePart := range attributeReferenceParts { + if math.Mod(float64(idx), 2) == 0 { + continue + } + + attributeReferencePartInt, err := strconv.Atoi(attributeReferencePart) + + if err != nil { + return attributeReferenceParts, configurationBlockReferenceErr + } + + if attributeReferencePartInt != 0 { + return attributeReferenceParts, configurationBlockReferenceErr + } + } + + return attributeReferenceParts, nil +} diff --git a/passes/S015/S015.go b/passes/S015/S015.go index 1e62bd2d..9ecfe7bc 100644 --- a/passes/S015/S015.go +++ b/passes/S015/S015.go @@ -5,7 +5,6 @@ package S015 import ( "go/ast" - "regexp" "strings" "golang.org/x/tools/go/analysis" @@ -37,8 +36,6 @@ func run(pass *analysis.Pass) (interface{}, error) { ignorer := pass.ResultOf[commentignore.Analyzer].(*commentignore.Ignorer) schemamapcompositelits := pass.ResultOf[schemamapcompositelit.Analyzer].([]*ast.CompositeLit) - attributeNameRegex := regexp.MustCompile(`^[a-z0-9_]+$`) - for _, smap := range schemamapcompositelits { if ignorer.ShouldIgnore(analyzerName, smap) { continue @@ -51,7 +48,7 @@ func run(pass *analysis.Pass) (interface{}, error) { case *ast.BasicLit: value := strings.Trim(t.Value, `"`) - if !attributeNameRegex.MatchString(value) { + if !schema.AttributeNameRegexp.MatchString(value) { pass.Reportf(t.Pos(), "%s: schema attribute names should only be lowercase alphanumeric characters or underscores", analyzerName) } } diff --git a/passes/S035/README.md b/passes/S035/README.md new file mode 100644 index 00000000..28233004 --- /dev/null +++ b/passes/S035/README.md @@ -0,0 +1,32 @@ +# S035 + +The S035 analyzer reports cases of Schemas which include `AtLeastOneOf` and have invalid schema attribute references. + +NOTE: This only verifies the syntax of attribute references. The Terraform Plugin SDK can unit test attribute references to verify the references against the full schema. + +## Flagged Code + +```go +&schema.Schema{ + AtLeastOneOf: []string{"config_block_attr.nested_attr"}, +} +``` + +## Passing Code + +```go +&schema.Schema{ + AtLeastOneOf: []string{"config_block_attr.0.nested_attr"}, +} +``` + +## Ignoring Reports + +Singular reports can be ignored by adding the a `//lintignore:S035` Go code comment at the end of the offending line or on the line immediately proceding, e.g. + +```go +//lintignore:S035 +&schema.Schema{ + AtLeastOneOf: []string{"config_block_attr.nested_attr"}, +} +``` diff --git a/passes/S035/S035.go b/passes/S035/S035.go new file mode 100644 index 00000000..48a57473 --- /dev/null +++ b/passes/S035/S035.go @@ -0,0 +1,8 @@ +package S035 + +import ( + "github.com/bflad/tfproviderlint/helper/analysisutils" + "github.com/bflad/tfproviderlint/helper/terraformtype/helper/schema" +) + +var Analyzer = analysisutils.SchemaAttributeReferencesAnalyzer("S035", schema.SchemaFieldAtLeastOneOf) diff --git a/passes/S035/S035_test.go b/passes/S035/S035_test.go new file mode 100644 index 00000000..fd804f81 --- /dev/null +++ b/passes/S035/S035_test.go @@ -0,0 +1,12 @@ +package S035 + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" +) + +func TestS035(t *testing.T) { + testdata := analysistest.TestData() + analysistest.Run(t, testdata, Analyzer, "a") +} diff --git a/passes/S035/testdata/src/a/alias.go b/passes/S035/testdata/src/a/alias.go new file mode 100644 index 00000000..d55cb0ea --- /dev/null +++ b/passes/S035/testdata/src/a/alias.go @@ -0,0 +1,21 @@ +package a + +import ( + s "github.com/hashicorp/terraform-plugin-sdk/helper/schema" +) + +func falias() { + _ = s.Schema{ + AtLeastOneOf: []string{ + ".invalidreference", // want "invalid AtLeastOneOf attribute reference" + }, + } + + _ = map[string]*s.Schema{ + "name": { + AtLeastOneOf: []string{ + ".invalidreference", // want "invalid AtLeastOneOf attribute reference" + }, + }, + } +} diff --git a/passes/S035/testdata/src/a/main.go b/passes/S035/testdata/src/a/main.go new file mode 100644 index 00000000..87efaba8 --- /dev/null +++ b/passes/S035/testdata/src/a/main.go @@ -0,0 +1,78 @@ +package a + +import ( + "github.com/hashicorp/terraform-plugin-sdk/helper/schema" +) + +func f() { + // Comment ignored + + //lintignore:S035 + _ = schema.Schema{ + AtLeastOneOf: []string{ + ".invalidreference", + "InvalidReference", + "invalidreference!", + "invalid-reference", + "invalid.reference", + "invalid..reference", + "invalid.123.reference", + "invalid.0.sub.reference", + "invalid.sub.0.reference", + }, + } + + // Failing + + _ = schema.Schema{ + AtLeastOneOf: []string{ + ".invalidreference", // want "invalid AtLeastOneOf attribute reference" + "InvalidReference", // want "invalid AtLeastOneOf attribute reference" + "invalidreference!", // want "invalid AtLeastOneOf attribute reference" + "invalid-reference", // want "invalid AtLeastOneOf attribute reference" + "invalid.reference", // want "invalid AtLeastOneOf attribute reference" + "invalid..reference", // want "invalid AtLeastOneOf attribute reference" + "invalid.123.reference", // want "invalid AtLeastOneOf attribute reference" + "invalid.0.sub.reference", // want "invalid AtLeastOneOf attribute reference" + "invalid.sub.0.reference", // want "invalid AtLeastOneOf attribute reference" + }, + } + + _ = map[string]*schema.Schema{ + "name": { + AtLeastOneOf: []string{ + ".invalidreference", // want "invalid AtLeastOneOf attribute reference" + "InvalidReference", // want "invalid AtLeastOneOf attribute reference" + "invalidreference!", // want "invalid AtLeastOneOf attribute reference" + "invalid-reference", // want "invalid AtLeastOneOf attribute reference" + "invalid.reference", // want "invalid AtLeastOneOf attribute reference" + "invalid..reference", // want "invalid AtLeastOneOf attribute reference" + "invalid.123.reference", // want "invalid AtLeastOneOf attribute reference" + "invalid.0.sub.reference", // want "invalid AtLeastOneOf attribute reference" + "invalid.sub.0.reference", // want "invalid AtLeastOneOf attribute reference" + }, + }, + } + + // Passing + + _ = schema.Schema{ + AtLeastOneOf: []string{ + "validreference", + "valid_reference", + "valid.0.reference", + "valid.0.sub.0.reference", + }, + } + + _ = map[string]*schema.Schema{ + "name": { + AtLeastOneOf: []string{ + "validreference", + "valid_reference", + "valid.0.reference", + "valid.0.sub.0.reference", + }, + }, + } +} diff --git a/passes/S035/testdata/src/a/outside_package.go b/passes/S035/testdata/src/a/outside_package.go new file mode 100644 index 00000000..d1a1be2c --- /dev/null +++ b/passes/S035/testdata/src/a/outside_package.go @@ -0,0 +1,21 @@ +package a + +import ( + "a/schema" +) + +func foutside() { + _ = schema.Schema{ + AtLeastOneOf: []string{ + ".invalidreference", + }, + } + + _ = map[string]*schema.Schema{ + "name": { + AtLeastOneOf: []string{ + ".invalidreference", + }, + }, + } +} diff --git a/passes/S035/testdata/src/a/schema/schema.go b/passes/S035/testdata/src/a/schema/schema.go new file mode 100644 index 00000000..d5aeffa2 --- /dev/null +++ b/passes/S035/testdata/src/a/schema/schema.go @@ -0,0 +1,5 @@ +package schema + +type Schema struct { + AtLeastOneOf []string +} diff --git a/passes/S035/testdata/src/a/vendor b/passes/S035/testdata/src/a/vendor new file mode 120000 index 00000000..776800d2 --- /dev/null +++ b/passes/S035/testdata/src/a/vendor @@ -0,0 +1 @@ +../../../../../vendor \ No newline at end of file diff --git a/passes/S036/README.md b/passes/S036/README.md new file mode 100644 index 00000000..1b00945d --- /dev/null +++ b/passes/S036/README.md @@ -0,0 +1,32 @@ +# S036 + +The S036 analyzer reports cases of Schemas which include `ConflictsWith` and have invalid schema attribute references. + +NOTE: This only verifies the syntax of attribute references. The Terraform Plugin SDK can unit test attribute references to verify the references against the full schema. + +## Flagged Code + +```go +&schema.Schema{ + ConflictsWith: []string{"config_block_attr.nested_attr"}, +} +``` + +## Passing Code + +```go +&schema.Schema{ + ConflictsWith: []string{"config_block_attr.0.nested_attr"}, +} +``` + +## Ignoring Reports + +Singular reports can be ignored by adding the a `//lintignore:S036` Go code comment at the end of the offending line or on the line immediately proceding, e.g. + +```go +//lintignore:S036 +&schema.Schema{ + ConflictsWith: []string{"config_block_attr.nested_attr"}, +} +``` diff --git a/passes/S036/S036.go b/passes/S036/S036.go new file mode 100644 index 00000000..8121a618 --- /dev/null +++ b/passes/S036/S036.go @@ -0,0 +1,8 @@ +package S036 + +import ( + "github.com/bflad/tfproviderlint/helper/analysisutils" + "github.com/bflad/tfproviderlint/helper/terraformtype/helper/schema" +) + +var Analyzer = analysisutils.SchemaAttributeReferencesAnalyzer("S036", schema.SchemaFieldConflictsWith) diff --git a/passes/S036/S036_test.go b/passes/S036/S036_test.go new file mode 100644 index 00000000..eecc598b --- /dev/null +++ b/passes/S036/S036_test.go @@ -0,0 +1,12 @@ +package S036 + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" +) + +func TestS036(t *testing.T) { + testdata := analysistest.TestData() + analysistest.Run(t, testdata, Analyzer, "a") +} diff --git a/passes/S036/testdata/src/a/alias.go b/passes/S036/testdata/src/a/alias.go new file mode 100644 index 00000000..6e1329be --- /dev/null +++ b/passes/S036/testdata/src/a/alias.go @@ -0,0 +1,21 @@ +package a + +import ( + s "github.com/hashicorp/terraform-plugin-sdk/helper/schema" +) + +func falias() { + _ = s.Schema{ + ConflictsWith: []string{ + ".invalidreference", // want "invalid ConflictsWith attribute reference" + }, + } + + _ = map[string]*s.Schema{ + "name": { + ConflictsWith: []string{ + ".invalidreference", // want "invalid ConflictsWith attribute reference" + }, + }, + } +} diff --git a/passes/S036/testdata/src/a/main.go b/passes/S036/testdata/src/a/main.go new file mode 100644 index 00000000..a8bf7fae --- /dev/null +++ b/passes/S036/testdata/src/a/main.go @@ -0,0 +1,78 @@ +package a + +import ( + "github.com/hashicorp/terraform-plugin-sdk/helper/schema" +) + +func f() { + // Comment ignored + + //lintignore:S036 + _ = schema.Schema{ + ConflictsWith: []string{ + ".invalidreference", + "InvalidReference", + "invalidreference!", + "invalid-reference", + "invalid.reference", + "invalid..reference", + "invalid.123.reference", + "invalid.0.sub.reference", + "invalid.sub.0.reference", + }, + } + + // Failing + + _ = schema.Schema{ + ConflictsWith: []string{ + ".invalidreference", // want "invalid ConflictsWith attribute reference" + "InvalidReference", // want "invalid ConflictsWith attribute reference" + "invalidreference!", // want "invalid ConflictsWith attribute reference" + "invalid-reference", // want "invalid ConflictsWith attribute reference" + "invalid.reference", // want "invalid ConflictsWith attribute reference" + "invalid..reference", // want "invalid ConflictsWith attribute reference" + "invalid.123.reference", // want "invalid ConflictsWith attribute reference" + "invalid.0.sub.reference", // want "invalid ConflictsWith attribute reference" + "invalid.sub.0.reference", // want "invalid ConflictsWith attribute reference" + }, + } + + _ = map[string]*schema.Schema{ + "name": { + ConflictsWith: []string{ + ".invalidreference", // want "invalid ConflictsWith attribute reference" + "InvalidReference", // want "invalid ConflictsWith attribute reference" + "invalidreference!", // want "invalid ConflictsWith attribute reference" + "invalid-reference", // want "invalid ConflictsWith attribute reference" + "invalid.reference", // want "invalid ConflictsWith attribute reference" + "invalid..reference", // want "invalid ConflictsWith attribute reference" + "invalid.123.reference", // want "invalid ConflictsWith attribute reference" + "invalid.0.sub.reference", // want "invalid ConflictsWith attribute reference" + "invalid.sub.0.reference", // want "invalid ConflictsWith attribute reference" + }, + }, + } + + // Passing + + _ = schema.Schema{ + ConflictsWith: []string{ + "validreference", + "valid_reference", + "valid.0.reference", + "valid.0.sub.0.reference", + }, + } + + _ = map[string]*schema.Schema{ + "name": { + ConflictsWith: []string{ + "validreference", + "valid_reference", + "valid.0.reference", + "valid.0.sub.0.reference", + }, + }, + } +} diff --git a/passes/S036/testdata/src/a/outside_package.go b/passes/S036/testdata/src/a/outside_package.go new file mode 100644 index 00000000..670a74a3 --- /dev/null +++ b/passes/S036/testdata/src/a/outside_package.go @@ -0,0 +1,21 @@ +package a + +import ( + "a/schema" +) + +func foutside() { + _ = schema.Schema{ + ConflictsWith: []string{ + ".invalidreference", + }, + } + + _ = map[string]*schema.Schema{ + "name": { + ConflictsWith: []string{ + ".invalidreference", + }, + }, + } +} diff --git a/passes/S036/testdata/src/a/schema/schema.go b/passes/S036/testdata/src/a/schema/schema.go new file mode 100644 index 00000000..86dcde30 --- /dev/null +++ b/passes/S036/testdata/src/a/schema/schema.go @@ -0,0 +1,5 @@ +package schema + +type Schema struct { + ConflictsWith []string +} diff --git a/passes/S036/testdata/src/a/vendor b/passes/S036/testdata/src/a/vendor new file mode 120000 index 00000000..776800d2 --- /dev/null +++ b/passes/S036/testdata/src/a/vendor @@ -0,0 +1 @@ +../../../../../vendor \ No newline at end of file diff --git a/passes/S037/README.md b/passes/S037/README.md new file mode 100644 index 00000000..ff3ed841 --- /dev/null +++ b/passes/S037/README.md @@ -0,0 +1,32 @@ +# S037 + +The S037 analyzer reports cases of Schemas which include `ExactlyOneOf` and have invalid schema attribute references. + +NOTE: This only verifies the syntax of attribute references. The Terraform Plugin SDK can unit test attribute references to verify the references against the full schema. + +## Flagged Code + +```go +&schema.Schema{ + ExactlyOneOf: []string{"config_block_attr.nested_attr"}, +} +``` + +## Passing Code + +```go +&schema.Schema{ + ExactlyOneOf: []string{"config_block_attr.0.nested_attr"}, +} +``` + +## Ignoring Reports + +Singular reports can be ignored by adding the a `//lintignore:S037` Go code comment at the end of the offending line or on the line immediately proceding, e.g. + +```go +//lintignore:S037 +&schema.Schema{ + ExactlyOneOf: []string{"config_block_attr.nested_attr"}, +} +``` diff --git a/passes/S037/S037.go b/passes/S037/S037.go new file mode 100644 index 00000000..aa9ebe5e --- /dev/null +++ b/passes/S037/S037.go @@ -0,0 +1,8 @@ +package S037 + +import ( + "github.com/bflad/tfproviderlint/helper/analysisutils" + "github.com/bflad/tfproviderlint/helper/terraformtype/helper/schema" +) + +var Analyzer = analysisutils.SchemaAttributeReferencesAnalyzer("S037", schema.SchemaFieldExactlyOneOf) diff --git a/passes/S037/S037_test.go b/passes/S037/S037_test.go new file mode 100644 index 00000000..ea120b7b --- /dev/null +++ b/passes/S037/S037_test.go @@ -0,0 +1,12 @@ +package S037 + +import ( + "testing" + + "golang.org/x/tools/go/analysis/analysistest" +) + +func TestS037(t *testing.T) { + testdata := analysistest.TestData() + analysistest.Run(t, testdata, Analyzer, "a") +} diff --git a/passes/S037/testdata/src/a/alias.go b/passes/S037/testdata/src/a/alias.go new file mode 100644 index 00000000..b253460c --- /dev/null +++ b/passes/S037/testdata/src/a/alias.go @@ -0,0 +1,21 @@ +package a + +import ( + s "github.com/hashicorp/terraform-plugin-sdk/helper/schema" +) + +func falias() { + _ = s.Schema{ + ExactlyOneOf: []string{ + ".invalidreference", // want "invalid ExactlyOneOf attribute reference" + }, + } + + _ = map[string]*s.Schema{ + "name": { + ExactlyOneOf: []string{ + ".invalidreference", // want "invalid ExactlyOneOf attribute reference" + }, + }, + } +} diff --git a/passes/S037/testdata/src/a/main.go b/passes/S037/testdata/src/a/main.go new file mode 100644 index 00000000..f080f51c --- /dev/null +++ b/passes/S037/testdata/src/a/main.go @@ -0,0 +1,78 @@ +package a + +import ( + "github.com/hashicorp/terraform-plugin-sdk/helper/schema" +) + +func f() { + // Comment ignored + + //lintignore:S037 + _ = schema.Schema{ + ExactlyOneOf: []string{ + ".invalidreference", + "InvalidReference", + "invalidreference!", + "invalid-reference", + "invalid.reference", + "invalid..reference", + "invalid.123.reference", + "invalid.0.sub.reference", + "invalid.sub.0.reference", + }, + } + + // Failing + + _ = schema.Schema{ + ExactlyOneOf: []string{ + ".invalidreference", // want "invalid ExactlyOneOf attribute reference" + "InvalidReference", // want "invalid ExactlyOneOf attribute reference" + "invalidreference!", // want "invalid ExactlyOneOf attribute reference" + "invalid-reference", // want "invalid ExactlyOneOf attribute reference" + "invalid.reference", // want "invalid ExactlyOneOf attribute reference" + "invalid..reference", // want "invalid ExactlyOneOf attribute reference" + "invalid.123.reference", // want "invalid ExactlyOneOf attribute reference" + "invalid.0.sub.reference", // want "invalid ExactlyOneOf attribute reference" + "invalid.sub.0.reference", // want "invalid ExactlyOneOf attribute reference" + }, + } + + _ = map[string]*schema.Schema{ + "name": { + ExactlyOneOf: []string{ + ".invalidreference", // want "invalid ExactlyOneOf attribute reference" + "InvalidReference", // want "invalid ExactlyOneOf attribute reference" + "invalidreference!", // want "invalid ExactlyOneOf attribute reference" + "invalid-reference", // want "invalid ExactlyOneOf attribute reference" + "invalid.reference", // want "invalid ExactlyOneOf attribute reference" + "invalid..reference", // want "invalid ExactlyOneOf attribute reference" + "invalid.123.reference", // want "invalid ExactlyOneOf attribute reference" + "invalid.0.sub.reference", // want "invalid ExactlyOneOf attribute reference" + "invalid.sub.0.reference", // want "invalid ExactlyOneOf attribute reference" + }, + }, + } + + // Passing + + _ = schema.Schema{ + ExactlyOneOf: []string{ + "validreference", + "valid_reference", + "valid.0.reference", + "valid.0.sub.0.reference", + }, + } + + _ = map[string]*schema.Schema{ + "name": { + ExactlyOneOf: []string{ + "validreference", + "valid_reference", + "valid.0.reference", + "valid.0.sub.0.reference", + }, + }, + } +} diff --git a/passes/S037/testdata/src/a/outside_package.go b/passes/S037/testdata/src/a/outside_package.go new file mode 100644 index 00000000..e80c225b --- /dev/null +++ b/passes/S037/testdata/src/a/outside_package.go @@ -0,0 +1,21 @@ +package a + +import ( + "a/schema" +) + +func foutside() { + _ = schema.Schema{ + ExactlyOneOf: []string{ + ".invalidreference", + }, + } + + _ = map[string]*schema.Schema{ + "name": { + ExactlyOneOf: []string{ + ".invalidreference", + }, + }, + } +} diff --git a/passes/S037/testdata/src/a/schema/schema.go b/passes/S037/testdata/src/a/schema/schema.go new file mode 100644 index 00000000..d61969eb --- /dev/null +++ b/passes/S037/testdata/src/a/schema/schema.go @@ -0,0 +1,5 @@ +package schema + +type Schema struct { + ExactlyOneOf []string +} diff --git a/passes/S037/testdata/src/a/vendor b/passes/S037/testdata/src/a/vendor new file mode 120000 index 00000000..776800d2 --- /dev/null +++ b/passes/S037/testdata/src/a/vendor @@ -0,0 +1 @@ +../../../../../vendor \ No newline at end of file diff --git a/passes/checks.go b/passes/checks.go index 76e26ee4..e42e9570 100644 --- a/passes/checks.go +++ b/passes/checks.go @@ -57,6 +57,9 @@ import ( "github.com/bflad/tfproviderlint/passes/S032" "github.com/bflad/tfproviderlint/passes/S033" "github.com/bflad/tfproviderlint/passes/S034" + "github.com/bflad/tfproviderlint/passes/S035" + "github.com/bflad/tfproviderlint/passes/S036" + "github.com/bflad/tfproviderlint/passes/S037" "github.com/bflad/tfproviderlint/passes/V001" "github.com/bflad/tfproviderlint/passes/V002" "github.com/bflad/tfproviderlint/passes/V003" @@ -128,6 +131,9 @@ var AllChecks = []*analysis.Analyzer{ S032.Analyzer, S033.Analyzer, S034.Analyzer, + S035.Analyzer, + S036.Analyzer, + S037.Analyzer, V001.Analyzer, V002.Analyzer, V003.Analyzer, diff --git a/passes/helper/schema/crudfuncinfo/crudfuncinfo.go b/passes/helper/schema/crudfuncinfo/crudfuncinfo.go index d176266b..9ad00f00 100644 --- a/passes/helper/schema/crudfuncinfo/crudfuncinfo.go +++ b/passes/helper/schema/crudfuncinfo/crudfuncinfo.go @@ -40,11 +40,11 @@ func run(pass *analysis.Pass) (interface{}, error) { return } - if !astutils.IsFieldListType(funcType.Params, 1, astutils.IsFunctionParameterTypeInterface) { + if !astutils.IsFieldListType(funcType.Params, 1, astutils.IsExprTypeInterface) { return } - if !astutils.IsFieldListType(funcType.Results, 0, astutils.IsFunctionParameterTypeError) { + if !astutils.IsFieldListType(funcType.Results, 0, astutils.IsExprTypeError) { return } diff --git a/passes/helper/schema/schemavalidatefuncinfo/schemavalidatefuncinfo.go b/passes/helper/schema/schemavalidatefuncinfo/schemavalidatefuncinfo.go index 92c577e1..5af0f0b6 100644 --- a/passes/helper/schema/schemavalidatefuncinfo/schemavalidatefuncinfo.go +++ b/passes/helper/schema/schemavalidatefuncinfo/schemavalidatefuncinfo.go @@ -36,19 +36,19 @@ func run(pass *analysis.Pass) (interface{}, error) { return } - if !astutils.IsFieldListType(funcType.Params, 0, astutils.IsFunctionParameterTypeInterface) { + if !astutils.IsFieldListType(funcType.Params, 0, astutils.IsExprTypeInterface) { return } - if !astutils.IsFieldListType(funcType.Params, 1, astutils.IsFunctionParameterTypeString) { + if !astutils.IsFieldListType(funcType.Params, 1, astutils.IsExprTypeString) { return } - if !astutils.IsFieldListType(funcType.Results, 0, astutils.IsFunctionParameterTypeArrayString) { + if !astutils.IsFieldListType(funcType.Results, 0, astutils.IsExprTypeArrayString) { return } - if !astutils.IsFieldListType(funcType.Results, 1, astutils.IsFunctionParameterTypeArrayError) { + if !astutils.IsFieldListType(funcType.Results, 1, astutils.IsExprTypeArrayError) { return }