github-mirror/n8n
1
0
Fork 0
mirror of https://github.com/n8n-io/n8n.git synced 2026-06-05 02:59:27 +02:00
Code Issues Actions 13 Packages Projects Releases Wiki Activity Graph

refactor: Consolidate janitor + code-health onto a shared rules-engine AST substrate (#31417)

Browse Source 
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Declan Carroll 2026-06-04 09:27:01 +01:00 committed by GitHub
parent 85ecd70ccc
commit 07119ce61d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
41 changed files with 646 additions and 873 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
packages/testing/code-health/package.json
View File

@ -32,6 +32,7 @@
"@n8n/vitest-config": "workspace:*",
"@vitest/coverage-v8": "catalog:",
"fast-glob": "catalog:",
"ts-morph": "catalog:",
"tsx": "catalog:",
"typescript": "catalog:",
"vitest": "catalog:",

10
packages/testing/code-health/src/index.ts
View File

@ -3,12 +3,14 @@ import type { RuleSettingsMap } from '@n8n/rules-engine';
import type { CodeHealthContext } from './context.js';
import { CatalogViolationsRule } from './rules/catalog-violations.rule.js';
import { EndpointScopeCoverageRule } from './rules/endpoint-scope-coverage.rule.js';
import { MigrationTimestampRule } from './rules/migration-timestamp.rule.js';
import { StaleOverridesRule } from './rules/stale-overrides.rule.js';
import { WorkflowPrTargetSafetyRule } from './rules/workflow-pr-target-safety.rule.js';
export type { CodeHealthContext } from './context.js';
export { CatalogViolationsRule } from './rules/catalog-violations.rule.js';
export { EndpointScopeCoverageRule } from './rules/endpoint-scope-coverage.rule.js';
export { MigrationTimestampRule } from './rules/migration-timestamp.rule.js';
export { StaleOverridesRule } from './rules/stale-overrides.rule.js';
export { WorkflowPrTargetSafetyRule } from './rules/workflow-pr-target-safety.rule.js';
@ -34,6 +36,13 @@ const defaultRuleSettings: RuleSettingsMap = {
severity: 'warning',
options: { workspaceFile: 'pnpm-workspace.yaml', lockFile: 'pnpm-lock.yaml' },
},
'endpoint-scope-coverage': {
// Disabled by default: enabling gates CI on ~129 existing authenticated-unscoped
// routes, which must first be reviewed and grandfathered into the baseline.
enabled: false,
severity: 'warning',
options: { packages: ['packages/cli'] },
},
};
function mergeSettings(defaults: RuleSettingsMap, overrides?: RuleSettingsMap): RuleSettingsMap {
@ -55,6 +64,7 @@ export function createDefaultRunner(settings?: RuleSettingsMap): RuleRunner<Code
runner.registerRule(new WorkflowPrTargetSafetyRule());
runner.registerRule(new MigrationTimestampRule());
runner.registerRule(new StaleOverridesRule());
runner.registerRule(new EndpointScopeCoverageRule());
runner.applySettings(mergeSettings(defaultRuleSettings, settings));
return runner;
}

60
packages/testing/code-health/src/rules/endpoint-scope-coverage.rule.test.ts Normal file
View File

@ -0,0 +1,60 @@
import { createInMemoryProject } from '@n8n/rules-engine/ast';
import { describe, it, expect } from 'vitest';
import { EndpointScopeCoverageRule } from './endpoint-scope-coverage.rule.js';
const SOURCE = `
@RestController('/x')
export class XController {
@Get('/scoped')
@ProjectScope('workflow:read')
scoped() {}
@Post('/unscoped')
unscoped() {}
@Get('/public', { skipAuth: true })
pub() {}
@Get('/unauth', { allowUnauthenticated: true })
unauth() {}
@Get('/apikey', { apiKeyAuth: true })
apiKey() {}
}
// Not a controller — its routes must be ignored.
export class NotAController {
@Get('/ignored')
ignored() {}
}
`;
describe('EndpointScopeCoverageRule', () => {
const rule = new EndpointScopeCoverageRule();
const analyze = (code: string) => {
const project = createInMemoryProject();
project.createSourceFile('x.controller.ts', code);
return rule.analyzeProject(project);
};
it('flags only the authenticated, unscoped route on a controller', () => {
const violations = analyze(SOURCE);
expect(violations).toHaveLength(1);
expect(violations[0].message).toContain('XController.unscoped');
});
it('does not flag scoped, public, or non-controller routes', () => {
const messages = analyze(SOURCE)
.map((v) => v.message)
.join('\n');
expect(messages).not.toContain('XController.scoped'); // has @ProjectScope
expect(messages).not.toContain('XController.pub'); // skipAuth
expect(messages).not.toContain('XController.unauth'); // allowUnauthenticated
expect(messages).not.toContain('XController.apiKey'); // apiKeyAuth
expect(messages).not.toContain('NotAController'); // not a @RestController
});
});

67
packages/testing/code-health/src/rules/endpoint-scope-coverage.rule.ts Normal file
View File

@ -0,0 +1,67 @@
import type { Violation } from '@n8n/rules-engine';
import {
AstRule,
classHasDecorator,
getDecoratorByName,
getDecoratorObjectFlag,
} from '@n8n/rules-engine/ast';
import type { AstProjectConfig } from '@n8n/rules-engine/ast';
import type { Project } from 'ts-morph';
import type { CodeHealthContext } from '../context.js';
const CONTROLLER_DECORATORS = ['RestController', 'RootLevelController'];
const ROUTE_DECORATORS = ['Get', 'Post', 'Put', 'Patch', 'Delete', 'Head', 'Options'];
const SCOPE_DECORATORS = ['GlobalScope', 'ProjectScope'];
/** Route options that opt out of requiring an authenticated, scoped user. */
const PUBLIC_FLAGS = ['skipAuth', 'allowUnauthenticated', 'apiKeyAuth'];
/**
* Flags authenticated controller routes with no `@GlobalScope`/`@ProjectScope`.
* Baseline-gated: the existing (largely intentional) set is grandfathered, so
* only newly-added unscoped routes fail.
*/
export class EndpointScopeCoverageRule extends AstRule<CodeHealthContext> {
readonly id = 'endpoint-scope-coverage';
readonly name = 'Endpoint Scope Coverage';
readonly description =
'Authenticated controller routes should declare an RBAC scope; new unscoped routes are flagged (existing ones grandfathered via baseline)';
readonly severity = 'warning' as const;
protected projectConfig(): AstProjectConfig {
const packages = (this.getOptions().packages as string[]) ?? ['packages/cli'];
return { packages };
}
analyze(context: CodeHealthContext): Violation[] {
return this.projects(context).flatMap(({ project }) => this.analyzeProject(project));
}
/** Exposed for unit tests against an in-memory project. */
analyzeProject(project: Project): Violation[] {
const violations: Violation[] = [];
for (const file of project.getSourceFiles()) {
for (const cls of file.getClasses()) {
if (!classHasDecorator(cls, CONTROLLER_DECORATORS)) continue;
for (const method of cls.getMethods()) {
const route = getDecoratorByName(method, ROUTE_DECORATORS);
if (!route) continue;
if (getDecoratorByName(method, SCOPE_DECORATORS)) continue;
if (PUBLIC_FLAGS.some((flag) => getDecoratorObjectFlag(route, 1, flag))) continue;
violations.push(
this.nodeViolation(
route,
`Authenticated route ${cls.getName()}.${method.getName()} declares no @GlobalScope/@ProjectScope.`,
'Add a scope decorator, or mark the route { skipAuth: true } / { allowUnauthenticated: true } if it is intentionally public.',
),
);
}
}
}
return violations;
}
}

30
packages/testing/janitor/src/cli.ts
View File

