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

[Eslint Plugin] Update ts-naming-options rule to expect a more reasonable name for constructor options #11403

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
deyaaeldeen marked this conversation as resolved.
Show resolved Hide resolved
/* code to return instance of Item */
}
}
Expand All @@ -21,6 +24,9 @@ class ServiceClient {

```ts
class ServiceClient {
constructor(options: Options) {
/* code */
}
createItem(options: Options): Item {
/* code to return instance of Item */
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/

Expand Down Expand Up @@ -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
Expand All @@ -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`
});
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {}; };"
Expand All @@ -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"
}
]
}
]
});