From c649b9f5c4b74bb09ddbd03510c7d05ba4296fcc Mon Sep 17 00:00:00 2001 From: deyaaeldeen Date: Sun, 20 Sep 2020 16:25:35 -0400 Subject: [PATCH 1/3] better handling of class constructors in ts-naming-options rule --- .../docs/rules/ts-naming-options.md | 8 +++++++- .../src/rules/ts-naming-options.ts | 13 +++++++++---- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/common/tools/eslint-plugin-azure-sdk/docs/rules/ts-naming-options.md b/common/tools/eslint-plugin-azure-sdk/docs/rules/ts-naming-options.md index 2e24a43f564b..6fc366975ef9 100644 --- a/common/tools/eslint-plugin-azure-sdk/docs/rules/ts-naming-options.md +++ b/common/tools/eslint-plugin-azure-sdk/docs/rules/ts-naming-options.md @@ -8,10 +8,13 @@ Requires client method options parameter types to be suffixed with `Options` and ```ts class ServiceClient { + constructor(options: ServiceClientOptions) { + /* code */ + } createItem(options: CreateItemOptions): Item { /* code to return instance of Item */ } - upsertItem(options: UpsertItemOptions): Item { + insertItem(options: InsertItemOptions): Item { /* code to return instance of Item */ } } @@ -21,6 +24,9 @@ class ServiceClient { ```ts class ServiceClient { + constructor(options: Options) { + /* code */ + } createItem(options: Options): Item { /* code to return instance of Item */ } diff --git a/common/tools/eslint-plugin-azure-sdk/src/rules/ts-naming-options.ts b/common/tools/eslint-plugin-azure-sdk/src/rules/ts-naming-options.ts index afad7fee1208..1303e07ade0c 100644 --- a/common/tools/eslint-plugin-azure-sdk/src/rules/ts-naming-options.ts +++ b/common/tools/eslint-plugin-azure-sdk/src/rules/ts-naming-options.ts @@ -2,7 +2,7 @@ // Licensed under the MIT license. /** - * @file Rule to require client method option parameter type names to be suffixed with Options and prefixed with the method name. + * @file Rule to require client method option parameter type names to be suffixed with Options and prefixed with the class name if it is a class constructor and prefixed with the method name otherwise. * @author Arpan Laha */ @@ -33,7 +33,12 @@ export = { getPublicMethods(node).forEach((method: MethodDefinition): void => { const methodIdentifier = method.key as Identifier; const TSFunction = method.value as TSESTree.FunctionExpression; - + const optionsRegex = + // the null check will always succeed because we apply this only for + // classes where id.name=/Client$/. + method.kind === "constructor" && node.id !== null + ? new RegExp(`${node.id.name}Options`, "i") + : new RegExp(`${methodIdentifier.name}Options`, "i"); // look for parameters with types suffixed with Options TSFunction.params.forEach((param: TSESTree.Parameter): void => { // checks to validate parameter @@ -46,11 +51,11 @@ export = { const paramTypeName = typeAnnotation.typeName.name; if (paramTypeName.endsWith("Options")) { // check that parameter is prefixed with method name - const optionsRegex = new RegExp(`${methodIdentifier.name}Options`, "i"); if (!optionsRegex.test(paramTypeName)) { + const prefixKind = method.kind === "constructor" ? "class" : "method"; context.report({ node: param, - message: "options parameter type is not prefixed with the method name" + message: `options parameter type is not prefixed with the ${prefixKind} name` }); } } From 744170afdfb5ec24e71449fd9898e23abd75db50 Mon Sep 17 00:00:00 2001 From: deyaaeldeen Date: Sun, 20 Sep 2020 16:46:23 -0400 Subject: [PATCH 2/3] adding tests --- .../tests/rules/ts-naming-options.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/common/tools/eslint-plugin-azure-sdk/tests/rules/ts-naming-options.ts b/common/tools/eslint-plugin-azure-sdk/tests/rules/ts-naming-options.ts index 2893856a73ec..128c8fe7a171 100644 --- a/common/tools/eslint-plugin-azure-sdk/tests/rules/ts-naming-options.ts +++ b/common/tools/eslint-plugin-azure-sdk/tests/rules/ts-naming-options.ts @@ -32,6 +32,10 @@ ruleTester.run("ts-naming-options", rule, { code: "class ExampleClient { createExample(options: CreateExampleOptions) {}; upsertExample(options: UpsertExampleOptions) {}; };" }, + // class constructor + { + code: "class ExampleClient { constructor(options: ExampleClientOptions) {}; };" + }, // not a client { code: "class Example { createExample(options: Options) {}; };" @@ -45,6 +49,14 @@ ruleTester.run("ts-naming-options", rule, { message: "options parameter type is not prefixed with the method name" } ] + }, + { + code: "class ExampleClient { constructor(options: Options) {}; };", + errors: [ + { + message: "options parameter type is not prefixed with the class name" + } + ] } ] }); From cd84246c33c3bdf65e6eb7793f7f5b6236d58091 Mon Sep 17 00:00:00 2001 From: deyaaeldeen Date: Thu, 24 Sep 2020 16:28:45 -0400 Subject: [PATCH 3/3] adding upsert to my dictionary --- .../eslint-plugin-azure-sdk/docs/rules/ts-naming-options.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/tools/eslint-plugin-azure-sdk/docs/rules/ts-naming-options.md b/common/tools/eslint-plugin-azure-sdk/docs/rules/ts-naming-options.md index 6fc366975ef9..50c083796dc2 100644 --- a/common/tools/eslint-plugin-azure-sdk/docs/rules/ts-naming-options.md +++ b/common/tools/eslint-plugin-azure-sdk/docs/rules/ts-naming-options.md @@ -14,7 +14,7 @@ class ServiceClient { createItem(options: CreateItemOptions): Item { /* code to return instance of Item */ } - insertItem(options: InsertItemOptions): Item { + upsertItem(options: UpsertItemOptions): Item { /* code to return instance of Item */ } }