feat(allowlist): add matlab support - #574
Conversation
|
✅ OpenCodeReview: No comments generated. Looks good to me. |
lizhengfeng101
left a comment
There was a problem hiding this comment.
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
-
Path-based heuristics — put ObjC-specific glob patterns (e.g.
**/{ios,macos,tvos,watchos,Pods,Classes}/**/*.m) before**/*.minsystem_rules.json, leveraging "first match wins" order. Downside: incomplete coverage, maintenance burden. -
Content-based detection — sniff the first line of the file (
function→ MATLAB,#import/#include→ ObjC). Downside: requires changes to theResolveinterface which currently only takes a path. -
Accept the limitation — document that
.mdefaults to MATLAB and ObjC projects should use a project-level.opencodereview/rule.jsonto 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!
Description
Type of Change
How Has This Been Tested?
make testpasses locallyChecklist
go fmt,go vet)Related Issues