Skip to content

Commit

Permalink
New Checks: S025 through S033 (#89)
Browse files Browse the repository at this point in the history
* New Analyzer: helper/passes/schema/schemainfocomputedonly

Returns all Schema that are Computed: true only. Refactors existing checks to using it.

* New Checks: S025 through S033

Reference: #65

* **New Check:** `S025`: check for `Schema` of only `Computed` enabled with `AtLeastOneOf` configured
* **New Check:** `S026`: check for `Schema` of only `Computed` enabled with `ConflictsWith` configured
* **New Check:** `S027`: check for `Schema` of only `Computed` enabled with `Default` configured
* **New Check:** `S028`: check for `Schema` of only `Computed` enabled with `DefaultFunc` configured
* **New Check:** `S029`: check for `Schema` of only `Computed` enabled with `ExactlyOneOf` configured
* **New Check:** `S030`: check for `Schema` of only `Computed` enabled with `InputDefault` configured
* **New Check:** `S031`: check for `Schema` of only `Computed` enabled with `MaxItems` configured
* **New Check:** `S032`: check for `Schema` of only `Computed` enabled with `MinItems` configured
* **New Check:** `S033`: check for `Schema` of only `Computed` enabled with `StateFunc` configured
  • Loading branch information
bflad authored Feb 3, 2020
1 parent 693d76b commit ddad891
Show file tree
Hide file tree
Showing 89 changed files with 1,779 additions and 23 deletions.
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,15 @@ BREAKING CHANGES
FEATURES

* **New Check:** `S024`: check for `Schema` that should omit `ForceNew` in data source schema attributes
* **New Check:** `S025`: check for `Schema` of only `Computed` enabled with `AtLeastOneOf` configured
* **New Check:** `S026`: check for `Schema` of only `Computed` enabled with `ConflictsWith` configured
* **New Check:** `S027`: check for `Schema` of only `Computed` enabled with `Default` configured
* **New Check:** `S028`: check for `Schema` of only `Computed` enabled with `DefaultFunc` configured
* **New Check:** `S029`: check for `Schema` of only `Computed` enabled with `ExactlyOneOf` configured
* **New Check:** `S030`: check for `Schema` of only `Computed` enabled with `InputDefault` configured
* **New Check:** `S031`: check for `Schema` of only `Computed` enabled with `MaxItems` configured
* **New Check:** `S032`: check for `Schema` of only `Computed` enabled with `MinItems` configured
* **New Check:** `S033`: check for `Schema` of only `Computed` enabled with `StateFunc` configured
* **New Check:** `V002`: check for deprecated `CIDRNetwork` validation function usage
* **New Check:** `V003`: check for deprecated `IPRange` validation function usage
* **New Check:** `V004`: check for deprecated `SingleIP` validation function usage
Expand Down
9 changes: 9 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,15 @@ Standard lint checks are enabled by default in the `tfproviderlint` tool. Opt-in
| [S022](passes/S022/README.md) | check for `Schema` of `TypeMap` with invalid `Elem` of `*schema.Resource` | AST |
| [S023](passes/S023/README.md) | check for `Schema` that should omit `Elem` with incompatible `Type` | AST |
| [S024](passes/S024/README.md) | check for `Schema` that should omit `ForceNew` in data source schema attributes | AST |
| [S025](passes/S025/README.md) | check for `Schema` of `Computed` only with `AtLeastOneOf` configured | AST |
| [S026](passes/S026/README.md) | check for `Schema` of `Computed` only with `ConflictsWith` configured | AST |
| [S027](passes/S027/README.md) | check for `Schema` of `Computed` only with `Default` configured | AST |
| [S028](passes/S028/README.md) | check for `Schema` of `Computed` only with `DefaultFunc` configured | AST |
| [S029](passes/S029/README.md) | check for `Schema` of `Computed` only with `ExactlyOneOf` configured | AST |
| [S030](passes/S030/README.md) | check for `Schema` of `Computed` only with `InputDefault` configured | AST |
| [S031](passes/S031/README.md) | check for `Schema` of `Computed` only with `MaxItems` configured | AST |
| [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 |

### Standard Validation Checks

Expand Down
20 changes: 20 additions & 0 deletions helper/terraformtype/helper/schema/type_schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,10 @@ func NewSchemaInfo(cl *ast.CompositeLit, info *types.Info) *SchemaInfo {
TypesInfo: info,
}

if kvExpr := result.Fields[SchemaFieldAtLeastOneOf]; kvExpr != nil && astutils.ExprValue(kvExpr.Value) != nil {
result.Schema.AtLeastOneOf = []string{}
}

if kvExpr := result.Fields[SchemaFieldComputed]; kvExpr != nil && astutils.ExprBoolValue(kvExpr.Value) != nil {
result.Schema.Computed = *astutils.ExprBoolValue(kvExpr.Value)
}
Expand All @@ -79,10 +83,18 @@ func NewSchemaInfo(cl *ast.CompositeLit, info *types.Info) *SchemaInfo {
result.Schema.Default = func() (interface{}, error) { return nil, nil }
}

if kvExpr := result.Fields[SchemaFieldDefaultFunc]; kvExpr != nil && astutils.ExprValue(kvExpr.Value) != nil {
result.Schema.DefaultFunc = func() (interface{}, error) { return nil, nil }
}

if kvExpr := result.Fields[SchemaFieldDescription]; kvExpr != nil && astutils.ExprStringValue(kvExpr.Value) != nil {
result.Schema.Description = *astutils.ExprStringValue(kvExpr.Value)
}

if kvExpr := result.Fields[SchemaFieldExactlyOneOf]; kvExpr != nil && astutils.ExprValue(kvExpr.Value) != nil {
result.Schema.ExactlyOneOf = []string{}
}

if kvExpr := result.Fields[SchemaFieldDiffSuppressFunc]; kvExpr != nil && astutils.ExprValue(kvExpr.Value) != nil {
result.Schema.DiffSuppressFunc = func(k, old, new string, d *tfschema.ResourceData) bool { return false }
}
Expand All @@ -91,6 +103,10 @@ func NewSchemaInfo(cl *ast.CompositeLit, info *types.Info) *SchemaInfo {
result.Schema.ForceNew = *astutils.ExprBoolValue(kvExpr.Value)
}

if kvExpr := result.Fields[SchemaFieldInputDefault]; kvExpr != nil && astutils.ExprStringValue(kvExpr.Value) != nil {
result.Schema.InputDefault = *astutils.ExprStringValue(kvExpr.Value)
}

if kvExpr := result.Fields[SchemaFieldMaxItems]; kvExpr != nil && astutils.ExprIntValue(kvExpr.Value) != nil {
result.Schema.MaxItems = *astutils.ExprIntValue(kvExpr.Value)
}
Expand All @@ -111,6 +127,10 @@ func NewSchemaInfo(cl *ast.CompositeLit, info *types.Info) *SchemaInfo {
result.Schema.Sensitive = *astutils.ExprBoolValue(kvExpr.Value)
}

if kvExpr := result.Fields[SchemaFieldStateFunc]; kvExpr != nil && astutils.ExprValue(kvExpr.Value) != nil {
result.Schema.StateFunc = func(interface{}) string { return "" }
}

if kvExpr := result.Fields[SchemaFieldValidateFunc]; kvExpr != nil && astutils.ExprValue(kvExpr.Value) != nil {
result.Schema.ValidateFunc = func(interface{}, string) ([]string, []error) { return nil, nil }
}
Expand Down
13 changes: 4 additions & 9 deletions passes/S010/S010.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,10 @@ package S010
import (
"go/ast"

"golang.org/x/tools/go/analysis"

"github.com/bflad/tfproviderlint/helper/terraformtype/helper/schema"
"github.com/bflad/tfproviderlint/passes/commentignore"
"github.com/bflad/tfproviderlint/passes/helper/schema/schemainfo"
"github.com/bflad/tfproviderlint/passes/helper/schema/schemainfocomputedonly"
"golang.org/x/tools/go/analysis"
)

const Doc = `check for Schema with only Computed enabled and ValidateFunc configured
Expand All @@ -23,24 +22,20 @@ var Analyzer = &analysis.Analyzer{
Name: analyzerName,
Doc: Doc,
Requires: []*analysis.Analyzer{
schemainfo.Analyzer,
commentignore.Analyzer,
schemainfocomputedonly.Analyzer,
},
Run: run,
}

func run(pass *analysis.Pass) (interface{}, error) {
ignorer := pass.ResultOf[commentignore.Analyzer].(*commentignore.Ignorer)
schemaInfos := pass.ResultOf[schemainfo.Analyzer].([]*schema.SchemaInfo)
schemaInfos := pass.ResultOf[schemainfocomputedonly.Analyzer].([]*schema.SchemaInfo)
for _, schemaInfo := range schemaInfos {
if ignorer.ShouldIgnore(analyzerName, schemaInfo.AstCompositeLit) {
continue
}

if !schemaInfo.Schema.Computed || schemaInfo.Schema.Optional || schemaInfo.Schema.Required {
continue
}

if schemaInfo.Schema.ValidateFunc == nil {
continue
}
Expand Down
13 changes: 4 additions & 9 deletions passes/S011/S011.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,10 @@ package S011
import (
"go/ast"

"golang.org/x/tools/go/analysis"

"github.com/bflad/tfproviderlint/helper/terraformtype/helper/schema"
"github.com/bflad/tfproviderlint/passes/commentignore"
"github.com/bflad/tfproviderlint/passes/helper/schema/schemainfo"
"github.com/bflad/tfproviderlint/passes/helper/schema/schemainfocomputedonly"
"golang.org/x/tools/go/analysis"
)

const Doc = `check for Schema with only Computed enabled and DiffSuppressFunc configured
Expand All @@ -23,24 +22,20 @@ var Analyzer = &analysis.Analyzer{
Name: analyzerName,
Doc: Doc,
Requires: []*analysis.Analyzer{
schemainfo.Analyzer,
commentignore.Analyzer,
schemainfocomputedonly.Analyzer,
},
Run: run,
}

func run(pass *analysis.Pass) (interface{}, error) {
ignorer := pass.ResultOf[commentignore.Analyzer].(*commentignore.Ignorer)
schemaInfos := pass.ResultOf[schemainfo.Analyzer].([]*schema.SchemaInfo)
schemaInfos := pass.ResultOf[schemainfocomputedonly.Analyzer].([]*schema.SchemaInfo)
for _, schemaInfo := range schemaInfos {
if ignorer.ShouldIgnore(analyzerName, schemaInfo.AstCompositeLit) {
continue
}

if !schemaInfo.Schema.Computed || schemaInfo.Schema.Optional || schemaInfo.Schema.Required {
continue
}

if schemaInfo.Schema.DiffSuppressFunc == nil {
continue
}
Expand Down
9 changes: 4 additions & 5 deletions passes/S020/S020.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,10 @@ package S020
import (
"go/ast"

"golang.org/x/tools/go/analysis"

"github.com/bflad/tfproviderlint/helper/terraformtype/helper/schema"
"github.com/bflad/tfproviderlint/passes/commentignore"
"github.com/bflad/tfproviderlint/passes/helper/schema/schemainfo"
"github.com/bflad/tfproviderlint/passes/helper/schema/schemainfocomputedonly"
"golang.org/x/tools/go/analysis"
)

const Doc = `check for Schema with only Computed enabled and ForceNew enabled
Expand All @@ -23,15 +22,15 @@ var Analyzer = &analysis.Analyzer{
Name: analyzerName,
Doc: Doc,
Requires: []*analysis.Analyzer{
schemainfo.Analyzer,
commentignore.Analyzer,
schemainfocomputedonly.Analyzer,
},
Run: run,
}

func run(pass *analysis.Pass) (interface{}, error) {
ignorer := pass.ResultOf[commentignore.Analyzer].(*commentignore.Ignorer)
schemaInfos := pass.ResultOf[schemainfo.Analyzer].([]*schema.SchemaInfo)
schemaInfos := pass.ResultOf[schemainfocomputedonly.Analyzer].([]*schema.SchemaInfo)
for _, schemaInfo := range schemaInfos {
if ignorer.ShouldIgnore(analyzerName, schemaInfo.AstCompositeLit) {
continue
Expand Down
33 changes: 33 additions & 0 deletions passes/S025/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# S025

The S025 analyzer reports cases of schemas which enables only `Computed`
and configures `AtLeastOneOf`, which is not valid.

## Flagged Code

```go
&schema.Schema{
AtLeastOneOf: []string{"example"},
Computed: true,
}
```

## Passing Code

```go
&schema.Schema{
Computed: true,
}
```

## Ignoring Reports

Singular reports can be ignored by adding the a `//lintignore:S025` Go code comment at the end of the offending line or on the line immediately proceding, e.g.

```go
//lintignore:S025
&schema.Schema{
AtLeastOneOf: []string{"example"},
Computed: true,
}
```
50 changes: 50 additions & 0 deletions passes/S025/S025.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package S025

import (
"go/ast"

"github.com/bflad/tfproviderlint/helper/terraformtype/helper/schema"
"github.com/bflad/tfproviderlint/passes/commentignore"
"github.com/bflad/tfproviderlint/passes/helper/schema/schemainfocomputedonly"
"golang.org/x/tools/go/analysis"
)

const Doc = `check for Schema with only Computed enabled and AtLeastOneOf configured
The S025 analyzer reports cases of schemas which only enables Computed
and configures AtLeastOneOf, which is not valid.`

const analyzerName = "S025"

var Analyzer = &analysis.Analyzer{
Name: analyzerName,
Doc: Doc,
Requires: []*analysis.Analyzer{
commentignore.Analyzer,
schemainfocomputedonly.Analyzer,
},
Run: run,
}

func run(pass *analysis.Pass) (interface{}, error) {
ignorer := pass.ResultOf[commentignore.Analyzer].(*commentignore.Ignorer)
schemaInfos := pass.ResultOf[schemainfocomputedonly.Analyzer].([]*schema.SchemaInfo)
for _, schemaInfo := range schemaInfos {
if ignorer.ShouldIgnore(analyzerName, schemaInfo.AstCompositeLit) {
continue
}

if schemaInfo.Schema.AtLeastOneOf == nil {
continue
}

switch t := schemaInfo.AstCompositeLit.Type.(type) {
default:
pass.Reportf(schemaInfo.AstCompositeLit.Lbrace, "%s: schema should not only enable Computed and configure AtLeastOneOf", analyzerName)
case *ast.SelectorExpr:
pass.Reportf(t.Sel.Pos(), "%s: schema should not only enable Computed and configure AtLeastOneOf", analyzerName)
}
}

return nil, nil
}
13 changes: 13 additions & 0 deletions passes/S025/S025_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package S025_test

import (
"testing"

"github.com/bflad/tfproviderlint/passes/S025"
"golang.org/x/tools/go/analysis/analysistest"
)

func TestS025(t *testing.T) {
testdata := analysistest.TestData()
analysistest.Run(t, testdata, S025.Analyzer, "a")
}
19 changes: 19 additions & 0 deletions passes/S025/testdata/src/a/alias.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package a

import (
s "github.com/hashicorp/terraform-plugin-sdk/helper/schema"
)

func falias() {
_ = s.Schema{ // want "schema should not only enable Computed and configure AtLeastOneOf"
AtLeastOneOf: []string{"test"},
Computed: true,
}

_ = map[string]*s.Schema{
"name": { // want "schema should not only enable Computed and configure AtLeastOneOf"
AtLeastOneOf: []string{"test"},
Computed: true,
},
}
}
13 changes: 13 additions & 0 deletions passes/S025/testdata/src/a/comment_ignore.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package a

import (
"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
)

func fcommentignore() {
//lintignore:S025
_ = schema.Schema{
AtLeastOneOf: []string{"test"},
Computed: true,
}
}
33 changes: 33 additions & 0 deletions passes/S025/testdata/src/a/main.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package a

import (
"github.com/hashicorp/terraform-plugin-sdk/helper/schema"
)

func f() {
_ = schema.Schema{ // want "schema should not only enable Computed and configure AtLeastOneOf"
AtLeastOneOf: []string{"test"},
Computed: true,
}

_ = schema.Schema{
AtLeastOneOf: nil,
Computed: true,
}

_ = schema.Schema{
Computed: true,
}

_ = schema.Schema{
AtLeastOneOf: []string{"test"},
Optional: true,
}

_ = map[string]*schema.Schema{
"name": { // want "schema should not only enable Computed and configure AtLeastOneOf"
AtLeastOneOf: []string{"test"},
Computed: true,
},
}
}
19 changes: 19 additions & 0 deletions passes/S025/testdata/src/a/outside_package.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package a

import (
"a/schema"
)

func foutside() {
_ = schema.Schema{
AtLeastOneOf: []string{"test"},
Computed: true,
}

_ = map[string]*schema.Schema{
"name": {
AtLeastOneOf: []string{"test"},
Computed: true,
},
}
}
6 changes: 6 additions & 0 deletions passes/S025/testdata/src/a/schema/schema.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package schema

type Schema struct {
AtLeastOneOf []string
Computed bool
}
1 change: 1 addition & 0 deletions passes/S025/testdata/src/a/vendor
Loading

0 comments on commit ddad891

Please sign in to comment.