mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-13 06:36:26 +03:00
Four new layout detections: - nested-cards: jsdom DOM walk finds card-like elements (shadow + rounded + bg) nested inside other card-like elements. Excludes dropdowns (absolute/fixed), form inputs, code blocks, badges (<20 chars), and known component classes. - identical-card-grid: detects grid/flex parents with 3+ children sharing the same structural fingerprint (icon + heading + paragraph template pattern). - monotonous-spacing: regex on raw HTML collects padding/margin/gap values (px, rem, Tailwind classes), rounds to nearest 4px, flags when >60% use the same value with <=3 distinct values. - everything-centered: regex counts text-align:center and Tailwind text-center on text elements, flags when >70% of 5+ text elements are centered. Also narrowed pure-black-white to only flag #000 as background color — text-black, text-white, bg-white, and #fff are no longer flagged (too common, per user feedback). Extensive should-pass fixture covers: shadcn card sub-components, cards with form inputs/dropdowns/code blocks/badges/accordions/tabs/images, pricing cards, varied spacing, mixed centered/left-aligned layouts. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
430 lines
18 KiB
JavaScript
430 lines
18 KiB
JavaScript
import { describe, test, expect } from 'bun:test';
|
|
import fs from 'fs';
|
|
import path from 'path';
|
|
import { spawnSync } from 'child_process';
|
|
import {
|
|
ANTIPATTERNS, checkElementBorders, isNeutralColor, isFullPage,
|
|
detectHtml, detectText,
|
|
walkDir, SCANNABLE_EXTENSIONS,
|
|
} from '../source/skills/critique/scripts/detect-antipatterns.mjs';
|
|
|
|
const FIXTURES = path.join(import.meta.dir, 'fixtures', 'antipatterns');
|
|
const SCRIPT = path.join(import.meta.dir, '..', 'source', 'skills', 'critique', 'scripts', 'detect-antipatterns.mjs');
|
|
|
|
// ---------------------------------------------------------------------------
|
|
// Core: checkElementBorders (computed style simulation)
|
|
// ---------------------------------------------------------------------------
|
|
|
|
describe('checkElementBorders', () => {
|
|
function mockStyle(overrides) {
|
|
return { borderTopWidth: '0', borderRightWidth: '0', borderBottomWidth: '0', borderLeftWidth: '0',
|
|
borderTopColor: '', borderRightColor: '', borderBottomColor: '', borderLeftColor: '',
|
|
borderRadius: '0', ...overrides };
|
|
}
|
|
|
|
test('detects side-tab with radius', () => {
|
|
const f = checkElementBorders('div', mockStyle({
|
|
borderLeftWidth: '4', borderLeftColor: 'rgb(59, 130, 246)', borderRadius: '12',
|
|
}));
|
|
expect(f.length).toBe(1);
|
|
expect(f[0].id).toBe('side-tab');
|
|
});
|
|
|
|
test('detects side-tab without radius (thick)', () => {
|
|
const f = checkElementBorders('div', mockStyle({
|
|
borderLeftWidth: '4', borderLeftColor: 'rgb(59, 130, 246)',
|
|
}));
|
|
expect(f.length).toBe(1);
|
|
expect(f[0].id).toBe('side-tab');
|
|
});
|
|
|
|
test('skips side border below threshold without radius', () => {
|
|
const f = checkElementBorders('div', mockStyle({
|
|
borderLeftWidth: '2', borderLeftColor: 'rgb(59, 130, 246)',
|
|
}));
|
|
expect(f).toHaveLength(0);
|
|
});
|
|
|
|
test('detects border-accent-on-rounded (top)', () => {
|
|
const f = checkElementBorders('div', mockStyle({
|
|
borderTopWidth: '3', borderTopColor: 'rgb(139, 92, 246)', borderRadius: '12',
|
|
}));
|
|
expect(f.length).toBe(1);
|
|
expect(f[0].id).toBe('border-accent-on-rounded');
|
|
});
|
|
|
|
test('skips safe tags', () => {
|
|
const f = checkElementBorders('blockquote', mockStyle({
|
|
borderLeftWidth: '4', borderLeftColor: 'rgb(59, 130, 246)',
|
|
}));
|
|
expect(f).toHaveLength(0);
|
|
});
|
|
|
|
test('skips neutral colors', () => {
|
|
const f = checkElementBorders('div', mockStyle({
|
|
borderLeftWidth: '4', borderLeftColor: 'rgb(200, 200, 200)',
|
|
}));
|
|
expect(f).toHaveLength(0);
|
|
});
|
|
|
|
test('skips uniform borders (not accent)', () => {
|
|
const f = checkElementBorders('div', mockStyle({
|
|
borderTopWidth: '2', borderRightWidth: '2', borderBottomWidth: '2', borderLeftWidth: '2',
|
|
borderTopColor: 'rgb(59, 130, 246)', borderRightColor: 'rgb(59, 130, 246)',
|
|
borderBottomColor: 'rgb(59, 130, 246)', borderLeftColor: 'rgb(59, 130, 246)',
|
|
}));
|
|
expect(f).toHaveLength(0);
|
|
});
|
|
});
|
|
|
|
// ---------------------------------------------------------------------------
|
|
// isNeutralColor
|
|
// ---------------------------------------------------------------------------
|
|
|
|
describe('isNeutralColor', () => {
|
|
test('gray is neutral', () => expect(isNeutralColor('rgb(200, 200, 200)')).toBe(true));
|
|
test('blue is not neutral', () => expect(isNeutralColor('rgb(59, 130, 246)')).toBe(false));
|
|
test('transparent is neutral', () => expect(isNeutralColor('transparent')).toBe(true));
|
|
test('null is neutral', () => expect(isNeutralColor(null)).toBe(true));
|
|
});
|
|
|
|
// ---------------------------------------------------------------------------
|
|
// Regex fallback (detectText)
|
|
// ---------------------------------------------------------------------------
|
|
|
|
describe('detectText — Tailwind side-tab', () => {
|
|
test('detects border-l-4 (thick, no rounded needed)', () => {
|
|
const f = detectText('<div class="border-l-4 border-blue-500">', 'test.html');
|
|
expect(f.some(r => r.antipattern === 'side-tab')).toBe(true);
|
|
});
|
|
|
|
test('detects border-l-1 + rounded', () => {
|
|
const f = detectText('<div class="border-l-1 border-blue-500 rounded-md">', 'test.html');
|
|
expect(f.some(r => r.antipattern === 'side-tab')).toBe(true);
|
|
});
|
|
|
|
test('ignores border-l-1 without rounded', () => {
|
|
const f = detectText('<div class="border-l-1 border-gray-300">', 'test.html');
|
|
expect(f.filter(r => r.antipattern === 'side-tab')).toHaveLength(0);
|
|
});
|
|
|
|
test('ignores border-t without rounded', () => {
|
|
const f = detectText('<div class="border-t-4 border-b-4">', 'test.html');
|
|
expect(f.filter(r => r.antipattern === 'border-accent-on-rounded')).toHaveLength(0);
|
|
});
|
|
});
|
|
|
|
describe('detectText — CSS borders', () => {
|
|
test('detects border-left shorthand', () => {
|
|
const f = detectText('.card { border-left: 4px solid #3b82f6; }', 'test.css');
|
|
expect(f.some(r => r.antipattern === 'side-tab')).toBe(true);
|
|
});
|
|
|
|
test('ignores neutral border', () => {
|
|
const f = detectText('.card { border-left: 4px solid #e5e7eb; }', 'test.css');
|
|
expect(f.filter(r => r.antipattern === 'side-tab')).toHaveLength(0);
|
|
});
|
|
|
|
test('skips blockquote', () => {
|
|
const f = detectText('<blockquote style="border-left: 4px solid #ccc;">', 'test.html');
|
|
expect(f.filter(r => r.antipattern === 'side-tab')).toHaveLength(0);
|
|
});
|
|
});
|
|
|
|
describe('detectText — overused fonts', () => {
|
|
test('detects Inter', () => {
|
|
const f = detectText("body { font-family: 'Inter', sans-serif; }", 'test.css');
|
|
expect(f.some(r => r.antipattern === 'overused-font')).toBe(true);
|
|
});
|
|
|
|
test('does not flag distinctive fonts', () => {
|
|
const f = detectText("body { font-family: 'Instrument Sans', sans-serif; }", 'test.css');
|
|
expect(f.filter(r => r.antipattern === 'overused-font')).toHaveLength(0);
|
|
});
|
|
});
|
|
|
|
describe('detectText — flat type hierarchy', () => {
|
|
test('flags sizes too close together', () => {
|
|
const page = '<!DOCTYPE html><html><style>h1{font-size:18px}h2{font-size:16px}h3{font-size:15px}p{font-size:14px}.s{font-size:13px}</style></html>';
|
|
const f = detectText(page, 'test.html');
|
|
expect(f.some(r => r.antipattern === 'flat-type-hierarchy')).toBe(true);
|
|
});
|
|
|
|
test('passes good hierarchy', () => {
|
|
const page = '<!DOCTYPE html><html><style>h1{font-size:48px}h2{font-size:32px}p{font-size:16px}.s{font-size:12px}</style></html>';
|
|
const f = detectText(page, 'test.html');
|
|
expect(f.filter(r => r.antipattern === 'flat-type-hierarchy')).toHaveLength(0);
|
|
});
|
|
});
|
|
|
|
// ---------------------------------------------------------------------------
|
|
// jsdom detection (detectHtml)
|
|
// ---------------------------------------------------------------------------
|
|
|
|
describe('detectHtml — jsdom', () => {
|
|
test('catches side-tab from inline style', async () => {
|
|
const f = await detectHtml(path.join(FIXTURES, 'should-flag.html'));
|
|
expect(f.some(r => r.antipattern === 'side-tab')).toBe(true);
|
|
});
|
|
|
|
test('catches border-accent-on-rounded', async () => {
|
|
const f = await detectHtml(path.join(FIXTURES, 'should-flag.html'));
|
|
expect(f.some(r => r.antipattern === 'border-accent-on-rounded')).toBe(true);
|
|
});
|
|
|
|
test('should-pass has zero border findings', async () => {
|
|
const f = await detectHtml(path.join(FIXTURES, 'should-pass.html'));
|
|
const borderFindings = f.filter(r => r.antipattern === 'side-tab' || r.antipattern === 'border-accent-on-rounded');
|
|
expect(borderFindings).toHaveLength(0);
|
|
});
|
|
|
|
test('catches side-tab from linked stylesheet', async () => {
|
|
const f = await detectHtml(path.join(FIXTURES, 'linked-stylesheet.html'));
|
|
expect(f.some(r => r.antipattern === 'side-tab')).toBe(true);
|
|
});
|
|
|
|
test('catches border-accent-on-rounded from linked stylesheet', async () => {
|
|
const f = await detectHtml(path.join(FIXTURES, 'linked-stylesheet.html'));
|
|
expect(f.some(r => r.antipattern === 'border-accent-on-rounded')).toBe(true);
|
|
});
|
|
|
|
test('does not flag clean card from linked stylesheet', async () => {
|
|
const f = await detectHtml(path.join(FIXTURES, 'linked-stylesheet.html'));
|
|
const cleanFindings = f.filter(r => r.snippet?.includes('clean'));
|
|
expect(cleanFindings).toHaveLength(0);
|
|
});
|
|
|
|
test('partial-component: flags borders, skips page-level', async () => {
|
|
const f = await detectHtml(path.join(FIXTURES, 'partial-component.html'));
|
|
expect(f.some(r => r.antipattern === 'side-tab')).toBe(true);
|
|
expect(f.filter(r => r.antipattern === 'flat-type-hierarchy')).toHaveLength(0);
|
|
expect(f.filter(r => r.antipattern === 'single-font')).toHaveLength(0);
|
|
expect(f.filter(r => r.antipattern === 'overused-font')).toHaveLength(0);
|
|
});
|
|
|
|
test('color-should-flag detects all five color issues', async () => {
|
|
const f = await detectHtml(path.join(FIXTURES, 'color-should-flag.html'));
|
|
expect(f.some(r => r.antipattern === 'pure-black-white')).toBe(true);
|
|
expect(f.some(r => r.antipattern === 'gray-on-color')).toBe(true);
|
|
expect(f.some(r => r.antipattern === 'low-contrast')).toBe(true);
|
|
expect(f.some(r => r.antipattern === 'gradient-text')).toBe(true);
|
|
expect(f.some(r => r.antipattern === 'ai-color-palette')).toBe(true);
|
|
});
|
|
|
|
test('color-should-pass has zero findings', async () => {
|
|
const f = await detectHtml(path.join(FIXTURES, 'color-should-pass.html'));
|
|
expect(f).toHaveLength(0);
|
|
});
|
|
|
|
test('legitimate-borders has minimal false positives', async () => {
|
|
const f = await detectHtml(path.join(FIXTURES, 'legitimate-borders.html'));
|
|
const borderFindings = f.filter(r => r.antipattern === 'side-tab' || r.antipattern === 'border-accent-on-rounded');
|
|
// Alert banner is the only expected detection
|
|
expect(borderFindings.length).toBeLessThanOrEqual(1);
|
|
});
|
|
|
|
test('typography-should-flag detects all three issues', async () => {
|
|
const f = await detectHtml(path.join(FIXTURES, 'typography-should-flag.html'));
|
|
expect(f.some(r => r.antipattern === 'overused-font')).toBe(true);
|
|
expect(f.some(r => r.antipattern === 'single-font')).toBe(true);
|
|
expect(f.some(r => r.antipattern === 'flat-type-hierarchy')).toBe(true);
|
|
});
|
|
|
|
test('typography-should-pass has zero findings', async () => {
|
|
const f = await detectHtml(path.join(FIXTURES, 'typography-should-pass.html'));
|
|
expect(f).toHaveLength(0);
|
|
});
|
|
});
|
|
|
|
// ---------------------------------------------------------------------------
|
|
// Full page vs partial detection
|
|
// ---------------------------------------------------------------------------
|
|
|
|
describe('isFullPage', () => {
|
|
test('detects DOCTYPE', () => expect(isFullPage('<!DOCTYPE html><html>')).toBe(true));
|
|
test('detects <html>', () => expect(isFullPage('<html><head></head>')).toBe(true));
|
|
test('detects <head>', () => expect(isFullPage('<head><meta charset="UTF-8"></head>')).toBe(true));
|
|
test('rejects component/partial', () => expect(isFullPage('<div class="card">content</div>')).toBe(false));
|
|
test('rejects JSX', () => expect(isFullPage('export default function Card() { return <div>hi</div> }')).toBe(false));
|
|
});
|
|
|
|
describe('partials skip page-level checks', () => {
|
|
test('regex: partial with flat hierarchy is not flagged', () => {
|
|
const partial = '<div style="font-size: 14px">text</div>\n<div style="font-size: 16px">text</div>\n<div style="font-size: 15px">text</div>';
|
|
const f = detectText(partial, 'card.tsx');
|
|
expect(f.filter(r => r.antipattern === 'flat-type-hierarchy')).toHaveLength(0);
|
|
});
|
|
|
|
test('regex: partial with single overused font is not flagged for single-font', () => {
|
|
const partial = `<div style="font-family: 'Inter', sans-serif; font-size: 14px">text</div>\n`.repeat(25);
|
|
const f = detectText(partial, 'card.tsx');
|
|
expect(f.filter(r => r.antipattern === 'single-font')).toHaveLength(0);
|
|
});
|
|
|
|
test('regex: partial still flags border anti-patterns', () => {
|
|
const partial = '<div class="border-l-4 border-blue-500 rounded-lg">card</div>';
|
|
const f = detectText(partial, 'card.tsx');
|
|
expect(f.some(r => r.antipattern === 'side-tab')).toBe(true);
|
|
});
|
|
|
|
test('regex: full page with flat hierarchy IS flagged', () => {
|
|
const page = '<!DOCTYPE html><html><head></head><body>\n' +
|
|
'<h1 style="font-size: 18px">h1</h1>\n<h2 style="font-size: 16px">h2</h2>\n' +
|
|
'<p style="font-size: 14px">p</p>\n<span style="font-size: 15px">s</span>\n' +
|
|
'<small style="font-size: 13px">sm</small>\n</body></html>';
|
|
const f = detectText(page, 'index.html');
|
|
expect(f.some(r => r.antipattern === 'flat-type-hierarchy')).toBe(true);
|
|
});
|
|
});
|
|
|
|
// ---------------------------------------------------------------------------
|
|
// Layout anti-patterns
|
|
// ---------------------------------------------------------------------------
|
|
|
|
describe('detectHtml — layout', () => {
|
|
test('layout-should-flag: detects nested cards', async () => {
|
|
const f = await detectHtml(path.join(FIXTURES, 'layout-should-flag.html'));
|
|
expect(f.some(r => r.antipattern === 'nested-cards')).toBe(true);
|
|
});
|
|
|
|
test('layout-should-flag: detects identical card grid', async () => {
|
|
const f = await detectHtml(path.join(FIXTURES, 'layout-should-flag.html'));
|
|
expect(f.some(r => r.antipattern === 'identical-card-grid')).toBe(true);
|
|
});
|
|
|
|
test('detects monotonous spacing via regex', () => {
|
|
// A page where every padding/margin is 16px
|
|
const html = '<!DOCTYPE html><html><body>' +
|
|
'<div style="padding: 16px; margin-bottom: 16px;"><p style="margin-bottom: 16px;">a</p></div>'.repeat(5) +
|
|
'</body></html>';
|
|
const f = detectText(html, 'test.html');
|
|
expect(f.some(r => r.antipattern === 'monotonous-spacing')).toBe(true);
|
|
});
|
|
|
|
test('detects everything centered via regex', () => {
|
|
const html = `<!DOCTYPE html><html><body>
|
|
<h1 style="text-align: center;">Title</h1>
|
|
<p style="text-align: center;">Paragraph one more text here</p>
|
|
<p style="text-align: center;">Paragraph two more text here</p>
|
|
<p style="text-align: center;">Paragraph three more text here</p>
|
|
<p style="text-align: center;">Paragraph four more text here</p>
|
|
<p style="text-align: center;">Paragraph five more text here</p>
|
|
<p style="text-align: center;">Paragraph six more text here</p>
|
|
</body></html>`;
|
|
const f = detectText(html, 'test.html');
|
|
expect(f.some(r => r.antipattern === 'everything-centered')).toBe(true);
|
|
});
|
|
|
|
test('layout-should-pass: no nested-cards false positives', async () => {
|
|
const f = await detectHtml(path.join(FIXTURES, 'layout-should-pass.html'));
|
|
expect(f.filter(r => r.antipattern === 'nested-cards')).toHaveLength(0);
|
|
});
|
|
|
|
test('layout-should-pass: no identical-card-grid false positives', async () => {
|
|
const f = await detectHtml(path.join(FIXTURES, 'layout-should-pass.html'));
|
|
expect(f.filter(r => r.antipattern === 'identical-card-grid')).toHaveLength(0);
|
|
});
|
|
|
|
test('layout-should-pass: no monotonous-spacing false positives', async () => {
|
|
const f = await detectHtml(path.join(FIXTURES, 'layout-should-pass.html'));
|
|
expect(f.filter(r => r.antipattern === 'monotonous-spacing')).toHaveLength(0);
|
|
});
|
|
|
|
test('layout-should-pass: no everything-centered false positives', async () => {
|
|
const f = await detectHtml(path.join(FIXTURES, 'layout-should-pass.html'));
|
|
expect(f.filter(r => r.antipattern === 'everything-centered')).toHaveLength(0);
|
|
});
|
|
});
|
|
|
|
// ---------------------------------------------------------------------------
|
|
// ANTIPATTERNS registry
|
|
// ---------------------------------------------------------------------------
|
|
|
|
describe('ANTIPATTERNS registry', () => {
|
|
test('has at least 5 entries', () => {
|
|
expect(ANTIPATTERNS.length).toBeGreaterThanOrEqual(5);
|
|
});
|
|
|
|
test('each entry has required fields', () => {
|
|
for (const ap of ANTIPATTERNS) {
|
|
expect(ap.id).toBeTypeOf('string');
|
|
expect(ap.name).toBeTypeOf('string');
|
|
expect(ap.description).toBeTypeOf('string');
|
|
}
|
|
});
|
|
});
|
|
|
|
// ---------------------------------------------------------------------------
|
|
// walkDir
|
|
// ---------------------------------------------------------------------------
|
|
|
|
describe('walkDir', () => {
|
|
test('finds scannable files', () => {
|
|
const files = walkDir(FIXTURES);
|
|
expect(files.length).toBeGreaterThanOrEqual(3);
|
|
expect(files.every(f => SCANNABLE_EXTENSIONS.has(path.extname(f)))).toBe(true);
|
|
});
|
|
|
|
test('returns empty for nonexistent dir', () => {
|
|
expect(walkDir('/nonexistent/path/12345')).toHaveLength(0);
|
|
});
|
|
});
|
|
|
|
// ---------------------------------------------------------------------------
|
|
// CLI integration
|
|
// ---------------------------------------------------------------------------
|
|
|
|
describe('CLI', () => {
|
|
function run(...args) {
|
|
const result = spawnSync('node', [SCRIPT, ...args], { encoding: 'utf-8', timeout: 15000 });
|
|
return { stdout: result.stdout || '', stderr: result.stderr || '', code: result.status };
|
|
}
|
|
|
|
test('--help exits 0', () => {
|
|
const { stdout, code } = run('--help');
|
|
expect(code).toBe(0);
|
|
expect(stdout).toContain('Usage:');
|
|
});
|
|
|
|
test('should-pass exits 0', () => {
|
|
const { code } = run(path.join(FIXTURES, 'should-pass.html'));
|
|
expect(code).toBe(0);
|
|
});
|
|
|
|
test('should-flag exits 2 with findings', () => {
|
|
const { code, stderr } = run(path.join(FIXTURES, 'should-flag.html'));
|
|
expect(code).toBe(2);
|
|
expect(stderr).toContain('side-tab');
|
|
});
|
|
|
|
test('--json outputs valid JSON', () => {
|
|
const { stderr, code } = run('--json', path.join(FIXTURES, 'should-flag.html'));
|
|
expect(code).toBe(2);
|
|
const parsed = JSON.parse(stderr.trim());
|
|
expect(parsed).toBeArray();
|
|
expect(parsed.length).toBeGreaterThan(0);
|
|
});
|
|
|
|
test('--json on clean file outputs empty array', () => {
|
|
const { stdout, code } = run('--json', path.join(FIXTURES, 'should-pass.html'));
|
|
expect(code).toBe(0);
|
|
expect(JSON.parse(stdout.trim())).toEqual([]);
|
|
});
|
|
|
|
test('--fast mode works', () => {
|
|
const { code } = run('--fast', path.join(FIXTURES, 'should-flag.html'));
|
|
expect(code).toBe(2);
|
|
});
|
|
|
|
test('linked stylesheet detected (jsdom default)', () => {
|
|
const { code, stderr } = run(path.join(FIXTURES, 'linked-stylesheet.html'));
|
|
expect(code).toBe(2);
|
|
expect(stderr).toContain('side-tab');
|
|
});
|
|
|
|
test('warns on nonexistent path', () => {
|
|
const { stderr } = run('/nonexistent/file/xyz.html');
|
|
expect(stderr).toContain('Warning');
|
|
});
|
|
});
|