MM-17888 Check plugin Helpers minimum server version comments (#12663)

Makefile

@@ -154,7 +154,7 @@ govet: ## Runs govet against all packages.
 	env GO111MODULE=off $(GO) get golang.org/x/tools/go/analysis/passes/shadow/cmd/shadow
 	$(GO) vet $(GOFLAGS) $(ALL_PACKAGES) || exit 1
 	$(GO) vet -vettool=$(GOPATH)/bin/shadow $(GOFLAGS) $(ALL_PACKAGES) || exit 1
-	$(GO) run $(GOFLAGS) plugin/checker/main.go
+	$(GO) run $(GOFLAGS) ./plugin/checker
 
 gofmt: ## Runs gofmt against all packages.
 	@echo Running GOFMT

plugin/checker/check_api.go

@@ -0,0 +1,50 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See License.txt for license information.
+
+package main
+
+import (
+	"fmt"
+	"go/ast"
+	"go/token"
+
+	"github.com/mattermost/mattermost-server/plugin/checker/internal/asthelpers"
+	"github.com/mattermost/mattermost-server/plugin/checker/internal/version"
+)
+
+func checkAPIVersionComments(pkgPath string) (result, error) {
+	pkg, err := asthelpers.GetPackage(pkgPath)
+	if err != nil {
+		return result{}, err
+	}
+
+	apiInterface, err := asthelpers.FindInterface("API", pkg.Syntax)
+	if err != nil {
+		return result{}, err
+	}
+
+	invalidMethods := findInvalidMethods(apiInterface.Methods.List)
+	return result{Errors: renderErrors(pkg.Fset, invalidMethods)}, nil
+}
+
+func findInvalidMethods(methods []*ast.Field) []*ast.Field {
+	var invalid []*ast.Field
+	for _, m := range methods {
+		if !hasValidMinimumVersionComment(m.Doc.Text()) {
+			invalid = append(invalid, m)
+		}
+	}
+	return invalid
+}
+
+func hasValidMinimumVersionComment(s string) bool {
+	return version.ExtractMinimumVersionFromComment(s) != ""
+}
+
+func renderErrors(fset *token.FileSet, methods []*ast.Field) []string {
+	var out []string
+	for _, m := range methods {
+		out = append(out, renderWithFilePosition(fset, m.Pos(), fmt.Sprintf("missing a minimum server version comment on method %s", m.Names[0].Name)))
+	}
+	return out
+}

plugin/checker/check_api_test.go

@@ -10,29 +10,32 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func TestRunCheck(t *testing.T) {
+func TestCheckAPIVersionComments(t *testing.T) {
 	testCases := []struct {
 		name, pkgPath, err string
+		expected           result
 	}{
 		{
 			name:    "valid comments",
-			pkgPath: "github.com/mattermost/mattermost-server/plugin/checker/test/valid",
+			pkgPath: "github.com/mattermost/mattermost-server/plugin/checker/internal/test/valid",
 			err:     "",
 		},
 		{
 			name:    "invalid comments",
-			pkgPath: "github.com/mattermost/mattermost-server/plugin/checker/test/invalid",
-			err:     "test/invalid/invalid.go:15:2: missing a minimum server version comment\n",
+			pkgPath: "github.com/mattermost/mattermost-server/plugin/checker/internal/test/invalid",
+			expected: result{
+				Errors: []string{"internal/test/invalid/invalid.go:15:2: missing a minimum server version comment on method InvalidMethod"},
+			},
 		},
 		{
 			name:    "missing API interface",
-			pkgPath: "github.com/mattermost/mattermost-server/plugin/checker/test/missing",
-			err:     "could not find API interface in package github.com/mattermost/mattermost-server/plugin/checker/test/missing",
+			pkgPath: "github.com/mattermost/mattermost-server/plugin/checker/internal/test/missing",
+			err:     "could not find API interface",
 		},
 		{
 			name:    "non-existent package path",
-			pkgPath: "github.com/mattermost/mattermost-server/plugin/checker/test/does_not_exist",
-			err:     "could not find API interface in package github.com/mattermost/mattermost-server/plugin/checker/test/does_not_exist",
+			pkgPath: "github.com/mattermost/mattermost-server/plugin/checker/internal/test/does_not_exist",
+			err:     "could not find API interface",
 		},
 	}
 
@@ -43,7 +46,8 @@ func TestRunCheck(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			err := runCheck(tc.pkgPath)
+			res, err := checkAPIVersionComments(tc.pkgPath)
+			assert.Equal(t, res, tc.expected)
 
 			if tc.err != "" {
 				assert.EqualError(t, err, tc.err)

plugin/checker/check_helpers.go

@@ -0,0 +1,137 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See License.txt for license information.
+
+package main
+
+import (
+	"fmt"
+	"go/ast"
+	"go/token"
+	"go/types"
+
+	"github.com/mattermost/mattermost-server/plugin/checker/internal/asthelpers"
+	"github.com/mattermost/mattermost-server/plugin/checker/internal/version"
+
+	"github.com/pkg/errors"
+)
+
+func checkHelpersVersionComments(pkgPath string) (result, error) {
+	pkg, err := asthelpers.GetPackage(pkgPath)
+	if err != nil {
+		return result{}, err
+	}
+
+	api, apiIdent, err := asthelpers.FindInterfaceWithIdent("API", pkg.Syntax)
+	if err != nil {
+		return result{}, err
+	}
+
+	apiObj := pkg.TypesInfo.ObjectOf(apiIdent)
+	if apiObj == nil {
+		return result{}, errors.New("could not find type object for API interface")
+	}
+
+	helpers, err := asthelpers.FindInterface("Helpers", pkg.Syntax)
+	if err != nil {
+		return result{}, err
+	}
+
+	apiVersions := mapMinimumVersionsByMethodName(api.Methods.List)
+
+	helpersPositions := mapPositionsByMethodName(helpers.Methods.List)
+	helpersVersions := mapMinimumVersionsByMethodName(helpers.Methods.List)
+
+	implMethods := asthelpers.FindReceiverMethods("HelpersImpl", pkg.Syntax)
+	implVersions := mapEffectiveVersionByMethod(pkg.TypesInfo, apiObj.Type(), apiVersions, implMethods)
+
+	return validateMethods(pkg.Fset, helpersPositions, helpersVersions, implVersions), nil
+}
+
+func validateMethods(
+	fset *token.FileSet,
+	helpersPositions map[string]token.Pos,
+	helpersVersions map[string]version.V,
+	implVersions map[string]version.V,
+) result {
+	var res result
+
+	for name, helperVer := range helpersVersions {
+		pos := helpersPositions[name]
+
+		implVer, ok := implVersions[name]
+		if !ok {
+			res.Errors = append(res.Errors, renderWithFilePosition(
+				fset,
+				pos,
+				fmt.Sprintf("missing implementation for method %s", name)),
+			)
+			continue
+		}
+
+		if helperVer == "" {
+			res.Errors = append(res.Errors, renderWithFilePosition(
+				fset,
+				pos,
+				fmt.Sprintf("missing a minimum server version comment on method %s", name)),
+			)
+			continue
+		}
+
+		if helperVer == implVer {
+			continue
+		}
+
+		if helperVer.LessThan(implVer) {
+			res.Errors = append(res.Errors, renderWithFilePosition(
+				fset,
+				pos,
+				fmt.Sprintf("documented minimum server version too low on method %s", name)),
+			)
+		} else {
+			res.Warnings = append(res.Warnings, renderWithFilePosition(
+				fset,
+				pos,
+				fmt.Sprintf("documented minimum server version too high on method %s", name)),
+			)
+		}
+	}
+
+	return res
+}
+
+func mapEffectiveVersionByMethod(info *types.Info, apiType types.Type, versions map[string]version.V, methods []*ast.FuncDecl) map[string]version.V {
+	effectiveVersions := map[string]version.V{}
+	for _, m := range methods {
+		apiMethodsCalled := asthelpers.FindMethodsCalledOnType(info, apiType, m)
+		effectiveVersions[m.Name.Name] = getEffectiveMinimumVersion(versions, apiMethodsCalled)
+	}
+	return effectiveVersions
+}
+
+func mapMinimumVersionsByMethodName(methods []*ast.Field) map[string]version.V {
+	versions := map[string]version.V{}
+	for _, m := range methods {
+		versions[m.Names[0].Name] = version.V(version.ExtractMinimumVersionFromComment(m.Doc.Text()))
+	}
+	return versions
+}
+
+func mapPositionsByMethodName(methods []*ast.Field) map[string]token.Pos {
+	pos := map[string]token.Pos{}
+	for _, m := range methods {
+		pos[m.Names[0].Name] = m.Pos()
+	}
+	return pos
+}
+
+func getEffectiveMinimumVersion(info map[string]version.V, methods []string) version.V {
+	var highest version.V
+	for _, m := range methods {
+		if current, ok := info[m]; ok {
+			if current.GreaterThanOrEqualTo(highest) {
+				highest = current
+			}
+		}
+	}
+	return highest
+}

plugin/checker/check_helpers_test.go

@@ -0,0 +1,48 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See License.txt for license information.
+
+package main
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestCheckHelpersVersionComments(t *testing.T) {
+	testCases := []struct {
+		name, pkgPath string
+		expected      result
+		err           string
+	}{
+		{
+			name:     "valid versions",
+			pkgPath:  "github.com/mattermost/mattermost-server/plugin/checker/internal/test/valid",
+			expected: result{},
+		},
+		{
+			name:    "invalid versions",
+			pkgPath: "github.com/mattermost/mattermost-server/plugin/checker/internal/test/invalid",
+			expected: result{
+				Errors:   []string{"internal/test/invalid/invalid.go:20:2: documented minimum server version too low on method LowerVersionMethod"},
+				Warnings: []string{"internal/test/invalid/invalid.go:23:2: documented minimum server version too high on method HigherVersionMethod"},
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			assert := assert.New(t)
+
+			res, err := checkHelpersVersionComments(tc.pkgPath)
+			assert.Equal(tc.expected, res)
+
+			if tc.err != "" {
+				assert.EqualError(err, tc.err)
+			} else {
+				assert.NoError(err)
+			}
+
+		})
+	}
+}

plugin/checker/internal/asthelpers/helpers.go

@@ -0,0 +1,129 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See License.txt for license information.
+
+package asthelpers
+
+import (
+	"go/ast"
+	"go/types"
+
+	"github.com/pkg/errors"
+	"golang.org/x/tools/go/packages"
+)
+
+func GetPackage(pkgPath string) (*packages.Package, error) {
+	cfg := &packages.Config{
+		Mode: packages.NeedName | packages.NeedTypes | packages.NeedSyntax | packages.NeedTypesInfo,
+	}
+	pkgs, err := packages.Load(cfg, pkgPath)
+	if err != nil {
+		return nil, err
+	}
+
+	if len(pkgs) == 0 {
+		return nil, errors.Errorf("could not find package %s", pkgPath)
+	}
+	return pkgs[0], nil
+}
+
+func FindInterface(name string, files []*ast.File) (*ast.InterfaceType, error) {
+	iface, _, err := FindInterfaceWithIdent(name, files)
+	return iface, err
+}
+
+func FindInterfaceWithIdent(name string, files []*ast.File) (*ast.InterfaceType, *ast.Ident, error) {
+	var (
+		ident *ast.Ident
+		iface *ast.InterfaceType
+	)
+
+	for _, f := range files {
+		ast.Inspect(f, func(n ast.Node) bool {
+			if t, ok := n.(*ast.TypeSpec); ok {
+				if iface != nil {
+					return false
+				}
+
+				if i, ok := t.Type.(*ast.InterfaceType); ok && t.Name.Name == name {
+					ident = t.Name
+					iface = i
+					return false
+				}
+			}
+			return true
+		})
+
+		if iface != nil {
+			return iface, ident, nil
+		}
+	}
+	return nil, nil, errors.Errorf("could not find %s interface", name)
+}
+
+func FindMethodsCalledOnType(info *types.Info, typ types.Type, caller *ast.FuncDecl) []string {
+	var methods []string
+
+	ast.Inspect(caller, func(n ast.Node) bool {
+		if s, ok := n.(*ast.SelectorExpr); ok {
+
+			var receiver *ast.Ident
+			switch r := s.X.(type) {
+			case *ast.Ident:
+				// Left-hand side of the selector is an identifier, eg:
+				//
+				//   a := p.API
+				//   a.GetTeams()
+				//
+				receiver = r
+			case *ast.SelectorExpr:
+				// Left-hand side of the selector is a selector, eg:
+				//
+				//   p.API.GetTeams()
+				//
+				receiver = r.Sel
+			}
+
+			if receiver != nil {
+				obj := info.ObjectOf(receiver)
+				if obj != nil && types.Identical(obj.Type(), typ) {
+					methods = append(methods, s.Sel.Name)
+				}
+				return false
+			}
+
+		}
+		return true
+	})
+
+	return methods
+}
+
+func FindReceiverMethods(receiverName string, files []*ast.File) []*ast.FuncDecl {
+	var fns []*ast.FuncDecl
+	for _, f := range files {
+		ast.Inspect(f, func(n ast.Node) bool {
+			if fn, ok := n.(*ast.FuncDecl); ok {
+				r := extractReceiverTypeName(fn)
+				if r == receiverName {
+					fns = append(fns, fn)
+				}
+			}
+			return true
+		})
+	}
+	return fns
+}
+
+func extractReceiverTypeName(fn *ast.FuncDecl) string {
+	if fn.Recv != nil {
+		t := fn.Recv.List[0].Type
+		// Unwrap the pointer type (a star expression)
+		if se, ok := t.(*ast.StarExpr); ok {
+			t = se.X
+		}
+		if id, ok := t.(*ast.Ident); ok {
+			return id.Name
+		}
+	}
+	return ""
+}

plugin/checker/internal/test/invalid/invalid.go

@@ -14,3 +14,23 @@ type API interface {
 	// plugin comment checker with an invalid comment.
 	InvalidMethod()
 }
+
+type Helpers interface {
+	// Minimum server version: 1.1
+	LowerVersionMethod()
+
+	// Minimum server version: 1.3
+	HigherVersionMethod()
+}
+
+type HelpersImpl struct {
+	api API
+}
+
+func (h *HelpersImpl) LowerVersionMethod() {
+	h.api.ValidMethod()
+}
+
+func (h *HelpersImpl) HigherVersionMethod() {
+	h.api.ValidMethod()
+}

plugin/checker/internal/test/valid/valid.go

@@ -0,0 +1,44 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See License.txt for license information.
+
+package valid
+
+type API interface {
+	// ValidMethod is a fake method for testing the
+	// plugin comment checker with a valid comment.
+	//
+	// Minimum server version: 1.2.3
+	ValidMethod()
+
+	// Minimum server version: 1.5
+	NewerValidMethod()
+}
+
+type Helpers interface {
+	// Minimum server version: 1.2.3
+	ValidHelperMethod()
+
+	// Minimum server version: 1.5
+	NewerValidHelperMethod()
+
+	// Minimum server version: 1.5
+	IndirectReferenceMethod()
+}
+
+type HelpersImpl struct {
+	api API
+}
+
+func (h *HelpersImpl) ValidHelperMethod() {
+	h.api.ValidMethod()
+}
+
+func (h *HelpersImpl) NewerValidHelperMethod() {
+	h.api.NewerValidMethod()
+	h.api.ValidMethod()
+}
+
+func (h *HelpersImpl) IndirectReferenceMethod() {
+	a := h.api
+	a.NewerValidMethod()
+}

plugin/checker/internal/version/comments.go

@@ -0,0 +1,22 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See License.txt for license information.
+
+package version
+
+import (
+	"regexp"
+	"strings"
+)
+
+var versionCommentRE = regexp.MustCompile(`^Minimum server version: (\d+\.\d+(?:\.\d+[\w-]*)?)$`)
+
+func ExtractMinimumVersionFromComment(s string) string {
+	lines := strings.Split(strings.TrimSpace(s), "\n")
+	if len(lines) > 0 {
+		lastLine := lines[len(lines)-1]
+		if m := versionCommentRE.FindStringSubmatch(lastLine); len(m) >= 1 {
+			return m[1]
+		}
+	}
+	return ""
+}

plugin/checker/internal/version/comments_test.go

@@ -0,0 +1,49 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See License.txt for license information.
+
+package version
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestExtractVersionFromComment(t *testing.T) {
+	testCases := []struct {
+		input    string
+		expected string
+	}{
+		{
+			input:    "This is a comment.\n\nMinimum server version: 1.2.3-rc1\n",
+			expected: "1.2.3-rc1",
+		},
+		{
+			input:    "This is a comment.\n\nMinimum server version: 1.2.3\n",
+			expected: "1.2.3",
+		},
+		{
+			input:    "This is a comment.\n\nMinimum server version: 1.2\n",
+			expected: "1.2",
+		},
+		{
+			input:    "This is a comment.\n\nMinimum server version: 1\n",
+			expected: "",
+		},
+		{
+			input:    "This is a comment.\n",
+			expected: "",
+		},
+		{
+			input:    "",
+			expected: "",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(fmt.Sprintf("%+v", tc), func(t *testing.T) {
+			assert.Equal(t, tc.expected, ExtractMinimumVersionFromComment(tc.input))
+		})
+	}
+}

plugin/checker/internal/version/version.go

@@ -0,0 +1,80 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See License.txt for license information.
+
+package version
+
+import (
+	"regexp"
+	"strconv"
+	"strings"
+)
+
+type V string
+
+func (v V) GreaterThanOrEqualTo(other V) bool {
+	return !v.LessThan(other)
+}
+
+func (v V) LessThan(other V) bool {
+	leftParts, leftCount := split(v)
+	rightParts, rightCount := split(other)
+
+	var length int
+	if leftCount < rightCount {
+		length = rightCount
+	} else {
+		length = leftCount
+	}
+
+	for i := 0; i < length; i++ {
+		var left, right string
+
+		if i < leftCount {
+			left = leftParts[i]
+		}
+
+		if i < rightCount {
+			right = rightParts[i]
+		}
+
+		if left == right {
+			continue
+		}
+
+		leftInt := parseInt(left)
+		rightInt := parseInt(right)
+
+		isNumericalComparison := leftInt != nil && rightInt != nil
+
+		if isNumericalComparison {
+			return *leftInt < *rightInt
+		}
+
+		return left < right
+	}
+
+	return false
+}
+
+func split(v V) ([]string, int) {
+	var chunks []string
+
+	for _, part := range strings.Split(string(v), ".") {
+		chunks = append(chunks, splitNumericalChunks(part)...)
+	}
+
+	return chunks, len(chunks)
+}
+
+var numericalOrAlphaRE = regexp.MustCompile(`(\d+|\D+)`)
+
+func splitNumericalChunks(s string) []string {
+	return numericalOrAlphaRE.FindAllString(s, -1)
+}
+
+func parseInt(s string) *int64 {
+	if n, err := strconv.ParseInt(s, 10, 64); err == nil {
+		return &n
+	}
+	return nil
+}

plugin/checker/internal/version/version_test.go

@@ -0,0 +1,58 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See License.txt for license information.
+
+package version
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestVersionComparison(t *testing.T) {
+	testCases := []struct {
+		a, b V
+	}{
+		{
+			a: V("1.2"),
+			b: V("1.10"),
+		},
+		{
+			a: V("1.2.1"),
+			b: V("1.2.3"),
+		},
+		{
+			a: V("1.2"),
+			b: V("1.2.3"),
+		},
+		{
+			a: V("1.2.1"),
+			b: V("1.2.3"),
+		},
+		{
+			a: V("1.1"),
+			b: V("1.2.3"),
+		},
+		{
+			a: V("1.2.3"),
+			b: V("1.3"),
+		},
+		{
+			a: V("1.2.1-rc2"),
+			b: V("1.2.1-rc10"),
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(fmt.Sprintf("%+v", tc), func(t *testing.T) {
+			assert.True(t, tc.a.LessThan(tc.b))
+			assert.False(t, tc.b.LessThan(tc.a))
+
+			assert.True(t, tc.b.GreaterThanOrEqualTo(tc.a))
+			assert.False(t, tc.a.GreaterThanOrEqualTo(tc.b))
+		})
+	}
+
+	assert.True(t, V("1.2").GreaterThanOrEqualTo("1.2"))
+}

plugin/checker/main.go

@@ -4,123 +4,69 @@
 package main
 
 import (
-	"bytes"
 	"fmt"
 	"os"
-	"path/filepath"
-	"regexp"
+	"sort"
 	"strings"
-
-	"go/ast"
-
-	"golang.org/x/tools/go/packages"
-
-	"github.com/pkg/errors"
 )
 
 const pluginPackagePath = "github.com/mattermost/mattermost-server/plugin"
 
+type result struct {
+	Warnings []string
+	Errors   []string
+}
+
+type checkFn func(pkgPath string) (result, error)
+
+var checks = []checkFn{
+	checkAPIVersionComments,
+	checkHelpersVersionComments,
+}
+
 func main() {
-	if err := runCheck(pluginPackagePath); err != nil {
+	var res result
+	for _, check := range checks {
+		res = runCheck(res, check)
+	}
+
+	var msgs []string
+	msgs = append(msgs, res.Errors...)
+	msgs = append(msgs, res.Warnings...)
+	sort.Strings(msgs)
+
+	if len(msgs) > 0 {
 		fmt.Fprintln(os.Stderr, "#", pluginPackagePath)
-		fmt.Fprintln(os.Stderr, err)
+		fmt.Fprintln(os.Stderr, strings.Join(msgs, "\n"))
+	}
+
+	if len(res.Errors) > 0 {
 		os.Exit(1)
 	}
 }
 
-func runCheck(pkgPath string) error {
-	pkg, err := getPackage(pkgPath)
+func runCheck(prev result, fn checkFn) result {
+	res, err := fn(pluginPackagePath)
 	if err != nil {
-		return err
+		prev.Errors = append(prev.Errors, err.Error())
+		return prev
 	}
 
-	apiInterface := findAPIInterface(pkg.Syntax)
-	if apiInterface == nil {
-		return errors.Errorf("could not find API interface in package %s", pkgPath)
+	if len(res.Warnings) > 0 {
+		prev.Warnings = append(prev.Warnings, mapWarnings(res.Warnings)...)
 	}
 
-	invalidMethods := findInvalidMethods(apiInterface.Methods.List)
-	if len(invalidMethods) > 0 {
-		return errors.New(renderErrorMessage(pkg, invalidMethods))
+	if len(res.Errors) > 0 {
+		prev.Errors = append(prev.Errors, res.Errors...)
 	}
-	return nil
+
+	return prev
 }
 
-func getPackage(pkgPath string) (*packages.Package, error) {
-	cfg := &packages.Config{
-		Mode: packages.NeedName | packages.NeedTypes | packages.NeedSyntax,
+func mapWarnings(ss []string) []string {
+	var out []string
+	for _, s := range ss {
+		out = append(out, "[warn] "+s)
 	}
-	pkgs, err := packages.Load(cfg, pkgPath)
-	if err != nil {
-		return nil, err
-	}
-
-	if len(pkgs) == 0 {
-		return nil, errors.Errorf("could not find package %s", pkgPath)
-	}
-	return pkgs[0], nil
-}
-
-func findAPIInterface(files []*ast.File) *ast.InterfaceType {
-	for _, f := range files {
-		var iface *ast.InterfaceType
-
-		ast.Inspect(f, func(n ast.Node) bool {
-			if t, ok := n.(*ast.TypeSpec); ok {
-				if i, ok := t.Type.(*ast.InterfaceType); ok && t.Name.Name == "API" {
-					iface = i
-					return false
-				}
-			}
-			return true
-		})
-
-		if iface != nil {
-			return iface
-		}
-	}
-	return nil
-}
-
-func findInvalidMethods(methods []*ast.Field) []*ast.Field {
-	var invalid []*ast.Field
-	for _, m := range methods {
-		if !hasValidMinimumVersionComment(m.Doc.Text()) {
-			invalid = append(invalid, m)
-		}
-	}
-	return invalid
-}
-
-var versionRequirementRE = regexp.MustCompile(`^Minimum server version: \d+\.\d+(\.\d+)?$`)
-
-func hasValidMinimumVersionComment(s string) bool {
-	lines := strings.Split(strings.TrimSpace(s), "\n")
-	if len(lines) > 0 {
-		lastLine := lines[len(lines)-1]
-		return versionRequirementRE.MatchString(lastLine)
-	}
-	return false
-}
-
-func renderErrorMessage(pkg *packages.Package, methods []*ast.Field) string {
-	cwd, _ := os.Getwd()
-	out := &bytes.Buffer{}
-
-	for _, m := range methods {
-		pos := pkg.Fset.Position(m.Pos())
-		filename, err := filepath.Rel(cwd, pos.Filename)
-		if err != nil {
-			// If deriving a relative path fails for some reason,
-			// we prefer to still print the absolute path to the file.
-			filename = pos.Filename
-		}
-		fmt.Fprintf(out,
-			"%s:%d:%d: missing a minimum server version comment\n",
-			filename,
-			pos.Line,
-			pos.Column,
-		)
-	}
-	return out.String()
+	return out
 }

plugin/checker/render.go

@@ -0,0 +1,29 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See License.txt for license information.
+
+package main
+
+import (
+	"fmt"
+	"go/token"
+	"os"
+	"path/filepath"
+)
+
+func renderWithFilePosition(fset *token.FileSet, pos token.Pos, msg string) string {
+	var cwd string
+	if d, err := os.Getwd(); err == nil {
+		cwd = d
+	}
+
+	fpos := fset.Position(pos)
+
+	filename, err := filepath.Rel(cwd, fpos.Filename)
+	if err != nil {
+		// If deriving a relative path fails for some reason,
+		// we prefer to still print the absolute path to the file.
+		filename = fpos.Filename
+	}
+
+	return fmt.Sprintf("%s:%d:%d: %s", filename, fpos.Line, fpos.Column, msg)
+}

plugin/checker/test/valid/valid.go

@@ -1,12 +0,0 @@
-// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
-// See License.txt for license information.
-
-package valid
-
-type API interface {
-	// ValidMethod is a fake method for testing the
-	// plugin comment checker with a valid comment.
-	//
-	// Minimum server version: 1.2.3
-	ValidMethod()
-}

plugin/helpers.go

@@ -8,9 +8,13 @@ import "github.com/mattermost/mattermost-server/model"
 type Helpers interface {
 	// EnsureBot either returns an existing bot user matching the given bot, or creates a bot user from the given bot.
 	// Returns the id of the resulting bot.
+	//
+	// Minimum server version: 5.10
 	EnsureBot(bot *model.Bot) (string, error)
 
 	// KVSetJSON stores a key-value pair, unique per plugin, marshalling the given value as a JSON string.
+	//
+	// Minimum server version: 5.2
 	KVSetJSON(key string, value interface{}) error
 
 	// KVCompareAndSetJSON updates a key-value pair, unique per plugin, but only if the current value matches the given oldValue after marshalling as a JSON string.
@@ -31,6 +35,8 @@ type Helpers interface {
 	KVCompareAndDeleteJSON(key string, oldValue interface{}) (bool, error)
 
 	// KVGetJSON retrieves a value based on the key, unique per plugin, unmarshalling the previously set JSON string into the given value. Returns true if the key exists.
+	//
+	// Minimum server version: 5.2
 	KVGetJSON(key string, value interface{}) (bool, error)
 
 	// KVSetWithExpiryJSON stores a key-value pair with an expiry time, unique per plugin, marshalling the given value as a JSON string.