Merge pull request #38 from terraform-linters/revise_package_structure

Revise package structure

Author: wata727

helper/doc.go

@@ -0,0 +1,9 @@
+// Package helper contains implementations for plugin testing.
+//
+// You can test the implemented rules using the mock Runner that is not
+// an RPC client. It is similar to TFLint's Runner, but is implemented
+// from scratch to avoid Terraform dependencies.
+//
+// Some implementations of the mock Runner have been simplified. As a result,
+// note that some features may not behave exactly as they should.
+package helper

helper/issue.go

@@ -6,12 +6,12 @@ import (
 )
 
 // Issue is a stub that has the same structure as the actually used issue object.
-// This is only used for testing, as the Runner client doesn't depend on the actual Issue structure.
+// This is only used for testing, as the mock Runner doesn't depend on the actual Issue structure.
 type Issue struct {
 	Rule    tflint.Rule
 	Message string
 	Range   hcl.Range
 }
 
-// Issues is a list of Issue
+// Issues is a list of Issue.
 type Issues []*Issue

helper/runner.go

@@ -8,16 +8,13 @@ import (
 	"github.com/zclconf/go-cty/cty/gocty"
 )
 
-// Runner is a pseudo Runner client provided for plugin testing.
-// Actually, it is provided as an RPC client, but for the sake of simplicity,
-// only the methods that satisfy the minimum required Runner interface are implemented.
-// Specifically, there are restrictions on evaluation, annotation comments, module inspection, and so on.
+// Runner is a mock that satisfies the Runner interface for plugin testing.
 type Runner struct {
 	Files  map[string]*hcl.File
 	Issues Issues
 }
 
