Skip to content

Commit

Permalink
fix(eslint): enforce rules on exports
Browse files Browse the repository at this point in the history
Dependencies can be accessed directly via exports, and these cases need to
be checked against existing rules. The CLI was already detecting such
violations, but ESLint missed them. This commit ensures that exports are
now properly checked as well.
  • Loading branch information
rainerhahnekamp committed Oct 5, 2024
1 parent a418b7d commit 883a472
Show file tree
Hide file tree
Showing 15 changed files with 986 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
} from '../traverse-filesystem';
import { describe, it, expect } from 'vitest';
import { buildFileInfo } from '../../test/build-file-info';
import getFs from '../../fs/getFs';

function setup(fileTree: FileTree): UnassignedFileInfo {
createProject(fileTree);
Expand Down Expand Up @@ -211,7 +212,7 @@ describe('traverse filesystem', () => {

it('should return undefined if TS resolving does not work', () => {
const resolveFn: ResolveFn = () => ({
resolvedModule: undefined
resolvedModule: undefined,
});

expect(
Expand Down Expand Up @@ -360,4 +361,30 @@ describe('traverse filesystem', () => {
expect(fileInfo).toEqual(buildFileInfo('/project/src/main.ts', []));
});
});

it('should recognize exports as imports', () => {
createProject({
'tsconfig.json': tsConfig(),
src: {
app: {
'app.component.ts': [],
customers: {
'customer.component.ts': [],
},
},
},
});

getFs().writeFile(
'/project/src/app/app.component.ts',
`export * from './customers/customer.component'`,
);

const tsData = generateTsData(toFsPath('/project/tsconfig.json'));
const mainPath = toFsPath('/project/src/app/app.component.ts');

const unassignedFileInfo = traverseFilesystem(mainPath, new Map<FsPath, UnassignedFileInfo>(), tsData);

expect(unassignedFileInfo.imports).toHaveLength(1)
});
});
6 changes: 5 additions & 1 deletion packages/core/src/lib/file-info/unassigned-file-info.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@ import { FsPath } from './fs-path';
import throwIfNull from '../util/throw-if-null';

/**
* Represents a TypeScript file with its dependencies.
* Represents a TypeScript file with its dependencies but does
* not yet have an assignment to a module.
*
* After module assignment is done, it becomes a type `FileInfo`.
*
* If an import cannot be resolved, it doesn't throw an error
* but is added to unresolvableImports.
*
Expand Down
13 changes: 10 additions & 3 deletions packages/eslint-plugin/src/lib/rules/create-rule.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { Rule } from 'eslint';
import { ImportDeclaration, ImportExpression } from 'estree';
import { Executor } from './executor';
import {Executor, ExecutorNode} from './executor';
import { UserError } from '@softarc/sheriff-core';

/**
Expand All @@ -22,13 +21,19 @@ export const createRule: (
let isFirstRun = true;
let hasInternalError = false;
const executeRuleWithContext = (
node: ImportExpression | ImportDeclaration,
node: ExecutorNode,
) => {
const filename = context.filename ?? context.getFilename();
const sourceCode =
context.sourceCode?.text ?? context.getSourceCode().text;

if (!hasInternalError) {
try {
// don't process special export `export const value = {n: 1};`
if (!node.source) {
return;
}

executor(context, node, isFirstRun, filename, sourceCode);
} catch (error) {
hasInternalError = true;
Expand All @@ -50,6 +55,8 @@ export const createRule: (
return {
ImportExpression: executeRuleWithContext,
ImportDeclaration: executeRuleWithContext,
ExportAllDeclaration: executeRuleWithContext,
ExportNamedDeclaration: executeRuleWithContext,
};
},
});
11 changes: 1 addition & 10 deletions packages/eslint-plugin/src/lib/rules/deep-import.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,9 @@
import { Rule } from 'eslint';
import { hasDeepImport } from '@softarc/sheriff-core';
import { ImportDeclaration, ImportExpression } from 'estree';
import { createRule } from './create-rule';

export const deepImport = createRule(
'Deep Import',
(
context: Rule.RuleContext,
node: ImportExpression | ImportDeclaration,
isFirstRun: boolean,
filename: string,
sourceCode: string,
) => {
// ESTree does not have source on `ImportExpression`.
(context, node, isFirstRun, filename, sourceCode) => {
const importValue = (node.source as { value: string }).value;
const message = hasDeepImport(
filename,
Expand Down
10 changes: 1 addition & 9 deletions packages/eslint-plugin/src/lib/rules/dependency-rule.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,9 @@
import { Rule } from 'eslint';
import { violatesDependencyRule } from '@softarc/sheriff-core';
import { ImportDeclaration, ImportExpression } from 'estree';
import { createRule } from './create-rule';

export const dependencyRule = createRule(
'Dependency Rule',
(
context: Rule.RuleContext,
node: ImportExpression | ImportDeclaration,
isFirstRun: boolean,
filename: string,
sourceCode: string,
) => {
(context, node, isFirstRun, filename, sourceCode) => {
const importValue = (node.source as { value: string }).value;
const message = violatesDependencyRule(
filename,
Expand Down
6 changes: 4 additions & 2 deletions packages/eslint-plugin/src/lib/rules/executor.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import { Rule } from 'eslint';
import { ImportDeclaration, ImportExpression } from 'estree';
import {ExportAllDeclaration, ExportNamedDeclaration, ImportDeclaration, ImportExpression} from 'estree';

export type ExecutorNode = ImportExpression | ImportDeclaration | ExportNamedDeclaration | ExportAllDeclaration;

export type Executor = (
context: Rule.RuleContext,
node: ImportExpression | ImportDeclaration,
node: ExecutorNode,
isFirstRun: boolean,
filename: string,
sourceCode: string,
Expand Down
18 changes: 18 additions & 0 deletions packages/eslint-plugin/src/lib/rules/tests/create-rule.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,4 +71,22 @@ describe('create rule', () => {
expect(spy).toHaveBeenCalledTimes(1);
});
}

it('should also match export declarations', () => {
tester.run('test-rule', testRule, {
valid: [
{
code: `
export * from '../index';
export { Component } from './component';
export const value = {n: 1};
export default {a: 1};
`,
},
],
invalid: []
});

expect(spy).toHaveBeenCalledTimes(2);
});
});
10 changes: 10 additions & 0 deletions test-projects/angular-i/integration-test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -69,3 +69,13 @@ npx ng lint --force --format json --output-file tests/actual/auto-tagging-lint.j
../remove-paths.mjs tests/actual/auto-tagging-lint.json
diff tests/actual/auto-tagging-lint.json tests/expected/auto-tagging-lint.json
cp sheriff.config.ts.original sheriff.config.ts

## Re-Exports Check
echo 'checking for re-exports'
cp src/app/customers/api/index.ts src/app/customers/api/index.ts.original
cp tests/customer-api-re-exports.index.ts src/app/customers/api/index.ts
npx ng lint --force --format json --output-file tests/actual/re-exports-lint.json
../remove-paths.mjs tests/actual/re-exports-lint.json
diff tests/actual/re-exports-lint.json tests/expected/re-exports-lint.json
mv src/app/customers/api/index.ts.original src/app/customers/api/index.ts

11 changes: 11 additions & 0 deletions test-projects/angular-i/tests/customer-api-re-exports.index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { inject, Injectable } from '@angular/core';
import { CustomersRepository } from '../data';
export { bookingsRoutes } from '../../bookings';

@Injectable({ providedIn: 'root' })
export class CustomersApi {
repo = inject(CustomersRepository);
get selectedCustomer$() {
return this.repo.selectedCustomer$;
}
}
25 changes: 25 additions & 0 deletions test-projects/angular-i/tests/expected/re-exports-lint.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
[
{
"filePath": "./src/app/customers/api/index.ts",
"messages": [
{
"ruleId": "@softarc/sheriff/dependency-rule",
"severity": 2,
"message": "module /src/app/customers/api cannot access /src/app/bookings. Tag domain:customers:api has no clearance for tags domain:bookings, type:feature",
"line": 3,
"column": 1,
"nodeType": "ExportNamedDeclaration",
"endLine": 3,
"endColumn": 49
}
],
"suppressedMessages": [],
"errorCount": 1,
"fatalErrorCount": 0,
"warningCount": 0,
"fixableErrorCount": 0,
"fixableWarningCount": 0,
"source": "import { inject, Injectable } from '@angular/core';\nimport { CustomersRepository } from '../data';\nexport { bookingsRoutes } from '../../bookings';\n\n@Injectable({ providedIn: 'root' })\nexport class CustomersApi {\n repo = inject(CustomersRepository);\n get selectedCustomer$() {\n return this.repo.selectedCustomer$;\n }\n}\n",
"usedDeprecatedRules": []
}
]
Loading

0 comments on commit 883a472

Please sign in to comment.