Skip to content

Commit

Permalink
fix index and tests
Browse files Browse the repository at this point in the history
Signed-off-by: Timon Wong <[email protected]>
  • Loading branch information
timonwong committed Aug 29, 2022
1 parent 882273a commit 385ccfa
Show file tree
Hide file tree
Showing 6 changed files with 31 additions and 13 deletions.
27 changes: 14 additions & 13 deletions internal/rules/rules.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"strings"

"github.com/timonwong/loggercheck/internal/bytebufferpool"
"github.com/timonwong/loggercheck/internal/sets"
)

var ErrInvalidRule = errors.New("invalid rule format")
Expand All @@ -36,7 +35,7 @@ type Ruleset struct {
PackageImport string
Rules []FuncRule

funcNames sets.StringSet
ruleIndicesByFuncName map[string][]int
}

func (rs *Ruleset) Match(fn *types.Func, pkg *types.Package) bool {
Expand All @@ -48,12 +47,13 @@ func (rs *Ruleset) Match(fn *types.Func, pkg *types.Package) bool {
sig := fn.Type().(*types.Signature) // it's safe since we already checked

// Fail fast if func name not contains in index
if !rs.funcNames.Has(fn.Name()) {
indices, ok := rs.ruleIndicesByFuncName[fn.Name()]
if !ok {
return false
}

for i := range rs.Rules {
rule := &rs.Rules[i]
for _, idx := range indices {
rule := &rs.Rules[idx]
if matchRule(rule, sig) {
return true
}
Expand All @@ -65,7 +65,7 @@ func (rs *Ruleset) Match(fn *types.Func, pkg *types.Package) bool {
func emptyQualifier(*types.Package) string { return "" }

func matchRule(p *FuncRule, sig *types.Signature) bool {
// we do not check package import and func name here since it's already checked in Match()
// we do not check package import here since it's already checked in Match()
recv := sig.Recv()
isReceiver := recv != nil
if isReceiver != p.IsReceiver {
Expand Down Expand Up @@ -153,16 +153,17 @@ func ParseRules(lines []string) (result RulesetList, err error) {
}

for packageImport, rules := range rulesByImport {
funcNames := make([]string, 0, len(rules))
for _, rule := range rules {
funcNames = append(funcNames, rule.FuncName)
ruleIndicesByFuncName := make(map[string][]int, len(rules))
for idx, rule := range rules {
fnName := rule.FuncName
ruleIndicesByFuncName[fnName] = append(ruleIndicesByFuncName[fnName], idx)
}

result = append(result, Ruleset{
Name: "", // NOTE(timonwong) Always "" for custom rule
PackageImport: packageImport,
Rules: rules,
funcNames: sets.NewString(funcNames...),
Name: "", // NOTE(timonwong) Always "" for custom rule
PackageImport: packageImport,
Rules: rules,
ruleIndicesByFuncName: ruleIndicesByFuncName,
})
}
return result, nil
Expand Down
3 changes: 3 additions & 0 deletions loggercheck_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ func TestOptions(t *testing.T) {
"(*a/customonly.Logger).Warnw",
"(*a/customonly.Logger).Errorw",
"(*a/customonly.Logger).With",

"(a/customonly.Logger).XXXDebugw",

"a/customonly.Debugw",
"a/customonly.Infow",
"a/customonly.Warnw",
Expand Down
3 changes: 3 additions & 0 deletions testdata/custom-rules.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@
(*a/customonly.Logger).Errorw
(*a/customonly.Logger).With

# Test case for the wrong receiver type
(a/customonly.Logger).XXXDebugw

# Exported package level functions
a/customonly.Debugw
a/customonly.Infow
Expand Down
3 changes: 3 additions & 0 deletions testdata/src/a/customonly/example.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ func ExampleCustomLogger() {
log := New()
defer log.Sync()

// won't bail because rules file wrong
log.XXXDebugw("message", "key1")

log.Infow("abc", "key1", "value1")
log.Infow("abc", "key1", "value1", "key2") // want `odd number of arguments passed as key-value pairs for logging`

Expand Down
4 changes: 4 additions & 0 deletions testdata/src/a/customonly/zap_wrapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ func (l *Logger) With(keysAndValues ...interface{}) *Logger {
}
}

func (l *Logger) XXXDebugw(msg string, keysAndValues ...interface{}) {
l.s.Debugw(msg, keysAndValues...)
}

func (l *Logger) Debugw(msg string, keysAndValues ...interface{}) {
l.s.Debugw(msg, keysAndValues...)
}
Expand Down
4 changes: 4 additions & 0 deletions testdata/src/a/klogonly/example.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,16 @@ func logrIgnored() {
func ExampleKlog() {
err := fmt.Errorf("error")

klog.Infof("message: %s", "key1")

klog.InfoS("abc", "key1", "value1")
klog.InfoS("abc", "key1", "value1", "key2") // want `odd number of arguments passed as key-value pairs for logging`

klog.ErrorS(err, "abc", "key1", "value1")
klog.ErrorS(err, "abc", "key1", "value1", "key2") // want `odd number of arguments passed as key-value pairs for logging`

klog.V(1).Infof("message: %s", "key1")

klog.V(1).InfoS("message", "key1", "value1")
klog.V(1).InfoS("message", "key1", "value1", "key2") // want `odd number of arguments passed as key-value pairs for logging`

Expand Down

0 comments on commit 385ccfa

Please sign in to comment.