Skip to content

Commit

Permalink
Standardization of our documentation comments (#12912)
Browse files Browse the repository at this point in the history
# Status quo
Some of our documentation comments are [TypeDoc](http://typedoc.org/guides/doccomments/) and some of them are JSDoc with type description in the comments even though it is for typed TS code.

# Standardization
I decided the best way to go about this is to migrate to [TSDoc](https://github.com/Microsoft/tsdoc) and enforcing it using the [tsdoc eslint plugin](https://www.npmjs.com/package/eslint-plugin-tsdoc).

Pros:
- TSDoc is a proposal to standardize the doc comments used in TypeScript code, so that different tools can extract content without getting confused by each other’s markup.
- It is being developed at Microsoft, with the TypeScript team.
- It has an ESLint plugin that enforces it and will error when it sees unsupported tags (e.g. `@memberof`, `@class`, `@constructor`, `@type`, etc).

Cons:
- It is still in early stages (adoption is ongoing though, e.g. it is being used by the API extractor tool).
- TSDoc != TypeDoc (the tool we currently use for generating our documentation). However, TypeDoc plans to officially support TSDoc in v1.1 (see TypeStrong/typedoc#1266).

# Notable tag changes
- `@ignore` is a JSDoc tag and was used in conjunction with `@internal`. These tags were needed because [TypeDoc does not yet support documenting only definitions exported by the entry point](TypeStrong/typedoc#1184 (comment)) and still documents everything exported from all files. I removed `@ignore` because [`@internal`](https://tsdoc.org/pages/tags/internal) only should suffice
- `@ignore` when used alone is replaced with TypeDoc's [`hidden`](http://typedoc.org/guides/doccomments/#hidden-and-ignore). EDIT: I replaced `@ignore` with [`@hidden`](https://github.com/TypeStrong/typedoc/releases/tag/v0.12.0) because the TypeDoc version used for `docs.microsoft.com` is v0.15.0 which does not support `--stripInternal`. After, they upgrade, I will remove all `@hidden` tags. 
- `@summary` is gone because it is not part of TSDoc or TypeDoc

This PR applies the changes to packages that respect our linting rules. Ones that do not yet will be migrated later when we start fixing their linting issues.

Here are vanilla examples of TypeDoc 0.18.0 (version used by our EngSys) after the changes here as a sanity check:
- random method:
![typedoc](https://user-images.githubusercontent.com/6074665/102302881-f6186380-3f27-11eb-8cc6-93e4c8f7d42d.PNG)
- a class constructor that used to have type information in the documentation comments:
![constructor](https://user-images.githubusercontent.com/6074665/102357078-f8a4a880-3f7b-11eb-92d1-c086ecc39c0b.PNG)

# `@hidden` works the same way as `@ignore`
Here are the list of documented functions generated by `TypeDoc v0.15.0` for the text analytics package and there is no function that was marked `@hidden`, e.g. `combineSuccessfulAndErroneousDocumentsWithStatisticsAndModelVersion`
![image](https://user-images.githubusercontent.com/6074665/102426196-e018aa80-3fdc-11eb-8b69-1ac265391fad.png)

# Things to consider
- Our documentation must be generated using the TypeDoc flag [`--stripInternal`](http://typedoc.org/guides/options/#stripinternal)
- Should we add a `docs` npm script to our `package.json`s (similar to [Cosmos's](https://github.com/Azure/azure-sdk-for-js/blob/2424b74f029273677f62433f28dd1390806f682c/sdk/cosmosdb/cosmos/package.json#L60)) so that we can see how our docs are generated as we write our comments?

Fixes #3027.
  • Loading branch information
deyaaeldeen committed Dec 17, 2020
1 parent 67f06a6 commit 78db83f
Show file tree
Hide file tree
Showing 177 changed files with 1,492 additions and 1,490 deletions.
25 changes: 23 additions & 2 deletions common/config/rush/pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion common/tools/eslint-plugin-azure-sdk/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@
"@typescript-eslint/parser": "^4.9.0",
"eslint": "^7.15.0",
"eslint-plugin-no-only-tests": "^2.4.0",
"eslint-plugin-promise": "^4.2.1"
"eslint-plugin-promise": "^4.2.1",
"eslint-plugin-tsdoc": "^0.2.10"
},
"dependencies": {
"@typescript-eslint/typescript-estree": "^4.9.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export default {
sourceType: "module",
extraFileExtensions: [".json"]
},
plugins: ["@typescript-eslint", "no-only-tests", "promise"],
plugins: ["@typescript-eslint", "no-only-tests", "promise", "eslint-plugin-tsdoc"],
extends: [
"plugin:@typescript-eslint/recommended",
"eslint:recommended",
Expand Down Expand Up @@ -111,6 +111,7 @@ export default {
// https://github.com/Azure/azure-sdk-for-js/issues/7609
"@azure/azure-sdk/ts-pagination-list": "off",
// https://github.com/Azure/azure-sdk-for-js/issues/7610
"@azure/azure-sdk/ts-doc-internal": "off"
"@azure/azure-sdk/ts-doc-internal": "off",
"tsdoc/syntax": "error"
}
};
1 change: 1 addition & 0 deletions eng/tools/generate-doc/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ const executeTypedoc = async (exclusionList, inclusionList, generateIndexWithTem
"--excludeNotExported",
'--exclude "node_modules/**/*"',
"--ignoreCompilerErrors",
"--stripInternal",
"--mode file",
docOutputFolder,
]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export class AnomalyDetectorClient {

/**
* @internal
* @ignore
* @hidden
* A reference to the auto-generated AnomalyDetector HTTP client.
*/
private client: GeneratedClient;
Expand All @@ -61,9 +61,9 @@ export class AnomalyDetectorClient {
* new AzureKeyCredential("<api key>")
* );
* ```
* @param string endpointUrl Url to an Azure Anomaly Detector service endpoint
* @param {TokenCredential | KeyCredential} credential Used to authenticate requests to the service.
* @param FormRecognizerClientOptions options Used to configure the Form Recognizer client.
* @param endpointUrl - Url to an Azure Anomaly Detector service endpoint
* @param credential - Used to authenticate requests to the service.
* @param options - Used to configure the Form Recognizer client.
*/
constructor(
endpointUrl: string,
Expand Down Expand Up @@ -107,9 +107,9 @@ export class AnomalyDetectorClient {
* This operation generates a model using an entire series, each point is detected with the same model.
* With this method, points before and after a certain point are used to determine whether it is an
* anomaly. The entire detection can give user an overall status of the time series.
* @param body Time series points and period if needed. Advanced model parameters can also be set in
* @param body - Time series points and period if needed. Advanced model parameters can also be set in
* the request.
* @param options The options parameters.
* @param options - The options parameters.
*/
public detectEntireSeries(
body: DetectRequest,
Expand Down Expand Up @@ -138,9 +138,9 @@ export class AnomalyDetectorClient {
* This operation generates a model using points before the latest one. With this method, only
* historical points are used to determine whether the target point is an anomaly. The latest point
* detecting operation matches the scenario of real-time monitoring of business metrics.
* @param body Time series points and period if needed. Advanced model parameters can also be set in
* @param body - Time series points and period if needed. Advanced model parameters can also be set in
* the request.
* @param options The options parameters.
* @param options - The options parameters.
*/
public detectLastPoint(
body: DetectRequest,
Expand All @@ -167,9 +167,9 @@ export class AnomalyDetectorClient {

/**
* Evaluate change point score of every series point
* @param body Time series points and granularity is needed. Advanced model parameters can also be set
* @param body - Time series points and granularity is needed. Advanced model parameters can also be set
* in the request if needed.
* @param options The options parameters.
* @param options - The options parameters.
*/
detectChangePoint(
body: DetectChangePointRequest,
Expand Down
2 changes: 1 addition & 1 deletion sdk/anomalydetector/ai-anomaly-detector/src/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@
import { createClientLogger } from "@azure/logger";

/**
* The @azure/logger configuration for this package.
* The \@azure/logger configuration for this package.
*/
export const logger = createClientLogger("ai-anomaly-detector");
4 changes: 2 additions & 2 deletions sdk/anomalydetector/ai-anomaly-detector/src/models.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export interface DetectRequest {
*/
granularity: TimeGranularity;
/**
* Custom Interval is used to set non-standard time interval, for example, if the series is 5 minutes, request can be set as {"granularity":"minutely", "customInterval":5}.
* Custom Interval is used to set non-standard time interval, for example, if the series is 5 minutes, request can be set as `{"granularity":"minutely", "customInterval":5}`.
*/
customInterval?: number;
/**
Expand All @@ -77,7 +77,7 @@ export interface DetectChangePointRequest {
*/
granularity: TimeGranularity;
/**
* Custom Interval is used to set non-standard time interval, for example, if the series is 5 minutes, request can be set as {"granularity":"minutely", "customInterval":5}.
* Custom Interval is used to set non-standard time interval, for example, if the series is 5 minutes, request can be set as `{"granularity":"minutely", "customInterval":5}`.
*/
customInterval?: number;
/**
Expand Down
7 changes: 4 additions & 3 deletions sdk/anomalydetector/ai-anomaly-detector/src/tracing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@ import { OperationOptions } from "@azure/core-http";

/**
* Creates a span using the global tracer.
* @ignore
* @param name The name of the operation being performed.
* @param tracingOptions The options for the underlying http request.
* @internal
* @hidden
* @param name - The name of the operation being performed.
* @param tracingOptions - The options for the underlying http request.
*/
export function createSpan<T extends OperationOptions>(
operationName: string,
Expand Down
4 changes: 4 additions & 0 deletions sdk/anomalydetector/ai-anomaly-detector/tsdoc.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"$schema": "https://developer.microsoft.com/json-schemas/tsdoc/v0/tsdoc.schema.json",
"extends": ["../../../tsdoc.json"]
}
33 changes: 14 additions & 19 deletions sdk/core/abort-controller/src/AbortController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { AbortSignal, abortSignal, AbortSignalLike } from "./AbortSignal";
* error matches `"AbortError"`.
*
* @example
* ```ts
* const controller = new AbortController();
* controller.abort();
* try {
Expand All @@ -18,6 +19,7 @@ import { AbortSignal, abortSignal, AbortSignalLike } from "./AbortSignal";
* // handle abort error here.
* }
* }
* ```
*/
export class AbortError extends Error {
constructor(message?: string) {
Expand All @@ -31,44 +33,44 @@ export class AbortError extends Error {
* that an asynchronous operation should be aborted.
*
* @example
* // Abort an operation when another event fires
* Abort an operation when another event fires
* ```ts
* const controller = new AbortController();
* const signal = controller.signal;
* doAsyncWork(signal);
* button.addEventListener('click', () => controller.abort());
* ```
*
* @example
* // Share aborter cross multiple operations in 30s
* Share aborter cross multiple operations in 30s
* ```ts
* // Upload the same data to 2 different data centers at the same time,
* // abort another when any of them is finished
* const controller = AbortController.withTimeout(30 * 1000);
* doAsyncWork(controller.signal).then(controller.abort);
* doAsyncWork(controller.signal).then(controller.abort);
*```
*
* @example
* // Cascaded aborting
* Cascaded aborting
* ```ts
* // All operations can't take more than 30 seconds
* const aborter = Aborter.timeout(30 * 1000);
*
* // Following 2 operations can't take more than 25 seconds
* await doAsyncWork(aborter.withTimeout(25 * 1000));
* await doAsyncWork(aborter.withTimeout(25 * 1000));
*
* @export
* @class AbortController
* @implements {AbortSignalLike}
* ```
*/
export class AbortController {
private _signal: AbortSignal;

/**
* @param {AbortSignalLike[]} [parentSignals] The AbortSignals that will signal aborted on the AbortSignal associated with this controller.
* @constructor
* @param parentSignals - The AbortSignals that will signal aborted on the AbortSignal associated with this controller.
*/
constructor(parentSignals?: AbortSignalLike[]);
/**
* @param {...AbortSignalLike} parentSignals The AbortSignals that will signal aborted on the AbortSignal associated with this controller.
* @constructor
* @param parentSignals - The AbortSignals that will signal aborted on the AbortSignal associated with this controller.
*/
constructor(...parentSignals: AbortSignalLike[]);
// eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
Expand Down Expand Up @@ -102,8 +104,6 @@ export class AbortController {
* when the abort method is called on this controller.
*
* @readonly
* @type {AbortSignal}
* @memberof AbortController
*/
public get signal(): AbortSignal {
return this._signal;
Expand All @@ -112,19 +112,14 @@ export class AbortController {
/**
* Signal that any operations passed this controller's associated abort signal
* to cancel any remaining work and throw an `AbortError`.
*
* @memberof AbortController
*/
abort(): void {
abortSignal(this._signal);
}

/**
* Creates a new AbortSignal instance that will abort after the provided ms.
*
* @static
* @params {number} ms Elapsed time in milliseconds to trigger an abort.
* @returns {AbortSignal}
* @param ms - Elapsed time in milliseconds to trigger an abort.
*/
public static timeout(ms: number): AbortSignal {
const signal = new AbortSignal();
Expand Down
25 changes: 7 additions & 18 deletions sdk/core/abort-controller/src/AbortSignal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,10 @@ export interface AbortSignalLike {
* cannot or will not ever be cancelled.
*
* @example
* // Abort without timeout
* Abort without timeout
* ```ts
* await doAsyncWork(AbortSignal.none);
*
* @export
* @class AbortSignal
* @implements {AbortSignalLike}
* ```
*/
export class AbortSignal implements AbortSignalLike {
constructor() {
Expand All @@ -61,8 +59,6 @@ export class AbortSignal implements AbortSignalLike {
* Status of whether aborted or not.
*
* @readonly
* @type {boolean}
* @memberof AbortSignal
*/
public get aborted(): boolean {
if (!abortedMap.has(this)) {
Expand All @@ -76,27 +72,21 @@ export class AbortSignal implements AbortSignalLike {
* Creates a new AbortSignal instance that will never be aborted.
*
* @readonly
* @static
* @type {AbortSignal}
* @memberof AbortSignal
*/
public static get none(): AbortSignal {
return new AbortSignal();
}

/**
* onabort event listener.
*
* @memberof AbortSignal
*/
public onabort: ((ev?: Event) => any) | null = null;

/**
* Added new "abort" event listener, only support "abort" event.
*
* @param {"abort"} _type Only support "abort" event
* @param {(this: AbortSignalLike, ev: any) => any} listener
* @memberof AbortSignal
* @param _type - Only support "abort" event
* @param listener - The listener to be added
*/
public addEventListener(
// tslint:disable-next-line:variable-name
Expand All @@ -114,9 +104,8 @@ export class AbortSignal implements AbortSignalLike {
/**
* Remove "abort" event listener, only support "abort" event.
*
* @param {"abort"} _type Only support "abort" event
* @param {(this: AbortSignalLike, ev: any) => any} listener
* @memberof AbortSignal
* @param _type - Only support "abort" event
* @param listener - The listener to be removed
*/
public removeEventListener(
// tslint:disable-next-line:variable-name
Expand Down
4 changes: 2 additions & 2 deletions sdk/core/core-auth/src/azureKeyCredential.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export class AzureKeyCredential implements KeyCredential {
* Create an instance of an AzureKeyCredential for use
* with a service client.
*
* @param key the initial value of the key to use in authentication
* @param key - The initial value of the key to use in authentication
*/
constructor(key: string) {
if (!key) {
Expand All @@ -45,7 +45,7 @@ export class AzureKeyCredential implements KeyCredential {
* Updates will take effect upon the next request after
* updating the key value.
*
* @param newKey the new key value to be used
* @param newKey - The new key value to be used
*/
public update(newKey: string): void {
this._key = newKey;
Expand Down

0 comments on commit 78db83f

Please sign in to comment.