From 3d0c5f1e9b347ec157ceae42d8b51edfaa56d7a4 Mon Sep 17 00:00:00 2001 From: Brett Zamir Date: Fri, 3 Oct 2025 20:43:27 -0700 Subject: [PATCH] fix(`valid-types`): parse distinctly for names vs. namepaths Also: - chore: update jsdocccomment and jsdoc-type-pratt-parser - chore: avoid `eval` in build script --- docs/rules/valid-types.md | 13 +++++++++++- package.json | 6 +++--- pnpm-lock.yaml | 32 ++++++++++++++-------------- src/bin/generateOptions.js | 12 ++++++----- src/iterateJsdoc.js | 9 ++++++-- src/jsdocUtils.js | 33 +++++++++++++++++++++++++++++ src/rules/validTypes.js | 31 +++++++++++++++++++++++---- test/rules/assertions/validTypes.js | 33 ++++++++++++++++++++++++++++- 8 files changed, 137 insertions(+), 32 deletions(-) diff --git a/docs/rules/valid-types.md b/docs/rules/valid-types.md index 8dda9c915..e9f481411 100644 --- a/docs/rules/valid-types.md +++ b/docs/rules/valid-types.md @@ -233,7 +233,18 @@ function quux() { /** * @typedef {string} UserStr%ng */ -// Message: Syntax error in namepath: UserStr%ng +// Message: Syntax error in name: UserStr%ng + +/** + * @typedef {string} module:abc/def + */ +// Message: Syntax error in name: module:abc/def + +/** + * @typedef {string} module:abc/def + */ +// Settings: {"jsdoc":{"mode":"permissive"}} +// Message: Syntax error in name: module:abc/def /** * @this diff --git a/package.json b/package.json index 482bc4092..98df67e85 100644 --- a/package.json +++ b/package.json @@ -5,7 +5,7 @@ "url": "http://gajus.com" }, "dependencies": { - "@es-joy/jsdoccomment": "~0.67.2", + "@es-joy/jsdoccomment": "~0.68.1", "are-docs-informative": "^0.0.2", "comment-parser": "1.4.1", "debug": "^4.4.3", @@ -27,7 +27,7 @@ "@babel/plugin-syntax-class-properties": "^7.12.13", "@babel/plugin-transform-flow-strip-types": "^7.27.1", "@babel/preset-env": "^7.28.3", - "@es-joy/escodegen": "^4.0.3", + "@es-joy/escodegen": "^4.2.0", "@es-joy/jsdoc-eslint-parser": "^0.24.0", "@eslint/core": "^0.16.0", "@hkdobrev/run-if-changed": "^0.6.3", @@ -58,7 +58,7 @@ "glob": "^11.0.3", "globals": "^16.4.0", "husky": "^9.1.7", - "jsdoc-type-pratt-parser": "^6.3.2", + "jsdoc-type-pratt-parser": "^6.3.3", "json-schema": "^0.4.0", "json-schema-to-typescript": "^15.0.4", "lint-staged": "^16.2.3", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index bfa523945..bf64e6460 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -12,8 +12,8 @@ importers: .: dependencies: '@es-joy/jsdoccomment': - specifier: ~0.67.2 - version: 0.67.2 + specifier: ~0.68.1 + version: 0.68.1 are-docs-informative: specifier: ^0.0.2 version: 0.0.2 @@ -70,8 +70,8 @@ importers: specifier: ^7.28.3 version: 7.28.3(@babel/core@7.28.4) '@es-joy/escodegen': - specifier: ^4.0.3 - version: 4.0.3 + specifier: ^4.2.0 + version: 4.2.0 '@es-joy/jsdoc-eslint-parser': specifier: ^0.24.0 version: 0.24.0(jiti@2.6.1) @@ -163,8 +163,8 @@ importers: specifier: ^9.1.7 version: 9.1.7 jsdoc-type-pratt-parser: - specifier: ^6.3.2 - version: 6.3.2 + specifier: ^6.3.3 + version: 6.3.3 json-schema: specifier: ^0.4.0 version: 0.4.0 @@ -775,8 +775,8 @@ packages: resolution: {integrity: sha512-CsFmA3u3c2QoLDTfEpGr4t25fjMU31nyvse7IzWTvb0ZycuPjMjb0fjlheh+PbhBYb9YLugnT2uY6Mwcg1o+Zg==} engines: {node: '>=18.0.0'} - '@es-joy/escodegen@4.0.3': - resolution: {integrity: sha512-cX8TsWfGHIyVpLK74fY05RSZ/s6NsIPo18C8oD5hR49TXajotOR/fk59eI08sWdnUsrXkNeb97ShUJvu3osAZg==} + '@es-joy/escodegen@4.2.0': + resolution: {integrity: sha512-jWqqS3StW6vGwJeT8yu6alS4nipOBi3kD/pR63+9hG2NfGafCJ4viDME15mVf+fyupyptcUJDXfv63gFF5Gzfg==} engines: {node: '>=6.0'} hasBin: true @@ -796,8 +796,8 @@ packages: resolution: {integrity: sha512-yWi6sm7INEwnfS7IJvE0dU+RTrwzLPFcY7e7eGpu/l5Q9lWfQ2ROwZ0qVnc242jw2TUPsfHX3XMIISkGBv57RQ==} engines: {node: '>=20.11.0'} - '@es-joy/jsdoccomment@0.67.2': - resolution: {integrity: sha512-roH+w/W97L5+gXv9c9n2RrAS/KwIn/10fi7SG+gB9ZOs3pDhhpnnq8lObKCEUTWABL4JTpDXeThU3Q64VqT9UQ==} + '@es-joy/jsdoccomment@0.68.1': + resolution: {integrity: sha512-wJJXnfG2Pq7ZC8IeOupkkAVtEH6OYy1uVxRTeshXDQfSNsZneS7FUQlNf710osZ5Yz/b5ev9xChvybTT7CM63g==} engines: {node: '>=20.11.0'} '@eslint-community/eslint-utils@4.9.0': @@ -3367,8 +3367,8 @@ packages: resolution: {integrity: sha512-TYzkACp/wPvDJLRY7qpHXtrhgwoAaojIOnLaaVNi+AbPU2u1kkjfKd9hXXTq0qSAGsyYXvwUXt99h9I5iCmjjw==} engines: {node: '>=12.0.0'} - jsdoc-type-pratt-parser@6.3.2: - resolution: {integrity: sha512-u5bv2XxJjwoSL4jyQ+QIrFmo7tFollOnE+BDQbxWJv/m7/kSiZ/P2+E2Y2TmYmST8EeavzFzrGoG4VstTLP5bA==} + jsdoc-type-pratt-parser@6.3.3: + resolution: {integrity: sha512-N1HQK15ZXdwgmXsALjUWW9Cwyg1BQS5hfP8KzDkbR4mjb+Ep8Uxcfwz+OU1tsNCRg99rk62d8GMNdG8Qz4k+Gg==} engines: {node: '>=20.0.0'} jsdom@6.5.1: @@ -5876,7 +5876,7 @@ snapshots: '@whatwg-node/promise-helpers': 1.3.2 tslib: 2.8.1 - '@es-joy/escodegen@4.0.3': + '@es-joy/escodegen@4.2.0': dependencies: '@es-joy/estraverse': 7.1.1 '@es-joy/jsdoccomment': 0.62.0 @@ -5917,13 +5917,13 @@ snapshots: esquery: 1.6.0 jsdoc-type-pratt-parser: 5.9.2 - '@es-joy/jsdoccomment@0.67.2': + '@es-joy/jsdoccomment@0.68.1': dependencies: '@types/estree': 1.0.8 '@typescript-eslint/types': 8.45.0 comment-parser: 1.4.1 esquery: 1.6.0 - jsdoc-type-pratt-parser: 6.3.2 + jsdoc-type-pratt-parser: 6.3.3 '@eslint-community/eslint-utils@4.9.0(eslint@9.37.0(jiti@2.6.1))': dependencies: @@ -8940,7 +8940,7 @@ snapshots: jsdoc-type-pratt-parser@5.9.2: {} - jsdoc-type-pratt-parser@6.3.2: {} + jsdoc-type-pratt-parser@6.3.3: {} jsdom@6.5.1: dependencies: diff --git a/src/bin/generateOptions.js b/src/bin/generateOptions.js index e8eac19bd..8db5c2c49 100644 --- a/src/bin/generateOptions.js +++ b/src/bin/generateOptions.js @@ -36,11 +36,13 @@ for (const file of dirContents) { ' Property[key.name="meta"] Property[key.name="schema"]', ); if (results[0]?.value) { - const schema = generate(results[0]?.value); - - // eslint-disable-next-line no-eval -- Need some parser - const json = eval('JSON.stringify(' + schema + ', null, 2)'); - const parsed = JSON.parse(json); + const schema = generate(results[0]?.value, { + format: { + json: true, + quotes: 'double', + }, + }); + const parsed = JSON.parse(schema); let initial = ''; if (Array.isArray(parsed)) { diff --git a/src/iterateJsdoc.js b/src/iterateJsdoc.js index 7064f9d9a..ff5eec69f 100644 --- a/src/iterateJsdoc.js +++ b/src/iterateJsdoc.js @@ -96,6 +96,7 @@ import esquery from 'esquery'; * isNamepathReferencingTag: IsNamepathX, * isNamepathOrUrlReferencingTag: IsNamepathX, * tagMightHaveNameOrNamepath: IsNamepathX, + * tagMightHaveName: IsNamepathX * }} BasicUtils */ @@ -530,6 +531,8 @@ import esquery from 'esquery'; * isNamepathReferencingTag: IsNamepathX, * isNamepathOrUrlReferencingTag: IsNamepathX, * tagMightHaveNameOrNamepath: IsNamepathX, + * tagMightHaveName: IsNamepathX, + * tagMightHaveNamepath: IsNamepathX, * getTagStructureForMode: GetTagStructureForMode, * mayBeUndefinedTypeTag: MayBeUndefinedTypeTag, * hasValueOrExecutorHasNonEmptyResolveValue: HasValueOrExecutorHasNonEmptyResolveValue, @@ -618,14 +621,16 @@ const getBasicUtils = (context, { 'isNamepathReferencingTag', 'isNamepathOrUrlReferencingTag', 'tagMightHaveNameOrNamepath', + 'tagMightHaveName', + 'tagMightHaveNamepath', ]) { /** @type {IsNamepathX} */ utils[ - /** @type {"isNameOrNamepathDefiningTag"|"isNamepathReferencingTag"|"isNamepathOrUrlReferencingTag"|"tagMightHaveNameOrNamepath"} */ ( + /** @type {"isNameOrNamepathDefiningTag"|"isNamepathReferencingTag"|"isNamepathOrUrlReferencingTag"|"tagMightHaveNameOrNamepath"|"tagMightHaveName"} */ ( method )] = (tagName) => { return jsdocUtils[ - /** @type {"isNameOrNamepathDefiningTag"|"isNamepathReferencingTag"|"isNamepathOrUrlReferencingTag"|"tagMightHaveNameOrNamepath"} */ + /** @type {"isNameOrNamepathDefiningTag"|"isNamepathReferencingTag"|"isNamepathOrUrlReferencingTag"|"tagMightHaveNameOrNamepath"|"tagMightHaveName"} */ (method) ](tagName); }; diff --git a/src/jsdocUtils.js b/src/jsdocUtils.js index 4a6554fa3..ba26ede3e 100644 --- a/src/jsdocUtils.js +++ b/src/jsdocUtils.js @@ -1145,6 +1145,37 @@ const tagMightHaveNameOrNamepath = (tag, tagMap = tagStructure) => { namepathTypes.has(/** @type {string} */ (nampathRole)); }; +/** + * @param {string} tag + * @param {import('./getDefaultTagStructureForMode.js').TagStructure} tagMap + * @returns {boolean} + */ +const tagMightHaveNamepath = (tag, tagMap = tagStructure) => { + const tagStruct = ensureMap(tagMap, tag); + + const nampathRole = tagStruct.get('namepathRole'); + + return nampathRole !== false && + [ + 'namepath-defining', + 'namepath-referencing', + ].includes(/** @type {string} */ (nampathRole)); +}; + +/** + * @param {string} tag + * @param {import('./getDefaultTagStructureForMode.js').TagStructure} tagMap + * @returns {boolean} + */ +const tagMightHaveName = (tag, tagMap = tagStructure) => { + const tagStruct = ensureMap(tagMap, tag); + + const nampathRole = tagStruct.get('namepathRole'); + + return nampathRole !== false && + nampathRole === 'name-defining'; +}; + /** * @param {string} tag * @param {import('./getDefaultTagStructureForMode.js').TagStructure} tagMap @@ -1907,7 +1938,9 @@ export { setTagStructure, strictNativeTypes, tagMightHaveEitherTypeOrNamePosition, + tagMightHaveName, tagMightHaveNameOrNamepath, + tagMightHaveNamepath, tagMightHaveNamePosition, tagMightHaveTypePosition, tagMissingRequiredTypeOrNamepath, diff --git a/src/rules/validTypes.js b/src/rules/validTypes.js index 7a9e32703..5e44ec420 100644 --- a/src/rules/validTypes.js +++ b/src/rules/validTypes.js @@ -1,7 +1,7 @@ import iterateJsdoc from '../iterateJsdoc.js'; import { parse, - // parseName, + parseName, parseNamePath, traverse, tryParse, @@ -99,6 +99,23 @@ const tryParsePathIgnoreError = (path, mode) => { return false; }; +/** + * @param {string} name + * @param {import('jsdoc-type-pratt-parser').ParseMode|"permissive"} mode + * @returns {boolean} + */ +const tryParseNameIgnoreError = (name, mode) => { + try { + parseName(name, mode === 'permissive' ? 'jsdoc' : mode); + + return true; + } catch { + // Keep the original error for including the whole type + } + + return false; +}; + // eslint-disable-next-line complexity export default iterateJsdoc(({ context, @@ -350,12 +367,12 @@ export default iterateJsdoc(({ } // VALID NAME/NAMEPATH - const hasNameOrNamepathPosition = ( + const hasNamepathPosition = ( tagMustHaveNamePosition !== false || - utils.tagMightHaveNameOrNamepath(tag.tag) + utils.tagMightHaveNamepath(tag.tag) ) && Boolean(tag.name); - if (hasNameOrNamepathPosition) { + if (hasNamepathPosition) { if (mode !== 'jsdoc' && tag.tag === 'template') { if (!tryParsePathIgnoreError( // May be an issue with the commas of @@ -373,6 +390,12 @@ export default iterateJsdoc(({ } } + const hasNamePosition = utils.tagMightHaveName(tag.tag) && + Boolean(tag.name); + if (hasNamePosition && !tryParseNameIgnoreError(tag.name, mode)) { + report(`Syntax error in name: ${tag.name}`, null, tag); + } + for (const inlineTag of tag.inlineTags) { if (inlineTags.has(inlineTag.tag) && !inlineTag.text && !inlineTag.namepathOrURL) { report(`Inline tag "${inlineTag.tag}" missing content`, null, tag); diff --git a/test/rules/assertions/validTypes.js b/test/rules/assertions/validTypes.js index 76db1b39b..349260fef 100644 --- a/test/rules/assertions/validTypes.js +++ b/test/rules/assertions/validTypes.js @@ -233,10 +233,41 @@ export default /** @type {import('../index.js').TestCases} */ ({ errors: [ { line: 3, - message: 'Syntax error in namepath: UserStr%ng', + message: 'Syntax error in name: UserStr%ng', }, ], }, + { + code: ` + /** + * @typedef {string} module:abc/def + */ + `, + errors: [ + { + line: 3, + message: 'Syntax error in name: module:abc/def', + }, + ], + }, + { + code: ` + /** + * @typedef {string} module:abc/def + */ + `, + errors: [ + { + line: 3, + message: 'Syntax error in name: module:abc/def', + }, + ], + settings: { + jsdoc: { + mode: 'permissive', + }, + }, + }, { code: ` /**