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

[Linter] Update linter rule to check if sdk-type exists in package.json #18597

Merged
merged 8 commits into from
Nov 10, 2021
Merged
Show file tree
Hide file tree
Changes from 5 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
@@ -0,0 +1,47 @@
# ts-package-json-sdktype

Requires the existence of the `sdk-type` field and be either 'client' or 'mgmt'.

## Examples

### Good

```json
{
"sdk-type": "client"
}
```

```json
{
"sdk-type": "mgmt"
}
```

### Bad

```json
{
"sdk-type": "invalid"
}
```

```json
{
"sdk-type": 1
}
```

```json
{
"not-sdk-type": "client"
}
```

```json
{}
```

## When to turn off

Only if the rule breaks.
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ export = {
"@azure/azure-sdk/ts-no-const-enums": "warn",
"@azure/azure-sdk/ts-no-namespaces": "error",
"@azure/azure-sdk/ts-no-window": "error",
"@azure/azure-sdk/ts-package-json-sdktype": "error",
"@azure/azure-sdk/ts-package-json-author": "error",
"@azure/azure-sdk/ts-package-json-bugs": "error",
"@azure/azure-sdk/ts-package-json-engine-is-present": "error",
Expand All @@ -61,6 +60,7 @@ export = {
"@azure/azure-sdk/ts-package-json-name": "error",
"@azure/azure-sdk/ts-package-json-repo": "error",
"@azure/azure-sdk/ts-package-json-required-scripts": "error",
"@azure/azure-sdk/ts-package-json-sdktype": "error",
"@azure/azure-sdk/ts-package-json-sideeffects": "error",
"@azure/azure-sdk/ts-package-json-types": "error",
"@azure/azure-sdk/ts-pagination-list": "error",
Expand Down
2 changes: 1 addition & 1 deletion common/tools/eslint-plugin-azure-sdk/src/rules/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ import tsNoConstEnums from "./ts-no-const-enums";
import tsNoNamespaces from "./ts-no-namespaces";
import tsNoWindow from "./ts-no-window";
import tsPackageJsonAuthor from "./ts-package-json-author";
import tsPackageJsonSdkType from "./ts-package-json-sdktype";
import tsPackageJsonBugs from "./ts-package-json-bugs";
import tsPackageJsonEngineIsPresent from "./ts-package-json-engine-is-present";
import tsPackageJsonFilesRequired from "./ts-package-json-files-required";
Expand All @@ -45,6 +44,7 @@ import tsPackageJsonModule from "./ts-package-json-module";
import tsPackageJsonName from "./ts-package-json-name";
import tsPackageJsonRepo from "./ts-package-json-repo";
import tsPackageJsonRequiredScripts from "./ts-package-json-required-scripts";
import tsPackageJsonSdkType from "./ts-package-json-sdktype";
import tsPackageJsonSideEffects from "./ts-package-json-sideeffects";
import tsPackageJsonTypes from "./ts-package-json-types";
import tsPaginationList from "./ts-pagination-list";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@
// Licensed under the MIT license.

/**
* @file Rule to force package.json's 'sdk-type' value to be valid
* @author Arpan Laha
* @file Rule to force package.json's 'sdk-type' value to be valid (and exist)
* @author Arpan Laha, Ben Zhang
*/

import { Rule } from "eslint";
import { Property } from "estree";
import { getRuleMetaData, stripPath } from "../utils";
import { getRuleMetaData, getVerifiers, stripPath } from "../utils";

//------------------------------------------------------------------------------
// Rule Definition
Expand All @@ -17,41 +17,46 @@ import { getRuleMetaData, stripPath } from "../utils";
export = {
meta: getRuleMetaData(
"ts-package-json-sdktype",
"force package.json's sdk-type value to contain be 'client' or 'mgmt'"
"force package.json's sdk-type to exist and for its value to be 'client' or 'mgmt'",
"code"
),
create: (context: Rule.RuleContext): Rule.RuleListener =>
stripPath(context.getFilename()) === "package.json"
create: (context: Rule.RuleContext): Rule.RuleListener => {
const verifiers = getVerifiers(context, {
outer: "sdk-type"
});
return stripPath(context.getFilename()) === "package.json"
? ({
// callback functions

// check the node corresponding to sdk-type to see if its value contains "client" or "mgmt"
"ExpressionStatement > ObjectExpression > Property[key.value='sdk-type']": (
node: Property
) => {
if (!node) {
// Track1 packages don't have this property. Stop checking
return;
}

const { value } = node;
if (value.type !== "Literal" || typeof value.value !== "string") {
context.report({
node: node.value,
message: "sdk-type is not set to a string"
});
return;
}

const strValue = stripPath(value.value);

if (!["client", "mgmt"].includes(strValue)) {
context.report({
node: node.value,
message: "sdk-type is not set to `client` or `mgmt`"
});
return;
}
// callback functions

// check to see if package.json includes 'sdk-type'
"ExpressionStatement > ObjectExpression": verifiers.existsInFile,

// check the node corresponding to sdk-type to see if its value contains "client" or "mgmt"
"ExpressionStatement > ObjectExpression > Property[key.value='sdk-type']": (
node: Property
): void => {
const { value } = node;

// check for valid type
if (value.type !== "Literal" || typeof value.value !== "string") {
context.report({
node: node.value,
message: "sdk-type is not set to a string"
});
return;
}
} as Rule.RuleListener)
: {}

const strValue = stripPath(value.value);

if (!["client", "mgmt"].includes(strValue)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add "utility" into the allowed list. It was just added earlier in #18402.

context.report({
node: node.value,
message: "sdk-type is not set to `client` or `mgmt`"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps it could be useful to print the unrecognized value too?

Suggested change
message: "sdk-type is not set to `client` or `mgmt`"
message: `unrecognized sdk-type value: ${strValue}. Expected `client` or `mgmt` instead.`

Please note the error messages in the tests will need to be updated too.

});
return;
}
}
} as Rule.RuleListener)
: {};
}
};
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 @@ -53,6 +53,7 @@ const ruleList = [
"ts-package-json-name",
"ts-package-json-repo",
"ts-package-json-required-scripts",
"ts-package-json-sdktype",
"ts-package-json-sideeffects",
"ts-package-json-types",
"ts-pagination-list",
Expand Down
Loading