Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[api-extractor] Replace the old "tsdocFlavor" field with a dist/tsdoc-metadata.json output #960

Merged
merged 11 commits into from
Nov 28, 2018
10 changes: 3 additions & 7 deletions apps/api-documenter/src/start.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,14 @@

import * as os from 'os';
import * as colors from 'colors';
import * as path from 'path';

import { FileConstants } from '@microsoft/node-core-library';
import { PackageJsonLookup } from '@microsoft/node-core-library';

import { ApiDocumenterCommandLine } from './cli/ApiDocumenterCommandLine';

const myPackageJsonFilename: string = path.resolve(path.join(
__dirname, '..', FileConstants.PackageJson)
);
const myPackageJson: { version: string } = require(myPackageJsonFilename);
const myPackageVersion: string = PackageJsonLookup.loadOwnPackageJson(__dirname).version;

console.log(os.EOL + colors.bold(`api-documenter ${myPackageJson.version} `
console.log(os.EOL + colors.bold(`api-documenter ${myPackageVersion} `
+ colors.cyan(' - http://aka.ms/extractor') + os.EOL));

const parser: ApiDocumenterCommandLine = new ApiDocumenterCommandLine();
Expand Down
2 changes: 1 addition & 1 deletion apps/api-extractor/.vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
"run",
"-l"
],
"sourceMaps": false
"sourceMaps": true
},
{
"type": "node",
Expand Down
14 changes: 8 additions & 6 deletions apps/api-extractor/src/ExtractorContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@ export interface IExtractorContextOptions {
* abstract syntax tree.
*/
export class ExtractorContext {
public typeChecker: ts.TypeChecker;
public readonly program: ts.Program;
public readonly typeChecker: ts.TypeChecker;
public package: AstPackage;

/**
Expand All @@ -73,12 +74,12 @@ export class ExtractorContext {

public readonly validationRules: IExtractorValidationRulesConfig;

public readonly logger: ILogger;

// If the entry point is "C:\Folder\project\src\index.ts" and the nearest package.json
// is "C:\Folder\project\package.json", then the packageFolder is "C:\Folder\project"
private _packageFolder: string;

private _logger: ILogger;

constructor(options: IExtractorContextOptions) {
this.packageJsonLookup = new PackageJsonLookup();

Expand All @@ -97,7 +98,7 @@ export class ExtractorContext {

this.docItemLoader = new DocItemLoader(this._packageFolder);

this._logger = options.logger;
this.logger = options.logger;

// This runs a full type analysis, and then augments the Abstract Syntax Tree (i.e. declarations)
// with semantic information (i.e. symbols). The "diagnostics" are a subset of the everyday
Expand All @@ -107,6 +108,7 @@ export class ExtractorContext {
this.reportError(`TypeScript: ${errorText}`, diagnostic.file, diagnostic.start);
}

this.program = options.program;
this.typeChecker = options.program.getTypeChecker();

const rootFile: ts.SourceFile | undefined = options.program.getSourceFile(options.entryPointFile);
Expand Down Expand Up @@ -146,10 +148,10 @@ export class ExtractorContext {

// Format the error so that VS Code can follow it. For example:
// "src\MyClass.ts(15,1): The JSDoc tag "@blah" is not supported by AEDoc"
this._logger.logError(`${shownPath}(${lineAndCharacter.line + 1},${lineAndCharacter.character + 1}): `
this.logger.logError(`${shownPath}(${lineAndCharacter.line + 1},${lineAndCharacter.character + 1}): `
+ message);
} else {
this._logger.logError(message);
this.logger.logError(message);
}
}

Expand Down
16 changes: 14 additions & 2 deletions apps/api-extractor/src/extractor/Extractor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ import {
JsonFile,
JsonSchema,
Path,
FileSystem
FileSystem,
PackageJsonLookup
} from '@microsoft/node-core-library';
import {
IExtractorConfig,
Expand All @@ -25,6 +26,7 @@ import { ApiFileGenerator } from '../generators/ApiFileGenerator';
import { DtsRollupGenerator, DtsRollupKind } from '../generators/dtsRollup/DtsRollupGenerator';
import { MonitoredLogger } from './MonitoredLogger';
import { TypeScriptMessageFormatter } from '../utils/TypeScriptMessageFormatter';
import { PackageMetadataManager } from '../generators/dtsRollup/PackageMetadataManager';

/**
* Options for {@link Extractor.processProject}.
Expand Down Expand Up @@ -119,6 +121,13 @@ export class Extractor {
private readonly _monitoredLogger: MonitoredLogger;
private readonly _absoluteRootFolder: string;

/**
* The NPM package version for the currently executing instance of the "\@microsoft/api-extractor" library.
*/
public static get version(): string {
return PackageJsonLookup.loadOwnPackageJson(__dirname).version;
}

/**
* Given a list of absolute file paths, return a list containing only the declaration
* files. Duplicates are also eliminated.
Expand Down Expand Up @@ -399,6 +408,9 @@ export class Extractor {

this._generateRollupDtsFiles(context);

// Write the tsdoc-metadata.json file for this project
PackageMetadataManager.writeTsdocMetadataFile(context.packageFolder);

if (this._localBuild) {
// For a local build, fail if there were errors (but ignore warnings)
return this._monitoredLogger.errorCount === 0;
Expand Down Expand Up @@ -486,7 +498,7 @@ export class Extractor {
this._monitoredLogger.logVerbose(`Writing package typings: ${mainDtsRollupFullPath}`);

dtsRollupGenerator.writeTypingsFile(mainDtsRollupFullPath, dtsKind);
}
}

private _getShortFilePath(absolutePath: string): string {
if (!path.isAbsolute(absolutePath)) {
Expand Down
30 changes: 12 additions & 18 deletions apps/api-extractor/src/generators/dtsRollup/AstSymbolTable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { AstSymbol } from './AstSymbol';
import { AstImport } from './AstImport';
import { AstEntryPoint, IExportedMember } from './AstEntryPoint';
import { PackageMetadataManager } from './PackageMetadataManager';
import { ILogger } from '../../extractor/ILogger';

/**
* AstSymbolTable is the workhorse that builds AstSymbol and AstDeclaration objects.
Expand All @@ -22,6 +23,7 @@ import { PackageMetadataManager } from './PackageMetadataManager';
* is "exported" or not.)
*/
export class AstSymbolTable {
private readonly _program: ts.Program;
private readonly _typeChecker: ts.TypeChecker;
private readonly _packageJsonLookup: PackageJsonLookup;
private readonly _packageMetadataManager: PackageMetadataManager;
Expand Down Expand Up @@ -54,10 +56,13 @@ export class AstSymbolTable {
private readonly _astEntryPointsBySourceFile: Map<ts.SourceFile, AstEntryPoint>
= new Map<ts.SourceFile, AstEntryPoint>();

public constructor(typeChecker: ts.TypeChecker, packageJsonLookup: PackageJsonLookup) {
public constructor(program: ts.Program, typeChecker: ts.TypeChecker, packageJsonLookup: PackageJsonLookup,
logger: ILogger) {

this._program = program;
this._typeChecker = typeChecker;
this._packageJsonLookup = packageJsonLookup;
this._packageMetadataManager = new PackageMetadataManager(packageJsonLookup);
this._packageMetadataManager = new PackageMetadataManager(packageJsonLookup, logger);
}

/**
Expand All @@ -73,19 +78,6 @@ export class AstSymbolTable {
throw new Error('Unable to find a root declaration for ' + sourceFile.fileName);
}

if (!this._packageMetadataManager.isAedocSupportedFor(sourceFile.fileName)) {
const packageJsonPath: string | undefined = this._packageJsonLookup
.tryGetPackageJsonFilePathFor(sourceFile.fileName);

if (packageJsonPath) {
throw new Error(`Please add a field such as "tsdoc": { "tsdocFlavor": "AEDoc" } to this file:\n`
+ packageJsonPath);
} else {
throw new Error(`The specified entry point does not appear to have an associated package.json file:\n`
+ sourceFile.fileName);
}
}

const exportSymbols: ts.Symbol[] = this._typeChecker.getExportsOfModule(rootFileSymbol) || [];

const exportedMembers: IExportedMember[] = [];
Expand Down Expand Up @@ -314,9 +306,11 @@ export class AstSymbolTable {

// If the file is from a package that does not support AEDoc, then we process the
// symbol itself, but we don't attempt to process any parent/children of it.
if (!this._packageMetadataManager.isAedocSupportedFor(
followedSymbol.declarations[0].getSourceFile().fileName)) {
nominal = true;
const followedSymbolSourceFile: ts.SourceFile = followedSymbol.declarations[0].getSourceFile();
if (this._program.isSourceFileFromExternalLibrary(followedSymbolSourceFile)) {
if (!this._packageMetadataManager.isAedocSupportedFor(followedSymbolSourceFile.fileName)) {
nominal = true;
}
}

let parentAstSymbol: AstSymbol | undefined = undefined;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ export class DtsRollupGenerator {
this._context = context;
this._typeChecker = context.typeChecker;
this._tsdocParser = new tsdoc.TSDocParser();
this._astSymbolTable = new AstSymbolTable(this._context.typeChecker, this._context.packageJsonLookup);
this._astSymbolTable = new AstSymbolTable(this._context.program, this._context.typeChecker,
this._context.packageJsonLookup, context.logger);
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,17 @@
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
// See LICENSE in the project root for license information.

import * as path from 'path';

import {
PackageJsonLookup,
IPackageJson
IPackageJson,
FileSystem,
JsonFile,
NewlineKind
} from '@microsoft/node-core-library';
import { Extractor } from '../../extractor/Extractor';
import { ILogger } from '../../extractor/ILogger';

/**
* Represents analyzed information for a package.json file.
Expand All @@ -26,37 +33,64 @@ export class PackageMetadata {
*/
public readonly aedocSupported: boolean;

private readonly _packageJsonLookup: PackageJsonLookup;

public constructor(packageJsonPath: string, packageJsonLookup: PackageJsonLookup) {
this._packageJsonLookup = packageJsonLookup;
public constructor(packageJsonPath: string, packageJson: IPackageJson, aedocSupported: boolean) {
this.packageJsonPath = packageJsonPath;

this.packageJson = this._packageJsonLookup.loadPackageJson(packageJsonPath);

this.aedocSupported = false;

if (this.packageJson.tsdoc) {
if (this.packageJson.tsdoc.tsdocFlavor) {
if (this.packageJson.tsdoc.tsdocFlavor.toUpperCase() === 'AEDOC') {
this.aedocSupported = true;
}
}
}
this.packageJson = packageJson;
this.aedocSupported = aedocSupported;
}
}

/**
* This class maintains a cache of analyzed information obtained from package.json
* files. It is built on top of the PackageJsonLookup class.
*
* @remarks
*
* IMPORTANT: Don't use PackageMetadataManager to analyze source files from the current project:
* 1. Files such as tsdoc-metadata.json may not have been built yet, and thus may contain incorrect information.
* 2. The current project is not guaranteed to have a package.json file at all. For example, API Extractor can
* be invoked on a bare .d.ts file.
*
* Use ts.program.isSourceFileFromExternalLibrary() to test source files before passing the to PackageMetadataManager.
Copy link
Member

@iclanton iclanton Nov 27, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would the perf impact be if we tested for this in PackageMetadataManager? #WontFix

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally if you're inside PackageMetadataManager for a local file, it's already too late.

I could add an assertion here to catch that mistake however.

PackageMetadataManager is a fairly half-baked file right now, though. I want to make tsdoc-metadata.json a feature of the TSDoc library, and then we'll redesign PackageMetadataManager when we update it to use that.


In reply to: 236576224 [](ancestors = 236576224)

*/
export class PackageMetadataManager {
public static tsdocMetadataFilename: string = 'tsdoc-metadata.json';

private readonly _packageJsonLookup: PackageJsonLookup;
private readonly _logger: ILogger;
private readonly _packageMetadataByPackageJsonPath: Map<string, PackageMetadata>
= new Map<string, PackageMetadata>();

public constructor(packageJsonLookup: PackageJsonLookup) {
public static writeTsdocMetadataFile(packageJsonFolder: string): void {
// This feature is still being standardized: https://github.com/Microsoft/tsdoc/issues/7
// In the future we will use the @microsoft/tsdoc library to read this file.
const tsdocMetadataPath: string = path.join(packageJsonFolder,
'dist', PackageMetadataManager.tsdocMetadataFilename);
Copy link
Member

@iclanton iclanton Nov 27, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dist is an arbitrary assumption to make. This should probably be placed in the same directory as the rollup file. #Resolved

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rollup file may not exist. But we could put it in the same directory as the default entry point for the NPM package.


In reply to: 236576554 [](ancestors = 236576554)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated TSDoc #7 to reflect this idea.

However I'm not going to implement it here, since this is a temporary workaround.


In reply to: 236597139 [](ancestors = 236597139,236576554)


const fileObject: Object = {
tsdocVersion: '0.12',
toolPackages: [
{
packageName: '@microsoft/api-extractor',
packageVersion: Extractor.version
}
]
};

const fileContent: string =
'// This file is read by tools that parse documentation comments conforming to the TSDoc standard.\n'
+ '// It should be published with your NPM package. It should not be tracked by Git.\n'
+ JsonFile.stringify(fileObject);

FileSystem.writeFile(tsdocMetadataPath, fileContent, {
convertLineEndings: NewlineKind.CrLf,
ensureFolderExists: true
});
}

public constructor(packageJsonLookup: PackageJsonLookup, logger: ILogger) {
this._packageJsonLookup = packageJsonLookup;
this._logger = logger;
}

/**
Expand All @@ -72,16 +106,34 @@ export class PackageMetadataManager {
}
let packageMetadata: PackageMetadata | undefined
= this._packageMetadataByPackageJsonPath.get(packageJsonFilePath);

if (!packageMetadata) {
packageMetadata = new PackageMetadata(packageJsonFilePath, this._packageJsonLookup);
const packageJson: IPackageJson = this._packageJsonLookup.loadPackageJson(packageJsonFilePath);

const packageJsonFolder: string = path.dirname(packageJsonFilePath);

// This feature is still being standardized: https://github.com/Microsoft/tsdoc/issues/7
// In the future we will use the @microsoft/tsdoc library to read this file.
let aedocSupported: boolean = false;

const tsdocMetadataPath: string = path.join(packageJsonFolder,
'dist', PackageMetadataManager.tsdocMetadataFilename);

if (FileSystem.exists(tsdocMetadataPath)) {
this._logger.logVerbose('Found metadata in ' + tsdocMetadataPath);
// If the file exists at all, assume it was written by API Extractor
aedocSupported = true;
}

packageMetadata = new PackageMetadata(packageJsonFilePath, packageJson, aedocSupported);
this._packageMetadataByPackageJsonPath.set(packageJsonFilePath, packageMetadata);
}

return packageMetadata;
}

/**
* Returns true if the source file has an associated PackageMetadata object
* with aedocSupported=true.
* Returns true if the source file is part of a package whose .d.ts files support AEDoc annotations.
*/
public isAedocSupportedFor(sourceFilePath: string): boolean {
const packageMetadata: PackageMetadata | undefined = this.tryFetchPackageMetadata(sourceFilePath);
Expand Down
11 changes: 3 additions & 8 deletions apps/api-extractor/src/start.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,13 @@

import * as os from 'os';
import * as colors from 'colors';
import * as path from 'path';

import { FileConstants } from '@microsoft/node-core-library';

import { ApiExtractorCommandLine } from './cli/ApiExtractorCommandLine';
import { Extractor } from './extractor/Extractor';

const myPackageJsonFilename: string = path.resolve(path.join(
__dirname, '..', FileConstants.PackageJson)
);
const myPackageJson: { version: string } = require(myPackageJsonFilename);
const myPackageVersion: string = Extractor.version;

console.log(os.EOL + colors.bold(`api-extractor ${myPackageJson.version} `
console.log(os.EOL + colors.bold(`api-extractor ${myPackageVersion} `
+ colors.cyan(' - http://aka.ms/extractor') + os.EOL));

const parser: ApiExtractorCommandLine = new ApiExtractorCommandLine();
Expand Down
Loading