Skip to content

Commit a25cd2c

Browse files
davidagustinclaude
davidagustin
and
claude
committed
fix(ui-patterns): comprehensive false positive detection and generator hardening
Expand false positive test from 3 to 7 detection categories (ternary logic, Math operations, string chains, object chains, unblanked function bodies, single/multi-line callbacks) and harden the starter code generator to blank all implementation patterns — async functions, property assignments, function() keyword callbacks, forEach/Observer constructors, and framework-aware React component skipping. 506 tests, all passing. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent b2b0b39 commit a25cd2c

6 files changed

Lines changed: 1171 additions & 1227 deletions

File tree

‎lib/__tests__/ui-pattern-false-positives.test.ts‎

Lines changed: 263 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,11 @@
44
* When the generate-starters.ts script creates skeleton code from reference
55
* implementations, it blanks function bodies but can miss "implementation
66
* expressions" — variable assignments containing method chains with callbacks
7-
* (e.g. `const filtered = items.filter(f => ...)`). These leaked expressions
8-
* cause behavioral tests to pass before the user writes any code.
7+
* (e.g. `const filtered = items.filter(f => ...)`), ternary logic, Math
8+
* operations, chained string methods, Object.keys/values/entries chains,
9+
* unblanked function bodies, and inline callbacks with implementation.
10+
* These leaked expressions cause behavioral tests to pass before the user
11+
* writes any code.
912
*
1013
* This test iterates every UI pattern across all 4 frameworks and flags
1114
* starters that contain implementation logic which should have been blanked.
@@ -37,6 +40,21 @@ const IMPL_METHOD_CHAIN = [
3740
/\.\s*forEach\s*\(\s*(?:\w+|\([^)]*\))\s*=>/,
3841
];
3942

