Add missing electrical role flags to pinAttributes - #839
Conversation
| test.each([...electricalTypes])( | ||
| "chip preserves electricalType %s", | ||
| (electricalType) => { | ||
| const attributes: PinAttributeMap = { electricalType } | ||
| const parsed = chipProps.parse({ | ||
| name: "U1", | ||
| pinAttributes: { pin1: attributes }, | ||
| }) | ||
| expect(parsed.pinAttributes?.pin1).toEqual(attributes) | ||
| }, | ||
| ) | ||
|
|
||
| test("electricalType is optional and does not change existing attributes", () => { | ||
| const pinAttributes: Record<string, PinAttributeMap> = { | ||
| pin1: {}, | ||
| pin2: { requiresPower: true, requiresVoltage: "3.3V" }, | ||
| pin3: { electricalType: "power_output", providesPower: true }, | ||
| pin4: { electricalType: "free", doNotConnect: false }, | ||
| pin5: { electricalType: "unspecified" }, | ||
| pin6: { canUseOpenDrain: true, isUsingOpenDrain: true }, | ||
| pin7: { doNotConnect: true }, | ||
| // Classification is independent metadata, not a cross-field ERC validator. | ||
| pin8: { electricalType: "input", providesPower: true }, | ||
| } | ||
| const parsed = chipProps.parse({ name: "U1", pinAttributes }) | ||
| expect(parsed.pinAttributes).toEqual(pinAttributes) | ||
| expect(parsed.pinAttributes?.pin1).not.toHaveProperty("electricalType") | ||
| expect(chipProps.parse({ name: "U1" }).pinAttributes).toBeUndefined() | ||
| }) | ||
|
|
||
| test.each(["invalid", "tri-state", "powerInput", "open_drain", "", null, 1])( | ||
| "rejects invalid electricalType %s", | ||
| (electricalType) => { | ||
| const result = chipProps.safeParse({ | ||
| name: "U1", | ||
| pinAttributes: { pin1: { electricalType } }, | ||
| }) | ||
| expect(result.success).toBe(false) | ||
| if (!result.success) { | ||
| expect(result.error.issues[0]?.path).toEqual([ | ||
| "pinAttributes", | ||
| "pin1", | ||
| "electricalType", | ||
| ]) | ||
| } | ||
| }, | ||
| ) | ||
|
|
||
| test("electricalType rejects invalid values at compile time", () => { | ||
| // @ts-expect-error Only the documented electrical pin types are accepted. | ||
| const attributes: PinAttributeMap = { electricalType: "invalid" } | ||
| void attributes | ||
| }) |
There was a problem hiding this comment.
This file contains more than one test(...) call (lines 18, 30, 48, and 66), which violates the rule that a *.test.ts file may have AT MOST one test(...). Please split this into multiple numbered files, e.g. pinAttributes-electrical-type1.test.ts, pinAttributes-electrical-type2.test.ts, pinAttributes-electrical-type3.test.ts, and pinAttributes-electrical-type4.test.ts (or a similarly numbered scheme consistent with the project).
Spotted by Graphite (based on custom rule: Custom rule)
Is this helpful? React 👍 or 👎 to let us know.
| import { expect, test } from "bun:test" | ||
| import { chipProps, type PinAttributeMap } from "lib" | ||
|
|
||
| const electricalFlags = [ | ||
| "isInput", | ||
| "isOutput", | ||
| "isPassive", | ||
| "isFree", | ||
| "isUnspecified", | ||
| "canUseTriState", | ||
| "isUsingTriState", | ||
| "canUseOpenCollector", | ||
| "isUsingOpenCollector", | ||
| "canUseOpenEmitter", | ||
| "isUsingOpenEmitter", | ||
| ] as const satisfies readonly (keyof PinAttributeMap)[] | ||
|
|
||
| test.each([...electricalFlags])( | ||
| "chip preserves true and false for %s", | ||
| (flag) => { | ||
| for (const value of [true, false]) { | ||
| const attributes: PinAttributeMap = { [flag]: value } | ||
| expect( | ||
| chipProps.parse({ name: "U1", pinAttributes: { pin1: attributes } }) | ||
| .pinAttributes?.pin1, | ||
| ).toEqual(attributes) | ||
| } | ||
| }, | ||
| ) | ||
|
|
||
| test.each([...electricalFlags])("rejects non-boolean values for %s", (flag) => { | ||
| for (const value of ["true", 1, null]) { | ||
| const result = chipProps.safeParse({ | ||
| name: "U1", | ||
| pinAttributes: { pin1: { [flag]: value } }, | ||
| }) | ||
| expect(result.success).toBe(false) | ||
| if (!result.success) { | ||
| expect(result.error.issues[0]?.path).toEqual([ | ||
| "pinAttributes", | ||
| "pin1", | ||
| flag, | ||
| ]) | ||
| } | ||
| } | ||
| }) | ||
|
|
||
| test("electrical flags compose with existing attributes without inferred defaults", () => { | ||
| const pinAttributes: Record<string, PinAttributeMap> = { | ||
| pin1: {}, | ||
| pin2: { isInput: true, isOutput: true, isGpio: true }, | ||
| pin3: { requiresPower: true, requiresVoltage: "3.3V" }, | ||
| pin4: { providesPower: true }, | ||
| pin5: { isOutput: true, canUseTriState: true, isUsingTriState: false }, | ||
| pin6: { isFree: true, doNotConnect: false }, | ||
| pin7: { isUnspecified: true }, | ||
| pin8: { isPassive: true }, | ||
| pin9: { | ||
| isOutput: true, | ||
| canUseOpenCollector: true, | ||
| isUsingOpenCollector: true, | ||
| }, | ||
| pin10: { | ||
| isOutput: true, | ||
| canUseOpenEmitter: true, | ||
| isUsingOpenEmitter: true, | ||
| }, | ||
| pin11: { canUseOpenDrain: true, isUsingOpenDrain: true }, | ||
| pin12: { doNotConnect: true }, | ||
| } | ||
| expect(chipProps.parse({ name: "U1", pinAttributes }).pinAttributes).toEqual( | ||
| pinAttributes, | ||
| ) | ||
| expect(chipProps.parse({ name: "U1" }).pinAttributes).toBeUndefined() | ||
| }) | ||
|
|
||
| test("electrical flags are boolean at compile time", () => { | ||
| // @ts-expect-error Electrical flags accept booleans only. | ||
| const attributes: PinAttributeMap = { isInput: "true" } | ||
| void attributes | ||
| }) |
There was a problem hiding this comment.
This file contains more than one test(...) call. The style guide rule states that a *.test.ts file may have AT MOST one test(...). This file has four test invocations: two test.each(...) calls (lines 18 and 31) and two plain test(...) calls (lines 48 and 77). These should be split into multiple numbered files, e.g. pinAttributes-electrical-flags1.test.ts, pinAttributes-electrical-flags2.test.ts, etc.
| import { expect, test } from "bun:test" | |
| import { chipProps, type PinAttributeMap } from "lib" | |
| const electricalFlags = [ | |
| "isInput", | |
| "isOutput", | |
| "isPassive", | |
| "isFree", | |
| "isUnspecified", | |
| "canUseTriState", | |
| "isUsingTriState", | |
| "canUseOpenCollector", | |
| "isUsingOpenCollector", | |
| "canUseOpenEmitter", | |
| "isUsingOpenEmitter", | |
| ] as const satisfies readonly (keyof PinAttributeMap)[] | |
| test.each([...electricalFlags])( | |
| "chip preserves true and false for %s", | |
| (flag) => { | |
| for (const value of [true, false]) { | |
| const attributes: PinAttributeMap = { [flag]: value } | |
| expect( | |
| chipProps.parse({ name: "U1", pinAttributes: { pin1: attributes } }) | |
| .pinAttributes?.pin1, | |
| ).toEqual(attributes) | |
| } | |
| }, | |
| ) | |
| test.each([...electricalFlags])("rejects non-boolean values for %s", (flag) => { | |
| for (const value of ["true", 1, null]) { | |
| const result = chipProps.safeParse({ | |
| name: "U1", | |
| pinAttributes: { pin1: { [flag]: value } }, | |
| }) | |
| expect(result.success).toBe(false) | |
| if (!result.success) { | |
| expect(result.error.issues[0]?.path).toEqual([ | |
| "pinAttributes", | |
| "pin1", | |
| flag, | |
| ]) | |
| } | |
| } | |
| }) | |
| test("electrical flags compose with existing attributes without inferred defaults", () => { | |
| const pinAttributes: Record<string, PinAttributeMap> = { | |
| pin1: {}, | |
| pin2: { isInput: true, isOutput: true, isGpio: true }, | |
| pin3: { requiresPower: true, requiresVoltage: "3.3V" }, | |
| pin4: { providesPower: true }, | |
| pin5: { isOutput: true, canUseTriState: true, isUsingTriState: false }, | |
| pin6: { isFree: true, doNotConnect: false }, | |
| pin7: { isUnspecified: true }, | |
| pin8: { isPassive: true }, | |
| pin9: { | |
| isOutput: true, | |
| canUseOpenCollector: true, | |
| isUsingOpenCollector: true, | |
| }, | |
| pin10: { | |
| isOutput: true, | |
| canUseOpenEmitter: true, | |
| isUsingOpenEmitter: true, | |
| }, | |
| pin11: { canUseOpenDrain: true, isUsingOpenDrain: true }, | |
| pin12: { doNotConnect: true }, | |
| } | |
| expect(chipProps.parse({ name: "U1", pinAttributes }).pinAttributes).toEqual( | |
| pinAttributes, | |
| ) | |
| expect(chipProps.parse({ name: "U1" }).pinAttributes).toBeUndefined() | |
| }) | |
| test("electrical flags are boolean at compile time", () => { | |
| // @ts-expect-error Electrical flags accept booleans only. | |
| const attributes: PinAttributeMap = { isInput: "true" } | |
| void attributes | |
| }) | |
| import { expect, test } from "bun:test" | |
| import { chipProps, type PinAttributeMap } from "lib" | |
| const electricalFlags = [ | |
| "isInput", | |
| "isOutput", | |
| "isPassive", | |
| "isFree", | |
| "isUnspecified", | |
| "canUseTriState", | |
| "isUsingTriState", | |
| "canUseOpenCollector", | |
| "isUsingOpenCollector", | |
| "canUseOpenEmitter", | |
| "isUsingOpenEmitter", | |
| ] as const satisfies readonly (keyof PinAttributeMap)[] | |
| test.each([...electricalFlags])( | |
| "chip preserves true and false for %s", | |
| (flag) => { | |
| for (const value of [true, false]) { | |
| const attributes: PinAttributeMap = { [flag]: value } | |
| expect( | |
| chipProps.parse({ name: "U1", pinAttributes: { pin1: attributes } }) | |
| .pinAttributes?.pin1, | |
| ).toEqual(attributes) | |
| } | |
| }, | |
| ) | |
Spotted by Graphite (based on custom rule: Custom rule)
Is this helpful? React 👍 or 👎 to let us know.
| @@ -0,0 +1,78 @@ | |||
| import { expect, test } from "bun:test" | |||
There was a problem hiding this comment.
The file name pinAttributes-electrical-flags.test.ts mixes camelCase and kebab-case (the pinAttributes prefix is camelCase while the rest uses kebab-case). File names should be consistent with the project convention — typically fully kebab-case. It should be renamed to something like pin-attributes-electrical-flags.test.ts (and then split into numbered files per the one-test-per-file rule).
Spotted by Graphite (based on custom rule: Custom rule)
Is this helpful? React 👍 or 👎 to let us know.
| recommendedDecouplingCapacitorCapacitance?: string | number | ||
| isGpio?: boolean | ||
| } | ||
| /** Whether the pin is configured as an open-emitter output. */ |
There was a problem hiding this comment.
Orphaned JSDoc comment is placed before the schema declaration instead of being attached to a field. This comment /** Whether the pin is configured as an open-emitter output. */ appears to belong to the isUsingOpenEmitter field but is separated from it. It should either be removed (since documentation already exists in PROPS_OVERVIEW.md) or moved to the appropriate field location:
export const pinAttributeMap = z.object({
// ... other fields ...
/** Whether the pin is configured as an open-emitter output. */
isUsingOpenEmitter: z.boolean().optional(),
// ...
})Currently, this misplaced comment may cause documentation generators to associate it with the entire pinAttributeMap object, which is incorrect.
Spotted by Graphite
Is this helpful? React 👍 or 👎 to let us know.
|
Thank you for your contribution! 🎉 PR Rating: ⭐⭐⭐ Track your contributions and see the leaderboard at: tscircuit Contribution Tracker |
Extend pinAttributes with missing electrical roles using the existing optional boolean pattern.
isInput,isOutput,isBidirectional, andisPassive.canUseTriState/isUsingTriState,canUseOpenCollector/isUsingOpenCollector, andcanUseOpenEmitter/isUsingOpenEmitter, following the existing drive-mode pattern.requiresPowerandprovidesPowerfor power input/output.New fields preserve explicit true/false values and remain unset when omitted. No enum, aliases, inferred values, or cross-field validation are added. Existing attributes require no migration. Circuit JSON propagation and ERC conflict checking need downstream implementation.
Validation: 508 tests pass, including new flag parsing, rejection of non-booleans, isBidirectional support, existing-attribute compatibility, and compile-time checking. Type checking, formatting, package build, and all four required generation scripts pass.