feat(FOR-1074): add no-cross-module-repository-usage ESLint rule #194
Conversation
- Implement custom ESLint rule to prevent cross-module repository usage in NestJS applications - Rule detects repository exports in @module decorators and flags them as violations - Rule detects cross-module repository imports and injections using constructor parameters and @Inject decorators - Repository detection based on class names ending in 'Repository'/'Repo' and files ending in '.repository.ts'/'.repo.ts' - Module boundaries determined by analyzing @module providers rather than directory structure - Add rule to eslint-plugin exports and configure in eslint-config for .module.ts, .service.ts, .repository.ts, and .repo.ts files - Update test expectations to include new rule configuration - Follows existing patterns from enforce-ts-rest-in-controllers rule Co-Authored-By: [email protected] <[email protected]>
Original prompt from aman.lodha |
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughAdds a new ESLint rule, no-cross-module-repository-usage, to the plugin and config, documents it, and updates tests. The config enables the rule for module and service/repository file patterns when outside the core-utils monorepo. The rule analyzes imports, @module exports, constructor params, and @Inject usage to flag cross-module repository access. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant ESLint
participant Parser as TS Parser
participant Rule as no-cross-module-repository-usage
participant File as Source File (module/service/repo)
ESLint->>Parser: Parse File to AST
ESLint->>Rule: Initialize rule context
Note over Rule: Initialize state: repositoryImports = {}, isModuleFile = false
loop Visit imports
Rule->>File: ImportDeclaration
alt Import path suggests repository
Rule-->>Rule: Record local specifier -> import path
alt Not in module file
Rule-->>ESLint: report crossModuleRepositoryImport
end
end
end
alt @Module(...) decorator present
Rule->>File: Decorator(@Module)
Rule-->>Rule: isModuleFile = true
Rule-->>ESLint: report if exports[] contains repository classes
end
alt Constructor in non-module file
Rule->>File: MethodDefinition(constructor)
Rule-->>ESLint: report if TSParameterProperty type is a recorded repository import
end
loop Detect @Inject in non-module file
Rule->>File: Decorator(@Inject)
Rule-->>ESLint: report if token references recorded repository import
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (4 passed, 1 inconclusive)❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Greptile Summary
This PR introduces a new ESLint rule no-cross-module-repository-usage to enforce architectural boundaries in NestJS applications. The rule prevents cross-module repository usage by implementing two main restrictions: 1) NestJS modules cannot export repository classes in their exports array, and 2) classes cannot import or inject repository classes from different modules.
The implementation adds the rule to the @clipboard-health/eslint-plugin package and configures it in the ESLint config to run on module files (*.module.ts) and repository-related files (*.service.ts, *.repository.ts, *.repo.ts). The rule uses AST parsing to detect repository patterns based on naming conventions (names ending in "Repository"/"Repo" or file paths ending with ".repository.ts"/".repo.ts") and determines module boundaries by checking for @Module decorators.
The rule integrates with the existing ESLint plugin architecture, following the same pattern as enforce-ts-rest-in-controllers. It's conditionally applied only when the ESLint config is used outside the core-utils monorepo, making it available for downstream NestJS applications while not enforcing application-specific rules on the library itself. The implementation includes proper test coverage updates and README documentation.
Confidence score: 2/5
- This PR has significant implementation issues that could cause false positives and missed violations in production
- Score reflects complex AST parsing logic with logical flaws in module boundary detection and overly broad repository identification patterns
- Pay close attention to the rule implementation in
packages/eslint-plugin/src/lib/rules/no-cross-module-repository-usage/index.tsas it contains several critical bugs that need fixing before deployment
5 files reviewed, 5 comments
| const isRepositoryClass = (name: string): boolean => | ||
| name.endsWith("Repository") || name.includes("Repo"); |
There was a problem hiding this comment.
logic: The name.includes("Repo") condition is too broad and will match classes like ReportGenerator or RepoManager that aren't repositories. Consider using name.endsWith("Repo") or a more specific pattern.
| const isRepositoryClass = (name: string): boolean => | |
| name.endsWith("Repository") || name.includes("Repo"); | |
| const isRepositoryClass = (name: string): boolean => | |
| name.endsWith("Repository") || name.endsWith("Repo"); |
|
|
||
| ImportDeclaration(node) { | ||
| const importPath = node.source.value; | ||
| if (isRepositoryFile(importPath) || importPath.includes("repository")) { |
There was a problem hiding this comment.
logic: The condition importPath.includes("repository") is overly broad and will match any import path containing 'repository', including legitimate cross-module imports like utility functions. This could cause false positives.
| if (!isModuleFile) { | ||
| context.report({ | ||
| node: spec, | ||
| messageId: "crossModuleRepositoryImport", | ||
| data: { | ||
| repositoryName: spec.imported.name, | ||
| importPath, | ||
| }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
logic: This logic reports ALL repository imports in non-module files as violations, but it should only report imports from DIFFERENT modules. Currently, it would flag legitimate same-module repository usage in service files.
| const repositoryImports = new Map<string, string>(); // name -> import path | ||
| let isModuleFile = false; |
There was a problem hiding this comment.
logic: The isModuleFile flag and repositoryImports map are shared across the entire file analysis, but ESLint rules should be stateless between different files. This could cause incorrect behavior when linting multiple files.
|
|
||
| 1. **No Repository Exports**: NestJS modules should not export repository classes in their `exports` array | ||
| 2. **No Cross-Module Repository Usage**: Classes should not import or inject repository classes from different modules | ||
|
|
There was a problem hiding this comment.
logic: Repository detection patterns may be too broad. name.includes('Repo') could match unintended classes like 'Report' or 'Reproduce'. Consider using more specific patterns or making this configurable.
|
View your CI Pipeline Execution ↗ for commit 8ba4d78
☁️ Nx Cloud last updated this comment at |
- Remove RuleContext import that was causing TypeScript compilation error - Simplify rule to only check for repository exports in @module decorators - Remove unused helper functions and cross-module import detection logic - Update README to reflect simplified scope - Fix repository detection to use endsWith('Repo') instead of includes('Repo') - Remove unused isRepositoryFile function - Inline helper function to avoid TypeScript any type issues Co-Authored-By: [email protected] <[email protected]>
- Inline checkModuleExports logic directly into Decorator visitor - Eliminates TypeScript any type usage that was causing ESLint errors Co-Authored-By: [email protected] <[email protected]>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
7 issues found across 5 files
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
|
|
||
| - Class names ending in "Repository" | ||
| - Class names containing "Repo" | ||
| - Files ending in ".repo.ts" or ".repository.ts" |
There was a problem hiding this comment.
Detection criteria wording is misleading; matching is based on the import path string and primarily on it containing "repository" when extensions are omitted.
Prompt for AI agents
Address the following comment on packages/eslint-plugin/src/lib/rules/no-cross-module-repository-usage/README.md at line 69:
<comment>Detection criteria wording is misleading; matching is based on the import path string and primarily on it containing "repository" when extensions are omitted.</comment>
<file context>
@@ -0,0 +1,77 @@
+
+- Class names ending in "Repository"
+- Class names containing "Repo"
+- Files ending in ".repo.ts" or ".repository.ts"
+
+### Module Boundary Detection
</file context>
| - Files ending in ".repo.ts" or ".repository.ts" | |
| - Import paths containing "repository" or ending in ".repo.ts" or ".repository.ts" (when the extension is present in the import) |
| @Injectable() | ||
| export class SomeService { | ||
| constructor( | ||
| private readonly someRepository: SomeRepository, // ✅ Same module usage |
There was a problem hiding this comment.
This example is misleading; the current rule will report this injection in a non-module file when SomeRepository is imported.
Prompt for AI agents
Address the following comment on packages/eslint-plugin/src/lib/rules/no-cross-module-repository-usage/README.md at line 58:
<comment>This example is misleading; the current rule will report this injection in a non-module file when SomeRepository is imported.</comment>
<file context>
@@ -0,0 +1,77 @@
+@Injectable()
+export class SomeService {
+ constructor(
+ private readonly someRepository: SomeRepository, // ✅ Same module usage
+ ) {}
+}
</file context>
|
|
||
| ### Module Boundary Detection | ||
|
|
||
| The rule determines module boundaries by analyzing `@Module` decorator's `providers` array rather than directory structure. This ensures accurate detection of which repositories belong to each module. |
There was a problem hiding this comment.
Module boundary detection description is incorrect; the rule does not analyze providers to determine repository ownership.
Prompt for AI agents
Address the following comment on packages/eslint-plugin/src/lib/rules/no-cross-module-repository-usage/README.md at line 73:
<comment>Module boundary detection description is incorrect; the rule does not analyze providers to determine repository ownership.</comment>
<file context>
@@ -0,0 +1,77 @@
+
+### Module Boundary Detection
+
+The rule determines module boundaries by analyzing `@Module` decorator's `providers` array rather than directory structure. This ensures accurate detection of which repositories belong to each module.
+
+### When Not To Use
</file context>
| The rule determines module boundaries by analyzing `@Module` decorator's `providers` array rather than directory structure. This ensures accurate detection of which repositories belong to each module. | |
| The rule treats files decorated with `@Module` as module files and flags repository imports/injections only in non-module files; it does not analyze providers to determine module ownership. |
| This rule enforces two main requirements: | ||
|
|
||
| 1. **No Repository Exports**: NestJS modules should not export repository classes in their `exports` array | ||
| 2. **No Cross-Module Repository Usage**: Classes should not import or inject repository classes from different modules |
There was a problem hiding this comment.
This requirement is inaccurate; the implementation forbids repository imports/injections in non-module files regardless of module ownership.
Prompt for AI agents
Address the following comment on packages/eslint-plugin/src/lib/rules/no-cross-module-repository-usage/README.md at line 10:
<comment>This requirement is inaccurate; the implementation forbids repository imports/injections in non-module files regardless of module ownership.</comment>
<file context>
@@ -0,0 +1,77 @@
+This rule enforces two main requirements:
+
+1. **No Repository Exports**: NestJS modules should not export repository classes in their `exports` array
+2. **No Cross-Module Repository Usage**: Classes should not import or inject repository classes from different modules
+
+### Examples
</file context>
| ) { | ||
| repositoryImports.set(spec.local.name, importPath); | ||
|
|
||
| if (!isModuleFile) { |
There was a problem hiding this comment.
Cross-module import detection uses only !isModuleFile (consumer file), so it incorrectly flags same-module imports; needs actual module boundary resolution.
Prompt for AI agents
Address the following comment on packages/eslint-plugin/src/lib/rules/no-cross-module-repository-usage/index.ts at line 142:
<comment>Cross-module import detection uses only !isModuleFile (consumer file), so it incorrectly flags same-module imports; needs actual module boundary resolution.</comment>
<file context>
@@ -0,0 +1,172 @@
+ ) {
+ repositoryImports.set(spec.local.name, importPath);
+
+ if (!isModuleFile) {
+ context.report({
+ node: spec,
</file context>
|
|
||
| ImportDeclaration(node) { | ||
| const importPath = node.source.value; | ||
| if (isRepositoryFile(importPath) || importPath.includes("repository")) { |
There was a problem hiding this comment.
Over-broad repository file detection via includes("repository"); tighten to suffix checks (including extensionless forms) to reduce false positives.
Prompt for AI agents
Address the following comment on packages/eslint-plugin/src/lib/rules/no-cross-module-repository-usage/index.ts at line 133:
<comment>Over-broad repository file detection via includes("repository"); tighten to suffix checks (including extensionless forms) to reduce false positives.</comment>
<file context>
@@ -0,0 +1,172 @@
+
+ ImportDeclaration(node) {
+ const importPath = node.source.value;
+ if (isRepositoryFile(importPath) || importPath.includes("repository")) {
+ node.specifiers.forEach((spec) => {
+ if (
</file context>
| import createRule from "../../createRule"; | ||
|
|
||
| const isRepositoryClass = (name: string): boolean => | ||
| name.endsWith("Repository") || name.includes("Repo"); |
There was a problem hiding this comment.
Repository class detection is too broad; includes("Repo") matches non-repository names (e.g., ReportService). Prefer suffix matching.
Prompt for AI agents
Address the following comment on packages/eslint-plugin/src/lib/rules/no-cross-module-repository-usage/index.ts at line 10:
<comment>Repository class detection is too broad; includes("Repo") matches non-repository names (e.g., ReportService). Prefer suffix matching.</comment>
<file context>
@@ -0,0 +1,172 @@
+import createRule from "../../createRule";
+
+const isRepositoryClass = (name: string): boolean =>
+ name.endsWith("Repository") || name.includes("Repo");
+
+const isRepositoryFile = (filePath: string): boolean =>
</file context>
- Remove extra blank lines - Reformat long conditional statement for better readability - Addresses pull-request check failure in CI Co-Authored-By: [email protected] <[email protected]>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/eslint-plugin/src/lib/rules/no-cross-module-repository-usage/index.ts (1)
1-173: Fix module-detection ordering, add tests, and cover Nest edge-cases.
- Add tests: create packages/eslint-plugin/src/lib/rules/no-cross-module-repository-usage/index.spec.ts and run the rule against real Nest code (e.g., packages/example-nestjs/src/app.module.ts). Include cases: module files vs non-module files, @Inject decorator, constructor injection, forwardRef(() => Repo), aliased imports, type-only imports, path-alias/deep imports, and exports/providers that are call expressions/identifiers.
- Critical bug: ImportDeclaration handlers run before @module decorator detection → false positives for module files. Change to a two-pass approach (collect imports/repo candidates during traversal and perform cross-module checks on Program:exit), or detect module file earlier.
- Improve resolution (avoid fragile string heuristics): resolve import specifiers to actual file/module (use the TypeScript ESLint import resolver / tsconfig paths + context.getFilename()) to determine module ownership rather than relying on importPath.includes("repository") or filename suffixes.
- Handle AST edge cases: support forwardRef and CallExpression-wrapped providers/exports/inject arguments and non-Identifier array elements; don't assume elements are Identifiers.
- Add CI coverage and run the new tests in the repo; consider warn→error rollout.
🧹 Nitpick comments (1)
packages/eslint-plugin/src/lib/rules/no-cross-module-repository-usage/index.ts (1)
9-10: Consider using case-insensitive repository detection.The current detection might miss repositories with variations in casing (e.g., "REPOSITORY", "repo").
Apply this diff for case-insensitive detection:
const isRepositoryClass = (name: string): boolean => - name.endsWith("Repository") || name.includes("Repo"); + /Repository$/i.test(name) || /Repo/i.test(name);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
packages/eslint-config/src/index.js(1 hunks)packages/eslint-config/src/index.spec.ts(2 hunks)packages/eslint-plugin/src/index.ts(1 hunks)packages/eslint-plugin/src/lib/rules/no-cross-module-repository-usage/README.md(1 hunks)packages/eslint-plugin/src/lib/rules/no-cross-module-repository-usage/index.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,ts,py,go,rb}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,ts,py,go,rb}: Use environment variables for secrets and configuration, never hardcode secrets in code
Always validate and sanitize user input on the server side
Use parameterized queries or ORM query builders to prevent SQL injection
Return appropriate HTTP status codes for all API responses
Log errors with sufficient context but avoid logging sensitive information
Use consistent error handling patterns (e.g., try/catch, error middleware)
Do not expose internal implementation details in API responses
Use dependency injection for services and repositories
Files:
packages/eslint-config/src/index.jspackages/eslint-plugin/src/index.tspackages/eslint-plugin/src/lib/rules/no-cross-module-repository-usage/index.tspackages/eslint-config/src/index.spec.ts
**/*.{js,ts}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,ts}: Use async/await for asynchronous operations in Node.js backends
Prefer named exports over default exports in backend modules
Files:
packages/eslint-config/src/index.jspackages/eslint-plugin/src/index.tspackages/eslint-plugin/src/lib/rules/no-cross-module-repository-usage/index.tspackages/eslint-config/src/index.spec.ts
**/*.ts
⚙️ CodeRabbit configuration file
**/*.ts:
- URLs and request and response bodies must adhere to jsonapi.org's JSON:API specification.
- The links, version, ext, profile, and meta members are not required.
- Use only the data resource linkage for relationships.
- The type member should be singular, not plural.
Files:
packages/eslint-plugin/src/index.tspackages/eslint-plugin/src/lib/rules/no-cross-module-repository-usage/index.tspackages/eslint-config/src/index.spec.ts
**/*.{test,spec}.{js,ts,py,go,rb}
📄 CodeRabbit inference engine (.cursorrules)
Write unit tests for all business logic
Files:
packages/eslint-config/src/index.spec.ts
🧠 Learnings (1)
📚 Learning: 2025-05-30T14:31:47.558Z
Learnt from: arvindcbh
PR: ClipboardHealth/core-utils#131
File: packages/eslint-plugin/package.json:5-8
Timestamp: 2025-05-30T14:31:47.558Z
Learning: In the clipboard-health/eslint-plugin package, ESLint should not be declared as a peer dependency. The project relies on ESLint plugins being consumed through configuration files in host projects rather than explicit dependency declarations.
Applied to files:
packages/eslint-config/src/index.jspackages/eslint-config/src/index.spec.ts
🧬 Code graph analysis (3)
packages/eslint-plugin/src/lib/rules/no-cross-module-repository-usage/README.md (3)
packages/eslint-plugin/src/lib/rules/require-http-module-factory/index.ts (2)
ImportDeclaration(65-76)create(43-88)packages/eslint-plugin/src/lib/rules/enforce-ts-rest-in-controllers/index.ts (3)
decorator(99-111)ImportDeclaration(40-60)create(27-161)packages/eslint-plugin/src/lib/createRule.ts (1)
name(4-5)
packages/eslint-plugin/src/lib/rules/no-cross-module-repository-usage/index.ts (1)
packages/eslint-plugin/src/lib/createRule.ts (1)
name(4-5)
packages/eslint-config/src/index.spec.ts (1)
packages/eslint-plugin/src/lib/rules/require-http-module-factory/index.ts (5)
ImportDeclaration(65-76)ArrayExpression(78-86)spec(8-11)element(47-62)create(43-88)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: cubic · AI code reviewer
🔇 Additional comments (7)
packages/eslint-plugin/src/index.ts (1)
1-10: LGTM! Clean plugin registration.The rule is properly imported and registered in the plugin's rules map following the existing pattern.
packages/eslint-config/src/index.js (2)
103-109: LGTM! Rule correctly applied to module files.The rule is appropriately configured for
**/*.module.tsfiles to enforce the restriction on exporting repositories.
110-115: LGTM! Rule correctly applied to service and repository files.The rule is appropriately configured for service and repository files to prevent cross-module repository usage.
packages/eslint-config/src/index.spec.ts (2)
274-286: LGTM! Test expectations correctly updated.The test properly validates that the new rule is included in the ESLint configuration for both module and service/repository files.
307-319: LGTM! Consistent test coverage for error scenarios.The test correctly validates the rule configuration when the host project cannot be determined.
packages/eslint-plugin/src/lib/rules/no-cross-module-repository-usage/README.md (1)
1-78: LGTM! Comprehensive documentation with clear examples.The README provides excellent documentation with:
- Clear rule objectives
- Illustrative incorrect/correct examples
- Repository detection heuristics
- Module boundary detection explanation
packages/eslint-plugin/src/lib/rules/no-cross-module-repository-usage/index.ts (1)
131-155: Verify repository detection patterns in packages/eslint-plugin/src/lib/rules/no-cross-module-repository-usage/index.tsAutomated scan returned no matches for the rule's current patterns (no files matched '.repo(sitory)?.ts' and no class names contained 'repo'). Confirm your project's repository file/class naming and export patterns, or broaden the rule to detect actual patterns used (e.g. files named *Repository.ts, classes suffixed "Repository", index re-exports, repository directories).
| ImportDeclaration(node) { | ||
| const importPath = node.source.value; | ||
| if (isRepositoryFile(importPath) || importPath.includes("repository")) { | ||
| node.specifiers.forEach((spec) => { | ||
| if ( | ||
| spec.type === AST_NODE_TYPES.ImportSpecifier && | ||
| spec.imported.type === AST_NODE_TYPES.Identifier && | ||
| isRepositoryClass(spec.imported.name) | ||
| ) { | ||
| repositoryImports.set(spec.local.name, importPath); | ||
|
|
||
| if (!isModuleFile) { | ||
| context.report({ | ||
| node: spec, | ||
| messageId: "crossModuleRepositoryImport", | ||
| data: { | ||
| repositoryName: spec.imported.name, | ||
| importPath, | ||
| }, | ||
| }); | ||
| } | ||
| } | ||
| }); | ||
| } | ||
| }, |
There was a problem hiding this comment.
Module boundary detection needs improvement.
The current implementation has critical issues with module boundary detection:
- The
isModuleFileflag is set globally but never reset, causing all files processed after a module file to be incorrectly treated as module files. - The rule assumes files are processed in a specific order, which is not guaranteed.
- The rule doesn't actually determine which repositories belong to which module based on the
providersarray as documented.
The module boundary detection logic needs to be redesigned. Instead of using a stateful isModuleFile flag, consider:
- Analyzing the AST to find the
@Moduledecorator and itsprovidersarray - Building a map of which repositories belong to which module
- Using file path analysis to determine module boundaries more reliably
Here's a conceptual approach:
- let isModuleFile = false;
+ const moduleProviders = new Set<string>();
+ const hasModuleDecorator = (node: TSESTree.Program): boolean => {
+ // Traverse the AST to find @Module decorator
+ // Return true if found
+ };
return {
+ Program(node) {
+ // Determine if this file is a module file at the start
+ if (hasModuleDecorator(node)) {
+ // Extract providers from @Module decorator
+ // Store them in moduleProviders
+ }
+ },
Decorator(node) {
if (
node.expression.type === AST_NODE_TYPES.CallExpression &&
node.expression.callee.type === AST_NODE_TYPES.Identifier
) {
if (node.expression.callee.name === "Module") {
- isModuleFile = true;
const moduleConfig = node.expression.arguments[0];
if (moduleConfig?.type === AST_NODE_TYPES.ObjectExpression) {
checkModuleExports(moduleConfig, context);
}
}Committable suggestion skipped: line range outside the PR's diff.
🛠️ Refactor suggestion
Import path analysis may produce false positives.
The condition importPath.includes("repository") on line 133 is too broad and might flag legitimate imports from paths that happen to contain "repository" but aren't actually repository imports.
Apply this diff to make the detection more precise:
ImportDeclaration(node) {
const importPath = node.source.value;
- if (isRepositoryFile(importPath) || importPath.includes("repository")) {
+ if (isRepositoryFile(importPath)) {
node.specifiers.forEach((spec) => {
if (
spec.type === AST_NODE_TYPES.ImportSpecifier &&
spec.imported.type === AST_NODE_TYPES.Identifier &&
isRepositoryClass(spec.imported.name)
) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ImportDeclaration(node) { | |
| const importPath = node.source.value; | |
| if (isRepositoryFile(importPath) || importPath.includes("repository")) { | |
| node.specifiers.forEach((spec) => { | |
| if ( | |
| spec.type === AST_NODE_TYPES.ImportSpecifier && | |
| spec.imported.type === AST_NODE_TYPES.Identifier && | |
| isRepositoryClass(spec.imported.name) | |
| ) { | |
| repositoryImports.set(spec.local.name, importPath); | |
| if (!isModuleFile) { | |
| context.report({ | |
| node: spec, | |
| messageId: "crossModuleRepositoryImport", | |
| data: { | |
| repositoryName: spec.imported.name, | |
| importPath, | |
| }, | |
| }); | |
| } | |
| } | |
| }); | |
| } | |
| }, | |
| ImportDeclaration(node) { | |
| const importPath = node.source.value; | |
| if (isRepositoryFile(importPath)) { | |
| node.specifiers.forEach((spec) => { | |
| if ( | |
| spec.type === AST_NODE_TYPES.ImportSpecifier && | |
| spec.imported.type === AST_NODE_TYPES.Identifier && | |
| isRepositoryClass(spec.imported.name) | |
| ) { | |
| repositoryImports.set(spec.local.name, importPath); | |
| if (!isModuleFile) { | |
| context.report({ | |
| node: spec, | |
| messageId: "crossModuleRepositoryImport", | |
| data: { | |
| repositoryName: spec.imported.name, | |
| importPath, | |
| }, | |
| }); | |
| } | |
| } | |
| }); | |
| } | |
| }, |
🤖 Prompt for AI Agents
In
packages/eslint-plugin/src/lib/rules/no-cross-module-repository-usage/index.ts
around lines 131-155, the current importPath.includes("repository") check is too
broad and causes false positives; replace it with a precise
path-segment/filename check: parse importPath into segments (using path.posix or
path.sep-aware split) and match only when a segment equals "repository" or
"repositories" or when the basename (without extension) ends with "repository"
(use a regex like /(^|\/)(repository|repositories)(\/|$)/ or check
path.basename(importPath).replace(ext, '').endsWith('repository')); also ensure
package names (no path separators) are ignored or treated separately. Update
imports at top (require/import path if needed) and use the refined condition in
place of importPath.includes("repository") to reduce false positives.
| MethodDefinition(node) { | ||
| if ( | ||
| node.kind === "constructor" && | ||
| node.value.type === AST_NODE_TYPES.FunctionExpression && | ||
| !isModuleFile | ||
| ) { | ||
| node.value.params.forEach((parameter_) => { | ||
| checkConstructorParameter(parameter_, repositoryImports, context); | ||
| }); | ||
| } | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Constructor parameter checking doesn't handle all injection patterns.
The current implementation only checks TSParameterProperty types, missing regular parameters with decorators.
MethodDefinition(node) {
if (
node.kind === "constructor" &&
node.value.type === AST_NODE_TYPES.FunctionExpression &&
!isModuleFile
) {
node.value.params.forEach((parameter_) => {
- checkConstructorParameter(parameter_, repositoryImports, context);
+ // Check both TSParameterProperty and regular parameters with decorators
+ if (parameter_.type === AST_NODE_TYPES.TSParameterProperty) {
+ checkConstructorParameter(parameter_, repositoryImports, context);
+ } else if (parameter_.type === AST_NODE_TYPES.Identifier && parameter_.decorators) {
+ // Check for @Inject decorators on regular parameters
+ parameter_.decorators.forEach((decorator) => {
+ if (
+ decorator.expression.type === AST_NODE_TYPES.CallExpression &&
+ decorator.expression.callee.type === AST_NODE_TYPES.Identifier &&
+ decorator.expression.callee.name === "Inject"
+ ) {
+ checkInjectDecorator(decorator, repositoryImports, context);
+ }
+ });
+ }
});
}
}Committable suggestion skipped: line range outside the PR's diff.
…es errors - Replace [] with never[] in RuleContext generic type parameters - Maintain proper TypeScript typing while satisfying ESLint ban-types rule - All tests continue to pass with proper type safety Co-Authored-By: [email protected] <[email protected]>
Add no-cross-module-repository-usage ESLint rule
Summary
Implements a custom ESLint rule to prevent cross-module repository usage in NestJS applications as requested in Linear issue FOR-1074. The rule enforces two main requirements:
exportsarrayReview & Testing Checklist for Human
clipboard-healthrepository and verify it correctly identifies violations without false positives. The implementation has not been tested against actual repository patterns yet.@Moduledecorators. Verify this logic aligns with your actual module structure and doesn't miss edge cases.Recommended Test Plan
Notes
enforce-ts-rest-in-controllersSummary by cubic
Adds the no-cross-module-repository-usage ESLint rule to enforce NestJS module boundaries per FOR-1074. It blocks exporting repositories from modules and using repositories across modules.
New Features
Migration