Skip to content

Commit

Permalink
BaseStruct: No shadowing with extra properties (#436)
Browse files Browse the repository at this point in the history
This PR addresses an edge case not covered by the previous PR.
  • Loading branch information
danfuzz authored Nov 18, 2024
2 parents fe8b124 + 7350dc7 commit 7696ce5
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 4 deletions.
11 changes: 7 additions & 4 deletions src/structy/export/BaseStruct.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,10 @@ export class BaseStruct {
/**
* Checks to see if a property name is allowed. This is called by the base
* class for any encountered property which doesn't have a corresponding
* `_prop_*()` method. If it returns `true`, then the property-value pair is
* passed to {@link #_impl_extraProperty} for further processing. The default
* (base class) implementation always returns `false`.
* `_prop_*()` method and which doesn't already exist in the class (shadowing
* a pre-existing property isn't allowed). If it returns `true`, then the
* property-value pair is passed to {@link #_impl_extraProperty} for further
* processing. The default (base class) implementation always returns `false`.
*
* @param {string} name Property name.
* @returns {boolean} `true` if `name` is allowed on the instance, or `false`
Expand Down Expand Up @@ -147,7 +148,9 @@ export class BaseStruct {
const disallowed = [];

for (const name of leftovers) {
if (this._impl_allowExtraProperty(name)) {
if (Reflect.has(this, name)) {
throw new Error(`Extra property would shadow pre-existing property: ${name}`);
} else if (this._impl_allowExtraProperty(name)) {
const value = this._impl_extraProperty(name, rawObject[name]);
if (value === undefined) {
throw new Error(`Extra property checker \`_impl_extraProperty()\` did not return a value. Maybe missing a \`return\`?`);
Expand Down
9 changes: 9 additions & 0 deletions src/structy/tests/BaseStruct.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,10 @@ describe('using a subclass which allows extra properties', () => {
class SomeStruct extends BaseStruct {
// @defaultConstructor

get yesNo() {
return 'maybe';
}

_impl_allowExtraProperty(name) {
return name.startsWith('yes');
}
Expand Down Expand Up @@ -370,5 +374,10 @@ describe('using a subclass which allows extra properties', () => {
const arg = { yesUndefined: 123 };
expect(() => new SomeStruct(arg)).toThrow(/did not return/);
});

test('throws given an extra property which would shadow a pre-existing property', () => {
const arg = { yesNo: 12345 };
expect(() => new SomeStruct(arg)).toThrow(/would shadow/);
});
});
});

0 comments on commit 7696ce5

Please sign in to comment.