Skip to content

Commit 1dbd41d

Browse files
authored
[eslint-plugin-azure-sdk] New rule to make sure TSdoc comments for private members don't use internal tags (Azure#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
1 parent 103d45e commit 1dbd41d

File tree

10 files changed

+355
-6
lines changed

10 files changed

+355
-6
lines changed

common/tools/eslint-plugin-azure-sdk/README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ Some rules (see table below) are fixable using the `--fix` ESLint option (added
9393
| [**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` |
9494
| [**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` |
9595
| [**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` |
96+
| [**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` |
9697
| [**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` |
9798
| [**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` |
9899
| [**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` |
Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
# ts-doc-internal-private-member
2+
3+
Prevents the usage of the `@internal` tag in TSDoc comments for private members.
4+
5+
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.**
6+
7+
## Examples
8+
9+
### Good
10+
11+
```ts
12+
/**
13+
* Class documentation
14+
*/
15+
class ExampleClass {
16+
private x = 0;
17+
private y: number;
18+
19+
private [s: string]: boolean | ((s: string) => boolean);
20+
21+
/**
22+
* Method documentation
23+
*/
24+
private testMethod(private x: number, private y: number) {}
25+
}
26+
```
27+
28+
29+
```ts
30+
/**
31+
* Class documentation
32+
*/
33+
class ExampleClass {
34+
/**
35+
* @internal
36+
*/
37+
x = 0;
38+
39+
/**
40+
* @internal
41+
*/
42+
y: number;
43+
44+
/**
45+
* @internal
46+
*/
47+
[s: string]: boolean | ((s: string) => boolean);
48+
49+
/**
50+
* Method documentation
51+
* @internal
52+
*/
53+
testMethod(private x: number, private y: number) {}
54+
}
55+
```
56+
57+
### Bad
58+
59+
```ts
60+
/**
61+
* Class documentation
62+
*/
63+
class ExampleClass {
64+
/**
65+
* @internal
66+
*/
67+
private x = 0;
68+
69+
/**
70+
* @internal
71+
*/
72+
private y: number;
73+
74+
/**
75+
* @internal
76+
*/
77+
private [s: string]: boolean | ((s: string) => boolean);
78+
79+
/**
80+
* Method documentation
81+
* @internal
82+
*/
83+
private testMethod(private x: number, private y: number) {}
84+
}
85+
```
86+
87+
## When to turn off
88+
89+
If you are not using TypeDoc.
90+
91+
## Source
92+
93+
Suggestion by [@deyaaeldeen](https://github.com/deyaaeldeen).

common/tools/eslint-plugin-azure-sdk/src/configs/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ export = {
2828
"@azure/azure-sdk/ts-apisurface-supportcancellation": "error",
2929
"@azure/azure-sdk/ts-config-include": "error",
3030
"@azure/azure-sdk/ts-doc-internal": "error",
31+
"@azure/azure-sdk/ts-doc-internal-private-member": "warn",
3132
"@azure/azure-sdk/ts-error-handling": "off",
3233
"@azure/azure-sdk/ts-modules-only-named": "error",
3334
"@azure/azure-sdk/ts-naming-drop-noun": "error",

common/tools/eslint-plugin-azure-sdk/src/rules/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import tsApisurfaceStandardizedVerbs from "./ts-apisurface-standardized-verbs";
1212
import tsApisurfaceSupportcancellation from "./ts-apisurface-supportcancellation";
1313
import tsConfigInclude from "./ts-config-include";
1414
import tsDocInternal from "./ts-doc-internal";
15+
import tsDocInternalPrivateMember from "./ts-doc-internal-private-member";
1516
import tsErrorHandling from "./ts-error-handling";
1617
import tsModulesOnlyNamed from "./ts-modules-only-named";
1718
import tsNamingDropNoun from "./ts-naming-drop-noun";
@@ -49,6 +50,7 @@ export = {
4950
"ts-apisurface-supportcancellation": tsApisurfaceSupportcancellation,
5051
"ts-config-include": tsConfigInclude,
5152
"ts-doc-internal": tsDocInternal,
53+
"ts-doc-internal-private-member": tsDocInternalPrivateMember,
5254
"ts-error-handling": tsErrorHandling,
5355
"ts-modules-only-named": tsModulesOnlyNamed,
5456
"ts-naming-drop-noun": tsNamingDropNoun,
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT license.
3+
4+
/**
5+
* @file Rule to check if a private class member is tagged with @internal
6+
* @author Hamsa Shankar
7+
*/
8+
9+
import { ParserServices, TSESTree } from "@typescript-eslint/experimental-utils";
10+
import { Node } from "estree";
11+
import { ParserWeakMapESTreeToTSNode } from "@typescript-eslint/typescript-estree/dist/parser-options";
12+
import { Rule } from "eslint";
13+
import { SyntaxKind } from "typescript";
14+
import { getRuleMetaData } from "../utils";
15+
16+
//------------------------------------------------------------------------------
17+
// Rule Definition
18+
//------------------------------------------------------------------------------
19+
20+
/**
21+
* Helper method for reporting on a node
22+
* @param node the Node being operated on
23+
* @param context the ESLint runtime context
24+
* @param converter a converter from TSESTree Nodes to TSNodes
25+
* @param typeChecker the TypeScript TypeChecker
26+
* @throws if the Node passes throught the initial checks and has an internal tag
27+
*/
28+
const reportInternal = (
29+
node: Node,
30+
context: Rule.RuleContext,
31+
converter: ParserWeakMapESTreeToTSNode,
32+
): void => {
33+
const tsNode = converter.get(node as TSESTree.Node);
34+
const modifiers = tsNode.modifiers;
35+
36+
// if type is internal and has a TSDoc and is a private member
37+
if ((tsNode as any).jsDoc !== undefined
38+
&& modifiers?.some((modifier) => modifier.kind === SyntaxKind.PrivateKeyword)) {
39+
// fetch all tags
40+
for (const comment of (tsNode as any).jsDoc) {
41+
if (comment.tags !== undefined) {
42+
for (const tag of comment.tags) {
43+
if (tag.tagName.escapedText.match(/internal/)) {
44+
context.report({
45+
node,
46+
message: "private class members should not include an @internal tag"
47+
});
48+
}
49+
}
50+
}
51+
}
52+
}
53+
};
54+
55+
export = {
56+
meta: getRuleMetaData(
57+
"ts-doc-internal-private-member",
58+
"requires TSDoc comments to not include an '@internal' tag if the object is private"
59+
),
60+
create: (context: Rule.RuleContext): Rule.RuleListener => {
61+
62+
const parserServices = context.parserServices as ParserServices;
63+
if (
64+
parserServices.program === undefined ||
65+
parserServices.esTreeNodeToTSNodeMap === undefined
66+
) {
67+
return {};
68+
}
69+
70+
const converter = parserServices.esTreeNodeToTSNodeMap;
71+
72+
return {
73+
":matches(ClassProperty, MethodDefinition, TSMethodSignature, TSPropertySignature, TSIndexSignature, TSParameterProperty)": (
74+
node: Node
75+
): void => reportInternal(node, context, converter)
76+
};
77+
}
78+
};

common/tools/eslint-plugin-azure-sdk/tests/plugin.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ const ruleList = [
2020
"ts-apisurface-supportcancellation",
2121
"ts-config-include",
2222
"ts-doc-internal",
23+
"ts-doc-internal-private-member",
2324
"ts-error-handling",
2425
"ts-modules-only-named",
2526
"ts-naming-drop-noun",
Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT license.
3+
4+
/**
5+
* @file Testing the ts-doc-internal-private-member rule.
6+
* @author Hamsa Shankar
7+
*/
8+
9+
import { RuleTester } from "eslint";
10+
import rule from "../../src/rules/ts-doc-internal-private-member";
11+
12+
//------------------------------------------------------------------------------
13+
// Tests
14+
//------------------------------------------------------------------------------
15+
16+
const ruleTester = new RuleTester({
17+
parser: require.resolve("@typescript-eslint/parser"),
18+
parserOptions: {
19+
createDefaultProgram: true,
20+
project: "./tsconfig.json"
21+
}
22+
});
23+
24+
ruleTester.run("ts-doc-internal-private-member", rule, {
25+
valid: [
26+
// all private
27+
{
28+
code: `
29+
/**
30+
* Class documentation
31+
*/
32+
class ExampleClass {
33+
/**
34+
* Class Property
35+
*/
36+
private x = 0;
37+
38+
/**
39+
* Property Signature
40+
*/
41+
private y: number;
42+
43+
/**
44+
* Index signature
45+
*/
46+
private [s: string]: boolean | ((s: string) => boolean);
47+
48+
/**
49+
* Parameter Property
50+
*/
51+
private testMethod(private x: number, private y: number) {}
52+
53+
/**
54+
* Method Definition
55+
*/
56+
private get getter(): number { return 0 }
57+
58+
/**
59+
* Method Signature
60+
*/
61+
private method1(): any;
62+
63+
}`,
64+
filename: "src/test.ts"
65+
},
66+
// all internal
67+
{
68+
code: `
69+
/**
70+
* Class documentation
71+
*/
72+
class ExampleClass {
73+
/**
74+
* Class Property
75+
* @internal
76+
*/
77+
x = 0;
78+
79+
/**
80+
* Property Signature
81+
* @internal
82+
*/
83+
y: number;
84+
85+
/**
86+
* Index signature
87+
* @internal
88+
*/
89+
[s: string]: boolean | ((s: string) => boolean);
90+
91+
/**
92+
* Parameter Property
93+
* @internal
94+
*/
95+
testMethod(private x: number, private y: number) {}
96+
97+
/**
98+
* Method Definition
99+
* @internal
100+
*/
101+
get getter(): number { return 0 }
102+
103+
/**
104+
* Method Signature
105+
* @internal
106+
*/
107+
method1(): any;
108+
}`,
109+
filename: "src/test.ts"
110+
}
111+
],
112+
invalid: [
113+
{
114+
code: `
115+
/**
116+
* Class documentation
117+
*/
118+
class ExampleClass {
119+
/**
120+
* Class Property
121+
* @internal
122+
*/
123+
private x = 0;
124+
125+
/**
126+
* Property Signature
127+
* @internal
128+
*/
129+
private y: number;
130+
131+
/**
132+
* Index signature
133+
* @internal
134+
*/
135+
private [s: string]: boolean | ((s: string) => boolean);
136+
137+
/**
138+
* Parameter Property
139+
* @internal
140+
*/
141+
private testMethod(private x: number, private y: number) {}
142+
143+
/**
144+
* Method Definition
145+
* @internal
146+
*/
147+
private get getter(): number { return 0 }
148+
149+
/**
150+
* Method Signature
151+
* @internal
152+
*/
153+
private method1(): any;
154+
155+
}`,
156+
filename: "src/test.ts",
157+
errors: [
158+
{
159+
message: "private class members should not include an @internal tag"
160+
},
161+
{
162+
message: "private class members should not include an @internal tag"
163+
},
164+
{
165+
message: "private class members should not include an @internal tag"
166+
},
167+
{
168+
message: "private class members should not include an @internal tag"
169+
},
170+
{
171+
message: "private class members should not include an @internal tag"
172+
},
173+
{
174+
message: "private class members should not include an @internal tag"
175+
}
176+
]
177+
}
178+
]
179+
});

0 commit comments

Comments
 (0)