refactor: rename methodrule -> funcrule and improve coverage (#21)

* refactor: rename methodrule -> funcrule

Signed-off-by: Timon Wong <timon86.wang@gmail.com>

* chore: improve coverage

Signed-off-by: Timon Wong <timon86.wang@gmail.com>

* chore: Update badges section in README

Signed-off-by: Timon Wong <timon86.wang@gmail.com>

Signed-off-by: Timon Wong <timon86.wang@gmail.com>

README.md

@@ -1,10 +1,19 @@
 # loggercheck
 
+## Description
+
 A linter checks the odd number of key and value pairs for common logger libraries:
 - [logr](https://github.com/go-logr/logr)
 - [klog](https://github.com/kubernetes/klog)
 - [zap](https://github.com/uber-go/zap)
 
+## Badges
+
+![Build Status](https://github.com/timonwong/loggercheck/workflows/CI/badge.svg)
+[![Coverage](https://img.shields.io/codecov/c/github/timonwong/loggercheck?token=Nutf41gwoG)](https://app.codecov.io/gh/timonwong/loggercheck)
+[![License](https://img.shields.io/github/license/timonwong/loggercheck.svg)](/LICENSE)
+[![Release](https://img.shields.io/github/release/timonwong/loggercheck.svg)](https://github.com/timonwong/loggercheck/releases/latest)
+
 ## Install
 
 ```shel

flags_test.go

@@ -6,30 +6,60 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
-func TestIgnoredLoggerFlag(t *testing.T) {
-	f := loggerCheckersFlag{}
+func TestLoggerCheckersFlag(t *testing.T) {
+	testCases := []struct {
+		name      string
+		flagValue string
+		wantError string
+		want      []string
+	}{
+		{
+			name:      "empty",
+			flagValue: "",
+			want:      nil,
+		},
+		{
+			name:      "klog",
+			flagValue: "klog",
+			want:      []string{"klog"},
+		},
+		{
+			name:      "klog-and-logr",
+			flagValue: "logr,klog",
+			want:      []string{"klog", "logr"},
+		},
+		{
+			name:      "invalid-logger",
+			flagValue: "klog,logr,xxx",
+			wantError: "-ignoredloggers: unknown logger: \"xxx\"",
+		},
+	}
 
-	fs := flag.NewFlagSet("test", flag.ContinueOnError)
-	fs.SetOutput(io.Discard)
-	fs.Var(&f, "ignoredloggers", "")
-
-	var err error
-
-	err = fs.Parse([]string{"-ignoredloggers=klog"})
-	assert.NoError(t, err)
-	assert.Equal(t, []string{"klog"}, f.List())
+	for _, tc := range testCases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
 
-	err = fs.Parse([]string{"-ignoredloggers=logr,klog"})
-	assert.NoError(t, err)
-	assert.Equal(t, []string{"klog", "logr"}, f.List())
+			f := loggerCheckersFlag{}
+			fs := flag.NewFlagSet("test", flag.ContinueOnError)
+			fs.SetOutput(io.Discard)
+			fs.Var(&f, "ignoredloggers", "")
 
-	err = fs.Parse([]string{"-ignoredloggers=logr,klog,unknownlogger"})
-	assert.ErrorContains(t, err, "-ignoredloggers: unknown logger: \"unknownlogger\"")
+			err := fs.Parse([]string{"-ignoredloggers=" + tc.flagValue})
+			if tc.wantError != "" {
+				assert.ErrorContains(t, err, tc.wantError)
+			} else {
+				require.NoError(t, err)
+				assert.Equal(t, tc.want, f.List())
+			}
+		})
+	}
 }
 
-func TestNoRuleFile(t *testing.T) {
+func TestRuleFileFlag_NoRuleFile(t *testing.T) {
 	f := ruleFileFlag{}
 
 	fs := flag.NewFlagSet("test", flag.ContinueOnError)
@@ -40,7 +70,7 @@ func TestNoRuleFile(t *testing.T) {
 	assert.ErrorContains(t, err, "open testdata/xxx-not-exists-xxx.txt: no such file or directory")
 }
 
-func TestWrongRuleFile(t *testing.T) {
+func TestRuleFileFlag_WrongRuleFile(t *testing.T) {
 	f := ruleFileFlag{}
 
 	fs := flag.NewFlagSet("test", flag.ContinueOnError)

rules/rules.go

@@ -42,7 +42,7 @@ func (rl RulesetList) Names() []string {
 type Ruleset struct {
 	Name          string
 	PackageImport string
-	Rules         []MethodRule
+	Rules         []FuncRule
 }
 
 func (rs *Ruleset) Match(fn *types.Func, pkg *types.Package) bool {
@@ -63,13 +63,13 @@ func (rs *Ruleset) Match(fn *types.Func, pkg *types.Package) bool {
 
 func emptyQualifier(*types.Package) string { return "" }
 
-type MethodRule struct { // package import should be accessed from Rulset
+type FuncRule struct { // package import should be accessed from Rulset
 	ReceiverType string
 	MethodName   string
 	IsReceiver   bool
 }
 
-func (p *MethodRule) match(fn *types.Func, sig *types.Signature) bool {
+func (p *FuncRule) match(fn *types.Func, sig *types.Signature) bool {
 	// we do not check package import here since it's already checked
 	if fn.Name() != p.MethodName {
 		return false
@@ -94,7 +94,7 @@ func (p *MethodRule) match(fn *types.Func, sig *types.Signature) bool {
 	return true
 }
 
-func ParseMethodRule(rule string) (packageImport string, pat MethodRule, err error) {
+func ParseFuncRule(rule string) (packageImport string, pat FuncRule, err error) {
 	lastDot := strings.LastIndexFunc(rule, func(r rune) bool {
 		return r == '.' || r == '/'
 	})
@@ -107,7 +107,7 @@ func ParseMethodRule(rule string) (packageImport string, pat MethodRule, err err
 
 	if strings.HasPrefix(rule, "(") { // package
 		if !strings.HasSuffix(importOrReceiver, ")") {
-			return "", MethodRule{}, ErrInvalidRule
+			return "", FuncRule{}, ErrInvalidRule
 		}
 
 		var isPointerReceiver bool
@@ -122,7 +122,7 @@ func ParseMethodRule(rule string) (packageImport string, pat MethodRule, err err
 			return r == '.' || r == '/'
 		})
 		if typeDotIdx == -1 || receiver[typeDotIdx] == '/' {
-			return "", MethodRule{}, ErrInvalidRule
+			return "", FuncRule{}, ErrInvalidRule
 		}
 		receiverType := receiver[typeDotIdx+1:]
 		if isPointerReceiver {
@@ -138,7 +138,7 @@ func ParseMethodRule(rule string) (packageImport string, pat MethodRule, err err
 }
 
 func ParseRules(lines []string) (result RulesetList, err error) {
-	rulesByImport := make(map[string][]MethodRule)
+	rulesByImport := make(map[string][]FuncRule)
 	for i, line := range lines {
 		if line == "" {
 			continue
@@ -148,7 +148,7 @@ func ParseRules(lines []string) (result RulesetList, err error) {
 			continue
 		}
 
-		packageImport, pat, err := ParseMethodRule(line)
+		packageImport, pat, err := ParseFuncRule(line)
 		if err != nil {
 			return nil, fmt.Errorf("error parse rule at line %d: %w", i+1, err)
 		}

rules/rules_test.go

@@ -1,58 +1,71 @@
 package rules
 
 import (
+	"errors"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
-func TestParseMethodRule(t *testing.T) {
+type brokenIOReader struct{}
+
+func (*brokenIOReader) Read(p []byte) (n int, err error) {
+	return 0, errors.New("broken IO")
+}
+
+func TestParseRuleFile_IOError(t *testing.T) {
+	r := &brokenIOReader{}
+	_, err := ParseRuleFile(r)
+	assert.EqualError(t, err, "broken IO")
+}
+
+func TestParseFuncRule(t *testing.T) {
 	testCases := []struct {
 		name              string
-		methodRule        string
+		rule              string
 		wantError         error
 		wantPackageImport string
-		wantRule          MethodRule
+		wantRule          FuncRule
 	}{
 		{
-			name:       "invalid-rule-missing-paren",
-			methodRule: "(*go.uber.org/zap/SugaredLogger.Debugw",
-			wantError:  ErrInvalidRule,
+			name:      "invalid-rule-missing-paren",
+			rule:      "(*go.uber.org/zap/SugaredLogger.Debugw",
+			wantError: ErrInvalidRule,
 		},
 		{
-			name:       "invalid-rule-receiver-no-type",
-			methodRule: "(*go.uber.org/zap/SugaredLogger).Debugw",
-			wantError:  ErrInvalidRule,
+			name:      "invalid-rule-receiver-no-type",
+			rule:      "(*go.uber.org/zap/SugaredLogger).Debugw",
+			wantError: ErrInvalidRule,
 		},
 		{
-			name:       "invalid-rule-just-import",
-			methodRule: "go.uber.org/zap",
-			wantError:  ErrInvalidRule,
+			name:      "invalid-rule-just-import",
+			rule:      "go.uber.org/zap",
+			wantError: ErrInvalidRule,
 		},
 		{
 			name:              "zap",
-			methodRule:        "(*go.uber.org/zap.SugaredLogger).Debugw",
+			rule:              "(*go.uber.org/zap.SugaredLogger).Debugw",
 			wantPackageImport: "go.uber.org/zap",
-			wantRule: MethodRule{
+			wantRule: FuncRule{
 				IsReceiver:   true,
 				ReceiverType: "*SugaredLogger",
 				MethodName:   "Debugw",
 			},
 		},
 		{
 			name:              "klog-no-receiver",
-			methodRule:        "k8s.io/klog/v2.InfoS",
+			rule:              "k8s.io/klog/v2.InfoS",
 			wantPackageImport: "k8s.io/klog/v2",
-			wantRule: MethodRule{
+			wantRule: FuncRule{
 				MethodName: "InfoS",
 			},
 		},
 		{
 			name:              "logr",
-			methodRule:        "(github.com/go-logr/logr.Logger).Error",
+			rule:              "(github.com/go-logr/logr.Logger).Error",
 			wantPackageImport: "github.com/go-logr/logr",
-			wantRule: MethodRule{
+			wantRule: FuncRule{
 				IsReceiver:   true,
 				ReceiverType: "Logger",
 				MethodName:   "Error",
@@ -65,7 +78,7 @@ func TestParseMethodRule(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
 
-			gotPackageImport, gotRule, err := ParseMethodRule(tc.methodRule)
+			gotPackageImport, gotRule, err := ParseFuncRule(tc.rule)
 			if tc.wantError != nil {
 				assert.EqualError(t, err, tc.wantError.Error())
 			} else {

testdata/custom-rules.txt

@@ -5,6 +5,7 @@
 (*a/customonly.Logger).Warnw
 (*a/customonly.Logger).Errorw
 (*a/customonly.Logger).With
+
 # Exported package level functions
 a/customonly.Debugw
 a/customonly.Infow

testdata/src/a/all/example.go

@@ -10,6 +10,14 @@ import (
 	"k8s.io/klog/v2"
 )
 
+func ExampleInvalid() {
+	// function pointer is not supported
+
+	log := logr.Discard()
+	logFn := log.Info
+	logFn("message", "key1") // cannot be detected
+}
+
 func ExampleLogr() {
 	err := fmt.Errorf("error")