From 89f9112cdd2a709a27edafcb758f8290f7b4a51b Mon Sep 17 00:00:00 2001 From: Kouji Takao Date: Mon, 16 Mar 2026 09:12:25 +0900 Subject: [PATCH] style: fix all JSDoc lint warnings and enforce zero-warning policy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix 23 JSDoc warnings across 11 files in scratch-gui: - Add missing @param and @returns declarations - Remove @param that doesn't match function signature - Escape inline JSDoc tags (@ruby → `@ruby`, @a → `@a`) - Fix type casing (Object → object) - Add missing JSDoc comments for exported functions - Remove extra blank line after block description Enforce --max-warnings 0 in scratch-gui's lint script so warnings are treated as errors in both local development and CI. Update CLAUDE.md and .claude/rules/code-style.md to document the zero-warning lint policy. Closes #316 Co-Authored-By: Claude Opus 4.6 (1M context) --- .claude/rules/code-style.md | 5 +++-- CLAUDE.md | 2 ++ packages/scratch-gui/package.json | 2 +- .../src/containers/ruby-downloader.jsx | 1 - .../src/containers/ruby-tab/furigana-renderer.js | 2 ++ .../scratch-gui/src/lib/furigana-annotator.js | 15 ++++++++++++--- .../scratch-gui/src/lib/furigana-call-helpers.js | 2 ++ packages/scratch-gui/src/lib/module-sync.js | 1 - .../scratch-gui/src/lib/ruby-generator/data.js | 4 +++- .../src/lib/ruby-generator/face_sensing.js | 4 ++++ .../src/lib/ruby-to-blocks-converter/looks.js | 2 ++ .../lib/ruby-to-blocks-converter/node-utils.js | 2 +- .../src/lib/ruby-to-blocks-converter/variables.js | 10 ++++++---- packages/scratch-gui/src/lib/version-checker.js | 2 +- 14 files changed, 39 insertions(+), 15 deletions(-) diff --git a/.claude/rules/code-style.md b/.claude/rules/code-style.md index b4f96735d6c..f6a74258b17 100644 --- a/.claude/rules/code-style.md +++ b/.claude/rules/code-style.md @@ -5,8 +5,9 @@ ### Linting - **Linter**: ESLint with `eslint-config-scratch` -- **MUST pass before committing**: Always run `npm run lint` before creating commits -- Fix linting errors automatically when possible: `npm run lint -- --fix` +- **MUST pass with zero errors AND zero warnings before committing**: Always run `npm run lint` before creating commits. The `scratch-gui` package enforces `--max-warnings 0`. +- Fix linting errors and warnings automatically when possible: `npm run lint -- --fix` +- **JSDoc warnings count as failures**: Missing `@param`, `@returns`, unescaped inline tags (`@ruby` → `` `@ruby` ``), and incorrect types (`Object` → `object`) must all be fixed. ### Code Style Guidelines diff --git a/CLAUDE.md b/CLAUDE.md index 584376ede0d..ee815abdb9f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -129,6 +129,8 @@ docker compose run --rm app bash -c "cd packages/scratch-gui && npm exec jest te docker compose run --rm app npm run lint ``` +**IMPORTANT**: Lint must pass with **zero errors AND zero warnings**. The `scratch-gui` package uses `--max-warnings 0` to enforce this. Fix all warnings (including JSDoc issues) before committing. + ### Clean Remove build artifacts from all packages: diff --git a/packages/scratch-gui/package.json b/packages/scratch-gui/package.json index 967a85c6a17..1d94e2ee51a 100644 --- a/packages/scratch-gui/package.json +++ b/packages/scratch-gui/package.json @@ -63,7 +63,7 @@ "start": "webpack serve", "start:dist": "npx serve dist -p 8601", "test": "npm run test:lint && npm run test:unit && npm run test:integration", - "lint": "eslint", + "lint": "eslint --max-warnings 0", "test:integration": "cross-env JEST_JUNIT_OUTPUT_NAME=integration-tests-results.xml jest --maxWorkers=4 test[\\\\/]integration", "test:lint": "eslint", "test:unit": "cross-env JEST_JUNIT_OUTPUT_NAME=unit-tests-results.xml jest test[\\\\/]unit", diff --git a/packages/scratch-gui/src/containers/ruby-downloader.jsx b/packages/scratch-gui/src/containers/ruby-downloader.jsx index 9e0ac12b004..3a745bd2ffa 100644 --- a/packages/scratch-gui/src/containers/ruby-downloader.jsx +++ b/packages/scratch-gui/src/containers/ruby-downloader.jsx @@ -26,7 +26,6 @@ class RubyDownloader extends React.Component { /** * Validate Ruby code by converting to blocks. If conversion succeeds, * apply blocks to update sprite/stage state (round-trip). - * @param {boolean} converted - set to true after successful conversion * @returns {Promise} true if conversion succeeded or was unnecessary */ async validateAndConvert () { diff --git a/packages/scratch-gui/src/containers/ruby-tab/furigana-renderer.js b/packages/scratch-gui/src/containers/ruby-tab/furigana-renderer.js index 9543916f1bb..764affd4cf6 100644 --- a/packages/scratch-gui/src/containers/ruby-tab/furigana-renderer.js +++ b/packages/scratch-gui/src/containers/ruby-tab/furigana-renderer.js @@ -141,6 +141,8 @@ class FuriganaRenderer { /** * Measure text width using an offscreen canvas (no DOM insertion needed). * The canvas context is cached for performance. + * @param {string} text - The text to measure. + * @param {number} fontSize - The font size in pixels. */ _measureTextWidth (text, fontSize) { if (!this._measureCanvas) { diff --git a/packages/scratch-gui/src/lib/furigana-annotator.js b/packages/scratch-gui/src/lib/furigana-annotator.js index bfc470812fe..e3011b13725 100644 --- a/packages/scratch-gui/src/lib/furigana-annotator.js +++ b/packages/scratch-gui/src/lib/furigana-annotator.js @@ -49,7 +49,10 @@ class FuriganaAnnotator { // ---- Internal helpers ---- - /** Build UTF-8 byte-based line offsets and a byteToChar mapping. */ + /** + * Build UTF-8 byte-based line offsets and a byteToChar mapping. + * @param {string} source - The Ruby source code. + */ _buildMappings (source) { const encoder = new TextEncoder(); const bytes = encoder.encode(source); @@ -95,7 +98,10 @@ class FuriganaAnnotator { }; } - /** Location spanning from receiver start to messageLoc end (for self.xxx). */ + /** + * Location spanning from receiver start to messageLoc end (for self.xxx). + * @param {object} node - A Prism CallNode. + */ _receiverSpanLoc (node) { if (!node.receiver || !node.receiver.location || !node.messageLoc) return node.messageLoc; const recLoc = node.receiver.location; @@ -140,7 +146,10 @@ class FuriganaAnnotator { t === 'ConcatStringNode'; } - /** Dispatch to _handleXxxNode or fall back to walking children. */ + /** + * Dispatch to _handleXxxNode or fall back to walking children. + * @param {object} node - A Prism AST node. + */ _walkNode (node) { if (!node || typeof node !== 'object') return; const typeName = typeof node.toJSON === 'function' ? node.toJSON().type : null; diff --git a/packages/scratch-gui/src/lib/furigana-call-helpers.js b/packages/scratch-gui/src/lib/furigana-call-helpers.js index 7fdb5476e58..dbdedc529b5 100644 --- a/packages/scratch-gui/src/lib/furigana-call-helpers.js +++ b/packages/scratch-gui/src/lib/furigana-call-helpers.js @@ -126,6 +126,8 @@ const callHelpers = { /** * Check if a node's receiver is a predefined extension name. * Handles both LocalVariableReadNode and CallNode patterns. + * @param {object} node - A Prism CallNode. + * @param {string} extensionName - The extension name to match. */ _isPredefinedReceiver (node, extensionName) { if (!node.receiver) return false; diff --git a/packages/scratch-gui/src/lib/module-sync.js b/packages/scratch-gui/src/lib/module-sync.js index 6d1058babf2..5207d9c2c4e 100644 --- a/packages/scratch-gui/src/lib/module-sync.js +++ b/packages/scratch-gui/src/lib/module-sync.js @@ -86,7 +86,6 @@ export const replaceModuleCode = (code, moduleName, newModuleCode) => { /** * Sync module changes from a source target to all other targets that include the same modules. * Called after Ruby→Blocks conversion completes on the source target. - * * @param {object} vm - Scratch VM * @param {object} sourceTarget - The target whose code was just converted * @param {object} intl - react-intl instance for error translation diff --git a/packages/scratch-gui/src/lib/ruby-generator/data.js b/packages/scratch-gui/src/lib/ruby-generator/data.js index a68299574db..b4fe62080e7 100644 --- a/packages/scratch-gui/src/lib/ruby-generator/data.js +++ b/packages/scratch-gui/src/lib/ruby-generator/data.js @@ -147,7 +147,9 @@ export default function (Generator) { * Convert Scratch 1-indexed list index to Ruby 0-indexed array index. * For literal numbers, subtracts 1 directly. * For expressions, generates "(expr - 1)". - * Detects operator_add(x, 1) with @ruby:array:index_offset comment for round-trip. + * Detects operator_add(x, 1) with `@ruby`:array:index_offset comment for round-trip. + * @param {object} block - The Scratch block containing the index input. + * @returns {string} The Ruby array index expression. */ const getListIndex = function (block) { // Check for operator_add(x, 1) round-trip pattern diff --git a/packages/scratch-gui/src/lib/ruby-generator/face_sensing.js b/packages/scratch-gui/src/lib/ruby-generator/face_sensing.js index 24c6bf698fe..55c6265c866 100644 --- a/packages/scratch-gui/src/lib/ruby-generator/face_sensing.js +++ b/packages/scratch-gui/src/lib/ruby-generator/face_sensing.js @@ -16,6 +16,10 @@ const PartLabel = { 7: 'top_of_head' }; +/** + * Register face sensing block generators. + * @param {object} Generator - The Ruby code generator instance. + */ export default function (Generator) { Generator.faceSensing_goToPart = function (block) { const part = Generator.getFieldValue(block, 'PART', '2'); diff --git a/packages/scratch-gui/src/lib/ruby-to-blocks-converter/looks.js b/packages/scratch-gui/src/lib/ruby-to-blocks-converter/looks.js index 724de088639..feec35ba809 100644 --- a/packages/scratch-gui/src/lib/ruby-to-blocks-converter/looks.js +++ b/packages/scratch-gui/src/lib/ruby-to-blocks-converter/looks.js @@ -100,6 +100,8 @@ const validateBackdrop = function (converter, backdropName, args) { /** * Convert a symbol Primitive to its string name and collect it. * Returns the symbol name (without colon) or null if not a symbol. + * @param {object} converter - The Ruby-to-blocks converter instance. + * @param {object} arg - The argument to check for symbol type. */ const resolveSymbolArg = function (converter, arg) { if (converter._isPrimitive(arg) && arg.type === 'sym') { diff --git a/packages/scratch-gui/src/lib/ruby-to-blocks-converter/node-utils.js b/packages/scratch-gui/src/lib/ruby-to-blocks-converter/node-utils.js index b93e5a54179..109afdd2b1c 100644 --- a/packages/scratch-gui/src/lib/ruby-to-blocks-converter/node-utils.js +++ b/packages/scratch-gui/src/lib/ruby-to-blocks-converter/node-utils.js @@ -132,7 +132,7 @@ const NodeUtils = { /** * Get the string value of a symbol node. * Works for Prism SymbolNode instances ({unescaped: {value: '...'}}), - * Primitive('sym', value), and blocks with @ruby:symbol: comments. + * Primitive('sym', value), and blocks with `@ruby`:symbol: comments. * @param {object} node - A symbol node. * @returns {string|null} The symbol value, or null if not a symbol. */ diff --git a/packages/scratch-gui/src/lib/ruby-to-blocks-converter/variables.js b/packages/scratch-gui/src/lib/ruby-to-blocks-converter/variables.js index 7f211a011ce..fa6cef9bd0b 100644 --- a/packages/scratch-gui/src/lib/ruby-to-blocks-converter/variables.js +++ b/packages/scratch-gui/src/lib/ruby-to-blocks-converter/variables.js @@ -31,11 +31,11 @@ const VariablesConverter = { register: function (converter) { /** * Convert a data_variable block to data_listcontents for list operations. - * When $a (global) or @a (instance) is used with array methods like .push(), + * When $a (global) or `@a` (instance) is used with array methods like .push(), * the variable read creates a data_variable block, but list operations need * a data_listcontents block with LIST field. - * @param {Object} block - The receiver block to convert - * @returns {{block: Object|null, converted: boolean}} The converted block and whether conversion happened + * @param {object} block - The receiver block to convert + * @returns {{block: object|null, converted: boolean}} The converted block and whether conversion happened */ const convertToListBlock = function (block) { if (!converter._isBlock(block)) return {block: null, converted: false}; @@ -86,8 +86,10 @@ const VariablesConverter = { /** * Adjust a 0-indexed Ruby array index to 1-indexed Scratch list index. - * Always wraps in operator_add(index, 1) with @ruby:array:index comment + * Always wraps in operator_add(index, 1) with `@ruby`:array:index comment * to enable round-trip conversion. + * @param {object} index - The index block or value. + * @param {boolean} converted - Whether the receiver was converted to a list block. */ const adjustIndex = function (index, converted) { if (!converted) return index; diff --git a/packages/scratch-gui/src/lib/version-checker.js b/packages/scratch-gui/src/lib/version-checker.js index fd1eac82c9a..f0577f8e8ed 100644 --- a/packages/scratch-gui/src/lib/version-checker.js +++ b/packages/scratch-gui/src/lib/version-checker.js @@ -11,7 +11,7 @@ const DEFAULT_INTERVAL_MS = 60 * 60 * 1000; // 1 hour * @param {Function} options.onUpdateAvailable - Called when a newer version is found. * @param {number} [options.initialDelayMs] - Delay before first check (default 5s). * @param {number} [options.intervalMs] - Interval between checks (default 1h). - * @returns {{start: Function, stop: Function, check: Function}} + * @returns {{start: Function, stop: Function, check: Function}} Version checker controller. */ const createVersionChecker = ({ currentCommitId,