diff --git a/skills/coderuler-new-rule/reference.md b/skills/coderuler-new-rule/reference.md index 3067a1a..a0ac979 100644 --- a/skills/coderuler-new-rule/reference.md +++ b/skills/coderuler-new-rule/reference.md @@ -334,6 +334,25 @@ Results validate; supplied messages survive reporting. Violations without a mess `changedFiles` contains paths that changed in the project. `changedLines` maps changed paths to added or replaced lines. These values are absent outside `--diff` and `--pr` runs. In those runs, an empty set means no changed files. An empty map means no exact line ranges are available. A missing entry for a path means line data is unavailable, for example for binary or conversion-attributed files. An entry with `[]` means the file changed but has no added lines. +`isDiffMode(context)` returns `true` when either `changedFiles` or `changedLines` is defined, even if empty; it returns `false` when both are absent. `isInDiff(subject, context)` returns `true` when the subject's line span overlaps a known changed range; a missing span or range returns `false`. + +Use both helpers to keep full scans outside diff mode: + +```ts +import { + Rule, + isDiffMode, + isInDiff, + type ProjectContext, + type ScopeSubject, +} from '@alex-dbs/coderuler'; + +export default class Local extends Rule { + filter = (subject: ScopeSubject, context: ProjectContext) => + !isDiffMode(context) || isInDiff(subject, context); +} +``` + Use `trace` for decision evidence visible during inspection; keep normal execution free of ad-hoc console output. AI/regex projection callbacks receive only `ProjectContext`, not the full check context. diff --git a/src/app/rule.ts b/src/app/rule.ts index efef452..e9976c7 100644 --- a/src/app/rule.ts +++ b/src/app/rule.ts @@ -140,6 +140,21 @@ export interface CheckContext extends SelectionContext { ai(options: AiOptions): Promise; } +export function isDiffMode(context: ProjectContext): boolean { + return context.changedFiles !== undefined || context.changedLines !== undefined; +} + +export function isInDiff(subject: ScopeSubject, context: ProjectContext): boolean { + const lines = subject.lines; + const ranges = context.changedLines?.get(subject.filepath); + if (lines === undefined || ranges === undefined) return false; + for (const range of ranges) { + if (range.start <= lines.end && lines.start <= range.end) return true; + } + return false; +} + + export const RULE_BRAND: symbol = Symbol.for('coderuler.Rule'); export abstract class Rule { diff --git a/src/index.ts b/src/index.ts index 3a4a4fc..daf0d86 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,4 +1,4 @@ -export { Rule } from './app/rule.js'; +export { Rule, isDiffMode, isInDiff } from './app/rule.js'; export { test } from './app/rule-tests.js'; export type { RuleTestCase } from './app/rule-tests.js'; export type { diff --git a/tests/cli-diff.test.ts b/tests/cli-diff.test.ts index 7ce6cd0..c71bd0d 100644 --- a/tests/cli-diff.test.ts +++ b/tests/cli-diff.test.ts @@ -493,6 +493,64 @@ test('nested project cwd uses project-relative paths rather than repository-rela ]); }); +test('diff filters queried scopes with isInDiff and keeps full scans explicit', async () => { + const target = repository('diff-filter'); + writeFile(join(target.cwd, '.coderuler/local.rule.ts'), `import { Rule, isInDiff, scopes, violation } from '@alex-dbs/coderuler'; +export default class Local extends Rule { + files = ['src/*.ts']; + scope = scopes.astGrep({ language: 'typescript', rule: { kind: 'function_declaration' } }); + filter(subject, context) { + return context.changedLines === undefined || isInDiff(subject, context); + } + check = () => violation('Review function'); +} +`); + files(target, { + 'src/a.ts': 'function unchanged() {\n return 1;\n}\n\nfunction changed() {\n return 1;\n}\n', + }); + const base = commit(target); + files(target, { + 'src/a.ts': 'function unchanged() {\n return 1;\n}\n\nfunction changed() {\n return 2;\n}\n', + }); + + const projectEvaluations = (report: CheckReportShape) => report.evaluations.map(({ subject, result }) => { + assert.equal(subject.kind, 'scope'); + if (subject.kind !== 'scope') throw new Error('Expected a scope'); + return { + filepath: subject.filepath, + source: subject.source, + lines: subject.lines, + result: { status: result.status, message: result.message }, + }; + }); + + const diff = await runCli(['check', '.', '--diff-only', '--base', base, '--json', '--output-all'], target); + assert.equal(diff.status, 1, diff.stderr); + assert.deepEqual(projectEvaluations(parseCheckOutput(diff, 'diff filter')[0]!), [{ + filepath: 'src/a.ts', + source: 'function changed() {\n return 2;\n}', + lines: { start: 5, end: 7 }, + result: { status: 'violation', message: 'Review function' }, + }]); + + const full = await runCli(['check', '.', '--json', '--output-all'], target); + assert.equal(full.status, 1, full.stderr); + assert.deepEqual(projectEvaluations(parseCheckOutput(full, 'full scan')[0]!), [ + { + filepath: 'src/a.ts', + source: 'function unchanged() {\n return 1;\n}', + lines: { start: 1, end: 3 }, + result: { status: 'violation', message: 'Review function' }, + }, + { + filepath: 'src/a.ts', + source: 'function changed() {\n return 2;\n}', + lines: { start: 5, end: 7 }, + result: { status: 'violation', message: 'Review function' }, + }, + ]); +}); + test('custom selectors cannot evaluate unchanged subjects during diff checks', async () => { const target = repository('custom-select'); rule(target, 'custom', `async select(context) { diff --git a/tests/contracts.test.ts b/tests/contracts.test.ts index 5286d34..a357d67 100644 --- a/tests/contracts.test.ts +++ b/tests/contracts.test.ts @@ -5,6 +5,8 @@ import { test } from 'node:test'; import { Rule, ai, + isDiffMode, + isInDiff, pass, regex, violation, @@ -228,6 +230,96 @@ function expectThrows(label: string, run: () => unknown): void { assert.ok(threw, `expected a thrown error for ${label}`); } +test('isDiffMode detects available diff data', () => { + const cases: readonly (readonly [string, ProjectContext, boolean])[] = [ + ['no diff data', { project: new MemoryProject(), trace: () => {} }, false], + ['changedFiles is present', { + project: new MemoryProject(), + changedFiles: new Set(), + trace: () => {}, + }, true], + ['changedLines is present', { + project: new MemoryProject(), + changedLines: new Map(), + trace: () => {}, + }, true], + ]; + + for (const [label, context, expected] of cases) { + assert.equal(isDiffMode(context), expected, label); + } +}); + +test('isInDiff matches inclusive changed-line ranges', () => { + const subject = scopeSubject({ + filepath: 'src/a.ts', + id: 'function', + source: 'function', + lines: { start: 5, end: 7 }, + }); + const cases: readonly (readonly [string, readonly { start: number; end: number }[], boolean])[] = [ + ['touching the scope start', [{ start: 4, end: 5 }], true], + ['touching the scope end', [{ start: 7, end: 8 }], true], + ['inside the scope', [{ start: 6, end: 6 }], true], + ['containing the scope', [{ start: 1, end: 10 }], true], + ['before the scope', [{ start: 1, end: 4 }], false], + ['after the scope', [{ start: 8, end: 10 }], false], + ['scope in a gap', [{ start: 1, end: 3 }, { start: 9, end: 10 }], false], + ['overlap in a later range', [{ start: 1, end: 2 }, { start: 6, end: 6 }], true], + ['deletion-only change', [], false], + ]; + + for (const [label, ranges, expected] of cases) { + const context: ProjectContext = { + project: new MemoryProject(), + changedLines: new Map([['src/a.ts', ranges]]), + trace: () => {}, + }; + assert.equal(isInDiff(subject, context), expected, label); + } +}); + +test('isInDiff requires scope lines and known changed-line ranges', () => { + const subject = scopeSubject({ + filepath: 'src/a.ts', + id: 'function', + source: 'function', + lines: { start: 5, end: 7 }, + }); + const contexts: readonly (readonly [string, ProjectContext])[] = [ + ['absent changedLines map', { project: new MemoryProject(), trace: () => {} }], + ['empty changedLines map', { + project: new MemoryProject(), + changedLines: new Map(), + trace: () => {}, + }], + ['another filepath only', { + project: new MemoryProject(), + changedLines: new Map([['src/b.ts', [{ start: 5, end: 7 }]]]), + trace: () => {}, + }], + ['changed filepath missing from map', { + project: new MemoryProject(), + changedFiles: new Set(['src/a.ts']), + changedLines: new Map(), + trace: () => {}, + }], + ]; + + for (const [label, context] of contexts) { + assert.equal(isInDiff(subject, context), false, label); + } + + const knownRanges: ProjectContext = { + project: new MemoryProject(), + changedLines: new Map([['src/a.ts', [{ start: 5, end: 7 }]]]), + trace: () => {}, + }; + const subjectWithoutLines = scopeSubject({ filepath: 'src/a.ts', id: 'function', source: 'function' }); + assert.equal(isInDiff(subjectWithoutLines, knownRanges), false); +}); + + test('Rule exposes its documented defaults', async () => { const rule = new DefaultsRule(); assert.equal(rule.enabled, true); diff --git a/website/src/content/docs/writing-rules.md b/website/src/content/docs/writing-rules.md index 3dfe7e9..43b6f1f 100644 --- a/website/src/content/docs/writing-rules.md +++ b/website/src/content/docs/writing-rules.md @@ -133,17 +133,35 @@ context.changedFiles // ReadonlySet of changed project-relative paths context.changedLines // ReadonlyMap ``` -Both values are `undefined` when no diff was collected. In diff mode, empty collections mean no changes. Ranges are 1-based, inclusive added or replaced lines in the current file. A known changed path can have `[]` when it has no added lines. If a path is missing from `changedLines`, its line information is unavailable, for example for binary or conversion-attributed files; it is not unchanged. +Both values are `undefined` when no diff was collected. `isDiffMode(context)` returns `true` when either value is available. In diff mode, empty collections mean no changes. `changedLines` contains 1-based, inclusive ranges for added or replaced lines in the current file. A known changed path can have `[]` when it has no added lines. If a path is missing from `changedLines`, its line information is unavailable, for example for binary or conversion-attributed files. -For example, select only scopes whose line spans overlap a changed range: +Use `isInDiff` in `filter` to keep only scopes that overlap a changed range. It returns `false` when diff ranges or scope line data are unavailable. ```ts -const ranges = context.changedLines?.get(filepath); -const overlaps = scope.lines !== undefined && ranges?.some(range => - range.start <= scope.lines.end && scope.lines.start <= range.end) === true; +import { Rule, isInDiff } from '@alex-dbs/coderuler'; + +class Local extends Rule { + filter = isInDiff; +} +``` + +Use `isDiffMode` to preserve full scans when no diff was collected: + +```ts +import { + Rule, + isDiffMode, + isInDiff, + type ProjectContext, + type ScopeSubject, +} from '@alex-dbs/coderuler'; + +class Local extends Rule { + filter = (subject: ScopeSubject, context: ProjectContext) => + !isDiffMode(context) || isInDiff(subject, context); +} ``` -When diff data is absent, a selector can keep its full-scan behavior. In diff mode, use `changedFiles` to distinguish changed paths from unchanged paths and map presence to distinguish unavailable line information from an empty range list. | Project reader | Result | | --- | --- |