Skip to content

Commit

Permalink
Remove Check method from tflint.RuleSet interface (#258)
Browse files Browse the repository at this point in the history
  • Loading branch information
wata727 authored Jun 3, 2023
1 parent b8f431f commit 30d991e
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 27 deletions.
42 changes: 32 additions & 10 deletions plugin/host2plugin/host2plugin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,23 +87,45 @@ func (r *mockRuleSet) NewRunner(runner tflint.Runner) (tflint.Runner, error) {
return runner, nil
}

func (r *mockRuleSet) Check(runner tflint.Runner) error {
if r.impl.check != nil {
return r.impl.check(runner)
}
return nil
}

func newMockRuleSet(name, version string, impl mockRuleSetImpl) *mockRuleSet {
return &mockRuleSet{
BuiltinRuleSet: tflint.BuiltinRuleSet{
Name: name,
Version: version,
EnabledRules: []tflint.Rule{
&mockRule{check: impl.check},
},
},
impl: impl,
}
}

var _ tflint.Rule = &mockRule{}

type mockRule struct {
tflint.DefaultRule
check func(tflint.Runner) error
}

func (r *mockRule) Check(runner tflint.Runner) error {
if r.check != nil {
return r.check(runner)
}
return nil
}

func (r *mockRule) Name() string {
return "mock_rule"
}

func (r *mockRule) Severity() tflint.Severity {
return tflint.ERROR
}

func (r *mockRule) Enabled() bool {
return true
}

func TestRuleSetName(t *testing.T) {
// default error check helper
neverHappend := func(err error) bool { return err != nil }
Expand Down Expand Up @@ -669,7 +691,7 @@ resource "aws_instance" "foo" {
return err
},
ErrCheck: func(err error) bool {
return err == nil || err.Error() != "unexpected error"
return err == nil || err.Error() != `failed to check "mock_rule" rule: unexpected error`
},
},
{
Expand All @@ -681,7 +703,7 @@ resource "aws_instance" "foo" {
return errors.New("unexpected error")
},
ErrCheck: func(err error) bool {
return err == nil || err.Error() != "unexpected error"
return err == nil || err.Error() != `failed to check "mock_rule" rule: unexpected error`
},
},
{
Expand All @@ -696,7 +718,7 @@ resource "aws_instance" "foo" {
return errors.New(runner.(*mockCustomRunner).Hello())
},
ErrCheck: func(err error) bool {
return err == nil || err.Error() != "Hello from custom runner!"
return err == nil || err.Error() != `failed to check "mock_rule" rule: Hello from custom runner!`
},
},
}
Expand Down
13 changes: 8 additions & 5 deletions plugin/host2plugin/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package host2plugin

import (
"context"
"fmt"

"github.com/hashicorp/go-plugin"
"github.com/terraform-linters/tflint-plugin-sdk/logger"
Expand Down Expand Up @@ -107,8 +108,8 @@ func (s *GRPCServer) ApplyConfig(ctx context.Context, req *proto.ApplyConfig_Req
return &proto.ApplyConfig_Response{}, nil
}

// Check calls its own plugin implementation with an gRPC client that can send
// requests to the host process.
// Check calls plugin rules with a gRPC client that can send requests
// to the host process.
func (s *GRPCServer) Check(ctx context.Context, req *proto.Check_Request) (*proto.Check_Response, error) {
conn, err := s.broker.Dial(req.Runner)
if err != nil {
Expand All @@ -120,9 +121,11 @@ func (s *GRPCServer) Check(ctx context.Context, req *proto.Check_Request) (*prot
if err != nil {
return nil, toproto.Error(codes.FailedPrecondition, err)
}
err = s.impl.Check(runner)
if err != nil {
return nil, toproto.Error(codes.Aborted, err)

for _, rule := range s.impl.BuiltinImpl().EnabledRules {
if err := rule.Check(runner); err != nil {
return nil, toproto.Error(codes.Aborted, fmt.Errorf(`failed to check "%s" rule: %s`, rule.Name(), err))
}
}
return &proto.Check_Response{}, nil
}
4 changes: 2 additions & 2 deletions tflint/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,9 @@ type RuleSet interface {
// Custom rulesets can override this method to inject a custom runner.
NewRunner(Runner) (Runner, error)

// Check is a entrypoint for all inspections.
// BuiltinImpl returns the receiver itself as BuiltinRuleSet.
// This is not supposed to be overridden from custom rulesets.
Check(Runner) error
BuiltinImpl() *BuiltinRuleSet

// All Ruleset must embed the builtin ruleset.
mustEmbedBuiltinRuleSet()
Expand Down
14 changes: 4 additions & 10 deletions tflint/ruleset.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package tflint

import (
"fmt"

"github.com/terraform-linters/tflint-plugin-sdk/hclext"
"github.com/terraform-linters/tflint-plugin-sdk/logger"
)
Expand Down Expand Up @@ -104,14 +102,10 @@ func (r *BuiltinRuleSet) NewRunner(runner Runner) (Runner, error) {
return runner, nil
}

// Check runs inspection for each rule by applying Runner.
func (r *BuiltinRuleSet) Check(runner Runner) error {
for _, rule := range r.EnabledRules {
if err := rule.Check(runner); err != nil {
return fmt.Errorf("Failed to check `%s` rule: %s", rule.Name(), err)
}
}
return nil
// BuiltinImpl returns the receiver itself as BuiltinRuleSet.
// This is not supposed to be overridden from custom rulesets.
func (r *BuiltinRuleSet) BuiltinImpl() *BuiltinRuleSet {
return r
}

func (r *BuiltinRuleSet) mustEmbedBuiltinRuleSet() {}

0 comments on commit 30d991e

Please sign in to comment.