From 59f1ce7979e1ff7d984cf73e58e80184cc41d718 Mon Sep 17 00:00:00 2001 From: Guy Carmeli Date: Tue, 5 Nov 2024 10:23:25 +0200 Subject: [PATCH] Look for dependencies in super class (#189) This PR fixes an issue in the unresolved-dependencies ESLint rule. If a graph inherited from another graph, dependencies in the super class were not accounted for. --- packages/eslint-plugin-obsidian/src/dto/class.ts | 7 ++++++- .../eslint-plugin-obsidian/src/dto/classFile.ts | 4 ++++ packages/eslint-plugin-obsidian/src/dto/file.ts | 15 +++++++++++++++ .../classResolver.ts | 11 +++++++++++ .../unresolvedProviderDependencies/createRule.ts | 6 +++++- .../dependencyResolver.ts | 16 +++++++++++++++- .../tests/stubs/PathResolverStub.ts | 2 ++ .../fixtures/abstractGraph.ts | 8 ++++++++ .../fixtures/validGraphs.ts | 13 +++++++++++++ .../unresolvedDependencies.test.ts | 2 ++ 10 files changed, 81 insertions(+), 3 deletions(-) create mode 100644 packages/eslint-plugin-obsidian/src/rules/unresolvedProviderDependencies/classResolver.ts create mode 100644 packages/eslint-plugin-obsidian/tests/unresolvedProviderDependencies/fixtures/abstractGraph.ts diff --git a/packages/eslint-plugin-obsidian/src/dto/class.ts b/packages/eslint-plugin-obsidian/src/dto/class.ts index 0c7f75cd..3b0b6c06 100644 --- a/packages/eslint-plugin-obsidian/src/dto/class.ts +++ b/packages/eslint-plugin-obsidian/src/dto/class.ts @@ -3,6 +3,7 @@ import { Decorator } from './decorator'; import { assertDefined } from '../utils/assertions'; import { isMethodDefinition } from '../utils/ast'; import { Method } from './method'; +import { Identifier } from './identifier'; export class Clazz { constructor(public node: TSESTree.ClassDeclaration) { @@ -21,6 +22,10 @@ export class Clazz { }); } + get superClass() { + return this.node.superClass && new Identifier(this.node.superClass).name; + } + get body() { return this.node.body.body; } @@ -40,7 +45,7 @@ export class Clazz { return decorator; } - private get name() { + public get name() { return this.node.id?.name; } } \ No newline at end of file diff --git a/packages/eslint-plugin-obsidian/src/dto/classFile.ts b/packages/eslint-plugin-obsidian/src/dto/classFile.ts index 8d13f0e6..afa0199c 100644 --- a/packages/eslint-plugin-obsidian/src/dto/classFile.ts +++ b/packages/eslint-plugin-obsidian/src/dto/classFile.ts @@ -8,4 +8,8 @@ export class ClassFile { public readonly imports: Import[], public readonly path: string, ) {} + + get superClass() { + return this.clazz.superClass; + } } diff --git a/packages/eslint-plugin-obsidian/src/dto/file.ts b/packages/eslint-plugin-obsidian/src/dto/file.ts index 1ed6cf20..1feba423 100644 --- a/packages/eslint-plugin-obsidian/src/dto/file.ts +++ b/packages/eslint-plugin-obsidian/src/dto/file.ts @@ -57,6 +57,21 @@ export class File { }) as Clazz[]; } + findClass(byName: string) { + const clazz = this.classes.find(($clazz) => $clazz.name === byName); + return clazz && new ClassFile(clazz, this.imports, this.path); + } + + private get classes() { + return this.body + .filter(isClassLike) + .map((node) => { + const clazz = getClassDeclaration(node); + return clazz && new Clazz(clazz); + }) + .filter(Boolean) as Clazz[]; + } + get variables() { return this.body .filter(isVariableDeclaration) diff --git a/packages/eslint-plugin-obsidian/src/rules/unresolvedProviderDependencies/classResolver.ts b/packages/eslint-plugin-obsidian/src/rules/unresolvedProviderDependencies/classResolver.ts new file mode 100644 index 00000000..2505fd5c --- /dev/null +++ b/packages/eslint-plugin-obsidian/src/rules/unresolvedProviderDependencies/classResolver.ts @@ -0,0 +1,11 @@ +import type { ClassFile } from '../../dto/classFile'; +import type { FileReader } from '../../framework/fileReader'; + +export class ClassResolver { + constructor(private fileReader: FileReader) { } + + public resolve(clazz: string, from: ClassFile) { + const classPath = from.imports.find(($import) => $import.includes(clazz)); + return classPath && this.fileReader.read(from.path, classPath.path).findClass(clazz); + } +} \ No newline at end of file diff --git a/packages/eslint-plugin-obsidian/src/rules/unresolvedProviderDependencies/createRule.ts b/packages/eslint-plugin-obsidian/src/rules/unresolvedProviderDependencies/createRule.ts index 6a73eed5..e9f0d007 100644 --- a/packages/eslint-plugin-obsidian/src/rules/unresolvedProviderDependencies/createRule.ts +++ b/packages/eslint-plugin-obsidian/src/rules/unresolvedProviderDependencies/createRule.ts @@ -6,10 +6,14 @@ import { FileReader } from '../../framework/fileReader'; import { DependencyResolver } from './dependencyResolver'; import { Import } from '../../dto/import'; import { SubgraphResolver } from './subgraphResolver'; +import { ClassResolver } from './classResolver'; export function create(context: Context, fileReader: FileReader) { const imports: Import[] = []; - const dependencyResolver = new DependencyResolver(new SubgraphResolver(fileReader)); + const dependencyResolver = new DependencyResolver( + new SubgraphResolver(fileReader), + new ClassResolver(fileReader), + ); const graphHandler = new GraphHandler(context, dependencyResolver); return { diff --git a/packages/eslint-plugin-obsidian/src/rules/unresolvedProviderDependencies/dependencyResolver.ts b/packages/eslint-plugin-obsidian/src/rules/unresolvedProviderDependencies/dependencyResolver.ts index c5031e47..025a4145 100644 --- a/packages/eslint-plugin-obsidian/src/rules/unresolvedProviderDependencies/dependencyResolver.ts +++ b/packages/eslint-plugin-obsidian/src/rules/unresolvedProviderDependencies/dependencyResolver.ts @@ -3,15 +3,20 @@ import type { Import } from '../../dto/import'; import type { Clazz } from '../../dto/class'; import type { SubgraphResolver } from './subgraphResolver'; import type { Context } from '../../dto/context'; +import type { ClassResolver } from './classResolver'; export class DependencyResolver { - constructor(private subgraphResolver: SubgraphResolver) { } + constructor( + private subgraphResolver: SubgraphResolver, + private classResolver: ClassResolver, + ) { } public resolve(context: Context, clazz: Clazz, imports: Import[]) { const classWithImports = new ClassFile(clazz, imports, context.currentFilePath); return [ ...this.getGraphDependencies(classWithImports), ...this.getDependenciesFromSubgraphs(classWithImports), + ...this.getDependenciesFromSuperClass(classWithImports), ]; } @@ -26,4 +31,13 @@ export class DependencyResolver { .getDecoratedMethods('Provides') .map((method) => method.name); } + + private getDependenciesFromSuperClass(clazz: ClassFile) { + if (!clazz.superClass || clazz.superClass === 'ObjectGraph') return []; + return this.classResolver + .resolve(clazz.superClass, clazz) + ?.clazz + ?.getDecoratedMethods('Provides') + ?.map((method) => method.name) ?? []; + } } \ No newline at end of file diff --git a/packages/eslint-plugin-obsidian/tests/stubs/PathResolverStub.ts b/packages/eslint-plugin-obsidian/tests/stubs/PathResolverStub.ts index 2a0e73ab..7c83faa1 100644 --- a/packages/eslint-plugin-obsidian/tests/stubs/PathResolverStub.ts +++ b/packages/eslint-plugin-obsidian/tests/stubs/PathResolverStub.ts @@ -11,6 +11,8 @@ export class PathResolverStub implements PathResolver { return `${cwd}/tests/unresolvedProviderDependencies/fixtures/graphWithSubgraph.ts`; case './namedExportSubgraph': return `${cwd}/tests/unresolvedProviderDependencies/fixtures/namedExportSubgraph.ts`; + case './abstractGraph': + return `${cwd}/tests/unresolvedProviderDependencies/fixtures/abstractGraph.ts`; default: throw new Error(`PathResolverStub: Unhandled relativeFilePath: ${relativeFilePath}`); } diff --git a/packages/eslint-plugin-obsidian/tests/unresolvedProviderDependencies/fixtures/abstractGraph.ts b/packages/eslint-plugin-obsidian/tests/unresolvedProviderDependencies/fixtures/abstractGraph.ts new file mode 100644 index 00000000..b712764f --- /dev/null +++ b/packages/eslint-plugin-obsidian/tests/unresolvedProviderDependencies/fixtures/abstractGraph.ts @@ -0,0 +1,8 @@ +import { ObjectGraph, Provides } from "react-obsidian"; + +export abstract class AbstractGraph extends ObjectGraph { + @Provides() + bar(): string { + return "bar"; + } +} diff --git a/packages/eslint-plugin-obsidian/tests/unresolvedProviderDependencies/fixtures/validGraphs.ts b/packages/eslint-plugin-obsidian/tests/unresolvedProviderDependencies/fixtures/validGraphs.ts index 610fc8c9..e59da2a7 100644 --- a/packages/eslint-plugin-obsidian/tests/unresolvedProviderDependencies/fixtures/validGraphs.ts +++ b/packages/eslint-plugin-obsidian/tests/unresolvedProviderDependencies/fixtures/validGraphs.ts @@ -110,3 +110,16 @@ export default class SimpleGraph extends ObjectGraph { return 'foo'; } }`; + +export const validGraphThatExtendsAnotherGraph = ` +import { Graph, ObjectGraph, Provides } from 'src'; +import {AbstractGraph} from './abstractGraph'; + +@Graph() +export default class GraphA extends AbstractGraph { + @Provides() + foo(bar: string): string { + return 'foo' + bar; + } +} +`; diff --git a/packages/eslint-plugin-obsidian/tests/unresolvedProviderDependencies/unresolvedDependencies.test.ts b/packages/eslint-plugin-obsidian/tests/unresolvedProviderDependencies/unresolvedDependencies.test.ts index b39a0485..501e34a4 100644 --- a/packages/eslint-plugin-obsidian/tests/unresolvedProviderDependencies/unresolvedDependencies.test.ts +++ b/packages/eslint-plugin-obsidian/tests/unresolvedProviderDependencies/unresolvedDependencies.test.ts @@ -4,6 +4,7 @@ import { PathResolverStub } from '../stubs/PathResolverStub'; import { validFileWithTwoGraphs, validGraph, + validGraphThatExtendsAnotherGraph, validGraphWithNamedExportSubgraph, validGraphWithNestedSubgraphs, validGraphWithRegularMethod, @@ -26,6 +27,7 @@ ruleTester.run( validGraphWithNamedExportSubgraph, validGraphWithRegularMethod, validGraphWithNestedSubgraphs, + validGraphThatExtendsAnotherGraph, ], invalid: [ {