-// WalkResourceAttributes searches for resources and passes the appropriate attributes to the walker function
+// WalkResourceAttributes visits all specified attributes from Files.
 func (r *Runner) WalkResourceAttributes(resourceType, attributeName string, walker func(*hcl.Attribute) error) error {
 	for _, file := range r.Files {
 		resources, _, diags := file.Body.PartialContent(&hcl.BodySchema{
@@ -60,7 +57,7 @@ func (r *Runner) WalkResourceAttributes(resourceType, attributeName string, walk
 	return nil
 }
 
-// WalkResources searches for resources with a specific type and passes to the walker function
+// WalkResources visits all specified resources from Files.
 func (r *Runner) WalkResources(resourceType string, walker func(*terraform.Resource) error) error {
 	for _, file := range r.Files {
 		resources, _, diags := file.Body.PartialContent(&hcl.BodySchema{
@@ -105,7 +102,7 @@ func (r *Runner) EvaluateExpr(expr hcl.Expression, ret interface{}) error {
 	return gocty.FromCtyValue(val, ret)
 }
 
-// EmitIssue adds an issue into the self
+// EmitIssue adds an issue to the runner itself.
 func (r *Runner) EmitIssue(rule tflint.Rule, message string, location hcl.Range, meta tflint.Metadata) error {
 	r.Issues = append(r.Issues, &Issue{
 		Rule:    rule,
@@ -115,7 +112,7 @@ func (r *Runner) EmitIssue(rule tflint.Rule, message string, location hcl.Range,
 	return nil
 }
 
-// EnsureNoError is a method that simply run a function if there is no error
+// EnsureNoError is a method that simply runs a function if there is no error.
 func (r *Runner) EnsureNoError(err error, proc func() error) error {
 	if err == nil {
 		return proc()

helper/testing.go

@@ -11,7 +11,8 @@ import (
 	"github.com/terraform-linters/tflint-plugin-sdk/tflint"
 )
 
-// TestRunner returns a pseudo Runner for testing
+// TestRunner returns a mock Runner for testing.
+// You can pass the map of file names and their contents in the second argument.
 func TestRunner(t *testing.T, files map[string]string) *Runner {
 	runner := &Runner{Files: map[string]*hcl.File{}, Issues: Issues{}}
 	parser := hclparse.NewParser()
@@ -27,7 +28,7 @@ func TestRunner(t *testing.T, files map[string]string) *Runner {
 	return runner
 }
 
-// AssertIssues is an assertion helper for comparing issues
+// AssertIssues is an assertion helper for comparing issues.
 func AssertIssues(t *testing.T, expected Issues, actual Issues) {
 	opts := []cmp.Option{
 		// Byte field will be ignored because it's not important in tests such as positions
@@ -39,7 +40,7 @@ func AssertIssues(t *testing.T, expected Issues, actual Issues) {
 	}
 }
 
-// AssertIssuesWithoutRange is an assertion helper for comparing issues
+// AssertIssuesWithoutRange is an assertion helper for comparing issues except for range.
 func AssertIssuesWithoutRange(t *testing.T, expected Issues, actual Issues) {
 	opts := []cmp.Option{
 		cmpopts.IgnoreFields(Issue{}, "Range"),

plugin/client.go

@@ -8,20 +8,21 @@ import (
 	"github.com/hashicorp/go-hclog"
 	plugin "github.com/hashicorp/go-plugin"
 	"github.com/terraform-linters/tflint-plugin-sdk/tflint"
+	tfserver "github.com/terraform-linters/tflint-plugin-sdk/tflint/server"
 )
 
-// Client is an RPC client for use by the host
+// Client is an RPC client for the host.
 type Client struct {
 	rpcClient *rpc.Client
 	broker    *plugin.MuxBroker
 }
 
-// ClientOpts is an option for initializing the RPC client
+// ClientOpts is an option for initializing a Client.
 type ClientOpts struct {
 	Cmd *exec.Cmd
 }
 
-// NewClient is a wrapper of plugin.NewClient
+// NewClient is a wrapper of plugin.NewClient.
 func NewClient(opts *ClientOpts) *plugin.Client {
 	return plugin.NewClient(&plugin.ClientConfig{
 		HandshakeConfig: handshakeConfig,
@@ -37,35 +38,37 @@ func NewClient(opts *ClientOpts) *plugin.Client {
 	})
 }
 
-// RuleSetName queries the RPC server for RuleSetName
+// RuleSetName calls the server-side RuleSetName method and returns its name.
 func (c *Client) RuleSetName() (string, error) {
 	var resp string
 	err := c.rpcClient.Call("Plugin.RuleSetName", new(interface{}), &resp)
 	return resp, err
 }
 
-// RuleSetVersion queries the RPC server for RuleSetVersion
+// RuleSetVersion calls the server-side RuleSetVersion method and returns its version.
 func (c *Client) RuleSetVersion() (string, error) {
 	var resp string
 	err := c.rpcClient.Call("Plugin.RuleSetVersion", new(interface{}), &resp)
 	return resp, err
 }
 
-// RuleNames queries the RPC server for RuleNames
+// RuleNames calls the server-side RuleNames method and returns the list of names.
 func (c *Client) RuleNames() ([]string, error) {
 	var resp []string
 	err := c.rpcClient.Call("Plugin.RuleNames", new(interface{}), &resp)
 	return resp, err
 }
 
-// ApplyConfig queries the RPC server for ApplyConfig
+// ApplyConfig calls the server-side ApplyConfig method.
 func (c *Client) ApplyConfig(config *tflint.Config) error {
 	return c.rpcClient.Call("Plugin.ApplyConfig", config, new(interface{}))
 }
 
-// Check queries the RPC server for Check
-// For bi-directional communication, you can pass a server that accepts Runner's queries
-func (c *Client) Check(server tflint.Server) error {
+// Check calls the server-side Check method.
+// At the same time, it starts the server to respond to requests from the plugin side.
+// Note that this server (tfserver.Server) serves clients that satisfy the Runner interface
+// and is different from the server (plugin.Server) that provides the plugin system.
+func (c *Client) Check(server tfserver.Server) error {
 	brokerID := c.broker.NextId()
 	go c.broker.AcceptAndServe(brokerID, server)
 

plugin/doc.go

@@ -0,0 +1,11 @@
+// Package plugin contains the implementations needed to make
+// the built binary act as a plugin.
+//
+// A plugin is implemented as an RPC server and the host acts
+// as the client, sending analysis requests to the plugin.
+// Note that the server-client relationship here is the opposite of
+// the communication that takes place during the checking phase.
+//
+// Implementation details are hidden in go-plugin. This package is
+// essentially a wrapper for go-plugin.
+package plugin

plugin/plugin.go

@@ -8,30 +8,28 @@ import (
 	"github.com/terraform-linters/tflint-plugin-sdk/tflint"
 )
 
-// handShakeConfig is used for UX. ProcotolVersion will be updated by incompatible changes
+// handShakeConfig is used for UX. ProcotolVersion will be updated by incompatible changes.
 var handshakeConfig = plugin.HandshakeConfig{
 	ProtocolVersion:  1,
 	MagicCookieKey:   "TFLINT_RULESET_PLUGIN",
 	MagicCookieValue: "5adSn1bX8nrDfgBqiAqqEkC6OE1h3iD8SqbMc5UUONx8x3xCF0KlPDsBRNDjoYDP",
 }
 
-// RuleSetPlugin is a wrapper to satisfy the interface of go-plugin
+// RuleSetPlugin is a wrapper to satisfy the interface of go-plugin.
 type RuleSetPlugin struct {
 	impl tflint.RuleSet
 }
 
-// Server returns an RPC server acting as a plugin
+// Server returns an RPC server acting as a plugin.
 func (p *RuleSetPlugin) Server(b *plugin.MuxBroker) (interface{}, error) {
 	return &Server{impl: p.impl, broker: b}, nil
 }
 
-// Client returns an RPC client for use by the host
+// Client returns an RPC client for the host.
 func (RuleSetPlugin) Client(b *plugin.MuxBroker, c *rpc.Client) (interface{}, error) {
 	return &Client{rpcClient: c, broker: b}, nil
 }
 
-// In order to communicate the interface correctly with RPC,
-// the type of the related structure is registered in gob at the initial time.
 func init() {
 	gob.Register(tflint.Error{})
 }

plugin/server.go

@@ -3,21 +3,22 @@ package plugin
 import (
 	plugin "github.com/hashicorp/go-plugin"
 	"github.com/terraform-linters/tflint-plugin-sdk/tflint"
+	tfclient "github.com/terraform-linters/tflint-plugin-sdk/tflint/client"
 )
 
-// Server is an RPC server acting as a plugin
+// Server is an RPC server acting as a plugin.
 type Server struct {
 	impl   tflint.RuleSet
 	broker *plugin.MuxBroker
 }
 
-// ServeOpts is an option for serving a plugin
-// Each plugin can pass a RuleSet that represents its own functionality
+// ServeOpts is an option for serving a plugin.
+// Each plugin can pass a RuleSet that represents its own functionality.
 type ServeOpts struct {
 	RuleSet tflint.RuleSet
 }
 
-// Serve is a wrapper of plugin.Serve. This is entrypoint of all plugins
+// Serve is a wrapper of plugin.Serve. This is entrypoint of all plugins.
 func Serve(opts *ServeOpts) {
 	plugin.Serve(&plugin.ServeConfig{
 		HandshakeConfig: handshakeConfig,
@@ -27,36 +28,37 @@ func Serve(opts *ServeOpts) {
 	})
 }
 
-// RuleSetName replies its own the result of RuleSetName
+// RuleSetName returns the name of the plugin.
 func (s *Server) RuleSetName(args interface{}, resp *string) error {
 	*resp = s.impl.RuleSetName()
 	return nil
 }
 
-// RuleSetVersion replies its own the result of RuleSetVersion
+// RuleSetVersion returns the version of the plugin.
 func (s *Server) RuleSetVersion(args interface{}, resp *string) error {
 	*resp = s.impl.RuleSetVersion()
 	return nil
 }
 
-// RuleNames replies its own the result of RuleNames
+// RuleNames returns the list of rule names provided by the plugin.
 func (s *Server) RuleNames(args interface{}, resp *[]string) error {
 	*resp = s.impl.RuleNames()
 	return nil
 }
 
-// ApplyConfig applies the passed config to its own plugin implementation
+// ApplyConfig applies the passed config to its own plugin implementation.
 func (s *Server) ApplyConfig(config *tflint.Config, resp *interface{}) error {
 	s.impl.ApplyConfig(config)
 	return nil
 }
 
-// Check initializes an RPC client that can query to the host process and pass it to the Check method
+// Check calls its own plugin implementation with an RPC client that can send
+// requests to the host process.
 func (s *Server) Check(brokerID uint32, resp *interface{}) error {
 	conn, err := s.broker.Dial(brokerID)
 	if err != nil {
 		return err
 	}
 
-	return s.impl.Check(tflint.NewClient(conn))
+	return s.impl.Check(tfclient.NewClient(conn))
 }