@ -80,7 +80,7 @@ import {
} from './core/method-usage-analyzer.js';
import { orchestrate } from './core/orchestrator.js';
import { createProject } from './core/project-loader.js';
import { toJSON, toConsole, printFixResults } from './core/reporter.js';
import { toJSON, toConsole } from './core/reporter.js';
import { filterToFailedSpecs } from './core/retry-filter.js';
import { computeScope, formatScope } from './core/scope-analyzer.js';
import { TcrExecutor, formatTcrResultConsole, formatTcrResultJSON } from './core/tcr-executor.js';
@ -290,9 +290,7 @@ function runTcr(options: CliOptions): void {
function runAnalyze(options: CliOptions): void {
const config = getConfig();
const runner = createDefaultRunner();
// Create project
const { project, root } = createProject(config.rootDir);
const root = config.rootDir;
// Build run options
const runOptions: RunOptions = {};
@ -301,11 +299,6 @@ function runAnalyze(options: CliOptions): void {
runOptions.files = options.files;
}
if (options.fix) {
runOptions.fix = true;
runOptions.write = options.write;
}
// Pass rule-specific config
if (options.allowInExpect) {
runOptions.ruleConfig = {
@ -315,21 +308,20 @@ function runAnalyze(options: CliOptions): void {
// Run analysis
let report = options.rule
? runner.runRule(project, root, options.rule, runOptions)
: runner.run(project, root, runOptions);
? runner.runRule(options.rule, { rootDir: root }, runOptions)
: runner.run({ rootDir: root }, runOptions);
if (!report) {
console.error('Failed to generate report');
process.exit(1);
}
// Auto-filter by baseline if present (unless --ignore-baseline or --fix)
// Auto-filter by baseline if present (unless --ignore-baseline)
const baseline = loadBaseline(config.rootDir);
let baselineFiltered = false;
const originalViolations = report.summary.totalViolations;
if (baseline && !options.fix && !options.ignoreBaseline) {
// Don't filter during fix mode or when baseline is ignored
if (baseline && !options.ignoreBaseline) {
report = filterReportByBaseline(report, baseline, config.rootDir);
baselineFiltered = true;
}
@ -350,14 +342,10 @@ function runAnalyze(options: CliOptions): void {
}
toConsole(report, options.verbose);
if (options.fix) {
printFixResults(report, options.write);
}
}
// Exit with error code if violations found
if (report.summary.totalViolations > 0 && !(options.fix && options.write)) {
if (report.summary.totalViolations > 0) {
process.exit(1);
}
}
@ -365,12 +353,10 @@ function runAnalyze(options: CliOptions): void {
function runBaseline(options: CliOptions): void {
const config = getConfig();
// Create runner and project
const runner = createDefaultRunner();
const { project, root } = createProject(config.rootDir);
// Run full analysis (no file filter, no baseline filter)
const report = runner.run(project, root, {});
const report = runner.run({ rootDir: config.rootDir }, {});
if (!report) {
console.error('Failed to generate report');

14
packages/testing/janitor/src/cli/arg-parser.test.ts
View File

@ -82,18 +82,6 @@ describe('arg-parser', () => {
expect(result.verbose).toBe(true);
});
it('parses --fix', () => {
setArgs(['--fix']);
const result = parseArgs();
expect(result.fix).toBe(true);
});
it('parses --write', () => {
setArgs(['--write']);
const result = parseArgs();
expect(result.write).toBe(true);
});
it('parses --list', () => {
setArgs(['--list']);
const result = parseArgs();
@ -259,8 +247,6 @@ describe('arg-parser', () => {
expect(result.files).toEqual([]);
expect(result.json).toBe(false);
expect(result.verbose).toBe(false);
expect(result.fix).toBe(false);
expect(result.write).toBe(false);
expect(result.help).toBe(false);
expect(result.list).toBe(false);
expect(result.execute).toBe(false);

10
packages/testing/janitor/src/cli/arg-parser.ts
View File

@ -29,8 +29,6 @@ export interface CliOptions {
files?: string[];
json: boolean;
verbose: boolean;
fix: boolean;
write: boolean;
help: boolean;
list: boolean;
// TCR-specific options
@ -110,12 +108,6 @@ const FLAG_HANDLERS: Record<string, FlagHandler> = {
'-v': (opts) => {
opts.verbose = true;
},
'--fix': (opts) => {
opts.fix = true;
},
'--write': (opts) => {
opts.write = true;
},
'--list': (opts) => {
opts.list = true;
},
@ -242,8 +234,6 @@ function createDefaultOptions(): CliOptions {
files: [],
json: false,
verbose: false,
fix: false,
write: false,
help: false,
list: false,
execute: false,

3
packages/testing/janitor/src/cli/help.ts
View File

@ -29,8 +29,6 @@ Analysis Options:
--files=<p1,p2> Analyze multiple files (comma-separated)
--json Output as JSON
--verbose, -v Detailed output with suggestions
--fix Preview fixes (dry run)
--fix --write Apply fixes to disk
--list, -l List available rules
--allow-in-expect Skip selector-purity violations inside expect()
--ignore-baseline Show all violations, ignoring .janitor-baseline.json
@ -41,7 +39,6 @@ Examples:
playwright-janitor --rule=dead-code # Run specific rule
playwright-janitor inventory # Show codebase structure
playwright-janitor impact --file=pages/X # Show what tests are affected
playwright-janitor --fix --write # Apply auto-fixes
For command-specific help:
playwright-janitor rules --help

6
packages/testing/janitor/src/config.ts
View File

@ -266,3 +266,9 @@ export function hasConfig(): boolean {
export function resetConfig(): void {
currentConfig = null;
}
/** Does `text` match any configured allow-pattern for the given rule? */
export function ruleAllows(ruleId: string, text: string): boolean {
const allowPatterns = getConfig().rules?.[ruleId]?.allowPatterns ?? [];
return allowPatterns.some((pattern) => pattern.test(text));
}

56
packages/testing/janitor/src/core/reporter.ts
View File

@ -1,4 +1,4 @@
import type { JanitorReport, Violation, FixResult } from '../types.js';
import type { JanitorReport, Violation } from '../types.js';
import { getRelativePath } from './project-loader.js';
/**
@ -143,57 +143,3 @@ function getSeverityIcon(severity: string): string {
return '[?]';
}
}
/**
* Output fix results to console
*/
export function printFixResults(report: JanitorReport, write: boolean): void {
const allFixes: FixResult[] = [];
for (const result of report.results) {
if (result.fixes) {
allFixes.push(...result.fixes);
}
}
if (allFixes.length === 0) {
console.log('\nNo fixable violations found.\n');
return;
}
console.log('\n------------------------------------');
console.log(write ? ' FIXES APPLIED' : ' FIX PREVIEW (dry run)');
console.log('------------------------------------\n');
for (const fix of allFixes) {
const icon = fix.applied ? '[OK]' : '[PREVIEW]';
const actionLabel = getActionLabel(fix.action);
const target = fix.target ? ` ${fix.target}` : '';
console.log(` ${icon} ${actionLabel}${target}`);
console.log(` ${fix.file}`);
}
console.log('');
if (!write) {
console.log(`${allFixes.length} fix(es) would be applied.`);
console.log('Use --fix --write to apply changes.\n');
} else {
console.log(`${allFixes.length} fix(es) applied.`);
console.log('Run: pnpm typecheck && pnpm lint\n');
}
}
function getActionLabel(action: FixResult['action']): string {
switch (action) {
case 'remove-file':
return 'DELETE FILE';
case 'remove-method':
return 'REMOVE METHOD';
case 'remove-property':
return 'REMOVE PROPERTY';
case 'edit':
return 'EDIT';
default:
return action;
}
}

255
packages/testing/janitor/src/core/rule-runner.ts
View File

@ -1,7 +1,5 @@
import * as path from 'node:path';
import type { Project, SourceFile } from 'ts-morph';
import { performance } from 'node:perf_hooks';
import type { BaseRule } from '../rules/base-rule.js';
import type {
JanitorReport,
RuleResult,
@ -9,270 +7,151 @@ import type {
RunOptions,
RuleConfig,
RuleInfo,
Violation,
} from '../types.js';
import { getSourceFiles } from './project-loader.js';
import { getConfig } from '../config.js';
/** Context passed to every rule. Rules self-build their scoped project from `rootDir`. */
export interface JanitorRunContext {
rootDir: string;
/** Repo-relative changed files, when scoping a run (e.g. TCR). */
changedFiles?: string[];
}
/** The rule surface the runner depends on — satisfied by `AstRule` subclasses + `getTargetGlobs`. */
export interface RunnableRule {
readonly id: string;
readonly name: string;
readonly description: string;
readonly severity: Severity;
readonly fixable: boolean;
getTargetGlobs(): string[];
analyze(context: JanitorRunContext): Violation[];
configure(settings: { options?: Record<string, unknown> }): void;
}
export class RuleRunner {
private rules: Map<string, BaseRule> = new Map();
private rules: Map<string, RunnableRule> = new Map();
private enabledRules: Set<string> = new Set();
private ruleConfigs: Map<string, RuleConfig> = new Map();
/**
* Register a rule with the runner
*/
registerRule(rule: BaseRule): void {
registerRule(rule: RunnableRule): void {
this.rules.set(rule.id, rule);
this.enabledRules.add(rule.id);
}
/**
* Enable a specific rule
*/
enableRule(ruleId: string): void {
if (this.rules.has(ruleId)) {
this.enabledRules.add(ruleId);
}
if (this.rules.has(ruleId)) this.enabledRules.add(ruleId);
}
/**
* Disable a specific rule
*/
disableRule(ruleId: string): void {
this.enabledRules.delete(ruleId);
}
/**
* Enable only specific rules (disable all others)
*/
enableOnly(ruleIds: string[]): void {
this.enabledRules.clear();
for (const id of ruleIds) {
if (this.rules.has(id)) {
this.enabledRules.add(id);
}
if (this.rules.has(id)) this.enabledRules.add(id);
}
}
/**
* Get all registered rule IDs
*/
getRegisteredRules(): string[] {
return Array.from(this.rules.keys());
}
/**
* Check if a rule is fixable
*/
isRuleFixable(ruleId: string): boolean {
return this.rules.get(ruleId)?.fixable ?? false;
}
/**
* Get enabled rule IDs
*/
getEnabledRules(): string[] {
return Array.from(this.enabledRules);
}
/**
* Get disabled rule IDs
*/
getDisabledRules(): string[] {
return Array.from(this.rules.keys()).filter((id) => !this.enabledRules.has(id));
}
/**
* Get detailed information about all registered rules
*/
getRuleDetails(): RuleInfo[] {
return Array.from(this.rules.values()).map((rule) => ({
id: rule.id,
name: rule.name,
description: rule.description,
severity: rule.severity,
fixable: rule.fixable,
enabled: this.enabledRules.has(rule.id),
targetGlobs: rule.getTargetGlobs(),
}));
return Array.from(this.rules.values()).map((rule) => this.toRuleInfo(rule));
}
/**
* Get detailed information about a specific rule
*/
getRuleInfo(ruleId: string): RuleInfo | undefined {
const rule = this.rules.get(ruleId);
if (!rule) return undefined;
return {
id: rule.id,
name: rule.name,
description: rule.description,
severity: rule.severity,
fixable: rule.fixable,
enabled: this.enabledRules.has(rule.id),
targetGlobs: rule.getTargetGlobs(),
};
return rule ? this.toRuleInfo(rule) : undefined;
}
/**
* Configure a specific rule
*/
configureRule(ruleId: string, config: RuleConfig): void {
this.ruleConfigs.set(ruleId, config);
const rule = this.rules.get(ruleId);
if (rule) {
rule.configure(config);
}
this.rules.get(ruleId)?.configure({ options: { ...config } });
}
/**
* Get files for a rule, optionally filtered by specific file paths
*/
private getFilesForRule(project: Project, rule: BaseRule, targetFiles?: string[]): SourceFile[] {
const root = getConfig().rootDir;
if (targetFiles && targetFiles.length > 0) {
// Filter to only the specified files that match rule's globs
const ruleGlobs = rule.getTargetGlobs();
const allRuleFiles = getSourceFiles(project, ruleGlobs);
const allRuleFilePaths = new Set(allRuleFiles.map((f) => f.getFilePath()));
// Resolve target files to absolute paths and filter
return targetFiles
.map((f) => {
const absolutePath = path.isAbsolute(f) ? f : path.join(root, f);
return project.getSourceFile(absolutePath);
})
.filter((f): f is SourceFile => f !== undefined && allRuleFilePaths.has(f.getFilePath()));
}
return getSourceFiles(project, rule.getTargetGlobs());
}
/**
* Run all enabled rules and return results
*/
run(project: Project, projectRoot: string, options?: RunOptions): JanitorReport {
const results: RuleResult[] = [];
const allFilesAnalyzed = new Set<string>();
// Apply rule configurations from options
run(context: JanitorRunContext, options?: RunOptions): JanitorReport {
if (options?.ruleConfig) {
for (const [ruleId, config] of Object.entries(options.ruleConfig)) {
this.configureRule(ruleId, config);
}
}
const runContext: JanitorRunContext = { ...context, changedFiles: options?.files };
const results: RuleResult[] = [];
for (const ruleId of this.enabledRules) {
const rule = this.rules.get(ruleId);
if (!rule) continue;
// Get files for this rule (optionally filtered)
const files = this.getFilesForRule(project, rule, options?.files);
// Track all files analyzed
for (const file of files) {
allFilesAnalyzed.add(file.getFilePath());
}
// Execute the rule
const result = rule.execute(project, files);
// Apply fixes if in fix mode and rule supports it
if (options?.fix && rule.fixable) {
const fixableViolations = result.violations.filter((v) => v.fixable);
if (fixableViolations.length > 0) {
result.fixes = rule.fix(project, fixableViolations, options.write ?? false);
}
}
results.push(result);
if (rule) results.push(this.execute(rule, runContext));
}
// Build summary
const summary = this.buildSummary(results, allFilesAnalyzed.size);
return {
timestamp: new Date().toISOString(),
projectRoot,
rules: {
enabled: this.getEnabledRules(),
disabled: this.getDisabledRules(),
},
results,
summary,
};
return this.buildReport(context.rootDir, results);
}
/**
* Run a specific rule by ID
*/
runRule(
project: Project,
projectRoot: string,
ruleId: string,
options?: RunOptions,
): JanitorReport | null {
runRule(ruleId: string, context: JanitorRunContext, options?: RunOptions): JanitorReport | null {
const rule = this.rules.get(ruleId);
if (!rule) {
return null;
}
if (!rule) return null;
// Apply rule configuration from options
if (options?.ruleConfig?.[ruleId]) {
rule.configure(options.ruleConfig[ruleId]);
}
if (options?.ruleConfig?.[ruleId]) this.configureRule(ruleId, options.ruleConfig[ruleId]);
const files = this.getFilesForRule(project, rule, options?.files);
const result = rule.execute(project, files);
const runContext: JanitorRunContext = { ...context, changedFiles: options?.files };
return this.buildReport(context.rootDir, [this.execute(rule, runContext)]);
}
// Apply fixes if in fix mode and rule supports it
if (options?.fix && rule.fixable) {
const fixableViolations = result.violations.filter((v) => v.fixable);
if (fixableViolations.length > 0) {
result.fixes = rule.fix(project, fixableViolations, options.write ?? false);
}
}
const summary = this.buildSummary([result], files.length);
private execute(rule: RunnableRule, context: JanitorRunContext): RuleResult {
const start = performance.now();
const violations = rule.analyze(context);
const executionTimeMs = Math.round((performance.now() - start) * 100) / 100;
return {
timestamp: new Date().toISOString(),
projectRoot,
rules: {
enabled: [ruleId],
disabled: this.getRegisteredRules().filter((id) => id !== ruleId),
},
results: [result],
summary,
rule: rule.id,
violations,
filesAnalyzed: 0,
executionTimeMs,
fixable: rule.fixable,
};
}
private buildSummary(results: RuleResult[], filesAnalyzed: number): JanitorReport['summary'] {
const byRule: Record<string, number> = {};
const bySeverity: Record<Severity, number> = {
error: 0,
warning: 0,
info: 0,
private toRuleInfo(rule: RunnableRule): RuleInfo {
return {
id: rule.id,
name: rule.name,
description: rule.description,
severity: rule.severity,
fixable: rule.fixable,
enabled: this.enabledRules.has(rule.id),
targetGlobs: rule.getTargetGlobs(),
};
}
private buildReport(projectRoot: string, results: RuleResult[]): JanitorReport {
const byRule: Record<string, number> = {};
const bySeverity: Record<Severity, number> = { error: 0, warning: 0, info: 0 };
let totalViolations = 0;
for (const result of results) {
byRule[result.rule] = result.violations.length;
totalViolations += result.violations.length;
for (const violation of result.violations) {
bySeverity[violation.severity]++;
}
for (const violation of result.violations) bySeverity[violation.severity]++;
}
return {
totalViolations,
byRule,
bySeverity,
filesAnalyzed,
timestamp: new Date().toISOString(),
projectRoot,
rules: { enabled: this.getEnabledRules(), disabled: this.getDisabledRules() },
results,
summary: { totalViolations, byRule, bySeverity, filesAnalyzed: 0 },
};
}
}

4
packages/testing/janitor/src/core/tcr-executor.ts
View File

@ -11,7 +11,6 @@ import { loadBaseline, filterNewViolations } from './baseline.js';
import { extractDiffs } from './extract-diffs.js';
import { ImpactAnalyzer } from './impact-analyzer.js';
import { MethodUsageAnalyzer, type MethodUsageIndex } from './method-usage-analyzer.js';
import { createProject } from './project-loader.js';
import { RuleRunner } from './rule-runner.js';
import { ApiPurityRule } from '../rules/api-purity.rule.js';
import { BoundaryProtectionRule } from '../rules/boundary-protection.rule.js';
@ -366,7 +365,6 @@ export class TcrExecutor {
private runRules(changedFiles: string[]): number {
const runner = this.createRuleRunner();
const { project } = createProject(this.root);
const tsFiles = changedFiles
.filter((f) => f.endsWith('.ts'))
@ -374,7 +372,7 @@ export class TcrExecutor {
if (tsFiles.length === 0) return 0;
const report = runner.run(project, this.root, { files: tsFiles });
const report = runner.run({ rootDir: this.root }, { files: tsFiles });
return this.countNewViolations(report);
}

11
packages/testing/janitor/src/index.ts
View File

@ -45,12 +45,9 @@ export type {
PropertyFixData,
ClassFixData,
EditFixData,
FixAction,
FixResult,
RuleResult,
ReportSummary,
JanitorReport,
Rule,
RuleInfo,
FilePatterns,
FacadeConfig,
@ -71,6 +68,7 @@ export type {
export { SyntaxKind } from './types.js';
export { RuleRunner } from './core/rule-runner.js';
export type { RunnableRule, JanitorRunContext } from './core/rule-runner.js';
export { FacadeResolver, extractTypeName, type FacadeMapping } from './core/facade-resolver.js';
export {
createProject,
@ -79,10 +77,9 @@ export {
getRelativePath,
type ProjectContext,
} from './core/project-loader.js';
export { toJSON, toConsole, printFixResults } from './core/reporter.js';
export { toJSON, toConsole } from './core/reporter.js';
export {
BaseRule,
BoundaryProtectionRule,
ScopeLockdownRule,
SelectorPurityRule,
@ -201,7 +198,6 @@ export {
} from './core/baseline.js';
import { setConfig, type JanitorConfig } from './config.js';
import { createProject } from './core/project-loader.js';
import { RuleRunner } from './core/rule-runner.js';
import { ApiPurityRule } from './rules/api-purity.rule.js';
import { BoundaryProtectionRule } from './rules/boundary-protection.rule.js';
@ -232,7 +228,6 @@ export function createDefaultRunner(): RuleRunner {
export function runAnalysis(config: JanitorConfig, options?: RunOptions): JanitorReport {
setConfig(config);
const { project, root } = createProject(config.rootDir);
const runner = createDefaultRunner();
return runner.run(project, root, options);
return runner.run({ rootDir: config.rootDir }, options);
}

14
packages/testing/janitor/src/rules/api-purity.rule.test.ts
View File

@ -19,7 +19,7 @@ test('creates workflow', async ({ n8n, api }) => {
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
expect(violations).toHaveLength(0);
});
@ -36,7 +36,7 @@ test('gets workflow', async ({ request }) => {
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
expect(violations).toHaveLength(1);
expect(violations[0].message).toContain('request.get');
@ -54,7 +54,7 @@ test('creates workflow', async ({ request }) => {
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
expect(violations).toHaveLength(1);
expect(violations[0].suggestion).toContain('api');
@ -72,7 +72,7 @@ test('fetches data', async () => {
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
expect(violations).toHaveLength(1);
expect(violations[0].message).toContain('fetch');
@ -92,7 +92,7 @@ test('multiple API calls', async ({ request }) => {
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
expect(violations).toHaveLength(3);
});
@ -109,7 +109,7 @@ export class WorkflowComposer {
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
expect(violations).toHaveLength(1);
});
@ -127,7 +127,7 @@ test('test', async ({ request }) => {
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
expect(violations).toHaveLength(1);
expect(violations[0].line).toBe(6);

21
packages/testing/janitor/src/rules/api-purity.rule.ts
View File

@ -1,7 +1,8 @@
import { AstRule } from '@n8n/rules-engine/ast';
import type { AstProjectConfig } from '@n8n/rules-engine/ast';
import type { Project, SourceFile } from 'ts-morph';
import { BaseRule } from './base-rule.js';
import { getConfig } from '../config.js';
import { getConfig, ruleAllows } from '../config.js';
import type { Violation } from '../types.js';
import { truncateText } from '../utils/ast-helpers.js';
@ -28,7 +29,7 @@ import { truncateText } from '../utils/ast-helpers.js';
* - API service files themselves (services/**) can make raw calls
* - Fixture setup files can make raw calls
*/
export class ApiPurityRule extends BaseRule {
export class ApiPurityRule extends AstRule<{ rootDir: string }> {
readonly id = 'api-purity';
readonly name = 'API Purity';
readonly description = 'Tests and composables should use API services, not raw HTTP calls';
@ -39,7 +40,15 @@ export class ApiPurityRule extends BaseRule {
return [...config.patterns.tests, ...config.patterns.flows];
}
analyze(_project: Project, files: SourceFile[]): Violation[] {
protected projectConfig(): AstProjectConfig {
return { packages: ['.'], spec: { globs: this.getTargetGlobs() } };
}
analyze(context: { rootDir: string }): Violation[] {
return this.projects(context).flatMap(({ project }) => this.analyzeProject(project));
}
analyzeProject(project: Project, files: SourceFile[] = project.getSourceFiles()): Violation[] {
const violations: Violation[] = [];
const config = getConfig();
@ -62,7 +71,7 @@ export class ApiPurityRule extends BaseRule {
const context = content.substring(contextStart, contextEnd);
// Check if this matches any allow patterns
if (this.isAllowed(context)) {
if (ruleAllows(this.id, context)) {
continue;
}
@ -77,7 +86,7 @@ export class ApiPurityRule extends BaseRule {
const suggestion = this.getSuggestion(matchText);
violations.push(
this.createViolation(
this.fileViolation(
file,
line,
column,

127
packages/testing/janitor/src/rules/base-rule.ts
View File

@ -1,127 +0,0 @@
import type { Project, SourceFile } from 'ts-morph';
import { getConfig } from '../config.js';
import type { Severity, Violation, RuleResult, RuleConfig, FixResult, FixData } from '../types.js';
export abstract class BaseRule {
abstract readonly id: string;
abstract readonly name: string;
abstract readonly description: string;
abstract readonly severity: Severity;
/** Whether this rule supports auto-fixing */
readonly fixable: boolean = false;
/** Rule-specific configuration */
protected config: RuleConfig = {};
/**
* Get the effective severity for this rule (may be overridden in config)
*/
getEffectiveSeverity(): Severity {
const ruleConfig = getConfig().rules?.[this.id];
if (ruleConfig?.severity === 'off') {
return 'info'; // Will be filtered out
}
if (ruleConfig?.severity) {
return ruleConfig.severity;
}
return this.severity;
}
/**
* Check if this rule is enabled
*/
isEnabled(): boolean {
const ruleConfig = getConfig().rules?.[this.id];
if (ruleConfig?.enabled === false) return false;
if (ruleConfig?.severity === 'off') return false;
return true;
}
/**
* Check if a text matches any of the allow patterns for this rule
*/
protected isAllowed(text: string): boolean {
const ruleConfig = getConfig().rules?.[this.id];
const allowPatterns = ruleConfig?.allowPatterns ?? [];
return allowPatterns.some((pattern) => pattern.test(text));
}
/**
* Define which file patterns this rule should analyze
* @returns Array of glob patterns relative to root
*/
abstract getTargetGlobs(): string[];
/**
* Analyze files and return violations
* @param project - ts-morph Project instance
* @param files - Source files matching the target globs
* @returns Array of violations found
*/
abstract analyze(project: Project, files: SourceFile[]): Violation[];
/**
* Apply fixes for violations. Override in fixable rules.
* @param project - ts-morph Project instance
* @param violations - Violations to fix
* @param write - Whether to write changes to disk
* @returns Array of fix results
*/
fix(_project: Project, _violations: Violation[], _write: boolean): FixResult[] {
return [];
}
/**
* Configure the rule with options
*/
configure(config: RuleConfig): void {
this.config = { ...this.config, ...config };
}
/**
* Execute the rule with timing instrumentation
* @param project - ts-morph Project instance
* @param files - Source files matching the target globs
* @returns Rule result with violations and metadata
*/
execute(project: Project, files: SourceFile[]): RuleResult {
const startTime = performance.now();
const violations = this.analyze(project, files);
const endTime = performance.now();
return {
rule: this.id,
violations,
filesAnalyzed: files.length,
executionTimeMs: Math.round((endTime - startTime) * 100) / 100,
fixable: this.fixable,
};
}
/**
* Helper to create a violation with consistent structure
*/
protected createViolation(
file: SourceFile,
line: number,
column: number,
message: string,
suggestion?: string,
fixable?: boolean,
fixData?: FixData,
): Violation {
return {
file: file.getFilePath(),
line,
column,
rule: this.id,
message,
severity: this.getEffectiveSeverity(),
suggestion,
fixable,
fixData,
};
}
}

95
packages/testing/janitor/src/rules/boundary-protection.rule.test.ts
View File

@ -3,95 +3,36 @@ import { describe } from 'vitest';
import { BoundaryProtectionRule } from './boundary-protection.rule.js';
import { test, expect } from '../test/fixtures.js';
// Uses janitor's test fixture so getConfig() (excludeFromPages, patterns) is set —
// the rule respects configured exclusions via isExcludedPage.
describe('BoundaryProtectionRule', () => {
const rule = new BoundaryProtectionRule();
test('should allow imports from components', ({ project, createFile }) => {
const file = createFile(
'/pages/CanvasPage.ts',
`
import { NodePanel } from './components/NodePanel';
export class CanvasPage {
readonly nodePanel = new NodePanel();
}
`,
);
const violations = rule.analyze(project, [file]);
expect(violations).toHaveLength(0);
test('allows imports from components', ({ project, createFile }) => {
createFile('/pages/CanvasPage.ts', "import { NodePanel } from './components/NodePanel';");
expect(rule.analyzeProject(project)).toHaveLength(0);
});
test('should flag direct page imports', ({ project, createFile }) => {
const file = createFile(
'/pages/CanvasPage.ts',
`
import { WorkflowPage } from './WorkflowPage';
export class CanvasPage {
readonly workflows = new WorkflowPage();
}
`,
);
// Also create the imported file so ts-morph can resolve it
createFile(
'/pages/WorkflowPage.ts',
`
export class WorkflowPage {}
`,
);
const violations = rule.analyze(project, [file]);
test('flags a direct page import', ({ project, createFile }) => {
createFile('/pages/CanvasPage.ts', "import { WorkflowPage } from './WorkflowPage';");
createFile('/pages/WorkflowPage.ts', 'export class WorkflowPage {}');
const violations = rule.analyzeProject(project);
expect(violations).toHaveLength(1);
expect(violations[0].message).toContain('imports another page');
expect(violations[0].rule).toBe('boundary-protection');
});
test('should allow imports from base classes', ({ project, createFile }) => {
const file = createFile(
'/pages/CanvasPage.ts',
`
import { BasePage } from './BasePage';
export class CanvasPage extends BasePage {
}
`,
);
const violations = rule.analyze(project, [file]);
expect(violations).toHaveLength(0);
test('allows imports from base classes', ({ project, createFile }) => {
createFile('/pages/CanvasPage.ts', "import { BasePage } from './BasePage';");
expect(rule.analyzeProject(project)).toHaveLength(0);
});
test('should skip excluded files', ({ project, createFile }) => {
const file = createFile(
'/pages/BasePage.ts',
`
import { SomePage } from './SomePage';
export class BasePage {
}
`,
);
const violations = rule.analyze(project, [file]);
expect(violations).toHaveLength(0);
test('skips files excluded via config (excludeFromPages)', ({ project, createFile }) => {
createFile('/pages/BasePage.ts', "import { SomePage } from './SomePage';");
expect(rule.analyzeProject(project)).toHaveLength(0);
});
test('should allow external package imports', ({ project, createFile }) => {
const file = createFile(
'/pages/CanvasPage.ts',
`
import { Page } from '@playwright/test';
import { expect } from '@playwright/test';
export class CanvasPage {
constructor(private page: Page) {}
}
`,
);
const violations = rule.analyze(project, [file]);
expect(violations).toHaveLength(0);
test('allows external package imports', ({ project, createFile }) => {
createFile('/pages/CanvasPage.ts', "import { Page } from '@playwright/test';");
expect(rule.analyzeProject(project)).toHaveLength(0);
});
});

86
packages/testing/janitor/src/rules/boundary-protection.rule.ts
View File

@ -1,73 +1,53 @@
import type { Project, SourceFile } from 'ts-morph';
import type { Violation } from '@n8n/rules-engine';
import { AstRule } from '@n8n/rules-engine/ast';
import type { AstProjectConfig } from '@n8n/rules-engine/ast';
import type { Project } from 'ts-morph';
import { BaseRule } from './base-rule.js';
import { getConfig } from '../config.js';
import type { Violation } from '../types.js';
import { getImportPaths, isPageImport } from '../utils/ast-helpers.js';
import { isPageImport } from '../utils/ast-helpers.js';
import { isExcludedPage } from '../utils/paths.js';
/**
* Boundary Protection Rule
*
* Pages should not import other pages directly.
* This prevents tight coupling between page objects.
*
* Allowed:
* - Imports from components directory
* - Imports from base classes (BasePage, BaseModal, etc.)
* - Imports from external packages
*
* Violations:
* - import { CanvasPage } from './CanvasPage' (in a page file)
* - import { WorkflowPage } from '../WorkflowPage'
* Pages must not import other pages directly. Janitor-owned (playwright domain
* logic via {@link isPageImport} + config-driven {@link isExcludedPage}) but
* built on the shared `@n8n/rules-engine/ast` substrate rather than a janitor base.
*/
export class BoundaryProtectionRule extends BaseRule {
export class BoundaryProtectionRule extends AstRule<{ rootDir: string }> {
readonly id = 'boundary-protection';
readonly name = 'Boundary Protection';
readonly description = 'Pages should not import other pages directly';
readonly description = 'Page objects should not import other page objects directly';
readonly severity = 'error' as const;
getTargetGlobs(): string[] {
const config = getConfig();
// Only analyze page files (excluding components)
return config.patterns.pages.filter((p) => !p.includes('components'));
return getConfig().patterns.pages.filter((p) => !p.includes('components'));
}
analyze(_project: Project, files: SourceFile[]): Violation[] {
protected projectConfig(): AstProjectConfig {
return { packages: ['.'], spec: { globs: this.getTargetGlobs() } };
}
analyze(context: { rootDir: string }): Violation[] {
return this.projects(context).flatMap(({ project }) => this.analyzeProject(project));
}
analyzeProject(project: Project): Violation[] {
const violations: Violation[] = [];
for (const file of files) {
for (const file of project.getSourceFiles()) {
const filePath = file.getFilePath();
if (isExcludedPage(filePath)) continue;
if (filePath.includes('/components/')) continue;
// Skip excluded files (facades, base classes)
if (isExcludedPage(filePath)) {
continue;
}
// Skip if file is in components directory
if (filePath.includes('/components/')) {
continue;
}
const importPaths = getImportPaths(file);
for (const importPath of importPaths) {
if (isPageImport(importPath, filePath)) {
const importDecl = file
.getImportDeclarations()
.find((d) => d.getModuleSpecifierValue() === importPath);
if (importDecl) {
violations.push(
this.createViolation(
file,
importDecl.getStartLineNumber(),
0,
`Page imports another page: ${importPath}`,
'Use composition through composables or inject via constructor instead',
),
);
}
for (const decl of file.getImportDeclarations()) {
const spec = decl.getModuleSpecifierValue();
if (isPageImport(spec, filePath)) {
violations.push(
this.nodeViolation(
decl,
`Page imports another page: ${spec}`,
'Use composition through composables or inject via constructor instead',
),
);
}
}
}

24
packages/testing/janitor/src/rules/dead-code.rule.test.ts
View File

@ -30,7 +30,7 @@ page.clickButton();
);
const pageFile = project.getSourceFile('/pages/TestPage.ts')!;
const violations = rule.analyze(project, [pageFile]);
const violations = rule.analyzeProject(project, [pageFile]);
expect(violations).toHaveLength(0);
});
@ -55,7 +55,7 @@ page.usedMethod();
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
expect(violations).toHaveLength(1);
expect(violations[0].message).toContain('unusedMethod');
@ -82,7 +82,7 @@ console.log(page.usedProp);
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
expect(violations).toHaveLength(1);
expect(violations[0].message).toContain('unusedProp');
@ -98,7 +98,7 @@ export class DeadPage {
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
expect(violations).toHaveLength(1);
expect(violations[0].message).toContain('Dead class');
@ -127,7 +127,7 @@ page.publicMethod();
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
expect(violations).toHaveLength(0);
});
@ -152,7 +152,7 @@ page.publicMethod();
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
expect(violations).toHaveLength(0);
});
@ -189,7 +189,7 @@ const h = new MyHelper();
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
// archive() should NOT be flagged — text fallback finds '.archive('
// reallyUnused() SHOULD be flagged — no text match anywhere
@ -225,7 +225,7 @@ const p = new TestPage();
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
expect(violations).toHaveLength(1);
expect(violations[0].message).toContain('deadProp');
@ -251,7 +251,7 @@ thing.saveAll();
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
expect(violations).toHaveLength(1);
expect(violations[0].message).toContain('save');
@ -279,7 +279,7 @@ const arr = [obj.usedInArray, other];
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
expect(violations).toHaveLength(0);
});
@ -306,7 +306,7 @@ thing.$reset();
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
expect(violations).toHaveLength(0);
});
@ -329,7 +329,7 @@ const page = new TestPage();
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
expect(violations).toHaveLength(1);
const fixData = violations[0].fixData;

171
packages/testing/janitor/src/rules/dead-code.rule.ts
View File

@ -1,32 +1,27 @@
import * as fs from 'node:fs';
import { AstRule } from '@n8n/rules-engine/ast';
import type { AstProjectConfig } from '@n8n/rules-engine/ast';
import { SyntaxKind, type Project, type SourceFile } from 'ts-morph';
import { BaseRule } from './base-rule.js';
import { getConfig } from '../config.js';
import { isMethodFix, isPropertyFix } from '../types.js';
import type { Violation, FixResult } from '../types.js';
import { getRelativePath } from '../utils/paths.js';
import type { Violation } from '../types.js';
import { matchesPatterns } from '../utils/paths.js';
/**
* Dead Code Rule
*
* Finds and optionally removes unused methods, properties, and classes.
* Uses reference tracing to detect code that isn't used anywhere in the codebase.
* Finds unused methods, properties, and classes via reference tracing.
*
* Detects:
* - Unused public methods (no external references)
* - Unused public properties (no external references)
* - Dead classes (entire class not referenced externally)
* - Empty classes (no public members)
*
* Supports auto-fixing with --fix --write
*/
export class DeadCodeRule extends BaseRule {
export class DeadCodeRule extends AstRule<{ rootDir: string }> {
readonly id = 'dead-code';
readonly name = 'Dead Code';
readonly description = 'Find and remove unused methods, properties, and classes';
readonly description = 'Find unused methods, properties, and classes';
readonly severity = 'warning' as const;
readonly fixable = true;
getTargetGlobs(): string[] {
const config = getConfig();
@ -38,7 +33,25 @@ export class DeadCodeRule extends BaseRule {
];
}
analyze(project: Project, files: SourceFile[]): Violation[] {
protected projectConfig(): AstProjectConfig {
// Reference resolution needs the whole suite loaded (a method used only by a
// test must be seen as referenced) — so load broadly and filter to targets below.
return {
packages: ['.'],
spec: { globs: ['**/*.ts', '!**/*.d.ts', '!node_modules/**'], resolveDependencies: true },
};
}
analyze(context: { rootDir: string }): Violation[] {
return this.projects(context).flatMap(({ project }) => this.analyzeProject(project));
}
analyzeProject(
project: Project,
files: SourceFile[] = project
.getSourceFiles()
.filter((f) => matchesPatterns(f.getFilePath(), this.getTargetGlobs())),
): Violation[] {
const violations: Violation[] = [];
for (const file of files) {
@ -75,7 +88,7 @@ export class DeadCodeRule extends BaseRule {
classDecl: ReturnType<SourceFile['getClasses']>[0],
className: string,
): Violation {
return this.createViolation(
return this.fileViolation(
file,
classDecl.getStartLineNumber(),
0,
@ -103,7 +116,7 @@ export class DeadCodeRule extends BaseRule {
!this.hasTextUsage(project, file, methodName)
) {
violations.push(
this.createViolation(
this.fileViolation(
file,
method.getStartLineNumber(),
0,
@ -133,7 +146,7 @@ export class DeadCodeRule extends BaseRule {
const propName = prop.getName();
if (!this.hasExternalReferences(file, prop) && !this.hasTextUsage(project, file, propName)) {
violations.push(
this.createViolation(
this.fileViolation(
file,
prop.getStartLineNumber(),
0,
@ -189,130 +202,4 @@ export class DeadCodeRule extends BaseRule {
ref.getStartLineNumber() !== node.getStartLineNumber(),
);
}
fix(project: Project, violations: Violation[], write: boolean): FixResult[] {
const results: FixResult[] = [];
const byFile = this.groupViolationsByFile(violations);
const filesToDelete = new Set<string>();
for (const [filePath, fileViolations] of byFile) {
const sourceFile = project.getSourceFile(filePath);
if (!sourceFile) continue;
// Check if entire file should be deleted
if (this.shouldDeleteFile(sourceFile, fileViolations)) {
filesToDelete.add(filePath);
results.push(this.createFixResult(filePath, 'remove-file', write));
continue;
}
// Process individual member removals
results.push(...this.processFileViolations(sourceFile, filePath, fileViolations, write));
}
// Apply changes
if (write) {
project.saveSync();
this.deleteFiles(filesToDelete);
}
return results;
}
private groupViolationsByFile(violations: Violation[]): Map<string, Violation[]> {
const byFile = new Map<string, Violation[]>();
for (const v of violations) {
if (!v.fixable || !v.fixData) continue;
const existing = byFile.get(v.file) ?? [];
existing.push(v);
byFile.set(v.file, existing);
}
return byFile;
}
private shouldDeleteFile(sourceFile: SourceFile, fileViolations: Violation[]): boolean {
const deadClassViolations = fileViolations.filter((v) => v.fixData?.type === 'class');
const allClassesInFile = sourceFile.getClasses();
return deadClassViolations.length === allClassesInFile.length && allClassesInFile.length > 0;
}
private createFixResult(
filePath: string,
action: 'remove-file' | 'remove-method' | 'remove-property',
write: boolean,
target?: string,
): FixResult {
return {
file: getRelativePath(filePath),
action,
target,
applied: write,
};
}
private processFileViolations(
sourceFile: SourceFile,
filePath: string,
fileViolations: Violation[],
write: boolean,
): FixResult[] {
const results: FixResult[] = [];
for (const violation of fileViolations) {
const fixData = violation.fixData;
if (!fixData) continue;
if (!isMethodFix(fixData) && !isPropertyFix(fixData)) continue;
const classDecl = sourceFile.getClass(fixData.className);
if (!classDecl) continue;
const result = this.removeMember(classDecl, filePath, fixData, write);
if (result) results.push(result);
}
return results;
}
private removeMember(
classDecl: ReturnType<SourceFile['getClass']>,
filePath: string,
fixData: { type: 'method' | 'property'; className: string; memberName: string },
write: boolean,
): FixResult | null {
if (!classDecl) return null;
if (fixData.type === 'method') {
const method = classDecl.getMethod(fixData.memberName);
if (method) {
if (write) method.remove();
return this.createFixResult(
filePath,
'remove-method',
write,
`${fixData.className}.${fixData.memberName}()`,
);
}
} else if (fixData.type === 'property') {
const prop = classDecl.getProperty(fixData.memberName);
if (prop) {
if (write) prop.remove();
return this.createFixResult(
filePath,
'remove-property',
write,
`${fixData.className}.${fixData.memberName}`,
);
}
}
return null;
}
private deleteFiles(filesToDelete: Set<string>): void {
for (const filePath of filesToDelete) {
if (fs.existsSync(filePath)) {
fs.unlinkSync(filePath);
}
}
}
}

10
packages/testing/janitor/src/rules/deduplication.rule.test.ts
View File

@ -28,7 +28,7 @@ export class PageB {
`,
);
const violations = rule.analyze(project, [file1, file2]);
const violations = rule.analyzeProject(project, [file1, file2]);
expect(violations.length).toBeGreaterThanOrEqual(1);
expect(violations[0].message).toContain('my-button');
@ -56,7 +56,7 @@ export class PageB {
`,
);
const violations = rule.analyze(project, [file1, file2]);
const violations = rule.analyzeProject(project, [file1, file2]);
expect(violations).toHaveLength(0);
});
@ -77,7 +77,7 @@ export class PageA {
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
expect(violations).toHaveLength(0);
});
@ -104,7 +104,7 @@ export class ComponentA {
`,
);
const violations = rule.analyze(project, [pageFile, componentFile]);
const violations = rule.analyzeProject(project, [pageFile, componentFile]);
expect(violations).toHaveLength(0);
});
@ -131,7 +131,7 @@ export class PageB {
`,
);
const violations = rule.analyze(project, [file1, file2]);
const violations = rule.analyzeProject(project, [file1, file2]);
expect(violations).toHaveLength(0);
});

17
packages/testing/janitor/src/rules/deduplication.rule.ts
View File

@ -1,6 +1,7 @@
import { AstRule } from '@n8n/rules-engine/ast';
import type { AstProjectConfig } from '@n8n/rules-engine/ast';
import { SyntaxKind, type Project, type SourceFile, type CallExpression } from 'ts-morph';
import { BaseRule } from './base-rule.js';
import { getConfig } from '../config.js';
import type { Violation } from '../types.js';
import { isComponentFile, getRelativePath } from '../utils/paths.js';
@ -30,7 +31,7 @@ interface TestIdUsage {
*
* Only flags when the same {root}:{testId} combination appears in multiple files.
*/
export class DeduplicationRule extends BaseRule {
export class DeduplicationRule extends AstRule<{ rootDir: string }> {
readonly id = 'deduplication';
readonly name = 'Deduplication';
readonly description = 'Test IDs should be unique within their scope';
@ -41,7 +42,15 @@ export class DeduplicationRule extends BaseRule {
return config.patterns.pages;
}
analyze(_project: Project, files: SourceFile[]): Violation[] {
protected projectConfig(): AstProjectConfig {
return { packages: ['.'], spec: { globs: this.getTargetGlobs() } };
}
analyze(context: { rootDir: string }): Violation[] {
return this.projects(context).flatMap(({ project }) => this.analyzeProject(project));
}
analyzeProject(project: Project, files: SourceFile[] = project.getSourceFiles()): Violation[] {
const { pagesScope, componentsScope } = this.buildSelectorMaps(files);
return [
@ -139,7 +148,7 @@ export class DeduplicationRule extends BaseRule {
if (!file) continue;
violations.push(
this.createViolation(
this.fileViolation(
file,
usage.line,
usage.column,

14
packages/testing/janitor/src/rules/duplicate-logic.rule.test.ts
View File

@ -58,7 +58,7 @@ describe('DuplicateLogicRule', () => {
});
const files = project.getSourceFiles();
const violations = rule.analyze(project, files);
const violations = rule.analyzeProject(project, files);
expect(violations.length).toBe(1);
expect(violations[0].message).toContain('Duplicate method logic');
@ -91,7 +91,7 @@ describe('DuplicateLogicRule', () => {
});
const files = project.getSourceFiles();
const violations = rule.analyze(project, files);
const violations = rule.analyzeProject(project, files);
expect(violations.length).toBe(0);
});
@ -116,7 +116,7 @@ describe('DuplicateLogicRule', () => {
});
const files = project.getSourceFiles();
const violations = rule.analyze(project, files);
const violations = rule.analyzeProject(project, files);
// Same file duplicates are allowed (may be intentional)
expect(violations.length).toBe(0);
@ -144,7 +144,7 @@ describe('DuplicateLogicRule', () => {
});
const files = project.getSourceFiles();
const violations = rule.analyze(project, files);
const violations = rule.analyzeProject(project, files);
expect(violations.length).toBe(1);
expect(violations[0].message).toContain('Duplicate test logic');
@ -175,7 +175,7 @@ describe('DuplicateLogicRule', () => {
});
const files = project.getSourceFiles();
const violations = rule.analyze(project, files);
const violations = rule.analyzeProject(project, files);
// Should detect that test duplicates HomePage.login()
expect(violations.length).toBe(1);
@ -204,7 +204,7 @@ describe('DuplicateLogicRule', () => {
});
const files = project.getSourceFiles();
const violations = rule.analyze(project, files);
const violations = rule.analyzeProject(project, files);
// Only 1 statement, below threshold of 2
expect(violations.length).toBe(0);
@ -231,7 +231,7 @@ describe('DuplicateLogicRule', () => {
});
const files = project.getSourceFiles();
const violations = rule.analyze(project, files);
const violations = rule.analyzeProject(project, files);
// 2 statements meets the threshold
expect(violations.length).toBe(1);

21
packages/testing/janitor/src/rules/duplicate-logic.rule.ts
View File

@ -1,3 +1,5 @@
import { AstRule } from '@n8n/rules-engine/ast';
import type { AstProjectConfig } from '@n8n/rules-engine/ast';
import {
SyntaxKind,
type Project,
@ -6,7 +8,6 @@ import {
type MethodDeclaration,
} from 'ts-morph';
import { BaseRule } from './base-rule.js';
import { getConfig } from '../config.js';
import type { Violation } from '../types.js';
import { getRelativePath, matchesPatterns } from '../utils/paths.js';
@ -35,7 +36,7 @@ interface TestFingerprint {
* Detects duplicate code using AST structural fingerprinting.
* Normalizes code to structural patterns, ignoring variable names and literals.
*/
export class DuplicateLogicRule extends BaseRule {
export class DuplicateLogicRule extends AstRule<{ rootDir: string }> {
readonly id = 'duplicate-logic';
readonly name = 'Duplicate Logic';
readonly description = 'Detects duplicate code patterns across tests and page objects';
@ -54,7 +55,15 @@ export class DuplicateLogicRule extends BaseRule {
];
}
analyze(_project: Project, files: SourceFile[]): Violation[] {
protected projectConfig(): AstProjectConfig {
return { packages: ['.'], spec: { globs: this.getTargetGlobs() } };
}
analyze(context: { rootDir: string }): Violation[] {
return this.projects(context).flatMap(({ project }) => this.analyzeProject(project));
}
analyzeProject(project: Project, files: SourceFile[] = project.getSourceFiles()): Violation[] {
const config = getConfig();
// Separate files by type
@ -138,7 +147,7 @@ export class DuplicateLogicRule extends BaseRule {
if (!file) continue;
violations.push(
this.createViolation(
this.fileViolation(
file,
method.line,
0,
@ -173,7 +182,7 @@ export class DuplicateLogicRule extends BaseRule {
if (!file) continue;
violations.push(
this.createViolation(
this.fileViolation(
file,
test.line,
0,
@ -218,7 +227,7 @@ export class DuplicateLogicRule extends BaseRule {
if (!file) continue;
violations.push(
this.createViolation(
this.fileViolation(
file,
test.line,
0,

3
packages/testing/janitor/src/rules/index.ts
View File

@ -1,4 +1,3 @@
export { BaseRule } from './base-rule.js';
export { BoundaryProtectionRule } from './boundary-protection.rule.js';
export { ScopeLockdownRule } from './scope-lockdown.rule.js';
export { SelectorPurityRule } from './selector-purity.rule.js';
@ -11,4 +10,4 @@ export { DuplicateLogicRule } from './duplicate-logic.rule.js';
export { NoDirectPageInstantiationRule } from './no-direct-page-instantiation.rule.js';
// Re-export types for convenience
export type { Violation, FixResult, RuleResult, RuleConfig } from '../types.js';
export type { Violation, RuleResult, RuleConfig } from '../types.js';

14
packages/testing/janitor/src/rules/no-direct-page-instantiation.rule.test.ts
View File

@ -19,7 +19,7 @@ test('my test', async ({ page }) => {
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
expect(violations).toHaveLength(1);
expect(violations[0].message).toContain('Direct page instantiation');
@ -40,7 +40,7 @@ test('my test', async ({ page }) => {
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
expect(violations).toHaveLength(2);
});
@ -55,7 +55,7 @@ test('my test', async ({ page }) => {
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
expect(violations).toHaveLength(1);
expect(violations[0].suggestion).toContain('n8n.canvas');
@ -74,7 +74,7 @@ test('my test', async () => {
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
expect(violations).toHaveLength(0);
});
@ -101,7 +101,7 @@ test('my test', async ({ page }) => {
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
expect(violations).toHaveLength(2);
expect(violations[0].suggestion).toContain('n8n.settingsPersonal');
@ -119,7 +119,7 @@ test('my test', async () => {
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
expect(violations).toHaveLength(0);
});
@ -139,7 +139,7 @@ test('my test', async ({ page }) => {
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
expect(violations).toHaveLength(1);
expect(violations[0].message).toContain('CanvasPage');

22
packages/testing/janitor/src/rules/no-direct-page-instantiation.rule.ts
View File

@ -23,14 +23,16 @@
* - Page instantiation in page object files (for composition)
* - Page instantiation in composables (if they need to create pages)
*/
import { AstRule } from '@n8n/rules-engine/ast';
import type { AstProjectConfig } from '@n8n/rules-engine/ast';
import { SyntaxKind, type Project, type SourceFile } from 'ts-morph';
import { BaseRule } from './base-rule.js';
import { getConfig } from '../config.js';
import { getConfig, ruleAllows } from '../config.js';
import type { Violation } from '../types.js';
import { truncateText } from '../utils/ast-helpers.js';
export class NoDirectPageInstantiationRule extends BaseRule {
export class NoDirectPageInstantiationRule extends AstRule<{ rootDir: string }> {
readonly id = 'no-direct-page-instantiation';
readonly name = 'No Direct Page Instantiation';
readonly description =
@ -42,7 +44,15 @@ export class NoDirectPageInstantiationRule extends BaseRule {
return config.patterns.tests;
}
analyze(_project: Project, files: SourceFile[]): Violation[] {
protected projectConfig(): AstProjectConfig {
return { packages: ['.'], spec: { globs: this.getTargetGlobs() } };
}
analyze(context: { rootDir: string }): Violation[] {
return this.projects(context).flatMap(({ project }) => this.analyzeProject(project));
}
analyzeProject(project: Project, files: SourceFile[] = project.getSourceFiles()): Violation[] {
const violations: Violation[] = [];
for (const file of files) {
@ -53,7 +63,7 @@ export class NoDirectPageInstantiationRule extends BaseRule {
const className = expr.getText();
if (this.isPageInstantiation(className)) {
if (this.isAllowed(className)) {
if (ruleAllows(this.id, className)) {
continue;
}
@ -62,7 +72,7 @@ export class NoDirectPageInstantiationRule extends BaseRule {
const truncated = truncateText(newExpr.getText(), 60);
violations.push(
this.createViolation(
this.fileViolation(
file,
startLine,
startColumn,

14
packages/testing/janitor/src/rules/no-page-in-flow.rule.test.ts
View File

@ -21,7 +21,7 @@ export class WorkflowComposer {
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
expect(violations).toHaveLength(0);
});
@ -40,7 +40,7 @@ export class WorkflowComposer {
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
expect(violations).toHaveLength(1);
expect(violations[0].message).toContain('page.getByTestId');
@ -60,7 +60,7 @@ export class WorkflowComposer {
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
expect(violations).toHaveLength(1);
});
@ -79,7 +79,7 @@ export class WorkflowComposer {
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
expect(violations).toHaveLength(1);
expect(violations[0].suggestion).toContain('navigation');
@ -101,7 +101,7 @@ export class WorkflowComposer {
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
expect(violations).toHaveLength(3);
});
@ -120,7 +120,7 @@ export class WorkflowComposer {
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
expect(violations).toHaveLength(1);
expect(violations[0].line).toBe(6);
@ -140,7 +140,7 @@ export class WorkflowComposer {
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
expect(violations).toHaveLength(1);
expect(violations[0].suggestion).toBeDefined();

21
packages/testing/janitor/src/rules/no-page-in-flow.rule.ts
View File

@ -1,7 +1,8 @@
import { AstRule } from '@n8n/rules-engine/ast';
import type { AstProjectConfig } from '@n8n/rules-engine/ast';
import { SyntaxKind, type Project, type SourceFile } from 'ts-morph';
import { BaseRule } from './base-rule.js';
import { getConfig } from '../config.js';
import { getConfig, ruleAllows } from '../config.js';
import type { Violation } from '../types.js';
import { LOCATOR_METHODS } from '../utils/ast-helpers.js';
@ -28,7 +29,7 @@ import { LOCATOR_METHODS } from '../utils/ast-helpers.js';
* This is stricter than selector-purity, which allows page-level methods
* like keyboard, evaluate, etc. in both composables and tests.
*/
export class NoPageInFlowRule extends BaseRule {
export class NoPageInFlowRule extends AstRule<{ rootDir: string }> {
readonly id = 'no-page-in-flow';
readonly name = 'No Page In Flow';
readonly severity = 'warning' as const;
@ -42,7 +43,15 @@ export class NoPageInFlowRule extends BaseRule {
return getConfig().patterns.flows;
}
analyze(_project: Project, files: SourceFile[]): Violation[] {
protected projectConfig(): AstProjectConfig {
return { packages: ['.'], spec: { globs: this.getTargetGlobs() } };
}
analyze(context: { rootDir: string }): Violation[] {
return this.projects(context).flatMap(({ project }) => this.analyzeProject(project));
}
analyzeProject(project: Project, files: SourceFile[] = project.getSourceFiles()): Violation[] {
const violations: Violation[] = [];
const config = getConfig();
const fixtureName = config.fixtureObjectName;
@ -61,7 +70,7 @@ export class NoPageInFlowRule extends BaseRule {
if (match) {
// Check if this matches any allow patterns
if (this.isAllowed(text)) {
if (ruleAllows(this.id, text)) {
continue;
}
@ -77,7 +86,7 @@ export class NoPageInFlowRule extends BaseRule {
const methodName = match[1];
violations.push(
this.createViolation(
this.fileViolation(
file,
startLine,
startColumn,

16
packages/testing/janitor/src/rules/scope-lockdown.rule.test.ts
View File

@ -22,7 +22,7 @@ export class TestPage {
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
const unscopedCall = violations.find((v) => v.message.includes('Unscoped locator'));
expect(unscopedCall).toBeDefined();
@ -44,7 +44,7 @@ export class TestPage {
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
expect(violations).toHaveLength(0);
});
@ -68,7 +68,7 @@ export class TestPage {
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
expect(violations).toHaveLength(0);
});
@ -87,7 +87,7 @@ export class TestComponent {
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
expect(violations).toHaveLength(0);
});
@ -104,7 +104,7 @@ export class BasePage {
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
expect(violations).toHaveLength(0);
});
@ -125,7 +125,7 @@ export class LoginPage {
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
// Has navigation method = explicit standalone page, no violations
expect(violations).toHaveLength(0);
@ -146,7 +146,7 @@ export class AmbiguousPage {
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
// No container AND no navigation method = ambiguous
expect(violations).toHaveLength(1);
@ -169,7 +169,7 @@ export class SettingsPage {
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
// 'navigate' is in default config, so this should pass
expect(violations).toHaveLength(0);

19
packages/testing/janitor/src/rules/scope-lockdown.rule.ts
View File

@ -1,3 +1,5 @@
import { AstRule } from '@n8n/rules-engine/ast';
import type { AstProjectConfig } from '@n8n/rules-engine/ast';
import {
SyntaxKind,
type Project,
@ -6,7 +8,6 @@ import {
type ClassDeclaration,
} from 'ts-morph';
import { BaseRule } from './base-rule.js';
import { getConfig } from '../config.js';
import type { Violation } from '../types.js';
import {
@ -69,7 +70,7 @@ function hasNavigationMethod(classDecl: ClassDeclaration, methodNames: string[])
* - this.page.goto(), this.page.waitForURL() etc. (page-level operations)
* - Pages with navigation methods (explicit standalone pages)
*/
export class ScopeLockdownRule extends BaseRule {
export class ScopeLockdownRule extends AstRule<{ rootDir: string }> {
readonly id = 'scope-lockdown';
readonly name = 'Scope Lockdown';
readonly description = 'Page locators must be scoped to container';
@ -85,7 +86,15 @@ export class ScopeLockdownRule extends BaseRule {
return config.patterns.pages;
}
analyze(_project: Project, files: SourceFile[]): Violation[] {
protected projectConfig(): AstProjectConfig {
return { packages: ['.'], spec: { globs: this.getTargetGlobs() } };
}
analyze(context: { rootDir: string }): Violation[] {
return this.projects(context).flatMap(({ project }) => this.analyzeProject(project));
}
analyzeProject(project: Project, files: SourceFile[] = project.getSourceFiles()): Violation[] {
const violations: Violation[] = [];
const navigationMethods = this.getNavigationMethods();
@ -113,7 +122,7 @@ export class ScopeLockdownRule extends BaseRule {
const startColumn = classDecl.getStart() - classDecl.getStartLinePos();
violations.push(
this.createViolation(
this.fileViolation(
file,
startLine,
startColumn,
@ -149,7 +158,7 @@ export class ScopeLockdownRule extends BaseRule {
const startColumn = call.getStart() - call.getStartLinePos();
violations.push(
this.createViolation(
this.fileViolation(
file,
startLine,
startColumn,

16
packages/testing/janitor/src/rules/selector-purity.rule.test.ts
View File

@ -20,7 +20,7 @@ export class TestComposer {
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
expect(violations.length).toBeGreaterThan(0);
});
@ -40,7 +40,7 @@ export class TestComposer {
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
expect(violations).toHaveLength(0);
});
@ -62,7 +62,7 @@ export class TestComposer {
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
expect(violations).toHaveLength(0);
});
@ -77,7 +77,7 @@ test('my test', async ({ n8n }) => {
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
expect(violations.length).toBeGreaterThan(0);
});
@ -98,7 +98,7 @@ export class TestComposer {
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
expect(violations.length).toBeGreaterThanOrEqual(3);
});
@ -115,7 +115,7 @@ test('my test', async ({ n8n }) => {
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
expect(violations.length).toBeGreaterThan(0);
expect(violations[0].message).toContain('Chained locator call');
@ -134,7 +134,7 @@ test('my test', async ({ n8n }) => {
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
expect(violations.length).toBeGreaterThan(0);
expect(violations[0].message).toContain('Chained locator call');
@ -159,7 +159,7 @@ export class TestComposer {
`,
);
const violations = rule.analyze(project, [file]);
const violations = rule.analyzeProject(project, [file]);
expect(violations).toHaveLength(0);
});

23
packages/testing/janitor/src/rules/selector-purity.rule.ts
View File

@ -1,6 +1,7 @@
import { AstRule } from '@n8n/rules-engine/ast';
import type { AstProjectConfig } from '@n8n/rules-engine/ast';
import { SyntaxKind, type Project, type SourceFile, type CallExpression } from 'ts-morph';
import { BaseRule } from './base-rule.js';
import { getConfig } from '../config.js';
import type { Violation } from '../types.js';
import { LOCATOR_METHODS, PAGE_LEVEL_METHODS, truncateText } from '../utils/ast-helpers.js';
@ -29,7 +30,7 @@ import { matchesPatterns } from '../utils/paths.js';
* Configuration:
* - allowInExpect: Skip violations inside expect() calls (default: false)
*/
export class SelectorPurityRule extends BaseRule {
export class SelectorPurityRule extends AstRule<{ rootDir: string }> {
readonly id = 'selector-purity';
readonly name = 'Selector Purity';
readonly description = 'Composables and tests should use page objects, not direct locators';
@ -40,7 +41,15 @@ export class SelectorPurityRule extends BaseRule {
return [...config.patterns.flows, ...config.patterns.tests];
}
analyze(_project: Project, files: SourceFile[]): Violation[] {
protected projectConfig(): AstProjectConfig {
return { packages: ['.'], spec: { globs: this.getTargetGlobs() } };
}
analyze(context: { rootDir: string }): Violation[] {
return this.projects(context).flatMap(({ project }) => this.analyzeProject(project));
}
analyzeProject(project: Project, files: SourceFile[] = project.getSourceFiles()): Violation[] {
const violations: Violation[] = [];
const config = getConfig();
@ -70,7 +79,7 @@ export class SelectorPurityRule extends BaseRule {
// Skip violations inside expect() if allowInExpect is enabled
// Check both runtime config and config file settings
const ruleSettings = getConfig().rules?.[this.id];
const allowInExpect = this.config.allowInExpect ?? ruleSettings?.allowInExpect;
const allowInExpect = this.getOptions().allowInExpect ?? ruleSettings?.allowInExpect;
if (allowInExpect && this.isInsideExpect(call)) {
continue;
}
@ -83,7 +92,7 @@ export class SelectorPurityRule extends BaseRule {
const suggestion = this.getSuggestion(text);
violations.push(
this.createViolation(
this.fileViolation(
file,
startLine,
startColumn,
@ -102,7 +111,7 @@ export class SelectorPurityRule extends BaseRule {
// Skip violations inside expect() if allowInExpect is enabled
const ruleSettings = getConfig().rules?.[this.id];
const allowInExpect = this.config.allowInExpect ?? ruleSettings?.allowInExpect;
const allowInExpect = this.getOptions().allowInExpect ?? ruleSettings?.allowInExpect;
if (allowInExpect && this.isInsideExpect(call)) {
continue;
}
@ -112,7 +121,7 @@ export class SelectorPurityRule extends BaseRule {
const truncated = truncateText(call.getText(), 60);
violations.push(
this.createViolation(
this.fileViolation(
file,
startLine,
startColumn,

59
packages/testing/janitor/src/rules/test-data-hygiene.rule.ts
View File

@ -1,12 +1,11 @@
import * as fs from 'node:fs';
import { AstRule } from '@n8n/rules-engine/ast';
import type { AstProjectConfig } from '@n8n/rules-engine/ast';
import * as path from 'node:path';
import { SyntaxKind, type Project, type SourceFile } from 'ts-morph';
import { BaseRule } from './base-rule.js';
import { getConfig } from '../config.js';
import { isEditFix } from '../types.js';
import type { Violation, FixResult } from '../types.js';
import { getRootDir, getRelativePath, getTestDataFiles } from '../utils/paths.js';
import type { Violation } from '../types.js';
import { getRootDir, getTestDataFiles } from '../utils/paths.js';
/**
* Test Data Hygiene Rule
@ -24,12 +23,11 @@ import { getRootDir, getRelativePath, getTestDataFiles } from '../utils/paths.js
* - Workflows: filename must be referenced in test files
* - Expectations: folder must be referenced via loadExpectations()
*/
export class TestDataHygieneRule extends BaseRule {
export class TestDataHygieneRule extends AstRule<{ rootDir: string }> {
readonly id = 'test-data-hygiene';
readonly name = 'Test Data Hygiene';
readonly description = 'Ensure test data files are well-named and actually used';
readonly severity = 'warning' as const;
readonly fixable = true;
// Patterns that indicate poor naming
private readonly badNamePatterns = [
@ -58,7 +56,15 @@ export class TestDataHygieneRule extends BaseRule {
];
}
analyze(project: Project, _files: SourceFile[]): Violation[] {
protected projectConfig(): AstProjectConfig {
return { packages: ['.'], spec: { globs: this.getTargetGlobs() } };
}
analyze(context: { rootDir: string }): Violation[] {
return this.projects(context).flatMap(({ project }) => this.analyzeProject(project));
}
analyzeProject(project: Project, _files?: SourceFile[]): Violation[] {
const violations: Violation[] = [];
const config = getConfig();
const root = getRootDir();
@ -99,8 +105,6 @@ export class TestDataHygieneRule extends BaseRule {
root,
`Orphaned test data: ${fileName} is not referenced in any test`,
'Remove the file or add a test that uses it',
true, // fixable
{ filePath: dataFile },
),
);
}
@ -207,8 +211,6 @@ export class TestDataHygieneRule extends BaseRule {
root: string,
message: string,
suggestion: string,
fixable?: boolean,
fixData?: { filePath: string },
): Violation {
return {
file: path.join(root, relativePath),
@ -218,39 +220,6 @@ export class TestDataHygieneRule extends BaseRule {
message,
severity: this.severity,
suggestion,
fixable,
fixData: fixData ? { type: 'edit', replacement: JSON.stringify(fixData) } : undefined,
};
}
fix(_project: Project, violations: Violation[], write: boolean): FixResult[] {
const results: FixResult[] = [];
for (const violation of violations) {
if (!violation.fixable || !violation.fixData) continue;
if (!isEditFix(violation.fixData)) continue;
try {
const data = JSON.parse(violation.fixData.replacement) as { filePath: string };
const filePath = data.filePath;
const relativePath = getRelativePath(filePath);
results.push({
file: relativePath,
action: 'remove-file',
target: path.basename(filePath),
applied: write,
});
if (write && fs.existsSync(filePath)) {
fs.unlinkSync(filePath);
}
} catch {
// Skip invalid fix data
}
}
return results;
}
}

23
packages/testing/janitor/src/types.ts
View File

@ -1,6 +1,3 @@
import type { Violation, RuleResult, FixResult } from '@n8n/rules-engine';
import type { Project, SourceFile } from 'ts-morph';
export type {
Project,
SourceFile,
@ -17,8 +14,6 @@ export type {
Violation,
RuleResult,
ReportSummary,
FixAction,
FixResult,
FixData,
} from '@n8n/rules-engine';
export type { Report as JanitorReport } from '@n8n/rules-engine';
@ -57,8 +52,6 @@ export type RuleSettingsMap = {
export interface RunOptions {
files?: string[];
ruleConfig?: Record<string, RuleConfig>;
fix?: boolean;
write?: boolean;
}
// Janitor-specific fix data narrowing
@ -107,22 +100,6 @@ export function isEditFix(data: { type: string }): data is EditFixData {
return data.type === 'edit';
}
export interface Rule {
readonly id: string;
readonly name: string;
readonly description: string;
readonly severity: 'error' | 'warning' | 'info';
readonly fixable: boolean;
getTargetGlobs(): string[];
analyze(project: Project, files: SourceFile[]): Violation[];
fix?(project: Project, violations: Violation[], write: boolean): FixResult[];
isEnabled(): boolean;
getEffectiveSeverity(): 'error' | 'warning' | 'info';
configure(config: RuleConfig): void;
execute(project: Project, files: SourceFile[]): RuleResult;
}
export interface FilePatterns {
pages: string[];
components: string[];

13
packages/testing/rules-engine/package.json
View File

@ -5,6 +5,16 @@
"description": "Lightweight rules engine for registering, running, and reporting rule violations",
"main": "dist/index.js",
"types": "dist/index.d.ts",
"exports": {
".": {
"types": "./dist/index.d.ts",
"default": "./dist/index.js"
},
"./ast": {
"types": "./dist/ast/index.d.ts",
"default": "./dist/ast/index.js"
}
},
"files": [
"dist"
],
@ -23,6 +33,9 @@
"typecheck": "tsc --noEmit"
},
"license": "MIT",
"dependencies": {
"ts-morph": "catalog:"
},
"devDependencies": {
"@n8n/vitest-config": "workspace:*",
"@vitest/coverage-v8": "catalog:",

63
packages/testing/rules-engine/src/ast/ast-rule.ts Normal file
View File

@ -0,0 +1,63 @@
import type { Node, SourceFile } from 'ts-morph';
import { BaseRule } from '../base-rule.js';
import type { FixData, Violation } from '../types.js';
import { buildPackageProjects } from './project.js';
import type { AstProjectConfig, PackageProject } from './project.js';
/**
* Base for AST-backed rules. A rule opts into ts-morph by overriding
* {@link projectConfig}; rules needing no AST leave it undefined and pay nothing.
* The engine core stays ts-morph-free — this lives behind the `/ast` entry point.
*/
export abstract class AstRule<
TContext extends { rootDir: string } = { rootDir: string },
> extends BaseRule<TContext> {
protected projectConfig(_context: TContext): AstProjectConfig | undefined {
return undefined;
}
protected projects(context: TContext): PackageProject[] {
const config = this.projectConfig(context);
return config ? buildPackageProjects(context.rootDir, config) : [];
}
protected nodeViolation(
node: Node,
message: string,
suggestion?: string,
fixable?: boolean,
fixData?: FixData,
): Violation {
return this.createViolation(
node.getSourceFile().getFilePath(),
node.getStartLineNumber(),
0,
message,
suggestion,
fixable,
fixData,
);
}
/** Create a violation for a source file at an explicit line/column. */
protected fileViolation(
file: SourceFile,
line: number,
column: number,
message: string,
suggestion?: string,
fixable?: boolean,
fixData?: FixData,
): Violation {
return this.createViolation(
file.getFilePath(),
line,
column,
message,
suggestion,
fixable,
fixData,
);
}
}

24
packages/testing/rules-engine/src/ast/decorators.ts Normal file
View File

@ -0,0 +1,24 @@
import { Node } from 'ts-morph';
import type { ClassDeclaration, Decorator, MethodDeclaration } from 'ts-morph';
type Decoratable = ClassDeclaration | MethodDeclaration;
export function getDecoratorByName(
node: Decoratable,
names: Iterable<string>,
): Decorator | undefined {
const set = names instanceof Set ? names : new Set(names);
return node.getDecorators().find((d) => set.has(d.getName()));
}
export function classHasDecorator(cls: ClassDeclaration, names: Iterable<string>): boolean {
return getDecoratorByName(cls, names) !== undefined;
}
/** Read a boolean flag from a decorator's options-object arg, e.g. `@Get('/x', { skipAuth: true })`. */
export function getDecoratorObjectFlag(deco: Decorator, index: number, flag: string): boolean {
const arg = deco.getArguments()[index];
if (!arg || !Node.isObjectLiteralExpression(arg)) return false;
const prop = arg.getProperty(flag);
return !!prop && Node.isPropertyAssignment(prop) && prop.getInitializer()?.getText() === 'true';
}

4
packages/testing/rules-engine/src/ast/index.ts Normal file
View File

@ -0,0 +1,4 @@
export { AstRule } from './ast-rule.js';
export { buildPackageProjects, createInMemoryProject } from './project.js';
export type { AstProjectConfig, PackageProjectSpec, PackageProject } from './project.js';
export { classHasDecorator, getDecoratorByName, getDecoratorObjectFlag } from './decorators.js';

51
packages/testing/rules-engine/src/ast/project.ts Normal file
View File

@ -0,0 +1,51 @@
import { Project } from 'ts-morph';
import type { SourceFile } from 'ts-morph';
export interface PackageProjectSpec {
/** Globs relative to the package dir; `!`-prefixed entries exclude. Defaults to source `.ts`, no tests. */
globs?: string[];
/** Resolve cross-file imports — required by reference-based rules (dead-code, impact). Off by default. */
resolveDependencies?: boolean;
}
/** A rule's project requirement. Rules needing no AST return undefined. */
export interface AstProjectConfig {
/** Package dirs relative to the repo root, e.g. `'packages/cli'`. */
packages: string[];
spec?: PackageProjectSpec;
/** Per-package overrides, merged over `spec`. */
overrides?: Record<string, PackageProjectSpec>;
}
export interface PackageProject {
package: string;
project: Project;
files: SourceFile[];
}
const DEFAULT_GLOBS = ['src/**/*.ts', '!src/**/*.test.ts'];
/** POSIX-join glob segments — `\` breaks globby/minimatch on Windows. */
const toGlob = (...segments: string[]): string =>
segments.join('/').replace(/\\/g, '/').replace(/\/+/g, '/');
/** One scoped Project per package — bounded memory, and `resolveDependencies` can differ per package. */
export function buildPackageProjects(rootDir: string, config: AstProjectConfig): PackageProject[] {
return config.packages.map((pkg) => {
const spec: PackageProjectSpec = { ...config.spec, ...config.overrides?.[pkg] };
const project = new Project({
skipAddingFilesFromTsConfig: true,
skipFileDependencyResolution: !spec.resolveDependencies,
});
const patterns = (spec.globs ?? DEFAULT_GLOBS).map((g) =>
g.startsWith('!') ? `!${toGlob(rootDir, pkg, g.slice(1))}` : toGlob(rootDir, pkg, g),
);
const files = project.addSourceFilesAtPaths(patterns);
return { package: pkg, project, files };
});
}
/** In-memory Project for unit tests — no real filesystem. */
export function createInMemoryProject(): Project {
return new Project({ useInMemoryFileSystem: true });
}

7
pnpm-lock.yaml
View File

@ -5030,6 +5030,9 @@ importers:
fast-glob:
specifier: 'catalog:'
version: 3.2.12
ts-morph:
specifier: 'catalog:'
version: 27.0.2
tsx:
specifier: 'catalog:'
version: 4.19.3
@ -5234,6 +5237,10 @@ importers:
version: 3.25.67
packages/testing/rules-engine:
dependencies:
ts-morph:
specifier: 'catalog:'
version: 27.0.2
devDependencies:
'@n8n/vitest-config':
specifier: workspace:*