Skip to content

Commit 4ef105f

Browse files
committed
feat(check): rule reactive-props-use-declare flags class-field foot-gun
A reactive property listed in `static properties = { … }` MUST be typed via `declare propName: Type` and have its default set inside `constructor()`. A plain class-field initializer (`propName = value` or `propName: Type = value`) compiles under modern class-field semantics to `Object.defineProperty(this, …)` *after* super(), which uses [[Define]] semantics and overwrites the reactive accessor the framework installed in `_initializeProperties`. The reactive contract is then silently broken — `this.propName = x` no longer goes through the setter, so no `requestUpdate`, no reflect, no `hasChanged`. The rule extracts each `class … extends WebComponent { … }` body (brace-counted, comment- and string-aware), reads the `static properties` keys, and flags any class-field initializer at the top level of the body (depth 0) whose name matches one of those keys. `this.x = …` inside methods is ignored (depth > 0). Non-reactive helper fields whose names are NOT in `static properties` (sockets, counters, refs) are ignored. Tests cover: bug detected on typed initializer, bug detected on untyped initializer, declare + constructor passes, non-reactive fields ignored, `this.x = …` inside methods ignored. 5 new tests.
1 parent 161d061 commit 4ef105f

2 files changed

Lines changed: 353 additions & 0 deletions

File tree

packages/server/src/check.js

Lines changed: 223 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,11 @@ export const RULES = [
7272
description:
7373
'Static tag = \'...\' in component files must contain a hyphen (HTML custom element spec).',
7474
},
75+
{
76+
name: 'reactive-props-use-declare',
77+
description:
78+
'Reactive properties listed in `static properties = { … }` must be typed with `declare propName: Type` (no value), and have their default set in `constructor()`. Plain class-field initializers (`prop = value` or `prop: Type = value`) compile to Object.defineProperty *after* super() under modern class-field semantics, clobbering the framework\'s reactive accessor and silently breaking re-renders.',
79+
},
7580
];
7681

7782
/** Set of all known rule names for fast lookup. */
@@ -203,6 +208,205 @@ function countExportedFunctions(content) {
203208
return seen.size;
204209
}
205210

