Skip to content

Commit

Permalink
[ESLint Plugin] Enforce inclusion of dist-esm and exclusion of src in…
Browse files Browse the repository at this point in the history 
… files field in package.json (#11411)

* enforce exclusion

* more refactoring

* adding comments

* enforce inclusion of dist-esm/src back

* update docs

* remove obselete comment

* fix issue

* rename

* more refactoring

* update tests

* remove src from files in storage-datalake

* address feedback

* address more feedback

* make requiredPatterns constant for extra safety
  • Loading branch information
@deyaaeldeen
deyaaeldeen authored Oct 14, 2020
1 parent 17fede0 commit 39cf3a2
Show file tree
Hide file tree
Showing 4 changed files with 109 additions and 65 deletions.
26 changes: 5 additions & 21 deletions common/tools/eslint-plugin-azure-sdk/docs/rules/ts-package-json-files-required.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,49 +12,33 @@ This rule is fixable using the `--fix` option.

```json
{
"files": [
"dist",
"dist-esm/src"
"src"
]
"files": ["dist", "dist-esm/src"]
}
```

```json
{
"files": [
"./dist",
"./dist-esm/src"
"./src"
]
"files": ["./dist", "./dist-esm/src"]
}
```

```json
{
"files": [
"dist/",
"dist-esm/src/"
"src/"
]
"files": ["dist/", "dist-esm/"]
}
```

```json
{
"files": [
"dist/lib",
"dist-esm/src/lib"
"src/lib"
]
"files": ["dist/lib", "dist-esm/src/lib"]
}
```

### Bad

```json
{
"files": ["dist", "dist-esm/src"]
"files": ["dist", "src"]
}
```

Expand Down
125 changes: 96 additions & 29 deletions common/tools/eslint-plugin-azure-sdk/src/rules/ts-package-json-files-required.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Licensed under the MIT license.

/**
* @file Rule to force package.json's files value to contain paths to the package contents.
* @file Rule to force package.json's files value to contain paths to the package contents and excludes source code files.
* @author Arpan Laha
*/

Expand All @@ -14,6 +14,21 @@ import { arrayToString, getRuleMetaData, getVerifiers, stripPath } from "../util
// Rule Definition
//------------------------------------------------------------------------------

let requiredPatternSuggestionMap: Map<string, string | undefined> = new Map();
function addRequiredPattern(pattern: string, suggestion?: string): void {
requiredPatternSuggestionMap.set(pattern, suggestion);
}

/**
* The rule is configurable by those two vars, where badPatterns is the list of
* patterns that should not be in package.json's files list and requiredPatterns
* is the list of patterns that should be.
*/
const badPatterns = ["src"];
addRequiredPattern("dist");
addRequiredPattern("dist-esm", "src");
const requiredPatterns = Array.from(requiredPatternSuggestionMap.keys());

export = {
meta: getRuleMetaData(
"ts-package-json-files-required",
Expand All @@ -31,7 +46,6 @@ export = {
// check to see if files exists at the outermost level
"ExpressionStatement > ObjectExpression": verifiers.existsInFile,

// check that files contains dist, dist-esm, and src
"ExpressionStatement > ObjectExpression > Property[key.value='files']": (
node: Property
): void => {
Expand All @@ -44,40 +58,69 @@ export = {
return;
}

// sorting the patterns descendingly so in cases of overlap between
// pattern (e.g. dist and dist-esm), the regex tries to match the
// longer first.
const regExprStr = `^(?:.\/)?(${badPatterns
.concat(requiredPatterns)
.sort()
.reverse()
.join("|")})(?:\/)?(?:.+)?`;

const nodeValue = node.value;
const elements = nodeValue.elements as Literal[];
const elementValues = elements.map((element: Literal): unknown => element.value);
let filesList = (nodeValue.elements as Literal[]).map(
(element): unknown => element.value
);

// looks for 'dist' with optional leading './' and optional trailing '/'
if (
elements.every(
(element: Literal): boolean =>
!/^(.\/)?((dist\/)|(dist$))/.test(element.value as string)
)
) {
context.report({
node: nodeValue,
message: "dist is not included in files",
fix: (fixer: Rule.RuleFixer): Rule.Fix => {
elementValues.push("dist");
return fixer.replaceText(nodeValue, arrayToString(elementValues));
let currBadPatterns: string[] = [];
let currRequiredPatterns = [...requiredPatterns];
const fullMatchIndex = 0;
const patternRootMatchIndex = 1;
// Looking for both required and bad patterns
for (const filePattern of filesList) {
const patternMatchResult = (filePattern as string).match(regExprStr);
if (patternMatchResult !== null) {
const patternRoot = patternMatchResult[patternRootMatchIndex];
if (badPatterns.indexOf(patternRoot) >= 0) {
currBadPatterns.push(patternMatchResult[fullMatchIndex]);
} else if (requiredPatterns.indexOf(patternRoot) >= 0) {
currRequiredPatterns.splice(currRequiredPatterns.indexOf(patternRoot), 1);
}
});
}
}

// looks for 'dist-esm/src' with optional leading './' and optional trailing '/'
if (
elements.every(
(element: Literal): boolean =>
!/^(.\/)?dist-esm\/((src\/)|(src$))/.test(element.value as string)
)
) {
let errorMessage = "";
// Make sure there are no bad patterns, but if there are, create
// a meaningful error message for them and remove them from the
// files list
if (currBadPatterns.length > 0) {
errorMessage = `${currBadPatterns.join()} ${
currBadPatterns.length === 1 ? "is" : "are"
} included in files`;
filesList = filesList.filter(
(filePattern) => currBadPatterns.indexOf(filePattern as string) < 0
);
}
// If there are required patterns missing from the files' list,
// create a meaningful error message and add them to the list (with
// the default suggestion)
if (currRequiredPatterns.length > 0) {
updateFixRequiredPatterns(currRequiredPatterns);
filesList = filesList.concat(currRequiredPatterns);
if (errorMessage.length > 0) {
errorMessage = errorMessage + " and ";
}
errorMessage =
errorMessage +
`${currRequiredPatterns.join()} ${
currRequiredPatterns.length === 1 ? "is" : "are"
} not included in files`;
}
if (errorMessage.length > 0) {
context.report({
node: nodeValue,
message: "dist-esm/src is not included in files",
message: errorMessage,
fix: (fixer: Rule.RuleFixer): Rule.Fix => {
elementValues.push("dist-esm/src");
return fixer.replaceText(nodeValue, arrayToString(elementValues));
return fixer.replaceText(nodeValue, arrayToString(filesList));
}
});
}
Expand All @@ -86,3 +129,27 @@ export = {
: {};
}
};

/**
* Creates the more specific and recommended pattern that will be added to the
* files list by the fixer.
* @param pat - A pattern that is missing from the files list
*/
function buildFixRequiredPattern(pat: string): string {
return requiredPatternSuggestionMap.get(pat) !== undefined
? pat + "/" + requiredPatternSuggestionMap.get(pat)
: pat;
}

/**
* Updates the patterns in the input list to be the more specific and
* recommended patterns.
* @param currRequiredPatterns - A list of patterns that are required
* but missing from the files list
*/
function updateFixRequiredPatterns(currRequiredPatterns: string[]): void {
for (let i = 0; i < currRequiredPatterns.length; ++i) {
const pat = currRequiredPatterns[i];
currRequiredPatterns[i] = buildFixRequiredPattern(pat);
}
}
22 changes: 8 additions & 14 deletions common/tools/eslint-plugin-azure-sdk/tests/rules/ts-package-json-files-required.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -309,20 +309,20 @@ ruleTester.run("ts-package-json-files-required", rule, {
filename: "package.json",
errors: [
{
message: "dist-esm/src is not included in files"
message: "src is included in files and dist-esm/src is not included in files"
}
],
output: '{"files": ["dist", "src", "dist-esm/src"]}'
output: '{"files": ["dist", "dist-esm/src"]}'
},
{
code: '{"files": ["src", "dist-esm/src"]}',
filename: "package.json",
errors: [
{
message: "dist is not included in files"
message: "src is included in files and dist is not included in files"
}
],
output: '{"files": ["src", "dist-esm/src", "dist"]}'
output: '{"files": ["dist-esm/src", "dist"]}'
},
{
code: '{"files": ["dist"]}',
Expand All @@ -349,26 +349,20 @@ ruleTester.run("ts-package-json-files-required", rule, {
filename: "package.json",
errors: [
{
message: "dist is not included in files"
},
{
message: "dist-esm/src is not included in files"
message: "dist,dist-esm/src are not included in files"
}
],
output: '{"files": ["dist"]}'
output: '{"files": ["dist", "dist-esm/src"]}'
},
{
// example file with src not in files
code: examplePackageBad,
filename: "package.json",
errors: [
{
message: "dist is not included in files"
},
{
message: "dist-esm/src is not included in files"
message: "dist,dist-esm/src are not included in files"
}
]
}
]
});
});
1 change: 0 additions & 1 deletion sdk/storage/storage-datalake/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@
"esm/**/*.js.map",
"esm/**/*.d.ts",
"esm/**/*.d.ts.map",
"src/**/*.ts",
"README.md",
"rollup.config.js",
"tsconfig.json"
Expand Down

0 comments on commit 39cf3a2

Please sign in to comment.