Skip to content

Commit 30d991e

Browse files
authored
Remove Check method from tflint.RuleSet interface (#258)
1 parent b8f431f commit 30d991e

File tree

4 files changed

+46
-27
lines changed

4 files changed

+46
-27
lines changed

plugin/host2plugin/host2plugin_test.go

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -87,23 +87,45 @@ func (r *mockRuleSet) NewRunner(runner tflint.Runner) (tflint.Runner, error) {
8787
return runner, nil
8888
}
8989

90-
func (r *mockRuleSet) Check(runner tflint.Runner) error {
91-
if r.impl.check != nil {
92-
return r.impl.check(runner)
93-
}
94-
return nil
95-
}
96-
9790
func newMockRuleSet(name, version string, impl mockRuleSetImpl) *mockRuleSet {
9891
return &mockRuleSet{
9992
BuiltinRuleSet: tflint.BuiltinRuleSet{
10093
Name: name,
10194
Version: version,
95+
EnabledRules: []tflint.Rule{
96+
&mockRule{check: impl.check},
97+
},
10298
},
10399
impl: impl,
104100
}
105101
}
106102

103+
var _ tflint.Rule = &mockRule{}
104+
105+
type mockRule struct {
106+
tflint.DefaultRule
107+
check func(tflint.Runner) error
108+
}
109+
110+
func (r *mockRule) Check(runner tflint.Runner) error {
111+
if r.check != nil {
112+
return r.check(runner)
113+
}
114+
return nil
115+
}
116+
117+
func (r *mockRule) Name() string {
118+
return "mock_rule"
119+
}
120+
121+
func (r *mockRule) Severity() tflint.Severity {
122+
return tflint.ERROR
123+
}
124+
125+
func (r *mockRule) Enabled() bool {
126+
return true
127+
}
128+
107129
func TestRuleSetName(t *testing.T) {
108130
// default error check helper
109131
neverHappend := func(err error) bool { return err != nil }
@@ -669,7 +691,7 @@ resource "aws_instance" "foo" {
669691
return err
670692
},
671693
ErrCheck: func(err error) bool {
672-
return err == nil || err.Error() != "unexpected error"
694+
return err == nil || err.Error() != `failed to check "mock_rule" rule: unexpected error`
673695
},
674696
},
675697
{
@@ -681,7 +703,7 @@ resource "aws_instance" "foo" {
681703
return errors.New("unexpected error")
682704
},
683705
ErrCheck: func(err error) bool {
684-
return err == nil || err.Error() != "unexpected error"
706+
return err == nil || err.Error() != `failed to check "mock_rule" rule: unexpected error`
685707
},
686708
},
687709
{
@@ -696,7 +718,7 @@ resource "aws_instance" "foo" {
696718
return errors.New(runner.(*mockCustomRunner).Hello())
697719
},
698720
ErrCheck: func(err error) bool {
699-
return err == nil || err.Error() != "Hello from custom runner!"
721+
return err == nil || err.Error() != `failed to check "mock_rule" rule: Hello from custom runner!`
700722
},
701723
},
702724
}

plugin/host2plugin/server.go

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package host2plugin
22

33
import (
44
"context"
5+
"fmt"
56

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

110-
// Check calls its own plugin implementation with an gRPC client that can send
111-
// requests to the host process.
111+
// Check calls plugin rules with a gRPC client that can send requests
112+
// to the host process.
112113
func (s *GRPCServer) Check(ctx context.Context, req *proto.Check_Request) (*proto.Check_Response, error) {
113114
conn, err := s.broker.Dial(req.Runner)
114115
if err != nil {
@@ -120,9 +121,11 @@ func (s *GRPCServer) Check(ctx context.Context, req *proto.Check_Request) (*prot
120121
if err != nil {
121122
return nil, toproto.Error(codes.FailedPrecondition, err)
122123
}
123-
err = s.impl.Check(runner)
124-
if err != nil {
125-
return nil, toproto.Error(codes.Aborted, err)
124+
125+
for _, rule := range s.impl.BuiltinImpl().EnabledRules {
126+
if err := rule.Check(runner); err != nil {
127+
return nil, toproto.Error(codes.Aborted, fmt.Errorf(`failed to check "%s" rule: %s`, rule.Name(), err))
128+
}
126129
}
127130
return &proto.Check_Response{}, nil
128131
}

tflint/interface.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,9 @@ type RuleSet interface {
6868
// Custom rulesets can override this method to inject a custom runner.
6969
NewRunner(Runner) (Runner, error)
7070

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

7575
// All Ruleset must embed the builtin ruleset.
7676
mustEmbedBuiltinRuleSet()

tflint/ruleset.go

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
package tflint
22

33
import (
4-
"fmt"
5-
64
"github.com/terraform-linters/tflint-plugin-sdk/hclext"
75
"github.com/terraform-linters/tflint-plugin-sdk/logger"
86
)
@@ -104,14 +102,10 @@ func (r *BuiltinRuleSet) NewRunner(runner Runner) (Runner, error) {
104102
return runner, nil
105103
}
106104

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

117111
func (r *BuiltinRuleSet) mustEmbedBuiltinRuleSet() {}

0 commit comments

Comments
 (0)