211+
/**
212+
* Extract the body of every `class … extends WebComponent { … }` block.
213+
* Brace-counts to handle nested template literals, methods, and arrow
214+
* functions. String state is tracked so braces inside strings/templates
215+
* don't shift depth.
216+
*
217+
* @param {string} content
218+
* @returns {string[]}
219+
*/
220+
function extractWebComponentClassBodies(content) {
221+
const bodies = [];
222+
const re = /class\s+\w+\s+extends\s+WebComponent\s*\{/g;
223+
let m;
224+
while ((m = re.exec(content)) !== null) {
225+
const bodyStart = m.index + m[0].length;
226+
const end = matchClosingBrace(content, bodyStart);
227+
if (end !== -1) bodies.push(content.slice(bodyStart, end));
228+
}
229+
return bodies;
230+
}
231+
232+
/**
233+
* Walk forward from `start` (just after an opening `{`) and return the
234+
* index of the matching `}`. Tracks string/template-literal state so
235+
* `}` inside `'…'`, `"…"`, or backtick templates don't decrement depth.
236+
* Returns -1 if no balanced brace is found.
237+
*
238+
* @param {string} s
239+
* @param {number} start
240+
*/
241+
function matchClosingBrace(s, start) {
242+
let depth = 1;
243+
let i = start;
244+
let str = ''; // '', "'", '"', or '`'
245+
while (i < s.length) {
246+
const c = s[i];
247+
if (str) {
248+
if (c === '\\') { i += 2; continue; }
249+
if (c === str) str = '';
250+
else if (str === '`' && c === '$' && s[i + 1] === '{') {
251+
// template hole: count its closing `}` toward our brace depth.
252+
depth++;
253+
i += 2;
254+
continue;
255+
}
256+
i++;
257+
continue;
258+
}
259+
if (c === "'" || c === '"' || c === '`') { str = c; i++; continue; }
260+
if (c === '/' && s[i + 1] === '/') { // line comment
261+
while (i < s.length && s[i] !== '\n') i++;
262+
continue;
263+
}
264+
if (c === '/' && s[i + 1] === '*') { // block comment
265+
i += 2;
266+
while (i < s.length && !(s[i] === '*' && s[i + 1] === '/')) i++;
267+
i += 2;
268+
continue;
269+
}
270+
if (c === '{') depth++;
271+
else if (c === '}') { depth--; if (depth === 0) return i; }
272+
i++;
273+
}
274+
return -1;
275+
}
276+
277+
/**
278+
* Find every `<key>:` entry inside the first `static properties = { … }`
279+
* literal in `classBody`. Returns the bare property names — the keys
280+
* we'll then look up as class fields.
281+
*
282+
* @param {string} classBody
283+
* @returns {Set<string>}
284+
*/
285+
function extractStaticPropertyNames(classBody) {
286+
/** @type {Set<string>} */
287+
const names = new Set();
288+
const m = /static\s+properties\s*=\s*\{/.exec(classBody);
289+
if (!m) return names;
290+
const objStart = m.index + m[0].length;
291+
const objEnd = matchClosingBrace(classBody, objStart);
292+
if (objEnd === -1) return names;
293+
const obj = classBody.slice(objStart, objEnd);
294+
// Match keys at the top level of the object literal. A nested `{ … }`
295+
// (the per-property declaration) is skipped via brace counting.
296+
let i = 0;
297+
while (i < obj.length) {
298+
// Skip whitespace and commas.
299+
while (i < obj.length && /[\s,]/.test(obj[i])) i++;
300+
if (i >= obj.length) break;
301+
// Read the identifier or string-literal key.
302+
let key = '';
303+
if (obj[i] === '"' || obj[i] === "'") {
304+
const quote = obj[i++];
305+
while (i < obj.length && obj[i] !== quote) { key += obj[i++]; }
306+
i++; // closing quote
307+
} else {
308+
while (i < obj.length && /[A-Za-z0-9_$]/.test(obj[i])) key += obj[i++];
309+
}
310+
// Skip whitespace, then expect `:`.
311+
while (i < obj.length && /\s/.test(obj[i])) i++;
312+
if (obj[i] !== ':') break;
313+
i++;
314+
// Skip whitespace, then skip the value (either a `{ … }` literal or
315+
// a single token like `String`).
316+
while (i < obj.length && /\s/.test(obj[i])) i++;
317+
if (obj[i] === '{') {
318+
const valEnd = matchClosingBrace(obj, i + 1);
319+
if (valEnd === -1) break;
320+
i = valEnd + 1;
321+
} else {
322+
while (i < obj.length && obj[i] !== ',' && obj[i] !== '}') i++;
323+
}
324+
if (key) names.add(key);
325+
}
326+
return names;
327+
}
328+
329+
/**
330+
* Scan a class body for class-field initializers naming any of `props`.
331+
* "Class-field" means: at the top of the class body (brace depth 0
332+
* relative to the body), at the start of a line, NOT prefixed with
333+
* `declare`, `static`, or `this.`.
334+
*
335+
* Returns the offending property names. The caller maps these to
336+
* Violation objects.
337+
*
338+
* @param {string} classBody
339+
* @param {Set<string>} props
340+
* @returns {string[]}
341+
*/
342+
function findFieldInitializers(classBody, props) {
343+
/** @type {string[]} */
344+
const out = [];
345+
// Walk the body, tracking brace depth. At depth 0, look for
346+
// class-field-shaped lines.
347+
let depth = 0;
348+
let i = 0;
349+
let lineStart = 0;
350+
let str = '';
351+
while (i < classBody.length) {
352+
const c = classBody[i];
353+
if (str) {
354+
if (c === '\\') { i += 2; continue; }
355+
if (c === str) str = '';
356+
else if (str === '`' && c === '$' && classBody[i + 1] === '{') {
357+
depth++;
358+
i += 2;
359+
continue;
360+
}
361+
i++;
362+
continue;
363+
}
364+
if (c === '\n') {
365+
lineStart = i + 1;
366+
i++;
367+
continue;
368+
}
369+
if (c === '/' && classBody[i + 1] === '/') {
370+
while (i < classBody.length && classBody[i] !== '\n') i++;
371+
continue;
372+
}
373+
if (c === '/' && classBody[i + 1] === '*') {
374+
i += 2;
375+
while (i < classBody.length && !(classBody[i] === '*' && classBody[i + 1] === '/')) i++;
376+
i += 2;
377+
continue;
378+
}
379+
if (c === "'" || c === '"' || c === '`') { str = c; i++; continue; }
380+
if (c === '{') { depth++; i++; continue; }
381+
if (c === '}') { depth--; i++; continue; }
382+
// At class-body top level, examine candidate lines starting at lineStart.
383+
if (depth === 0 && i === lineStart || (depth === 0 && /\s/.test(c) && i === lineStart)) {
384+
// Take the rest of the line up to a newline.
385+
let j = lineStart;
386+
while (j < classBody.length && classBody[j] !== '\n') j++;
387+
const line = classBody.slice(lineStart, j);
388+
// Match: optional whitespace, optional `public/private/protected/readonly`,
389+
// an identifier, optional `: <type>`, then `=`.
390+
const fieldRe = /^\s*(?:(public|private|protected|readonly)\s+)?([A-Za-z_$][\w$]*)\s*(?::\s*[^=;]+)?\s*=\s*[^=>]/;
391+
const m = fieldRe.exec(line);
392+
if (m) {
393+
const name = m[2];
394+
// `declare`, `static`, and `this.` patterns shouldn't reach here
395+
// (declare/static start with their keyword, this.x has the dot in
396+
// the regex group), but guard against matching keywords as names:
397+
if (name !== 'declare' && name !== 'static' && props.has(name)) {
398+
out.push(name);
399+
}
400+
}
401+
// Advance past this line so we don't re-match.
402+
i = j;
403+
continue;
404+
}
405+
i++;
406+
}
407+
return out;
408+
}
409+
206410
/**
207411
* Scan a webjs app directory and report convention violations.
208412
*
@@ -326,6 +530,25 @@ export async function checkConventions(appDir, opts) {
326530
}
327531
}
328532

533+
// --- Rule: reactive-props-use-declare ---
534+
if (isRuleEnabled('reactive-props-use-declare', overrides)) {
535+
for (const { rel, content } of files) {
536+
if (!/class\s+\w+\s+extends\s+WebComponent/.test(content)) continue;
537+
for (const body of extractWebComponentClassBodies(content)) {
538+
const propNames = extractStaticPropertyNames(body);
539+
if (propNames.size === 0) continue;
540+
for (const bad of findFieldInitializers(body, propNames)) {
541+
violations.push({
542+
rule: 'reactive-props-use-declare',
543+
file: rel,
544+
message: `Reactive prop \`${bad}\` uses a class-field initializer; this clobbers the framework's reactive accessor under modern class-field semantics.`,
545+
fix: `Replace with \`declare ${bad}: <Type>;\` and set the default inside \`constructor()\` after \`super()\`.`,
546+
});
547+
}
548+
}
549+
}
550+
}
551+
329552
// --- Rule: no-server-imports-in-components ---
330553
if (isRuleEnabled('no-server-imports-in-components', overrides)) {
331554
for (const { abs, rel, content } of files) {

test/check.test.js

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,136 @@ customElements.define('native-comp', NativeComp);
112112
}
113113
});
114114

115+
test('reactive-props-use-declare: flags class-field initializer on reactive prop', async () => {
116+
const appDir = await makeTempApp();
117+
try {
118+
await mkdir(join(appDir, 'components'), { recursive: true });
119+
await writeFile(
120+
join(appDir, 'components', 'bad.ts'),
121+
`import { WebComponent } from '@webjskit/core';
122+
class BadProp extends WebComponent {
123+
static properties = { count: { type: Number } };
124+
count: number = 0;
125+
}
126+
BadProp.register('bad-prop');
127+
`,
128+
);
129+
const violations = await checkConventions(appDir);
130+
const v = violations.find((v) => v.rule === 'reactive-props-use-declare');
131+
assert.ok(v, 'expected reactive-props-use-declare violation');
132+
assert.ok(v.message.includes('count'));
133+
assert.ok(v.fix.includes('declare'));
134+
} finally {
135+
await rm(appDir, { recursive: true, force: true });
136+
}
137+
});
138+
139+
test('reactive-props-use-declare: also flags untyped initializer', async () => {
140+
const appDir = await makeTempApp();
141+
try {
142+
await mkdir(join(appDir, 'components'), { recursive: true });
143+
await writeFile(
144+
join(appDir, 'components', 'bad.ts'),
145+
`import { WebComponent } from '@webjskit/core';
146+
class BadProp extends WebComponent {
147+
static properties = { name: { type: String } };
148+
name = 'Anonymous';
149+
}
150+
BadProp.register('bad-prop');
151+
`,
152+
);
153+
const violations = await checkConventions(appDir);
154+
const v = violations.find((v) => v.rule === 'reactive-props-use-declare');
155+
assert.ok(v, 'expected violation for `name = "Anonymous"`');
156+
assert.ok(v.message.includes('name'));
157+
} finally {
158+
await rm(appDir, { recursive: true, force: true });
159+
}
160+
});
161+
162+
test('reactive-props-use-declare: passes when declare + constructor are used', async () => {
163+
const appDir = await makeTempApp();
164+
try {
165+
await mkdir(join(appDir, 'components'), { recursive: true });
166+
await writeFile(
167+
join(appDir, 'components', 'good.ts'),
168+
`import { WebComponent } from '@webjskit/core';
169+
class GoodProp extends WebComponent {
170+
static properties = { count: { type: Number } };
171+
declare count: number;
172+
173+
constructor() {
174+
super();
175+
this.count = 0;
176+
}
177+
}
178+
GoodProp.register('good-prop');
179+
`,
180+
);
181+
const violations = await checkConventions(appDir);
182+
const v = violations.find((v) => v.rule === 'reactive-props-use-declare');
183+
assert.equal(v, undefined, 'declare + constructor should be clean');
184+
} finally {
185+
await rm(appDir, { recursive: true, force: true });
186+
}
187+
});
188+
189+
test('reactive-props-use-declare: ignores non-reactive plain fields', async () => {
190+
// Fields whose names are NOT in `static properties` are free-form —
191+
// no reactive accessor exists, so a class-field initializer is fine.
192+
const appDir = await makeTempApp();
193+
try {
194+
await mkdir(join(appDir, 'components'), { recursive: true });
195+
await writeFile(
196+
join(appDir, 'components', 'mixed.ts'),
197+
`import { WebComponent } from '@webjskit/core';
198+
class Mixed extends WebComponent {
199+
static properties = { count: { type: Number } };
200+
declare count: number;
201+
_conn: WebSocket | null = null; // not reactive — fine
202+
_retries = 0; // not reactive — fine
203+
204+
constructor() { super(); this.count = 0; }
205+
}
206+
Mixed.register('mixed-prop');
207+
`,
208+
);
209+
const violations = await checkConventions(appDir);
210+
const v = violations.find((v) => v.rule === 'reactive-props-use-declare');
211+
assert.equal(v, undefined, 'non-reactive class fields should not trigger the rule');
212+
} finally {
213+
await rm(appDir, { recursive: true, force: true });
214+
}
215+
});
216+
217+
test('reactive-props-use-declare: does not trip on `this.x = …` inside methods', async () => {
218+
const appDir = await makeTempApp();
219+
try {
220+
await mkdir(join(appDir, 'components'), { recursive: true });
221+
await writeFile(
222+
join(appDir, 'components', 'methods.ts'),
223+
`import { WebComponent } from '@webjskit/core';
224+
class Methods extends WebComponent {
225+
static properties = { count: { type: Number } };
226+
declare count: number;
227+
228+
constructor() { super(); this.count = 0; }
229+
230+
increment() {
231+
this.count = this.count + 1;
232+
}
233+
}
234+
Methods.register('methods-prop');
235+
`,
236+
);
237+
const violations = await checkConventions(appDir);
238+
const v = violations.find((v) => v.rule === 'reactive-props-use-declare');
239+
assert.equal(v, undefined, 'this.x = … inside methods is correct');
240+
} finally {
241+
await rm(appDir, { recursive: true, force: true });
242+
}
243+
});
244+
115245
test('no violations for empty app', async () => {
116246
const appDir = await makeTempApp();
117247
try {

0 commit comments

Comments
 (0)