From e551d3ec0d1bb1bf72173204b615eae807ffb849 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Tue, 28 Jul 2026 00:21:13 -0700 Subject: [PATCH] perf(lint): consolidate code-quality gates into Oxlint (#11117) Consolidate standalone code-quality scanners into Oxlint, preserve focused native/type-aware enforcement, add custom plugin coverage, and harden deferred PTY test cleanup. --- .github/workflows/pr.yml | 14 +- .oxlintrc.json | 30 ++ ...> oxlint-code-quality-native-plugins.json} | 14 +- config/oxlint-code-quality-type-aware.json | 6 +- .../quadratic-buffer-concat.mjs | 266 ++++++++++++++ .../renderer-scrollbar-style.mjs | 292 +++++++++++++++ config/oxlint-switch-exhaustiveness.json | 19 - .../app-store-performance-plugin.test.mjs | 41 +-- config/scripts/check-changed-code-quality.mjs | 7 +- .../scripts/check-quadratic-buffer-concat.mjs | 343 ------------------ .../check-quadratic-buffer-concat.test.mjs | 146 -------- config/scripts/check-styled-scrollbars.mjs | 87 ----- .../scripts/check-styled-scrollbars.test.mjs | 199 ---------- config/scripts/oxlint-plugin-test-runner.mjs | 58 +++ .../quadratic-buffer-concat-plugin.test.mjs | 113 ++++++ .../renderer-scrollbar-style-plugin.test.mjs | 116 ++++++ .../styled-scrollbar-jsx-check.mjs | 312 ---------------- package.json | 7 +- .../terminal-pane/pty-connection.test.ts | 18 +- 19 files changed, 915 insertions(+), 1173 deletions(-) rename config/{oxlint-code-quality.json => oxlint-code-quality-native-plugins.json} (71%) create mode 100644 config/oxlint-plugins/quadratic-buffer-concat.mjs create mode 100644 config/oxlint-plugins/renderer-scrollbar-style.mjs delete mode 100644 config/oxlint-switch-exhaustiveness.json delete mode 100644 config/scripts/check-quadratic-buffer-concat.mjs delete mode 100644 config/scripts/check-quadratic-buffer-concat.test.mjs delete mode 100644 config/scripts/check-styled-scrollbars.mjs delete mode 100644 config/scripts/check-styled-scrollbars.test.mjs create mode 100644 config/scripts/oxlint-plugin-test-runner.mjs create mode 100644 config/scripts/quadratic-buffer-concat-plugin.test.mjs create mode 100644 config/scripts/renderer-scrollbar-style-plugin.test.mjs delete mode 100644 config/scripts/styled-scrollbars/styled-scrollbar-jsx-check.mjs diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index f51e54b96..aa65296d3 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -32,11 +32,11 @@ jobs: - name: Lint run: pnpm exec oxlint --format github - - name: Enforce full code-quality baseline - run: pnpm run audit:code-quality:native && pnpm run audit:code-quality:type-aware + - name: Enforce focused code-quality plugins + run: pnpm run audit:code-quality:native - - name: Check switch exhaustiveness - run: pnpm run lint:switch-exhaustiveness + - name: Enforce type-aware code-quality baseline + run: pnpm run audit:code-quality:type-aware - name: Enforce changed-code quality run: pnpm run check:code-quality:changed -- "${{ github.event.pull_request.base.sha }}" @@ -47,12 +47,6 @@ jobs: - name: Check Zustand selector fan-out budget run: pnpm run check:zustand-selector-fanout - - name: Check styled scrollbars - run: pnpm check:styled-scrollbars - - - name: Check quadratic buffer concatenation - run: pnpm run check:quadratic-buffer-concat - - name: Check reliability gate manifest run: pnpm run check:reliability-gates diff --git a/.oxlintrc.json b/.oxlintrc.json index 5eb541a89..9c64c7532 100644 --- a/.oxlintrc.json +++ b/.oxlintrc.json @@ -5,12 +5,29 @@ { "name": "mobile-pairing", "specifier": "./config/oxlint-plugins/mobile-pairing-qrcode-import.mjs" + }, + { + "name": "app-store-performance", + "specifier": "./config/oxlint-plugins/app-store-performance.mjs" + }, + { + "name": "quadratic-buffer-concat", + "specifier": "./config/oxlint-plugins/quadratic-buffer-concat.mjs" + }, + { + "name": "renderer-scrollbar-style", + "specifier": "./config/oxlint-plugins/renderer-scrollbar-style.mjs" } ], "categories": { "correctness": "error" }, "rules": { + "app-store-performance/require-selector": "error", + "app-store-performance/no-identity-selector": "error", + "app-store-performance/no-fresh-selector-result": "error", + "no-fallthrough": "error", + "quadratic-buffer-concat/no-loop-carried-concat": "error", "react/jsx-no-duplicate-props": "error", "react/jsx-no-undef": "error", "react/no-children-prop": "error", @@ -93,6 +110,19 @@ "unicorn/throw-new-error": "error" }, "overrides": [ + { + "files": ["src/renderer/src/**/*.{ts,tsx}"], + "rules": { + "renderer-scrollbar-style/require-styled-vertical-scrollbar": "error" + } + }, + { + "files": ["**/*.test.*", "**/*.spec.*", "**/*-benchmark.*"], + "rules": { + "quadratic-buffer-concat/no-loop-carried-concat": "off", + "renderer-scrollbar-style/require-styled-vertical-scrollbar": "off" + } + }, { "files": ["**/*.ts"], "rules": { diff --git a/config/oxlint-code-quality.json b/config/oxlint-code-quality-native-plugins.json similarity index 71% rename from config/oxlint-code-quality.json rename to config/oxlint-code-quality-native-plugins.json index bfa1c115c..5d9cd118e 100644 --- a/config/oxlint-code-quality.json +++ b/config/oxlint-code-quality-native-plugins.json @@ -1,6 +1,6 @@ { "$schema": "../node_modules/oxlint/configuration_schema.json", - "plugins": ["import", "jsx-a11y", "promise", "react", "react-hooks", "typescript", "vitest"], + "plugins": ["import", "jsx-a11y", "react-hooks", "vitest"], "categories": { "correctness": "off", "suspicious": "off", @@ -10,23 +10,13 @@ "restriction": "off", "nursery": "off" }, - "jsPlugins": [ - { - "name": "app-store-performance", - "specifier": "./oxlint-plugins/app-store-performance.mjs" - } - ], "rules": { - "app-store-performance/require-selector": "warn", - "app-store-performance/no-identity-selector": "warn", - "app-store-performance/no-fresh-selector-result": "warn", "import/export": "warn", "import/named": "warn", "import/namespace": "warn", "import/no-cycle": ["warn", { "maxDepth": 3 }], "import/no-duplicates": "warn", - "import/no-self-import": "warn", - "no-fallthrough": "warn" + "import/no-self-import": "warn" }, "overrides": [ { diff --git a/config/oxlint-code-quality-type-aware.json b/config/oxlint-code-quality-type-aware.json index 9198df657..4c0e01fa7 100644 --- a/config/oxlint-code-quality-type-aware.json +++ b/config/oxlint-code-quality-type-aware.json @@ -13,7 +13,11 @@ "rules": { "typescript/await-thenable": "warn", "typescript/restrict-plus-operands": "warn", - "typescript/restrict-template-expressions": "warn" + "typescript/restrict-template-expressions": "warn", + "typescript/switch-exhaustiveness-check": [ + "error", + { "allowDefaultCaseForExhaustiveSwitch": false } + ] }, "overrides": [ { diff --git a/config/oxlint-plugins/quadratic-buffer-concat.mjs b/config/oxlint-plugins/quadratic-buffer-concat.mjs new file mode 100644 index 000000000..b2399345f --- /dev/null +++ b/config/oxlint-plugins/quadratic-buffer-concat.mjs @@ -0,0 +1,266 @@ +const LOOP_TYPES = new Set([ + 'ForStatement', + 'ForInStatement', + 'ForOfStatement', + 'WhileStatement', + 'DoWhileStatement' +]) +const ASSIGNMENT_OPERATORS = new Set(['=', '+=', '??=', '||=', '&&=']) +const EXPRESSION_WRAPPERS = new Set([ + 'ChainExpression', + 'TSAsExpression', + 'TSNonNullExpression', + 'TSSatisfiesExpression', + 'TypeCastExpression' +]) + +function normalizeReferenceText(text) { + return text.replaceAll(/\s+/g, '') +} + +function sourceText(context, node) { + return context.sourceCode.getText(node) +} + +function memberPropertyName(node) { + if (node?.type !== 'MemberExpression') { + return null + } + if (!node.computed && node.property.type === 'Identifier') { + return node.property.name + } + return node.property.type === 'Literal' && typeof node.property.value === 'string' + ? node.property.value + : null +} + +function rootReferenceText(context, node) { + if (node?.type === 'Identifier') { + return node.name + } + if (node?.type === 'MemberExpression') { + return node.object.type === 'ThisExpression' + ? normalizeReferenceText(sourceText(context, node)) + : rootReferenceText(context, node.object) + } + if (node?.type === 'CallExpression') { + return rootReferenceText(context, node.callee) + } + if (EXPRESSION_WRAPPERS.has(node?.type)) { + return rootReferenceText(context, node.expression) + } + return null +} + +function isBufferConcatCall(node) { + return ( + node.type === 'CallExpression' && + node.callee.type === 'MemberExpression' && + node.callee.object.type === 'Identifier' && + node.callee.object.name === 'Buffer' && + memberPropertyName(node.callee) === 'concat' && + node.arguments[0]?.type === 'ArrayExpression' + ) +} + +function enclosingLoop(node) { + for (let current = node.parent; current; current = current.parent) { + if (LOOP_TYPES.has(current.type)) { + return current + } + } + return null +} + +function nodeStart(node) { + return node.start ?? node.range?.[0] ?? 0 +} + +function nodeEnd(node) { + return node.end ?? node.range?.[1] ?? 0 +} + +function isDeclaredInsideLoop(declarationStart, loop) { + if (declarationStart < nodeStart(loop) || declarationStart >= nodeEnd(loop)) { + return false + } + if (loop.type !== 'ForStatement' || !loop.init) { + return true + } + return declarationStart < nodeStart(loop.init) || declarationStart >= nodeEnd(loop.init) +} + +function collectBindingNames(pattern, names) { + if (!pattern) { + return + } + if (pattern.type === 'Identifier') { + names.push(pattern.name) + } else if (pattern.type === 'RestElement') { + collectBindingNames(pattern.argument, names) + } else if (pattern.type === 'AssignmentPattern') { + collectBindingNames(pattern.left, names) + } else if (pattern.type === 'ObjectPattern') { + for (const property of pattern.properties) { + collectBindingNames( + property.type === 'RestElement' ? property.argument : property.value, + names + ) + } + } else if (pattern.type === 'ArrayPattern') { + for (const element of pattern.elements) { + collectBindingNames(element, names) + } + } +} + +function visitChildren(node, visit) { + for (const [key, child] of Object.entries(node)) { + if (['parent', 'loc', 'range'].includes(key)) { + continue + } + if (Array.isArray(child)) { + for (const item of child) { + if (item?.type) { + visit(item) + } + } + } else if (child?.type) { + visit(child) + } + } +} + +function collectAssignedRoots(context, loop) { + const assigned = new Set() + const visit = (node) => { + if (node.type === 'AssignmentExpression' && ASSIGNMENT_OPERATORS.has(node.operator)) { + const root = rootReferenceText(context, node.left) + if (root) { + assigned.add(root) + } + } + visitChildren(node, visit) + } + visit(loop.body) + return assigned +} + +function assignmentTargetOf(context, call) { + let node = call + let parent = node.parent + while ( + parent && + (EXPRESSION_WRAPPERS.has(parent.type) || + (parent.type === 'ConditionalExpression' && parent.test !== node)) + ) { + node = parent + parent = parent.parent + } + if (parent?.type !== 'AssignmentExpression' || parent.operator !== '=' || parent.right !== node) { + return null + } + return { + text: normalizeReferenceText(sourceText(context, parent.left)), + root: rootReferenceText(context, parent.left) + } +} + +function concatOperands(context, call) { + return call.arguments[0].elements.filter(Boolean).map((element) => { + const spread = element.type === 'SpreadElement' + const expression = spread ? element.argument : element + return { + spread, + text: normalizeReferenceText(sourceText(context, expression)), + root: rootReferenceText(context, expression) + } + }) +} + +function isLoopCarried(root, loop, declarations) { + const starts = declarations.get(root) + return !starts || !starts.some((start) => isDeclaredInsideLoop(start, loop)) +} + +function quadraticAccumulator(context, call, loop, declarations, assignedRoots) { + const operands = concatOperands(context, call) + const target = assignmentTargetOf(context, call) + const selfOperand = target + ? operands.find((operand) => operand.text === target.text || operand.root === target.root) + : null + if (selfOperand && target.root && isLoopCarried(target.root, loop, declarations)) { + return target.text + } + + for (const operand of operands) { + if ( + !operand.spread && + operand.root && + assignedRoots.has(operand.root) && + isLoopCarried(operand.root, loop, declarations) + ) { + return operand.root + } + } + return null +} + +function createRule(context) { + const declarations = new Map() + const assignedRootsByLoop = new WeakMap() + const recordBindings = (pattern, owner) => { + const names = [] + collectBindingNames(pattern, names) + for (const name of names) { + const starts = declarations.get(name) ?? [] + starts.push(nodeStart(owner)) + declarations.set(name, starts) + } + } + const recordParameters = (node) => { + for (const parameter of node.params) { + recordBindings(parameter, parameter) + } + } + + return { + VariableDeclarator(node) { + recordBindings(node.id, node) + }, + FunctionDeclaration: recordParameters, + FunctionExpression: recordParameters, + ArrowFunctionExpression: recordParameters, + CatchClause(node) { + recordBindings(node.param, node.param) + }, + CallExpression(node) { + if (!isBufferConcatCall(node)) { + return + } + const loop = enclosingLoop(node) + if (!loop) { + return + } + let assignedRoots = assignedRootsByLoop.get(loop) + if (!assignedRoots) { + assignedRoots = collectAssignedRoots(context, loop) + assignedRootsByLoop.set(loop, assignedRoots) + } + const accumulator = quadraticAccumulator(context, node, loop, declarations, assignedRoots) + if (accumulator) { + context.report({ + node, + message: `Buffer.concat rebuilds loop-carried ${accumulator}; collect chunks and concatenate once after the loop.` + }) + } + } + } +} + +export default { + meta: { name: 'quadratic-buffer-concat' }, + rules: { + 'no-loop-carried-concat': { create: createRule } + } +} diff --git a/config/oxlint-plugins/renderer-scrollbar-style.mjs b/config/oxlint-plugins/renderer-scrollbar-style.mjs new file mode 100644 index 000000000..4938d66ef --- /dev/null +++ b/config/oxlint-plugins/renderer-scrollbar-style.mjs @@ -0,0 +1,292 @@ +const STYLED_SCROLLBAR_CLASSES = new Set([ + 'scrollbar-sleek', + 'scrollbar-editor', + 'worktree-sidebar-scrollbar' +]) +const VERTICAL_SCROLL_CLASSES = new Set([ + 'overflow-auto', + 'overflow-scroll', + 'overflow-y-auto', + 'overflow-y-scroll' +]) +const VERTICAL_SCROLL_STYLE_VALUES = new Set(['auto', 'scroll']) + +function withoutImportantModifier(className) { + const withoutPrefix = className.startsWith('!') ? className.slice(1) : className + return withoutPrefix.endsWith('!') ? withoutPrefix.slice(0, -1) : withoutPrefix +} + +export function plainClassName(token) { + const normalizedToken = token.startsWith('!') ? token.slice(1) : token + const parts = [] + let bracketDepth = 0 + let currentPart = '' + + for (const char of normalizedToken) { + if (char === '[') { + bracketDepth += 1 + } else if (char === ']') { + bracketDepth = Math.max(0, bracketDepth - 1) + } + if (char === ':' && bracketDepth === 0) { + parts.push(currentPart) + currentPart = '' + } else { + currentPart += char + } + } + + parts.push(currentPart) + return withoutImportantModifier(parts.at(-1) ?? '') +} + +function classTokenParts(token) { + const variants = [] + let bracketDepth = 0 + let currentPart = '' + + for (const char of token.startsWith('!') ? token.slice(1) : token) { + if (char === '[') { + bracketDepth += 1 + } else if (char === ']') { + bracketDepth = Math.max(0, bracketDepth - 1) + } + if (char === ':' && bracketDepth === 0) { + variants.push(currentPart) + currentPart = '' + } else { + currentPart += char + } + } + + return { className: withoutImportantModifier(currentPart), variants: variants.filter(Boolean) } +} + +function classTokens(text) { + return text.split(/\s+/).filter(Boolean).map(classTokenParts) +} + +function sameVariants(left, right) { + return left.length === right.length && left.every((variant, index) => variant === right[index]) +} + +function literalHasScrollbarForVertical(text, verticalToken) { + return classTokens(text).some( + (candidate) => + STYLED_SCROLLBAR_CLASSES.has(candidate.className) && + (candidate.variants.length === 0 || sameVariants(candidate.variants, verticalToken.variants)) + ) +} + +function uncoveredVerticalClass(text) { + return classTokens(text).find( + (token) => + VERTICAL_SCROLL_CLASSES.has(token.className) && !literalHasScrollbarForVertical(text, token) + ) +} + +function stringLiteralTexts(node) { + if (node?.type === 'Literal' && typeof node.value === 'string') { + return [node.value] + } + if (node?.type !== 'TemplateLiteral') { + return [] + } + return node.quasis.map((quasi) => quasi.value.cooked ?? quasi.value.raw) +} + +function visitChildren(node, visit) { + for (const [key, child] of Object.entries(node)) { + if (['parent', 'loc', 'range'].includes(key)) { + continue + } + if (Array.isArray(child)) { + for (const item of child) { + if (item?.type) { + visit(item) + } + } + } else if (child?.type) { + visit(child) + } + } +} + +function collectClassLiteralReports(node) { + const reports = [] + const visit = (current) => { + for (const text of stringLiteralTexts(current)) { + const uncovered = uncoveredVerticalClass(text) + if (uncovered) { + reports.push({ node: current, detail: uncovered.className }) + } + } + visitChildren(current, visit) + } + visit(node) + return reports +} + +function expressionHasStyledScrollbarLiteral(node) { + let found = false + const visit = (current) => { + if (found || current.type === 'ConditionalExpression' || current.type === 'LogicalExpression') { + return + } + found = stringLiteralTexts(current).some((text) => + classTokens(text).some((token) => STYLED_SCROLLBAR_CLASSES.has(token.className)) + ) + if (!found) { + visitChildren(current, visit) + } + } + visit(node) + return found +} + +function propertyName(node) { + if (node?.type !== 'Property') { + return null + } + if (!node.computed && node.key.type === 'Identifier') { + return node.key.name + } + return node.key.type === 'Literal' && typeof node.key.value === 'string' ? node.key.value : null +} + +function styleValueIsVerticalScroll(name, value) { + const parts = value.trim().toLowerCase().split(/\s+/).filter(Boolean) + if (parts.length === 0) { + return false + } + if (name === 'overflowY' || name === 'overflow-y') { + return VERTICAL_SCROLL_STYLE_VALUES.has(parts[0]) + } + if (name !== 'overflow') { + return false + } + return VERTICAL_SCROLL_STYLE_VALUES.has(parts.length > 1 ? parts[1] : parts[0]) +} + +function collectStyleReports(node) { + const reports = [] + const visit = (current) => { + if (current.type === 'Property') { + const name = propertyName(current) + for (const value of name ? stringLiteralTexts(current.value) : []) { + if (styleValueIsVerticalScroll(name, value)) { + reports.push({ node: current, detail: 'inline vertical scroll' }) + } + } + visit(current.value) + return + } + visitChildren(current, visit) + } + visit(node) + return reports +} + +function unwrapExpression(node) { + if ( + ['TSAsExpression', 'TSSatisfiesExpression', 'TSNonNullExpression', 'ChainExpression'].includes( + node?.type + ) + ) { + return unwrapExpression(node.expression) + } + return node +} + +function spreadPropExpressions(expression, propName) { + const node = unwrapExpression(expression) + if (!node) { + return [] + } + if (node.type === 'ConditionalExpression') { + return [ + ...spreadPropExpressions(node.consequent, propName), + ...spreadPropExpressions(node.alternate, propName) + ] + } + if (node.type === 'LogicalExpression' || node.type === 'BinaryExpression') { + return [ + ...spreadPropExpressions(node.left, propName), + ...spreadPropExpressions(node.right, propName) + ] + } + if (node.type !== 'ObjectExpression') { + return [] + } + return node.properties.flatMap((property) => { + if (property.type === 'SpreadElement') { + return spreadPropExpressions(property.argument, propName) + } + return propertyName(property) === propName ? [property.value] : [] + }) +} + +function jsxAttributeExpression(attribute) { + if (attribute.value?.type === 'Literal') { + return attribute.value + } + return attribute.value?.type === 'JSXExpressionContainer' ? attribute.value.expression : null +} + +function jsxElementReports(node) { + let classExpression = null + const styleExpressions = [] + + for (const attribute of node.attributes) { + if (attribute.type === 'JSXSpreadAttribute') { + const spreadClassExpression = spreadPropExpressions(attribute.argument, 'className').at(-1) + if (spreadClassExpression) { + classExpression = spreadClassExpression + } + styleExpressions.push(...spreadPropExpressions(attribute.argument, 'style')) + } else if (attribute.name?.name === 'className') { + classExpression = jsxAttributeExpression(attribute) + } else if (attribute.name?.name === 'style') { + const expression = jsxAttributeExpression(attribute) + if (expression) { + styleExpressions.push(expression) + } + } + } + + const reports = classExpression ? collectClassLiteralReports(classExpression) : [] + if (!classExpression || !expressionHasStyledScrollbarLiteral(classExpression)) { + for (const expression of styleExpressions) { + reports.push(...collectStyleReports(expression)) + } + } + return reports +} + +function bindContext(createVisitors) { + return (context) => { + const visitors = createVisitors() + for (const [nodeType, visit] of Object.entries(visitors)) { + visitors[nodeType] = visit.bind(context) + } + return visitors + } +} + +export default { + meta: { name: 'renderer-scrollbar-style' }, + rules: { + 'require-styled-vertical-scrollbar': { + create: bindContext(() => ({ + JSXOpeningElement(node) { + for (const report of jsxElementReports(node)) { + this.report({ + node: report.node, + message: `Vertical scroll container (${report.detail}) must use scrollbar-sleek, scrollbar-editor, or worktree-sidebar-scrollbar.` + }) + } + } + })) + } + } +} diff --git a/config/oxlint-switch-exhaustiveness.json b/config/oxlint-switch-exhaustiveness.json deleted file mode 100644 index c967a7b9b..000000000 --- a/config/oxlint-switch-exhaustiveness.json +++ /dev/null @@ -1,19 +0,0 @@ -{ - "$schema": "../node_modules/oxlint/configuration_schema.json", - "plugins": ["typescript"], - "categories": { - "correctness": "off", - "suspicious": "off", - "pedantic": "off", - "perf": "off", - "style": "off", - "restriction": "off", - "nursery": "off" - }, - "rules": { - "typescript/switch-exhaustiveness-check": [ - "error", - { "allowDefaultCaseForExhaustiveSwitch": false } - ] - } -} diff --git a/config/scripts/app-store-performance-plugin.test.mjs b/config/scripts/app-store-performance-plugin.test.mjs index 344918309..bb2f305ba 100644 --- a/config/scripts/app-store-performance-plugin.test.mjs +++ b/config/scripts/app-store-performance-plugin.test.mjs @@ -1,41 +1,20 @@ -import { spawnSync } from 'node:child_process' -import { mkdtempSync, writeFileSync } from 'node:fs' -import { tmpdir } from 'node:os' import path from 'node:path' import { describe, expect, it } from 'vitest' +import { runOxlintPluginOnSource } from './oxlint-plugin-test-runner.mjs' const pluginPath = path.resolve('config/oxlint-plugins/app-store-performance.mjs') -const oxlintPath = path.resolve( - process.platform === 'win32' ? 'node_modules/.bin/oxlint.cmd' : 'node_modules/.bin/oxlint' -) function lintSource(source) { - const directory = mkdtempSync(path.join(tmpdir(), 'orca-app-store-lint-')) - const sourcePath = path.join(directory, 'sample.tsx') - const configPath = path.join(directory, 'oxlint.json') - writeFileSync(sourcePath, source) - writeFileSync( - configPath, - JSON.stringify({ - categories: { - correctness: 'off' - }, - jsPlugins: [{ name: 'app-store-performance', specifier: pluginPath }], - rules: { - 'app-store-performance/require-selector': 'warn', - 'app-store-performance/no-identity-selector': 'warn', - 'app-store-performance/no-fresh-selector-result': 'warn' - } - }) - ) - const result = spawnSync(oxlintPath, ['--config', configPath, '--format', 'json', sourcePath], { - encoding: 'utf8' + return runOxlintPluginOnSource({ + pluginName: 'app-store-performance', + pluginPath, + source, + rules: { + 'app-store-performance/require-selector': 'warn', + 'app-store-performance/no-identity-selector': 'warn', + 'app-store-performance/no-fresh-selector-result': 'warn' + } }) - if (result.error) { - throw result.error - } - expect(result.status).toBe(0) - return JSON.parse(result.stdout).diagnostics } describe('app store performance Oxlint plugin', () => { diff --git a/config/scripts/check-changed-code-quality.mjs b/config/scripts/check-changed-code-quality.mjs index 7efcbe480..a8fd5b739 100644 --- a/config/scripts/check-changed-code-quality.mjs +++ b/config/scripts/check-changed-code-quality.mjs @@ -9,12 +9,7 @@ const SOURCE_FILE_PATTERN = /\.(?:[cm]?[jt]sx?)$/ const OXLINT_SCANS = [ { label: 'code quality', - args: [ - '--config', - 'config/oxlint-code-quality.json', - '--report-unused-disable-directives-severity', - 'warn' - ] + args: ['--config', '.oxlintrc.json', '--report-unused-disable-directives-severity', 'warn'] }, { label: 'type-aware code quality', diff --git a/config/scripts/check-quadratic-buffer-concat.mjs b/config/scripts/check-quadratic-buffer-concat.mjs deleted file mode 100644 index 611157c3e..000000000 --- a/config/scripts/check-quadratic-buffer-concat.mjs +++ /dev/null @@ -1,343 +0,0 @@ -import fs from 'node:fs/promises' -import path from 'node:path' -import { pathToFileURL } from 'node:url' -import process from 'node:process' - -// TypeScript 7 is a native CLI; AST consumers still need the legacy JavaScript API. -import ts from 'typescript-api' - -const SOURCE_EXTENSIONS = new Set(['.ts', '.tsx', '.js', '.jsx', '.mjs', '.mts', '.cts']) -const SKIP_PATH_PARTS = new Set(['node_modules', 'dist', 'out', '.git', '__snapshots__']) -const SCAN_ROOTS = ['src', 'config/scripts', 'tools', 'tests', 'mobile'] - -const LOOP_KINDS = new Set([ - ts.SyntaxKind.ForStatement, - ts.SyntaxKind.ForInStatement, - ts.SyntaxKind.ForOfStatement, - ts.SyntaxKind.WhileStatement, - ts.SyntaxKind.DoStatement -]) - -const ASSIGNMENT_OPERATORS = new Set([ - ts.SyntaxKind.EqualsToken, - ts.SyntaxKind.PlusEqualsToken, - ts.SyntaxKind.QuestionQuestionEqualsToken, - ts.SyntaxKind.BarBarEqualsToken, - ts.SyntaxKind.AmpersandAmpersandEqualsToken -]) - -function normalizeReferenceText(text) { - return text.replaceAll(/\s+/g, '') -} - -// The variable a member/call chain ultimately reads, so `carry.subarray(0, n)` -// still resolves to `carry`. `this.x` stops at the property: the class field is -// the accumulator. -function rootReferenceText(node) { - if (ts.isIdentifier(node)) { - return node.text - } - if (ts.isPropertyAccessExpression(node)) { - return node.expression.kind === ts.SyntaxKind.ThisKeyword - ? normalizeReferenceText(node.getText()) - : rootReferenceText(node.expression) - } - if ( - ts.isElementAccessExpression(node) || - ts.isCallExpression(node) || - ts.isNonNullExpression(node) || - ts.isParenthesizedExpression(node) || - ts.isAsExpression(node) - ) { - return rootReferenceText(node.expression) - } - return undefined -} - -function isBufferConcatCall(node) { - return ( - ts.isCallExpression(node) && - ts.isPropertyAccessExpression(node.expression) && - node.expression.name.text === 'concat' && - ts.isIdentifier(node.expression.expression) && - node.expression.expression.text === 'Buffer' && - node.arguments.length > 0 && - ts.isArrayLiteralExpression(node.arguments[0]) - ) -} - -function enclosingLoop(node) { - for (let current = node.parent; current; current = current.parent) { - if (LOOP_KINDS.has(current.kind)) { - return current - } - } - return undefined -} - -// A `for (let acc = ...; ;)` binding is loop-carried even though it sits inside -// the loop node, so the initializer does not count as loop-local. -function isDeclaredInsideLoop(declarationStart, loop) { - if (declarationStart < loop.getStart() || declarationStart >= loop.end) { - return false - } - const initializer = ts.isForStatement(loop) ? loop.initializer : undefined - if (!initializer) { - return true - } - return declarationStart < initializer.getStart() || declarationStart >= initializer.end -} - -function collectBindingNames(name, into) { - if (ts.isIdentifier(name)) { - into.push(name) - return - } - if (ts.isObjectBindingPattern(name) || ts.isArrayBindingPattern(name)) { - for (const element of name.elements) { - if (ts.isBindingElement(element)) { - collectBindingNames(element.name, into) - } - } - } -} - -function collectDeclarations(sourceFile) { - const declarations = new Map() - const record = (identifier, node) => { - const existing = declarations.get(identifier.text) - const start = node.getStart(sourceFile) - if (existing) { - existing.push(start) - } else { - declarations.set(identifier.text, [start]) - } - } - - const visit = (node) => { - if (ts.isVariableDeclaration(node) || ts.isParameter(node)) { - const names = [] - collectBindingNames(node.name, names) - for (const identifier of names) { - record(identifier, node) - } - } - ts.forEachChild(node, visit) - } - - visit(sourceFile) - return declarations -} - -function collectAssignedRoots(loop) { - const assigned = new Set() - const visit = (node) => { - if (ts.isBinaryExpression(node) && ASSIGNMENT_OPERATORS.has(node.operatorToken.kind)) { - const root = rootReferenceText(node.left) - if (root) { - assigned.add(root) - } - } - ts.forEachChild(node, visit) - } - visit(loop.statement) - return assigned -} - -// Unwrap the wrappers a concat result passes through before it lands on the -// left-hand side, so `acc = cond ? Buffer.concat([acc, c]) : c` still counts. -function assignmentTargetOf(call) { - let node = call - let parent = node.parent - while ( - parent && - (ts.isParenthesizedExpression(parent) || - ts.isAsExpression(parent) || - ts.isNonNullExpression(parent) || - (ts.isConditionalExpression(parent) && parent.condition !== node)) - ) { - node = parent - parent = parent.parent - } - if ( - parent && - ts.isBinaryExpression(parent) && - parent.operatorToken.kind === ts.SyntaxKind.EqualsToken && - parent.right === node - ) { - return { - text: normalizeReferenceText(parent.left.getText()), - root: rootReferenceText(parent.left) - } - } - return undefined -} - -function concatOperands(call) { - return call.arguments[0].elements.map((element) => { - const spread = ts.isSpreadElement(element) - const expression = spread ? element.expression : element - return { - spread, - text: normalizeReferenceText(expression.getText()), - root: rootReferenceText(expression) - } - }) -} - -// A binding reset on every iteration cannot accumulate; only one that outlives -// the iteration carries the cost forward. -function isLoopCarried(root, loop, declarations) { - const starts = declarations.get(root) - if (!starts) { - return true - } - return !starts.some((start) => isDeclaredInsideLoop(start, loop)) -} - -function quadraticAccumulator(call, loop, declarations, assignedRoots) { - const operands = concatOperands(call) - const target = assignmentTargetOf(call) - // The result feeds straight back into its own input: every iteration re-copies - // everything accumulated so far. - const selfOperand = target - ? operands.find((operand) => operand.text === target.text || operand.root === target.root) - : undefined - if (selfOperand && target.root && isLoopCarried(target.root, loop, declarations)) { - return target.text - } - - // No direct self-assignment, so look for a loop-carried operand reassigned - // inside the loop: the concat result reaches it by some other path. Spread - // operands are the sanctioned chunk-list fix, not this bug, so skip them. - for (const operand of operands) { - if (operand.spread || !operand.root || !assignedRoots.has(operand.root)) { - continue - } - if (isLoopCarried(operand.root, loop, declarations)) { - return operand.root - } - } - return undefined -} - -export function reportQuadraticBufferConcat(filePath, sourceText) { - if (!sourceText.includes('Buffer.concat')) { - return [] - } - const sourceFile = ts.createSourceFile(filePath, sourceText, ts.ScriptTarget.Latest, true) - const declarations = collectDeclarations(sourceFile) - const assignedRootsByLoop = new Map() - const reports = [] - const seen = new Set() - - const visit = (node) => { - if (isBufferConcatCall(node)) { - const loop = enclosingLoop(node) - if (loop) { - let assignedRoots = assignedRootsByLoop.get(loop) - if (!assignedRoots) { - assignedRoots = collectAssignedRoots(loop) - assignedRootsByLoop.set(loop, assignedRoots) - } - const accumulator = quadraticAccumulator(node, loop, declarations, assignedRoots) - if (accumulator) { - const start = node.getStart(sourceFile) - if (!seen.has(start)) { - seen.add(start) - const { line, character } = sourceFile.getLineAndCharacterOfPosition(start) - reports.push({ - filePath, - line: line + 1, - column: character + 1, - accumulator, - text: node.getText() - }) - } - } - } - } - ts.forEachChild(node, visit) - } - - visit(sourceFile) - return reports -} - -export function normalizePath(root, filePath) { - return path.relative(root, filePath).split(path.sep).join('/') -} - -function isSkippedFile(root, filePath) { - const relative = normalizePath(root, filePath) - // Benchmarks keep the pre-fix shape on purpose so they can measure against it. - if ( - relative.includes('.test.') || - relative.includes('.spec.') || - relative.includes('-benchmark.') - ) { - return true - } - return relative.split('/').some((part) => SKIP_PATH_PARTS.has(part)) -} - -async function collectSourceFiles(root, dir) { - let entries - try { - entries = await fs.readdir(dir, { withFileTypes: true }) - } catch { - return [] - } - const files = [] - - for (const entry of entries) { - const fullPath = path.join(dir, entry.name) - if (entry.isDirectory()) { - if (!SKIP_PATH_PARTS.has(entry.name)) { - files.push(...(await collectSourceFiles(root, fullPath))) - } - } else if ( - entry.isFile() && - SOURCE_EXTENSIONS.has(path.extname(entry.name)) && - !isSkippedFile(root, fullPath) - ) { - files.push(fullPath) - } - } - - return files -} - -function formatReports(root, reports) { - return reports - .map( - (report) => - `${normalizePath(root, report.filePath)}:${report.line}:${report.column} ${report.accumulator} — ${report.text.replaceAll(/\s+/g, ' ')}` - ) - .join('\n') -} - -export async function main(root = process.cwd()) { - const reports = [] - for (const scanRoot of SCAN_ROOTS) { - const files = await collectSourceFiles(root, path.join(root, scanRoot)) - for (const filePath of files) { - const sourceText = await fs.readFile(filePath, 'utf8') - reports.push(...reportQuadraticBufferConcat(filePath, sourceText)) - } - } - if (reports.length === 0) { - return 0 - } - - console.error('Buffer.concat must not rebuild a loop-carried accumulator.') - console.error('Each iteration re-copies everything accumulated so far, so the loop is O(n^2).') - console.error('Collect the pieces in a Buffer[] and Buffer.concat(chunks) once, after the loop.') - console.error('') - console.error(formatReports(root, reports)) - return 1 -} - -if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { - process.exit(await main()) -} diff --git a/config/scripts/check-quadratic-buffer-concat.test.mjs b/config/scripts/check-quadratic-buffer-concat.test.mjs deleted file mode 100644 index bc17ce8c7..000000000 --- a/config/scripts/check-quadratic-buffer-concat.test.mjs +++ /dev/null @@ -1,146 +0,0 @@ -import { describe, expect, it } from 'vitest' - -import { reportQuadraticBufferConcat } from './check-quadratic-buffer-concat.mjs' - -describe('check-quadratic-buffer-concat', () => { - it('reports an accumulator rebuilt from itself in a for-of loop', () => { - const reports = reportQuadraticBufferConcat( - 'Example.ts', - 'let acc = Buffer.alloc(0); for (const chunk of chunks) { acc = Buffer.concat([acc, chunk]) }' - ) - - expect(reports).toHaveLength(1) - expect(reports[0].accumulator).toBe('acc') - }) - - it('reports the accumulator when it trails the new chunk', () => { - const reports = reportQuadraticBufferConcat( - 'Example.ts', - 'let acc = Buffer.alloc(0); for (const chunk of chunks) { acc = Buffer.concat([chunk, acc]) }' - ) - - expect(reports).toHaveLength(1) - }) - - it('reports a spread of the accumulator into its own concat', () => { - const reports = reportQuadraticBufferConcat( - 'Example.ts', - 'let acc = Buffer.alloc(0); for (const chunk of chunks) { acc = Buffer.concat([...acc, chunk]) }' - ) - - expect(reports).toHaveLength(1) - }) - - it('reports the real pre-fix ai-vault stream carry', () => { - const reports = reportQuadraticBufferConcat( - 'session-scanner-parse-cache.ts', - `async function read(stream) { - let remainder = null - for await (const chunk of stream) { - const data = remainder ? Buffer.concat([remainder, chunk]) : chunk - remainder = Buffer.from(data.subarray(lineStart)) - } - }` - ) - - expect(reports).toHaveLength(1) - expect(reports[0].accumulator).toBe('remainder') - }) - - it('reports the real pre-fix transcript reader carry, which never names itself on the left', () => { - const reports = reportQuadraticBufferConcat( - 'agent-hook-listener.ts', - `function read(fd, size) { - let carryBytes = Buffer.alloc(0) - while (bytesRead < size) { - const combined = Buffer.concat([buffer.subarray(0, n), carryBytes]) - carryBytes = combined.subarray(0, firstNewline) - } - }` - ) - - expect(reports).toHaveLength(1) - expect(reports[0].accumulator).toBe('carryBytes') - }) - - it('accepts the chunk-list fix that joins once after the loop', () => { - const reports = reportQuadraticBufferConcat( - 'Example.ts', - 'const parts = []; for (const chunk of chunks) { parts.push(chunk) } const out = Buffer.concat(parts)' - ) - - expect(reports).toHaveLength(0) - }) - - it('accepts spreading a carry chunk list, which is the sanctioned fix', () => { - const reports = reportQuadraticBufferConcat( - 'Example.ts', - `let carryChunks = [] - while (scanEnd > 0) { - const region = carryChunks.length === 0 ? buffer : Buffer.concat([buffer, ...carryChunks]) - carryChunks = [buffer.subarray(0, firstNewline)] - }` - ) - - expect(reports).toHaveLength(0) - }) - - it('accepts a concat whose result never outlives the iteration', () => { - const reports = reportQuadraticBufferConcat( - 'Example.ts', - 'for (const group of groups) { const frame = Buffer.concat([group.header, group.body]); send(frame) }' - ) - - expect(reports).toHaveLength(0) - }) - - it('accepts a loop-local buffer declared and rebuilt inside the same iteration', () => { - const reports = reportQuadraticBufferConcat( - 'Example.ts', - 'for (const chunk of chunks) { let framed = HEADER; framed = Buffer.concat([framed, chunk]); send(framed) }' - ) - - expect(reports).toHaveLength(0) - }) - - it('reports an accumulator declared in a classic for initializer', () => { - const reports = reportQuadraticBufferConcat( - 'Example.ts', - 'for (let acc = Buffer.alloc(0), i = 0; i < n; i++) { acc = Buffer.concat([acc, chunks[i]]) }' - ) - - expect(reports).toHaveLength(1) - }) - - it('reports a class field accumulator rebuilt inside a loop', () => { - const reports = reportQuadraticBufferConcat( - 'Example.ts', - 'class Reader { read() { while (this.open) { this.pending = Buffer.concat([this.pending, chunk]) } } }' - ) - - expect(reports).toHaveLength(1) - expect(reports[0].accumulator).toBe('this.pending') - }) - - it('reports an accumulator guarded by an emptiness ternary', () => { - const reports = reportQuadraticBufferConcat( - 'Example.ts', - 'let acc = Buffer.alloc(0); while (open) { acc = acc.length === 0 ? chunk : Buffer.concat([acc, chunk]) }' - ) - - expect(reports).toHaveLength(1) - }) - - it('ignores a per-event accumulator with no enclosing loop', () => { - const reports = reportQuadraticBufferConcat( - 'scrcpy-stream-session.ts', - 'class S { handleVideoChunk(chunk) { let buffer = Buffer.concat([this.pendingVideo, chunk]); this.pendingVideo = parse(buffer).pending } }' - ) - - expect(reports).toHaveLength(0) - }) - - it('ignores files with no Buffer.concat at all', () => { - expect(reportQuadraticBufferConcat('Example.ts', 'export const x = 1')).toHaveLength(0) - }) -}) diff --git a/config/scripts/check-styled-scrollbars.mjs b/config/scripts/check-styled-scrollbars.mjs deleted file mode 100644 index 12317998e..000000000 --- a/config/scripts/check-styled-scrollbars.mjs +++ /dev/null @@ -1,87 +0,0 @@ -import fs from 'node:fs/promises' -import path from 'node:path' -import { pathToFileURL } from 'node:url' -import process from 'node:process' - -import { reportUnstyledScrollbars } from './styled-scrollbars/styled-scrollbar-jsx-check.mjs' -export { - plainClassName, - reportUnstyledScrollbars -} from './styled-scrollbars/styled-scrollbar-jsx-check.mjs' - -const SOURCE_EXTENSIONS = new Set(['.ts', '.tsx', '.js', '.jsx', '.mts', '.cts']) -const SKIP_PATH_PARTS = new Set(['node_modules', 'dist', 'out', '.git', '__snapshots__']) - -export function normalizePath(root, filePath) { - return path.relative(root, filePath).split(path.sep).join('/') -} - -function isSkippedFile(root, filePath) { - const relative = normalizePath(root, filePath) - if (relative.includes('.test.') || relative.includes('.spec.')) { - return true - } - return relative.split('/').some((part) => SKIP_PATH_PARTS.has(part)) -} - -async function collectSourceFiles(root, dir) { - const entries = await fs.readdir(dir, { withFileTypes: true }) - const files = [] - - for (const entry of entries) { - const fullPath = path.join(dir, entry.name) - if (entry.isDirectory()) { - if (!SKIP_PATH_PARTS.has(entry.name)) { - files.push(...(await collectSourceFiles(root, fullPath))) - } - } else if ( - entry.isFile() && - SOURCE_EXTENSIONS.has(path.extname(entry.name)) && - !isSkippedFile(root, fullPath) - ) { - files.push(fullPath) - } - } - - return files -} - -async function collectUnstyledScrollbarReports(root) { - const sourceRoot = path.join(root, 'src', 'renderer', 'src') - const files = await collectSourceFiles(root, sourceRoot) - const reports = [] - - for (const filePath of files) { - const sourceText = await fs.readFile(filePath, 'utf8') - reports.push(...reportUnstyledScrollbars(filePath, sourceText)) - } - - return reports -} - -function formatReports(root, reports) { - return reports - .map( - (report) => - `${normalizePath(root, report.filePath)}:${report.line}:${report.column} ${report.text.replace(/\s+/g, ' ')}` - ) - .join('\n') -} - -export async function main(root = process.cwd()) { - const reports = await collectUnstyledScrollbarReports(root) - if (reports.length === 0) { - return 0 - } - - console.error('Renderer vertical scroll containers must use an Orca scrollbar style.') - console.error('Put the scrollbar class in the same class literal as the vertical overflow class.') - console.error('Use scrollbar-sleek, scrollbar-editor, or worktree-sidebar-scrollbar.') - console.error('') - console.error(formatReports(root, reports)) - return 1 -} - -if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { - process.exit(await main()) -} diff --git a/config/scripts/check-styled-scrollbars.test.mjs b/config/scripts/check-styled-scrollbars.test.mjs deleted file mode 100644 index c6ff4a72a..000000000 --- a/config/scripts/check-styled-scrollbars.test.mjs +++ /dev/null @@ -1,199 +0,0 @@ -import { describe, expect, it } from 'vitest' - -import { plainClassName, reportUnstyledScrollbars } from './check-styled-scrollbars.mjs' - -describe('check-styled-scrollbars', () => { - it('reports renderer vertical scroll containers without an Orca scrollbar style', () => { - const reports = reportUnstyledScrollbars( - 'Example.tsx', - 'export function Example() { return
}' - ) - - expect(reports).toHaveLength(1) - }) - - it('accepts obvious styled vertical scroll containers', () => { - const reports = reportUnstyledScrollbars( - 'Example.tsx', - 'export function Example() { return
}' - ) - - expect(reports).toHaveLength(0) - }) - - it('does not accept nonexistent scrollbar classes as Orca scrollbar styles', () => { - const reports = reportUnstyledScrollbars( - 'Example.tsx', - 'export function Example() { return
}' - ) - - expect(reports).toHaveLength(1) - }) - - it('fails closed when a separate class composer argument supplies the scrollbar style', () => { - const reports = reportUnstyledScrollbars( - 'Example.tsx', - "export function Example() { return
}" - ) - - expect(reports).toHaveLength(1) - }) - - it('accepts static class composer arguments when the same literal is styled', () => { - const reports = reportUnstyledScrollbars( - 'Example.tsx', - "export function Example() { return
}" - ) - - expect(reports).toHaveLength(0) - }) - - it('fails closed when a scrollbar class is only conditionally present', () => { - const reports = reportUnstyledScrollbars( - 'Example.tsx', - "export function Example({ enabled }) { return
}" - ) - - expect(reports).toHaveLength(1) - }) - - it('accepts conditional branches when overflow and scrollbar live in the same class literal', () => { - const reports = reportUnstyledScrollbars( - 'Example.tsx', - "export function Example({ enabled }) { return
}" - ) - - expect(reports).toHaveLength(0) - }) - - it('reports vertical scroll inside arbitrary wrappers when the literal is unstyled', () => { - const reports = reportUnstyledScrollbars( - 'Example.tsx', - "export function Example() { return
}" - ) - - expect(reports).toHaveLength(1) - }) - - it('does not require a vertical scrollbar style for horizontal-only overflow', () => { - const reports = reportUnstyledScrollbars( - 'Example.tsx', - 'export function Example() { return
 }'
