chore: add markdown docs templates #6030
Conversation
|
WalkthroughThis change introduces a new Markdown-based documentation generation system for Solidity contracts. Five new files are added: a solidity-docgen configuration file pointing to Markdown templates and output directories, Handlebars templates for page and contract documentation structures, a helpers module providing template utilities for content processing and linking, and a properties module with AST node analysis functions. The hardhat configuration is updated to reference the new Markdown config instead of the previous configuration, enabling the generation of API documentation in Markdown format. Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
docs/templates-md/helpers.js (5)
29-29: Add null safety for params.The
params?.map()uses optional chaining which is good, but the function could still returnundefinedif params is undefined, instead of an empty string or safe default.Consider this adjustment:
-module.exports.names = params => params?.map(p => p.name).join(', '); +module.exports.names = params => params?.map(p => p.name).join(', ') || '';
63-65: Add null safety for params.Similar to the
nameshelper,typed-paramscould returnundefinedinstead of a safe default when params is undefined.Consider this adjustment:
-module.exports['typed-params'] = params => { - return params?.map(p => `${p.type}${p.indexed ? ' indexed' : ''}${p.name ? ' ' + p.name : ''}`).join(', '); -}; +module.exports['typed-params'] = params => { + return params?.map(p => `${p.type}${p.indexed ? ' indexed' : ''}${p.name ? ' ' + p.name : ''}`).join(', ') || ''; +};
67-72: Consider handling other falsy values.The function only checks for
undefinedbut not other falsy values likenullor empty string, which could also cause the regex to fail.const slug = (module.exports.slug = str => { - if (str === undefined) { + if (str == null || str === '') { throw new Error('Missing argument'); } return str.replace(/\W/g, '-'); });
77-124: Refactor duplicated cache setup.The cache initialization logic (lines 80-84 and 115-119) is duplicated. Extract it into a helper function.
+function getOrCreateCache(items, currentPage) { + let cache = linksCache.get(items); + if (!cache) { + cache = new Map(); + linksCache.set(items, cache); + } + return cache; +} + function getAllLinks(items, currentPage) { if (currentPage) { const cacheKey = currentPage; - let cache = linksCache.get(items); - if (!cache) { - cache = new Map(); - linksCache.set(items, cache); - } + const cache = getOrCreateCache(items, currentPage); if (cache.has(cacheKey)) { return cache.get(cacheKey); } } // ... rest of function ... if (currentPage) { - let cache = linksCache.get(items); - if (!cache) { - cache = new Map(); - linksCache.set(items, cache); - } + const cache = getOrCreateCache(items, currentPage); cache.set(currentPage, res); }
338-344: Use modern file system API for directory removal.Line 341 uses the deprecated
fs.rmdirSync, which may fail or be removed in future Node.js versions. The modern approach isfs.rmSyncwith therecursiveoption.// Cleanup temp files try { fs.unlinkSync(tempAdocFile); fs.unlinkSync(tempMdFile); - fs.rmdirSync(tempDir); + fs.rmSync(tempDir, { recursive: true }); } catch (cleanupError) { console.warn('Warning: Could not clean up temp files:', cleanupError.message); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
docs/config-md.js(1 hunks)docs/templates-md/contract.hbs(1 hunks)docs/templates-md/helpers.js(1 hunks)docs/templates-md/page.hbs(1 hunks)docs/templates-md/properties.js(1 hunks)hardhat.config.js(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Amxx
Repo: OpenZeppelin/openzeppelin-contracts PR: 5904
File: contracts/crosschain/ERC7786Recipient.sol:3-3
Timestamp: 2025-09-04T09:13:21.278Z
Learning: In OpenZeppelin contracts, hardhat.config.js uses yargs to configure the Solidity compiler version dynamically via command line arguments, with a default of '0.8.27' set on line 21 and the solidity configuration using argv.compile around lines 76-87, rather than hardcoded version strings.
📚 Learning: 2025-08-29T13:17:07.068Z
Learnt from: Amxx
Repo: OpenZeppelin/openzeppelin-contracts PR: 5904
File: contracts/crosschain/ERC7786Recipient.sol:22-31
Timestamp: 2025-08-29T13:17:07.068Z
Learning: In OpenZeppelin contracts, virtual functions that implement interfaces should use `public` visibility instead of `external`, even for calldata-heavy functions. This is because external functions cannot be called with `super` in derived contracts, which would limit extensibility. OpenZeppelin has strong guidelines to keep all public-facing virtual functions as `public` to maintain inheritance flexibility.
Applied to files:
docs/templates-md/contract.hbs
🧬 Code graph analysis (3)
hardhat.config.js (2)
docs/templates-md/helpers.js (2)
require(1-1)require(4-4)docs/templates-md/properties.js (1)
require(1-1)
docs/templates-md/helpers.js (1)
docs/templates-md/properties.js (5)
require(1-1)require(2-2)item(83-83)signature(11-11)signature(24-24)
docs/templates-md/properties.js (1)
docs/templates-md/helpers.js (2)
slug(67-72)i(139-139)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Redirect rules - solidity-contracts
- GitHub Check: Header rules - solidity-contracts
- GitHub Check: Pages changed - solidity-contracts
- GitHub Check: halmos
- GitHub Check: coverage
- GitHub Check: slither
- GitHub Check: tests-upgradeable
- GitHub Check: tests-foundry
- GitHub Check: tests
🔇 Additional comments (12)
docs/templates-md/page.hbs (1)
1-13: Page template structure looks good.YAML front matter plus the contract iteration read clean and match the helper usage—no changes needed.
hardhat.config.js (1)
124-124: Docgen config switch is consistent.Repointing to
./docs/config-mdcleanly hooks in the new Markdown generator without disturbing the rest of the setup.docs/templates-md/properties.js (1)
1-91: Metadata helpers look solid.The inheritance filtering and function/event/error aggregation give the templates the data they need with sensible guards; no adjustments required.
docs/templates-md/helpers.js (9)
1-9: LGTM!Module imports and version export are clean and appropriate for the templating system.
11-27: LGTM!README processing logic includes appropriate error handling and fallback behavior.
49-50: LGTM!Simple comparison helpers are correctly implemented with appropriate null safety.
126-158: LGTM!The relative path calculation logic correctly handles different navigation scenarios (same page, sibling pages, nested pages).
225-249: LGTM!Reference resolution with direct matching and fuzzy fallback is well-implemented.
251-282: LGTM!Multi-strategy matching logic provides robust fallback behavior for finding the best link match.
284-299: LGTM!HTML entity decoding and URL pattern conversion are appropriate for the markdown generation use case.
353-411: LGTM!Comprehensive callout processing handles multiple admonition formats and edge cases appropriately. The cleanup logic for "better viewed at" notices is a nice touch.
413-439: LGTM!Metadata helpers provide sensible defaults and proper content processing integration.
1 participant
This PR adds the
templates-mddirectory and theconfig-md.jsfile to thedocsdirectory. These are similar to the older predecessors but use markdown as the output and are formatted specifically forOpenZeppelin/docs.By adding these files we can generate API documentation for the new docs. See guide here
PR Checklist
npx changeset add)