43+
/** Ternary expressions with comparison logic (not simple fallbacks). */
44+
const IMPL_TERNARY =
45+
/\?[^:]*(?:[<>=!]=|\.test\(|\.includes\(|\.match\(|\.length\s*[<>=!]|Math\.|&&|\|\|).*:/;
46+
47+
/** Math.* operations (excluding Math.random alone). */
48+
const IMPL_MATH = /Math\.(floor|ceil|round|max|min|abs|pow|sqrt|sign|trunc)\s*\(/;
49+
50+
/** Chained string/array methods (2+ calls). */
51+
const IMPL_STRING_CHAIN =
52+
/\.\s*(?:replace|split|slice|substring|charAt|trim|padStart|padEnd)\s*\([^)]*\)\s*\.\s*(?:replace|split|join|slice|substring|charAt|trim|padStart|padEnd|toUpperCase|toLowerCase|map|filter)\s*\(/;
53+
54+
/** Object.keys/values/entries with callbacks. */
55+
const IMPL_OBJECT_CHAIN =
56+
/Object\.\s*(?:keys|values|entries)\s*\([^)]*\)\s*\.\s*(?:map|filter|reduce|forEach|some|every|find)\s*\(/;
57+
4058
/** Check whether a starter string looks like an actual skeleton. */
4159
function isSkeleton(code: string): boolean {
4260
return /\/\/\s*(TODO|Step\s+\d|Your\s+code|Your\s+answer|Implement)/i.test(code);
@@ -45,18 +63,31 @@ function isSkeleton(code: string): boolean {
4563
/**
4664
* Detect implementation expressions leaked into starter code.
4765
*
48-
* Only checks lines that are:
49-
* 1. Variable assignments (`const/let/var NAME = ...`)
50-
* 2. NOT function declarations (those are handled by blankFunctionBodies)
51-
* 3. NOT hook/state calls (useState, useRef, computed, ref, etc.)
52-
* 4. NOT simple destructuring from modules
53-
* 5. Contain method chains with arrow callbacks (.filter(f =>, .map(x =>, etc.)
66+
* Checks for:
67+
* 1. Variable assignments with method chains + arrow callbacks
68+
* 2. Ternary expressions with comparison logic
69+
* 3. Math.* operations combined with arithmetic
70+
* 4. Chained string/array methods (2+ calls)
71+
* 5. Object.keys/values/entries with callbacks
72+
* 6. Unblanked function/arrow bodies (no TODO comment)
73+
* 7. Single-line and multi-line callbacks with implementation
74+
*
75+
* Skips:
76+
* - Lines inside JSX return blocks
77+
* - Hook/reactive declarations
78+
* - Destructuring
79+
* - Lines containing TODO comments
80+
* - Simple fallbacks (x || 'default', x ?? fallback)
81+
* - React component functions (uppercase names) and depth-0 utilities (which contain JSX, not implementation)
5482
*
55-
* Returns array of { line, lineNumber, content } for each leak found.
83+
* Returns array of { lineNumber, content, category } for each leak found.
5684
*/
57-
function findImplementationLeaks(starterCode: string): { lineNumber: number; content: string }[] {
85+
function findImplementationLeaks(
86+
starterCode: string,
87+
frameworkName?: string,
88+
): { lineNumber: number; content: string; category: string }[] {
5889
const lines = starterCode.split('\n');
59-
const leaks: { lineNumber: number; content: string }[] = [];
90+
const leaks: { lineNumber: number; content: string; category: string }[] = [];
6091

6192
// Track brace depth to skip JSX/template regions
6293
let insideReturn = false;
@@ -65,10 +96,19 @@ function findImplementationLeaks(starterCode: string): { lineNumber: number; con
6596
// When a skipped line opens a block, skip all lines until the block closes.
6697
let skipBraceDepth = 0;
6798

99+
// Track overall brace depth to detect if we're inside the main component/function
100+
let overallBraceDepth = 0;
101+
68102
for (let i = 0; i < lines.length; i++) {
69103
const line = lines[i];
70104
const trimmed = line.trim();
71105

106+
// Track overall brace depth
107+
for (const ch of line) {
108+
if (ch === '{') overallBraceDepth++;
109+
if (ch === '}') overallBraceDepth--;
110+
}
111+
72112
// If inside a skipped block, track braces and skip until balanced
73113
if (skipBraceDepth > 0) {
74114
for (const ch of line) {
@@ -86,57 +126,228 @@ function findImplementationLeaks(starterCode: string): { lineNumber: number; con
86126
// Skip lines inside JSX return block
87127
if (insideReturn) continue;
88128

89-
// Only check const/let/var assignments
90-
if (!/^(const|let|var)\s+\w+\s*=/.test(trimmed)) continue;
129+
// Skip TODO/blanked lines (applies to all checks)
130+
if (/\/\/\s*TODO/i.test(trimmed)) continue;
131+
132+
// ── Variable assignment checks ──
133+
const isVarAssignment = /^(const|let|var)\s+\w+\s*=/.test(trimmed);
91134

92-
// Skip lines that are function declarations (already blanked or intentionally kept)
93-
// Arrow function declarations: `const fn = (params) => {` or `const fn = param => {`
94-
if (/^(const|let|var)\s+\w+\s*=\s*(?:\([^)]*\)|\w+)\s*=>\s*\{/.test(trimmed)) continue;
95-
// Function expressions: `const fn = function(`
96-
if (/^(const|let|var)\s+\w+\s*=\s*function\s*\(/.test(trimmed)) continue;
135+
if (isVarAssignment) {
136+
// Skip lines that are function declarations — handled by unblanked-function-body check below
137+
const isFuncDecl =
138+
/^(const|let|var)\s+\w+\s*=\s*(?:\([^)]*\)|\w+)\s*=>\s*\{/.test(trimmed) ||
139+
/^(const|let|var)\s+\w+\s*=\s*function\s*\(/.test(trimmed);
97140

98-
// Skip hook/state calls — also skip their block bodies if they open one
99-
if (/=\s*use\w+\s*\(/.test(trimmed) || /=\s*React\.use\w+\s*\(/.test(trimmed)) {
100-
for (const ch of line) {
101-
if (ch === '{') skipBraceDepth++;
102-
if (ch === '}') skipBraceDepth--;
141+
// Skip hook/state calls — also skip their block bodies if they open one
142+
if (/=\s*use\w+\s*\(/.test(trimmed) || /=\s*React\.use\w+\s*\(/.test(trimmed)) {
143+
for (const ch of line) {
144+
if (ch === '{') skipBraceDepth++;
145+
if (ch === '}') skipBraceDepth--;
146+
}
147+
continue;
103148
}
104-
continue;
105-
}
106-
if (/=\s*(?:ref|reactive|computed|watch)\s*\(/.test(trimmed)) {
107-
for (const ch of line) {
108-
if (ch === '{') skipBraceDepth++;
109-
if (ch === '}') skipBraceDepth--;
149+
if (/=\s*(?:ref|reactive|computed|watch)\s*\(/.test(trimmed)) {
150+
for (const ch of line) {
151+
if (ch === '{') skipBraceDepth++;
152+
if (ch === '}') skipBraceDepth--;
153+
}
154+
continue;
155+
}
156+
157+
// Skip destructuring from modules: const { ... } = React or const [...] = useState(...)
158+
if (/^(const|let|var)\s*[[{]/.test(trimmed)) continue;
159+
160+
// ── Check for unblanked function/arrow bodies ──
161+
if (isFuncDecl) {
162+
// Skip depth-0 functions (utilities before the main component) — these should have bodies
163+
// For React, also skip component functions (uppercase names) — they contain JSX, not implementation
164+
const fnNameMatch = trimmed.match(/^(?:const|let|var)\s+(\w+)/);
165+
const fnName = fnNameMatch?.[1];
166+
const isReactComponent = frameworkName === 'React' && fnName && /^[A-Z]/.test(fnName);
167+
const isDepth0 = overallBraceDepth === 1; // Opening brace already counted above, so depth 1 = top-level
168+
169+
if (isDepth0 || isReactComponent) {
170+
continue; // Depth-0 utility or React component — skip this check
171+
}
172+
173+
// Find matching close brace
174+
let depth = 0;
175+
let bodyEnd = i;
176+
for (let j = i; j < lines.length; j++) {
177+
for (const ch of lines[j]) {
178+
if (ch === '{') depth++;
179+
if (ch === '}') {
180+
depth--;
181+
if (depth === 0) {
182+
bodyEnd = j;
183+
break;
184+
}
185+
}
186+
}
187+
if (depth === 0) break;
188+
}
189+
190+
// Check body for TODO
191+
const bodyLines = lines.slice(i + 1, bodyEnd);
192+
const bodyText = bodyLines.join('\n');
193+
const hasTodo = /\/\/\s*TODO/i.test(bodyText);
194+
const nonEmptyBodyLines = bodyLines.filter((l) => l.trim().length > 0);
195+
196+
if (!hasTodo && nonEmptyBodyLines.length > 1) {
197+
leaks.push({ lineNumber: i + 1, content: trimmed, category: 'unblanked-function-body' });
198+
}
199+
continue;
200+
}
201+
202+
// ── Method chain with arrow callback (existing check) ──
203+
const hasImplMethodChain = IMPL_METHOD_CHAIN.some((rx) => rx.test(trimmed));
204+
205+
if (hasImplMethodChain) {
206+
leaks.push({ lineNumber: i + 1, content: trimmed, category: 'method-chain' });
207+
continue;
208+
}
209+
210+
// ── Ternary expressions with comparison logic ──
211+
// Skip simple fallbacks like `x || 'default'` and `x ?? fallback`
212+
const afterEquals = trimmed.replace(/^(const|let|var)\s+\w+\s*=\s*/, '');
213+
const isSimpleFallback =
214+
/^\w+(?:\.\w+)*\s*(?:\|\||&&|\?\?)\s*['"`\w]/.test(afterEquals) &&
215+
!/\?[^:]*:/.test(afterEquals);
216+
if (!isSimpleFallback && IMPL_TERNARY.test(trimmed)) {
217+
leaks.push({ lineNumber: i + 1, content: trimmed, category: 'ternary-logic' });
218+
continue;
219+
}
220+
221+
// ── Math.* operations combined with arithmetic ──
222+
if (IMPL_MATH.test(trimmed)) {
223+
// Only flag if combined with arithmetic operators (/, *, -, +) or nested expressions
224+
const hasMathWithArithmetic =
225+
/Math\.\w+\s*\([^)]*[/*\-+][^)]*\)/.test(trimmed) ||
226+
/[/*\-+]\s*Math\./.test(trimmed) ||
227+
/Math\.\w+\s*\([^)]*\)\s*[/*\-+]/.test(trimmed);
228+
if (hasMathWithArithmetic) {
229+
leaks.push({ lineNumber: i + 1, content: trimmed, category: 'math-operation' });
230+
continue;
231+
}
232+
}
233+
234+
// ── Chained string methods (2+ calls) ──
235+
if (IMPL_STRING_CHAIN.test(trimmed)) {
236+
leaks.push({ lineNumber: i + 1, content: trimmed, category: 'string-chain' });
237+
continue;
238+
}
239+
240+
// ── Object.keys/values/entries with callbacks ──
241+
if (IMPL_OBJECT_CHAIN.test(trimmed)) {
242+
leaks.push({ lineNumber: i + 1, content: trimmed, category: 'object-chain' });
243+
continue;
244+
}
245+
246+
// ── Multi-line continuation check for method chains (existing) ──
247+
if (!trimmed.endsWith(';')) {
248+
const contLines = [trimmed];
249+
for (let k = i + 1; k < Math.min(i + 5, lines.length); k++) {
250+
contLines.push(lines[k].trim());
251+
if (lines[k].trim().endsWith(';')) break;
252+
}
253+
const joined = contLines.join(' ');
254+
if (IMPL_METHOD_CHAIN.some((rx) => rx.test(joined))) {
255+
leaks.push({ lineNumber: i + 1, content: trimmed, category: 'method-chain' });
256+
continue;
257+
}
110258
}
259+
111260
continue;
112261
}
113262

114-
// Skip destructuring from modules: const { ... } = React or const [...] = useState(...)
115-
if (/^(const|let|var)\s*[[{]/.test(trimmed)) continue;
263+
// ── Non-variable-assignment checks ──
116264

117-
// Skip TODO/blanked lines
118-
if (/\/\/\s*TODO/i.test(trimmed)) continue;
265+
// Skip function/class declarations (handled separately)
266+
if (/^(function|class)\s/.test(trimmed)) {
267+
// Check for unblanked function body (standalone function declarations)
268+
const funcMatch = trimmed.match(/^function\s+(\w+)\s*\([^)]*\)\s*\{/);
269+
if (funcMatch) {
270+
// Skip depth-0 functions (utilities before the main component) — these should have bodies
271+
// For React, also skip component functions (uppercase names) — they contain JSX, not implementation
272+
const fnName = funcMatch[1];
273+
const isReactComponent = frameworkName === 'React' && /^[A-Z]/.test(fnName);
274+
const isDepth0 = overallBraceDepth === 1; // Opening brace already counted above, so depth 1 = top-level
119275

120-
// Check for implementation method chains with callbacks
121-
const hasImplMethodChain = IMPL_METHOD_CHAIN.some((rx) => rx.test(trimmed));
276+
if (isDepth0 || isReactComponent) {
277+
continue; // Depth-0 utility or React component — skip this check
278+
}
122279

123-
// Also check multi-line: if this line doesn't end with ; and continues
124-
// to the next line(s), join them and check — but stop at statement boundaries
125-
if (!hasImplMethodChain && !trimmed.endsWith(';')) {
126-
const contLines = [trimmed];
127-
for (let k = i + 1; k < Math.min(i + 5, lines.length); k++) {
128-
contLines.push(lines[k].trim());
129-
if (lines[k].trim().endsWith(';')) break;
280+
let depth = 0;
281+
let bodyEnd = i;
282+
for (let j = i; j < lines.length; j++) {
283+
for (const ch of lines[j]) {
284+
if (ch === '{') depth++;
285+
if (ch === '}') {
286+
depth--;
287+
if (depth === 0) {
288+
bodyEnd = j;
289+
break;
290+
}
291+
}
292+
}
293+
if (depth === 0) break;
294+
}
295+
296+
const bodyLines = lines.slice(i + 1, bodyEnd);
297+
const bodyText = bodyLines.join('\n');
298+
const hasTodo = /\/\/\s*TODO/i.test(bodyText);
299+
const nonEmptyBodyLines = bodyLines.filter((l) => l.trim().length > 0);
300+
301+
if (!hasTodo && nonEmptyBodyLines.length > 1) {
302+
leaks.push({ lineNumber: i + 1, content: trimmed, category: 'unblanked-function-body' });
303+
}
130304
}
131-
const joined = contLines.join(' ');
132-
if (IMPL_METHOD_CHAIN.some((rx) => rx.test(joined))) {
133-
leaks.push({ lineNumber: i + 1, content: trimmed });
134-
continue;
305+
continue;
306+
}
307+
308+
// ── Single-line callback detection ──
309+
// something.addEventListener('event', () => { logic; });
310+
// items.forEach(item => { logic; });
311+
const hasSingleLineCallback = /=>\s*\{[^}]+\}/.test(trimmed);
312+
const isCallbackStatement =
313+
/\.addEventListener\s*\(/.test(trimmed) ||
314+
/\.\s*(?:forEach|map|reduce|filter)\s*\(/.test(trimmed) ||
315+
/new\s+(?:IntersectionObserver|MutationObserver|ResizeObserver)\s*\(/.test(trimmed);
316+
317+
if (hasSingleLineCallback && isCallbackStatement) {
318+
// Extract the callback body and check if it has implementation
319+
const bodyMatch = trimmed.match(/=>\s*\{([^}]+)\}/);
320+
if (bodyMatch) {
321+
const callbackBody = bodyMatch[1].trim();
322+
// Skip if it's just a TODO
323+
if (!/\/\/\s*TODO/i.test(callbackBody) && callbackBody.length > 5) {
324+
leaks.push({ lineNumber: i + 1, content: trimmed, category: 'single-line-callback' });
325+
continue;
326+
}
135327
}
136328
}
137329

138-
if (hasImplMethodChain) {
139-
leaks.push({ lineNumber: i + 1, content: trimmed });
330+
// ── Multi-line callback without TODO (not on variable assignment) ──
331+
if (/=>\s*\{$/.test(trimmed) && isCallbackStatement) {
332+
let depth = 0;
333+
let bodyEnd = i;
334+
for (let j = i; j < lines.length; j++) {
335+
for (const ch of lines[j]) {
336+
if (ch === '{') depth++;
337+
if (ch === '}') {
338+
depth--;
339+
if (depth === 0) {
340+
bodyEnd = j;
341+
break;
342+
}
343+
}
344+
}
345+
if (depth === 0) break;
346+
}
347+
const bodyText = lines.slice(i + 1, bodyEnd).join('\n');
348+
if (!/\/\/\s*TODO/i.test(bodyText) && bodyText.trim().length > 5) {
349+
leaks.push({ lineNumber: i + 1, content: trimmed, category: 'multi-line-callback' });
350+
}
140351
}
141352
}
142353

@@ -187,10 +398,12 @@ describe('UI Pattern starter code — false positive detection', () => {
187398
if (!isSkeleton(starter)) continue;
188399

189400
it(`${patternId} — starter should not contain implementation expressions`, () => {
190-
const leaks = findImplementationLeaks(starter);
401+
const leaks = findImplementationLeaks(starter, fw.name);
191402

192403
if (leaks.length > 0) {
193-
const details = leaks.map((l) => ` line ${l.lineNumber}: ${l.content}`).join('\n');
404+
const details = leaks
405+
.map((l) => ` [${l.category}] line ${l.lineNumber}: ${l.content}`)
406+
.join('\n');
194407
expect.soft(leaks.length, `Implementation leaked in ${patternId}:\n${details}`).toBe(0);
195408
}
196409
});

0 commit comments

Comments
 (0)