-    )
-
-    expect(reports).toHaveLength(0)
-  })
-
-  it('does not let responsive scrollbar variants satisfy unconditional overflow', () => {
-    const reports = reportUnstyledScrollbars(
-      'Example.tsx',
-      'export function Example() { return 
}' - ) - - expect(reports).toHaveLength(1) - }) - - it('accepts matching responsive overflow and scrollbar variants', () => { - const reports = reportUnstyledScrollbars( - 'Example.tsx', - 'export function Example() { return
}' - ) - - expect(reports).toHaveLength(0) - }) - - it('accepts unconditional scrollbar styles for responsive overflow', () => { - const reports = reportUnstyledScrollbars( - 'Example.tsx', - 'export function Example() { return
}' - ) - - expect(reports).toHaveLength(0) - }) - - it('reports inline vertical overflow without an Orca scrollbar class', () => { - const reports = reportUnstyledScrollbars( - 'Example.tsx', - "export function Example() { return
}" - ) - - expect(reports).toHaveLength(1) - }) - - it('accepts inline vertical overflow with a stable Orca scrollbar class', () => { - const reports = reportUnstyledScrollbars( - 'Example.tsx', - 'export function Example() { return
}' - ) - - expect(reports).toHaveLength(0) - }) - - it('reports inline vertical overflow when the scrollbar class is conditional or short-circuited', () => { - for (const classNameExpression of [ - "enabled && 'scrollbar-sleek'", - "enabled ? 'scrollbar-sleek' : undefined", - "enabled || 'scrollbar-sleek'", - "enabled ?? 'scrollbar-sleek'" - ]) { - const reports = reportUnstyledScrollbars( - 'Example.tsx', - `export function Example({ enabled }) { return
}` - ) - - expect(reports, classNameExpression).toHaveLength(1) - } - }) - - it('reports logical inline style spreads without an Orca scrollbar class', () => { - const reports = reportUnstyledScrollbars( - 'Example.tsx', - "export function Example({ open }) { return
}" - ) - - expect(reports).toHaveLength(1) - }) - - it('reports JSX spread className props with unstyled vertical overflow', () => { - const reports = reportUnstyledScrollbars( - 'Example.tsx', - "export function Example() { return
}" - ) - - expect(reports).toHaveLength(1) - }) - - it('accepts JSX spread className props when the same literal is styled', () => { - const reports = reportUnstyledScrollbars( - 'Example.tsx', - "export function Example() { return
}" - ) - - expect(reports).toHaveLength(0) - }) - - it('uses later spread className props over earlier explicit className props', () => { - const reports = reportUnstyledScrollbars( - 'Example.tsx', - 'export function Example() { return
}' - ) - - expect(reports).toHaveLength(1) - }) - - it('supports variant helper className config', () => { - const reports = reportUnstyledScrollbars( - 'Example.tsx', - "export function Example() { return
}" - ) - - expect(reports).toHaveLength(0) - }) - - it('normalizes Tailwind variants and important prefixes before matching', () => { - expect(plainClassName('md:overflow-y-auto')).toBe('overflow-y-auto') - expect(plainClassName('[&:hover]:overflow-y-auto')).toBe('overflow-y-auto') - expect(plainClassName('md:!scrollbar-editor')).toBe('scrollbar-editor') - expect(plainClassName('!scrollbar-editor')).toBe('scrollbar-editor') - }) -}) diff --git a/config/scripts/oxlint-plugin-test-runner.mjs b/config/scripts/oxlint-plugin-test-runner.mjs new file mode 100644 index 000000000..c5a80e0c1 --- /dev/null +++ b/config/scripts/oxlint-plugin-test-runner.mjs @@ -0,0 +1,58 @@ +import { spawnSync } from 'node:child_process' +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { createRequire } from 'node:module' +import { tmpdir } from 'node:os' +import path from 'node:path' +import process from 'node:process' + +const oxlintPackageDirectory = path.dirname( + createRequire(import.meta.url).resolve('oxlint/package.json') +) +const oxlintPath = path.join(oxlintPackageDirectory, 'bin', 'oxlint') + +export function runOxlintPluginOnSource({ + pluginName, + pluginPath, + rules, + source, + extension = 'tsx' +}) { + const directory = mkdtempSync(path.join(tmpdir(), `orca-${pluginName}-lint-`)) + const sourcePath = path.join(directory, `sample.${extension}`) + const configPath = path.join(directory, 'oxlint.json') + + try { + writeFileSync(sourcePath, source) + writeFileSync( + configPath, + JSON.stringify({ + plugins: [], + categories: { + correctness: 'off', + suspicious: 'off', + pedantic: 'off', + perf: 'off', + style: 'off', + restriction: 'off', + nursery: 'off' + }, + jsPlugins: [{ name: pluginName, specifier: pluginPath }], + rules + }) + ) + const result = spawnSync( + process.execPath, + [oxlintPath, '--config', configPath, '--format', 'json', sourcePath], + { encoding: 'utf8' } + ) + if (result.error) { + throw result.error + } + if (!result.stdout.trim()) { + throw new Error(result.stderr || `${pluginName} did not produce Oxlint output`) + } + return JSON.parse(result.stdout).diagnostics + } finally { + rmSync(directory, { recursive: true, force: true }) + } +} diff --git a/config/scripts/quadratic-buffer-concat-plugin.test.mjs b/config/scripts/quadratic-buffer-concat-plugin.test.mjs new file mode 100644 index 000000000..68c47a0d6 --- /dev/null +++ b/config/scripts/quadratic-buffer-concat-plugin.test.mjs @@ -0,0 +1,113 @@ +import path from 'node:path' +import { describe, expect, it } from 'vitest' +import { runOxlintPluginOnSource } from './oxlint-plugin-test-runner.mjs' + +const pluginPath = path.resolve('config/oxlint-plugins/quadratic-buffer-concat.mjs') + +function lintSource(source) { + return runOxlintPluginOnSource({ + pluginName: 'quadratic-buffer-concat', + pluginPath, + source, + extension: 'ts', + rules: { + 'quadratic-buffer-concat/no-loop-carried-concat': 'warn' + } + }) +} + +const violations = [ + [ + 'self accumulator', + 'let acc = Buffer.alloc(0); for (const chunk of chunks) { acc = Buffer.concat([acc, chunk]) }', + 'acc' + ], + [ + 'trailing accumulator', + 'let acc = Buffer.alloc(0); for (const chunk of chunks) { acc = Buffer.concat([chunk, acc]) }', + 'acc' + ], + [ + 'spread self accumulator', + 'let acc = Buffer.alloc(0); for (const chunk of chunks) { acc = Buffer.concat([...acc, chunk]) }', + 'acc' + ], + [ + 'indirect stream carry', + `async function read(stream) { + let remainder = null + for await (const chunk of stream) { + const data = remainder ? Buffer.concat([remainder, chunk]) : chunk + remainder = Buffer.from(data.subarray(lineStart)) + } + }`, + 'remainder' + ], + [ + 'indirect transcript carry', + `function read(fd, size) { + let carryBytes = Buffer.alloc(0) + while (bytesRead < size) { + const combined = Buffer.concat([buffer.subarray(0, n), carryBytes]) + carryBytes = combined.subarray(0, firstNewline) + } + }`, + 'carryBytes' + ], + [ + 'classic for initializer', + 'for (let acc = Buffer.alloc(0), i = 0; i < n; i++) { acc = Buffer.concat([acc, chunks[i]]) }', + 'acc' + ], + [ + 'class field accumulator', + 'class Reader { read() { while (this.open) { this.pending = Buffer.concat([this.pending, chunk]) } } }', + 'this.pending' + ], + [ + 'guarded accumulator', + 'let acc = Buffer.alloc(0); while (open) { acc = acc.length === 0 ? chunk : Buffer.concat([acc, chunk]) }', + 'acc' + ] +] + +const accepted = [ + [ + 'single concat after loop', + 'const parts = []; for (const chunk of chunks) { parts.push(chunk) } const out = Buffer.concat(parts)' + ], + [ + 'spread chunk list', + `let carryChunks = [] + while (scanEnd > 0) { + const region = carryChunks.length === 0 ? buffer : Buffer.concat([buffer, ...carryChunks]) + carryChunks = [buffer.subarray(0, firstNewline)] + }` + ], + [ + 'iteration-local result', + 'for (const group of groups) { const frame = Buffer.concat([group.header, group.body]); send(frame) }' + ], + [ + 'iteration-local accumulator', + 'for (const chunk of chunks) { let framed = HEADER; framed = Buffer.concat([framed, chunk]); send(framed) }' + ], + [ + 'no enclosing loop', + 'class S { handle(chunk) { const buffer = Buffer.concat([this.pending, chunk]); this.pending = parse(buffer).pending } }' + ], + ['no Buffer concat', 'export const x = 1'] +] + +describe('quadratic Buffer.concat Oxlint plugin', () => { + it.each(violations)('reports %s', (_name, source, accumulator) => { + const diagnostics = lintSource(source) + + expect(diagnostics).toHaveLength(1) + expect(diagnostics[0].message).toContain(accumulator) + }) + + it.each(accepted)('accepts %s', (_name, source) => { + expect(lintSource(source)).toEqual([]) + }) +}) diff --git a/config/scripts/renderer-scrollbar-style-plugin.test.mjs b/config/scripts/renderer-scrollbar-style-plugin.test.mjs new file mode 100644 index 000000000..fd67ffbbd --- /dev/null +++ b/config/scripts/renderer-scrollbar-style-plugin.test.mjs @@ -0,0 +1,116 @@ +import path from 'node:path' +import { describe, expect, it } from 'vitest' +import { plainClassName } from '../oxlint-plugins/renderer-scrollbar-style.mjs' +import { runOxlintPluginOnSource } from './oxlint-plugin-test-runner.mjs' + +const pluginPath = path.resolve('config/oxlint-plugins/renderer-scrollbar-style.mjs') + +function lintSource(source) { + return runOxlintPluginOnSource({ + pluginName: 'renderer-scrollbar-style', + pluginPath, + source, + rules: { + 'renderer-scrollbar-style/require-styled-vertical-scrollbar': 'warn' + } + }) +} + +const violations = [ + ['unstyled class', 'export const X = () =>
'], + [ + 'unstyled suffix-important class', + 'export const X = () =>
' + ], + [ + 'unknown scrollbar class', + 'export const X = () =>
' + ], + [ + 'separate class composer arguments', + "export const X = () =>
" + ], + [ + 'conditional scrollbar', + "export const X = ({ enabled }) =>
" + ], + [ + 'arbitrary class wrapper', + "export const X = () =>
" + ], + [ + 'mismatched responsive variants', + 'export const X = () =>
' + ], + ['inline overflow', "export const X = () =>
"], + [ + 'logical inline style spread', + "export const X = ({ open }) =>
" + ], + ['JSX spread class', "export const X = () =>
"], + [ + 'later spread override', + 'export const X = () =>
' + ] +] + +const accepted = [ + [ + 'styled vertical class', + 'export const X = () =>
' + ], + [ + 'styled suffix-important classes', + 'export const X = () =>
' + ], + [ + 'same composer literal', + "export const X = () =>
" + ], + [ + 'same conditional literal', + "export const X = ({ enabled }) =>
" + ], + ['horizontal-only overflow', 'export const X = () =>
'],
+  [
+    'matching responsive variants',
+    'export const X = () => 
' + ], + [ + 'unconditional scrollbar', + 'export const X = () =>
' + ], + [ + 'styled inline overflow', + 'export const X = () =>
' + ], + [ + 'styled JSX spread class', + "export const X = () =>
" + ], + [ + 'variant configuration', + "export const X = () =>
" + ] +] + +describe('renderer scrollbar style Oxlint plugin', () => { + it.each(violations)('reports %s', (_name, source) => { + expect(lintSource(source)).toHaveLength(1) + }) + + it.each(accepted)('accepts %s', (_name, source) => { + expect(lintSource(source)).toEqual([]) + }) + + it.each([ + ['md:overflow-y-auto', 'overflow-y-auto'], + ['[&:hover]:overflow-y-auto', 'overflow-y-auto'], + ['md:!scrollbar-editor', 'scrollbar-editor'], + ['!scrollbar-editor', 'scrollbar-editor'], + ['overflow-y-auto!', 'overflow-y-auto'], + ['md:scrollbar-editor!', 'scrollbar-editor'] + ])('normalizes %s', (token, expected) => { + expect(plainClassName(token)).toBe(expected) + }) +}) diff --git a/config/scripts/styled-scrollbars/styled-scrollbar-jsx-check.mjs b/config/scripts/styled-scrollbars/styled-scrollbar-jsx-check.mjs deleted file mode 100644 index 2a9d13b6f..000000000 --- a/config/scripts/styled-scrollbars/styled-scrollbar-jsx-check.mjs +++ /dev/null @@ -1,312 +0,0 @@ -// TypeScript 7 is a native CLI; AST consumers still need the legacy JavaScript API. -import ts from 'typescript-api' - -const STYLED_SCROLLBAR_CLASSES = new Set( - 'scrollbar-sleek scrollbar-editor worktree-sidebar-scrollbar'.split(' ') -) -// Why: vertical scroll is where native scrollbar drift keeps recurring. The -// guard intentionally ignores horizontal-only overflow. -const VERTICAL_SCROLL_CLASSES = new Set( - 'overflow-auto overflow-scroll overflow-y-auto overflow-y-scroll'.split(' ') -) -const VERTICAL_SCROLL_STYLE_VALUES = new Set(['auto', 'scroll']) - -export function plainClassName(token) { - const normalizedToken = token.startsWith('!') ? token.slice(1) : token - const parts = [] - let bracketDepth = 0 - let currentPart = '' - - for (const char of normalizedToken) { - if (char === '[') { - bracketDepth += 1 - } else if (char === ']') { - bracketDepth = Math.max(0, bracketDepth - 1) - } - - if (char === ':' && bracketDepth === 0) { - parts.push(currentPart) - currentPart = '' - continue - } - currentPart += char - } - - parts.push(currentPart) - const className = parts.at(-1) ?? '' - return className.startsWith('!') ? className.slice(1) : className -} - -function classTokenParts(token) { - const variants = [] - let bracketDepth = 0 - let currentPart = '' - - for (const char of token.startsWith('!') ? token.slice(1) : token) { - if (char === '[') { - bracketDepth += 1 - } else if (char === ']') { - bracketDepth = Math.max(0, bracketDepth - 1) - } - if (char === ':' && bracketDepth === 0) { - variants.push(currentPart) - currentPart = '' - continue - } - currentPart += char - } - - return { className: plainClassName(token), variants: variants.filter(Boolean) } -} - -function classTokens(text) { - return text.split(/\s+/).filter(Boolean).map(classTokenParts) -} - -function sameVariants(left, right) { - return left.length === right.length && left.every((variant, index) => variant === right[index]) -} - -function literalHasScrollbarForVertical(text, verticalToken) { - return classTokens(text).some((candidate) => { - if (!STYLED_SCROLLBAR_CLASSES.has(candidate.className)) { - return false - } - return ( - candidate.variants.length === 0 || sameVariants(candidate.variants, verticalToken.variants) - ) - }) -} - -function uncoveredVerticalClass(text) { - return classTokens(text).find((token) => { - return ( - VERTICAL_SCROLL_CLASSES.has(token.className) && !literalHasScrollbarForVertical(text, token) - ) - }) -} - -function reportAt(node, filePath, sourceFile, text) { - const position = sourceFile.getLineAndCharacterOfPosition(node.getStart(sourceFile)) - return { - filePath, - line: position.line + 1, - column: position.character + 1, - text - } -} - -function stringLiteralTexts(node) { - if (ts.isStringLiteralLike(node) || ts.isNoSubstitutionTemplateLiteral(node)) { - return [node.text] - } - if (!ts.isTemplateExpression(node)) { - return [] - } - return [node.head.text, ...node.templateSpans.map((span) => span.literal.text)] -} - -function collectClassLiteralReports(node, filePath, sourceFile) { - const reports = [] - - function visit(current) { - for (const text of stringLiteralTexts(current)) { - const uncovered = uncoveredVerticalClass(text) - if (uncovered) { - reports.push(reportAt(current, filePath, sourceFile, uncovered.className)) - } - } - - ts.forEachChild(current, visit) - } - - visit(node) - return reports -} - -function expressionHasStyledScrollbarLiteral(node) { - let hasStyledScrollbar = false - - function visit(current) { - if (hasStyledScrollbar) { - return - } - if ( - stringLiteralTexts(current).some((text) => - classTokens(text).some((token) => STYLED_SCROLLBAR_CLASSES.has(token.className)) - ) - ) { - hasStyledScrollbar = true - return - } - // Why: a scrollbar literal that only renders on some branches must not be - // treated as covering an unconditional inline overflow. Skip conditional - // and short-circuit expressions when proving unconditional coverage. - if (ts.isConditionalExpression(current)) { - return - } - if ( - ts.isBinaryExpression(current) && - (current.operatorToken.kind === ts.SyntaxKind.AmpersandAmpersandToken || - current.operatorToken.kind === ts.SyntaxKind.BarBarToken || - current.operatorToken.kind === ts.SyntaxKind.QuestionQuestionToken) - ) { - return - } - ts.forEachChild(current, visit) - } - - visit(node) - return hasStyledScrollbar -} - -function propertyNameText(name) { - if (ts.isIdentifier(name) || ts.isStringLiteralLike(name)) { - return name.text - } - if (ts.isComputedPropertyName(name) && ts.isStringLiteralLike(name.expression)) { - return name.expression.text - } - return undefined -} - -function styleValueIsVerticalScroll(propertyName, value) { - const parts = value.trim().toLowerCase().split(/\s+/).filter(Boolean) - if (parts.length === 0) { - return false - } - if (propertyName === 'overflowY' || propertyName === 'overflow-y') { - return VERTICAL_SCROLL_STYLE_VALUES.has(parts[0]) - } - if (propertyName !== 'overflow') { - return false - } - const verticalValue = parts.length > 1 ? parts[1] : parts[0] - return VERTICAL_SCROLL_STYLE_VALUES.has(verticalValue) -} - -function collectStyleReports(node, filePath, sourceFile) { - const reports = [] - - function visit(current) { - if (ts.isPropertyAssignment(current)) { - const propertyName = propertyNameText(current.name) - for (const value of propertyName ? stringLiteralTexts(current.initializer) : []) { - if (styleValueIsVerticalScroll(propertyName, value)) { - reports.push(reportAt(current, filePath, sourceFile, 'inline vertical scroll')) - } - } - ts.forEachChild(current.initializer, visit) - return - } - ts.forEachChild(current, visit) - } - - visit(node) - return reports -} - -function jsxAttributeName(attribute) { - return ts.isIdentifier(attribute.name) ? attribute.name.text : undefined -} - -function jsxAttributeExpression(attribute) { - if (ts.isStringLiteral(attribute.initializer)) { - return attribute.initializer - } - if (attribute.initializer && ts.isJsxExpression(attribute.initializer)) { - return attribute.initializer.expression - } - return undefined -} - -function spreadPropExpressions(node, propName) { - if ( - ts.isParenthesizedExpression(node) || - ts.isAsExpression(node) || - ts.isSatisfiesExpression(node) - ) { - return spreadPropExpressions(node.expression, propName) - } - if (ts.isConditionalExpression(node)) { - return [ - ...spreadPropExpressions(node.whenTrue, propName), - ...spreadPropExpressions(node.whenFalse, propName) - ] - } - if (ts.isBinaryExpression(node)) { - return [ - ...spreadPropExpressions(node.left, propName), - ...spreadPropExpressions(node.right, propName) - ] - } - if (!ts.isObjectLiteralExpression(node)) { - return [] - } - return node.properties.flatMap((property) => { - if (ts.isSpreadAssignment(property)) { - return spreadPropExpressions(property.expression, propName) - } - if (ts.isPropertyAssignment(property) && propertyNameText(property.name) === propName) { - return [property.initializer] - } - return [] - }) -} - -function jsxElementReports(node, filePath, sourceFile) { - const reports = [] - let classExpression - const styleExpressions = [] - - for (const attribute of node.attributes.properties) { - if (ts.isJsxSpreadAttribute(attribute)) { - // Why: at runtime React applies attributes in source order, so a later - // spread that supplies className overrides an earlier explicit className. - const spreadClassExpression = spreadPropExpressions(attribute.expression, 'className').at(-1) - if (spreadClassExpression) { - classExpression = spreadClassExpression - } - styleExpressions.push(...spreadPropExpressions(attribute.expression, 'style')) - } else if (jsxAttributeName(attribute) === 'className') { - classExpression = jsxAttributeExpression(attribute) - } else if (jsxAttributeName(attribute) === 'style') { - const expression = jsxAttributeExpression(attribute) - if (expression) { - styleExpressions.push(expression) - } - } - } - - if (classExpression) { - reports.push(...collectClassLiteralReports(classExpression, filePath, sourceFile)) - } - if (classExpression && expressionHasStyledScrollbarLiteral(classExpression)) { - return reports - } - for (const expression of styleExpressions) { - reports.push(...collectStyleReports(expression, filePath, sourceFile)) - } - return reports -} - -export function reportUnstyledScrollbars(filePath, sourceText) { - const sourceFile = ts.createSourceFile( - filePath, - sourceText, - ts.ScriptTarget.Latest, - true, - ts.ScriptKind.TSX - ) - const reports = [] - - function visit(node) { - if (ts.isJsxOpeningElement(node) || ts.isJsxSelfClosingElement(node)) { - reports.push(...jsxElementReports(node, filePath, sourceFile)) - } - ts.forEachChild(node, visit) - } - - visit(sourceFile) - return reports -} diff --git a/package.json b/package.json index 921d0e59e..5190a892e 100644 --- a/package.json +++ b/package.json @@ -11,9 +11,9 @@ "main": "./out/main/index.js", "scripts": { "format": "oxfmt --write .", - "lint": "oxlint && pnpm run audit:code-quality:native && pnpm run audit:code-quality:type-aware && pnpm run lint:switch-exhaustiveness && node config/scripts/check-styled-scrollbars.mjs && pnpm run check:quadratic-buffer-concat && pnpm run check:reliability-gates && pnpm run check:max-lines-ratchet && pnpm run verify:bundled-skill-guides && pnpm run verify:skill-bundle-manifest && pnpm run verify:localization-catalog && pnpm run verify:localization-coverage", + "lint": "oxlint && pnpm run audit:code-quality:native && pnpm run audit:code-quality:type-aware && pnpm run check:reliability-gates && pnpm run check:max-lines-ratchet && pnpm run verify:bundled-skill-guides && pnpm run verify:skill-bundle-manifest && pnpm run verify:localization-catalog && pnpm run verify:localization-coverage", "audit:code-quality": "pnpm run audit:code-quality:native && pnpm run audit:code-quality:type-aware && pnpm run audit:react-doctor", - "audit:code-quality:native": "oxlint --config config/oxlint-code-quality.json src config tests mobile --deny-warnings", + "audit:code-quality:native": "oxlint --config config/oxlint-code-quality-native-plugins.json src config tests mobile --deny-warnings", "audit:code-quality:type-aware": "oxlint --type-aware --config config/oxlint-code-quality-type-aware.json src config tests --deny-warnings", "audit:react-doctor": "pnpm dlx react-doctor@0.9.1 . --yes --no-supply-chain --no-telemetry --blocking none", "check:code-quality:changed": "node config/scripts/check-changed-code-quality.mjs", @@ -22,12 +22,9 @@ "doctor": "pnpm dlx react-doctor@0.9.1 . --no-telemetry", "lint:react-doctor": "oxlint --config config/oxlint-react-doctor.json", "lint:react-doctor:changed": "node config/scripts/lint-react-doctor-changed.mjs", - "lint:switch-exhaustiveness": "oxlint --type-aware --config config/oxlint-switch-exhaustiveness.json src/main src/preload src/shared src/relay src/cli src/renderer/src config tests --quiet", "prepare": "husky", "test": "node config/scripts/ensure-native-runtime.mjs --runtime=node && vitest run --config config/vitest.config.ts", "test:repro:remote-agent-session": "pnpm run build:cli && pnpm run build:electron-vite && node config/scripts/remote-agent-session-authority-repro.mjs", - "check:styled-scrollbars": "node config/scripts/check-styled-scrollbars.mjs", - "check:quadratic-buffer-concat": "node config/scripts/check-quadratic-buffer-concat.mjs", "check:reliability-gates": "node config/scripts/check-reliability-gates.mjs", "check:max-lines-ratchet": "node config/scripts/check-max-lines-ratchet.mjs", "check:feature-wall-assets": "node config/scripts/check-feature-wall-assets.mjs", diff --git a/src/renderer/src/components/terminal-pane/pty-connection.test.ts b/src/renderer/src/components/terminal-pane/pty-connection.test.ts index 4d6431880..7f13b6aeb 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.test.ts @@ -38,6 +38,20 @@ async function flushAsyncTicks(count = 6): Promise { } } +async function drainFakeTimerWork(limit = 20): Promise { + await flushAsyncTicks(20) + if (!vi.isFakeTimers()) { + return + } + for (let iteration = 0; iteration < limit && vi.getTimerCount() > 0; iteration += 1) { + await vi.runOnlyPendingTimersAsync() + await flushAsyncTicks(20) + } + vi.clearAllTimers() + await flushAsyncTicks(20) + vi.clearAllTimers() +} + async function drainPendingTimeouts(pendingTimeouts: (() => void)[], limit = 100): Promise { let iterations = 0 while (pendingTimeouts.length > 0) { @@ -941,8 +955,8 @@ describe('connectPanePty', () => { }) afterEach(async () => { - // Why: drain in-flight foreground-confirm microtasks while this test still owns the store mock, so its async fallout can't leak into (and flake) the next test. - await flushAsyncTicks(20) + // Drain deferred confirmation work before the next test replaces its store mock. + await drainFakeTimerWork() vi.useRealTimers() vi.restoreAllMocks() if (originalRequestAnimationFrame) {