Skip to content

Commit 095602e

Browse files
mauriciogardinimauriciogardini
committed
Account for feedback
1 parent fb6f5e8 commit 095602e

File tree

4 files changed

+303
-157
lines changed

4 files changed

+303
-157
lines changed

‎packages/python/src/components/ClassDeclaration.tsx‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,9 @@ export function ClassDeclaration(props: ClassDeclarationProps) {
6060
"class",
6161
);
6262

63-
const childrenComputed = computed(() => childrenArray(() => props.children));
64-
const hasChildren = childrenComputed.value.some(Boolean);
63+
const hasChildren = computed(() =>
64+
childrenArray(() => props.children).some(Boolean),
65+
);
6566

6667
return (
6768
<Declaration symbol={sym}>
@@ -73,7 +74,7 @@ export function ClassDeclaration(props: ClassDeclarationProps) {
7374
{props.doc}
7475
<line />
7576
</Show>
76-
{hasChildren ? childrenComputed.value : "pass"}
77+
{hasChildren.value ? props.children : "pass"}
7778
</PythonBlock>
7879
</MemberScope>
7980
</Declaration>

‎packages/python/src/components/DataclassDeclaration.tsx‎

Lines changed: 132 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -3,102 +3,126 @@ import {
33
Show,
44
childrenArray,
55
computed,
6-
findKeyedChildren,
6+
namekey,
77
taggedComponent,
88
} from "@alloy-js/core";
99
import { dataclassesModule } from "../builtins/python.js";
10-
import { findMethodDeclaration } from "../utils.js";
10+
import { usePythonScope } from "../symbols/scopes.js";
1111
import { Atom } from "./Atom.jsx";
1212
import type { ClassDeclarationProps } from "./ClassDeclaration.js";
1313
import { ClassDeclaration } from "./ClassDeclaration.js";
1414
import { StatementList } from "./StatementList.js";
15+
import { VariableDeclaration } from "./VariableDeclaration.js";
1516

1617
/**
17-
* Validate the keyword arguments for the Python `@dataclass(...)` decorator.
18+
* Validate decorator-only rules that do not depend on class members.
1819
*/
19-
function validateDataclassDecoratorKwargs(
20-
children: any[],
21-
kwargs: DataclassDecoratorKwargs,
22-
) {
23-
if (kwargs.weakref_slot === true && kwargs.slots !== true) {
20+
function validateDataclassDecoratorArgs(kwargs: DataclassDecoratorKwargs) {
21+
if (kwargs.weakrefSlot === true && kwargs.slots !== true) {
2422
throw new Error(
2523
"weakref_slot=True requires slots=True in @dataclass decorator",
2624
);
2725
}
28-
2926
if (kwargs.order === true && kwargs.eq === false) {
3027
throw new Error("order=True requires eq=True in @dataclass decorator");
3128
}
29+
}
30+
31+
/**
32+
* Validate symbol-level conflicts. Must be called from within a member scope
33+
* (inside the class body) so member symbols are available.
34+
*/
35+
function validateDataclassMemberConflicts(kwargs: DataclassDecoratorKwargs) {
36+
const scope = usePythonScope();
37+
const owner: any = (scope as any).ownerSymbol;
38+
if (!owner) return;
39+
40+
const hasMemberNamed = (name: string): boolean => {
41+
for (const sym of owner.instanceMembers as Iterable<any>) {
42+
if (sym.originalName === name) return true;
43+
}
44+
for (const sym of owner.staticMembers as Iterable<any>) {
45+
if (sym.originalName === name) return true;
46+
}
47+
return false;
48+
};
3249

3350
if (kwargs.order === true) {
34-
const orderingMethods = ["__lt__", "__le__", "__gt__", "__ge__"];
35-
const conflict = findMethodDeclaration(children, orderingMethods);
36-
if (conflict) {
37-
throw new TypeError(
38-
`Cannot specify order=True when the class already defines ${conflict}()`,
39-
);
51+
for (const m of ["__lt__", "__le__", "__gt__", "__ge__"]) {
52+
if (hasMemberNamed(m)) {
53+
throw new TypeError(
54+
`Cannot specify order=True when the class already defines ${m}()`,
55+
);
56+
}
4057
}
4158
}
4259

43-
if (kwargs.unsafe_hash === true) {
44-
const conflict = findMethodDeclaration(children, ["__hash__"]);
45-
if (conflict) {
46-
throw new TypeError(
47-
`Cannot specify unsafe_hash=True when the class already defines ${conflict}()`,
48-
);
49-
}
60+
if (kwargs.unsafeHash === true && hasMemberNamed("__hash__")) {
61+
throw new TypeError(
62+
"Cannot specify unsafe_hash=True when the class already defines __hash__()",
63+
);
5064
}
5165

5266
if (kwargs.frozen === true) {
53-
const conflict = findMethodDeclaration(children, [
54-
"__setattr__",
55-
"__delattr__",
56-
]);
57-
if (conflict) {
67+
if (hasMemberNamed("__setattr__")) {
5868
throw new TypeError(
59-
`Cannot specify frozen=True when the class already defines ${conflict}()`,
69+
"Cannot specify frozen=True when the class already defines __setattr__()",
6070
);
6171
}
62-
}
63-
64-
if (kwargs.slots === true) {
65-
const conflict = findMethodDeclaration(children, ["__slots__"]);
66-
if (conflict) {
72+
if (hasMemberNamed("__delattr__")) {
6773
throw new TypeError(
68-
`Cannot specify slots=True when the class already defines ${conflict}()`,
74+
"Cannot specify frozen=True when the class already defines __delattr__()",
6975
);
7076
}
7177
}
78+
79+
if (kwargs.slots === true && hasMemberNamed("__slots__")) {
80+
throw new TypeError(
81+
"Cannot specify slots=True when the class already defines __slots__()",
82+
);
83+
}
84+
85+
// Enforce at most one KW_ONLY sentinel as a symbol
86+
let kwOnlyCount = 0;
87+
for (const sym of owner.instanceMembers as Iterable<any>) {
88+
if (sym.originalName === "_") kwOnlyCount++;
89+
}
90+
if (kwOnlyCount > 1) {
91+
throw new Error("Only one KW_ONLY sentinel is allowed per dataclass body");
92+
}
7293
}
7394

7495
/**
7596
* Allowed keyword arguments for the Python `@dataclass(...)` decorator.
76-
* Showcases arguments valid for Python 3.11+.
97+
* Single source of truth: runtime keys and compile-time type are derived here.
7798
*/
78-
export interface DataclassDecoratorKwargs {
79-
init?: boolean;
80-
repr?: boolean;
81-
eq?: boolean;
82-
order?: boolean;
83-
unsafe_hash?: boolean;
84-
frozen?: boolean;
85-
match_args?: boolean;
86-
kw_only?: boolean;
87-
slots?: boolean;
88-
weakref_slot?: boolean;
89-
}
99+
export const dataclassDecoratorKeys = [
100+
"init",
101+
"repr",
102+
"eq",
103+
"order",
104+
"unsafeHash",
105+
"frozen",
106+
"matchArgs",
107+
"kwOnly",
108+
"slots",
109+
"weakrefSlot",
110+
] as const;
111+
export type DataclassDecoratorKey = (typeof dataclassDecoratorKeys)[number];
112+
export type DataclassDecoratorKwargs = Partial<
113+
Record<DataclassDecoratorKey, boolean>
114+
>;
90115

91-
export interface DataclassDeclarationProps extends ClassDeclarationProps {
92-
/** Keyword arguments to pass to `@dataclass(...)` (only valid dataclass params). */
93-
decoratorKwargs?: DataclassDecoratorKwargs;
94-
}
116+
export interface DataclassDeclarationProps
117+
extends ClassDeclarationProps,
118+
DataclassDecoratorKwargs {}
95119

96120
/**
97121
* Renders a Python dataclass. Uses ClassDeclaration component internally.
98122
*
99123
* Example:
100124
* ```tsx
101-
* <py.DataclassDeclaration name="User" decoratorKwargs={{ kw_only: true }}>
125+
* <py.DataclassDeclaration name="User" kwOnly>
102126
* <py.VariableDeclaration instanceVariable omitNone name="id" type="int" />
103127
* <py.DataclassKWOnly />
104128
* <py.VariableDeclaration
@@ -122,50 +146,74 @@ export interface DataclassDeclarationProps extends ClassDeclarationProps {
122146
* ```
123147
*/
124148
export function DataclassDeclaration(props: DataclassDeclarationProps) {
125-
const kwargs = props.decoratorKwargs as DataclassDecoratorKwargs | undefined;
126-
const hasDecoratorArgs =
127-
kwargs !== undefined && Object.keys(kwargs).length > 0;
128-
const childrenComputed = computed(() => childrenArray(() => props.children));
129-
const hasBodyChildren = childrenComputed.value.some(Boolean);
130-
const children = childrenComputed.value;
131-
132-
if (props.decoratorKwargs) {
133-
validateDataclassDecoratorKwargs(children, props.decoratorKwargs);
149+
const decoratorKeys: (keyof DataclassDecoratorKwargs)[] = [
150+
...dataclassDecoratorKeys,
151+
];
152+
const validKeySet = new Set<string>(decoratorKeys as unknown as string[]);
153+
// Collect flags from props in the order they appear (preserves emission order)
154+
const orderedKwargs: Array<[keyof DataclassDecoratorKwargs, any]> = [];
155+
for (const key of Object.keys(props)) {
156+
// Only include known flags; skip undefined values
157+
if (validKeySet.has(key)) {
158+
const value = (props as any)[key];
159+
if (value !== undefined)
160+
orderedKwargs.push([key as keyof DataclassDecoratorKwargs, value]);
161+
}
134162
}
163+
// Materialize ordered entries into an object for validation/rendering
164+
const kwargs = orderedKwargs.reduce((acc, [k, v]) => {
165+
(acc as any)[k] = v;
166+
return acc;
167+
}, {} as DataclassDecoratorKwargs);
168+
const hasDecoratorArgs = orderedKwargs.length > 0;
169+
const toSnakeCase = (s: string): string =>
170+
s
171+
.replace(/([a-z\d])([A-Z])/g, "$1_$2")
172+
.replace(/([A-Z]+)([A-Z][a-z])/g, "$1_$2")
173+
.toLowerCase();
174+
const decoratorEntries = orderedKwargs.map(([k, v]) => {
175+
const pyKey = toSnakeCase(k as unknown as string);
176+
return [pyKey, v] as const;
177+
});
178+
const hasBodyChildren = computed(() =>
179+
childrenArray(() => props.children).some(Boolean),
180+
);
135181

136-
// Validate at most one KW_ONLY sentinel in children
137-
if (hasBodyChildren) {
138-
if (findKeyedChildren(children, DataclassKWOnly.tag).length > 1) {
139-
throw new Error(
140-
"Only one KW_ONLY sentinel is allowed per dataclass body",
141-
);
142-
}
182+
if (hasDecoratorArgs) {
183+
validateDataclassDecoratorArgs(kwargs);
184+
}
185+
186+
function RunSymbolValidation() {
187+
validateDataclassMemberConflicts(kwargs as DataclassDecoratorKwargs);
188+
return null;
143189
}
144190

191+
const classBody =
192+
hasBodyChildren.value ?
193+
<>
194+
<StatementList>{props.children}</StatementList>
195+
<RunSymbolValidation />
196+
</>
197+
: undefined;
198+
145199
return (
146200
<>
147201
{"@"}
148202
{dataclassesModule["."].dataclass}
149203
<Show when={hasDecoratorArgs}>
150-
{"("}
151-
<For
152-
each={Object.keys(kwargs ?? {}).map((k) => [k, (kwargs as any)[k]])}
153-
comma
154-
space
155-
>
204+
(
205+
<For each={decoratorEntries} comma space>
156206
{([k, v]) => (
157207
<>
158208
{k}=<Atom jsValue={v} />
159209
</>
160210
)}
161211
</For>
162-
{")"}
212+
)
163213
</Show>
164214
<hbr />
165215
<ClassDeclaration name={props.name} bases={props.bases} doc={props.doc}>
166-
{hasBodyChildren ?
167-
<StatementList>{props.children}</StatementList>
168-
: undefined}
216+
{classBody}
169217
</ClassDeclaration>
170218
</>
171219
);
@@ -182,7 +230,12 @@ export const DataclassKWOnly = taggedComponent(
182230
function DataclassKWOnly() {
183231
return (
184232
<>
185-
{"_"}: {dataclassesModule["."].KW_ONLY}
233+
<VariableDeclaration
234+
instanceVariable
235+
name={namekey("_", { ignoreNamePolicy: true })}
236+
type={dataclassesModule["."].KW_ONLY}
237+
omitNone
238+
/>
186239
</>
187240
);
188241
},

‎packages/python/src/components/UnionTypeExpression.tsx‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,23 @@
1-
import { Children, For, childrenArray, computed } from "@alloy-js/core";
1+
import { Children, List } from "@alloy-js/core";
22

33
export interface UnionTypeExpressionProps {
44
children: Children;
55
}
66

77
export function UnionTypeExpression(props: UnionTypeExpressionProps) {
8-
const items = computed(() => childrenArray(() => props.children));
98
return (
109
<group>
1110
<ifBreak>(</ifBreak>
1211
<indent>
1312
<sbr />
14-
<For
15-
each={items.value}
13+
<List
14+
children={props.children}
1615
joiner={
1716
<>
1817
<br />|{" "}
1918
</>
2019
}
21-
>
22-
{(child) => child}
23-
</For>
20+
/>
2421
</indent>
2522
<sbr />
2623
<ifBreak>)</ifBreak>

0 commit comments

Comments
 (0)