mirror of
https://github.com/stablyai/orca.git
synced 2026-09-26 16:02:43 +00:00
`anti-slop/no-pass-through-type-alias` rejects a type alias whose entire right-hand side is a bare reference to another named type, including the generic form where every type parameter is forwarded positionally and unchanged (`type A<T> = B<T>`). Those aliases add a second name for one type: readers have to resolve the indirection, "go to definition" lands on a rename rather than the shape, and the two names drift apart in review. Flipped the rule from "off" to "error" in `config/oxlint-anti-slop.json` and fixed all 195 violations reported across `src`, `config`, `tests`, and `mobile`. Fix approach, in order of preference per site: - Delete the alias and use the target type directly at every reference, updating imports. This covers the large majority of the 195. - Where the alias name was the better or more widely used name, rename the target declaration to the alias name instead of renaming call sites (for example `GitUncommittedEntry` -> `GitStatusEntry` in `src/shared/git-status-types.ts`). - Where a pass-through sat in front of a type that was itself only used through that alias, collapse the pair into a single declaration that keeps the real shape (intersection, `Pick`/`Omit`, or union) under one name. No alias was converted into an equivalent `interface X extends Y` to dodge the rule, and no new pass-through was introduced. No suppressions were added. The vendored anti-slop plugin source under `config/oxlint-plugins/anti-slop/` is excluded from the audit by the `--ignore-pattern` flag in `audit:anti-slop`, and stays byte-identical to upstream. Verified: - `npx oxlint --config config/oxlint-anti-slop.json --ignore-pattern 'config/oxlint-plugins/anti-slop/**' src config tests mobile` exits 0 (195 -> 0; baseline counted on a scratch worktree of `nwparker/2x-lint` with the rule flipped on). - `node config/scripts/run-typecheck-projects-in-parallel.mjs` exits 0. - `cd mobile && pnpm typecheck` exits 0 (the parallel script covers only the three desktop tsconfigs). - Root vitest over the changed files and their sibling tests: 210 files, 4072 passed, 1 skipped. - Mobile vitest over the changed files and their sibling tests: 36 files, 445 passed. - `npx oxlint` with the repo's default config over every changed file (root and mobile) exits 0. - `npx oxfmt --write` run over the changed files in both workspaces.
180 lines
5.2 KiB
TypeScript
180 lines
5.2 KiB
TypeScript
import type { GitStatusEntry } from '../../../src/shared/git-status-types'
|
|
import { describe, expect, it } from 'vitest'
|
|
import type { DiffComment, MobileDiffReviewState } from '../../../src/shared/diff-comment-types'
|
|
import type { GitBranchChangeEntry } from '../../../src/shared/git-diff-compare-types'
|
|
import {
|
|
buildMobileDiffReviewQueue,
|
|
createMobileDiffReviewFileKey,
|
|
filterMobileDiffReviewQueue,
|
|
summarizeMobileDiffReviewQueue,
|
|
type MobileDiffReviewQueueItem
|
|
} from './mobile-diff-review-queue'
|
|
|
|
const emptyReviewState: MobileDiffReviewState = { version: 1, files: {} }
|
|
|
|
function statusEntry(overrides: Partial<GitStatusEntry>): GitStatusEntry {
|
|
return {
|
|
path: 'src/app.ts',
|
|
status: 'modified',
|
|
area: 'unstaged',
|
|
...overrides
|
|
}
|
|
}
|
|
|
|
function branchEntry(overrides: Partial<GitBranchChangeEntry>): GitBranchChangeEntry {
|
|
return {
|
|
path: 'src/branch.ts',
|
|
status: 'modified',
|
|
...overrides
|
|
}
|
|
}
|
|
|
|
function comment(overrides: Partial<DiffComment> & Pick<DiffComment, 'id'>): DiffComment {
|
|
const { id, ...rest } = overrides
|
|
return {
|
|
id,
|
|
worktreeId: 'wt-1',
|
|
filePath: 'src/app.ts',
|
|
source: 'diff',
|
|
lineNumber: 2,
|
|
body: 'note',
|
|
createdAt: 10,
|
|
side: 'modified',
|
|
...rest
|
|
}
|
|
}
|
|
|
|
describe('mobile diff review queue', () => {
|
|
it('builds unstaged, staged, and branch entries in review order', () => {
|
|
const queue = buildMobileDiffReviewQueue({
|
|
worktreeId: 'wt-1',
|
|
statusEntries: [
|
|
statusEntry({ path: 'z.ts', area: 'staged' }),
|
|
statusEntry({ path: 'a.ts', area: 'unstaged' })
|
|
],
|
|
branchEntries: [branchEntry({ path: 'b.ts' })],
|
|
branchHeadOid: 'head',
|
|
branchMergeBase: 'base',
|
|
comments: [],
|
|
reviewState: emptyReviewState
|
|
})
|
|
|
|
expect(queue.map((item) => `${item.scope}:${item.filePath}`)).toEqual([
|
|
'unstaged:a.ts',
|
|
'staged:z.ts',
|
|
'branch:b.ts'
|
|
])
|
|
})
|
|
|
|
it('uses stable keys for renamed files', () => {
|
|
expect(createMobileDiffReviewFileKey('branch', 'branch', 'new.ts', 'old.ts')).toBe(
|
|
'branch\0branch\0old.ts\0new.ts'
|
|
)
|
|
})
|
|
|
|
it('counts unsent and stale notes for matching review items', () => {
|
|
const queue = buildMobileDiffReviewQueue({
|
|
worktreeId: 'wt-1',
|
|
statusEntries: [statusEntry({ path: 'src/app.ts', area: 'unstaged' })],
|
|
branchEntries: [],
|
|
comments: [
|
|
comment({ id: 'a', scope: 'unstaged', diffIdentity: 'stale' }),
|
|
comment({ id: 'b', scope: 'unstaged', sentAt: 20 })
|
|
],
|
|
reviewState: emptyReviewState
|
|
})
|
|
|
|
expect(queue[0]).toMatchObject({ noteCount: 2, unsentNoteCount: 1, staleNoteCount: 1 })
|
|
})
|
|
|
|
it('filters unreviewed files and noted files', () => {
|
|
const reviewState: MobileDiffReviewState = {
|
|
version: 1,
|
|
files: {
|
|
[createMobileDiffReviewFileKey('unstaged', 'unstaged', 'a.ts')]: {
|
|
key: createMobileDiffReviewFileKey('unstaged', 'unstaged', 'a.ts'),
|
|
filePath: 'a.ts',
|
|
scope: 'unstaged',
|
|
reviewedAt: 11,
|
|
reviewDiffIdentity: 'wrong'
|
|
}
|
|
}
|
|
}
|
|
const queue = buildMobileDiffReviewQueue({
|
|
worktreeId: 'wt-1',
|
|
statusEntries: [
|
|
statusEntry({ path: 'a.ts', area: 'unstaged' }),
|
|
statusEntry({ path: 'b.ts', area: 'unstaged' })
|
|
],
|
|
branchEntries: [],
|
|
comments: [comment({ id: 'a', filePath: 'b.ts' })],
|
|
reviewState
|
|
})
|
|
|
|
expect(filterMobileDiffReviewQueue(queue, 'unreviewed').map((item) => item.filePath)).toEqual([
|
|
'a.ts',
|
|
'b.ts'
|
|
])
|
|
expect(filterMobileDiffReviewQueue(queue, 'notes').map((item) => item.filePath)).toEqual([
|
|
'b.ts'
|
|
])
|
|
})
|
|
|
|
it('summarizes review counts in one pass without rereading item fields', () => {
|
|
const entryCount = 1_000
|
|
const reads = { isReviewed: 0, scope: 0, canStage: 0 }
|
|
let expectedReviewedCount = 0
|
|
let expectedReviewedUnstagedItems = 0
|
|
let expectedReviewedUnstagedCount = 0
|
|
const queue = Array.from({ length: entryCount }, (_, index) => {
|
|
const isReviewed = index % 2 === 0
|
|
const scope = index % 3 === 0 ? 'unstaged' : 'staged'
|
|
const canStage = index % 5 === 0
|
|
if (isReviewed) {
|
|
expectedReviewedCount += 1
|
|
if (scope === 'unstaged') {
|
|
expectedReviewedUnstagedItems += 1
|
|
if (canStage) {
|
|
expectedReviewedUnstagedCount += 1
|
|
}
|
|
}
|
|
}
|
|
const item = {} as MobileDiffReviewQueueItem
|
|
Object.defineProperties(item, {
|
|
isReviewed: {
|
|
enumerable: true,
|
|
get: () => {
|
|
reads.isReviewed += 1
|
|
return isReviewed
|
|
}
|
|
},
|
|
scope: {
|
|
enumerable: true,
|
|
get: () => {
|
|
reads.scope += 1
|
|
return scope
|
|
}
|
|
},
|
|
canStage: {
|
|
enumerable: true,
|
|
get: () => {
|
|
reads.canStage += 1
|
|
return canStage
|
|
}
|
|
}
|
|
})
|
|
return item
|
|
})
|
|
|
|
expect(summarizeMobileDiffReviewQueue(queue)).toEqual({
|
|
reviewedCount: expectedReviewedCount,
|
|
reviewedUnstagedCount: expectedReviewedUnstagedCount
|
|
})
|
|
expect(reads).toEqual({
|
|
isReviewed: entryCount,
|
|
scope: expectedReviewedCount,
|
|
canStage: expectedReviewedUnstagedItems
|
|
})
|
|
})
|
|
})
|