Skip to content

Commit 3888190

Browse files
committed
fix: require JSX return or hook usage for component detection (audit #1)
PascalCase functions in .tsx files were unconditionally indexed as components. Now maybeAddComponent checks for JSX elements/fragments in the function body or hook calls before registering — eliminates false positives like FormatCurrency(), ValidateEmail(), etc. Added negative test cases for the heuristic; updated the existing positive test to use actual JSX return.
1 parent 0db7658 commit 3888190

2 files changed

Lines changed: 48 additions & 1 deletion

File tree

src/parser.test.ts

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,46 @@ describe("extractFileData", () => {
3535
});
3636

3737
it("treats .tsx as JSX for PascalCase function components", () => {
38-
const src = `export function Button() { return null; }\n`;
38+
const src = `export function Button() { return <button>click</button>; }\n`;
3939
const d = extractFileData("/proj/x.tsx", src, "x.tsx");
4040
expect(d.components.some((c) => c.name === "Button")).toBe(true);
4141
});
4242
});
43+
44+
describe("component detection heuristic", () => {
45+
it("detects components that return JSX", () => {
46+
const src = `export function Card() { return <div>card</div>; }\n`;
47+
const d = extractFileData("/proj/x.tsx", src, "x.tsx");
48+
expect(d.components.some((c) => c.name === "Card")).toBe(true);
49+
});
50+
51+
it("detects arrow components that return JSX", () => {
52+
const src = `export const Card = () => <div>card</div>;\n`;
53+
const d = extractFileData("/proj/x.tsx", src, "x.tsx");
54+
expect(d.components.some((c) => c.name === "Card")).toBe(true);
55+
});
56+
57+
it("detects components that use hooks", () => {
58+
const src = `export function Timer() { useEffect(() => {}); return null; }\n`;
59+
const d = extractFileData("/proj/x.tsx", src, "x.tsx");
60+
expect(d.components.some((c) => c.name === "Timer")).toBe(true);
61+
});
62+
63+
it("rejects PascalCase functions without JSX or hooks", () => {
64+
const src = [
65+
`export function FormatCurrency(n: number): string { return "$"+n; }`,
66+
`export function ValidateEmail(e: string): boolean { return e.includes("@"); }`,
67+
].join("\n");
68+
const d = extractFileData("/proj/x.tsx", src, "x.tsx");
69+
expect(d.components).toHaveLength(0);
70+
});
71+
72+
it("rejects PascalCase functions that return null without hooks", () => {
73+
const src = `export function EmptyPlaceholder() { return null; }\n`;
74+
const d = extractFileData("/proj/x.tsx", src, "x.tsx");
75+
expect(d.components.some((c) => c.name === "EmptyPlaceholder")).toBe(
76+
false,
77+
);
78+
});
79+
});
4380
});

src/parser.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ export function extractFileData(
103103
}
104104

105105
const hookCalls = new Map<string, Set<string>>(); // function scope name -> hook names
106+
const jsxScopes = new Set<string>(); // function scopes that contain JSX
106107
let currentFunctionScope: string | null = null;
107108

108109
const visitor = new Visitor({
@@ -255,6 +256,13 @@ export function extractFileData(
255256
hookCalls.get(currentFunctionScope)?.add(callee.name);
256257
}
257258
},
259+
260+
JSXElement() {
261+
if (currentFunctionScope) jsxScopes.add(currentFunctionScope);
262+
},
263+
JSXFragment() {
264+
if (currentFunctionScope) jsxScopes.add(currentFunctionScope);
265+
},
258266
});
259267

260268
visitor.visit(result.program);
@@ -264,6 +272,8 @@ export function extractFileData(
264272
function maybeAddComponent(name: string, node: any, _isArrow: boolean) {
265273
if (!isTsx || !/^[A-Z]/.test(name)) return;
266274
const hooks = hookCalls.get(name);
275+
const hasJsx = jsxScopes.has(name);
276+
if (!hasJsx && !(hooks && hooks.size > 0)) return;
267277
const isDefault = defaultExportedNames.has(name);
268278

269279
let propsType: string | null = null;

0 commit comments

Comments
 (0)