From abd03a70fbb6ebf31a017e1d1d29f85379cdf21b Mon Sep 17 00:00:00 2001 From: Elad Bezalel Date: Tue, 4 Feb 2025 17:44:23 +0200 Subject: [PATCH] fix(core): map non-source import references into actual usage (#36) * fix(core): map non-source import references into actual usage Imports are ignored by the reference find recurse, therefor we need to map non-source references (file imports, etc..) into actual usage in the file changed --- .../monorepo/non-source-proj/data.json | 1 + .../monorepo/non-source-proj/index.ts | 7 ++ .../monorepo/non-source-proj/tsconfig.json | 14 +++ .../src/__fixtures__/monorepo/proj3/file.ts | 6 ++ .../src/__fixtures__/monorepo/tsconfig.json | 3 + libs/core/src/true-affected.spec.ts | 60 +++++++++--- libs/core/src/true-affected.ts | 91 +++++++++++++++++-- libs/core/src/utils.spec.ts | 22 ++++- libs/core/src/utils.ts | 13 ++- 9 files changed, 196 insertions(+), 21 deletions(-) create mode 100644 libs/core/src/__fixtures__/monorepo/non-source-proj/data.json create mode 100644 libs/core/src/__fixtures__/monorepo/non-source-proj/index.ts create mode 100644 libs/core/src/__fixtures__/monorepo/non-source-proj/tsconfig.json create mode 100644 libs/core/src/__fixtures__/monorepo/proj3/file.ts diff --git a/libs/core/src/__fixtures__/monorepo/non-source-proj/data.json b/libs/core/src/__fixtures__/monorepo/non-source-proj/data.json new file mode 100644 index 0000000..0967ef4 --- /dev/null +++ b/libs/core/src/__fixtures__/monorepo/non-source-proj/data.json @@ -0,0 +1 @@ +{} diff --git a/libs/core/src/__fixtures__/monorepo/non-source-proj/index.ts b/libs/core/src/__fixtures__/monorepo/non-source-proj/index.ts new file mode 100644 index 0000000..a1fb4d1 --- /dev/null +++ b/libs/core/src/__fixtures__/monorepo/non-source-proj/index.ts @@ -0,0 +1,7 @@ +// eslint-disable-next-line @typescript-eslint/ban-ts-comment +// @ts-ignore-next-line +import data from './data.json'; + +export function nonSourceProj() { + return data; +} diff --git a/libs/core/src/__fixtures__/monorepo/non-source-proj/tsconfig.json b/libs/core/src/__fixtures__/monorepo/non-source-proj/tsconfig.json new file mode 100644 index 0000000..be5a230 --- /dev/null +++ b/libs/core/src/__fixtures__/monorepo/non-source-proj/tsconfig.json @@ -0,0 +1,14 @@ +{ + "extends": "../tsconfig.json", + "compilerOptions": { + "module": "commonjs", + "forceConsistentCasingInFileNames": true, + "strict": true, + "noImplicitOverride": true, + "noPropertyAccessFromIndexSignature": true, + "noImplicitReturns": true, + "noFallthroughCasesInSwitch": true + }, + "include": ["**/*.ts"], + "exclude": [] +} diff --git a/libs/core/src/__fixtures__/monorepo/proj3/file.ts b/libs/core/src/__fixtures__/monorepo/proj3/file.ts new file mode 100644 index 0000000..cf479c9 --- /dev/null +++ b/libs/core/src/__fixtures__/monorepo/proj3/file.ts @@ -0,0 +1,6 @@ +import { nonSourceProj } from '@monorepo/non-source-proj'; + +export function proj3() { + nonSourceProj(); + return 'proj3'; +} diff --git a/libs/core/src/__fixtures__/monorepo/tsconfig.json b/libs/core/src/__fixtures__/monorepo/tsconfig.json index f92bd3b..791d454 100644 --- a/libs/core/src/__fixtures__/monorepo/tsconfig.json +++ b/libs/core/src/__fixtures__/monorepo/tsconfig.json @@ -24,6 +24,9 @@ "@monorepo/proj3": [ "./proj3/index.ts" ], + "@monorepo/non-source-proj": [ + "./non-source-proj/index.ts" + ] } }, } diff --git a/libs/core/src/true-affected.spec.ts b/libs/core/src/true-affected.spec.ts index 25b8102..3153868 100644 --- a/libs/core/src/true-affected.spec.ts +++ b/libs/core/src/true-affected.spec.ts @@ -247,6 +247,46 @@ describe('trueAffected', () => { expect(affected).toEqual(['angular-component']); }); + it('should find files and usages that are related to changed files which are not in projects files', async () => { + jest.spyOn(git, 'getChangedFiles').mockReturnValue([ + { + filePath: 'non-source-proj/data.json', + changedLines: [0], + }, + ]); + + const affected = await trueAffected({ + cwd, + base: 'main', + rootTsConfig: 'tsconfig.json', + projects: [ + { + name: 'proj1', + sourceRoot: 'proj1/', + tsConfig: 'proj1/tsconfig.json', + }, + { + name: 'proj2', + sourceRoot: 'proj2/', + tsConfig: 'proj2/tsconfig.json', + }, + { + name: 'proj3', + sourceRoot: 'proj3/', + tsConfig: 'proj3/tsconfig.json', + implicitDependencies: ['proj1'], + }, + { + name: 'non-source-proj', + sourceRoot: 'non-source-proj/', + tsConfig: 'non-source-proj/tsconfig.json', + }, + ], + }); + + expect(affected).toEqual(['non-source-proj', 'proj3']); + }); + describe('__experimentalLockfileCheck', () => { it('should find files that are related to changed modules from lockfile if flag is on', async () => { jest.spyOn(git, 'getChangedFiles').mockReturnValue([ @@ -491,17 +531,11 @@ describe('trueAffected', () => { const compilerOptions = { paths: { - "@monorepo/proj1": [ - "./proj1/index.ts" - ], - "@monorepo/proj2": [ - "./proj2/index.ts" - ], - "@monorepo/proj3": [ - "./proj3/index.ts" - ], - } - } + '@monorepo/proj1': ['./proj1/index.ts'], + '@monorepo/proj2': ['./proj2/index.ts'], + '@monorepo/proj3': ['./proj3/index.ts'], + }, + }; const affected = await trueAffected({ cwd, @@ -523,9 +557,9 @@ describe('trueAffected', () => { tsConfig: 'proj3/tsconfig.json', }, ], - compilerOptions + compilerOptions, }); expect(affected).toEqual(['proj1']); - }) + }); }); diff --git a/libs/core/src/true-affected.ts b/libs/core/src/true-affected.ts index c6fd5d6..8ccbde2 100644 --- a/libs/core/src/true-affected.ts +++ b/libs/core/src/true-affected.ts @@ -3,7 +3,7 @@ import { join, resolve } from 'path'; import { Project, Node, ts, SyntaxKind } from 'ts-morph'; import chalk from 'chalk'; import { ChangedFiles, getChangedFiles } from './git'; -import { findRootNode, getPackageNameByPath } from './utils'; +import { findNodeAtLine, findRootNode, getPackageNameByPath } from './utils'; import { TrueAffected, TrueAffectedProject } from './types'; import { findNonSourceAffectedFiles } from './assets'; import { @@ -129,6 +129,75 @@ export const trueAffected = async ({ nonSourceChangedFiles.length )} non-source affected files` ); + + logger.debug( + 'Mapping non-source affected files imports to actual references' + ); + + nonSourceChangedFiles = nonSourceChangedFiles.flatMap( + ({ filePath, changedLines }) => { + const file = project.getSourceFile(resolve(cwd, filePath)); + /* istanbul ignore next */ + if (file == null) return []; + + return changedLines.reduce( + (acc, line) => { + const changedNode = findNodeAtLine(file, line); + const rootNode = findRootNode(changedNode); + /* istanbul ignore next */ + if (!rootNode) return acc; + + if (!rootNode.isKind(SyntaxKind.ImportDeclaration)) { + return { + ...acc, + changedLines: [...acc.changedLines, ...changedLines], + }; + } + + logger.debug( + `Found changed node ${chalk.bold( + rootNode?.getText() ?? 'undefined' + )} at line ${chalk.bold(line)} in ${chalk.bold(filePath)}` + ); + + const identifier = + rootNode.getFirstChildByKind(SyntaxKind.Identifier) ?? + rootNode.getFirstDescendantByKind(SyntaxKind.Identifier); + + /* istanbul ignore next */ + if (identifier == null) return acc; + + logger.debug( + `Found identifier ${chalk.bold( + identifier.getText() + )} in ${chalk.bold(filePath)}` + ); + + const refs = identifier.findReferencesAsNodes(); + + logger.debug( + `Found ${chalk.bold( + refs.length + )} references for identifier ${chalk.bold( + identifier.getText() + )}` + ); + + return { + ...acc, + changedLines: [ + ...acc.changedLines, + ...refs.map((node) => node.getStartLineNumber()), + ], + }; + }, + { + filePath, + changedLines: [], + } as ChangedFiles + ); + } + ); } } @@ -177,9 +246,21 @@ export const trueAffected = async ({ const rootNode = findRootNode(node); /* istanbul ignore next */ - if (rootNode == null) return; + if (rootNode == null) { + logger.debug( + `Could not find root node for ${chalk.bold(node.getText())}` + ); + return; + } - if (ignoredRootNodeTypes.find((type) => rootNode.isKind(type))) return; + if (ignoredRootNodeTypes.find((type) => rootNode.isKind(type))) { + logger.debug( + `Ignoring root node ${chalk.bold( + rootNode.getText() + )} of type ${chalk.bold(rootNode.getKindName())}` + ); + return; + } const identifier = rootNode.getFirstChildByKind(SyntaxKind.Identifier) ?? @@ -235,9 +316,7 @@ export const trueAffected = async ({ changedLines.forEach((line) => { try { - const lineStartPos = - sourceFile.compilerNode.getPositionOfLineAndCharacter(line - 1, 0); - const changedNode = sourceFile.getDescendantAtPos(lineStartPos); + const changedNode = findNodeAtLine(sourceFile, line); /* istanbul ignore next */ if (!changedNode) return; diff --git a/libs/core/src/utils.spec.ts b/libs/core/src/utils.spec.ts index 0c910bc..afb8900 100644 --- a/libs/core/src/utils.spec.ts +++ b/libs/core/src/utils.spec.ts @@ -1,4 +1,4 @@ -import { findRootNode, getPackageNameByPath } from './utils'; +import { findRootNode, getPackageNameByPath, findNodeAtLine } from './utils'; import { Project, SyntaxKind } from 'ts-morph'; describe('findRootNode', () => { @@ -52,3 +52,23 @@ describe('getPackageNameByPath', () => { expect(packageName).toBeUndefined(); }); }); + +describe('findNodeAtLine', () => { + it('should find the node at line', () => { + const project = new Project({ useInMemoryFileSystem: true }); + + const file = project.createSourceFile( + 'file.ts', + `import { bar } from 'bar'; + export const foo = 1;` + ); + + const importNode = findNodeAtLine(file, 1); + + expect(importNode?.getKind()).toEqual(SyntaxKind.ImportKeyword); + + const exportNode = findNodeAtLine(file, 2); + + expect(exportNode?.getKind()).toEqual(SyntaxKind.ExportKeyword); + }); +}); diff --git a/libs/core/src/utils.ts b/libs/core/src/utils.ts index 3a201c7..4cd9b1e 100644 --- a/libs/core/src/utils.ts +++ b/libs/core/src/utils.ts @@ -1,4 +1,4 @@ -import { ts, SyntaxKind, Node } from 'ts-morph'; +import { ts, SyntaxKind, Node, SourceFile } from 'ts-morph'; import { TrueAffectedProject } from './types'; export const findRootNode = ( @@ -16,3 +16,14 @@ export const getPackageNameByPath = ( ): string | undefined => { return projects.find(({ sourceRoot }) => path.includes(sourceRoot))?.name; }; + +export const findNodeAtLine = ( + sourceFile: SourceFile, + line: number +): Node | undefined => { + const lineStartPos = sourceFile.compilerNode.getPositionOfLineAndCharacter( + line - 1, + 0 + ); + return sourceFile.getDescendantAtPos(lineStartPos); +};