From a7450d85e3f20e6970009a6ac650e4766390b57f Mon Sep 17 00:00:00 2001 From: "vilchik.elena" Date: Fri, 8 Apr 2022 09:56:53 +0200 Subject: [PATCH 1/5] WIP --- .../src/rules/no-implicit-dependencies.ts | 54 ++++++++++++------- .../nested-package-json-project/dir/file.js | 0 .../dir/package.json | 5 ++ .../nested-package-json-project/package.json | 5 ++ .../rules/no-implicit-dependencies.test.ts | 31 +++++++++++ 5 files changed, 77 insertions(+), 18 deletions(-) create mode 100644 eslint-bridge/tests/fixtures/no-implicit-dependencies/nested-package-json-project/dir/file.js create mode 100644 eslint-bridge/tests/fixtures/no-implicit-dependencies/nested-package-json-project/dir/package.json create mode 100644 eslint-bridge/tests/fixtures/no-implicit-dependencies/nested-package-json-project/package.json diff --git a/eslint-bridge/src/rules/no-implicit-dependencies.ts b/eslint-bridge/src/rules/no-implicit-dependencies.ts index ac593dcdbfd..3e0938793eb 100644 --- a/eslint-bridge/src/rules/no-implicit-dependencies.ts +++ b/eslint-bridge/src/rules/no-implicit-dependencies.ts @@ -146,36 +146,54 @@ function getPackageName(name: string) { } function getDependencies(fileName: string) { - const dirname = path.dirname(fileName); + const result = new Set(); + + let dirname = path.dirname(fileName); + while (true) { + getDependenciesFromDir(dirname).forEach(d => result.add(d)); + const upperDirname = path.dirname(dirname); + if (upperDirname === dirname) { + break; + } + dirname = upperDirname; + } + + return result; +} +function getDependenciesFromDir(dirname: string) { const cached = cache.get(dirname); if (cached) { return cached; } - const result = new Set(); const packageJsonPath = findPackageJson(path.resolve(dirname)); - if (packageJsonPath !== undefined) { - try { - // remove BOM from file content before parsing - const content = JSON.parse(fs.readFileSync(packageJsonPath, 'utf8').replace(/^\uFEFF/, '')); - if (content.dependencies !== undefined) { - addDependencies(result, content.dependencies); - } - if (content.devDependencies !== undefined) { - addDependencies(result, content.devDependencies); - } - if (content.peerDependencies !== undefined) { - addDependencies(result, content.peerDependencies); - } - } catch {} - } - + const result = packageJsonPath + ? getDependenciesFromPackageJson(packageJsonPath) + : new Set(); cache.set(dirname, result); return result; } +function getDependenciesFromPackageJson(packageJsonPath: string) { + const result = new Set(); + try { + // remove BOM from file content before parsing + const content = JSON.parse(fs.readFileSync(packageJsonPath, 'utf8').replace(/^\uFEFF/, '')); + if (content.dependencies !== undefined) { + addDependencies(result, content.dependencies); + } + if (content.devDependencies !== undefined) { + addDependencies(result, content.devDependencies); + } + if (content.peerDependencies !== undefined) { + addDependencies(result, content.peerDependencies); + } + } catch {} + return result; +} + function addDependencies(result: Set, dependencies: any) { Object.keys(dependencies).forEach(name => result.add(name.startsWith(DefinitelyTyped) ? name.substring(DefinitelyTyped.length) : name), diff --git a/eslint-bridge/tests/fixtures/no-implicit-dependencies/nested-package-json-project/dir/file.js b/eslint-bridge/tests/fixtures/no-implicit-dependencies/nested-package-json-project/dir/file.js new file mode 100644 index 00000000000..e69de29bb2d diff --git a/eslint-bridge/tests/fixtures/no-implicit-dependencies/nested-package-json-project/dir/package.json b/eslint-bridge/tests/fixtures/no-implicit-dependencies/nested-package-json-project/dir/package.json new file mode 100644 index 00000000000..08c274dd60a --- /dev/null +++ b/eslint-bridge/tests/fixtures/no-implicit-dependencies/nested-package-json-project/dir/package.json @@ -0,0 +1,5 @@ +{ + "dependencies": { + "nested-dependency": "latest" + } +} diff --git a/eslint-bridge/tests/fixtures/no-implicit-dependencies/nested-package-json-project/package.json b/eslint-bridge/tests/fixtures/no-implicit-dependencies/nested-package-json-project/package.json new file mode 100644 index 00000000000..6ff48ecd566 --- /dev/null +++ b/eslint-bridge/tests/fixtures/no-implicit-dependencies/nested-package-json-project/package.json @@ -0,0 +1,5 @@ +{ + "dependencies": { + "top-dependency": "latest" + } +} diff --git a/eslint-bridge/tests/rules/no-implicit-dependencies.test.ts b/eslint-bridge/tests/rules/no-implicit-dependencies.test.ts index 2f913372118..3e580cdc220 100644 --- a/eslint-bridge/tests/rules/no-implicit-dependencies.test.ts +++ b/eslint-bridge/tests/rules/no-implicit-dependencies.test.ts @@ -160,6 +160,37 @@ ruleTester.run('Dependencies should be explicit', rule, { ], }); +const ruleTesterNestedPackage = new RuleTester({ + parser: tsParserPath, + parserOptions: { ecmaVersion: 2018, sourceType: 'module' }, +}); + +const filenameNestedPackage = path.join( + __dirname, + '../fixtures/no-implicit-dependencies/nested-package-json-project/dir/file.js', +); + +ruleTesterNestedPackage.run('all levels of package.json should be considered', rule, { + valid: [ + { + code: ` + import { f as f1 } from 'top-dependency'; + import { f as f2 } from 'nested-dependency'; + `, + filename: filenameNestedPackage, + }, + ], + invalid: [ + { + code: ` + import { f as f1 } from 'nonexistent'; + `, + filename: filenameNestedPackage, + errors: 1, + }, + ], +}); + const ruleTesterForPathMappings = new RuleTester({ parser: require.resolve('@typescript-eslint/parser'), parserOptions: { From d5b8c13afba47d27a9d18fbd5f113960c543a544 Mon Sep 17 00:00:00 2001 From: "vilchik.elena" Date: Fri, 8 Apr 2022 11:58:53 +0200 Subject: [PATCH 2/5] Cache the computation --- .../src/rules/no-implicit-dependencies.ts | 65 +++++++++---------- 1 file changed, 29 insertions(+), 36 deletions(-) diff --git a/eslint-bridge/src/rules/no-implicit-dependencies.ts b/eslint-bridge/src/rules/no-implicit-dependencies.ts index 3e0938793eb..8cae63584ad 100644 --- a/eslint-bridge/src/rules/no-implicit-dependencies.ts +++ b/eslint-bridge/src/rules/no-implicit-dependencies.ts @@ -30,7 +30,12 @@ import { RequiredParserServices } from '../utils'; const DefinitelyTyped = '@types/'; /** - * Cache for each dirname the dependencies of the nearest package.json. + * Cache for each dirname the dependencies of the package.json in this directory, empty set when no package.json. + */ +const dirCache: Map> = new Map(); + +/** + * Cache for the available dependencies by dirname. */ const cache: Map> = new Map(); @@ -146,33 +151,36 @@ function getPackageName(name: string) { } function getDependencies(fileName: string) { + let dirname = path.dirname(fileName); + if (cache.get(dirname)) { + return cache.get(dirname); + } + const result = new Set(); + cache.set(dirname, result); - let dirname = path.dirname(fileName); + let current: string; while (true) { - getDependenciesFromDir(dirname).forEach(d => result.add(d)); - const upperDirname = path.dirname(dirname); - if (upperDirname === dirname) { - break; + const cached = dirCache.get(dirname); + if (cached) { + cached.forEach(d => result.add(d)); + } else { + const packageJsonPath = path.join(path.resolve(dirname), 'package.json'); + const dep = fs.existsSync(packageJsonPath) + ? getDependenciesFromPackageJson(packageJsonPath) + : new Set(); + dep.forEach(d => result.add(d)); + dirCache.set(dirname, dep); } - dirname = upperDirname; - } - - return result; -} -function getDependenciesFromDir(dirname: string) { - const cached = cache.get(dirname); - if (cached) { - return cached; + current = path.dirname(dirname); + if (current === dirname) { + break; + } else { + dirname = current; + } } - const packageJsonPath = findPackageJson(path.resolve(dirname)); - const result = packageJsonPath - ? getDependenciesFromPackageJson(packageJsonPath) - : new Set(); - cache.set(dirname, result); - return result; } @@ -200,21 +208,6 @@ function addDependencies(result: Set, dependencies: any) { ); } -function findPackageJson(current: string): string | undefined { - const fileName = path.join(current, 'package.json'); - if (fs.existsSync(fileName)) { - return fileName; - } - - const prev: string = current; - current = path.dirname(current); - - if (prev !== current) { - return findPackageJson(current); - } - return undefined; -} - /** * The matching pattern part of a path mapping specified * in `paths` in `tsconfig.json`. From 4a7fb257e4043c64553da7fb949b525d90f08fbc Mon Sep 17 00:00:00 2001 From: "vilchik.elena" Date: Fri, 8 Apr 2022 12:04:18 +0200 Subject: [PATCH 3/5] minor fixes --- .../src/rules/no-implicit-dependencies.ts | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/eslint-bridge/src/rules/no-implicit-dependencies.ts b/eslint-bridge/src/rules/no-implicit-dependencies.ts index 8cae63584ad..060f4926cd9 100644 --- a/eslint-bridge/src/rules/no-implicit-dependencies.ts +++ b/eslint-bridge/src/rules/no-implicit-dependencies.ts @@ -152,18 +152,18 @@ function getPackageName(name: string) { function getDependencies(fileName: string) { let dirname = path.dirname(fileName); - if (cache.get(dirname)) { - return cache.get(dirname); + const cached = cache.get(dirname); + if (cached) { + return cached; } const result = new Set(); cache.set(dirname, result); - let current: string; while (true) { - const cached = dirCache.get(dirname); - if (cached) { - cached.forEach(d => result.add(d)); + const dirCached = dirCache.get(dirname); + if (dirCached) { + dirCached.forEach(d => result.add(d)); } else { const packageJsonPath = path.join(path.resolve(dirname), 'package.json'); const dep = fs.existsSync(packageJsonPath) @@ -173,11 +173,11 @@ function getDependencies(fileName: string) { dirCache.set(dirname, dep); } - current = path.dirname(dirname); - if (current === dirname) { + const upperDir = path.dirname(dirname); + if (upperDir === dirname) { break; } else { - dirname = current; + dirname = upperDir; } } From 455f049b1995c51927d28c53e566c0d38fb85370 Mon Sep 17 00:00:00 2001 From: "vilchik.elena" Date: Mon, 11 Apr 2022 10:36:05 +0200 Subject: [PATCH 4/5] Update ruling --- .../expected/ts/desktop/typescript-S4328.json | 74 ------------------- 1 file changed, 74 deletions(-) delete mode 100644 its/ruling/src/test/expected/ts/desktop/typescript-S4328.json diff --git a/its/ruling/src/test/expected/ts/desktop/typescript-S4328.json b/its/ruling/src/test/expected/ts/desktop/typescript-S4328.json deleted file mode 100644 index da39cca95d7..00000000000 --- a/its/ruling/src/test/expected/ts/desktop/typescript-S4328.json +++ /dev/null @@ -1,74 +0,0 @@ -{ -'desktop:app/src/lib/dispatcher/app-shell.ts':[ -1, -], -'desktop:app/src/lib/dispatcher/app-store.ts':[ -2, -], -'desktop:app/src/lib/dispatcher/dispatcher.ts':[ -3, -], -'desktop:app/src/lib/logging/logger.ts':[ -1, -], -'desktop:app/src/lib/oauth.ts':[ -1, -], -'desktop:app/src/lib/open-file.ts':[ -1, -], -'desktop:app/src/main-process/app-window.ts':[ -1, -], -'desktop:app/src/main-process/error-page.ts':[ -2, -], -'desktop:app/src/main-process/main.ts':[ -1, -], -'desktop:app/src/main-process/menu/build-default-menu.ts':[ -2, -], -'desktop:app/src/main-process/squirrel-updater.ts':[ -1, -], -'desktop:app/src/shared-process/communication.ts':[ -1, -], -'desktop:app/src/shared-process/shared-process.ts':[ -1, -], -'desktop:app/src/ui/add-repository/add-existing-repository.tsx':[ -1, -], -'desktop:app/src/ui/add-repository/clone-repository.tsx':[ -1, -], -'desktop:app/src/ui/add-repository/create-repository.tsx':[ -1, -], -'desktop:app/src/ui/app-error.tsx':[ -12, -], -'desktop:app/src/ui/app.tsx':[ -2, -], -'desktop:app/src/ui/index.tsx':[ -5, -], -'desktop:app/src/ui/lib/app-proxy.ts':[ -1, -], -'desktop:app/src/ui/lib/link-button.tsx':[ -2, -], -'desktop:app/src/ui/lib/update-store.ts':[ -1, -], -'desktop:app/src/ui/main-process-proxy.ts':[ -1, -], -'desktop:app/src/ui/window/window-controls.tsx':[ -2, -], -} From 1fb3f67b173e23a2d22b652c94d14b63e50826e0 Mon Sep 17 00:00:00 2001 From: "vilchik.elena" Date: Mon, 11 Apr 2022 11:36:46 +0200 Subject: [PATCH 5/5] Empty commit