Skip to content

Commit

Permalink
[Linter] Update linter rule to check if sdk-type exists in package.js…
Browse files Browse the repository at this point in the history
…on (#18597)

This fixes #13214. This PR is an update from a past PR, #18565.

What's new?
- I updated ```ts-package-json-sdktype.ts``` to enforce the existence of```sdk-type``` and that it is either ```client``` or ```mgmt```. If it does not, then the linter will throw an error.
- I added/updated appropriate tests and verified the entire repository using ```rush lint```
- I also put the rules in the correct files (```src/configs/index.ts, src/rules/index.ts```, and ```tests/plugin.ts```)
- I have written a docs file to put in ```docs/rules``` which are now included in the commit! 
- I do need to still update the actual ```README.md``` file, but I'm not sure what the actual version is (hopefully this isn't too much trouble)
  • Loading branch information
bzhang0 authored Nov 10, 2021
1 parent b1ae6ca commit 41018c3
Show file tree
Hide file tree
Showing 8 changed files with 569 additions and 38 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 @@ -125,6 +125,7 @@ Some rules (see table below) are fixable using the `--fix` ESLint option (added
| [**ts-package-json-name**](https://github.com/Azure/azure-sdk-for-js/blob/main/common/tools/eslint-plugin-azure-sdk/docs/rules/ts-package-json-name.md) | :triangular_flag_on_post: | :x: | `1.0.0` |
| [**ts-package-json-repo**](https://github.com/Azure/azure-sdk-for-js/blob/main/common/tools/eslint-plugin-azure-sdk/docs/rules/ts-package-json-repo.md) | :triangular_flag_on_post: | :heavy_check_mark: | `1.0.0` |
| [**ts-package-json-required-scripts**](https://github.com/Azure/azure-sdk-for-js/blob/main/common/tools/eslint-plugin-azure-sdk/docs/rules/ts-package-json-required-scripts.md) | :triangular_flag_on_post: | :x: | `1.0.0` |
| [**ts-package-json-sdktype**](https://github.com/Azure/azure-sdk-for-js/blob/main/common/tools/eslint-plugin-azure-sdk/docs/rules/ts-package-json-sdktype.md) | :triangular_flag_on_post: | :x: | `3.1.0` |
| [**ts-package-json-sideeffects**](https://github.com/Azure/azure-sdk-for-js/blob/main/common/tools/eslint-plugin-azure-sdk/docs/rules/ts-package-json-sideeffects.md) | :triangular_flag_on_post: | :heavy_check_mark: | `1.0.0` |
| [**ts-package-json-types**](https://github.com/Azure/azure-sdk-for-js/blob/main/common/tools/eslint-plugin-azure-sdk/docs/rules/ts-package-json-types.md) | :triangular_flag_on_post: | :x: | `1.1.0` |
| [**ts-pagination-list**](https://github.com/Azure/azure-sdk-for-js/blob/main/common/tools/eslint-plugin-azure-sdk/docs/rules/ts-pagination-list.md) | :triangular_flag_on_post: | :x: | `1.2.0` |
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
# ts-package-json-sdktype

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

## Examples

### Good

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

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

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

### Bad

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

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

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

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

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

```json
{}
```

## When to turn off

Only if the rule breaks.
2 changes: 1 addition & 1 deletion common/tools/eslint-plugin-azure-sdk/src/configs/index.ts
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,14 @@
// Licensed under the MIT license.

/**
* @file Rule to force package.json's 'sdk-type' value to be valid
* @file Rule to force package.json's 'sdk-type' value to be valid (and exist)
* @author Arpan Laha
* @author 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 +18,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", "utility"].includes(strValue)) {
context.report({
node: node.value,
message: `unrecognized sdk-type value: ${strValue}. Expected either "client", "mgmt", or "utility."`
});
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

0 comments on commit 41018c3

Please sign in to comment.