From cc4e88eea0b1c3f60c9f0ac64017de847079e5b1 Mon Sep 17 00:00:00 2001 From: Pete Gonzalez <4673363+octogonz@users.noreply.github.com> Date: Thu, 1 Oct 2020 22:04:28 -0700 Subject: [PATCH 01/23] Set up a packlets-tutorial project for testing --- rush.json | 6 +++ tutorials/packlets-tutorial/.eslintrc.js | 7 +++ tutorials/packlets-tutorial/README.md | 2 + tutorials/packlets-tutorial/config/heft.json | 51 ++++++++++++++++++++ tutorials/packlets-tutorial/package.json | 18 +++++++ 5 files changed, 84 insertions(+) create mode 100644 tutorials/packlets-tutorial/.eslintrc.js create mode 100644 tutorials/packlets-tutorial/README.md create mode 100644 tutorials/packlets-tutorial/config/heft.json create mode 100644 tutorials/packlets-tutorial/package.json diff --git a/rush.json b/rush.json index a848b3ecb11..10c29e405fd 100644 --- a/rush.json +++ b/rush.json @@ -1059,6 +1059,12 @@ "reviewCategory": "tests", "shouldPublish": false }, + { + "packageName": "packlets-tutorial", + "projectFolder": "tutorials/packlets-tutorial", + "reviewCategory": "tests", + "shouldPublish": false + }, // "webpack" folder (alphabetical order) { diff --git a/tutorials/packlets-tutorial/.eslintrc.js b/tutorials/packlets-tutorial/.eslintrc.js new file mode 100644 index 00000000000..60160b354c4 --- /dev/null +++ b/tutorials/packlets-tutorial/.eslintrc.js @@ -0,0 +1,7 @@ +// This is a workaround for https://github.com/eslint/eslint/issues/3458 +require('@rushstack/eslint-config/patch/modern-module-resolution'); + +module.exports = { + extends: ['@rushstack/eslint-config/profile/node'], + parserOptions: { tsconfigRootDir: __dirname } +}; diff --git a/tutorials/packlets-tutorial/README.md b/tutorials/packlets-tutorial/README.md new file mode 100644 index 00000000000..fdf411a4c09 --- /dev/null +++ b/tutorials/packlets-tutorial/README.md @@ -0,0 +1,2 @@ +# packlets-tutorial + diff --git a/tutorials/packlets-tutorial/config/heft.json b/tutorials/packlets-tutorial/config/heft.json new file mode 100644 index 00000000000..99e058540fb --- /dev/null +++ b/tutorials/packlets-tutorial/config/heft.json @@ -0,0 +1,51 @@ +/** + * Defines configuration used by core Heft. + */ +{ + "$schema": "https://developer.microsoft.com/json-schemas/heft/heft.schema.json", + + "eventActions": [ + { + /** + * The kind of built-in operation that should be performed. + * The "deleteGlobs" action deletes files or folders that match the + * specified glob patterns. + */ + "actionKind": "deleteGlobs", + + /** + * The stage of the Heft run during which this action should occur. Note that actions specified in heft.json + * occur at the end of the stage of the Heft run. + */ + "heftEvent": "clean", + + /** + * A user-defined tag whose purpose is to allow configs to replace/delete handlers that were added by other + * configs. + */ + "actionId": "defaultClean", + + /** + * Glob patterns to be deleted. The paths are resolved relative to the project folder. + */ + "globsToDelete": ["dist", "lib", "temp"] + } + ], + + /** + * The list of Heft plugins to be loaded. + */ + "heftPlugins": [ + // { + // /** + // * The path to the plugin package. + // */ + // "plugin": "path/to/my-plugin", + // + // /** + // * An optional object that provides additional settings that may be defined by the plugin. + // */ + // // "options": { } + // } + ] +} diff --git a/tutorials/packlets-tutorial/package.json b/tutorials/packlets-tutorial/package.json new file mode 100644 index 00000000000..2dfd6155fe8 --- /dev/null +++ b/tutorials/packlets-tutorial/package.json @@ -0,0 +1,18 @@ +{ + "name": "packlets-tutorial", + "description": "This project illustrates how to use @rushstack/eslint-plugin-packlets", + "version": "1.0.0", + "private": true, + "license": "MIT", + "scripts": { + "build": "heft build --clean", + "start": "node lib/start.js" + }, + "devDependencies": { + "@rushstack/eslint-config": "workspace:*", + "@rushstack/heft": "workspace:*", + "@types/node": "10.17.13", + "eslint": "~7.2.0", + "typescript": "~3.9.7" + } +} From e1737b8ee0f6f2a2c07e5c5aeda444988c31f5d4 Mon Sep 17 00:00:00 2001 From: Pete Gonzalez <4673363+octogonz@users.noreply.github.com> Date: Thu, 1 Oct 2020 22:04:35 -0700 Subject: [PATCH 02/23] rush update --- common/config/rush/pnpm-lock.yaml | 13 +++++++++++++ common/config/rush/repo-state.json | 2 +- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/common/config/rush/pnpm-lock.yaml b/common/config/rush/pnpm-lock.yaml index 46b2e47f411..5f335504731 100644 --- a/common/config/rush/pnpm-lock.yaml +++ b/common/config/rush/pnpm-lock.yaml @@ -2223,6 +2223,19 @@ importers: style-loader: ~1.2.1 typescript: ~3.9.7 webpack: ~4.31.0 + ../../tutorials/packlets-tutorial: + devDependencies: + '@rushstack/eslint-config': 'link:../../stack/eslint-config' + '@rushstack/heft': 'link:../../apps/heft' + '@types/node': 10.17.13 + eslint: 7.2.0 + typescript: 3.9.7 + specifiers: + '@rushstack/eslint-config': 'workspace:*' + '@rushstack/heft': 'workspace:*' + '@types/node': 10.17.13 + eslint: ~7.2.0 + typescript: ~3.9.7 ../../webpack/loader-load-themed-styles: dependencies: '@microsoft/load-themed-styles': 'link:../../libraries/load-themed-styles' diff --git a/common/config/rush/repo-state.json b/common/config/rush/repo-state.json index 83cd304ad61..f7bb9f84c35 100644 --- a/common/config/rush/repo-state.json +++ b/common/config/rush/repo-state.json @@ -1,5 +1,5 @@ // DO NOT MODIFY THIS FILE. It is generated and used by Rush. { - "pnpmShrinkwrapHash": "c4342cbb84a614c6f5b789815d32aca66d0e5b1b", + "pnpmShrinkwrapHash": "885167eab43b7ea96909e595973332a2a63e4839", "preferredVersionsHash": "0f2f367d951f4cd546b698d668533c1ff056e334" } From d1fa83371a3e44be5e500a0f807d55d5fed5abcb Mon Sep 17 00:00:00 2001 From: Pete Gonzalez <4673363+octogonz@users.noreply.github.com> Date: Thu, 1 Oct 2020 22:04:58 -0700 Subject: [PATCH 03/23] Make a small sample app with some example packlets --- tutorials/packlets-tutorial/src/app/App.ts | 19 ++++++++++++++ .../src/packlets/data-model/DataModel.ts | 12 +++++++++ .../src/packlets/data-model/ExampleModel.ts | 21 ++++++++++++++++ .../src/packlets/data-model/index.ts | 5 ++++ .../src/packlets/logging/Logger.ts | 20 +++++++++++++++ .../src/packlets/logging/MessageType.ts | 8 ++++++ .../src/packlets/logging/index.ts | 5 ++++ .../src/packlets/reports/MainReport.ts | 24 ++++++++++++++++++ .../src/packlets/reports/index.ts | 4 +++ tutorials/packlets-tutorial/src/start.ts | 7 ++++++ tutorials/packlets-tutorial/tsconfig.json | 25 +++++++++++++++++++ 11 files changed, 150 insertions(+) create mode 100644 tutorials/packlets-tutorial/src/app/App.ts create mode 100644 tutorials/packlets-tutorial/src/packlets/data-model/DataModel.ts create mode 100644 tutorials/packlets-tutorial/src/packlets/data-model/ExampleModel.ts create mode 100644 tutorials/packlets-tutorial/src/packlets/data-model/index.ts create mode 100644 tutorials/packlets-tutorial/src/packlets/logging/Logger.ts create mode 100644 tutorials/packlets-tutorial/src/packlets/logging/MessageType.ts create mode 100644 tutorials/packlets-tutorial/src/packlets/logging/index.ts create mode 100644 tutorials/packlets-tutorial/src/packlets/reports/MainReport.ts create mode 100644 tutorials/packlets-tutorial/src/packlets/reports/index.ts create mode 100644 tutorials/packlets-tutorial/src/start.ts create mode 100644 tutorials/packlets-tutorial/tsconfig.json diff --git a/tutorials/packlets-tutorial/src/app/App.ts b/tutorials/packlets-tutorial/src/app/App.ts new file mode 100644 index 00000000000..5e30bf98e26 --- /dev/null +++ b/tutorials/packlets-tutorial/src/app/App.ts @@ -0,0 +1,19 @@ +import { DataModel, ExampleModel } from '../packlets/data-model'; +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. +// See LICENSE in the project root for license information. + +import { Logger, MessageType } from '../packlets/logging'; +import { MainReport } from '../packlets/reports'; + +export class App { + public run(): void { + const logger: Logger = new Logger(); + logger.log(MessageType.Info, 'Starting app...'); + + const dataModel: DataModel = new ExampleModel(); + const report: MainReport = new MainReport(logger); + report.showReport(dataModel); + + logger.log(MessageType.Info, 'Operation completed successfully'); + } +} diff --git a/tutorials/packlets-tutorial/src/packlets/data-model/DataModel.ts b/tutorials/packlets-tutorial/src/packlets/data-model/DataModel.ts new file mode 100644 index 00000000000..b651f3fc6df --- /dev/null +++ b/tutorials/packlets-tutorial/src/packlets/data-model/DataModel.ts @@ -0,0 +1,12 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. +// See LICENSE in the project root for license information. + +export interface IDataRecord { + firstName: string; + lastName: string; + age: number; +} + +export abstract class DataModel { + public abstract queryRecords(): IDataRecord[]; +} diff --git a/tutorials/packlets-tutorial/src/packlets/data-model/ExampleModel.ts b/tutorials/packlets-tutorial/src/packlets/data-model/ExampleModel.ts new file mode 100644 index 00000000000..61ad61c935b --- /dev/null +++ b/tutorials/packlets-tutorial/src/packlets/data-model/ExampleModel.ts @@ -0,0 +1,21 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. +// See LICENSE in the project root for license information. + +import { DataModel, IDataRecord } from './DataModel'; + +export class ExampleModel extends DataModel { + public queryRecords(): IDataRecord[] { + return [ + { + firstName: 'Alice', + lastName: 'Exampleton', + age: 27 + }, + { + firstName: 'Bob', + lastName: 'Examplemeyer', + age: 31 + } + ]; + } +} diff --git a/tutorials/packlets-tutorial/src/packlets/data-model/index.ts b/tutorials/packlets-tutorial/src/packlets/data-model/index.ts new file mode 100644 index 00000000000..44c53440756 --- /dev/null +++ b/tutorials/packlets-tutorial/src/packlets/data-model/index.ts @@ -0,0 +1,5 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. +// See LICENSE in the project root for license information. + +export * from './DataModel'; +export * from './ExampleModel'; diff --git a/tutorials/packlets-tutorial/src/packlets/logging/Logger.ts b/tutorials/packlets-tutorial/src/packlets/logging/Logger.ts new file mode 100644 index 00000000000..f7523792898 --- /dev/null +++ b/tutorials/packlets-tutorial/src/packlets/logging/Logger.ts @@ -0,0 +1,20 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. +// See LICENSE in the project root for license information. + +import { MessageType } from './MessageType'; + +export class Logger { + public log(messageType: MessageType, message: string): void { + switch (messageType) { + case MessageType.Info: + console.log('[info]: ' + message); + break; + case MessageType.Warning: + console.log('[warning]: ' + message); + break; + case MessageType.Error: + console.log('[error]: ' + message); + break; + } + } +} diff --git a/tutorials/packlets-tutorial/src/packlets/logging/MessageType.ts b/tutorials/packlets-tutorial/src/packlets/logging/MessageType.ts new file mode 100644 index 00000000000..ec7ac320a41 --- /dev/null +++ b/tutorials/packlets-tutorial/src/packlets/logging/MessageType.ts @@ -0,0 +1,8 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. +// See LICENSE in the project root for license information. + +export enum MessageType { + Info, + Warning, + Error +} diff --git a/tutorials/packlets-tutorial/src/packlets/logging/index.ts b/tutorials/packlets-tutorial/src/packlets/logging/index.ts new file mode 100644 index 00000000000..a9cb7beac4d --- /dev/null +++ b/tutorials/packlets-tutorial/src/packlets/logging/index.ts @@ -0,0 +1,5 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. +// See LICENSE in the project root for license information. + +export * from './Logger'; +export * from './MessageType'; diff --git a/tutorials/packlets-tutorial/src/packlets/reports/MainReport.ts b/tutorials/packlets-tutorial/src/packlets/reports/MainReport.ts new file mode 100644 index 00000000000..2fb96438317 --- /dev/null +++ b/tutorials/packlets-tutorial/src/packlets/reports/MainReport.ts @@ -0,0 +1,24 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. +// See LICENSE in the project root for license information. + +import { DataModel } from '../data-model'; +import { Logger, MessageType } from '../logging'; + +export class MainReport { + private readonly _logger: Logger; + + public constructor(logger: Logger) { + this._logger = logger; + this._logger.log(MessageType.Info, 'Constructing MainReport'); + } + + public showReport(dataModel: DataModel): void { + console.log('\n---------------------------------------'); + console.log('REPORT'); + console.log('---------------------------------------'); + for (const record of dataModel.queryRecords()) { + console.log(`${record.firstName} ${record.lastName}: Age=${record.age}`); + } + console.log('---------------------------------------\n'); + } +} diff --git a/tutorials/packlets-tutorial/src/packlets/reports/index.ts b/tutorials/packlets-tutorial/src/packlets/reports/index.ts new file mode 100644 index 00000000000..447a3eb6c4c --- /dev/null +++ b/tutorials/packlets-tutorial/src/packlets/reports/index.ts @@ -0,0 +1,4 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. +// See LICENSE in the project root for license information. + +export * from './MainReport'; diff --git a/tutorials/packlets-tutorial/src/start.ts b/tutorials/packlets-tutorial/src/start.ts new file mode 100644 index 00000000000..b36a51c36a0 --- /dev/null +++ b/tutorials/packlets-tutorial/src/start.ts @@ -0,0 +1,7 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. +// See LICENSE in the project root for license information. + +import { App } from './app/App'; + +const app: App = new App(); +app.run(); diff --git a/tutorials/packlets-tutorial/tsconfig.json b/tutorials/packlets-tutorial/tsconfig.json new file mode 100644 index 00000000000..8ab12336838 --- /dev/null +++ b/tutorials/packlets-tutorial/tsconfig.json @@ -0,0 +1,25 @@ +{ + "$schema": "http://json.schemastore.org/tsconfig", + + "compilerOptions": { + "outDir": "lib", + "rootDirs": ["src/"], + + "forceConsistentCasingInFileNames": true, + "jsx": "react", + "declaration": true, + "sourceMap": true, + "declarationMap": true, + "inlineSources": true, + "experimentalDecorators": true, + "strictNullChecks": true, + "noUnusedLocals": true, + "types": ["node"], + + "module": "commonjs", + "target": "es2017", + "lib": ["es2017"] + }, + "include": ["src/**/*.ts", "src/**/*.tsx"], + "exclude": ["node_modules", "lib"] +} From 17a9129d38bf3b53158808c78166ba93c3358750 Mon Sep 17 00:00:00 2001 From: Pete Gonzalez <4673363+octogonz@users.noreply.github.com> Date: Thu, 1 Oct 2020 22:12:18 -0700 Subject: [PATCH 04/23] Set up new project @rushstack/eslint-plugin-packlets --- rush.json | 7 ++++ .../.eslintrc.js.disabled | 5 +++ stack/eslint-plugin-packlets/.npmignore | 24 +++++++++++ stack/eslint-plugin-packlets/LICENSE | 24 +++++++++++ stack/eslint-plugin-packlets/README.md | 2 + .../config/jest.config.json | 3 ++ stack/eslint-plugin-packlets/config/rig.json | 7 ++++ stack/eslint-plugin-packlets/package.json | 40 +++++++++++++++++++ stack/eslint-plugin-packlets/src/index.ts | 14 +++++++ stack/eslint-plugin-packlets/tsconfig.json | 7 ++++ 10 files changed, 133 insertions(+) create mode 100644 stack/eslint-plugin-packlets/.eslintrc.js.disabled create mode 100644 stack/eslint-plugin-packlets/.npmignore create mode 100644 stack/eslint-plugin-packlets/LICENSE create mode 100644 stack/eslint-plugin-packlets/README.md create mode 100644 stack/eslint-plugin-packlets/config/jest.config.json create mode 100644 stack/eslint-plugin-packlets/config/rig.json create mode 100644 stack/eslint-plugin-packlets/package.json create mode 100644 stack/eslint-plugin-packlets/src/index.ts create mode 100644 stack/eslint-plugin-packlets/tsconfig.json diff --git a/rush.json b/rush.json index 10c29e405fd..3ecdc8df700 100644 --- a/rush.json +++ b/rush.json @@ -923,6 +923,13 @@ "shouldPublish": true, "cyclicDependencyProjects": ["@rushstack/heft-node-rig", "@rushstack/heft"] }, + { + "packageName": "@rushstack/eslint-plugin-packlets", + "projectFolder": "stack/eslint-plugin-packlets", + "reviewCategory": "libraries", + "shouldPublish": true, + "cyclicDependencyProjects": ["@rushstack/heft-node-rig", "@rushstack/heft"] + }, { "packageName": "@rushstack/eslint-plugin-security", "projectFolder": "stack/eslint-plugin-security", diff --git a/stack/eslint-plugin-packlets/.eslintrc.js.disabled b/stack/eslint-plugin-packlets/.eslintrc.js.disabled new file mode 100644 index 00000000000..94f24a7fc52 --- /dev/null +++ b/stack/eslint-plugin-packlets/.eslintrc.js.disabled @@ -0,0 +1,5 @@ +NOTE: We do not invoke ESLint on this project's source files, because ESLint's module resolution (as of 6.x) is naive +and fairly brittle. It gets confused between the dependencies of this project, versus the dependencies of +@rushstack/eslint-config (which imports the previously published version of this project). Normally we solve +this problem by using Rush's "cyclicDependencyProjects" feature, but that fails because ESLint does not correctly +implement NodeJS module resolution. diff --git a/stack/eslint-plugin-packlets/.npmignore b/stack/eslint-plugin-packlets/.npmignore new file mode 100644 index 00000000000..bf2eebaed77 --- /dev/null +++ b/stack/eslint-plugin-packlets/.npmignore @@ -0,0 +1,24 @@ +# Ignore everything by default +** + +# Use negative patterns to bring back the specific things we want to publish +!/bin/** +!/lib/** +!/dist/** +!ThirdPartyNotice.txt + +# Ignore certain files in the above folder +/dist/*.stats.* +/lib/**/test/* + +# NOTE: These don't need to be specified, because NPM includes them automatically. +# +# package.json +# README (and its variants) +# CHANGELOG (and its variants) +# LICENSE / LICENCE + +## Project specific definitions +# ----------------------------- + +# (Add your exceptions here) diff --git a/stack/eslint-plugin-packlets/LICENSE b/stack/eslint-plugin-packlets/LICENSE new file mode 100644 index 00000000000..579e79aa4f4 --- /dev/null +++ b/stack/eslint-plugin-packlets/LICENSE @@ -0,0 +1,24 @@ +@rushstack/eslint-plugin-packlets + +Copyright (c) Microsoft Corporation. All rights reserved. + +MIT License + +Permission is hereby granted, free of charge, to any person obtaining +a copy of this software and associated documentation files (the +"Software"), to deal in the Software without restriction, including +without limitation the rights to use, copy, modify, merge, publish, +distribute, sublicense, and/or sell copies of the Software, and to +permit persons to whom the Software is furnished to do so, subject to +the following conditions: + +The above copyright notice and this permission notice shall be +included in all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, +EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND +NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE +LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION +OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION +WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. diff --git a/stack/eslint-plugin-packlets/README.md b/stack/eslint-plugin-packlets/README.md new file mode 100644 index 00000000000..62e6e7bc644 --- /dev/null +++ b/stack/eslint-plugin-packlets/README.md @@ -0,0 +1,2 @@ +# @rushstack/eslint-plugin-packlets + diff --git a/stack/eslint-plugin-packlets/config/jest.config.json b/stack/eslint-plugin-packlets/config/jest.config.json new file mode 100644 index 00000000000..b88d4c3de66 --- /dev/null +++ b/stack/eslint-plugin-packlets/config/jest.config.json @@ -0,0 +1,3 @@ +{ + "preset": "./node_modules/@rushstack/heft/includes/jest-shared.config.json" +} diff --git a/stack/eslint-plugin-packlets/config/rig.json b/stack/eslint-plugin-packlets/config/rig.json new file mode 100644 index 00000000000..6ac88a96368 --- /dev/null +++ b/stack/eslint-plugin-packlets/config/rig.json @@ -0,0 +1,7 @@ +{ + // The "rig.json" file directs tools to look for their config files in an external package. + // Documentation for this system: https://www.npmjs.com/package/@rushstack/rig-package + "$schema": "https://developer.microsoft.com/json-schemas/rig-package/rig.schema.json", + + "rigPackageName": "@rushstack/heft-node-rig" +} diff --git a/stack/eslint-plugin-packlets/package.json b/stack/eslint-plugin-packlets/package.json new file mode 100644 index 00000000000..57145c2bfe2 --- /dev/null +++ b/stack/eslint-plugin-packlets/package.json @@ -0,0 +1,40 @@ +{ + "name": "@rushstack/eslint-plugin-packlets", + "version": "0.0.0", + "description": "A lightweight system for creating project subfolders with import relationships similar to NPM packages", + "license": "MIT", + "repository": { + "url": "https://github.com/microsoft/rushstack/tree/master/stack/eslint-plugin-packlets" + }, + "homepage": "https://rushstack.io", + "keywords": [ + "eslint", + "eslint-config", + "packlets", + "rules" + ], + "main": "lib/index.js", + "typings": "lib/index.d.ts", + "scripts": { + "build": "heft test --clean" + }, + "dependencies": { + "@rushstack/tree-pattern": "workspace:*" + }, + "peerDependencies": { + "eslint": "^6.0.0 || ^7.0.0" + }, + "devDependencies": { + "@rushstack/heft": "0.14.0", + "@rushstack/heft-node-rig": "0.1.0", + "@types/eslint": "7.2.0", + "@types/estree": "0.0.44", + "@types/heft-jest": "1.0.1", + "@types/node": "10.17.13", + "@typescript-eslint/experimental-utils": "3.4.0", + "@typescript-eslint/parser": "3.4.0", + "@typescript-eslint/typescript-estree": "3.4.0", + "eslint": "~7.2.0", + "typescript": "~3.9.7" + } +} diff --git a/stack/eslint-plugin-packlets/src/index.ts b/stack/eslint-plugin-packlets/src/index.ts new file mode 100644 index 00000000000..4166b4f967f --- /dev/null +++ b/stack/eslint-plugin-packlets/src/index.ts @@ -0,0 +1,14 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. +// See LICENSE in the project root for license information. + +import { TSESLint } from '@typescript-eslint/experimental-utils'; + +interface IPlugin { + rules: { [ruleName: string]: TSESLint.RuleModule }; +} + +const plugin: IPlugin = { + rules: {} +}; + +export = plugin; diff --git a/stack/eslint-plugin-packlets/tsconfig.json b/stack/eslint-plugin-packlets/tsconfig.json new file mode 100644 index 00000000000..1de40fe9a71 --- /dev/null +++ b/stack/eslint-plugin-packlets/tsconfig.json @@ -0,0 +1,7 @@ +{ + "extends": "./node_modules/@rushstack/heft-node-rig/profiles/default/tsconfig-base.json", + + "compilerOptions": { + "types": ["heft-jest"] + } +} From c791b29eceffa955ed983edea10b05cec865e590 Mon Sep 17 00:00:00 2001 From: Pete Gonzalez <4673363+octogonz@users.noreply.github.com> Date: Thu, 1 Oct 2020 22:12:26 -0700 Subject: [PATCH 05/23] rush update --- common/config/rush/pnpm-lock.yaml | 28 ++++++++++++++++++++++++++++ common/config/rush/repo-state.json | 2 +- 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/common/config/rush/pnpm-lock.yaml b/common/config/rush/pnpm-lock.yaml index 5f335504731..82c477f7c0f 100644 --- a/common/config/rush/pnpm-lock.yaml +++ b/common/config/rush/pnpm-lock.yaml @@ -1699,6 +1699,34 @@ importers: '@typescript-eslint/typescript-estree': 3.4.0 eslint: ~7.2.0 typescript: ~3.9.7 + ../../stack/eslint-plugin-packlets: + dependencies: + '@rushstack/tree-pattern': 'link:../../libraries/tree-pattern' + devDependencies: + '@rushstack/heft': 0.14.0 + '@rushstack/heft-node-rig': 0.1.0_@rushstack+heft@0.14.0 + '@types/eslint': 7.2.0 + '@types/estree': 0.0.44 + '@types/heft-jest': 1.0.1 + '@types/node': 10.17.13 + '@typescript-eslint/experimental-utils': 3.4.0_eslint@7.2.0+typescript@3.9.7 + '@typescript-eslint/parser': 3.4.0_eslint@7.2.0+typescript@3.9.7 + '@typescript-eslint/typescript-estree': 3.4.0_typescript@3.9.7 + eslint: 7.2.0 + typescript: 3.9.7 + specifiers: + '@rushstack/heft': 0.14.0 + '@rushstack/heft-node-rig': 0.1.0 + '@rushstack/tree-pattern': 'workspace:*' + '@types/eslint': 7.2.0 + '@types/estree': 0.0.44 + '@types/heft-jest': 1.0.1 + '@types/node': 10.17.13 + '@typescript-eslint/experimental-utils': 3.4.0 + '@typescript-eslint/parser': 3.4.0 + '@typescript-eslint/typescript-estree': 3.4.0 + eslint: ~7.2.0 + typescript: ~3.9.7 ../../stack/eslint-plugin-security: dependencies: '@rushstack/tree-pattern': 'link:../../libraries/tree-pattern' diff --git a/common/config/rush/repo-state.json b/common/config/rush/repo-state.json index f7bb9f84c35..e99a8707693 100644 --- a/common/config/rush/repo-state.json +++ b/common/config/rush/repo-state.json @@ -1,5 +1,5 @@ // DO NOT MODIFY THIS FILE. It is generated and used by Rush. { - "pnpmShrinkwrapHash": "885167eab43b7ea96909e595973332a2a63e4839", + "pnpmShrinkwrapHash": "8a89f626be4d7746badda0720b4a1d771468e646", "preferredVersionsHash": "0f2f367d951f4cd546b698d668533c1ff056e334" } From 4cca123d473128b39e1d53d89ee32704931052f3 Mon Sep 17 00:00:00 2001 From: Pete Gonzalez <4673363+octogonz@users.noreply.github.com> Date: Fri, 2 Oct 2020 22:18:57 -0700 Subject: [PATCH 06/23] Update packlets-tutorial to reference @rushstack/eslint-plugin-packlets --- tutorials/packlets-tutorial/.eslintrc.js | 13 ++++++++++++- tutorials/packlets-tutorial/package.json | 1 + 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/tutorials/packlets-tutorial/.eslintrc.js b/tutorials/packlets-tutorial/.eslintrc.js index 60160b354c4..0eadcc77421 100644 --- a/tutorials/packlets-tutorial/.eslintrc.js +++ b/tutorials/packlets-tutorial/.eslintrc.js @@ -3,5 +3,16 @@ require('@rushstack/eslint-config/patch/modern-module-resolution'); module.exports = { extends: ['@rushstack/eslint-config/profile/node'], - parserOptions: { tsconfigRootDir: __dirname } + plugins: ['@rushstack/eslint-plugin-packlets'], + parserOptions: { tsconfigRootDir: __dirname }, + overrides: [ + { + // Declare an override that applies to TypeScript files only + files: ['*.ts', '*.tsx'], + + rules: { + '@rushstack/packlets/import-path': 'warn' + } + } + ] }; diff --git a/tutorials/packlets-tutorial/package.json b/tutorials/packlets-tutorial/package.json index 2dfd6155fe8..922bfc54aab 100644 --- a/tutorials/packlets-tutorial/package.json +++ b/tutorials/packlets-tutorial/package.json @@ -10,6 +10,7 @@ }, "devDependencies": { "@rushstack/eslint-config": "workspace:*", + "@rushstack/eslint-plugin-packlets": "workspace:*", "@rushstack/heft": "workspace:*", "@types/node": "10.17.13", "eslint": "~7.2.0", From 379bc5d7077e20a42500550776011abad7f44c61 Mon Sep 17 00:00:00 2001 From: Pete Gonzalez <4673363+octogonz@users.noreply.github.com> Date: Fri, 2 Oct 2020 22:19:51 -0700 Subject: [PATCH 07/23] rush update --- common/config/rush/nonbrowser-approved-packages.json | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/common/config/rush/nonbrowser-approved-packages.json b/common/config/rush/nonbrowser-approved-packages.json index e2a8a151b98..0b8fccbc642 100644 --- a/common/config/rush/nonbrowser-approved-packages.json +++ b/common/config/rush/nonbrowser-approved-packages.json @@ -174,6 +174,10 @@ "name": "@rushstack/eslint-plugin", "allowedCategories": [ "libraries" ] }, + { + "name": "@rushstack/eslint-plugin-packlets", + "allowedCategories": [ "tests" ] + }, { "name": "@rushstack/eslint-plugin-security", "allowedCategories": [ "libraries" ] From 730dcec680dd52b25bcbb1fd3d8dfa4ca422b5a8 Mon Sep 17 00:00:00 2001 From: Pete Gonzalez <4673363+octogonz@users.noreply.github.com> Date: Fri, 2 Oct 2020 22:20:17 -0700 Subject: [PATCH 08/23] Initial sketch of eslint-plugin-packlets validations --- .../src/PackletImportAnalyzer.ts | 51 +++ stack/eslint-plugin-packlets/src/Path.ts | 34 ++ .../eslint-plugin-packlets/src/import-path.ts | 303 ++++++++++++++++++ stack/eslint-plugin-packlets/src/index.ts | 6 +- stack/eslint-plugin-packlets/tsconfig.json | 2 +- 5 files changed, 394 insertions(+), 2 deletions(-) create mode 100644 stack/eslint-plugin-packlets/src/PackletImportAnalyzer.ts create mode 100644 stack/eslint-plugin-packlets/src/Path.ts create mode 100644 stack/eslint-plugin-packlets/src/import-path.ts diff --git a/stack/eslint-plugin-packlets/src/PackletImportAnalyzer.ts b/stack/eslint-plugin-packlets/src/PackletImportAnalyzer.ts new file mode 100644 index 00000000000..ec46de1c8c1 --- /dev/null +++ b/stack/eslint-plugin-packlets/src/PackletImportAnalyzer.ts @@ -0,0 +1,51 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. +// See LICENSE in the project root for license information. + +import type { MessageIds } from './import-path'; + +export interface ILintError { + messageId: MessageIds; + data?: Readonly>; +} + +/* +export enum PathKind { + PackletEntryPoint, + PackletIndex, + PackageMember, + ProjectFile, + SourceFileError +} + +export interface IParsedFilePath { + pathKind: PathKind; + sourceFileError: ILintError | undefined; +} + +export interface IAnalyzeImportOptions { + sourceFileParsedPath: IParsedFilePath; + modulePath: string; +} + +export class PackletImportAnalyzer { + public static analyzeSourceFile(sourceFileAbsolutePath: string): IParsedFilePath { + // file path: c:/one/two/src/packlets/three/four.ts + // + // packletsFolder: c:/one/two/src/packlets + // srcFolder: c:/one/two/src/packlets + return { + pathKind: PathKind.SourceFileError, + sourceFileError: { + messageId: + } + }; + } + + public static analyzeImportExportPath(options: IAnalyzeImportOptions): IParsedPath { + return { + pathKind: PathKind.SourceFileError, + sourceFileError: new Error('Oops') + }; + } +} +*/ diff --git a/stack/eslint-plugin-packlets/src/Path.ts b/stack/eslint-plugin-packlets/src/Path.ts new file mode 100644 index 00000000000..9a576f0fa20 --- /dev/null +++ b/stack/eslint-plugin-packlets/src/Path.ts @@ -0,0 +1,34 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. +// See LICENSE in the project root for license information. + +import * as path from 'path'; + +export class Path { + private static _relativePathRegex: RegExp = /^[.\/\\]+$/; + + /** + * Returns true if "childPath" is located inside the "parentFolderPath" folder + * or one of its child folders. Note that "parentFolderPath" is not considered to be + * under itself. The "childPath" can refer to any type of file system object. + * + * @remarks + * The indicated file/folder objects are not required to actually exist on disk. + * For example, "parentFolderPath" is interpreted as a folder name even if it refers to a file. + * If the paths are relative, they will first be resolved using path.resolve(). + */ + public static isUnder(childPath: string, parentFolderPath: string): boolean { + const relativePath: string = path.relative(childPath, parentFolderPath); + return Path._relativePathRegex.test(relativePath); + } + + /** + * Returns true if `path1` and `path2` refer to the same underlying path. + * + * @remarks + * + * The comparison is performed using `path.relative()`. + */ + public static isEqual(path1: string, path2: string): boolean { + return path.relative(path1, path2) === ''; + } +} diff --git a/stack/eslint-plugin-packlets/src/import-path.ts b/stack/eslint-plugin-packlets/src/import-path.ts new file mode 100644 index 00000000000..ad17b1ac745 --- /dev/null +++ b/stack/eslint-plugin-packlets/src/import-path.ts @@ -0,0 +1,303 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. +// See LICENSE in the project root for license information. + +import * as path from 'path'; +import * as fs from 'fs'; + +import type { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils'; +import { AST_NODE_TYPES } from '@typescript-eslint/experimental-utils'; +import { Path } from './Path'; + +import { ILintError } from './PackletImportAnalyzer'; + +export type MessageIds = + | 'missing-tsconfig' + | 'missing-src-folder' + | 'packlet-folder-case' + | 'invalid-packlet-name' + | 'misplaced-packlets-folder' + | 'bypassed-entry-point' + | 'circular-entry-point' + | 'packlet-importing-project-file'; +type Options = []; + +const validPackletName: RegExp = /^[a-z0-9]+(-[a-z0-9]+)*$/; + +const importPath: TSESLint.RuleModule = { + meta: { + type: 'problem', + messages: { + 'missing-tsconfig': + 'In order to use @rushstack/eslint-plugin-packlets, your ESLint config file' + + ' must configure the TypeScript parser', + 'missing-src-folder': 'Expecting to find a "src" folder at: {{srcFolderPath}}', + 'packlet-folder-case': 'The packlets folder must be all lower case: {{packletsFolderPath}}', + 'invalid-packlet-name': + 'Invalid packlet name "{{packletName}}".' + + ' The name must be lowercase alphanumeric words separated by hyphens. Example: "my-packlet"', + 'misplaced-packlets-folder': 'The packlets folder must be located at "{{expectedPackletsFolder}}"', + 'bypassed-entry-point': + 'The import statement does not use the packlet\'s entry point "{{entryPointModulePath}}"', + 'circular-entry-point': 'Files under a packlet folder must not import from their own index.ts file', + 'packlet-importing-project-file': + 'A local project file cannot be imported.' + + " A packlet's dependencies must be NPM packages and/or other packlets." + }, + schema: [ + { + type: 'object', + additionalProperties: false + } + ], + docs: { + description: + 'Requires regular expressions to be constructed from string constants rather than dynamically' + + ' building strings at runtime.', + category: 'Best Practices', + recommended: 'warn', + url: 'https://www.npmjs.com/package/@rushstack/eslint-plugin-packlets' + } + }, + + create: (context: TSESLint.RuleContext) => { + interface IResult { + globalError: ILintError | undefined; + skip: boolean; + packletsEnabled: boolean; + packletsFolderPath: string | undefined; + packletName: string | undefined; + } + + function f(inputFilePath: string): IResult { + const result: IResult = { + globalError: undefined, + skip: false, + packletsEnabled: false, + packletsFolderPath: undefined, + packletName: undefined + }; + // Example: /path/to/my-project/tsconfig.json + const tsconfigFilePath: string | undefined = context.parserServices?.program.getCompilerOptions()[ + 'configFilePath' + ] as string; + + // Example: /path/to/my-project/src + let srcFolderPath: string | undefined; + + if (!tsconfigFilePath) { + result.globalError = { messageId: 'missing-tsconfig' }; + return result; + } + + srcFolderPath = path.join(path.dirname(tsconfigFilePath), 'src'); + + if (!fs.existsSync(srcFolderPath)) { + result.globalError = { messageId: 'missing-src-folder', data: { srcFolderPath } }; + return result; + } + + if (!Path.isUnder(inputFilePath, srcFolderPath)) { + // Ignore files outside the "src" folder + result.skip = true; + return result; + } + + // Example: packlets/my-packlet/index.ts + const inputFilePathRelativeToSrc: string = path.relative(srcFolderPath, inputFilePath); + + // Example: [ 'packlets', 'my-packlet', 'index.ts' ] + const pathParts: string[] = inputFilePathRelativeToSrc.split(/[\/\\]+/); + + let underPackletsFolder: boolean = false; + + const expectedPackletsFolder: string = path.join(srcFolderPath, 'packlets'); + + for (let i = 0; i < pathParts.length; ++i) { + const pathPart: string = pathParts[i]; + if (pathPart.toUpperCase() === 'PACKLETS') { + if (pathPart !== 'packlets') { + // Example: /path/to/my-project/src/PACKLETS + const packletsFolderPath: string = path.join(srcFolderPath, ...pathParts.slice(0, i + 1)); + result.globalError = { messageId: 'packlet-folder-case', data: { packletsFolderPath } }; + return result; + } + + if (i !== 0) { + result.globalError = { messageId: 'misplaced-packlets-folder', data: { expectedPackletsFolder } }; + return result; + } + + underPackletsFolder = true; + } + } + + if (underPackletsFolder || fs.existsSync(expectedPackletsFolder)) { + // packletsAbsolutePath + result.packletsEnabled = true; + result.packletsFolderPath = expectedPackletsFolder; + } + + if (underPackletsFolder && pathParts.length >= 2) { + // Example: 'my-packlet' + const packletName: string = pathParts[1]; + result.packletName = packletName; + + // Example: 'index.ts' or 'index.tsx' + const thirdPart: string = pathParts[2]; + + // Example: 'index' + const thirdPartWithoutExtension: string = path.parse(thirdPart).name; + + if (thirdPartWithoutExtension.toUpperCase() === 'INDEX') { + if (!validPackletName.test(packletName)) { + result.globalError = { messageId: 'invalid-packlet-name', data: { packletName } }; + return result; + } + } + } + + return result; + } + + // Example: /path/to/my-project/src/packlets/my-packlet/index.ts + const inputFilePath: string = context.getFilename(); + const result: IResult = f(inputFilePath); + if (result.skip) { + return {}; + } + if (result.globalError === undefined && !result.packletsEnabled) { + return {}; + } + + return { + // Match the first node in the source file. Ideally we should be matching "Program > :first-child" + // so a warning doesn't highlight the whole file. But that's blocked behind a bug in the query selector: + // https://github.com/estools/esquery/issues/114 + Program: (node: TSESTree.Node): void => { + if (result.globalError) { + context.report({ + node: node, + messageId: result.globalError.messageId, + data: result.globalError.data + }); + } + }, + + // ImportDeclaration matches these forms: + // import { X } from '../../packlets/other-packlet'; + // import X from '../../packlets/other-packlet'; + // import type { X, Y } from '../../packlets/other-packlet'; + // import * as X from '../../packlets/other-packlet'; + // + // ExportNamedDeclaration matches these forms: + // export { X } from '../../packlets/other-packlet'; + // + // ExportAllDeclaration matches these forms: + // export * from '../../packlets/other-packlet'; + // export * as X from '../../packlets/other-packlet'; + 'ImportDeclaration, ExportNamedDeclaration, ExportAllDeclaration': ( + node: TSESTree.ImportDeclaration | TSESTree.ExportNamedDeclaration | TSESTree.ExportAllDeclaration + ): void => { + if (node.source?.type === AST_NODE_TYPES.Literal) { + if (result.packletsEnabled && result.packletsFolderPath) { + // Extract the import/export module path + // Example: "../../packlets/other-packlet" + const modulePath = node.source.value; + if (typeof modulePath !== 'string') { + return; + } + + if (!(modulePath.startsWith('.') || modulePath.startsWith('..'))) { + // It's not a local import. + + // Examples: + // import { X } from "npm-package"; + // import { X } from "raw-loader!./webpack-file.ts"; + return; + } + + // Example: /path/to/my-project/src/packlets/my-packlet + const inputFileFolder: string = path.dirname(inputFilePath); + + // Example: /path/to/my-project/src/other-packlet/index + const importedPath: string = path.resolve(inputFileFolder, modulePath); + + // Is the imported path referring to a file under the src/packlets folder? + if (Path.isUnder(importedPath, result.packletsFolderPath)) { + // Example: other-packlet/index + const importedPathRelativeToPackletsFolder: string = path.relative( + result.packletsFolderPath, + importedPath + ); + // Example: [ 'other-packlet', 'index' ] + const importedPathParts: string[] = importedPathRelativeToPackletsFolder.split(/[\/\\]+/); + if (importedPathParts.length > 0) { + // Example: 'other-packlet' + const importedPackletName: string = importedPathParts[0]; + + // We are importing from a packlet. Is the input file part of the same packlet? + if (result.packletName && importedPackletName === result.packletName) { + // Yes. Then our import must NOT use the packlet entry point. + + // Example: 'index' + // + // We discard the file extension to handle a degenerate case like: + // import { X } from "../index.js"; + const lastPart: string = path.parse(importedPathParts[importedPathParts.length - 1]).name; + let pathToCompare: string; + if (lastPart.toUpperCase() === 'INDEX') { + // Example: + // importedPath = /path/to/my-project/src/other-packlet/index + // pathToCompare = /path/to/my-project/src/other-packlet + pathToCompare = path.dirname(importedPath); + } else { + pathToCompare = importedPath; + } + + // Example: /path/to/my-project/src/other-packlet + const entryPointPath: string = path.join(result.packletsFolderPath, importedPackletName); + + if (Path.isEqual(pathToCompare, entryPointPath)) { + context.report({ + node: node, + messageId: 'circular-entry-point' + }); + } + } else { + // No. If we are not part of the same packlet, then the module path must refer + // to the index.ts entry point. + + // Example: /path/to/my-project/src/other-packlet + const entryPointPath: string = path.join(result.packletsFolderPath, importedPackletName); + + if (!Path.isEqual(importedPath, entryPointPath)) { + const entryPointModulePath: string = path.posix.relative( + importedPackletName, + inputFileFolder + ); + + context.report({ + node: node, + messageId: 'bypassed-entry-point', + data: { entryPointModulePath } + }); + } + } + } + } else { + // The imported path does NOT refer to a file under the src/packlets folder + if (result.packletName) { + context.report({ + node: node, + messageId: 'packlet-importing-project-file' + }); + } + } + } + } + } + }; + } +}; + +export { importPath }; diff --git a/stack/eslint-plugin-packlets/src/index.ts b/stack/eslint-plugin-packlets/src/index.ts index 4166b4f967f..83953a92292 100644 --- a/stack/eslint-plugin-packlets/src/index.ts +++ b/stack/eslint-plugin-packlets/src/index.ts @@ -2,13 +2,17 @@ // See LICENSE in the project root for license information. import { TSESLint } from '@typescript-eslint/experimental-utils'; +import { importPath } from './import-path'; interface IPlugin { rules: { [ruleName: string]: TSESLint.RuleModule }; } const plugin: IPlugin = { - rules: {} + rules: { + // Full name: "@rushstack/packlets/import-path" + 'import-path': importPath + } }; export = plugin; diff --git a/stack/eslint-plugin-packlets/tsconfig.json b/stack/eslint-plugin-packlets/tsconfig.json index 1de40fe9a71..fbc2f5c0a6c 100644 --- a/stack/eslint-plugin-packlets/tsconfig.json +++ b/stack/eslint-plugin-packlets/tsconfig.json @@ -2,6 +2,6 @@ "extends": "./node_modules/@rushstack/heft-node-rig/profiles/default/tsconfig-base.json", "compilerOptions": { - "types": ["heft-jest"] + "types": ["heft-jest", "node"] } } From 9340f6df87af422acb78ed7abb12cc3afa52d8b4 Mon Sep 17 00:00:00 2001 From: Pete Gonzalez <4673363+octogonz@users.noreply.github.com> Date: Sat, 3 Oct 2020 00:32:49 -0700 Subject: [PATCH 09/23] Prototype of cyclic import detection --- .../eslint-plugin-packlets/src/import-path.ts | 336 ++++++++++++------ tutorials/packlets-tutorial/config/heft.json | 2 +- tutorials/packlets-tutorial/src/app/App.ts | 2 +- .../src/packlets/data-model/DataModel.ts | 7 + 4 files changed, 242 insertions(+), 105 deletions(-) diff --git a/stack/eslint-plugin-packlets/src/import-path.ts b/stack/eslint-plugin-packlets/src/import-path.ts index ad17b1ac745..e8622e2195d 100644 --- a/stack/eslint-plugin-packlets/src/import-path.ts +++ b/stack/eslint-plugin-packlets/src/import-path.ts @@ -1,11 +1,12 @@ // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. // See LICENSE in the project root for license information. +import type * as ts from 'typescript'; import * as path from 'path'; import * as fs from 'fs'; import type { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils'; -import { AST_NODE_TYPES } from '@typescript-eslint/experimental-utils'; +import { AST_NODE_TYPES, ESLintUtils } from '@typescript-eslint/experimental-utils'; import { Path } from './Path'; import { ILintError } from './PackletImportAnalyzer'; @@ -18,11 +19,192 @@ export type MessageIds = | 'misplaced-packlets-folder' | 'bypassed-entry-point' | 'circular-entry-point' - | 'packlet-importing-project-file'; + | 'packlet-importing-project-file' + | 'circular-import' + | 'debug'; type Options = []; const validPackletName: RegExp = /^[a-z0-9]+(-[a-z0-9]+)*$/; +export enum RefFileKind { + Import, + ReferenceFile, + TypeReferenceDirective +} + +interface RefFile { + referencedFileName: string; + kind: RefFileKind; + index: number; + file: string; +} + +interface IResult { + globalError: ILintError | undefined; + skip: boolean; + packletsEnabled: boolean; + packletsFolderPath: string | undefined; + packletName: string | undefined; + isEntryPoint: boolean; +} + +function f(inputFilePath: string, tsconfigFilePath: string | undefined): IResult { + const result: IResult = { + globalError: undefined, + skip: false, + packletsEnabled: false, + packletsFolderPath: undefined, + packletName: undefined, + isEntryPoint: false + }; + + // Example: /path/to/my-project/src + let srcFolderPath: string | undefined; + + if (!tsconfigFilePath) { + result.globalError = { messageId: 'missing-tsconfig' }; + return result; + } + + srcFolderPath = path.join(path.dirname(tsconfigFilePath), 'src'); + + if (!fs.existsSync(srcFolderPath)) { + result.globalError = { messageId: 'missing-src-folder', data: { srcFolderPath } }; + return result; + } + + if (!Path.isUnder(inputFilePath, srcFolderPath)) { + // Ignore files outside the "src" folder + result.skip = true; + return result; + } + + // Example: packlets/my-packlet/index.ts + const inputFilePathRelativeToSrc: string = path.relative(srcFolderPath, inputFilePath); + + // Example: [ 'packlets', 'my-packlet', 'index.ts' ] + const pathParts: string[] = inputFilePathRelativeToSrc.split(/[\/\\]+/); + + let underPackletsFolder: boolean = false; + + const expectedPackletsFolder: string = path.join(srcFolderPath, 'packlets'); + + for (let i = 0; i < pathParts.length; ++i) { + const pathPart: string = pathParts[i]; + if (pathPart.toUpperCase() === 'PACKLETS') { + if (pathPart !== 'packlets') { + // Example: /path/to/my-project/src/PACKLETS + const packletsFolderPath: string = path.join(srcFolderPath, ...pathParts.slice(0, i + 1)); + result.globalError = { messageId: 'packlet-folder-case', data: { packletsFolderPath } }; + return result; + } + + if (i !== 0) { + result.globalError = { messageId: 'misplaced-packlets-folder', data: { expectedPackletsFolder } }; + return result; + } + + underPackletsFolder = true; + } + } + + if (underPackletsFolder || fs.existsSync(expectedPackletsFolder)) { + // packletsAbsolutePath + result.packletsEnabled = true; + result.packletsFolderPath = expectedPackletsFolder; + } + + if (underPackletsFolder && pathParts.length >= 2) { + // Example: 'my-packlet' + const packletName: string = pathParts[1]; + result.packletName = packletName; + + // Example: 'index.ts' or 'index.tsx' + const thirdPart: string = pathParts[2]; + + // Example: 'index' + const thirdPartWithoutExtension: string = path.parse(thirdPart).name; + + if (thirdPartWithoutExtension.toUpperCase() === 'INDEX') { + if (!validPackletName.test(packletName)) { + result.globalError = { messageId: 'invalid-packlet-name', data: { packletName } }; + return result; + } + + result.isEntryPoint = true; + } + } + + return result; +} + +interface ILink { + previous: ILink | undefined; + packletName: string; + fromFilePath: string; +} + +function loop( + packletName: string, + startingPackletName: string, + refFileMap: Map, + program: ts.Program, + packletsFolderPath: string, + visitedPacklets: Set, + previousLink: ILink | undefined, + context: TSESLint.RuleContext +): ILink | undefined { + const packletEntryPoint: string = path.join(packletsFolderPath, packletName, 'index'); + + const tsSourceFile: ts.SourceFile | undefined = + program.getSourceFile(packletEntryPoint + '.ts') || program.getSourceFile(packletEntryPoint + '.tsx'); + if (tsSourceFile) { + const refFiles: RefFile[] | undefined = refFileMap.get((tsSourceFile as any).path as any); + if (refFiles) { + for (const refFile of refFiles) { + if (refFile.kind === RefFileKind.Import) { + const referencingFilePath: string = refFile.file; + + if (Path.isUnder(referencingFilePath, packletsFolderPath)) { + const referencingRelativePath: string = path.relative(packletsFolderPath, referencingFilePath); + const referencingPathParts: string[] = referencingRelativePath.split(/[\/\\]+/); + const referencingPackletName: string = referencingPathParts[0]; + + if (!visitedPacklets.has(packletName)) { + visitedPacklets.add(packletName); + + const link2: ILink = { + previous: previousLink, + fromFilePath: referencingFilePath, + packletName: packletName + }; + + if (referencingPackletName === startingPackletName) { + return link2; + } + + const result: ILink | undefined = loop( + referencingPackletName, + startingPackletName, + refFileMap, + program, + packletsFolderPath, + visitedPacklets, + link2, + context + ); + if (result) { + return result; + } + } + } + } + } + } + } + return undefined; +} + const importPath: TSESLint.RuleModule = { meta: { type: 'problem', @@ -41,7 +223,9 @@ const importPath: TSESLint.RuleModule = { 'circular-entry-point': 'Files under a packlet folder must not import from their own index.ts file', 'packlet-importing-project-file': 'A local project file cannot be imported.' + - " A packlet's dependencies must be NPM packages and/or other packlets." + " A packlet's dependencies must be NPM packages and/or other packlets.", + 'circular-import': 'Packlet imports create a circular reference:\n{{report}}', + debug: 'debug {{debug}}' }, schema: [ { @@ -60,108 +244,15 @@ const importPath: TSESLint.RuleModule = { }, create: (context: TSESLint.RuleContext) => { - interface IResult { - globalError: ILintError | undefined; - skip: boolean; - packletsEnabled: boolean; - packletsFolderPath: string | undefined; - packletName: string | undefined; - } - - function f(inputFilePath: string): IResult { - const result: IResult = { - globalError: undefined, - skip: false, - packletsEnabled: false, - packletsFolderPath: undefined, - packletName: undefined - }; - // Example: /path/to/my-project/tsconfig.json - const tsconfigFilePath: string | undefined = context.parserServices?.program.getCompilerOptions()[ - 'configFilePath' - ] as string; - - // Example: /path/to/my-project/src - let srcFolderPath: string | undefined; - - if (!tsconfigFilePath) { - result.globalError = { messageId: 'missing-tsconfig' }; - return result; - } - - srcFolderPath = path.join(path.dirname(tsconfigFilePath), 'src'); - - if (!fs.existsSync(srcFolderPath)) { - result.globalError = { messageId: 'missing-src-folder', data: { srcFolderPath } }; - return result; - } - - if (!Path.isUnder(inputFilePath, srcFolderPath)) { - // Ignore files outside the "src" folder - result.skip = true; - return result; - } - - // Example: packlets/my-packlet/index.ts - const inputFilePathRelativeToSrc: string = path.relative(srcFolderPath, inputFilePath); - - // Example: [ 'packlets', 'my-packlet', 'index.ts' ] - const pathParts: string[] = inputFilePathRelativeToSrc.split(/[\/\\]+/); - - let underPackletsFolder: boolean = false; - - const expectedPackletsFolder: string = path.join(srcFolderPath, 'packlets'); - - for (let i = 0; i < pathParts.length; ++i) { - const pathPart: string = pathParts[i]; - if (pathPart.toUpperCase() === 'PACKLETS') { - if (pathPart !== 'packlets') { - // Example: /path/to/my-project/src/PACKLETS - const packletsFolderPath: string = path.join(srcFolderPath, ...pathParts.slice(0, i + 1)); - result.globalError = { messageId: 'packlet-folder-case', data: { packletsFolderPath } }; - return result; - } - - if (i !== 0) { - result.globalError = { messageId: 'misplaced-packlets-folder', data: { expectedPackletsFolder } }; - return result; - } - - underPackletsFolder = true; - } - } - - if (underPackletsFolder || fs.existsSync(expectedPackletsFolder)) { - // packletsAbsolutePath - result.packletsEnabled = true; - result.packletsFolderPath = expectedPackletsFolder; - } - - if (underPackletsFolder && pathParts.length >= 2) { - // Example: 'my-packlet' - const packletName: string = pathParts[1]; - result.packletName = packletName; - - // Example: 'index.ts' or 'index.tsx' - const thirdPart: string = pathParts[2]; - - // Example: 'index' - const thirdPartWithoutExtension: string = path.parse(thirdPart).name; - - if (thirdPartWithoutExtension.toUpperCase() === 'INDEX') { - if (!validPackletName.test(packletName)) { - result.globalError = { messageId: 'invalid-packlet-name', data: { packletName } }; - return result; - } - } - } - - return result; - } - // Example: /path/to/my-project/src/packlets/my-packlet/index.ts const inputFilePath: string = context.getFilename(); - const result: IResult = f(inputFilePath); + + // Example: /path/to/my-project/tsconfig.json + const tsconfigFilePath: string | undefined = ESLintUtils.getParserServices( + context + ).program.getCompilerOptions()['configFilePath'] as string; + + const result: IResult = f(inputFilePath, tsconfigFilePath); if (result.skip) { return {}; } @@ -174,6 +265,45 @@ const importPath: TSESLint.RuleModule = { // so a warning doesn't highlight the whole file. But that's blocked behind a bug in the query selector: // https://github.com/estools/esquery/issues/114 Program: (node: TSESTree.Node): void => { + if (result.isEntryPoint && !result.globalError) { + const visitedPacklets: Set = new Set(); + const program: ts.Program | undefined = context.parserServices?.program; + if (program) { + const refFileMap: Map = (program as any).getRefFileMap(); + + const resultLink: ILink | undefined = loop( + result.packletName!, + result.packletName!, + refFileMap, + program, + result.packletsFolderPath!, + visitedPacklets, + undefined, + context + ); + + if (resultLink) { + const tsconfigFileFolder: string = path.dirname(tsconfigFilePath); + + let report: string = ''; + const affectedPackletNames: string[] = []; + + for (let current: ILink | undefined = resultLink; current; current = current.previous) { + affectedPackletNames.push(current.packletName); + const filePath: string = path.relative(tsconfigFileFolder, current.fromFilePath); + report += `"${current.packletName}" is referenced by ${filePath}\n`; + } + + // If 3 different packlets form a circular dependency, we don't need to report the same warning 3 times. + // Instead, only report the warning for the alphabetically smallest packlet. + affectedPackletNames.sort(); + if (affectedPackletNames[0] === result.packletName) { + result.globalError = { messageId: 'circular-import', data: { report: report } }; + } + } + } + } + if (result.globalError) { context.report({ node: node, diff --git a/tutorials/packlets-tutorial/config/heft.json b/tutorials/packlets-tutorial/config/heft.json index 99e058540fb..8a64ee4f00c 100644 --- a/tutorials/packlets-tutorial/config/heft.json +++ b/tutorials/packlets-tutorial/config/heft.json @@ -28,7 +28,7 @@ /** * Glob patterns to be deleted. The paths are resolved relative to the project folder. */ - "globsToDelete": ["dist", "lib", "temp"] + "globsToDelete": ["dist", "lib", "temp", ".heft/build-cache/eslint.json"] } ], diff --git a/tutorials/packlets-tutorial/src/app/App.ts b/tutorials/packlets-tutorial/src/app/App.ts index 5e30bf98e26..d1ff6599f28 100644 --- a/tutorials/packlets-tutorial/src/app/App.ts +++ b/tutorials/packlets-tutorial/src/app/App.ts @@ -10,7 +10,7 @@ export class App { const logger: Logger = new Logger(); logger.log(MessageType.Info, 'Starting app...'); - const dataModel: DataModel = new ExampleModel(); + const dataModel: DataModel = new ExampleModel(logger); const report: MainReport = new MainReport(logger); report.showReport(dataModel); diff --git a/tutorials/packlets-tutorial/src/packlets/data-model/DataModel.ts b/tutorials/packlets-tutorial/src/packlets/data-model/DataModel.ts index b651f3fc6df..56f50de2f2b 100644 --- a/tutorials/packlets-tutorial/src/packlets/data-model/DataModel.ts +++ b/tutorials/packlets-tutorial/src/packlets/data-model/DataModel.ts @@ -1,6 +1,8 @@ // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. // See LICENSE in the project root for license information. +import { Logger } from '../../packlets/logging'; + export interface IDataRecord { firstName: string; lastName: string; @@ -8,5 +10,10 @@ export interface IDataRecord { } export abstract class DataModel { + protected readonly logger: Logger; + + public constructor(logger: Logger) { + this.logger = logger; + } public abstract queryRecords(): IDataRecord[]; } From a280b252795ba07fe56a5ff70706d777161fab69 Mon Sep 17 00:00:00 2001 From: Pete Gonzalez <4673363+octogonz@users.noreply.github.com> Date: Sun, 4 Oct 2020 01:46:05 -0700 Subject: [PATCH 10/23] split "import-path" into two rules "mechanics" and "circular-deps" --- .../src/PackletImportAnalyzer.ts | 2 +- .../src/circular-deps.ts | 40 +++++++++++++++++++ stack/eslint-plugin-packlets/src/index.ts | 9 +++-- .../src/{import-path.ts => mechanics.ts} | 0 4 files changed, 47 insertions(+), 4 deletions(-) create mode 100644 stack/eslint-plugin-packlets/src/circular-deps.ts rename stack/eslint-plugin-packlets/src/{import-path.ts => mechanics.ts} (100%) diff --git a/stack/eslint-plugin-packlets/src/PackletImportAnalyzer.ts b/stack/eslint-plugin-packlets/src/PackletImportAnalyzer.ts index ec46de1c8c1..1b37637072f 100644 --- a/stack/eslint-plugin-packlets/src/PackletImportAnalyzer.ts +++ b/stack/eslint-plugin-packlets/src/PackletImportAnalyzer.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. // See LICENSE in the project root for license information. -import type { MessageIds } from './import-path'; +import type { MessageIds } from './mechanics'; export interface ILintError { messageId: MessageIds; diff --git a/stack/eslint-plugin-packlets/src/circular-deps.ts b/stack/eslint-plugin-packlets/src/circular-deps.ts new file mode 100644 index 00000000000..7a8986027bd --- /dev/null +++ b/stack/eslint-plugin-packlets/src/circular-deps.ts @@ -0,0 +1,40 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. +// See LICENSE in the project root for license information. + +import type * as ts from 'typescript'; +import * as path from 'path'; +import * as fs from 'fs'; + +import type { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils'; +import { AST_NODE_TYPES, ESLintUtils } from '@typescript-eslint/experimental-utils'; +import { Path } from './Path'; + +import { ILintError } from './PackletImportAnalyzer'; + +export type MessageIds = 'circular-import'; +type Options = []; + +const circularDeps: TSESLint.RuleModule = { + meta: { + type: 'problem', + messages: { 'circular-import': '' }, + schema: [ + { + type: 'object', + additionalProperties: false + } + ], + docs: { + description: '', + category: 'Best Practices', + recommended: 'warn', + url: 'https://www.npmjs.com/package/@rushstack/eslint-plugin-packlets' + } + }, + + create: (context: TSESLint.RuleContext) => { + return {}; + } +}; + +export { circularDeps }; diff --git a/stack/eslint-plugin-packlets/src/index.ts b/stack/eslint-plugin-packlets/src/index.ts index 83953a92292..24a44dd6ff8 100644 --- a/stack/eslint-plugin-packlets/src/index.ts +++ b/stack/eslint-plugin-packlets/src/index.ts @@ -2,7 +2,8 @@ // See LICENSE in the project root for license information. import { TSESLint } from '@typescript-eslint/experimental-utils'; -import { importPath } from './import-path'; +import { importPath } from './mechanics'; +import { circularDeps } from './circular-deps'; interface IPlugin { rules: { [ruleName: string]: TSESLint.RuleModule }; @@ -10,8 +11,10 @@ interface IPlugin { const plugin: IPlugin = { rules: { - // Full name: "@rushstack/packlets/import-path" - 'import-path': importPath + // Full name: "@rushstack/packlets/mechanics" + 'import-path': importPath, + // Full name: "@rushstack/packlets/circular-deps" + 'circular-deps': circularDeps } }; diff --git a/stack/eslint-plugin-packlets/src/import-path.ts b/stack/eslint-plugin-packlets/src/mechanics.ts similarity index 100% rename from stack/eslint-plugin-packlets/src/import-path.ts rename to stack/eslint-plugin-packlets/src/mechanics.ts From 6ed2d7129704da3d02a5869df78e847c8d9088a7 Mon Sep 17 00:00:00 2001 From: Pete Gonzalez <4673363+octogonz@users.noreply.github.com> Date: Sun, 4 Oct 2020 02:02:22 -0700 Subject: [PATCH 11/23] split logic into two rules "mechanics" and "circular-deps" --- .../src/PackletImportAnalyzer.ts | 140 +++++++--- .../src/circular-deps.ts | 150 ++++++++++- stack/eslint-plugin-packlets/src/index.ts | 4 +- stack/eslint-plugin-packlets/src/mechanics.ts | 251 +----------------- tutorials/packlets-tutorial/.eslintrc.js | 3 +- 5 files changed, 263 insertions(+), 285 deletions(-) diff --git a/stack/eslint-plugin-packlets/src/PackletImportAnalyzer.ts b/stack/eslint-plugin-packlets/src/PackletImportAnalyzer.ts index 1b37637072f..638b98bc692 100644 --- a/stack/eslint-plugin-packlets/src/PackletImportAnalyzer.ts +++ b/stack/eslint-plugin-packlets/src/PackletImportAnalyzer.ts @@ -1,51 +1,123 @@ // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. // See LICENSE in the project root for license information. -import type { MessageIds } from './mechanics'; +import * as path from 'path'; +import * as fs from 'fs'; +import { Path } from './Path'; + +export type MyMessageIds = + | 'missing-tsconfig' + | 'missing-src-folder' + | 'packlet-folder-case' + | 'invalid-packlet-name' + | 'misplaced-packlets-folder'; export interface ILintError { - messageId: MessageIds; + messageId: MyMessageIds; data?: Readonly>; } -/* -export enum PathKind { - PackletEntryPoint, - PackletIndex, - PackageMember, - ProjectFile, - SourceFileError +export interface IResult { + globalError: ILintError | undefined; + skip: boolean; + packletsEnabled: boolean; + packletsFolderPath: string | undefined; + packletName: string | undefined; + isEntryPoint: boolean; } -export interface IParsedFilePath { - pathKind: PathKind; - sourceFileError: ILintError | undefined; -} +const validPackletName: RegExp = /^[a-z0-9]+(-[a-z0-9]+)*$/; -export interface IAnalyzeImportOptions { - sourceFileParsedPath: IParsedFilePath; - modulePath: string; -} +export function analyze(inputFilePath: string, tsconfigFilePath: string | undefined): IResult { + const result: IResult = { + globalError: undefined, + skip: false, + packletsEnabled: false, + packletsFolderPath: undefined, + packletName: undefined, + isEntryPoint: false + }; + + // Example: /path/to/my-project/src + let srcFolderPath: string | undefined; + + if (!tsconfigFilePath) { + result.globalError = { messageId: 'missing-tsconfig' }; + return result; + } + + srcFolderPath = path.join(path.dirname(tsconfigFilePath), 'src'); + + if (!fs.existsSync(srcFolderPath)) { + result.globalError = { messageId: 'missing-src-folder', data: { srcFolderPath } }; + return result; + } + + if (!Path.isUnder(inputFilePath, srcFolderPath)) { + // Ignore files outside the "src" folder + result.skip = true; + return result; + } + + // Example: packlets/my-packlet/index.ts + const inputFilePathRelativeToSrc: string = path.relative(srcFolderPath, inputFilePath); + + // Example: [ 'packlets', 'my-packlet', 'index.ts' ] + const pathParts: string[] = inputFilePathRelativeToSrc.split(/[\/\\]+/); + + let underPackletsFolder: boolean = false; -export class PackletImportAnalyzer { - public static analyzeSourceFile(sourceFileAbsolutePath: string): IParsedFilePath { - // file path: c:/one/two/src/packlets/three/four.ts - // - // packletsFolder: c:/one/two/src/packlets - // srcFolder: c:/one/two/src/packlets - return { - pathKind: PathKind.SourceFileError, - sourceFileError: { - messageId: + const expectedPackletsFolder: string = path.join(srcFolderPath, 'packlets'); + + for (let i = 0; i < pathParts.length; ++i) { + const pathPart: string = pathParts[i]; + if (pathPart.toUpperCase() === 'PACKLETS') { + if (pathPart !== 'packlets') { + // Example: /path/to/my-project/src/PACKLETS + const packletsFolderPath: string = path.join(srcFolderPath, ...pathParts.slice(0, i + 1)); + result.globalError = { messageId: 'packlet-folder-case', data: { packletsFolderPath } }; + return result; + } + + if (i !== 0) { + result.globalError = { messageId: 'misplaced-packlets-folder', data: { expectedPackletsFolder } }; + return result; + } + + underPackletsFolder = true; + } + } + + if (underPackletsFolder || fs.existsSync(expectedPackletsFolder)) { + // packletsAbsolutePath + result.packletsEnabled = true; + result.packletsFolderPath = expectedPackletsFolder; + } + + if (underPackletsFolder && pathParts.length >= 2) { + // Example: 'my-packlet' + const packletName: string = pathParts[1]; + result.packletName = packletName; + + // Example: 'index.ts' or 'index.tsx' + const thirdPart: string = pathParts[2]; + + // Example: 'index' + const thirdPartWithoutExtension: string = path.parse(thirdPart).name; + + if (thirdPartWithoutExtension.toUpperCase() === 'INDEX') { + if (!validPackletName.test(packletName)) { + result.globalError = { messageId: 'invalid-packlet-name', data: { packletName } }; + return result; } - }; + + result.isEntryPoint = true; + } } - public static analyzeImportExportPath(options: IAnalyzeImportOptions): IParsedPath { - return { - pathKind: PathKind.SourceFileError, - sourceFileError: new Error('Oops') - }; + if (result.globalError === undefined && !result.packletsEnabled) { + result.skip = true; } + + return result; } -*/ diff --git a/stack/eslint-plugin-packlets/src/circular-deps.ts b/stack/eslint-plugin-packlets/src/circular-deps.ts index 7a8986027bd..067f89da445 100644 --- a/stack/eslint-plugin-packlets/src/circular-deps.ts +++ b/stack/eslint-plugin-packlets/src/circular-deps.ts @@ -3,13 +3,92 @@ import type * as ts from 'typescript'; import * as path from 'path'; -import * as fs from 'fs'; import type { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils'; -import { AST_NODE_TYPES, ESLintUtils } from '@typescript-eslint/experimental-utils'; +import { ESLintUtils } from '@typescript-eslint/experimental-utils'; import { Path } from './Path'; -import { ILintError } from './PackletImportAnalyzer'; +import { analyze, IResult } from './PackletImportAnalyzer'; + +export enum RefFileKind { + Import, + ReferenceFile, + TypeReferenceDirective +} + +interface RefFile { + referencedFileName: string; + kind: RefFileKind; + index: number; + file: string; +} + +interface ILink { + previous: ILink | undefined; + packletName: string; + fromFilePath: string; +} + +function loop( + packletName: string, + startingPackletName: string, + refFileMap: Map, + program: ts.Program, + packletsFolderPath: string, + visitedPacklets: Set, + previousLink: ILink | undefined, + context: TSESLint.RuleContext +): ILink | undefined { + const packletEntryPoint: string = path.join(packletsFolderPath, packletName, 'index'); + + const tsSourceFile: ts.SourceFile | undefined = + program.getSourceFile(packletEntryPoint + '.ts') || program.getSourceFile(packletEntryPoint + '.tsx'); + if (tsSourceFile) { + const refFiles: RefFile[] | undefined = refFileMap.get((tsSourceFile as any).path as any); + if (refFiles) { + for (const refFile of refFiles) { + if (refFile.kind === RefFileKind.Import) { + const referencingFilePath: string = refFile.file; + + if (Path.isUnder(referencingFilePath, packletsFolderPath)) { + const referencingRelativePath: string = path.relative(packletsFolderPath, referencingFilePath); + const referencingPathParts: string[] = referencingRelativePath.split(/[\/\\]+/); + const referencingPackletName: string = referencingPathParts[0]; + + if (!visitedPacklets.has(packletName)) { + visitedPacklets.add(packletName); + + const link2: ILink = { + previous: previousLink, + fromFilePath: referencingFilePath, + packletName: packletName + }; + + if (referencingPackletName === startingPackletName) { + return link2; + } + + const result: ILink | undefined = loop( + referencingPackletName, + startingPackletName, + refFileMap, + program, + packletsFolderPath, + visitedPacklets, + link2, + context + ); + if (result) { + return result; + } + } + } + } + } + } + } + return undefined; +} export type MessageIds = 'circular-import'; type Options = []; @@ -17,7 +96,7 @@ type Options = []; const circularDeps: TSESLint.RuleModule = { meta: { type: 'problem', - messages: { 'circular-import': '' }, + messages: { 'circular-import': 'Packlet imports create a circular reference:\n{{report}}' }, schema: [ { type: 'object', @@ -33,7 +112,68 @@ const circularDeps: TSESLint.RuleModule = { }, create: (context: TSESLint.RuleContext) => { - return {}; + // Example: /path/to/my-project/src/packlets/my-packlet/index.ts + const inputFilePath: string = context.getFilename(); + + // Example: /path/to/my-project/tsconfig.json + const tsconfigFilePath: string | undefined = ESLintUtils.getParserServices( + context + ).program.getCompilerOptions()['configFilePath'] as string; + + const result: IResult = analyze(inputFilePath, tsconfigFilePath); + if (result.skip) { + return {}; + } + + return { + // Match the first node in the source file. Ideally we should be matching "Program > :first-child" + // so a warning doesn't highlight the whole file. But that's blocked behind a bug in the query selector: + // https://github.com/estools/esquery/issues/114 + Program: (node: TSESTree.Node): void => { + if (result.isEntryPoint && !result.globalError) { + const visitedPacklets: Set = new Set(); + const program: ts.Program | undefined = context.parserServices?.program; + if (program) { + const refFileMap: Map = (program as any).getRefFileMap(); + + const resultLink: ILink | undefined = loop( + result.packletName!, + result.packletName!, + refFileMap, + program, + result.packletsFolderPath!, + visitedPacklets, + undefined, + context + ); + + if (resultLink) { + const tsconfigFileFolder: string = path.dirname(tsconfigFilePath); + + let report: string = ''; + const affectedPackletNames: string[] = []; + + for (let current: ILink | undefined = resultLink; current; current = current.previous) { + affectedPackletNames.push(current.packletName); + const filePath: string = path.relative(tsconfigFileFolder, current.fromFilePath); + report += `"${current.packletName}" is referenced by ${filePath}\n`; + } + + // If 3 different packlets form a circular dependency, we don't need to report the same warning 3 times. + // Instead, only report the warning for the alphabetically smallest packlet. + affectedPackletNames.sort(); + if (affectedPackletNames[0] === result.packletName) { + context.report({ + node: node, + messageId: 'circular-import', + data: { report: report } + }); + } + } + } + } + } + }; } }; diff --git a/stack/eslint-plugin-packlets/src/index.ts b/stack/eslint-plugin-packlets/src/index.ts index 24a44dd6ff8..3c91ae4b0c8 100644 --- a/stack/eslint-plugin-packlets/src/index.ts +++ b/stack/eslint-plugin-packlets/src/index.ts @@ -2,7 +2,7 @@ // See LICENSE in the project root for license information. import { TSESLint } from '@typescript-eslint/experimental-utils'; -import { importPath } from './mechanics'; +import { mechanics } from './mechanics'; import { circularDeps } from './circular-deps'; interface IPlugin { @@ -12,7 +12,7 @@ interface IPlugin { const plugin: IPlugin = { rules: { // Full name: "@rushstack/packlets/mechanics" - 'import-path': importPath, + mechanics: mechanics, // Full name: "@rushstack/packlets/circular-deps" 'circular-deps': circularDeps } diff --git a/stack/eslint-plugin-packlets/src/mechanics.ts b/stack/eslint-plugin-packlets/src/mechanics.ts index e8622e2195d..8271cd5d5f1 100644 --- a/stack/eslint-plugin-packlets/src/mechanics.ts +++ b/stack/eslint-plugin-packlets/src/mechanics.ts @@ -1,211 +1,22 @@ // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. // See LICENSE in the project root for license information. -import type * as ts from 'typescript'; import * as path from 'path'; -import * as fs from 'fs'; import type { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils'; import { AST_NODE_TYPES, ESLintUtils } from '@typescript-eslint/experimental-utils'; import { Path } from './Path'; -import { ILintError } from './PackletImportAnalyzer'; +import { analyze, IResult, MyMessageIds } from './PackletImportAnalyzer'; export type MessageIds = - | 'missing-tsconfig' - | 'missing-src-folder' - | 'packlet-folder-case' - | 'invalid-packlet-name' - | 'misplaced-packlets-folder' + | MyMessageIds | 'bypassed-entry-point' | 'circular-entry-point' - | 'packlet-importing-project-file' - | 'circular-import' - | 'debug'; + | 'packlet-importing-project-file'; type Options = []; -const validPackletName: RegExp = /^[a-z0-9]+(-[a-z0-9]+)*$/; - -export enum RefFileKind { - Import, - ReferenceFile, - TypeReferenceDirective -} - -interface RefFile { - referencedFileName: string; - kind: RefFileKind; - index: number; - file: string; -} - -interface IResult { - globalError: ILintError | undefined; - skip: boolean; - packletsEnabled: boolean; - packletsFolderPath: string | undefined; - packletName: string | undefined; - isEntryPoint: boolean; -} - -function f(inputFilePath: string, tsconfigFilePath: string | undefined): IResult { - const result: IResult = { - globalError: undefined, - skip: false, - packletsEnabled: false, - packletsFolderPath: undefined, - packletName: undefined, - isEntryPoint: false - }; - - // Example: /path/to/my-project/src - let srcFolderPath: string | undefined; - - if (!tsconfigFilePath) { - result.globalError = { messageId: 'missing-tsconfig' }; - return result; - } - - srcFolderPath = path.join(path.dirname(tsconfigFilePath), 'src'); - - if (!fs.existsSync(srcFolderPath)) { - result.globalError = { messageId: 'missing-src-folder', data: { srcFolderPath } }; - return result; - } - - if (!Path.isUnder(inputFilePath, srcFolderPath)) { - // Ignore files outside the "src" folder - result.skip = true; - return result; - } - - // Example: packlets/my-packlet/index.ts - const inputFilePathRelativeToSrc: string = path.relative(srcFolderPath, inputFilePath); - - // Example: [ 'packlets', 'my-packlet', 'index.ts' ] - const pathParts: string[] = inputFilePathRelativeToSrc.split(/[\/\\]+/); - - let underPackletsFolder: boolean = false; - - const expectedPackletsFolder: string = path.join(srcFolderPath, 'packlets'); - - for (let i = 0; i < pathParts.length; ++i) { - const pathPart: string = pathParts[i]; - if (pathPart.toUpperCase() === 'PACKLETS') { - if (pathPart !== 'packlets') { - // Example: /path/to/my-project/src/PACKLETS - const packletsFolderPath: string = path.join(srcFolderPath, ...pathParts.slice(0, i + 1)); - result.globalError = { messageId: 'packlet-folder-case', data: { packletsFolderPath } }; - return result; - } - - if (i !== 0) { - result.globalError = { messageId: 'misplaced-packlets-folder', data: { expectedPackletsFolder } }; - return result; - } - - underPackletsFolder = true; - } - } - - if (underPackletsFolder || fs.existsSync(expectedPackletsFolder)) { - // packletsAbsolutePath - result.packletsEnabled = true; - result.packletsFolderPath = expectedPackletsFolder; - } - - if (underPackletsFolder && pathParts.length >= 2) { - // Example: 'my-packlet' - const packletName: string = pathParts[1]; - result.packletName = packletName; - - // Example: 'index.ts' or 'index.tsx' - const thirdPart: string = pathParts[2]; - - // Example: 'index' - const thirdPartWithoutExtension: string = path.parse(thirdPart).name; - - if (thirdPartWithoutExtension.toUpperCase() === 'INDEX') { - if (!validPackletName.test(packletName)) { - result.globalError = { messageId: 'invalid-packlet-name', data: { packletName } }; - return result; - } - - result.isEntryPoint = true; - } - } - - return result; -} - -interface ILink { - previous: ILink | undefined; - packletName: string; - fromFilePath: string; -} - -function loop( - packletName: string, - startingPackletName: string, - refFileMap: Map, - program: ts.Program, - packletsFolderPath: string, - visitedPacklets: Set, - previousLink: ILink | undefined, - context: TSESLint.RuleContext -): ILink | undefined { - const packletEntryPoint: string = path.join(packletsFolderPath, packletName, 'index'); - - const tsSourceFile: ts.SourceFile | undefined = - program.getSourceFile(packletEntryPoint + '.ts') || program.getSourceFile(packletEntryPoint + '.tsx'); - if (tsSourceFile) { - const refFiles: RefFile[] | undefined = refFileMap.get((tsSourceFile as any).path as any); - if (refFiles) { - for (const refFile of refFiles) { - if (refFile.kind === RefFileKind.Import) { - const referencingFilePath: string = refFile.file; - - if (Path.isUnder(referencingFilePath, packletsFolderPath)) { - const referencingRelativePath: string = path.relative(packletsFolderPath, referencingFilePath); - const referencingPathParts: string[] = referencingRelativePath.split(/[\/\\]+/); - const referencingPackletName: string = referencingPathParts[0]; - - if (!visitedPacklets.has(packletName)) { - visitedPacklets.add(packletName); - - const link2: ILink = { - previous: previousLink, - fromFilePath: referencingFilePath, - packletName: packletName - }; - - if (referencingPackletName === startingPackletName) { - return link2; - } - - const result: ILink | undefined = loop( - referencingPackletName, - startingPackletName, - refFileMap, - program, - packletsFolderPath, - visitedPacklets, - link2, - context - ); - if (result) { - return result; - } - } - } - } - } - } - } - return undefined; -} - -const importPath: TSESLint.RuleModule = { +const mechanics: TSESLint.RuleModule = { meta: { type: 'problem', messages: { @@ -223,9 +34,7 @@ const importPath: TSESLint.RuleModule = { 'circular-entry-point': 'Files under a packlet folder must not import from their own index.ts file', 'packlet-importing-project-file': 'A local project file cannot be imported.' + - " A packlet's dependencies must be NPM packages and/or other packlets.", - 'circular-import': 'Packlet imports create a circular reference:\n{{report}}', - debug: 'debug {{debug}}' + " A packlet's dependencies must be NPM packages and/or other packlets." }, schema: [ { @@ -234,9 +43,7 @@ const importPath: TSESLint.RuleModule = { } ], docs: { - description: - 'Requires regular expressions to be constructed from string constants rather than dynamically' + - ' building strings at runtime.', + description: '', category: 'Best Practices', recommended: 'warn', url: 'https://www.npmjs.com/package/@rushstack/eslint-plugin-packlets' @@ -252,58 +59,16 @@ const importPath: TSESLint.RuleModule = { context ).program.getCompilerOptions()['configFilePath'] as string; - const result: IResult = f(inputFilePath, tsconfigFilePath); + const result: IResult = analyze(inputFilePath, tsconfigFilePath); if (result.skip) { return {}; } - if (result.globalError === undefined && !result.packletsEnabled) { - return {}; - } return { // Match the first node in the source file. Ideally we should be matching "Program > :first-child" // so a warning doesn't highlight the whole file. But that's blocked behind a bug in the query selector: // https://github.com/estools/esquery/issues/114 Program: (node: TSESTree.Node): void => { - if (result.isEntryPoint && !result.globalError) { - const visitedPacklets: Set = new Set(); - const program: ts.Program | undefined = context.parserServices?.program; - if (program) { - const refFileMap: Map = (program as any).getRefFileMap(); - - const resultLink: ILink | undefined = loop( - result.packletName!, - result.packletName!, - refFileMap, - program, - result.packletsFolderPath!, - visitedPacklets, - undefined, - context - ); - - if (resultLink) { - const tsconfigFileFolder: string = path.dirname(tsconfigFilePath); - - let report: string = ''; - const affectedPackletNames: string[] = []; - - for (let current: ILink | undefined = resultLink; current; current = current.previous) { - affectedPackletNames.push(current.packletName); - const filePath: string = path.relative(tsconfigFileFolder, current.fromFilePath); - report += `"${current.packletName}" is referenced by ${filePath}\n`; - } - - // If 3 different packlets form a circular dependency, we don't need to report the same warning 3 times. - // Instead, only report the warning for the alphabetically smallest packlet. - affectedPackletNames.sort(); - if (affectedPackletNames[0] === result.packletName) { - result.globalError = { messageId: 'circular-import', data: { report: report } }; - } - } - } - } - if (result.globalError) { context.report({ node: node, @@ -430,4 +195,4 @@ const importPath: TSESLint.RuleModule = { } }; -export { importPath }; +export { mechanics }; diff --git a/tutorials/packlets-tutorial/.eslintrc.js b/tutorials/packlets-tutorial/.eslintrc.js index 0eadcc77421..b69d9978ae3 100644 --- a/tutorials/packlets-tutorial/.eslintrc.js +++ b/tutorials/packlets-tutorial/.eslintrc.js @@ -11,7 +11,8 @@ module.exports = { files: ['*.ts', '*.tsx'], rules: { - '@rushstack/packlets/import-path': 'warn' + '@rushstack/packlets/mechanics': 'warn', + '@rushstack/packlets/circular-deps': 'warn' } } ] From 65970a2c3e58f6ea5b3cf9c8d914a03b0a6bb514 Mon Sep 17 00:00:00 2001 From: Pete Gonzalez <4673363+octogonz@users.noreply.github.com> Date: Sun, 4 Oct 2020 02:13:36 -0700 Subject: [PATCH 12/23] Move more logic into PackletImportAnalyzer.ts --- .../src/PackletImportAnalyzer.ts | 88 ++++++++++++++++- stack/eslint-plugin-packlets/src/mechanics.ts | 94 ++----------------- 2 files changed, 97 insertions(+), 85 deletions(-) diff --git a/stack/eslint-plugin-packlets/src/PackletImportAnalyzer.ts b/stack/eslint-plugin-packlets/src/PackletImportAnalyzer.ts index 638b98bc692..f84982e9e8f 100644 --- a/stack/eslint-plugin-packlets/src/PackletImportAnalyzer.ts +++ b/stack/eslint-plugin-packlets/src/PackletImportAnalyzer.ts @@ -12,12 +12,18 @@ export type MyMessageIds = | 'invalid-packlet-name' | 'misplaced-packlets-folder'; +export type MyMessageIds2 = + | 'bypassed-entry-point' + | 'circular-entry-point' + | 'packlet-importing-project-file'; + export interface ILintError { - messageId: MyMessageIds; + messageId: MyMessageIds | MyMessageIds2; data?: Readonly>; } export interface IResult { + inputFilePath: string; globalError: ILintError | undefined; skip: boolean; packletsEnabled: boolean; @@ -30,6 +36,7 @@ const validPackletName: RegExp = /^[a-z0-9]+(-[a-z0-9]+)*$/; export function analyze(inputFilePath: string, tsconfigFilePath: string | undefined): IResult { const result: IResult = { + inputFilePath, globalError: undefined, skip: false, packletsEnabled: false, @@ -121,3 +128,82 @@ export function analyze(inputFilePath: string, tsconfigFilePath: string | undefi return result; } + +export function analyze2(modulePath: string, result: IResult): ILintError | undefined { + if (!result.packletsFolderPath) { + // This should not happen + throw new Error('Missing packletsFolderPath'); + } + + // Example: /path/to/my-project/src/packlets/my-packlet + const inputFileFolder: string = path.dirname(result.inputFilePath); + + // Example: /path/to/my-project/src/other-packlet/index + const importedPath: string = path.resolve(inputFileFolder, modulePath); + + // Is the imported path referring to a file under the src/packlets folder? + if (Path.isUnder(importedPath, result.packletsFolderPath)) { + // Example: other-packlet/index + const importedPathRelativeToPackletsFolder: string = path.relative( + result.packletsFolderPath, + importedPath + ); + // Example: [ 'other-packlet', 'index' ] + const importedPathParts: string[] = importedPathRelativeToPackletsFolder.split(/[\/\\]+/); + if (importedPathParts.length > 0) { + // Example: 'other-packlet' + const importedPackletName: string = importedPathParts[0]; + + // We are importing from a packlet. Is the input file part of the same packlet? + if (result.packletName && importedPackletName === result.packletName) { + // Yes. Then our import must NOT use the packlet entry point. + + // Example: 'index' + // + // We discard the file extension to handle a degenerate case like: + // import { X } from "../index.js"; + const lastPart: string = path.parse(importedPathParts[importedPathParts.length - 1]).name; + let pathToCompare: string; + if (lastPart.toUpperCase() === 'INDEX') { + // Example: + // importedPath = /path/to/my-project/src/other-packlet/index + // pathToCompare = /path/to/my-project/src/other-packlet + pathToCompare = path.dirname(importedPath); + } else { + pathToCompare = importedPath; + } + + // Example: /path/to/my-project/src/other-packlet + const entryPointPath: string = path.join(result.packletsFolderPath, importedPackletName); + + if (Path.isEqual(pathToCompare, entryPointPath)) { + return { + messageId: 'circular-entry-point' + }; + } + } else { + // No. If we are not part of the same packlet, then the module path must refer + // to the index.ts entry point. + + // Example: /path/to/my-project/src/other-packlet + const entryPointPath: string = path.join(result.packletsFolderPath, importedPackletName); + + if (!Path.isEqual(importedPath, entryPointPath)) { + const entryPointModulePath: string = path.posix.relative(importedPackletName, inputFileFolder); + + return { + messageId: 'bypassed-entry-point', + data: { entryPointModulePath } + }; + } + } + } + } else { + // The imported path does NOT refer to a file under the src/packlets folder + if (result.packletName) { + return { + messageId: 'packlet-importing-project-file' + }; + } + } +} diff --git a/stack/eslint-plugin-packlets/src/mechanics.ts b/stack/eslint-plugin-packlets/src/mechanics.ts index 8271cd5d5f1..871107d96ff 100644 --- a/stack/eslint-plugin-packlets/src/mechanics.ts +++ b/stack/eslint-plugin-packlets/src/mechanics.ts @@ -5,15 +5,10 @@ import * as path from 'path'; import type { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils'; import { AST_NODE_TYPES, ESLintUtils } from '@typescript-eslint/experimental-utils'; -import { Path } from './Path'; -import { analyze, IResult, MyMessageIds } from './PackletImportAnalyzer'; +import { analyze, analyze2, ILintError, IResult, MyMessageIds, MyMessageIds2 } from './PackletImportAnalyzer'; -export type MessageIds = - | MyMessageIds - | 'bypassed-entry-point' - | 'circular-entry-point' - | 'packlet-importing-project-file'; +export type MessageIds = MyMessageIds | MyMessageIds2; type Options = []; const mechanics: TSESLint.RuleModule = { @@ -94,7 +89,7 @@ const mechanics: TSESLint.RuleModule = { node: TSESTree.ImportDeclaration | TSESTree.ExportNamedDeclaration | TSESTree.ExportAllDeclaration ): void => { if (node.source?.type === AST_NODE_TYPES.Literal) { - if (result.packletsEnabled && result.packletsFolderPath) { + if (result.packletsEnabled) { // Extract the import/export module path // Example: "../../packlets/other-packlet" const modulePath = node.source.value; @@ -111,82 +106,13 @@ const mechanics: TSESLint.RuleModule = { return; } - // Example: /path/to/my-project/src/packlets/my-packlet - const inputFileFolder: string = path.dirname(inputFilePath); - - // Example: /path/to/my-project/src/other-packlet/index - const importedPath: string = path.resolve(inputFileFolder, modulePath); - - // Is the imported path referring to a file under the src/packlets folder? - if (Path.isUnder(importedPath, result.packletsFolderPath)) { - // Example: other-packlet/index - const importedPathRelativeToPackletsFolder: string = path.relative( - result.packletsFolderPath, - importedPath - ); - // Example: [ 'other-packlet', 'index' ] - const importedPathParts: string[] = importedPathRelativeToPackletsFolder.split(/[\/\\]+/); - if (importedPathParts.length > 0) { - // Example: 'other-packlet' - const importedPackletName: string = importedPathParts[0]; - - // We are importing from a packlet. Is the input file part of the same packlet? - if (result.packletName && importedPackletName === result.packletName) { - // Yes. Then our import must NOT use the packlet entry point. - - // Example: 'index' - // - // We discard the file extension to handle a degenerate case like: - // import { X } from "../index.js"; - const lastPart: string = path.parse(importedPathParts[importedPathParts.length - 1]).name; - let pathToCompare: string; - if (lastPart.toUpperCase() === 'INDEX') { - // Example: - // importedPath = /path/to/my-project/src/other-packlet/index - // pathToCompare = /path/to/my-project/src/other-packlet - pathToCompare = path.dirname(importedPath); - } else { - pathToCompare = importedPath; - } - - // Example: /path/to/my-project/src/other-packlet - const entryPointPath: string = path.join(result.packletsFolderPath, importedPackletName); - - if (Path.isEqual(pathToCompare, entryPointPath)) { - context.report({ - node: node, - messageId: 'circular-entry-point' - }); - } - } else { - // No. If we are not part of the same packlet, then the module path must refer - // to the index.ts entry point. - - // Example: /path/to/my-project/src/other-packlet - const entryPointPath: string = path.join(result.packletsFolderPath, importedPackletName); - - if (!Path.isEqual(importedPath, entryPointPath)) { - const entryPointModulePath: string = path.posix.relative( - importedPackletName, - inputFileFolder - ); - - context.report({ - node: node, - messageId: 'bypassed-entry-point', - data: { entryPointModulePath } - }); - } - } - } - } else { - // The imported path does NOT refer to a file under the src/packlets folder - if (result.packletName) { - context.report({ - node: node, - messageId: 'packlet-importing-project-file' - }); - } + const lint: ILintError | undefined = analyze2(modulePath, result); + if (lint) { + context.report({ + node: node, + messageId: lint.messageId, + data: lint.data + }); } } } From 0fe8fef7310c78a438b7721626a356ae3b6eac2d Mon Sep 17 00:00:00 2001 From: Pete Gonzalez <4673363+octogonz@users.noreply.github.com> Date: Sun, 4 Oct 2020 02:21:01 -0700 Subject: [PATCH 13/23] Extract loop detection into DependencyAnalyzer.ts --- .../src/DependencyAnalyzer.ts | 100 ++++++++++++++++++ ...etImportAnalyzer.ts => PackletAnalyzer.ts} | 0 .../src/circular-deps.ts | 98 +---------------- 3 files changed, 103 insertions(+), 95 deletions(-) create mode 100644 stack/eslint-plugin-packlets/src/DependencyAnalyzer.ts rename stack/eslint-plugin-packlets/src/{PackletImportAnalyzer.ts => PackletAnalyzer.ts} (100%) diff --git a/stack/eslint-plugin-packlets/src/DependencyAnalyzer.ts b/stack/eslint-plugin-packlets/src/DependencyAnalyzer.ts new file mode 100644 index 00000000000..e4ae427f7de --- /dev/null +++ b/stack/eslint-plugin-packlets/src/DependencyAnalyzer.ts @@ -0,0 +1,100 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. +// See LICENSE in the project root for license information. + +import type * as ts from 'typescript'; +import * as path from 'path'; + +import { Path } from './Path'; +import { IResult } from './PackletAnalyzer'; + +export enum RefFileKind { + Import, + ReferenceFile, + TypeReferenceDirective +} + +interface RefFile { + referencedFileName: string; + kind: RefFileKind; + index: number; + file: string; +} + +export interface ILink { + previous: ILink | undefined; + packletName: string; + fromFilePath: string; +} + +export function loop( + packletName: string, + startingPackletName: string, + refFileMap: Map, + program: ts.Program, + packletsFolderPath: string, + visitedPacklets: Set, + previousLink: ILink | undefined +): ILink | undefined { + const packletEntryPoint: string = path.join(packletsFolderPath, packletName, 'index'); + + const tsSourceFile: ts.SourceFile | undefined = + program.getSourceFile(packletEntryPoint + '.ts') || program.getSourceFile(packletEntryPoint + '.tsx'); + if (tsSourceFile) { + const refFiles: RefFile[] | undefined = refFileMap.get((tsSourceFile as any).path as any); + if (refFiles) { + for (const refFile of refFiles) { + if (refFile.kind === RefFileKind.Import) { + const referencingFilePath: string = refFile.file; + + if (Path.isUnder(referencingFilePath, packletsFolderPath)) { + const referencingRelativePath: string = path.relative(packletsFolderPath, referencingFilePath); + const referencingPathParts: string[] = referencingRelativePath.split(/[\/\\]+/); + const referencingPackletName: string = referencingPathParts[0]; + + if (!visitedPacklets.has(packletName)) { + visitedPacklets.add(packletName); + + const link2: ILink = { + previous: previousLink, + fromFilePath: referencingFilePath, + packletName: packletName + }; + + if (referencingPackletName === startingPackletName) { + return link2; + } + + const result: ILink | undefined = loop( + referencingPackletName, + startingPackletName, + refFileMap, + program, + packletsFolderPath, + visitedPacklets, + link2 + ); + if (result) { + return result; + } + } + } + } + } + } + } + return undefined; +} + +export function loopp(result: IResult, program: ts.Program): ILink | undefined { + const refFileMap: Map = (program as any).getRefFileMap(); + const visitedPacklets: Set = new Set(); + return loop( + result.packletName!, + result.packletName!, + refFileMap, + program, + result.packletsFolderPath!, + visitedPacklets, + undefined + ); +} diff --git a/stack/eslint-plugin-packlets/src/PackletImportAnalyzer.ts b/stack/eslint-plugin-packlets/src/PackletAnalyzer.ts similarity index 100% rename from stack/eslint-plugin-packlets/src/PackletImportAnalyzer.ts rename to stack/eslint-plugin-packlets/src/PackletAnalyzer.ts diff --git a/stack/eslint-plugin-packlets/src/circular-deps.ts b/stack/eslint-plugin-packlets/src/circular-deps.ts index 067f89da445..077966aaf71 100644 --- a/stack/eslint-plugin-packlets/src/circular-deps.ts +++ b/stack/eslint-plugin-packlets/src/circular-deps.ts @@ -6,89 +6,9 @@ import * as path from 'path'; import type { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils'; import { ESLintUtils } from '@typescript-eslint/experimental-utils'; -import { Path } from './Path'; -import { analyze, IResult } from './PackletImportAnalyzer'; - -export enum RefFileKind { - Import, - ReferenceFile, - TypeReferenceDirective -} - -interface RefFile { - referencedFileName: string; - kind: RefFileKind; - index: number; - file: string; -} - -interface ILink { - previous: ILink | undefined; - packletName: string; - fromFilePath: string; -} - -function loop( - packletName: string, - startingPackletName: string, - refFileMap: Map, - program: ts.Program, - packletsFolderPath: string, - visitedPacklets: Set, - previousLink: ILink | undefined, - context: TSESLint.RuleContext -): ILink | undefined { - const packletEntryPoint: string = path.join(packletsFolderPath, packletName, 'index'); - - const tsSourceFile: ts.SourceFile | undefined = - program.getSourceFile(packletEntryPoint + '.ts') || program.getSourceFile(packletEntryPoint + '.tsx'); - if (tsSourceFile) { - const refFiles: RefFile[] | undefined = refFileMap.get((tsSourceFile as any).path as any); - if (refFiles) { - for (const refFile of refFiles) { - if (refFile.kind === RefFileKind.Import) { - const referencingFilePath: string = refFile.file; - - if (Path.isUnder(referencingFilePath, packletsFolderPath)) { - const referencingRelativePath: string = path.relative(packletsFolderPath, referencingFilePath); - const referencingPathParts: string[] = referencingRelativePath.split(/[\/\\]+/); - const referencingPackletName: string = referencingPathParts[0]; - - if (!visitedPacklets.has(packletName)) { - visitedPacklets.add(packletName); - - const link2: ILink = { - previous: previousLink, - fromFilePath: referencingFilePath, - packletName: packletName - }; - - if (referencingPackletName === startingPackletName) { - return link2; - } - - const result: ILink | undefined = loop( - referencingPackletName, - startingPackletName, - refFileMap, - program, - packletsFolderPath, - visitedPacklets, - link2, - context - ); - if (result) { - return result; - } - } - } - } - } - } - } - return undefined; -} +import { analyze, IResult } from './PackletAnalyzer'; +import { ILink, loopp } from './DependencyAnalyzer'; export type MessageIds = 'circular-import'; type Options = []; @@ -131,21 +51,9 @@ const circularDeps: TSESLint.RuleModule = { // https://github.com/estools/esquery/issues/114 Program: (node: TSESTree.Node): void => { if (result.isEntryPoint && !result.globalError) { - const visitedPacklets: Set = new Set(); const program: ts.Program | undefined = context.parserServices?.program; if (program) { - const refFileMap: Map = (program as any).getRefFileMap(); - - const resultLink: ILink | undefined = loop( - result.packletName!, - result.packletName!, - refFileMap, - program, - result.packletsFolderPath!, - visitedPacklets, - undefined, - context - ); + const resultLink: ILink | undefined = loopp(result, program); if (resultLink) { const tsconfigFileFolder: string = path.dirname(tsconfigFilePath); From df740c92e6084cd839b65ae269d61d056bb3b90a Mon Sep 17 00:00:00 2001 From: Pete Gonzalez <4673363+octogonz@users.noreply.github.com> Date: Sun, 4 Oct 2020 02:28:05 -0700 Subject: [PATCH 14/23] Encapsulate functions into classes --- .../src/DependencyAnalyzer.ts | 120 +++---- .../src/PackletAnalyzer.ts | 302 +++++++++--------- .../src/circular-deps.ts | 14 +- stack/eslint-plugin-packlets/src/mechanics.ts | 16 +- 4 files changed, 226 insertions(+), 226 deletions(-) diff --git a/stack/eslint-plugin-packlets/src/DependencyAnalyzer.ts b/stack/eslint-plugin-packlets/src/DependencyAnalyzer.ts index e4ae427f7de..3aa92423441 100644 --- a/stack/eslint-plugin-packlets/src/DependencyAnalyzer.ts +++ b/stack/eslint-plugin-packlets/src/DependencyAnalyzer.ts @@ -5,9 +5,9 @@ import type * as ts from 'typescript'; import * as path from 'path'; import { Path } from './Path'; -import { IResult } from './PackletAnalyzer'; +import { PacketAnalyzer } from './PackletAnalyzer'; -export enum RefFileKind { +enum RefFileKind { Import, ReferenceFile, TypeReferenceDirective @@ -26,75 +26,77 @@ export interface ILink { fromFilePath: string; } -export function loop( - packletName: string, - startingPackletName: string, - refFileMap: Map, - program: ts.Program, - packletsFolderPath: string, - visitedPacklets: Set, - previousLink: ILink | undefined -): ILink | undefined { - const packletEntryPoint: string = path.join(packletsFolderPath, packletName, 'index'); +export class DependencyAnalyzer { + private static _loop( + packletName: string, + startingPackletName: string, + refFileMap: Map, + program: ts.Program, + packletsFolderPath: string, + visitedPacklets: Set, + previousLink: ILink | undefined + ): ILink | undefined { + const packletEntryPoint: string = path.join(packletsFolderPath, packletName, 'index'); - const tsSourceFile: ts.SourceFile | undefined = - program.getSourceFile(packletEntryPoint + '.ts') || program.getSourceFile(packletEntryPoint + '.tsx'); - if (tsSourceFile) { - const refFiles: RefFile[] | undefined = refFileMap.get((tsSourceFile as any).path as any); - if (refFiles) { - for (const refFile of refFiles) { - if (refFile.kind === RefFileKind.Import) { - const referencingFilePath: string = refFile.file; + const tsSourceFile: ts.SourceFile | undefined = + program.getSourceFile(packletEntryPoint + '.ts') || program.getSourceFile(packletEntryPoint + '.tsx'); + if (tsSourceFile) { + const refFiles: RefFile[] | undefined = refFileMap.get((tsSourceFile as any).path as any); + if (refFiles) { + for (const refFile of refFiles) { + if (refFile.kind === RefFileKind.Import) { + const referencingFilePath: string = refFile.file; - if (Path.isUnder(referencingFilePath, packletsFolderPath)) { - const referencingRelativePath: string = path.relative(packletsFolderPath, referencingFilePath); - const referencingPathParts: string[] = referencingRelativePath.split(/[\/\\]+/); - const referencingPackletName: string = referencingPathParts[0]; + if (Path.isUnder(referencingFilePath, packletsFolderPath)) { + const referencingRelativePath: string = path.relative(packletsFolderPath, referencingFilePath); + const referencingPathParts: string[] = referencingRelativePath.split(/[\/\\]+/); + const referencingPackletName: string = referencingPathParts[0]; - if (!visitedPacklets.has(packletName)) { - visitedPacklets.add(packletName); + if (!visitedPacklets.has(packletName)) { + visitedPacklets.add(packletName); - const link2: ILink = { - previous: previousLink, - fromFilePath: referencingFilePath, - packletName: packletName - }; + const link2: ILink = { + previous: previousLink, + fromFilePath: referencingFilePath, + packletName: packletName + }; - if (referencingPackletName === startingPackletName) { - return link2; - } + if (referencingPackletName === startingPackletName) { + return link2; + } - const result: ILink | undefined = loop( - referencingPackletName, - startingPackletName, - refFileMap, - program, - packletsFolderPath, - visitedPacklets, - link2 - ); - if (result) { - return result; + const result: ILink | undefined = DependencyAnalyzer._loop( + referencingPackletName, + startingPackletName, + refFileMap, + program, + packletsFolderPath, + visitedPacklets, + link2 + ); + if (result) { + return result; + } } } } } } } + return undefined; } - return undefined; -} -export function loopp(result: IResult, program: ts.Program): ILink | undefined { - const refFileMap: Map = (program as any).getRefFileMap(); - const visitedPacklets: Set = new Set(); - return loop( - result.packletName!, - result.packletName!, - refFileMap, - program, - result.packletsFolderPath!, - visitedPacklets, - undefined - ); + public static loopp(packetAnalyzer: PacketAnalyzer, program: ts.Program): ILink | undefined { + const refFileMap: Map = (program as any).getRefFileMap(); + const visitedPacklets: Set = new Set(); + return DependencyAnalyzer._loop( + packetAnalyzer.packletName!, + packetAnalyzer.packletName!, + refFileMap, + program, + packetAnalyzer.packletsFolderPath!, + visitedPacklets, + undefined + ); + } } diff --git a/stack/eslint-plugin-packlets/src/PackletAnalyzer.ts b/stack/eslint-plugin-packlets/src/PackletAnalyzer.ts index f84982e9e8f..4c8c80fcbe3 100644 --- a/stack/eslint-plugin-packlets/src/PackletAnalyzer.ts +++ b/stack/eslint-plugin-packlets/src/PackletAnalyzer.ts @@ -22,188 +22,186 @@ export interface ILintError { data?: Readonly>; } -export interface IResult { - inputFilePath: string; - globalError: ILintError | undefined; - skip: boolean; - packletsEnabled: boolean; - packletsFolderPath: string | undefined; - packletName: string | undefined; - isEntryPoint: boolean; -} +export class PacketAnalyzer { + private static _validPackletName: RegExp = /^[a-z0-9]+(-[a-z0-9]+)*$/; + + public readonly inputFilePath: string; + public readonly globalError: ILintError | undefined; + public readonly skip: boolean; + public readonly packletsEnabled: boolean; + public readonly packletsFolderPath: string | undefined; + public readonly packletName: string | undefined; + public readonly isEntryPoint: boolean; + + public constructor(inputFilePath: string, tsconfigFilePath: string | undefined) { + this.inputFilePath = inputFilePath; + this.globalError = undefined; + this.skip = false; + this.packletsEnabled = false; + this.packletsFolderPath = undefined; + this.packletName = undefined; + this.isEntryPoint = false; + + // Example: /path/to/my-project/src + let srcFolderPath: string | undefined; + + if (!tsconfigFilePath) { + this.globalError = { messageId: 'missing-tsconfig' }; + return; + } -const validPackletName: RegExp = /^[a-z0-9]+(-[a-z0-9]+)*$/; - -export function analyze(inputFilePath: string, tsconfigFilePath: string | undefined): IResult { - const result: IResult = { - inputFilePath, - globalError: undefined, - skip: false, - packletsEnabled: false, - packletsFolderPath: undefined, - packletName: undefined, - isEntryPoint: false - }; - - // Example: /path/to/my-project/src - let srcFolderPath: string | undefined; - - if (!tsconfigFilePath) { - result.globalError = { messageId: 'missing-tsconfig' }; - return result; - } + srcFolderPath = path.join(path.dirname(tsconfigFilePath), 'src'); - srcFolderPath = path.join(path.dirname(tsconfigFilePath), 'src'); + if (!fs.existsSync(srcFolderPath)) { + this.globalError = { messageId: 'missing-src-folder', data: { srcFolderPath } }; + return; + } - if (!fs.existsSync(srcFolderPath)) { - result.globalError = { messageId: 'missing-src-folder', data: { srcFolderPath } }; - return result; - } + if (!Path.isUnder(inputFilePath, srcFolderPath)) { + // Ignore files outside the "src" folder + this.skip = true; + return; + } - if (!Path.isUnder(inputFilePath, srcFolderPath)) { - // Ignore files outside the "src" folder - result.skip = true; - return result; - } + // Example: packlets/my-packlet/index.ts + const inputFilePathRelativeToSrc: string = path.relative(srcFolderPath, inputFilePath); - // Example: packlets/my-packlet/index.ts - const inputFilePathRelativeToSrc: string = path.relative(srcFolderPath, inputFilePath); + // Example: [ 'packlets', 'my-packlet', 'index.ts' ] + const pathParts: string[] = inputFilePathRelativeToSrc.split(/[\/\\]+/); - // Example: [ 'packlets', 'my-packlet', 'index.ts' ] - const pathParts: string[] = inputFilePathRelativeToSrc.split(/[\/\\]+/); + let underPackletsFolder: boolean = false; - let underPackletsFolder: boolean = false; + const expectedPackletsFolder: string = path.join(srcFolderPath, 'packlets'); - const expectedPackletsFolder: string = path.join(srcFolderPath, 'packlets'); + for (let i = 0; i < pathParts.length; ++i) { + const pathPart: string = pathParts[i]; + if (pathPart.toUpperCase() === 'PACKLETS') { + if (pathPart !== 'packlets') { + // Example: /path/to/my-project/src/PACKLETS + const packletsFolderPath: string = path.join(srcFolderPath, ...pathParts.slice(0, i + 1)); + this.globalError = { messageId: 'packlet-folder-case', data: { packletsFolderPath } }; + return; + } - for (let i = 0; i < pathParts.length; ++i) { - const pathPart: string = pathParts[i]; - if (pathPart.toUpperCase() === 'PACKLETS') { - if (pathPart !== 'packlets') { - // Example: /path/to/my-project/src/PACKLETS - const packletsFolderPath: string = path.join(srcFolderPath, ...pathParts.slice(0, i + 1)); - result.globalError = { messageId: 'packlet-folder-case', data: { packletsFolderPath } }; - return result; - } + if (i !== 0) { + this.globalError = { messageId: 'misplaced-packlets-folder', data: { expectedPackletsFolder } }; + return; + } - if (i !== 0) { - result.globalError = { messageId: 'misplaced-packlets-folder', data: { expectedPackletsFolder } }; - return result; + underPackletsFolder = true; } + } - underPackletsFolder = true; + if (underPackletsFolder || fs.existsSync(expectedPackletsFolder)) { + // packletsAbsolutePath + this.packletsEnabled = true; + this.packletsFolderPath = expectedPackletsFolder; } - } - if (underPackletsFolder || fs.existsSync(expectedPackletsFolder)) { - // packletsAbsolutePath - result.packletsEnabled = true; - result.packletsFolderPath = expectedPackletsFolder; - } + if (underPackletsFolder && pathParts.length >= 2) { + // Example: 'my-packlet' + const packletName: string = pathParts[1]; + this.packletName = packletName; - if (underPackletsFolder && pathParts.length >= 2) { - // Example: 'my-packlet' - const packletName: string = pathParts[1]; - result.packletName = packletName; + // Example: 'index.ts' or 'index.tsx' + const thirdPart: string = pathParts[2]; - // Example: 'index.ts' or 'index.tsx' - const thirdPart: string = pathParts[2]; + // Example: 'index' + const thirdPartWithoutExtension: string = path.parse(thirdPart).name; - // Example: 'index' - const thirdPartWithoutExtension: string = path.parse(thirdPart).name; + if (thirdPartWithoutExtension.toUpperCase() === 'INDEX') { + if (!PacketAnalyzer._validPackletName.test(packletName)) { + this.globalError = { messageId: 'invalid-packlet-name', data: { packletName } }; + return; + } - if (thirdPartWithoutExtension.toUpperCase() === 'INDEX') { - if (!validPackletName.test(packletName)) { - result.globalError = { messageId: 'invalid-packlet-name', data: { packletName } }; - return result; + this.isEntryPoint = true; } - - result.isEntryPoint = true; } - } - if (result.globalError === undefined && !result.packletsEnabled) { - result.skip = true; + if (this.globalError === undefined && !this.packletsEnabled) { + this.skip = true; + } } - return result; -} - -export function analyze2(modulePath: string, result: IResult): ILintError | undefined { - if (!result.packletsFolderPath) { - // This should not happen - throw new Error('Missing packletsFolderPath'); - } + public analyze2(modulePath: string): ILintError | undefined { + if (!this.packletsFolderPath) { + // This should not happen + throw new Error('Missing packletsFolderPath'); + } - // Example: /path/to/my-project/src/packlets/my-packlet - const inputFileFolder: string = path.dirname(result.inputFilePath); - - // Example: /path/to/my-project/src/other-packlet/index - const importedPath: string = path.resolve(inputFileFolder, modulePath); - - // Is the imported path referring to a file under the src/packlets folder? - if (Path.isUnder(importedPath, result.packletsFolderPath)) { - // Example: other-packlet/index - const importedPathRelativeToPackletsFolder: string = path.relative( - result.packletsFolderPath, - importedPath - ); - // Example: [ 'other-packlet', 'index' ] - const importedPathParts: string[] = importedPathRelativeToPackletsFolder.split(/[\/\\]+/); - if (importedPathParts.length > 0) { - // Example: 'other-packlet' - const importedPackletName: string = importedPathParts[0]; - - // We are importing from a packlet. Is the input file part of the same packlet? - if (result.packletName && importedPackletName === result.packletName) { - // Yes. Then our import must NOT use the packlet entry point. - - // Example: 'index' - // - // We discard the file extension to handle a degenerate case like: - // import { X } from "../index.js"; - const lastPart: string = path.parse(importedPathParts[importedPathParts.length - 1]).name; - let pathToCompare: string; - if (lastPart.toUpperCase() === 'INDEX') { - // Example: - // importedPath = /path/to/my-project/src/other-packlet/index - // pathToCompare = /path/to/my-project/src/other-packlet - pathToCompare = path.dirname(importedPath); + // Example: /path/to/my-project/src/packlets/my-packlet + const inputFileFolder: string = path.dirname(this.inputFilePath); + + // Example: /path/to/my-project/src/other-packlet/index + const importedPath: string = path.resolve(inputFileFolder, modulePath); + + // Is the imported path referring to a file under the src/packlets folder? + if (Path.isUnder(importedPath, this.packletsFolderPath)) { + // Example: other-packlet/index + const importedPathRelativeToPackletsFolder: string = path.relative( + this.packletsFolderPath, + importedPath + ); + // Example: [ 'other-packlet', 'index' ] + const importedPathParts: string[] = importedPathRelativeToPackletsFolder.split(/[\/\\]+/); + if (importedPathParts.length > 0) { + // Example: 'other-packlet' + const importedPackletName: string = importedPathParts[0]; + + // We are importing from a packlet. Is the input file part of the same packlet? + if (this.packletName && importedPackletName === this.packletName) { + // Yes. Then our import must NOT use the packlet entry point. + + // Example: 'index' + // + // We discard the file extension to handle a degenerate case like: + // import { X } from "../index.js"; + const lastPart: string = path.parse(importedPathParts[importedPathParts.length - 1]).name; + let pathToCompare: string; + if (lastPart.toUpperCase() === 'INDEX') { + // Example: + // importedPath = /path/to/my-project/src/other-packlet/index + // pathToCompare = /path/to/my-project/src/other-packlet + pathToCompare = path.dirname(importedPath); + } else { + pathToCompare = importedPath; + } + + // Example: /path/to/my-project/src/other-packlet + const entryPointPath: string = path.join(this.packletsFolderPath, importedPackletName); + + if (Path.isEqual(pathToCompare, entryPointPath)) { + return { + messageId: 'circular-entry-point' + }; + } } else { - pathToCompare = importedPath; - } - - // Example: /path/to/my-project/src/other-packlet - const entryPointPath: string = path.join(result.packletsFolderPath, importedPackletName); - - if (Path.isEqual(pathToCompare, entryPointPath)) { - return { - messageId: 'circular-entry-point' - }; - } - } else { - // No. If we are not part of the same packlet, then the module path must refer - // to the index.ts entry point. + // No. If we are not part of the same packlet, then the module path must refer + // to the index.ts entry point. - // Example: /path/to/my-project/src/other-packlet - const entryPointPath: string = path.join(result.packletsFolderPath, importedPackletName); + // Example: /path/to/my-project/src/other-packlet + const entryPointPath: string = path.join(this.packletsFolderPath, importedPackletName); - if (!Path.isEqual(importedPath, entryPointPath)) { - const entryPointModulePath: string = path.posix.relative(importedPackletName, inputFileFolder); + if (!Path.isEqual(importedPath, entryPointPath)) { + const entryPointModulePath: string = path.posix.relative(importedPackletName, inputFileFolder); - return { - messageId: 'bypassed-entry-point', - data: { entryPointModulePath } - }; + return { + messageId: 'bypassed-entry-point', + data: { entryPointModulePath } + }; + } } } + } else { + // The imported path does NOT refer to a file under the src/packlets folder + if (this.packletName) { + return { + messageId: 'packlet-importing-project-file' + }; + } } - } else { - // The imported path does NOT refer to a file under the src/packlets folder - if (result.packletName) { - return { - messageId: 'packlet-importing-project-file' - }; - } + + return undefined; } } diff --git a/stack/eslint-plugin-packlets/src/circular-deps.ts b/stack/eslint-plugin-packlets/src/circular-deps.ts index 077966aaf71..eb3ca353739 100644 --- a/stack/eslint-plugin-packlets/src/circular-deps.ts +++ b/stack/eslint-plugin-packlets/src/circular-deps.ts @@ -7,8 +7,8 @@ import * as path from 'path'; import type { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils'; import { ESLintUtils } from '@typescript-eslint/experimental-utils'; -import { analyze, IResult } from './PackletAnalyzer'; -import { ILink, loopp } from './DependencyAnalyzer'; +import { PacketAnalyzer } from './PackletAnalyzer'; +import { DependencyAnalyzer, ILink } from './DependencyAnalyzer'; export type MessageIds = 'circular-import'; type Options = []; @@ -40,8 +40,8 @@ const circularDeps: TSESLint.RuleModule = { context ).program.getCompilerOptions()['configFilePath'] as string; - const result: IResult = analyze(inputFilePath, tsconfigFilePath); - if (result.skip) { + const packletAnalyzer: PacketAnalyzer = new PacketAnalyzer(inputFilePath, tsconfigFilePath); + if (packletAnalyzer.skip) { return {}; } @@ -50,10 +50,10 @@ const circularDeps: TSESLint.RuleModule = { // so a warning doesn't highlight the whole file. But that's blocked behind a bug in the query selector: // https://github.com/estools/esquery/issues/114 Program: (node: TSESTree.Node): void => { - if (result.isEntryPoint && !result.globalError) { + if (packletAnalyzer.isEntryPoint && !packletAnalyzer.globalError) { const program: ts.Program | undefined = context.parserServices?.program; if (program) { - const resultLink: ILink | undefined = loopp(result, program); + const resultLink: ILink | undefined = DependencyAnalyzer.loopp(packletAnalyzer, program); if (resultLink) { const tsconfigFileFolder: string = path.dirname(tsconfigFilePath); @@ -70,7 +70,7 @@ const circularDeps: TSESLint.RuleModule = { // If 3 different packlets form a circular dependency, we don't need to report the same warning 3 times. // Instead, only report the warning for the alphabetically smallest packlet. affectedPackletNames.sort(); - if (affectedPackletNames[0] === result.packletName) { + if (affectedPackletNames[0] === packletAnalyzer.packletName) { context.report({ node: node, messageId: 'circular-import', diff --git a/stack/eslint-plugin-packlets/src/mechanics.ts b/stack/eslint-plugin-packlets/src/mechanics.ts index 871107d96ff..3e14f571c0a 100644 --- a/stack/eslint-plugin-packlets/src/mechanics.ts +++ b/stack/eslint-plugin-packlets/src/mechanics.ts @@ -6,7 +6,7 @@ import * as path from 'path'; import type { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils'; import { AST_NODE_TYPES, ESLintUtils } from '@typescript-eslint/experimental-utils'; -import { analyze, analyze2, ILintError, IResult, MyMessageIds, MyMessageIds2 } from './PackletImportAnalyzer'; +import { PacketAnalyzer, ILintError, MyMessageIds, MyMessageIds2 } from './PackletAnalyzer'; export type MessageIds = MyMessageIds | MyMessageIds2; type Options = []; @@ -54,8 +54,8 @@ const mechanics: TSESLint.RuleModule = { context ).program.getCompilerOptions()['configFilePath'] as string; - const result: IResult = analyze(inputFilePath, tsconfigFilePath); - if (result.skip) { + const packletAnalyzer: PacketAnalyzer = new PacketAnalyzer(inputFilePath, tsconfigFilePath); + if (packletAnalyzer.skip) { return {}; } @@ -64,11 +64,11 @@ const mechanics: TSESLint.RuleModule = { // so a warning doesn't highlight the whole file. But that's blocked behind a bug in the query selector: // https://github.com/estools/esquery/issues/114 Program: (node: TSESTree.Node): void => { - if (result.globalError) { + if (packletAnalyzer.globalError) { context.report({ node: node, - messageId: result.globalError.messageId, - data: result.globalError.data + messageId: packletAnalyzer.globalError.messageId, + data: packletAnalyzer.globalError.data }); } }, @@ -89,7 +89,7 @@ const mechanics: TSESLint.RuleModule = { node: TSESTree.ImportDeclaration | TSESTree.ExportNamedDeclaration | TSESTree.ExportAllDeclaration ): void => { if (node.source?.type === AST_NODE_TYPES.Literal) { - if (result.packletsEnabled) { + if (packletAnalyzer.packletsEnabled) { // Extract the import/export module path // Example: "../../packlets/other-packlet" const modulePath = node.source.value; @@ -106,7 +106,7 @@ const mechanics: TSESLint.RuleModule = { return; } - const lint: ILintError | undefined = analyze2(modulePath, result); + const lint: ILintError | undefined = packletAnalyzer.analyze2(modulePath); if (lint) { context.report({ node: node, From a293f0e44c0fc479615d46c1b0307595cfc8f342 Mon Sep 17 00:00:00 2001 From: Pete Gonzalez <4673363+octogonz@users.noreply.github.com> Date: Sun, 4 Oct 2020 02:52:40 -0700 Subject: [PATCH 15/23] Clean up identifier names --- .../src/DependencyAnalyzer.ts | 136 +++++++++++------- .../src/PackletAnalyzer.ts | 79 ++++++---- stack/eslint-plugin-packlets/src/Path.ts | 1 + .../src/circular-deps.ts | 30 ++-- stack/eslint-plugin-packlets/src/mechanics.ts | 16 +-- 5 files changed, 167 insertions(+), 95 deletions(-) diff --git a/stack/eslint-plugin-packlets/src/DependencyAnalyzer.ts b/stack/eslint-plugin-packlets/src/DependencyAnalyzer.ts index 3aa92423441..71c742d63e2 100644 --- a/stack/eslint-plugin-packlets/src/DependencyAnalyzer.ts +++ b/stack/eslint-plugin-packlets/src/DependencyAnalyzer.ts @@ -13,6 +13,8 @@ enum RefFileKind { TypeReferenceDirective } +// TypeScript compiler internal: +// https://github.com/microsoft/TypeScript/blob/5ecdcef4cecfcdc86bd681b377636422447507d7/src/compiler/program.ts#L541 interface RefFile { referencedFileName: string; kind: RefFileKind; @@ -20,83 +22,121 @@ interface RefFile { file: string; } -export interface ILink { - previous: ILink | undefined; +/** + * Represents a packlet that imports another packlet. + */ +export interface IPackletImport { + /** + * The name of the packlet being imported. + */ packletName: string; + + /** + * The absolute path of the file that imports the packlet. + */ fromFilePath: string; } +/** + * Used to build a linked list of imports that represent a circular dependency. + */ +interface IImportListNode extends IPackletImport { + /** + * The previous link in the linked list. + */ + previousNode: IImportListNode | undefined; +} + export class DependencyAnalyzer { - private static _loop( + private static walkImports( packletName: string, startingPackletName: string, refFileMap: Map, program: ts.Program, packletsFolderPath: string, visitedPacklets: Set, - previousLink: ILink | undefined - ): ILink | undefined { + previousNode: IImportListNode | undefined + ): IImportListNode | undefined { const packletEntryPoint: string = path.join(packletsFolderPath, packletName, 'index'); const tsSourceFile: ts.SourceFile | undefined = program.getSourceFile(packletEntryPoint + '.ts') || program.getSourceFile(packletEntryPoint + '.tsx'); - if (tsSourceFile) { - const refFiles: RefFile[] | undefined = refFileMap.get((tsSourceFile as any).path as any); - if (refFiles) { - for (const refFile of refFiles) { - if (refFile.kind === RefFileKind.Import) { - const referencingFilePath: string = refFile.file; - - if (Path.isUnder(referencingFilePath, packletsFolderPath)) { - const referencingRelativePath: string = path.relative(packletsFolderPath, referencingFilePath); - const referencingPathParts: string[] = referencingRelativePath.split(/[\/\\]+/); - const referencingPackletName: string = referencingPathParts[0]; - - if (!visitedPacklets.has(packletName)) { - visitedPacklets.add(packletName); - - const link2: ILink = { - previous: previousLink, - fromFilePath: referencingFilePath, - packletName: packletName - }; - - if (referencingPackletName === startingPackletName) { - return link2; - } - - const result: ILink | undefined = DependencyAnalyzer._loop( - referencingPackletName, - startingPackletName, - refFileMap, - program, - packletsFolderPath, - visitedPacklets, - link2 - ); - if (result) { - return result; - } - } + if (!tsSourceFile) { + return undefined; + } + + const refFiles: RefFile[] | undefined = refFileMap.get((tsSourceFile as any).path as any); + if (!refFiles) { + return undefined; + } + + for (const refFile of refFiles) { + if (refFile.kind === RefFileKind.Import) { + const referencingFilePath: string = refFile.file; + + if (Path.isUnder(referencingFilePath, packletsFolderPath)) { + const referencingRelativePath: string = path.relative(packletsFolderPath, referencingFilePath); + const referencingPathParts: string[] = referencingRelativePath.split(/[\/\\]+/); + const referencingPackletName: string = referencingPathParts[0]; + + if (!visitedPacklets.has(packletName)) { + visitedPacklets.add(packletName); + + const importListNode: IImportListNode = { + previousNode: previousNode, + fromFilePath: referencingFilePath, + packletName: packletName + }; + + if (referencingPackletName === startingPackletName) { + return importListNode; + } + + const result: IImportListNode | undefined = DependencyAnalyzer.walkImports( + referencingPackletName, + startingPackletName, + refFileMap, + program, + packletsFolderPath, + visitedPacklets, + importListNode + ); + if (result) { + return result; } } } } } + return undefined; } - public static loopp(packetAnalyzer: PacketAnalyzer, program: ts.Program): ILink | undefined { + public static detectCircularImport( + packetAnalyzer: PacketAnalyzer, + program: ts.Program + ): IPackletImport[] | undefined { const refFileMap: Map = (program as any).getRefFileMap(); const visitedPacklets: Set = new Set(); - return DependencyAnalyzer._loop( - packetAnalyzer.packletName!, - packetAnalyzer.packletName!, + + const listNode: IImportListNode | undefined = DependencyAnalyzer.walkImports( + packetAnalyzer.inputFilePackletName!, + packetAnalyzer.inputFilePackletName!, refFileMap, program, packetAnalyzer.packletsFolderPath!, visitedPacklets, - undefined + undefined // previousNode ); + + if (listNode) { + const packletImports: IPackletImport[] = []; + for (let current: IImportListNode | undefined = listNode; current; current = current.previousNode) { + packletImports.push({ fromFilePath: current.fromFilePath, packletName: current.packletName }); + } + return packletImports; + } + + return undefined; } } diff --git a/stack/eslint-plugin-packlets/src/PackletAnalyzer.ts b/stack/eslint-plugin-packlets/src/PackletAnalyzer.ts index 4c8c80fcbe3..50f4dbd1dc1 100644 --- a/stack/eslint-plugin-packlets/src/PackletAnalyzer.ts +++ b/stack/eslint-plugin-packlets/src/PackletAnalyzer.ts @@ -17,7 +17,7 @@ export type MyMessageIds2 = | 'circular-entry-point' | 'packlet-importing-project-file'; -export interface ILintError { +export interface IAnalyzerError { messageId: MyMessageIds | MyMessageIds2; data?: Readonly>; } @@ -25,41 +25,72 @@ export interface ILintError { export class PacketAnalyzer { private static _validPackletName: RegExp = /^[a-z0-9]+(-[a-z0-9]+)*$/; + /** + * The input file being linted. + * + * Example: "/path/to/my-project/src/file.ts" + */ public readonly inputFilePath: string; - public readonly globalError: ILintError | undefined; - public readonly skip: boolean; - public readonly packletsEnabled: boolean; + + /** + * An error that occurred while analyzing the inputFilePath. + */ + public readonly error: IAnalyzerError | undefined; + + /** + * Returned to indicate that the linter can ignore this file. Possible reasons: + * - It's outside the "src" folder + * - The project doesn't define any packlets + */ + public readonly nothingToDo: boolean; + + /** + * If true, then the "src/packlets" folder exists. + */ + public readonly projectUsesPacklets: boolean; + + /** + * The absolute path of the "src/packlets" folder. + */ public readonly packletsFolderPath: string | undefined; - public readonly packletName: string | undefined; + + /** + * The packlet that the inputFilePath is under, if any. + */ + public readonly inputFilePackletName: string | undefined; + + /** + * Returns true if inputFilePath belongs to a packlet and is the entry point index.ts. + */ public readonly isEntryPoint: boolean; public constructor(inputFilePath: string, tsconfigFilePath: string | undefined) { this.inputFilePath = inputFilePath; - this.globalError = undefined; - this.skip = false; - this.packletsEnabled = false; + this.error = undefined; + this.nothingToDo = false; + this.projectUsesPacklets = false; this.packletsFolderPath = undefined; - this.packletName = undefined; + this.inputFilePackletName = undefined; this.isEntryPoint = false; // Example: /path/to/my-project/src let srcFolderPath: string | undefined; if (!tsconfigFilePath) { - this.globalError = { messageId: 'missing-tsconfig' }; + this.error = { messageId: 'missing-tsconfig' }; return; } srcFolderPath = path.join(path.dirname(tsconfigFilePath), 'src'); if (!fs.existsSync(srcFolderPath)) { - this.globalError = { messageId: 'missing-src-folder', data: { srcFolderPath } }; + this.error = { messageId: 'missing-src-folder', data: { srcFolderPath } }; return; } if (!Path.isUnder(inputFilePath, srcFolderPath)) { // Ignore files outside the "src" folder - this.skip = true; + this.nothingToDo = true; return; } @@ -79,12 +110,12 @@ export class PacketAnalyzer { if (pathPart !== 'packlets') { // Example: /path/to/my-project/src/PACKLETS const packletsFolderPath: string = path.join(srcFolderPath, ...pathParts.slice(0, i + 1)); - this.globalError = { messageId: 'packlet-folder-case', data: { packletsFolderPath } }; + this.error = { messageId: 'packlet-folder-case', data: { packletsFolderPath } }; return; } if (i !== 0) { - this.globalError = { messageId: 'misplaced-packlets-folder', data: { expectedPackletsFolder } }; + this.error = { messageId: 'misplaced-packlets-folder', data: { expectedPackletsFolder } }; return; } @@ -94,14 +125,14 @@ export class PacketAnalyzer { if (underPackletsFolder || fs.existsSync(expectedPackletsFolder)) { // packletsAbsolutePath - this.packletsEnabled = true; + this.projectUsesPacklets = true; this.packletsFolderPath = expectedPackletsFolder; } if (underPackletsFolder && pathParts.length >= 2) { // Example: 'my-packlet' const packletName: string = pathParts[1]; - this.packletName = packletName; + this.inputFilePackletName = packletName; // Example: 'index.ts' or 'index.tsx' const thirdPart: string = pathParts[2]; @@ -111,7 +142,7 @@ export class PacketAnalyzer { if (thirdPartWithoutExtension.toUpperCase() === 'INDEX') { if (!PacketAnalyzer._validPackletName.test(packletName)) { - this.globalError = { messageId: 'invalid-packlet-name', data: { packletName } }; + this.error = { messageId: 'invalid-packlet-name', data: { packletName } }; return; } @@ -119,15 +150,15 @@ export class PacketAnalyzer { } } - if (this.globalError === undefined && !this.packletsEnabled) { - this.skip = true; + if (this.error === undefined && !this.projectUsesPacklets) { + this.nothingToDo = true; } } - public analyze2(modulePath: string): ILintError | undefined { + public analyzeImport(modulePath: string): IAnalyzerError | undefined { if (!this.packletsFolderPath) { - // This should not happen - throw new Error('Missing packletsFolderPath'); + // The caller should ensure this can never happen + throw new Error('Internal error: packletsFolderPath is not defined'); } // Example: /path/to/my-project/src/packlets/my-packlet @@ -150,7 +181,7 @@ export class PacketAnalyzer { const importedPackletName: string = importedPathParts[0]; // We are importing from a packlet. Is the input file part of the same packlet? - if (this.packletName && importedPackletName === this.packletName) { + if (this.inputFilePackletName && importedPackletName === this.inputFilePackletName) { // Yes. Then our import must NOT use the packlet entry point. // Example: 'index' @@ -195,7 +226,7 @@ export class PacketAnalyzer { } } else { // The imported path does NOT refer to a file under the src/packlets folder - if (this.packletName) { + if (this.inputFilePackletName) { return { messageId: 'packlet-importing-project-file' }; diff --git a/stack/eslint-plugin-packlets/src/Path.ts b/stack/eslint-plugin-packlets/src/Path.ts index 9a576f0fa20..4dfafd0473b 100644 --- a/stack/eslint-plugin-packlets/src/Path.ts +++ b/stack/eslint-plugin-packlets/src/Path.ts @@ -3,6 +3,7 @@ import * as path from 'path'; +// These helpers are borrowed from @rushstack/node-core-library export class Path { private static _relativePathRegex: RegExp = /^[.\/\\]+$/; diff --git a/stack/eslint-plugin-packlets/src/circular-deps.ts b/stack/eslint-plugin-packlets/src/circular-deps.ts index eb3ca353739..2c4258a65e7 100644 --- a/stack/eslint-plugin-packlets/src/circular-deps.ts +++ b/stack/eslint-plugin-packlets/src/circular-deps.ts @@ -8,7 +8,7 @@ import type { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils'; import { ESLintUtils } from '@typescript-eslint/experimental-utils'; import { PacketAnalyzer } from './PackletAnalyzer'; -import { DependencyAnalyzer, ILink } from './DependencyAnalyzer'; +import { DependencyAnalyzer, IPackletImport } from './DependencyAnalyzer'; export type MessageIds = 'circular-import'; type Options = []; @@ -41,7 +41,7 @@ const circularDeps: TSESLint.RuleModule = { ).program.getCompilerOptions()['configFilePath'] as string; const packletAnalyzer: PacketAnalyzer = new PacketAnalyzer(inputFilePath, tsconfigFilePath); - if (packletAnalyzer.skip) { + if (packletAnalyzer.nothingToDo) { return {}; } @@ -50,27 +50,29 @@ const circularDeps: TSESLint.RuleModule = { // so a warning doesn't highlight the whole file. But that's blocked behind a bug in the query selector: // https://github.com/estools/esquery/issues/114 Program: (node: TSESTree.Node): void => { - if (packletAnalyzer.isEntryPoint && !packletAnalyzer.globalError) { + if (packletAnalyzer.isEntryPoint && !packletAnalyzer.error) { const program: ts.Program | undefined = context.parserServices?.program; if (program) { - const resultLink: ILink | undefined = DependencyAnalyzer.loopp(packletAnalyzer, program); + const packletImports: IPackletImport[] | undefined = DependencyAnalyzer.detectCircularImport( + packletAnalyzer, + program + ); - if (resultLink) { + if (packletImports) { const tsconfigFileFolder: string = path.dirname(tsconfigFilePath); - let report: string = ''; - const affectedPackletNames: string[] = []; - - for (let current: ILink | undefined = resultLink; current; current = current.previous) { - affectedPackletNames.push(current.packletName); - const filePath: string = path.relative(tsconfigFileFolder, current.fromFilePath); - report += `"${current.packletName}" is referenced by ${filePath}\n`; - } + const affectedPackletNames: string[] = packletImports.map((x) => x.packletName); // If 3 different packlets form a circular dependency, we don't need to report the same warning 3 times. // Instead, only report the warning for the alphabetically smallest packlet. affectedPackletNames.sort(); - if (affectedPackletNames[0] === packletAnalyzer.packletName) { + if (affectedPackletNames[0] === packletAnalyzer.inputFilePackletName) { + let report: string = ''; + for (const packletImport of packletImports) { + const filePath: string = path.relative(tsconfigFileFolder, packletImport.fromFilePath); + report += `"${packletImport.packletName}" is referenced by ${filePath}\n`; + } + context.report({ node: node, messageId: 'circular-import', diff --git a/stack/eslint-plugin-packlets/src/mechanics.ts b/stack/eslint-plugin-packlets/src/mechanics.ts index 3e14f571c0a..959bfd40468 100644 --- a/stack/eslint-plugin-packlets/src/mechanics.ts +++ b/stack/eslint-plugin-packlets/src/mechanics.ts @@ -1,12 +1,10 @@ // Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. // See LICENSE in the project root for license information. -import * as path from 'path'; - import type { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils'; import { AST_NODE_TYPES, ESLintUtils } from '@typescript-eslint/experimental-utils'; -import { PacketAnalyzer, ILintError, MyMessageIds, MyMessageIds2 } from './PackletAnalyzer'; +import { PacketAnalyzer, IAnalyzerError, MyMessageIds, MyMessageIds2 } from './PackletAnalyzer'; export type MessageIds = MyMessageIds | MyMessageIds2; type Options = []; @@ -55,7 +53,7 @@ const mechanics: TSESLint.RuleModule = { ).program.getCompilerOptions()['configFilePath'] as string; const packletAnalyzer: PacketAnalyzer = new PacketAnalyzer(inputFilePath, tsconfigFilePath); - if (packletAnalyzer.skip) { + if (packletAnalyzer.nothingToDo) { return {}; } @@ -64,11 +62,11 @@ const mechanics: TSESLint.RuleModule = { // so a warning doesn't highlight the whole file. But that's blocked behind a bug in the query selector: // https://github.com/estools/esquery/issues/114 Program: (node: TSESTree.Node): void => { - if (packletAnalyzer.globalError) { + if (packletAnalyzer.error) { context.report({ node: node, - messageId: packletAnalyzer.globalError.messageId, - data: packletAnalyzer.globalError.data + messageId: packletAnalyzer.error.messageId, + data: packletAnalyzer.error.data }); } }, @@ -89,7 +87,7 @@ const mechanics: TSESLint.RuleModule = { node: TSESTree.ImportDeclaration | TSESTree.ExportNamedDeclaration | TSESTree.ExportAllDeclaration ): void => { if (node.source?.type === AST_NODE_TYPES.Literal) { - if (packletAnalyzer.packletsEnabled) { + if (packletAnalyzer.projectUsesPacklets) { // Extract the import/export module path // Example: "../../packlets/other-packlet" const modulePath = node.source.value; @@ -106,7 +104,7 @@ const mechanics: TSESLint.RuleModule = { return; } - const lint: ILintError | undefined = packletAnalyzer.analyze2(modulePath); + const lint: IAnalyzerError | undefined = packletAnalyzer.analyzeImport(modulePath); if (lint) { context.report({ node: node, From 36c7e69c1ef0d7158bd3e9f743bd14997d7bc9e4 Mon Sep 17 00:00:00 2001 From: Pete Gonzalez <4673363+octogonz@users.noreply.github.com> Date: Sun, 4 Oct 2020 11:50:14 -0700 Subject: [PATCH 16/23] Fix incorrectly calculated module path --- stack/eslint-plugin-packlets/src/PackletAnalyzer.ts | 5 ++++- stack/eslint-plugin-packlets/src/Path.ts | 10 ++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/stack/eslint-plugin-packlets/src/PackletAnalyzer.ts b/stack/eslint-plugin-packlets/src/PackletAnalyzer.ts index 50f4dbd1dc1..767c21e05bc 100644 --- a/stack/eslint-plugin-packlets/src/PackletAnalyzer.ts +++ b/stack/eslint-plugin-packlets/src/PackletAnalyzer.ts @@ -215,7 +215,10 @@ export class PacketAnalyzer { const entryPointPath: string = path.join(this.packletsFolderPath, importedPackletName); if (!Path.isEqual(importedPath, entryPointPath)) { - const entryPointModulePath: string = path.posix.relative(importedPackletName, inputFileFolder); + // Example: "../packlets/other-packlet" + const entryPointModulePath: string = Path.convertToSlashes( + path.relative(inputFileFolder, entryPointPath) + ); return { messageId: 'bypassed-entry-point', diff --git a/stack/eslint-plugin-packlets/src/Path.ts b/stack/eslint-plugin-packlets/src/Path.ts index 4dfafd0473b..71fe92e5cdb 100644 --- a/stack/eslint-plugin-packlets/src/Path.ts +++ b/stack/eslint-plugin-packlets/src/Path.ts @@ -32,4 +32,14 @@ export class Path { public static isEqual(path1: string, path2: string): boolean { return path.relative(path1, path2) === ''; } + + /** + * Replaces Windows-style backslashes with POSIX-style slashes. + * + * @remarks + * POSIX is a registered trademark of the Institute of Electrical and Electronic Engineers, Inc. + */ + public static convertToSlashes(inputPath: string): string { + return inputPath.split('\\').join('/'); + } } From 5d9ef7e577641b0c79a3647fc4cdd40721fa4ad2 Mon Sep 17 00:00:00 2001 From: Pete Gonzalez <4673363+octogonz@users.noreply.github.com> Date: Sun, 4 Oct 2020 12:00:04 -0700 Subject: [PATCH 17/23] Add docs --- stack/eslint-plugin-packlets/README.md | 124 ++++++++++++++++++++++ stack/eslint-plugin-packlets/package.json | 2 +- tutorials/packlets-tutorial/README.md | 3 + 3 files changed, 128 insertions(+), 1 deletion(-) diff --git a/stack/eslint-plugin-packlets/README.md b/stack/eslint-plugin-packlets/README.md index 62e6e7bc644..5ecbe02ae1d 100644 --- a/stack/eslint-plugin-packlets/README.md +++ b/stack/eslint-plugin-packlets/README.md @@ -1,2 +1,126 @@ # @rushstack/eslint-plugin-packlets +Packlets provide a lightweight alternative to NPM packages for organizing source files within a single project. The formalism is enforced using ESLint rules. + +## Motivation + +When building a large application, it's a good idea to organize source files into modules, so that their dependencies can be managed. For example, suppose an application's source files can be grouped as follows: + +- `src/logging/*.ts` - the logging system +- `src/data-model/*.ts` - the data model +- `src/reports/*.ts` - the report engine +- `src/*.ts` - other arbitrary files such as startup code and the main application + +Using file folders is helpful, but it's not very strict. Files under `src/logging` can easily import files from `/src/reports`, creating a confusing circular import. They can also import arbitrary application files. Also, there is no clear distinction between which files are the "public API" for `src/logging` versus its private implementation details. + +All these problems can be solved by reorganizing the project into NPM packages (or [Rush projects](https://rushjs.io/)). Something like this: + +- `@my-app/logging` - the logging system +- `@my-app/data-model` - the data model +- `@my-app/reports` - the report engine +- `@my-app/application` - other arbitrary files such as startup code and the main application + +However, separating code in this ways has some downsides. The projects need to build separately, which has some tooling costs (for example, "watch mode" now needs to consider multiple projects). In a large monorepo, the library may attract other consumers, before the API has been fully worked out. + +Packlets provide a lightweight alternative that provides many of the same benefits of packages, but without the `package.json` file. It's a great way to prototype your project organization before later graduating your packlets into proper NPM packages. + +## 5 rules for packlets + +With packlets, our folders would be reorganized as follows: + +- `src/packlets/logging/*.ts` - the logging system +- `src/packlets/data-model/*.ts` - the data model +- `src/packlets/reports/*.ts` - the report engine +- `src/*.ts` - other arbitrary files such as startup code and the main application + +The [packlets-tutorial](https://github.com/microsoft/rushstack/tree/master/packlets/tutorials/packlets-tutorial) sample project shows this layout in more detail. + +The basic design can be summarized in 5 rules: + +1. A "packlet" is defined to be a folder path `./src/packlets//index.ts`. The **index.ts** file will have the exported APIs. The `` name must consist of lower case words separated by hyphens, similar to an NPM package name. + + Example file paths: + ``` + src/packlets/controls + src/packlets/logger + src/packlets/my-long-name + ``` + + > **NOTE:** The `packlets` cannot be nested deeper in the tree. Like with NPM packages, `src/packlets` is a flat namespace. + +2. Files outside the packlet folder can only import the packlet root **index.ts**: + + **src/app/App.ts** + ```ts + // Okay + import { MainReport } from '../packlets/reports'; + + // Error: The import statement does not use the packlet's entry point (@rushstack/packlets/mechanics) + import { MainReport } from '../packlets/reports/index'; + + // Error: The import statement does not use the packlet's entry point (@rushstack/packlets/mechanics) + import { MainReport } from '../packlets/reports/MainReport'; + ``` + +3. Files inside a packlet folder should import their siblings directly, not via their own **index.ts** (which might create a circular reference): + + **src/packlets/logging/Logger.ts** + ```ts + // Okay + import { MessageType } from "./MessageType"; + + // Error: Files under a packlet folder must not import from their own index.ts file (@rushstack/packlets/mechanics) + import { MessageType } from "."; + + // Error: Files under a packlet folder must not import from their own index.ts file (@rushstack/packlets/mechanics) + import { MessageType} from "./index"; + ``` + + +4. Packlets may reference other packlets, but not in a way that would introduce a circular dependency: + + **src/packlets/data-model/DataModel.ts** + ```ts + // Okay + import { Logger } from '../../packlets/logging'; + ``` + + **src/packlets/logging/Logger.ts** + ```ts + // Error: Packlet imports create a circular reference: (@rushstack/packlets/circular-deps) + // "logging" is referenced by src/packlets/data-model/DataModel.ts + // "data-model" is referenced by src/packlets/logging/Logger.ts + import { DataModel } from '../../packlets/data-model'; + ``` + +5. Other source files are allowed outside the **src/packlets** folder. They may import a packlet, but packlets must only import from other packlets or NPM packages. + + **src/app/App.ts** + + ```ts + // Okay + import { MainReport } from '../packlets/reports'; + ``` + + **src/packlets/data-model/ExampleModel.ts** + ```ts + // Error: A local project file cannot be imported. A packlet's dependencies must be + // NPM packages and/or other packlets. (@rushstack/packlets/mechanics) + import { App } from '../../app/App'; + ``` + +## Getting Started + + +## Links + +- [CHANGELOG.md]( + https://github.com/microsoft/rushstack/blob/master/stack/eslint-plugin-packlets/CHANGELOG.md) - Find + out what's new in the latest version + +- [@rushstack/eslint-config](https://www.npmjs.com/package/@rushstack/eslint-config) - A recommended ruleset + that includes support for packlets + +`@rushstack/eslint-plugin-packlets` is part of the [Rush Stack](https://rushstack.io/) family of projects. +The idea for packlets was originally proposed by [@bartvandenende-wm](https://github.com/bartvandenende-wm) +and [@victor-wm](https://github.com/victor-wm). diff --git a/stack/eslint-plugin-packlets/package.json b/stack/eslint-plugin-packlets/package.json index 57145c2bfe2..11263ac8cd1 100644 --- a/stack/eslint-plugin-packlets/package.json +++ b/stack/eslint-plugin-packlets/package.json @@ -1,7 +1,7 @@ { "name": "@rushstack/eslint-plugin-packlets", "version": "0.0.0", - "description": "A lightweight system for creating project subfolders with import relationships similar to NPM packages", + "description": "A lightweight alternative to NPM packages for organizing source files within a single project", "license": "MIT", "repository": { "url": "https://github.com/microsoft/rushstack/tree/master/stack/eslint-plugin-packlets" diff --git a/tutorials/packlets-tutorial/README.md b/tutorials/packlets-tutorial/README.md index fdf411a4c09..f4083878213 100644 --- a/tutorials/packlets-tutorial/README.md +++ b/tutorials/packlets-tutorial/README.md @@ -1,2 +1,5 @@ # packlets-tutorial +This code sample illustrates how to use "packlet" folders to organize source code within a project. + +For details, please see the [@rushstack/eslint-plugin-packlets](https://www.npmjs.com/package/@rushstack/eslint-plugin-packlets) documentation. From d9f2eba447e6ad79d7a93b00a49f5431bd1736a9 Mon Sep 17 00:00:00 2001 From: Pete Gonzalez <4673363+octogonz@users.noreply.github.com> Date: Sun, 4 Oct 2020 13:19:31 -0700 Subject: [PATCH 18/23] Add documentation --- stack/eslint-config/README.md | 90 ++++++++++++++--------- stack/eslint-config/mixins/packlets.js | 21 ++++++ stack/eslint-plugin-packlets/README.md | 46 +++++++++++- stack/eslint-plugin-packlets/src/index.ts | 10 +++ 4 files changed, 131 insertions(+), 36 deletions(-) create mode 100644 stack/eslint-config/mixins/packlets.js diff --git a/stack/eslint-config/README.md b/stack/eslint-config/README.md index 11b82e7d7cc..e889efba185 100644 --- a/stack/eslint-config/README.md +++ b/stack/eslint-config/README.md @@ -117,39 +117,6 @@ Optionally, you can add some "mixins" to your `extends` array to opt-in to some Important: Your **.eslintrc.js** `"extends"` field must load mixins after the profile entry. -#### `@rushstack/eslint-config/mixins/react` - -For projects using the [React](https://reactjs.org/) library, the `"@rushstack/eslint-config/mixins/react"` mixin -enables some recommended additional rules. These rules are selected via a mixin because they require you to: - -- Add `"jsx": "react"` to your **tsconfig.json** -- Configure your `settings.react.version` as shown below. This determines which React APIs will be considered - to be deprecated. (If you omit this, the React version will be detected automatically by - [loading the entire React library](https://github.com/yannickcr/eslint-plugin-react/blob/4da74518bd78f11c9c6875a159ffbae7d26be693/lib/util/version.js#L23) - into the linter's process, which is costly.) - -Add the mixin to your `"extends"` field like this: - -**.eslintrc.js** -```ts -// This is a workaround for https://github.com/eslint/eslint/issues/3458 -require('@rushstack/eslint-config/patch/modern-module-resolution'); - -module.exports = { - extends: [ - "@rushstack/eslint-config/profile/web-app", - "@rushstack/eslint-config/mixins/react" // <---- - ], - parserOptions: { tsconfigRootDir: __dirname }, - - settings: { - react: { - "version": "16.9" // <---- - } - } -}; -``` - #### `@rushstack/eslint-config/mixins/friendly-locals` Requires explicit type declarations for local variables. @@ -217,6 +184,28 @@ module.exports = { ``` +#### `@rushstack/eslint-config/mixins/packlets` + +Packlets provide a lightweight alternative to NPM packages for organizing source files within a single project. +This system is described in the [@rushstack/eslint-plugin-packlets](https://www.npmjs.com/package/@rushstack/eslint-plugin-packlets) +documentation. + +To use packlets, add the mixin to your `"extends"` field like this: + +**.eslintrc.js** +```ts +// This is a workaround for https://github.com/eslint/eslint/issues/3458 +require('@rushstack/eslint-config/patch/modern-module-resolution'); + +module.exports = { + extends: [ + "@rushstack/eslint-config/profile/node", + "@rushstack/eslint-config/profile/mixins/packlets" // <---- + ], + parserOptions: { tsconfigRootDir: __dirname } +}; +``` + #### `@rushstack/eslint-config/mixins/tsdoc` @@ -241,6 +230,41 @@ module.exports = { }; ``` + +#### `@rushstack/eslint-config/mixins/react` + +For projects using the [React](https://reactjs.org/) library, the `"@rushstack/eslint-config/mixins/react"` mixin +enables some recommended additional rules. These rules are selected via a mixin because they require you to: + +- Add `"jsx": "react"` to your **tsconfig.json** +- Configure your `settings.react.version` as shown below. This determines which React APIs will be considered + to be deprecated. (If you omit this, the React version will be detected automatically by + [loading the entire React library](https://github.com/yannickcr/eslint-plugin-react/blob/4da74518bd78f11c9c6875a159ffbae7d26be693/lib/util/version.js#L23) + into the linter's process, which is costly.) + +Add the mixin to your `"extends"` field like this: + +**.eslintrc.js** +```ts +// This is a workaround for https://github.com/eslint/eslint/issues/3458 +require('@rushstack/eslint-config/patch/modern-module-resolution'); + +module.exports = { + extends: [ + "@rushstack/eslint-config/profile/web-app", + "@rushstack/eslint-config/mixins/react" // <---- + ], + parserOptions: { tsconfigRootDir: __dirname }, + + settings: { + react: { + "version": "16.9" // <---- + } + } +}; +``` + + ## Links - [CHANGELOG.md]( diff --git a/stack/eslint-config/mixins/packlets.js b/stack/eslint-config/mixins/packlets.js new file mode 100644 index 00000000000..9c6b791e546 --- /dev/null +++ b/stack/eslint-config/mixins/packlets.js @@ -0,0 +1,21 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. +// See LICENSE in the project root for license information. + +// This mixin implements the "packlet" formalism for organizing source files. +// For more information, see the documentation here: +// https://www.npmjs.com/package/@rushstack/eslint-plugin-packlets +module.exports = { + plugins: ['@rushstack/eslint-plugin-packlets'], + + overrides: [ + { + // Declare an override that applies to TypeScript files only + files: ['*.ts', '*.tsx'], + + rules: { + '@rushstack/packlets/mechanics': 'warn', + '@rushstack/packlets/circular-deps': 'warn' + } + } + ] +}; diff --git a/stack/eslint-plugin-packlets/README.md b/stack/eslint-plugin-packlets/README.md index 5ecbe02ae1d..424b50bce65 100644 --- a/stack/eslint-plugin-packlets/README.md +++ b/stack/eslint-plugin-packlets/README.md @@ -111,15 +111,55 @@ The basic design can be summarized in 5 rules: ## Getting Started +To enable packlet validation for a simple `typescript-eslint` setup, reference the `@rushstack/eslint-plugin-packlets` project like this: + +**\/.eslintrc.js** +```js +module.exports = { + root: true, + parser: '@typescript-eslint/parser', + plugins: ['@typescript-eslint'], + extends: [ + 'eslint:recommended', + 'plugin:@typescript-eslint/recommended', + 'plugin:@rushstack/eslint-plugin-packlets/recommended' // <--- ADD THIS + ], + parserOptions: { + project: './tsconfig.json', + sourceType: 'module', + tsconfigRootDir: __dirname + } +}; +``` + +If you use the [@rushstack/eslint-config](https://www.npmjs.com/package/@rushstack/eslint-config) ruleset, add the `"packlets"` mixin like this: + +**\/.eslintrc.js** +```ts +// This is a workaround for https://github.com/eslint/eslint/issues/3458 +require('@rushstack/eslint-config/patch/modern-module-resolution'); + +module.exports = { + extends: [ + "@rushstack/eslint-config/profile/node", + "@rushstack/eslint-config/profile/mixins/packlets" // <---- + ], + parserOptions: { tsconfigRootDir: __dirname } +}; +``` + +The `@rushstack/eslint-plugin-packlets` plugin performs validation via two separate rules: + +- `@rushstack/packlets/mechanics` - This rule implements most of the basic checks for packlet folder names. +- `@rushstack/packlets/circular-deps` - This rule detects circular dependencies between packlets. It requires full type information from the TypeScript compiler. + ## Links - [CHANGELOG.md]( https://github.com/microsoft/rushstack/blob/master/stack/eslint-plugin-packlets/CHANGELOG.md) - Find out what's new in the latest version - -- [@rushstack/eslint-config](https://www.npmjs.com/package/@rushstack/eslint-config) - A recommended ruleset - that includes support for packlets +- [@rushstack/eslint-config](https://www.npmjs.com/package/@rushstack/eslint-config) documentation `@rushstack/eslint-plugin-packlets` is part of the [Rush Stack](https://rushstack.io/) family of projects. The idea for packlets was originally proposed by [@bartvandenende-wm](https://github.com/bartvandenende-wm) diff --git a/stack/eslint-plugin-packlets/src/index.ts b/stack/eslint-plugin-packlets/src/index.ts index 3c91ae4b0c8..892bb5f8103 100644 --- a/stack/eslint-plugin-packlets/src/index.ts +++ b/stack/eslint-plugin-packlets/src/index.ts @@ -7,6 +7,7 @@ import { circularDeps } from './circular-deps'; interface IPlugin { rules: { [ruleName: string]: TSESLint.RuleModule }; + configs: { [ruleName: string]: any }; } const plugin: IPlugin = { @@ -15,6 +16,15 @@ const plugin: IPlugin = { mechanics: mechanics, // Full name: "@rushstack/packlets/circular-deps" 'circular-deps': circularDeps + }, + configs: { + recommended: { + plugins: ['@rushstack/eslint-plugin-packlets'], + rules: { + '@rushstack/packlets/mechanics': 'warn', + '@rushstack/packlets/circular-deps': 'warn' + } + } } }; From 93b1630b57c0ba752eb21c83398b8fde063def98 Mon Sep 17 00:00:00 2001 From: Pete Gonzalez <4673363+octogonz@users.noreply.github.com> Date: Sun, 4 Oct 2020 18:38:38 -0700 Subject: [PATCH 19/23] Fix up packlets-tutorial reference --- .../rush/nonbrowser-approved-packages.json | 2 +- stack/eslint-config/package.json | 1 + tutorials/packlets-tutorial/.eslintrc.js | 16 ++-------------- tutorials/packlets-tutorial/package.json | 1 - 4 files changed, 4 insertions(+), 16 deletions(-) diff --git a/common/config/rush/nonbrowser-approved-packages.json b/common/config/rush/nonbrowser-approved-packages.json index 0b8fccbc642..d8d80d2f3fc 100644 --- a/common/config/rush/nonbrowser-approved-packages.json +++ b/common/config/rush/nonbrowser-approved-packages.json @@ -176,7 +176,7 @@ }, { "name": "@rushstack/eslint-plugin-packlets", - "allowedCategories": [ "tests" ] + "allowedCategories": [ "libraries", "tests" ] }, { "name": "@rushstack/eslint-plugin-security", diff --git a/stack/eslint-config/package.json b/stack/eslint-config/package.json index 48351b45458..a2742a094b7 100644 --- a/stack/eslint-config/package.json +++ b/stack/eslint-config/package.json @@ -26,6 +26,7 @@ "dependencies": { "@rushstack/eslint-patch": "workspace:*", "@rushstack/eslint-plugin": "workspace:*", + "@rushstack/eslint-plugin-packlets": "workspace:*", "@rushstack/eslint-plugin-security": "workspace:*", "@typescript-eslint/eslint-plugin": "3.4.0", "@typescript-eslint/experimental-utils": "3.4.0", diff --git a/tutorials/packlets-tutorial/.eslintrc.js b/tutorials/packlets-tutorial/.eslintrc.js index b69d9978ae3..62885de24de 100644 --- a/tutorials/packlets-tutorial/.eslintrc.js +++ b/tutorials/packlets-tutorial/.eslintrc.js @@ -2,18 +2,6 @@ require('@rushstack/eslint-config/patch/modern-module-resolution'); module.exports = { - extends: ['@rushstack/eslint-config/profile/node'], - plugins: ['@rushstack/eslint-plugin-packlets'], - parserOptions: { tsconfigRootDir: __dirname }, - overrides: [ - { - // Declare an override that applies to TypeScript files only - files: ['*.ts', '*.tsx'], - - rules: { - '@rushstack/packlets/mechanics': 'warn', - '@rushstack/packlets/circular-deps': 'warn' - } - } - ] + extends: ['@rushstack/eslint-config/profile/node', '@rushstack/eslint-config/mixins/packlets'], + parserOptions: { tsconfigRootDir: __dirname } }; diff --git a/tutorials/packlets-tutorial/package.json b/tutorials/packlets-tutorial/package.json index 922bfc54aab..2dfd6155fe8 100644 --- a/tutorials/packlets-tutorial/package.json +++ b/tutorials/packlets-tutorial/package.json @@ -10,7 +10,6 @@ }, "devDependencies": { "@rushstack/eslint-config": "workspace:*", - "@rushstack/eslint-plugin-packlets": "workspace:*", "@rushstack/heft": "workspace:*", "@types/node": "10.17.13", "eslint": "~7.2.0", From 51e24e2d3cb71c186d00c61313e8a8ceb1202cd9 Mon Sep 17 00:00:00 2001 From: Pete Gonzalez <4673363+octogonz@users.noreply.github.com> Date: Sun, 4 Oct 2020 18:39:01 -0700 Subject: [PATCH 20/23] PR feedback - some minor cleanups --- stack/eslint-plugin-packlets/README.md | 2 +- stack/eslint-plugin-packlets/src/PackletAnalyzer.ts | 12 ++++++++---- stack/eslint-plugin-packlets/src/circular-deps.ts | 2 +- stack/eslint-plugin-packlets/src/mechanics.ts | 6 +++--- 4 files changed, 13 insertions(+), 9 deletions(-) diff --git a/stack/eslint-plugin-packlets/README.md b/stack/eslint-plugin-packlets/README.md index 424b50bce65..5124c4c772e 100644 --- a/stack/eslint-plugin-packlets/README.md +++ b/stack/eslint-plugin-packlets/README.md @@ -1,6 +1,6 @@ # @rushstack/eslint-plugin-packlets -Packlets provide a lightweight alternative to NPM packages for organizing source files within a single project. The formalism is enforced using ESLint rules. +Packlets provide a lightweight alternative to NPM packages for organizing source files within a single project. The formalism is validated using ESLint rules. ## Motivation diff --git a/stack/eslint-plugin-packlets/src/PackletAnalyzer.ts b/stack/eslint-plugin-packlets/src/PackletAnalyzer.ts index 767c21e05bc..785a0f9f802 100644 --- a/stack/eslint-plugin-packlets/src/PackletAnalyzer.ts +++ b/stack/eslint-plugin-packlets/src/PackletAnalyzer.ts @@ -5,20 +5,20 @@ import * as path from 'path'; import * as fs from 'fs'; import { Path } from './Path'; -export type MyMessageIds = +export type InputFileMessageIds = | 'missing-tsconfig' | 'missing-src-folder' | 'packlet-folder-case' | 'invalid-packlet-name' | 'misplaced-packlets-folder'; -export type MyMessageIds2 = +export type ImportMessageIds = | 'bypassed-entry-point' | 'circular-entry-point' | 'packlet-importing-project-file'; export interface IAnalyzerError { - messageId: MyMessageIds | MyMessageIds2; + messageId: InputFileMessageIds | ImportMessageIds; data?: Readonly>; } @@ -64,7 +64,7 @@ export class PacketAnalyzer { */ public readonly isEntryPoint: boolean; - public constructor(inputFilePath: string, tsconfigFilePath: string | undefined) { + private constructor(inputFilePath: string, tsconfigFilePath: string | undefined) { this.inputFilePath = inputFilePath; this.error = undefined; this.nothingToDo = false; @@ -155,6 +155,10 @@ export class PacketAnalyzer { } } + public static analyzeInputFile(inputFilePath: string, tsconfigFilePath: string | undefined) { + return new PacketAnalyzer(inputFilePath, tsconfigFilePath); + } + public analyzeImport(modulePath: string): IAnalyzerError | undefined { if (!this.packletsFolderPath) { // The caller should ensure this can never happen diff --git a/stack/eslint-plugin-packlets/src/circular-deps.ts b/stack/eslint-plugin-packlets/src/circular-deps.ts index 2c4258a65e7..a9f674a4be8 100644 --- a/stack/eslint-plugin-packlets/src/circular-deps.ts +++ b/stack/eslint-plugin-packlets/src/circular-deps.ts @@ -40,7 +40,7 @@ const circularDeps: TSESLint.RuleModule = { context ).program.getCompilerOptions()['configFilePath'] as string; - const packletAnalyzer: PacketAnalyzer = new PacketAnalyzer(inputFilePath, tsconfigFilePath); + const packletAnalyzer: PacketAnalyzer = PacketAnalyzer.analyzeInputFile(inputFilePath, tsconfigFilePath); if (packletAnalyzer.nothingToDo) { return {}; } diff --git a/stack/eslint-plugin-packlets/src/mechanics.ts b/stack/eslint-plugin-packlets/src/mechanics.ts index 959bfd40468..731fcb51dcd 100644 --- a/stack/eslint-plugin-packlets/src/mechanics.ts +++ b/stack/eslint-plugin-packlets/src/mechanics.ts @@ -4,9 +4,9 @@ import type { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils'; import { AST_NODE_TYPES, ESLintUtils } from '@typescript-eslint/experimental-utils'; -import { PacketAnalyzer, IAnalyzerError, MyMessageIds, MyMessageIds2 } from './PackletAnalyzer'; +import { PacketAnalyzer, IAnalyzerError, InputFileMessageIds, ImportMessageIds } from './PackletAnalyzer'; -export type MessageIds = MyMessageIds | MyMessageIds2; +export type MessageIds = InputFileMessageIds | ImportMessageIds; type Options = []; const mechanics: TSESLint.RuleModule = { @@ -52,7 +52,7 @@ const mechanics: TSESLint.RuleModule = { context ).program.getCompilerOptions()['configFilePath'] as string; - const packletAnalyzer: PacketAnalyzer = new PacketAnalyzer(inputFilePath, tsconfigFilePath); + const packletAnalyzer: PacketAnalyzer = PacketAnalyzer.analyzeInputFile(inputFilePath, tsconfigFilePath); if (packletAnalyzer.nothingToDo) { return {}; } From 44b8d3cac9bf1e9f16c956bf3c06cf3aed49d86d Mon Sep 17 00:00:00 2001 From: Pete Gonzalez <4673363+octogonz@users.noreply.github.com> Date: Sun, 4 Oct 2020 18:39:45 -0700 Subject: [PATCH 21/23] rush change --- .../octogonz-packlets_2020-10-05-01-39.json | 11 +++++++++++ .../octogonz-packlets_2020-10-05-01-39.json | 11 +++++++++++ 2 files changed, 22 insertions(+) create mode 100644 common/changes/@rushstack/eslint-config/octogonz-packlets_2020-10-05-01-39.json create mode 100644 common/changes/@rushstack/eslint-plugin-packlets/octogonz-packlets_2020-10-05-01-39.json diff --git a/common/changes/@rushstack/eslint-config/octogonz-packlets_2020-10-05-01-39.json b/common/changes/@rushstack/eslint-config/octogonz-packlets_2020-10-05-01-39.json new file mode 100644 index 00000000000..71555c74848 --- /dev/null +++ b/common/changes/@rushstack/eslint-config/octogonz-packlets_2020-10-05-01-39.json @@ -0,0 +1,11 @@ +{ + "changes": [ + { + "packageName": "@rushstack/eslint-config", + "comment": "Add a mixin to support @rushstack/eslint-plugin-packlets", + "type": "minor" + } + ], + "packageName": "@rushstack/eslint-config", + "email": "4673363+octogonz@users.noreply.github.com" +} \ No newline at end of file diff --git a/common/changes/@rushstack/eslint-plugin-packlets/octogonz-packlets_2020-10-05-01-39.json b/common/changes/@rushstack/eslint-plugin-packlets/octogonz-packlets_2020-10-05-01-39.json new file mode 100644 index 00000000000..cb7cfb94975 --- /dev/null +++ b/common/changes/@rushstack/eslint-plugin-packlets/octogonz-packlets_2020-10-05-01-39.json @@ -0,0 +1,11 @@ +{ + "changes": [ + { + "packageName": "@rushstack/eslint-plugin-packlets", + "comment": "Initial release", + "type": "minor" + } + ], + "packageName": "@rushstack/eslint-plugin-packlets", + "email": "4673363+octogonz@users.noreply.github.com" +} \ No newline at end of file From ec72a6a400b96523b98c12856de33a99a0df044b Mon Sep 17 00:00:00 2001 From: Pete Gonzalez <4673363+octogonz@users.noreply.github.com> Date: Mon, 5 Oct 2020 15:07:29 -0700 Subject: [PATCH 22/23] PR feedback and some doc updates --- stack/eslint-plugin-packlets/README.md | 5 +- .../src/DependencyAnalyzer.ts | 66 ++++++++++++++++--- .../src/circular-deps.ts | 57 ++++++++-------- stack/eslint-plugin-packlets/src/mechanics.ts | 2 +- 4 files changed, 90 insertions(+), 40 deletions(-) diff --git a/stack/eslint-plugin-packlets/README.md b/stack/eslint-plugin-packlets/README.md index 5124c4c772e..de1dff2d60b 100644 --- a/stack/eslint-plugin-packlets/README.md +++ b/stack/eslint-plugin-packlets/README.md @@ -22,7 +22,7 @@ All these problems can be solved by reorganizing the project into NPM packages ( However, separating code in this ways has some downsides. The projects need to build separately, which has some tooling costs (for example, "watch mode" now needs to consider multiple projects). In a large monorepo, the library may attract other consumers, before the API has been fully worked out. -Packlets provide a lightweight alternative that provides many of the same benefits of packages, but without the `package.json` file. It's a great way to prototype your project organization before later graduating your packlets into proper NPM packages. +Packlets provide a lightweight alternative that offers many of the same benefits of packages, but without the `package.json` file. It's a great way to prototype your project organization before later graduating your packlets into proper NPM packages. ## 5 rules for packlets @@ -109,6 +109,7 @@ The basic design can be summarized in 5 rules: import { App } from '../../app/App'; ``` + ## Getting Started To enable packlet validation for a simple `typescript-eslint` setup, reference the `@rushstack/eslint-plugin-packlets` project like this: @@ -150,7 +151,7 @@ module.exports = { The `@rushstack/eslint-plugin-packlets` plugin performs validation via two separate rules: -- `@rushstack/packlets/mechanics` - This rule implements most of the basic checks for packlet folder names. +- `@rushstack/packlets/mechanics` - This rule validates most of the import path rules outlined above. It does not require full type information. - `@rushstack/packlets/circular-deps` - This rule detects circular dependencies between packlets. It requires full type information from the TypeScript compiler. diff --git a/stack/eslint-plugin-packlets/src/DependencyAnalyzer.ts b/stack/eslint-plugin-packlets/src/DependencyAnalyzer.ts index 71c742d63e2..bf227ace3a3 100644 --- a/stack/eslint-plugin-packlets/src/DependencyAnalyzer.ts +++ b/stack/eslint-plugin-packlets/src/DependencyAnalyzer.ts @@ -16,9 +16,16 @@ enum RefFileKind { // TypeScript compiler internal: // https://github.com/microsoft/TypeScript/blob/5ecdcef4cecfcdc86bd681b377636422447507d7/src/compiler/program.ts#L541 interface RefFile { + // The absolute path of the module that was imported. + // (Normalized to an all lowercase ts.Path string.) referencedFileName: string; + // The kind of reference. kind: RefFileKind; + // An index indicating the order in which items occur in a compound expression index: number; + + // The absolute path of the source file containing the import statement. + // (Normalized to an all lowercase ts.Path string.) file: string; } @@ -48,7 +55,17 @@ interface IImportListNode extends IPackletImport { } export class DependencyAnalyzer { - private static walkImports( + /** + * @param packletName - the packlet to be checked next in our traversal + * @param startingPackletName - the packlet that we started with; if the traversal reaches this packlet, + * then a circular dependency has been detected + * @param refFileMap - the compiler's `refFileMap` data structure describing import relationships + * @param program - the compiler's `ts.Program` object + * @param packletsFolderPath - the absolute path of the "src/packlets" folder. + * @param visitedPacklets - the set of packlets that have already been visited in this traversal + * @param previousNode - a linked list of import statements that brought us to this step in the traversal + */ + private static _walkImports( packletName: string, startingPackletName: string, refFileMap: Map, @@ -74,14 +91,17 @@ export class DependencyAnalyzer { if (refFile.kind === RefFileKind.Import) { const referencingFilePath: string = refFile.file; + // Is it a reference to a packlet? if (Path.isUnder(referencingFilePath, packletsFolderPath)) { const referencingRelativePath: string = path.relative(packletsFolderPath, referencingFilePath); const referencingPathParts: string[] = referencingRelativePath.split(/[\/\\]+/); const referencingPackletName: string = referencingPathParts[0]; + // Have we already analyzed this packlet? if (!visitedPacklets.has(packletName)) { visitedPacklets.add(packletName); + // Make a new linked list node to record this step of the traversal const importListNode: IImportListNode = { previousNode: previousNode, fromFilePath: referencingFilePath, @@ -89,10 +109,12 @@ export class DependencyAnalyzer { }; if (referencingPackletName === startingPackletName) { + // The traversal has returned to the packlet that we started from; + // this means we have detected a circular dependency return importListNode; } - const result: IImportListNode | undefined = DependencyAnalyzer.walkImports( + const result: IImportListNode | undefined = DependencyAnalyzer._walkImports( referencingPackletName, startingPackletName, refFileMap, @@ -112,24 +134,52 @@ export class DependencyAnalyzer { return undefined; } - public static detectCircularImport( - packetAnalyzer: PacketAnalyzer, + /** + * For the specified packlet, trace all modules that import it, looking for a circular dependency + * between packlets. If found, an array is returned describing the import statements that cause + * the problem. + * + * @remarks + * For example, suppose we have files like this: + * + * ``` + * src/packlets/logging/index.ts + * src/packlets/logging/Logger.ts --> imports "../data-model" + * src/packlets/data-model/index.ts + * src/packlets/data-model/DataModel.ts --> imports "../logging" + * ``` + * + * The returned array would be: + * ```ts + * [ + * { packletName: "logging", fromFilePath: "/path/to/src/packlets/data-model/DataModel.ts" }, + * { packletName: "data-model", fromFilePath: "/path/to/src/packlets/logging/Logger.ts" }, + * ] + * ``` + * + * If there is more than one circular dependency chain, only the first one that is encountered + * will be returned. + */ + public static checkEntryPointForCircularImport( + packletName: string, + packletAnalyzer: PacketAnalyzer, program: ts.Program ): IPackletImport[] | undefined { const refFileMap: Map = (program as any).getRefFileMap(); const visitedPacklets: Set = new Set(); - const listNode: IImportListNode | undefined = DependencyAnalyzer.walkImports( - packetAnalyzer.inputFilePackletName!, - packetAnalyzer.inputFilePackletName!, + const listNode: IImportListNode | undefined = DependencyAnalyzer._walkImports( + packletName, + packletName, refFileMap, program, - packetAnalyzer.packletsFolderPath!, + packletAnalyzer.packletsFolderPath!, visitedPacklets, undefined // previousNode ); if (listNode) { + // Convert the linked list to an array const packletImports: IPackletImport[] = []; for (let current: IImportListNode | undefined = listNode; current; current = current.previousNode) { packletImports.push({ fromFilePath: current.fromFilePath, packletName: current.packletName }); diff --git a/stack/eslint-plugin-packlets/src/circular-deps.ts b/stack/eslint-plugin-packlets/src/circular-deps.ts index a9f674a4be8..93ef4f050f4 100644 --- a/stack/eslint-plugin-packlets/src/circular-deps.ts +++ b/stack/eslint-plugin-packlets/src/circular-deps.ts @@ -4,7 +4,7 @@ import type * as ts from 'typescript'; import * as path from 'path'; -import type { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils'; +import type { ParserServices, TSESLint, TSESTree } from '@typescript-eslint/experimental-utils'; import { ESLintUtils } from '@typescript-eslint/experimental-utils'; import { PacketAnalyzer } from './PackletAnalyzer'; @@ -24,7 +24,7 @@ const circularDeps: TSESLint.RuleModule = { } ], docs: { - description: '', + description: 'Check for circular dependencies between packlets', category: 'Best Practices', recommended: 'warn', url: 'https://www.npmjs.com/package/@rushstack/eslint-plugin-packlets' @@ -36,9 +36,8 @@ const circularDeps: TSESLint.RuleModule = { const inputFilePath: string = context.getFilename(); // Example: /path/to/my-project/tsconfig.json - const tsconfigFilePath: string | undefined = ESLintUtils.getParserServices( - context - ).program.getCompilerOptions()['configFilePath'] as string; + const program: ts.Program = ESLintUtils.getParserServices(context).program; + const tsconfigFilePath: string | undefined = program.getCompilerOptions()['configFilePath'] as string; const packletAnalyzer: PacketAnalyzer = PacketAnalyzer.analyzeInputFile(inputFilePath, tsconfigFilePath); if (packletAnalyzer.nothingToDo) { @@ -51,34 +50,34 @@ const circularDeps: TSESLint.RuleModule = { // https://github.com/estools/esquery/issues/114 Program: (node: TSESTree.Node): void => { if (packletAnalyzer.isEntryPoint && !packletAnalyzer.error) { - const program: ts.Program | undefined = context.parserServices?.program; - if (program) { - const packletImports: IPackletImport[] | undefined = DependencyAnalyzer.detectCircularImport( - packletAnalyzer, - program - ); + const packletImports: + | IPackletImport[] + | undefined = DependencyAnalyzer.checkEntryPointForCircularImport( + packletAnalyzer.inputFilePackletName!, + packletAnalyzer, + program + ); - if (packletImports) { - const tsconfigFileFolder: string = path.dirname(tsconfigFilePath); + if (packletImports) { + const tsconfigFileFolder: string = path.dirname(tsconfigFilePath); - const affectedPackletNames: string[] = packletImports.map((x) => x.packletName); + const affectedPackletNames: string[] = packletImports.map((x) => x.packletName); - // If 3 different packlets form a circular dependency, we don't need to report the same warning 3 times. - // Instead, only report the warning for the alphabetically smallest packlet. - affectedPackletNames.sort(); - if (affectedPackletNames[0] === packletAnalyzer.inputFilePackletName) { - let report: string = ''; - for (const packletImport of packletImports) { - const filePath: string = path.relative(tsconfigFileFolder, packletImport.fromFilePath); - report += `"${packletImport.packletName}" is referenced by ${filePath}\n`; - } - - context.report({ - node: node, - messageId: 'circular-import', - data: { report: report } - }); + // If 3 different packlets form a circular dependency, we don't need to report the same warning 3 times. + // Instead, only report the warning for the alphabetically smallest packlet. + affectedPackletNames.sort(); + if (affectedPackletNames[0] === packletAnalyzer.inputFilePackletName) { + let report: string = ''; + for (const packletImport of packletImports) { + const filePath: string = path.relative(tsconfigFileFolder, packletImport.fromFilePath); + report += `"${packletImport.packletName}" is referenced by ${filePath}\n`; } + + context.report({ + node: node, + messageId: 'circular-import', + data: { report: report } + }); } } } diff --git a/stack/eslint-plugin-packlets/src/mechanics.ts b/stack/eslint-plugin-packlets/src/mechanics.ts index 731fcb51dcd..43ee5273a83 100644 --- a/stack/eslint-plugin-packlets/src/mechanics.ts +++ b/stack/eslint-plugin-packlets/src/mechanics.ts @@ -36,7 +36,7 @@ const mechanics: TSESLint.RuleModule = { } ], docs: { - description: '', + description: 'Check that file paths and imports follow the basic mechanics for the packlet formalism', category: 'Best Practices', recommended: 'warn', url: 'https://www.npmjs.com/package/@rushstack/eslint-plugin-packlets' From 72b8ecfeb5635a997f4fb3f9aaf601cd8e5a5b7b Mon Sep 17 00:00:00 2001 From: Pete Gonzalez <4673363+octogonz@users.noreply.github.com> Date: Mon, 5 Oct 2020 15:08:07 -0700 Subject: [PATCH 23/23] Fix spelling error --- stack/eslint-plugin-packlets/src/DependencyAnalyzer.ts | 4 ++-- stack/eslint-plugin-packlets/src/PackletAnalyzer.ts | 6 +++--- stack/eslint-plugin-packlets/src/circular-deps.ts | 7 +++++-- stack/eslint-plugin-packlets/src/mechanics.ts | 7 +++++-- 4 files changed, 15 insertions(+), 9 deletions(-) diff --git a/stack/eslint-plugin-packlets/src/DependencyAnalyzer.ts b/stack/eslint-plugin-packlets/src/DependencyAnalyzer.ts index bf227ace3a3..0f23fa4697d 100644 --- a/stack/eslint-plugin-packlets/src/DependencyAnalyzer.ts +++ b/stack/eslint-plugin-packlets/src/DependencyAnalyzer.ts @@ -5,7 +5,7 @@ import type * as ts from 'typescript'; import * as path from 'path'; import { Path } from './Path'; -import { PacketAnalyzer } from './PackletAnalyzer'; +import { PackletAnalyzer } from './PackletAnalyzer'; enum RefFileKind { Import, @@ -162,7 +162,7 @@ export class DependencyAnalyzer { */ public static checkEntryPointForCircularImport( packletName: string, - packletAnalyzer: PacketAnalyzer, + packletAnalyzer: PackletAnalyzer, program: ts.Program ): IPackletImport[] | undefined { const refFileMap: Map = (program as any).getRefFileMap(); diff --git a/stack/eslint-plugin-packlets/src/PackletAnalyzer.ts b/stack/eslint-plugin-packlets/src/PackletAnalyzer.ts index 785a0f9f802..4e0c1632722 100644 --- a/stack/eslint-plugin-packlets/src/PackletAnalyzer.ts +++ b/stack/eslint-plugin-packlets/src/PackletAnalyzer.ts @@ -22,7 +22,7 @@ export interface IAnalyzerError { data?: Readonly>; } -export class PacketAnalyzer { +export class PackletAnalyzer { private static _validPackletName: RegExp = /^[a-z0-9]+(-[a-z0-9]+)*$/; /** @@ -141,7 +141,7 @@ export class PacketAnalyzer { const thirdPartWithoutExtension: string = path.parse(thirdPart).name; if (thirdPartWithoutExtension.toUpperCase() === 'INDEX') { - if (!PacketAnalyzer._validPackletName.test(packletName)) { + if (!PackletAnalyzer._validPackletName.test(packletName)) { this.error = { messageId: 'invalid-packlet-name', data: { packletName } }; return; } @@ -156,7 +156,7 @@ export class PacketAnalyzer { } public static analyzeInputFile(inputFilePath: string, tsconfigFilePath: string | undefined) { - return new PacketAnalyzer(inputFilePath, tsconfigFilePath); + return new PackletAnalyzer(inputFilePath, tsconfigFilePath); } public analyzeImport(modulePath: string): IAnalyzerError | undefined { diff --git a/stack/eslint-plugin-packlets/src/circular-deps.ts b/stack/eslint-plugin-packlets/src/circular-deps.ts index 93ef4f050f4..b92daf90457 100644 --- a/stack/eslint-plugin-packlets/src/circular-deps.ts +++ b/stack/eslint-plugin-packlets/src/circular-deps.ts @@ -7,7 +7,7 @@ import * as path from 'path'; import type { ParserServices, TSESLint, TSESTree } from '@typescript-eslint/experimental-utils'; import { ESLintUtils } from '@typescript-eslint/experimental-utils'; -import { PacketAnalyzer } from './PackletAnalyzer'; +import { PackletAnalyzer } from './PackletAnalyzer'; import { DependencyAnalyzer, IPackletImport } from './DependencyAnalyzer'; export type MessageIds = 'circular-import'; @@ -39,7 +39,10 @@ const circularDeps: TSESLint.RuleModule = { const program: ts.Program = ESLintUtils.getParserServices(context).program; const tsconfigFilePath: string | undefined = program.getCompilerOptions()['configFilePath'] as string; - const packletAnalyzer: PacketAnalyzer = PacketAnalyzer.analyzeInputFile(inputFilePath, tsconfigFilePath); + const packletAnalyzer: PackletAnalyzer = PackletAnalyzer.analyzeInputFile( + inputFilePath, + tsconfigFilePath + ); if (packletAnalyzer.nothingToDo) { return {}; } diff --git a/stack/eslint-plugin-packlets/src/mechanics.ts b/stack/eslint-plugin-packlets/src/mechanics.ts index 43ee5273a83..ad1a2605663 100644 --- a/stack/eslint-plugin-packlets/src/mechanics.ts +++ b/stack/eslint-plugin-packlets/src/mechanics.ts @@ -4,7 +4,7 @@ import type { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils'; import { AST_NODE_TYPES, ESLintUtils } from '@typescript-eslint/experimental-utils'; -import { PacketAnalyzer, IAnalyzerError, InputFileMessageIds, ImportMessageIds } from './PackletAnalyzer'; +import { PackletAnalyzer, IAnalyzerError, InputFileMessageIds, ImportMessageIds } from './PackletAnalyzer'; export type MessageIds = InputFileMessageIds | ImportMessageIds; type Options = []; @@ -52,7 +52,10 @@ const mechanics: TSESLint.RuleModule = { context ).program.getCompilerOptions()['configFilePath'] as string; - const packletAnalyzer: PacketAnalyzer = PacketAnalyzer.analyzeInputFile(inputFilePath, tsconfigFilePath); + const packletAnalyzer: PackletAnalyzer = PackletAnalyzer.analyzeInputFile( + inputFilePath, + tsconfigFilePath + ); if (packletAnalyzer.nothingToDo) { return {}; }