From e395070a33e7eabd4ff972e9ab81a96cc5eea66d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Fri, 30 Aug 2024 15:03:46 +0200 Subject: [PATCH] Implemented a "no-undefined-types" rule --- common/config/rush/pnpm-lock.yaml | 10 +- eslint-plugin/package.json | 2 +- eslint-plugin/src/index.ts | 269 ++++++++++++++++++++++-------- 3 files changed, 210 insertions(+), 71 deletions(-) diff --git a/common/config/rush/pnpm-lock.yaml b/common/config/rush/pnpm-lock.yaml index e1f61fb8..22ce4eac 100644 --- a/common/config/rush/pnpm-lock.yaml +++ b/common/config/rush/pnpm-lock.yaml @@ -55,8 +55,8 @@ importers: specifier: ~2.6.11 version: 2.6.11(@rushstack/heft@0.66.13)(@types/node@14.18.36) '@types/eslint': - specifier: 8.40.1 - version: 8.40.1 + specifier: 8.56.10 + version: 8.56.10 '@types/estree': specifier: 1.0.1 version: 1.0.1 @@ -1445,12 +1445,12 @@ packages: /@types/eslint-scope@3.7.4: resolution: {integrity: sha512-9K4zoImiZc3HlIp6AVUDE4CWYx22a+lhSZMYNpbjW04+YF0KWj4pJXnEMjdnFTiQibFFmElcsasJXDbdI/EPhA==} dependencies: - '@types/eslint': 8.40.1 + '@types/eslint': 8.56.10 '@types/estree': 1.0.1 dev: true - /@types/eslint@8.40.1: - resolution: {integrity: sha512-vRb792M4mF1FBT+eoLecmkpLXwxsBHvWWRGJjzbYANBM6DtiJc6yETyv4rqDA6QNjF1pkj1U7LMA6dGb3VYlHw==} + /@types/eslint@8.56.10: + resolution: {integrity: sha512-Shavhk87gCtY2fhXDctcfS3e6FdxWkCx1iUZ9eEUbh7rTqlZT0/IzOkCOVt0fCjcFuZ9FPYfuezTBImfHCDBGQ==} dependencies: '@types/estree': 1.0.1 '@types/json-schema': 7.0.12 diff --git a/eslint-plugin/package.json b/eslint-plugin/package.json index 3d759e0e..e3e70ca3 100644 --- a/eslint-plugin/package.json +++ b/eslint-plugin/package.json @@ -33,7 +33,7 @@ "devDependencies": { "@rushstack/heft-node-rig": "~2.6.11", "@rushstack/heft": "^0.66.13", - "@types/eslint": "8.40.1", + "@types/eslint": "8.56.10", "@types/estree": "1.0.1", "@types/heft-jest": "1.0.3", "@types/node": "14.18.36", diff --git a/eslint-plugin/src/index.ts b/eslint-plugin/src/index.ts index 94b9e2b3..9e65eb09 100644 --- a/eslint-plugin/src/index.ts +++ b/eslint-plugin/src/index.ts @@ -3,7 +3,16 @@ import type * as eslint from 'eslint'; import type * as ESTree from 'estree'; -import { TSDocParser, TextRange, TSDocConfiguration, type ParserContext } from '@microsoft/tsdoc'; +import { + TSDocParser, + TextRange, + TSDocConfiguration, + type ParserContext, + DocNodeKind, + DocLinkTag, + DocDeclarationReference, + type DocSection +} from '@microsoft/tsdoc'; import type { TSDocConfigFile } from '@microsoft/tsdoc-config'; import { Debug } from './Debug'; @@ -20,6 +29,93 @@ interface IPlugin { rules: { [x: string]: eslint.Rule.RuleModule }; } +function createTSDocParser(context: eslint.Rule.RuleContext): TSDocParser { + const sourceFilePath: string = context.getFilename(); + Debug.log(`Linting: "${sourceFilePath}"`); + const tsdocConfiguration: TSDocConfiguration = new TSDocConfiguration(); + + try { + const tsdocConfigFile: TSDocConfigFile = ConfigCache.getForSourceFile(sourceFilePath); + if (!tsdocConfigFile.fileNotFound) { + if (tsdocConfigFile.hasErrors) { + context.report({ + loc: { line: 1, column: 1 }, + messageId: 'error-loading-config-file', + data: { + details: tsdocConfigFile.getErrorSummary() + } + }); + } + + try { + tsdocConfigFile.configureParser(tsdocConfiguration); + } catch (e) { + context.report({ + loc: { line: 1, column: 1 }, + messageId: 'error-applying-config', + data: { + details: e.message + } + }); + } + } + } catch (e) { + context.report({ + loc: { line: 1, column: 1 }, + messageId: 'error-loading-config-file', + data: { + details: `Unexpected exception: ${e.message}` + } + }); + } + + return new TSDocParser(tsdocConfiguration); +} + +function* parseTSDocComments( + tsdocParser: TSDocParser, + sourceCode: eslint.SourceCode +): Generator { + for (const comment of sourceCode.getAllComments()) { + if (comment.type !== 'Block') { + continue; + } + if (!comment.range) { + continue; + } + + const textRange: TextRange = TextRange.fromStringRange( + sourceCode.text, + comment.range[0], + comment.range[1] + ); + + // Smallest comment is "/***/" + if (textRange.length < 5) { + continue; + } + // Make sure it starts with "/**" + if (textRange.buffer[textRange.pos + 2] !== '*') { + continue; + } + + yield tsdocParser.parseRange(textRange); + } +} + +function findVariable(scope: eslint.Scope.Scope, identifier: string): eslint.Scope.Variable | undefined { + const variable: eslint.Scope.Variable | undefined = scope.set.get(identifier); + if (variable) { + return variable; + } + for (const child of scope.childScopes) { + const result: eslint.Scope.Variable | undefined = findVariable(child, identifier); + if (result) { + return result; + } + } +} + const plugin: IPlugin = { rules: { // NOTE: The actual ESLint rule name will be "tsdoc/syntax". It is calculated by deleting "eslint-plugin-" @@ -41,90 +137,133 @@ const plugin: IPlugin = { } }, create: (context: eslint.Rule.RuleContext) => { - const sourceFilePath: string = context.getFilename(); - Debug.log(`Linting: "${sourceFilePath}"`); - - const tsdocConfiguration: TSDocConfiguration = new TSDocConfiguration(); - - try { - const tsdocConfigFile: TSDocConfigFile = ConfigCache.getForSourceFile(sourceFilePath); - if (!tsdocConfigFile.fileNotFound) { - if (tsdocConfigFile.hasErrors) { - context.report({ - loc: { line: 1, column: 1 }, - messageId: 'error-loading-config-file', - data: { - details: tsdocConfigFile.getErrorSummary() - } - }); - } - - try { - tsdocConfigFile.configureParser(tsdocConfiguration); - } catch (e) { + const tsdocParser: TSDocParser = createTSDocParser(context); + const sourceCode: eslint.SourceCode = context.getSourceCode(); + const checkCommentBlocks: (node: ESTree.Node) => void = function (node: ESTree.Node) { + for (const parserContext of parseTSDocComments(tsdocParser, sourceCode)) { + for (const message of parserContext.log.messages) { context.report({ - loc: { line: 1, column: 1 }, - messageId: 'error-applying-config', + loc: { + start: sourceCode.getLocFromIndex(message.textRange.pos), + end: sourceCode.getLocFromIndex(message.textRange.end) + }, + messageId: message.messageId, data: { - details: e.message + unformattedText: message.unformattedText } }); } } - } catch (e) { - context.report({ - loc: { line: 1, column: 1 }, - messageId: 'error-loading-config-file', - data: { - details: `Unexpected exception: ${e.message}` - } - }); - } - - const tsdocParser: TSDocParser = new TSDocParser(tsdocConfiguration); + }; - const sourceCode: eslint.SourceCode = context.getSourceCode(); + return { + Program: checkCommentBlocks + }; + } + }, + 'no-undefined-types': { + meta: { + messages: { + 'error-loading-config-file': 'Error loading TSDoc config file:\n{{details}}', + 'error-applying-config': 'Error applying TSDoc configuration: {{details}}', + 'error-undefined-reference': 'A TSDoc-comment referenced "{{identifier}}" which is not defined' + }, + type: 'problem', + docs: { + description: 'Validates that TypeScript documentation comments reference only defined types', + // This package is experimental + recommended: false, + url: 'https://tsdoc.org/pages/packages/eslint-plugin-tsdoc' + } + }, + create: (context: eslint.Rule.RuleContext) => { + const tsdocParser: TSDocParser = createTSDocParser(context); + const sourceCode: eslint.SourceCode = context.sourceCode; const checkCommentBlocks: (node: ESTree.Node) => void = function (node: ESTree.Node) { - for (const comment of sourceCode.getAllComments()) { - if (comment.type !== 'Block') { - continue; + // TODO: Figure out a way to get the scope of the node which the comment is referencing instead + const scope: eslint.Scope.Scope = context.sourceCode.getScope(node); + + // TODO: Pass a list of comments to `parseTSDocComments` instead of `sourceCode` + for (const parserContext of parseTSDocComments(tsdocParser, sourceCode)) { + function markOrReportIdentifier(identifier: string): void { + const variable: eslint.Scope.Variable | undefined = findVariable(scope, identifier); + if (variable) { + sourceCode.markVariableAsUsed(identifier, node); + } else { + context.report({ + loc: { + // TODO: Narrow this further + start: sourceCode.getLocFromIndex(parserContext.commentRange.pos), + end: sourceCode.getLocFromIndex(parserContext.commentRange.end) + }, + messageId: 'error-undefined-reference', + data: { + identifier + } + }); + } } - if (!comment.range) { - continue; + + function visitDeclarationReference(reference: DocDeclarationReference): void { + for (const memberReferences of reference.memberReferences) { + // TODO: Support memberReferences.memberSymbol + if (!memberReferences.memberIdentifier) { + console.warn('Symbols in links are not supported'); + continue; + } + const { identifier } = memberReferences.memberIdentifier; + markOrReportIdentifier(identifier); + } } - const textRange: TextRange = TextRange.fromStringRange( - sourceCode.text, - comment.range[0], - comment.range[1] - ); + const { docComment } = parserContext; - // Smallest comment is "/***/" - if (textRange.length < 5) { - continue; - } - // Make sure it starts with "/**" - if (textRange.buffer[textRange.pos + 2] !== '*') { - continue; + const sections: DocSection[] = [docComment.summarySection]; + + for (const block of [ + docComment.remarksBlock, + docComment.privateRemarks, + docComment.deprecatedBlock, + ...docComment.params, + ...docComment.typeParams, + docComment.returnsBlock + ]) { + if (block) { + sections.push(block.content); + } } - const parserContext: ParserContext = tsdocParser.parseRange(textRange); - for (const message of parserContext.log.messages) { - context.report({ - loc: { - start: sourceCode.getLocFromIndex(message.textRange.pos), - end: sourceCode.getLocFromIndex(message.textRange.end) - }, - messageId: message.messageId, - data: { - unformattedText: message.unformattedText + for (const section of sections) { + // Find links in the summary + for (const childOfSection of section.nodes) { + if (childOfSection.kind !== DocNodeKind.Paragraph) { + console.warn( + 'Expected all direct children of summary sections to be paragraphs, got ' + + childOfSection.kind + ); + continue; } - }); + for (const childOfParagraph of childOfSection.getChildNodes()) { + if (!(childOfParagraph instanceof DocLinkTag)) { + continue; + } + const { codeDestination } = childOfParagraph; + if (!(codeDestination instanceof DocDeclarationReference)) { + continue; + } + visitDeclarationReference(codeDestination); + } + } + } + + if (docComment.inheritDocTag && docComment.inheritDocTag.declarationReference) { + visitDeclarationReference(docComment.inheritDocTag.declarationReference); } } }; return { + // TODO: Make this more granular, to resolve identifiers relative to the scope of the node which the comment is referencing to Program: checkCommentBlocks }; }