feat(allowlist): add matlab support

[GenosseFlosse]

Description

Type of Change

• Bug fix (non-breaking change that fixes an issue)
• New feature (non-breaking change that adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Refactoring (no functional changes)
• Documentation update
• CI / Build / Tooling

How Has This Been Tested?

• make test passes locally
• Manual testing (describe below)
  • ocr rules check reports the correct rules when checking manually

Checklist

• My code follows the project's coding style (go fmt, go vet)
• I have performed a self-review of my code
• I have added tests that prove my fix is effective or my feature works
• New and existing unit tests pass locally with my changes
• I have updated the documentation accordingly (if applicable)
• I have signed the CLA

Related Issues

[CLAassistant]

All committers have signed the CLA.

[github-actions]

✅ OpenCodeReview: No comments generated. Looks good to me.

[lizhengfeng101]

Thanks for the contribution — the MATLAB rule doc itself is excellent and very thorough!

One concern before merging: the .m extension is shared between MATLAB and Objective-C. This PR maps all **/*.m files to matlab.md, which means Objective-C files (e.g. ios/ViewController.m, AppDelegate.m) will receive MATLAB-specific review feedback — clearly incorrect for those files.

The test change confirms this: ios/ViewController.m was removed from the "fallback to default" test because it now matches the MATLAB rule.

Possible approaches

1. Path-based heuristics — put ObjC-specific glob patterns (e.g. **/{ios,macos,tvos,watchos,Pods,Classes}/**/*.m) before **/*.m in system_rules.json, leveraging "first match wins" order. Downside: incomplete coverage, maintenance burden.

2. Content-based detection — sniff the first line of the file (function → MATLAB, #import/#include → ObjC). Downside: requires changes to the Resolve interface which currently only takes a path.

3. Accept the limitation — document that .m defaults to MATLAB and ObjC projects should use a project-level .opencodereview/rule.json to override. Simplest, but shifts burden to ObjC users.

Minor formatting issues in system_rules.json

• Trailing space on the "**/*.bicep": "bicep.md", line
• The new "**/*.m" entry uses a tab for indentation, while the rest of the file uses 4 spaces

Would love to hear your thoughts on how you'd like to handle the extension ambiguity. Happy to discuss further!

[GenosseFlosse]

Dear @lizhengfeng101,

I opted for Option Number 2, as this will ensure that it just works instead of anyone stumbling onto this in a few months. I added a mock objc.md for someone to fill at a later date (I don't know anything about objective c). The beahviour is documented in review_rules.md

[GenosseFlosse]

Dear @lizhengfeng101
I have resolved the latest merge requests an I am happy to discuss any review you might bring up. Thank you for reviewing my changes :)

[lizhengfeng101]

@GenosseFlosse Thanks for adding MATLAB support — the rule doc is thorough and the test coverage looks solid!

One architectural suggestion before we move forward: the current approach threads a content parameter through the entire resolver pipeline (new interfaces, signature changes across 16 files) to handle what is essentially a single special case (.m disambiguation). This makes the change more invasive than it needs to be.

A lighter-weight alternative would be a decorator/sniffer wrapper around the existing resolver:

• The Resolver interface stays unchanged — still just Resolve(path string) string.
• A thin Sniffer struct wraps the composed resolver. It holds the repo path and a pre-loaded objc.md rule.
• When Resolve is called on a .m file, the Sniffer peeks the first line from disk. If it looks like Objective-C (#import, @interface, etc.), it returns the ObjC rule directly; otherwise it delegates to the inner resolver as usual.
• NewResolver wraps the result in a Sniffer before returning — callers (agent.go, scan/agent.go, delegate/rulegroup.go) never see the difference and need zero changes.

This keeps the sniffing logic in one new file (~50 lines), avoids new interfaces (ContentAwareResolver, ContentAwareDetailResolver), and leaves the composed resolver and all its callers untouched.

Would you be open to restructuring along these lines? Happy to pair on it if that would help.

Also, please rebase onto the latest main when you get a chance — there have been a few recent changes that may conflict.

[GenosseFlosse]

@lizhengfeng101 Yes, good point, I am on it. And sure, I can rebase.

[GenosseFlosse]

Resolver is unchanged again; ContentAwareResolver, ContentAwareDetailResolver and the content parameter are all gone. Logic now is in one new file, internal/config/rules/sniffer.go.

internal/delegate/rulegroup.go (and its test) and internal/agent/coverage_test.go are back to untouched.

Production code outside internal/config/rules is now 46 changed lines across 7 files, six of them one-liners.

Three notes on the implementation:

1. The wrapper sits on the system layer, not the outermost resolver. Wrapping the composed resolver would break rule priority: composedResolver checks user layers first and calls back into the system layer for merge_system_rule (system_rules.go:493). An outermost sniffer would short-circuit any .m file to objc.md and discard a user's own .opencodereview/rule.json entry. So composedResolver.system became a small unexported interface and the sniffer wraps that — still fully inside the package, callers unaffected. Two tests verify this.
2. It forwards ResolveDetail and CanonicalConfig. rules_cmd.go:54 type-asserts DetailResolver (without it, ocr rules check fails outright), and agent.go:855 asserts CanonicalConfig() []string — without forwarding, rule_config_sha256 would silently stop reflecting rule config. The forwarded CanonicalConfig folds in the objc rule text so editing objc.md still invalidates the hash.
3. Content is read at the reviewed ref, not the disk file from working tree. git show : for range/commit mode (mirroring finalizeDiff), working tree for scan/workspace/rules check. Defaulting to matlab.md on any read failure. NewResolver takes a ResolverOptions{Ref, Runner}. This keeps the decorator's simplicity without the correctness loss a disk read would introduce on refs that aren't checked out — there's a test for exactly that case. Cost is one extra git show per .m file, since finalizeDiff already fetched that blob; happy to drop it for a plain disk read if you'd rather trade that back.

Also fixed a bug the new tests surfaced: callers passed strings.ToLower(path), which was harmless for glob matching but broke content reads for mixed-case paths. The resolver already lowercases internally, so I removed it at the call sites.

Coverage went up — rules 85.8% → 88.2%, delegate 96.4% → 100%. make check and the full suite pass.

One open question: I used a ResolverOptions struct rather than two more positional params on NewResolver. Happy to switch if you prefer.