Docs-tools: Replace doctrine with jsdoc-type-pratt-parser

code/lib/docs-tools/package.json

@@ -47,9 +47,8 @@
     "@storybook/core-common": "workspace:*",
     "@storybook/preview-api": "workspace:*",
     "@storybook/types": "workspace:*",
-    "@types/doctrine": "^0.0.3",
-    "assert": "^2.1.0",
-    "doctrine": "^3.0.0",
+    "comment-parser": "^1.4.1",
+    "jsdoc-type-pratt-parser": "^4.0.0",
     "lodash": "^4.17.21"
   },
   "devDependencies": {

code/lib/docs-tools/src/argTypes/jsdocParser.test.ts

@@ -10,7 +10,7 @@ describe('parseJsDoc', () => {
     expect(extractedTags).toBeUndefined();
   });
 
-  it('should set includesJsDocto to false when the value dont contains JSDoc', () => {
+  it('should set includesJsDoc to false when the value dont contains JSDoc', () => {
     const { includesJsDoc, description, extractedTags } = parseJsDoc('Hey!');
 
     expect(includesJsDoc).toBeFalsy();
@@ -68,7 +68,7 @@ describe('parseJsDoc', () => {
       expect(extractedTags?.params).not.toBeNull();
       expect(extractedTags?.params?.[0].name).toBe('event');
       expect(extractedTags?.params?.[0].type).not.toBeNull();
-      expect(extractedTags?.params?.[0].type.name).toBe('SyntheticEvent');
+      expect(extractedTags?.params?.[0].type.value).toBe('SyntheticEvent');
       expect(extractedTags?.params?.[0].description).toBeNull();
     });
 
@@ -78,7 +78,7 @@ describe('parseJsDoc', () => {
       expect(extractedTags?.params).not.toBeNull();
       expect(extractedTags?.params?.[0].name).toBe('event');
       expect(extractedTags?.params?.[0].type).not.toBeNull();
-      expect(extractedTags?.params?.[0].type.name).toBe('SyntheticEvent');
+      expect(extractedTags?.params?.[0].type.value).toBe('SyntheticEvent');
       expect(extractedTags?.params?.[0].description).toBe('React event');
     });
 
@@ -90,7 +90,7 @@ describe('parseJsDoc', () => {
       ['event1', 'event2', 'event3'].forEach((x, i) => {
         expect(extractedTags?.params?.[i].name).toBe(x);
         expect(extractedTags?.params?.[i].type).not.toBeNull();
-        expect(extractedTags?.params?.[i].type.name).toBe('SyntheticEvent');
+        expect(extractedTags?.params?.[i].type.value).toBe('SyntheticEvent');
         expect(extractedTags?.params?.[i].description).toBe('React event');
       });
     });
@@ -129,7 +129,7 @@ describe('parseJsDoc', () => {
         expect(extractedTags?.params).not.toBeNull();
         expect(extractedTags?.params?.[0].name).toBe('event');
         expect(extractedTags?.params?.[0].type).not.toBeNull();
-        expect(extractedTags?.params?.[0].type.name).toBe('SyntheticEvent');
+        expect(extractedTags?.params?.[0].type.value).toBe('SyntheticEvent');
         expect(extractedTags?.params?.[0].description).toBe('React event');
       });
     });
@@ -251,15 +251,15 @@ describe('parseJsDoc', () => {
 
       expect(extractedTags?.returns).not.toBeNull();
       expect(extractedTags?.returns?.type).not.toBeNull();
-      expect(extractedTags?.returns?.type.name).toBe('string');
+      expect(extractedTags?.returns?.type.value).toBe('string');
     });
 
     it('should return a @returns with a type and a description', () => {
       const { extractedTags } = parseJsDoc('@returns {string} - A bar description');
 
       expect(extractedTags?.returns).not.toBeNull();
       expect(extractedTags?.returns?.type).not.toBeNull();
-      expect(extractedTags?.returns?.type.name).toBe('string');
+      expect(extractedTags?.returns?.type.value).toBe('string');
       expect(extractedTags?.returns?.description).toBe('A bar description');
     });
 
@@ -270,7 +270,7 @@ describe('parseJsDoc', () => {
 
       expect(extractedTags?.returns).not.toBeNull();
       expect(extractedTags?.returns?.type).not.toBeNull();
-      expect(extractedTags?.returns?.type.name).toBe('string');
+      expect(extractedTags?.returns?.type.value).toBe('string');
       expect(extractedTags?.returns?.description).toBe('This is\na multiline\ndescription');
     });
 
@@ -279,7 +279,7 @@ describe('parseJsDoc', () => {
 
       expect(extractedTags?.returns).not.toBeNull();
       expect(extractedTags?.returns?.type).not.toBeNull();
-      expect(extractedTags?.returns?.type.name).toBe('number');
+      expect(extractedTags?.returns?.type.value).toBe('number');
     });
 
     describe('getTypeName', () => {
@@ -353,9 +353,9 @@ describe('parseJsDoc', () => {
     expect(extractedTags?.params).not.toBeNull();
     expect(Object.keys(extractedTags?.params ?? []).length).toBe(1);
     expect(extractedTags?.params?.[0].name).toBe('event');
-    expect(extractedTags?.params?.[0].type.name).toBe('SyntheticEvent');
+    expect(extractedTags?.params?.[0].type.value).toBe('SyntheticEvent');
     expect(extractedTags?.params?.[0].description).toBe('Original event.');
     expect(extractedTags?.returns).not.toBeNull();
-    expect(extractedTags?.returns?.type.name).toBe('string');
+    expect(extractedTags?.returns?.type.value).toBe('string');
   });
 });

code/lib/docs-tools/src/argTypes/jsdocParser.ts

@@ -1,5 +1,11 @@
-import type { Annotation } from 'doctrine';
-import doctrine from 'doctrine';
+import type { Block, Spec } from 'comment-parser';
+import type { RootResult as JSDocType } from 'jsdoc-type-pratt-parser';
+import { parse as parseJSDoc } from 'comment-parser';
+import {
+  parse as parseJSDocType,
+  transform as transformJSDocType,
+  stringifyRules,
+} from 'jsdoc-type-pratt-parser';
 import type { JsDocParam, JsDocReturns } from './docgen';
 
 export interface ExtractedJsDocParam extends JsDocParam {
@@ -40,21 +46,25 @@ function containsJsDoc(value?: string | null): boolean {
   return value != null && value.includes('@');
 }
 
-function parse(content: string | null, tags?: string[]): Annotation {
-  let ast;
+function parse(content: string | null): Block {
+  const contentString = content ?? '';
+  const mappedLines = contentString
+    .split('\n')
+    .map((line) => ` * ${line}`)
+    .join('\n');
+  const normalisedContent = '/**\n' + mappedLines + '\n*/';
 
-  try {
-    ast = doctrine.parse(content ?? '', {
-      tags,
-      sloppy: true,
-    });
-  } catch (e) {
-    console.error(e);
+  const ast = parseJSDoc(normalisedContent, {
+    spacing: 'preserve',
+  });
 
+  if (!ast || ast.length === 0) {
     throw new Error('Cannot parse JSDoc tags.');
   }
 
-  return ast;
+  // Return the first block, since we shouldn't ever really encounter
+  // multiple blocks of JSDoc
+  return ast[0];
 }
 
 const DEFAULT_OPTIONS = {
@@ -69,9 +79,9 @@ export const parseJsDoc: ParseJsDoc = (value, options = DEFAULT_OPTIONS) => {
     };
   }
 
-  const jsDocAst = parse(value, options.tags);
+  const jsDocAst = parse(value);
 
-  const extractedTags = extractJsDocTags(jsDocAst);
+  const extractedTags = extractJsDocTags(jsDocAst, options.tags);
 
   if (extractedTags.ignore) {
     // There is no point in doing other stuff since this prop will not be rendered.
@@ -85,33 +95,36 @@ export const parseJsDoc: ParseJsDoc = (value, options = DEFAULT_OPTIONS) => {
     includesJsDoc: true,
     ignore: false,
     // Always use the parsed description to ensure JSDoc is removed from the description.
-    description: jsDocAst.description,
+    description: jsDocAst.description.trim(),
     extractedTags,
   };
 };
 
-function extractJsDocTags(ast: doctrine.Annotation): ExtractedJsDoc {
+function extractJsDocTags(ast: Block, tags?: string[]): ExtractedJsDoc {
   const extractedTags: ExtractedJsDoc = {
     params: null,
     deprecated: null,
     returns: null,
     ignore: false,
   };
 
-  for (let i = 0; i < ast.tags.length; i += 1) {
-    const tag = ast.tags[i];
+  for (const tagSpec of ast.tags) {
+    // Skip any tags we don't care about
+    if (tags !== undefined && !tags.includes(tagSpec.tag)) {
+      continue;
+    }
 
-    if (tag.title === 'ignore') {
+    if (tagSpec.tag === 'ignore') {
       extractedTags.ignore = true;
       // Once we reach an @ignore tag, there is no point in parsing the other tags since we will not render the prop.
       break;
     } else {
-      switch (tag.title) {
+      switch (tagSpec.tag) {
         // arg & argument are aliases for param.
         case 'param':
         case 'arg':
         case 'argument': {
-          const paramTag = extractParam(tag);
+          const paramTag = extractParam(tagSpec);
           if (paramTag != null) {
             if (extractedTags.params == null) {
               extractedTags.params = [];
@@ -121,14 +134,14 @@ function extractJsDocTags(ast: doctrine.Annotation): ExtractedJsDoc {
           break;
         }
         case 'deprecated': {
-          const deprecatedTag = extractDeprecated(tag);
+          const deprecatedTag = extractDeprecated(tagSpec);
           if (deprecatedTag != null) {
             extractedTags.deprecated = deprecatedTag;
           }
           break;
         }
         case 'returns': {
-          const returnsTag = extractReturns(tag);
+          const returnsTag = extractReturns(tagSpec);
           if (returnsTag != null) {
             extractedTags.returns = returnsTag;
           }
@@ -143,107 +156,87 @@ function extractJsDocTags(ast: doctrine.Annotation): ExtractedJsDoc {
   return extractedTags;
 }
 
-function extractParam(tag: doctrine.Tag): ExtractedJsDocParam | null {
-  const paramName = tag.name;
-
-  // When the @param doesn't have a name but have a type and a description, "null-null" is returned.
-  if (paramName != null && paramName !== 'null-null') {
-    return {
-      name: tag.name,
-      type: tag.type,
-      description: tag.description,
-      getPrettyName: () => {
-        if (paramName.includes('null')) {
-          // There is a few cases in which the returned param name contains "null".
-          // - @param {SyntheticEvent} event- Original SyntheticEvent
-          // - @param {SyntheticEvent} event.\n@returns {string}
-          return paramName.replace('-null', '').replace('.null', '');
-        }
+function normaliseParamName(name: string): string {
+  return name.replace(/[\.-]$/, '');
+}
 
-        return tag.name;
-      },
-      getTypeName: () => {
-        return tag.type != null ? extractTypeName(tag.type) : null;
-      },
-    };
+function extractParam(tag: Spec): ExtractedJsDocParam | null {
+  // Ignore tags with empty names or `-`.
+  // We ignore `-` since it means a comment was likely missing a name but
+  // using separators. For example: `@param {foo} - description`
+  if (!tag.name || tag.name === '-') {
+    return null;
   }
 
-  return null;
+  const type = extractType(tag.type);
+
+  return {
+    name: tag.name,
+    type: type,
+    description: normaliseDescription(tag.description),
+    getPrettyName: () => {
+      return normaliseParamName(tag.name);
+    },
+    getTypeName: () => {
+      return type ? extractTypeName(type) : null;
+    },
+  };
 }
 
-function extractDeprecated(tag: doctrine.Tag): string | null {
-  if (tag.title != null) {
-    return tag.description;
+function extractDeprecated(tag: Spec): string | null {
+  if (tag.name) {
+    return joinNameAndDescription(tag.name, tag.description);
   }
 
   return null;
 }
 
-function extractReturns(tag: doctrine.Tag): ExtractedJsDocReturns | null {
-  if (tag.type != null) {
+function joinNameAndDescription(name: string, desc: string): string | null {
+  const joined = name === '' ? desc : `${name} ${desc}`;
+  return normaliseDescription(joined);
+}
+
+function normaliseDescription(text: string): string | null {
+  const normalised = text.replace(/^- /g, '').trim();
+
+  return normalised === '' ? null : normalised;
+}
+
+function extractReturns(tag: Spec): ExtractedJsDocReturns | null {
+  const type = extractType(tag.type);
+
+  if (type) {
     return {
-      type: tag.type,
-      description: tag.description,
+      type: type,
+      description: joinNameAndDescription(tag.name, tag.description),
       getTypeName: () => {
-        return extractTypeName(tag.type);
+        return extractTypeName(type);
       },
     };
   }
 
   return null;
 }
 
-function extractTypeName(type?: doctrine.Type | null): string | null {
-  if (type?.type === 'NameExpression') {
-    return type.name;
-  }
-
-  if (type?.type === 'RecordType') {
-    const recordFields = type.fields.map((field: doctrine.Type) => {
-      if (field.type === 'FieldType' && field.value != null) {
-        const valueTypeName = extractTypeName(field.value);
-
-        return `${field.key}: ${valueTypeName}`;
-      }
-
-      return (field as doctrine.type.FieldType).key;
-    });
-
-    return `({${recordFields.join(', ')}})`;
-  }
-
-  if (type?.type === 'UnionType') {
-    const unionElements = type.elements.map(extractTypeName);
-
-    return `(${unionElements.join('|')})`;
-  }
-
-  // Only support untyped array: []. Might add more support later if required.
-  if (type?.type === 'ArrayType') {
-    return '[]';
-  }
-
-  if (type?.type === 'TypeApplication') {
-    if (type.expression != null) {
-      if ((type.expression as doctrine.type.NameExpression).name === 'Array') {
-        const arrayType = extractTypeName(type.applications[0]);
-
-        return `${arrayType}[]`;
-      }
-    }
-  }
-
-  if (
-    type?.type === 'NullableType' ||
-    type?.type === 'NonNullableType' ||
-    type?.type === 'OptionalType'
-  ) {
-    return extractTypeName(type.expression);
-  }
-
-  if (type?.type === 'AllLiteral') {
-    return 'any';
+const jsdocStringifyRules = stringifyRules();
+const originalJsdocStringifyObject = jsdocStringifyRules.JsdocTypeObject;
+jsdocStringifyRules.JsdocTypeAny = () => 'any';
+jsdocStringifyRules.JsdocTypeObject = (result, transform) =>
+  `(${originalJsdocStringifyObject(result, transform)})`;
+jsdocStringifyRules.JsdocTypeOptional = (result, transform) => transform(result.element);
+jsdocStringifyRules.JsdocTypeNullable = (result, transform) => transform(result.element);
+jsdocStringifyRules.JsdocTypeNotNullable = (result, transform) => transform(result.element);
+jsdocStringifyRules.JsdocTypeUnion = (result, transform) =>
+  result.elements.map(transform).join('|');
+
+function extractType(typeString: string): JSDocType | null {
+  try {
+    return parseJSDocType(typeString, 'typescript');
+  } catch (_err) {
+    return null;
   }
+}
 
-  return null;
+function extractTypeName(type: JSDocType): string {
+  return transformJSDocType(jsdocStringifyRules, type);
 }