Skip to content

feat(allowlist): add Mojo support - #977

Open
wakqasahmed wants to merge 6 commits into
alibaba:mainfrom
wakqasahmed:feat/issue-470-mojo-support
Open

feat(allowlist): add Mojo support#977
wakqasahmed wants to merge 6 commits into
alibaba:mainfrom
wakqasahmed:feat/issue-470-mojo-support

Conversation

@wakqasahmed

Copy link
Copy Markdown
Contributor

Summary

  • Add .mojo and .🔥 to the supported file types allowlist
  • No default exclude pattern added — Mojo tooling has not settled on a conventional test-directory layout yet, so allowed_ext_test.go documents this explicitly (matching the existing Prisma/Shader precedent)
  • Add internal/config/rules/rule_docs/mojo.md, a Mojo review rule doc covering ownership/borrowing conventions (borrowed/inout/owned argument mismatches, unintended transfers with ^, __copyinit__/__moveinit__/__del__ correctness), value semantics and struct design, unsafe pointer and C/Python FFI interop, the PythonObject interop boundary, parallelize/vectorize concurrency hazards, Mojo-specific performance anti-patterns, and security-sensitive raw-pointer/deserialization concerns
  • Register **/*.{mojo,🔥} -> mojo.md in system_rules.json
  • Add test coverage: allowed-extension matching (both extensions, including the fire-emoji extension), non-exclusion of Mojo files, and rule resolution

Part of the language-allowlist expansion tracked in #470.

Closes #976

Test plan

  • go test ./internal/config/allowlist/... -run 'TestIsAllowedExt|TestIsExcludedPath' -v — PASS
  • go test ./internal/config/rules/... -run TestResolve_DefaultRules -v — PASS
  • gofmt -s -l . — clean
  • go vet ./internal/config/... — clean
  • go run scripts/verify-english-only.go — clean (423 files scanned, no unapproved non-English text)

Add .mojo and .🔥 to the supported file types allowlist, a mojo.md
review rule doc covering ownership/borrowing (borrowed/inout/owned,
transfer semantics, copyinit/moveinit/del), unsafe pointer and FFI
interop, the PythonObject boundary, parallelize/vectorize concurrency
hazards, and Mojo-specific performance anti-patterns, and register
the glob-to-rule mapping in system_rules.json. No conventional
test-file exclusion pattern exists for Mojo yet.

Part of the language-allowlist expansion tracked in alibaba#470.
@github-actions

Copy link
Copy Markdown
Contributor

OpenCodeReview: Review complete: 0 finding(s) across 2 selected item(s).

@wu21-web wu21-web left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Site documentation updates missing, update at pages/src/content/docs/<locale>/review-rules.md.

@wakqasahmed

Copy link
Copy Markdown
Contributor Author

@wu21-web Thanks for the catch — docs added in the latest commit. Updated pages/src/content/docs/{en,ja,ru,zh}/review-rules.md with a row for the new **/*.{mojo,🔥}mojo.md mapping, mirroring the format used for the Elm entry (#978).

Note: the ja/ru/zh translations are my best effort — please have a native speaker double-check the phrasing before merge.

@wu21-web

Copy link
Copy Markdown
Contributor

@wakqasahmed Same for here, merge conflicts.

…upport

# Conflicts:
#	internal/config/allowlist/allowed_ext_test.go
#	internal/config/allowlist/supported_file_types.json
#	internal/config/rules/system_rules.json
#	internal/config/rules/system_rules_test.go
#	pages/src/content/docs/en/review-rules.md
#	pages/src/content/docs/ja/review-rules.md
#	pages/src/content/docs/ru/review-rules.md
#	pages/src/content/docs/zh/review-rules.md
@wakqasahmed

Copy link
Copy Markdown
Contributor Author

@wu21-web Resolved — rebased onto latest main and merged, fixing the conflicts in supported_file_types.json, system_rules.json, the two allowlist/rules test files, and all four locale review-rules.md docs (they were conflicting with the recently-merged jsonnet/zig/thrift/capnp entries). Should be clean to review now.

@wakqasahmed

Copy link
Copy Markdown
Contributor Author

Hi @wu21-web — just checking in on this one, it's been about 4 days with CI green and mergeable. Is there anything needed from our side to help move it forward, or any rough idea on timing? No rush, just following up. Thanks for the review!

@wu21-web

Copy link
Copy Markdown
Contributor

@wakqasahmed Merge conflicts, please rebase branch main.

@wakqasahmed

Copy link
Copy Markdown
Contributor Author

@wu21-web Rebased — merged latest main and resolved the conflicts in supported_file_types.json, system_rules.json, and allowed_ext_test.go (they were colliding with the just-merged Solidity/Vyper allowlist entries). Both the mojo and sol/vy entries are preserved. Should be clean now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(allowlist): add Mojo support

2 participants