Skip to content

Commit

Permalink
[eslint-plugin-azure-sdk] New rule to make sure TSdoc comments for pr…
Browse files Browse the repository at this point in the history
…ivate members don't use internal tags (#18761)

* worked on linter private member internal tag rule

* fixed linter errors in packages and removed exportedSettings

* fixed formatting in files with linter errors

* removed excluding code

* removed interfaces and modules check

* checking for all syntax nodes with accessibility modifier

* fixed sorted imports

* style changes

* fixed import sorting
  • Loading branch information
hamsa-s authored Feb 2, 2022
1 parent 103d45e commit 1dbd41d
Show file tree
Hide file tree
Showing 10 changed files with 355 additions and 6 deletions.
1 change: 1 addition & 0 deletions common/tools/eslint-plugin-azure-sdk/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ Some rules (see table below) are fixable using the `--fix` ESLint option (added
| [**ts-apisurface-supportcancellation**](https://github.com/Azure/azure-sdk-for-js/blob/main/common/tools/eslint-plugin-azure-sdk/docs/rules/ts-apisurface-supportcancellation.md) | :triangular_flag_on_post: | :x: | `1.2.0` |
| [**ts-config-include**](https://github.com/Azure/azure-sdk-for-js/blob/main/common/tools/eslint-plugin-azure-sdk/docs/rules/ts-config-include.md) | :triangular_flag_on_post: | :heavy_check_mark: | `1.0.0` |
| [**ts-doc-internal**](https://github.com/Azure/azure-sdk-for-js/blob/main/common/tools/eslint-plugin-azure-sdk/docs/rules/ts-doc-internal.md) | :triangular_flag_on_post: | :x: | `1.1.0` |
| [**ts-doc-internal-private-member**](https://github.com/Azure/azure-sdk-for-js/blob/main/common/tools/eslint-plugin-azure-sdk/docs/rules/ts-doc-internal-private-member.md) | :triangular_flag_on_post: | :x: | `3.1.0` |
| [**ts-error-handling**](https://github.com/Azure/azure-sdk-for-js/blob/main/common/tools/eslint-plugin-azure-sdk/docs/rules/ts-error-handling.md) | :heavy_multiplication_x: | :x: | `1.1.0` |
| [**ts-modules-only-named**](https://github.com/Azure/azure-sdk-for-js/blob/main/common/tools/eslint-plugin-azure-sdk/docs/rules/ts-modules-only-named.md) | :triangular_flag_on_post: | :x: | `1.1.0` |
| [**ts-naming-drop-noun**](https://github.com/Azure/azure-sdk-for-js/blob/main/common/tools/eslint-plugin-azure-sdk/docs/rules/ts-naming-drop-noun.md) | :triangular_flag_on_post: | :x: | `1.2.0` |
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
# ts-doc-internal-private-member

Prevents the usage of the `@internal` tag in TSDoc comments for private members.

Internal objects are defined as classes, interfaces, or standalone functions that are not exported from the main entrypoint to the package and are not members of any exports from the main entrypoint (defined recursively). Because private members are already ignored by the docs generation tools, we do not need to use `@internal` in the TSDoc comments **Files that are specified as excluded in a `typedoc.json` file are ignored by this rule.**

## Examples

### Good

```ts
/**
* Class documentation
*/
class ExampleClass {
private x = 0;
private y: number;

private [s: string]: boolean | ((s: string) => boolean);

/**
* Method documentation
*/
private testMethod(private x: number, private y: number) {}
}
```


```ts
/**
* Class documentation
*/
class ExampleClass {
/**
* @internal
*/
x = 0;

/**
* @internal
*/
y: number;

/**
* @internal
*/
[s: string]: boolean | ((s: string) => boolean);

/**
* Method documentation
* @internal
*/
testMethod(private x: number, private y: number) {}
}
```

### Bad

```ts
/**
* Class documentation
*/
class ExampleClass {
/**
* @internal
*/
private x = 0;

/**
* @internal
*/
private y: number;

/**
* @internal
*/
private [s: string]: boolean | ((s: string) => boolean);

/**
* Method documentation
* @internal
*/
private testMethod(private x: number, private y: number) {}
}
```

## When to turn off

If you are not using TypeDoc.

## Source

Suggestion by [@deyaaeldeen](https://github.com/deyaaeldeen).
1 change: 1 addition & 0 deletions common/tools/eslint-plugin-azure-sdk/src/configs/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export = {
"@azure/azure-sdk/ts-apisurface-supportcancellation": "error",
"@azure/azure-sdk/ts-config-include": "error",
"@azure/azure-sdk/ts-doc-internal": "error",
"@azure/azure-sdk/ts-doc-internal-private-member": "warn",
"@azure/azure-sdk/ts-error-handling": "off",
"@azure/azure-sdk/ts-modules-only-named": "error",
"@azure/azure-sdk/ts-naming-drop-noun": "error",
Expand Down
2 changes: 2 additions & 0 deletions common/tools/eslint-plugin-azure-sdk/src/rules/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import tsApisurfaceStandardizedVerbs from "./ts-apisurface-standardized-verbs";
import tsApisurfaceSupportcancellation from "./ts-apisurface-supportcancellation";
import tsConfigInclude from "./ts-config-include";
import tsDocInternal from "./ts-doc-internal";
import tsDocInternalPrivateMember from "./ts-doc-internal-private-member";
import tsErrorHandling from "./ts-error-handling";
import tsModulesOnlyNamed from "./ts-modules-only-named";
import tsNamingDropNoun from "./ts-naming-drop-noun";
Expand Down Expand Up @@ -49,6 +50,7 @@ export = {
"ts-apisurface-supportcancellation": tsApisurfaceSupportcancellation,
"ts-config-include": tsConfigInclude,
"ts-doc-internal": tsDocInternal,
"ts-doc-internal-private-member": tsDocInternalPrivateMember,
"ts-error-handling": tsErrorHandling,
"ts-modules-only-named": tsModulesOnlyNamed,
"ts-naming-drop-noun": tsNamingDropNoun,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

/**
* @file Rule to check if a private class member is tagged with @internal
* @author Hamsa Shankar
*/

import { ParserServices, TSESTree } from "@typescript-eslint/experimental-utils";
import { Node } from "estree";
import { ParserWeakMapESTreeToTSNode } from "@typescript-eslint/typescript-estree/dist/parser-options";
import { Rule } from "eslint";
import { SyntaxKind } from "typescript";
import { getRuleMetaData } from "../utils";

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------

/**
* Helper method for reporting on a node
* @param node the Node being operated on
* @param context the ESLint runtime context
* @param converter a converter from TSESTree Nodes to TSNodes
* @param typeChecker the TypeScript TypeChecker
* @throws if the Node passes throught the initial checks and has an internal tag
*/
const reportInternal = (
node: Node,
context: Rule.RuleContext,
converter: ParserWeakMapESTreeToTSNode,
): void => {
const tsNode = converter.get(node as TSESTree.Node);
const modifiers = tsNode.modifiers;

// if type is internal and has a TSDoc and is a private member
if ((tsNode as any).jsDoc !== undefined
&& modifiers?.some((modifier) => modifier.kind === SyntaxKind.PrivateKeyword)) {
// fetch all tags
for (const comment of (tsNode as any).jsDoc) {
if (comment.tags !== undefined) {
for (const tag of comment.tags) {
if (tag.tagName.escapedText.match(/internal/)) {
context.report({
node,
message: "private class members should not include an @internal tag"
});
}
}
}
}
}
};

export = {
meta: getRuleMetaData(
"ts-doc-internal-private-member",
"requires TSDoc comments to not include an '@internal' tag if the object is private"
),
create: (context: Rule.RuleContext): Rule.RuleListener => {

const parserServices = context.parserServices as ParserServices;
if (
parserServices.program === undefined ||
parserServices.esTreeNodeToTSNodeMap === undefined
) {
return {};
}

const converter = parserServices.esTreeNodeToTSNodeMap;

return {
":matches(ClassProperty, MethodDefinition, TSMethodSignature, TSPropertySignature, TSIndexSignature, TSParameterProperty)": (
node: Node
): void => reportInternal(node, context, converter)
};
}
};
1 change: 1 addition & 0 deletions common/tools/eslint-plugin-azure-sdk/tests/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const ruleList = [
"ts-apisurface-supportcancellation",
"ts-config-include",
"ts-doc-internal",
"ts-doc-internal-private-member",
"ts-error-handling",
"ts-modules-only-named",
"ts-naming-drop-noun",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

/**
* @file Testing the ts-doc-internal-private-member rule.
* @author Hamsa Shankar
*/

import { RuleTester } from "eslint";
import rule from "../../src/rules/ts-doc-internal-private-member";

//------------------------------------------------------------------------------
// Tests
//------------------------------------------------------------------------------

const ruleTester = new RuleTester({
parser: require.resolve("@typescript-eslint/parser"),
parserOptions: {
createDefaultProgram: true,
project: "./tsconfig.json"
}
});

ruleTester.run("ts-doc-internal-private-member", rule, {
valid: [
// all private
{
code: `
/**
* Class documentation
*/
class ExampleClass {
/**
* Class Property
*/
private x = 0;
/**
* Property Signature
*/
private y: number;
/**
* Index signature
*/
private [s: string]: boolean | ((s: string) => boolean);
/**
* Parameter Property
*/
private testMethod(private x: number, private y: number) {}
/**
* Method Definition
*/
private get getter(): number { return 0 }
/**
* Method Signature
*/
private method1(): any;
}`,
filename: "src/test.ts"
},
// all internal
{
code: `
/**
* Class documentation
*/
class ExampleClass {
/**
* Class Property
* @internal
*/
x = 0;
/**
* Property Signature
* @internal
*/
y: number;
/**
* Index signature
* @internal
*/
[s: string]: boolean | ((s: string) => boolean);
/**
* Parameter Property
* @internal
*/
testMethod(private x: number, private y: number) {}
/**
* Method Definition
* @internal
*/
get getter(): number { return 0 }
/**
* Method Signature
* @internal
*/
method1(): any;
}`,
filename: "src/test.ts"
}
],
invalid: [
{
code: `
/**
* Class documentation
*/
class ExampleClass {
/**
* Class Property
* @internal
*/
private x = 0;
/**
* Property Signature
* @internal
*/
private y: number;
/**
* Index signature
* @internal
*/
private [s: string]: boolean | ((s: string) => boolean);
/**
* Parameter Property
* @internal
*/
private testMethod(private x: number, private y: number) {}
/**
* Method Definition
* @internal
*/
private get getter(): number { return 0 }
/**
* Method Signature
* @internal
*/
private method1(): any;
}`,
filename: "src/test.ts",
errors: [
{
message: "private class members should not include an @internal tag"
},
{
message: "private class members should not include an @internal tag"
},
{
message: "private class members should not include an @internal tag"
},
{
message: "private class members should not include an @internal tag"
},
{
message: "private class members should not include an @internal tag"
},
{
message: "private class members should not include an @internal tag"
}
]
}
]
});
Loading

0 comments on commit 1dbd41d

Please sign in to comment.