From ab83ede171ed2c8a85f6946669ca3bf49466c92a Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Wed, 17 Apr 2024 12:09:50 -0700 Subject: [PATCH 1/3] Add lint error for declarations marked internal, but unexported --- scripts/eslint/rules/jsdoc-format.cjs | 34 ++++++++++++++++++++++++--- src/compiler/scanner.ts | 2 -- src/services/services.ts | 2 +- 3 files changed, 32 insertions(+), 6 deletions(-) diff --git a/scripts/eslint/rules/jsdoc-format.cjs b/scripts/eslint/rules/jsdoc-format.cjs index 4753fe9cc84cc..5bca3d5e9dc14 100644 --- a/scripts/eslint/rules/jsdoc-format.cjs +++ b/scripts/eslint/rules/jsdoc-format.cjs @@ -11,6 +11,7 @@ module.exports = createRule({ internalCommentNotLastError: `@internal should only appear in final JSDoc comment for declaration.`, multipleJSDocError: `Declaration has multiple JSDoc comments.`, internalCommentOnParameterProperty: `@internal cannot appear on a JSDoc comment; use a declared property and an assignment in the constructor instead.`, + internalCommentOnUnexported: `@internal should not appear on an unexported declaration.`, }, schema: [], type: "problem", @@ -23,6 +24,30 @@ module.exports = createRule({ const atInternal = "@internal"; const jsdocStart = "/**"; + /** @type {Map} */ + const isExportedCache = new Map(); + + /** @type {(node: import("@typescript-eslint/utils").TSESTree.Node) => boolean} */ + function isExported(node) { + const exported = isExportedCache.get(node); + if (exported !== undefined) { + return exported; + } + + /** @type {import("@typescript-eslint/utils").TSESTree.Node | undefined} */ + let current = node; + while (current) { + if (current.type.startsWith("Export")) { + isExportedCache.set(node, true); + return true; + } + current = current.parent; + } + + isExportedCache.set(node, false); + return false; + } + /** @type {(text: string) => boolean} */ function isJSDocText(text) { return text.startsWith(jsdocStart); @@ -81,12 +106,15 @@ module.exports = createRule({ if (!isJSDoc) { context.report({ messageId: "internalCommentInNonJSDocError", node: c, loc: getAtInternalLoc(c, indexInComment) }); } - else if (i !== last) { - context.report({ messageId: "internalCommentNotLastError", node: c, loc: getAtInternalLoc(c, indexInComment) }); - } else if (node.type === "TSParameterProperty") { context.report({ messageId: "internalCommentOnParameterProperty", node: c, loc: getAtInternalLoc(c, indexInComment) }); } + else if (!isExported(node)) { + context.report({ messageId: "internalCommentOnUnexported", node: c, loc: getAtInternalLoc(c, indexInComment) }); + } + else if (i !== last) { + context.report({ messageId: "internalCommentNotLastError", node: c, loc: getAtInternalLoc(c, indexInComment) }); + } } }; diff --git a/src/compiler/scanner.ts b/src/compiler/scanner.ts index 7b0d1ebb87d35..4521fb0600cd2 100644 --- a/src/compiler/scanner.ts +++ b/src/compiler/scanner.ts @@ -2805,13 +2805,11 @@ export function createScanner(languageVersion: ScriptTarget, skipTrivia: boolean } } -/** @internal */ function codePointAt(s: string, i: number): number { // TODO(jakebailey): this is wrong and should have ?? 0; but all users are okay with it return s.codePointAt(i)!; } -/** @internal */ function charSize(ch: number) { if (ch >= 0x10000) { return 2; diff --git a/src/services/services.ts b/src/services/services.ts index f97d3eb0651dc..18f0b4d151439 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -787,7 +787,7 @@ class IdentifierObject extends TokenOrIdentifierObject im declare _declarationBrand: any; declare _jsdocContainerBrand: any; declare _flowContainerBrand: any; - /** @internal */ typeArguments!: NodeArray; + typeArguments!: NodeArray; constructor(kind: SyntaxKind.Identifier, pos: number, end: number) { super(kind, pos, end); } From a064436501c27f4607a7e76c0a17e262c092c2d6 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Wed, 17 Apr 2024 12:31:27 -0700 Subject: [PATCH 2/3] Add comment --- scripts/eslint/rules/jsdoc-format.cjs | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/eslint/rules/jsdoc-format.cjs b/scripts/eslint/rules/jsdoc-format.cjs index 5bca3d5e9dc14..fc26224fe14c2 100644 --- a/scripts/eslint/rules/jsdoc-format.cjs +++ b/scripts/eslint/rules/jsdoc-format.cjs @@ -37,6 +37,7 @@ module.exports = createRule({ /** @type {import("@typescript-eslint/utils").TSESTree.Node | undefined} */ let current = node; while (current) { + // https://github.com/typescript-eslint/typescript-eslint/blob/e44a1a280f08f9fd0d29f74e5c3e73b7b64a9606/packages/eslint-plugin/src/util/collectUnusedVariables.ts#L440 if (current.type.startsWith("Export")) { isExportedCache.set(node, true); return true; From 2e1cbb673304d075e296765750682029f03caeb4 Mon Sep 17 00:00:00 2001 From: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Date: Wed, 17 Apr 2024 13:32:24 -0700 Subject: [PATCH 3/3] Set for all nodes in tree --- scripts/eslint/rules/jsdoc-format.cjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/eslint/rules/jsdoc-format.cjs b/scripts/eslint/rules/jsdoc-format.cjs index fc26224fe14c2..9e38ca0f99c29 100644 --- a/scripts/eslint/rules/jsdoc-format.cjs +++ b/scripts/eslint/rules/jsdoc-format.cjs @@ -42,10 +42,10 @@ module.exports = createRule({ isExportedCache.set(node, true); return true; } + isExportedCache.set(current, false); current = current.parent; } - isExportedCache.set(node, false); return false; }