Skip to content

Commit 9748dc5

Browse files
committed
fix(occurrence): reduce ApplySettings cognitive complexity; add missing test coverage
Extract per-setting helper methods from ApplySettings to satisfy the gocognit <= 30 limit. Also add tests for file-scope each mode, section-scope combined and pattern modes, type-error paths in ApplySettings helpers, and multi-section range-skip branches, raising unit coverage from 79% to 99%.
1 parent 2e4b5f6 commit 9748dc5

2 files changed

Lines changed: 262 additions & 68 deletions

File tree

internal/rules/occurrence/rule.go

Lines changed: 118 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -297,88 +297,116 @@ func (r *Rule) ApplySettings(s map[string]any) error {
297297
// before the pattern is compiled.
298298
rawPattern := ""
299299
for k, v := range s {
300+
var err error
300301
switch k {
301302
case "scope":
302-
str, ok := v.(string)
303-
if !ok {
304-
return fmt.Errorf("occurrence: scope must be a string, got %T", v)
305-
}
306-
switch str {
307-
case "file", "section", "paragraph":
308-
r.Scope = str
309-
default:
310-
return fmt.Errorf("occurrence: scope must be file, section, or paragraph, got %q", str)
311-
}
303+
err = r.applyScope(v)
312304
case "tokens":
313-
ss, ok := settings.ToStringSlice(v)
314-
if !ok {
315-
return fmt.Errorf("occurrence: tokens must be a list of strings, got %T", v)
316-
}
317-
r.Tokens = ss
318-
// tokens uses the default replace merge mode (not append); no
319-
// SettingMergeMode override is needed.
305+
err = r.applyTokens(v)
320306
case "pattern":
321-
str, ok := v.(string)
322-
if !ok {
323-
return fmt.Errorf("occurrence: pattern must be a string, got %T", v)
324-
}
325-
rawPattern = str
307+
rawPattern, err = extractPattern(v)
326308
case "min":
327-
n, ok := settings.ToInt(v)
328-
if !ok {
329-
return fmt.Errorf("occurrence: min must be an integer, got %T", v)
330-
}
331-
if n < 0 {
332-
return fmt.Errorf("occurrence: min must be >= 0, got %d", n)
333-
}
334-
r.Min = n
309+
err = r.applyMin(v)
335310
case "max":
336-
n, ok := settings.ToInt(v)
337-
if !ok {
338-
return fmt.Errorf("occurrence: max must be an integer, got %T", v)
339-
}
340-
r.Max = n
311+
err = r.applyMax(v)
341312
case "count":
342-
str, ok := v.(string)
343-
if !ok {
344-
return fmt.Errorf("occurrence: count must be a string, got %T", v)
345-
}
346-
switch str {
347-
case "each", "combined":
348-
r.Count = str
349-
default:
350-
return fmt.Errorf("occurrence: count must be each or combined, got %q", str)
351-
}
313+
err = r.applyCount(v)
352314
case "case-sensitive":
353-
b, ok := v.(bool)
354-
if !ok {
355-
return fmt.Errorf("occurrence: case-sensitive must be a bool, got %T", v)
356-
}
357-
r.CaseSensitive = b
315+
err = r.applyCaseSensitive(v)
358316
default:
359317
return fmt.Errorf("occurrence: unknown setting %q", k)
360318
}
319+
if err != nil {
320+
return err
321+
}
322+
}
323+
return r.finalizeSettings(rawPattern)
324+
}
325+
326+
func (r *Rule) applyScope(v any) error {
327+
str, ok := v.(string)
328+
if !ok {
329+
return fmt.Errorf("occurrence: scope must be a string, got %T", v)
330+
}
331+
switch str {
332+
case "file", "section", "paragraph":
333+
r.Scope = str
334+
return nil
335+
default:
336+
return fmt.Errorf("occurrence: scope must be file, section, or paragraph, got %q", str)
337+
}
338+
}
339+
340+
func (r *Rule) applyTokens(v any) error {
341+
ss, ok := settings.ToStringSlice(v)
342+
if !ok {
343+
return fmt.Errorf("occurrence: tokens must be a list of strings, got %T", v)
344+
}
345+
// tokens uses the default replace merge mode (not append); no
346+
// SettingMergeMode override is needed.
347+
r.Tokens = ss
348+
return nil
349+
}
350+
351+
func extractPattern(v any) (string, error) {
352+
str, ok := v.(string)
353+
if !ok {
354+
return "", fmt.Errorf("occurrence: pattern must be a string, got %T", v)
355+
}
356+
return str, nil
357+
}
358+
359+
func (r *Rule) applyMin(v any) error {
360+
n, ok := settings.ToInt(v)
361+
if !ok {
362+
return fmt.Errorf("occurrence: min must be an integer, got %T", v)
363+
}
364+
if n < 0 {
365+
return fmt.Errorf("occurrence: min must be >= 0, got %d", n)
366+
}
367+
r.Min = n
368+
return nil
369+
}
370+
371+
func (r *Rule) applyMax(v any) error {
372+
n, ok := settings.ToInt(v)
373+
if !ok {
374+
return fmt.Errorf("occurrence: max must be an integer, got %T", v)
375+
}
376+
r.Max = n
377+
return nil
378+
}
379+
380+
func (r *Rule) applyCount(v any) error {
381+
str, ok := v.(string)
382+
if !ok {
383+
return fmt.Errorf("occurrence: count must be a string, got %T", v)
361384
}
362-
// Compile pattern after all scalar settings are applied so CaseSensitive
363-
// is final when deciding the (?i) prefix.
385+
switch str {
386+
case "each", "combined":
387+
r.Count = str
388+
return nil
389+
default:
390+
return fmt.Errorf("occurrence: count must be each or combined, got %q", str)
391+
}
392+
}
393+
394+
func (r *Rule) applyCaseSensitive(v any) error {
395+
b, ok := v.(bool)
396+
if !ok {
397+
return fmt.Errorf("occurrence: case-sensitive must be a bool, got %T", v)
398+
}
399+
r.CaseSensitive = b
400+
return nil
401+
}
402+
403+
// finalizeSettings compiles the pattern (if any) and builds lowerTokens.
404+
// Called after all scalar settings are applied so CaseSensitive is final.
405+
func (r *Rule) finalizeSettings(rawPattern string) error {
364406
if rawPattern != "" {
365-
r.patternSource = rawPattern
366-
src := rawPattern
367-
if !r.CaseSensitive {
368-
src = "(?i)" + rawPattern
407+
if err := r.compileAndSetPattern(rawPattern); err != nil {
408+
return err
369409
}
370-
var compiled *regexp.Regexp
371-
if actual, loaded := compiledPatterns.Load(src); loaded {
372-
compiled = actual.(*regexp.Regexp)
373-
} else {
374-
re, err := regexp.Compile(src)
375-
if err != nil {
376-
return fmt.Errorf("occurrence: pattern %q is not a valid Go RE2 regex: %w", rawPattern, err)
377-
}
378-
actual, _ := compiledPatterns.LoadOrStore(src, re)
379-
compiled = actual.(*regexp.Regexp)
380-
}
381-
r.Pattern = compiled
382410
}
383411
if len(r.Tokens) > 0 && r.Pattern != nil {
384412
return fmt.Errorf("occurrence: tokens and pattern are mutually exclusive")
@@ -392,6 +420,28 @@ func (r *Rule) ApplySettings(s map[string]any) error {
392420
return nil
393421
}
394422

423+
// compileAndSetPattern compiles rawPattern (with (?i) prefix when not
424+
// CaseSensitive) and stores the result in r.Pattern and r.patternSource.
425+
func (r *Rule) compileAndSetPattern(rawPattern string) error {
426+
src := rawPattern
427+
if !r.CaseSensitive {
428+
src = "(?i)" + rawPattern
429+
}
430+
if actual, loaded := compiledPatterns.Load(src); loaded {
431+
r.patternSource = rawPattern
432+
r.Pattern = actual.(*regexp.Regexp)
433+
return nil
434+
}
435+
re, err := regexp.Compile(src)
436+
if err != nil {
437+
return fmt.Errorf("occurrence: pattern %q is not a valid Go RE2 regex: %w", rawPattern, err)
438+
}
439+
actual, _ := compiledPatterns.LoadOrStore(src, re)
440+
r.patternSource = rawPattern
441+
r.Pattern = actual.(*regexp.Regexp)
442+
return nil
443+
}
444+
395445
// DefaultSettings implements rule.Configurable.
396446
func (r *Rule) DefaultSettings() map[string]any {
397447
return map[string]any{

internal/rules/occurrence/rule_test.go

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,3 +277,147 @@ func TestDefaultSettings_Keys(t *testing.T) {
277277
assert.Contains(t, d, "count")
278278
assert.Contains(t, d, "case-sensitive")
279279
}
280+
281+
// --- file scope, each mode ---
282+
283+
func TestCheck_File_EachToken_ExceedsMax(t *testing.T) {
284+
r := &Rule{}
285+
mustApply(t, r, map[string]any{"tokens": []any{"word"}, "max": 3, "scope": "file", "count": "each"})
286+
// "word" total: 2 + 2 = 4 > max 3
287+
src := "# Title\n\nword word.\n\nalso word word.\n"
288+
diags := r.Check(mustFile(t, src))
289+
require.Len(t, diags, 1)
290+
assert.Contains(t, diags[0].Message, "file")
291+
}
292+
293+
func TestCheck_File_EachToken_UnderMax_NoDiagnostic(t *testing.T) {
294+
r := &Rule{}
295+
mustApply(t, r, map[string]any{"tokens": []any{"word"}, "max": 3, "scope": "file", "count": "each"})
296+
src := "# Title\n\nword word.\n\nalso word here.\n"
297+
assert.Empty(t, r.Check(mustFile(t, src)))
298+
}
299+
300+
func TestCheck_File_EachPattern_ExceedsMax(t *testing.T) {
301+
r := &Rule{}
302+
mustApply(t, r, map[string]any{"pattern": "—", "max": 2, "scope": "file", "count": "each"})
303+
// 3 em-dashes across two paragraphs → exceeds max 2
304+
src := "# T\n\nFirst — second.\n\nThird — fourth — end.\n"
305+
diags := r.Check(mustFile(t, src))
306+
require.Len(t, diags, 1)
307+
assert.Contains(t, diags[0].Message, "file")
308+
}
309+
310+
// --- section scope: combined and pattern ---
311+
312+
func TestCheck_Section_NoHeadings_NoDiagnostic(t *testing.T) {
313+
r := &Rule{}
314+
mustApply(t, r, map[string]any{"tokens": []any{"word"}, "max": 1, "scope": "section", "count": "each"})
315+
// no headings → no sections → no diagnostics
316+
src := "plain paragraph word word word.\n"
317+
assert.Empty(t, r.Check(mustFile(t, src)))
318+
}
319+
320+
func TestCheck_Section_Combined_ExceedsMax(t *testing.T) {
321+
r := &Rule{}
322+
mustApply(t, r, map[string]any{"tokens": []any{"a", "b"}, "max": 3, "scope": "section", "count": "combined"})
323+
// "a" × 3, "b" × 1 → combined 4 > max 3
324+
src := "# Title\n\na a a b.\n"
325+
diags := r.Check(mustFile(t, src))
326+
require.Len(t, diags, 1)
327+
assert.Contains(t, diags[0].Message, "section")
328+
}
329+
330+
func TestCheck_Section_Pattern_ExceedsMax(t *testing.T) {
331+
r := &Rule{}
332+
mustApply(t, r, map[string]any{"pattern": "—", "max": 2, "scope": "section", "count": "each"})
333+
// 3 em-dashes in the section → exceeds max 2
334+
src := "# Title\n\nFirst — second — third — end.\n"
335+
diags := r.Check(mustFile(t, src))
336+
require.Len(t, diags, 1)
337+
assert.Contains(t, diags[0].Message, "section")
338+
}
339+
340+
// --- ApplySettings type-error paths ---
341+
342+
func TestApplySettings_ScopeWrongType(t *testing.T) {
343+
r := &Rule{}
344+
err := r.ApplySettings(map[string]any{"scope": 42})
345+
require.Error(t, err)
346+
assert.Contains(t, err.Error(), "scope")
347+
}
348+
349+
func TestApplySettings_TokensWrongType(t *testing.T) {
350+
r := &Rule{}
351+
err := r.ApplySettings(map[string]any{"tokens": "word"})
352+
require.Error(t, err)
353+
assert.Contains(t, err.Error(), "tokens")
354+
}
355+
356+
func TestApplySettings_PatternWrongType(t *testing.T) {
357+
r := &Rule{}
358+
err := r.ApplySettings(map[string]any{"pattern": 42})
359+
require.Error(t, err)
360+
assert.Contains(t, err.Error(), "pattern")
361+
}
362+
363+
func TestApplySettings_MaxWrongType(t *testing.T) {
364+
r := &Rule{}
365+
err := r.ApplySettings(map[string]any{"max": "two"})
366+
require.Error(t, err)
367+
assert.Contains(t, err.Error(), "max")
368+
}
369+
370+
func TestApplySettings_CountWrongType(t *testing.T) {
371+
r := &Rule{}
372+
err := r.ApplySettings(map[string]any{"count": 42})
373+
require.Error(t, err)
374+
assert.Contains(t, err.Error(), "count")
375+
}
376+
377+
func TestApplySettings_CaseSensitiveWrongType(t *testing.T) {
378+
r := &Rule{}
379+
err := r.ApplySettings(map[string]any{"case-sensitive": "yes"})
380+
require.Error(t, err)
381+
assert.Contains(t, err.Error(), "case-sensitive")
382+
}
383+
384+
func TestApplySettings_MinWrongType(t *testing.T) {
385+
r := &Rule{}
386+
err := r.ApplySettings(map[string]any{"min": "two"})
387+
require.Error(t, err)
388+
assert.Contains(t, err.Error(), "min")
389+
}
390+
391+
// --- paragraph scope: pattern with count=each ---
392+
393+
func TestCheck_Paragraph_PatternEach_ExceedsMax(t *testing.T) {
394+
r := &Rule{}
395+
mustApply(t, r, map[string]any{"pattern": "—", "max": 2, "count": "each"})
396+
src := "# Title\n\nFirst — second — third — end.\n"
397+
diags := r.Check(mustFile(t, src))
398+
require.Len(t, diags, 1)
399+
assert.Contains(t, diags[0].Message, "3")
400+
assert.Contains(t, diags[0].Message, "max 2")
401+
}
402+
403+
// --- section scope: combined with multiple sections (exercises range-skip) ---
404+
405+
func TestCheck_Section_Combined_MultipleHeadings(t *testing.T) {
406+
r := &Rule{}
407+
mustApply(t, r, map[string]any{"tokens": []any{"a", "b"}, "max": 3, "scope": "section", "count": "combined"})
408+
// section A: "a"×3 + "b"×1 = 4 > max 3; section B: "a"×1 = 1 ≤ max 3
409+
src := "# A\n\na a a b.\n\n## B\n\na once.\n"
410+
diags := r.Check(mustFile(t, src))
411+
require.Len(t, diags, 1)
412+
assert.Equal(t, 1, diags[0].Line)
413+
}
414+
415+
func TestCheck_Section_PatternMultipleHeadings(t *testing.T) {
416+
r := &Rule{}
417+
mustApply(t, r, map[string]any{"pattern": "—", "max": 2, "scope": "section", "count": "each"})
418+
// section A: 3 dashes > max 2; section B: 1 dash ≤ max 2
419+
src := "# A\n\nFirst — second — third — end.\n\n## B\n\nOnly — one.\n"
420+
diags := r.Check(mustFile(t, src))
421+
require.Len(t, diags, 1)
422+
assert.Equal(t, 1, diags[0].Line)
423+
}

0 commit comments

Comments
 (0)