Skip to content

Commit

Permalink
Look for dependencies in super class (#189)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
guyca authored Nov 5, 2024
1 parent 231846c commit 59f1ce7
Show file tree
Hide file tree
Showing 10 changed files with 81 additions and 3 deletions.
7 changes: 6 additions & 1 deletion packages/eslint-plugin-obsidian/src/dto/class.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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;
}
Expand All @@ -40,7 +45,7 @@ export class Clazz {
return decorator;
}

private get name() {
public get name() {
return this.node.id?.name;
}
}
4 changes: 4 additions & 0 deletions packages/eslint-plugin-obsidian/src/dto/classFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,8 @@ export class ClassFile {
public readonly imports: Import[],
public readonly path: string,
) {}

get superClass() {
return this.clazz.superClass;
}
}
15 changes: 15 additions & 0 deletions packages/eslint-plugin-obsidian/src/dto/file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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),
];
}

Expand All @@ -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) ?? [];
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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}`);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { ObjectGraph, Provides } from "react-obsidian";

export abstract class AbstractGraph extends ObjectGraph {
@Provides()
bar(): string {
return "bar";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
`;
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { PathResolverStub } from '../stubs/PathResolverStub';
import {
validFileWithTwoGraphs,
validGraph,
validGraphThatExtendsAnotherGraph,
validGraphWithNamedExportSubgraph,
validGraphWithNestedSubgraphs,
validGraphWithRegularMethod,
Expand All @@ -26,6 +27,7 @@ ruleTester.run(
validGraphWithNamedExportSubgraph,
validGraphWithRegularMethod,
validGraphWithNestedSubgraphs,
validGraphThatExtendsAnotherGraph,
],
invalid: [
{
Expand Down

0 comments on commit 59f1ce7

Please sign in to comment.