Reject internal tag on private decls, strip comments from private decls in dtsBundler (#58869)

Author: jakebailey

scripts/dtsBundler.mjs

@@ -39,6 +39,13 @@ console.log(`Bundling ${entrypoint} to ${output} and ${internalOutput}`);
 const newLineKind = ts.NewLineKind.LineFeed;
 const newLine = newLineKind === ts.NewLineKind.LineFeed ? "\n" : "\r\n";
 
+/**
+ * @param {ts.Node} node
+ */
+function removeAllComments(node) {
+    /** @type {any} */ (ts).removeAllComments(node);
+}
+
 /**
  * @param {ts.VariableDeclaration} node
  * @returns {ts.VariableStatement}
@@ -422,6 +429,15 @@ function emitAsNamespace(name, parent, moduleSymbol, needExportModifier) {
                     if (ts.isInternalDeclaration(node)) {
                         return undefined;
                     }
+                    // TODO: remove after https://github.com/microsoft/TypeScript/pull/58187 is released
+                    if (ts.canHaveModifiers(node)) {
+                        for (const modifier of ts.getModifiers(node) ?? []) {
+                            if (modifier.kind === ts.SyntaxKind.PrivateKeyword) {
+                                removeAllComments(node);
+                                break;
+                            }
+                        }
+                    }
                     return node;
                 }, /*context*/ undefined);
 

scripts/eslint/rules/jsdoc-format.cjs

@@ -12,6 +12,7 @@ module.exports = createRule({
             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.`,
+            internalCommentOnPrivate: `@internal should not appear on a private declaration.`,
         },
         schema: [],
         type: "problem",
@@ -56,16 +57,9 @@ module.exports = createRule({
 
         /** @type {(c: import("@typescript-eslint/utils").TSESTree.Comment, indexInComment: number) => import("@typescript-eslint/utils").TSESTree.SourceLocation} */
         const getAtInternalLoc = (c, indexInComment) => {
-            const line = c.loc.start.line;
             return {
-                start: {
-                    line,
-                    column: c.loc.start.column + indexInComment,
-                },
-                end: {
-                    line,
-                    column: c.loc.start.column + indexInComment + atInternal.length,
-                },
+                start: context.sourceCode.getLocFromIndex(c.range[0] + indexInComment),
+                end: context.sourceCode.getLocFromIndex(c.range[0] + indexInComment + atInternal.length),
             };
         };
 
@@ -113,6 +107,10 @@ module.exports = createRule({
                 else if (!isExported(node)) {
                     context.report({ messageId: "internalCommentOnUnexported", node: c, loc: getAtInternalLoc(c, indexInComment) });
                 }
+                // eslint-disable-next-line local/no-in-operator
+                else if ("accessibility" in node && node.accessibility === "private") {
+                    context.report({ messageId: "internalCommentOnPrivate", node: c, loc: getAtInternalLoc(c, indexInComment) });
+                }
                 else if (i !== last) {
                     context.report({ messageId: "internalCommentNotLastError", node: c, loc: getAtInternalLoc(c, indexInComment) });
                 }

src/server/editorServices.ts

@@ -1302,16 +1302,12 @@ export class ProjectService {
     /** @internal */
     readonly watchFactory: WatchFactory<WatchType, Project | NormalizedPath>;
 
-    /** @internal */
     private readonly sharedExtendedConfigFileWatchers = new Map<Path, SharedExtendedConfigFileWatcher<NormalizedPath>>();
-    /** @internal */
     private readonly extendedConfigCache = new Map<string, ExtendedConfigCacheEntry>();
 
     /** @internal */
     readonly packageJsonCache: PackageJsonCache;
-    /** @internal */
     private packageJsonFilesMap: Map<Path, PackageJsonWatcher> | undefined;
-    /** @internal */
     private incompleteCompletionsCache: IncompleteCompletionsCache | undefined;
     /** @internal */
     readonly session: Session<unknown> | undefined;
@@ -1841,8 +1837,6 @@ export class ProjectService {
 
     /**
      * This is to watch whenever files are added or removed to the wildcard directories
-     *
-     * @internal
      */
     private watchWildcardDirectory(directory: string, flags: WatchDirectoryFlags, configFileName: NormalizedPath, config: ParsedConfig) {
         let watcher: FileWatcher | undefined = this.watchFactory.watchDirectory(
@@ -1941,7 +1935,6 @@ export class ProjectService {
         return result;
     }
 
-    /** @internal */
     private delayUpdateProjectsFromParsedConfigOnConfigFileChange(canonicalConfigFilePath: NormalizedPath, loadReason: string) {
         const configFileExistenceInfo = this.configFileExistenceInfoCache.get(canonicalConfigFilePath);
         if (!configFileExistenceInfo?.config) return false;
@@ -1978,7 +1971,6 @@ export class ProjectService {
         return scheduledAnyProjectUpdate;
     }
 
-    /** @internal */
     private onConfigFileChanged(configFileName: NormalizedPath, canonicalConfigFilePath: NormalizedPath, eventKind: FileWatcherEventKind) {
         const configFileExistenceInfo = this.configFileExistenceInfoCache.get(canonicalConfigFilePath)!;
         const project = this.getConfiguredProjectByCanonicalConfigFilePath(canonicalConfigFilePath);
@@ -2280,7 +2272,6 @@ export class ProjectService {
         return exists;
     }
 
-    /** @internal */
     private createConfigFileWatcherForParsedConfig(configFileName: NormalizedPath, canonicalConfigFilePath: NormalizedPath, forProject: ConfiguredProject) {
         const configFileExistenceInfo = this.configFileExistenceInfoCache.get(canonicalConfigFilePath)!;
         // When watching config file for parsed config, remove the noopFileWatcher that can be created for open files impacted by config file and watch for real
@@ -2745,8 +2736,6 @@ export class ProjectService {
 
     /**
      * Read the config file of the project, and update the project root file names.
-     *
-     * @internal
      */
     private loadConfiguredProject(project: ConfiguredProject, reason: string) {
         tracing?.push(tracing.Phase.Session, "loadConfiguredProject", { configFilePath: project.canonicalConfigFilePath });
@@ -3031,7 +3020,6 @@ export class ProjectService {
         return project.updateGraph();
     }
 
-    /** @internal */
     private reloadFileNamesOfParsedConfig(configFileName: NormalizedPath, config: ParsedConfig) {
         if (config.updateLevel === undefined) return config.parsedCommandLine!.fileNames;
         Debug.assert(config.updateLevel === ProgramUpdateLevel.RootNamesAndUpdate);
@@ -3085,7 +3073,6 @@ export class ProjectService {
         updateWithTriggerFile(project, project.triggerFileForConfigFileDiag ?? project.getConfigFilePath(), /*isReload*/ true);
     }
 
-    /** @internal */
     private clearSemanticCache(project: Project) {
         project.originalConfiguredProjects = undefined;
         project.resolutionCache.clear();
@@ -3800,7 +3787,6 @@ export class ProjectService {
         return this.getWatchOptionsFromProjectWatchOptions(project.getWatchOptions(), project.getCurrentDirectory());
     }
 
-    /** @internal */
     private getWatchOptionsFromProjectWatchOptions(projectOptions: WatchOptions | undefined, basePath: string) {
         const hostWatchOptions = !this.hostConfiguration.beforeSubstitution ? this.hostConfiguration.watchOptions :
             handleWatchOptionsConfigDirTemplateSubstitution(
@@ -5007,7 +4993,6 @@ export class ProjectService {
 
     /**
      * Performs the remaining steps of enabling a plugin after its module has been instantiated.
-     * @internal
      */
     private endEnablePlugin(project: Project, { pluginConfigEntry, resolvedModule, errorLogs }: BeginEnablePluginResult) {
         if (resolvedModule) {
@@ -5162,7 +5147,6 @@ export class ProjectService {
         });
     }
 
-    /** @internal */
     private watchPackageJsonFile(file: string, path: Path, project: Project | WildcardWatcher) {
         Debug.assert(project !== undefined);
         let result = (this.packageJsonFilesMap ??= new Map()).get(path);
@@ -5204,7 +5188,6 @@ export class ProjectService {
         (project.packageJsonWatches ??= new Set()).add(result);
     }
 
-    /** @internal */
     private onPackageJsonChange(result: PackageJsonWatcher) {
         result.projects.forEach(project => (project as Project).onPackageJsonChange?.());
     }

src/server/project.ts

@@ -327,9 +327,7 @@ export abstract class Project implements LanguageServiceHost, ModuleResolutionHo
 
     /** @internal */
     lastCachedUnresolvedImportsList: SortedReadonlyArray<string> | undefined;
-    /** @internal */
     private hasAddedorRemovedFiles = false;
-    /** @internal */
     private hasAddedOrRemovedSymlinks = false;
 
     /** @internal */
@@ -390,7 +388,6 @@ export abstract class Project implements LanguageServiceHost, ModuleResolutionHo
     /** @internal */
     typingFiles: SortedReadonlyArray<string> = emptyArray;
 
-    /** @internal */
     private typingWatchers: TypingWatchers | undefined;
 
     /** @internal */
@@ -492,13 +489,9 @@ export abstract class Project implements LanguageServiceHost, ModuleResolutionHo
     /** @internal */
     public readonly getCanonicalFileName: GetCanonicalFileName;
 
-    /** @internal */
     private exportMapCache: ExportInfoMap | undefined;
-    /** @internal */
     private changedFilesForExportMapCache: Set<Path> | undefined;
-    /** @internal */
     private moduleSpecifierCache = createModuleSpecifierCache(this);
-    /** @internal */
     private symlinks: SymlinkCache | undefined;
     /** @internal */
     autoImportProviderHost: AutoImportProviderProject | false | undefined;
@@ -1418,13 +1411,11 @@ export abstract class Project implements LanguageServiceHost, ModuleResolutionHo
         }
     }
 
-    /** @internal */
     private closeWatchingTypingLocations() {
         if (this.typingWatchers) clearMap(this.typingWatchers, closeFileWatcher);
         this.typingWatchers = undefined;
     }
 
-    /** @internal */
     private onTypingInstallerWatchInvoke() {
         this.typingWatchers!.isInvoked = true;
         this.projectService.updateTypingsForProject({ projectName: this.getProjectName(), kind: ActionInvalidate });
@@ -1828,7 +1819,6 @@ export abstract class Project implements LanguageServiceHost, ModuleResolutionHo
         return this.filesToStringWorker(writeProjectFileNames, /*writeFileExplaination*/ true, /*writeFileVersionAndText*/ false);
     }
 
-    /** @internal */
     private filesToStringWorker(writeProjectFileNames: boolean, writeFileExplaination: boolean, writeFileVersionAndText: boolean) {
         if (this.isInitialLoadPending()) return "\tFiles (0) InitialLoadPending\n";
         if (!this.program) return "\tFiles (0) NoProgram\n";
@@ -2203,7 +2193,6 @@ export abstract class Project implements LanguageServiceHost, ModuleResolutionHo
         }
     }
 
-    /** @internal */
     private isDefaultProjectForOpenFiles(): boolean {
         return !!forEachEntry(
             this.projectService.openFiles,
@@ -2250,7 +2239,6 @@ export abstract class Project implements LanguageServiceHost, ModuleResolutionHo
         }
     }
 
-    /** @internal */
     private getCompilerOptionsForNoDtsResolutionProject() {
         return {
             ...this.getCompilerOptions(),
@@ -2443,7 +2431,6 @@ export class AuxiliaryProject extends Project {
 }
 
 export class AutoImportProviderProject extends Project {
-    /** @internal */
     private static readonly maxDependencies = 10;
 
     /** @internal */
@@ -2781,7 +2768,6 @@ export class ConfiguredProject extends Project {
     /** @internal */
     sendLoadingProjectFinish = false;
 
-    /** @internal */
     private compilerHost?: CompilerHost;
 
     /** @internal */
@@ -2845,7 +2831,6 @@ export class ConfiguredProject extends Project {
         this.releaseParsedConfig(asNormalizedPath(this.projectService.toCanonicalFileName(asNormalizedPath(normalizePath(fileName)))));
     }
 
-    /** @internal */
     private releaseParsedConfig(canonicalConfigFilePath: NormalizedPath) {
         this.projectService.stopWatchingWildCards(canonicalConfigFilePath, this);
         this.projectService.releaseParsedConfig(canonicalConfigFilePath, this);

src/server/scriptInfo.ts

@@ -382,8 +382,6 @@ export class ScriptInfo {
 
     /**
      * Set to real path if path is different from info.path
-     *
-     * @internal
      */
     private realpath: Path | undefined;
 

src/typingsInstallerCore/typingsInstaller.ts

@@ -113,7 +113,6 @@ export abstract class TypingsInstaller {
     private readonly knownCachesSet = new Set<string>();
     private readonly projectWatchers = new Map<string, Set<string>>();
     private safeList: JsTyping.SafeList | undefined;
-    /** @internal */
     private pendingRunRequests: PendingRequest[] = [];
 
     private installRunCount = 1;