Repository: dubzzz/fast-check
Pull request #1329 (2021-01-07 19:23:39)

✨ Throw if non enumerable keys declared in requiredKeys of record
<!-- Why is this PR for? -->
<!-- Describe the reason why you opened this PR or give a link towards the associated issue. -->

## In a nutshell

✔️ New feature
❌ Fix an issue
❌ Documentation improvement
❌ Other: *please explain*

(✔️: yes, ❌: no)

## Potential impacts

None

github-actions[bot]: Give a try to https://github.com/dubzzz/fast-check/pull/1329/commits/ab150d9ac093d7173768481c0f18bb51142f689a with:

```bash
yarn add https://pkg.csb.dev/dubzzz/fast-check/commit/ab150d9a/fast-check
npm i https://pkg.csb.dev/dubzzz/fast-check/commit/ab150d9a/fast-check
```

⚠️ Package might not be accessible yet. Wait for CodeSandbox.

⚠️ By running one of these commands you will install the package defined by the head of the PR. But, tests are run against the result of the merge of the PR, not against the head of the PR*. *_If needed, you can install the package used for the tests by manually retrieving and installing the artifact bundle stored into GitHub Actions._

Useful links: [Codeclimate](https://codeclimate.com/github/dubzzz/fast-check/pull/1329), [Codecov](https://codecov.io/gh/dubzzz/fast-check/pull/1329), [CodeSandbox](https://ci.codesandbox.io/status/dubzzz/fast-check/pr/1329) and [GitHub Actions](https://github.com/dubzzz/fast-check/actions/runs/469775094).
codesandbox[bot]: This pull request is automatically built and testable in [CodeSandbox](https://codesandbox.io).

  To see build info of the built libraries, click [here](https://ci.codesandbox.io/status/dubzzz/fast-check/pr/1329/builds/89630) or the icon next to each commit SHA.

Latest deployment of this branch, based on commit ab150d9ac093d7173768481c0f18bb51142f689a:

|Sandbox| Source |
|--|--|
|[Vanilla](https://codesandbox.io/s/vanilla-zx4ds)| Configuration |

codecov[bot]: # [Codecov](https://codecov.io/gh/dubzzz/fast-check/pull/1329?src=pr&el=h1) Report
> Merging [#1329](https://codecov.io/gh/dubzzz/fast-check/pull/1329?src=pr&el=desc) (ab150d9) into [master](https://codecov.io/gh/dubzzz/fast-check/commit/16141b04e72dd6f6f34bbca7a5214f117379fe8f?el=desc) (16141b0) will **decrease** coverage by `9.18%`.
> The diff coverage is `75.00%`.

[![Impacted file tree graph](https://codecov.io/gh/dubzzz/fast-check/pull/1329/graphs/tree.svg?width=650&height=150&src=pr&token=fRi2fJN730)](https://codecov.io/gh/dubzzz/fast-check/pull/1329?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master    #1329      +/-   ##
==========================================
- Coverage   98.77%   89.59%   -9.19%     
==========================================
  Files          89       89              
  Lines        2937     2940       +3     
  Branches      547      548       +1     
==========================================
- Hits         2901     2634     -267     
- Misses         33      298     +265     
- Partials        3        8       +5     
```

| Flag | Coverage Δ | |
|---|---|---|
| e2e-tests | `89.59% <75.00%> (-0.06%)` | :arrow_down: |
| e2e-tests-10.x-Linux | `89.55% <75.00%> (-0.03%)` | :arrow_down: |
| e2e-tests-10.x-Windows | `?` | |
| e2e-tests-10.x-macOS | `?` | |
| e2e-tests-12.x-Linux | `89.59% <75.00%> (+0.01%)` | :arrow_up: |
| e2e-tests-12.x-Windows | `?` | |
| e2e-tests-12.x-macOS | `?` | |
| e2e-tests-14.x-Linux | `?` | |
| e2e-tests-14.x-Windows | `?` | |
| e2e-tests-14.x-macOS | `?` | |
| unit-tests | `?` | |
| unit-tests-10.x-Linux | `?` | |
| unit-tests-12.x-Linux | `?` | |
| unit-tests-14.x-Linux | `?` | |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/dubzzz/fast-check/pull/1329?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/check/arbitrary/RecordArbitrary.ts](https://codecov.io/gh/dubzzz/fast-check/pull/1329/diff?src=pr&el=tree#diff-c3JjL2NoZWNrL2FyYml0cmFyeS9SZWNvcmRBcmJpdHJhcnkudHM=) | `84.61% <75.00%> (-15.39%)` | :arrow_down: |
| [src/check/property/TimeoutProperty.ts](https://codecov.io/gh/dubzzz/fast-check/pull/1329/diff?src=pr&el=tree#diff-c3JjL2NoZWNrL3Byb3BlcnR5L1RpbWVvdXRQcm9wZXJ0eS50cw==) | `11.76% <0.00%> (-88.24%)` | :arrow_down: |
| [src/check/model/commands/ScheduledCommand.ts](https://codecov.io/gh/dubzzz/fast-check/pull/1329/diff?src=pr&el=tree#diff-c3JjL2NoZWNrL21vZGVsL2NvbW1hbmRzL1NjaGVkdWxlZENvbW1hbmQudHM=) | `7.14% <0.00%> (-82.15%)` | :arrow_down: |
| [src/utils/hash.ts](https://codecov.io/gh/dubzzz/fast-check/pull/1329/diff?src=pr&el=tree#diff-c3JjL3V0aWxzL2hhc2gudHM=) | `29.62% <0.00%> (-70.38%)` | :arrow_down: |
| [src/stream/LazyIterableIterator.ts](https://codecov.io/gh/dubzzz/fast-check/pull/1329/diff?src=pr&el=tree#diff-c3JjL3N0cmVhbS9MYXp5SXRlcmFibGVJdGVyYXRvci50cw==) | `63.63% <0.00%> (-36.37%)` | :arrow_down: |
| [src/check/property/AsyncProperty.generic.ts](https://codecov.io/gh/dubzzz/fast-check/pull/1329/diff?src=pr&el=tree#diff-c3JjL2NoZWNrL3Byb3BlcnR5L0FzeW5jUHJvcGVydHkuZ2VuZXJpYy50cw==) | `66.66% <0.00%> (-33.34%)` | :arrow_down: |
| [src/check/model/ModelRunner.ts](https://codecov.io/gh/dubzzz/fast-check/pull/1329/diff?src=pr&el=tree#diff-c3JjL2NoZWNrL21vZGVsL01vZGVsUnVubmVyLnRz) | `58.33% <0.00%> (-31.26%)` | :arrow_down: |
| [src/check/property/Property.generic.ts](https://codecov.io/gh/dubzzz/fast-check/pull/1329/diff?src=pr&el=tree#diff-c3JjL2NoZWNrL3Byb3BlcnR5L1Byb3BlcnR5LmdlbmVyaWMudHM=) | `70.58% <0.00%> (-29.42%)` | :arrow_down: |
| [.../check/runner/configuration/QualifiedParameters.ts](https://codecov.io/gh/dubzzz/fast-check/pull/1329/diff?src=pr&el=tree#diff-c3JjL2NoZWNrL3J1bm5lci9jb25maWd1cmF0aW9uL1F1YWxpZmllZFBhcmFtZXRlcnMudHM=) | `70.14% <0.00%> (-28.36%)` | :arrow_down: |
| [src/check/arbitrary/AsyncSchedulerArbitrary.ts](https://codecov.io/gh/dubzzz/fast-check/pull/1329/diff?src=pr&el=tree#diff-c3JjL2NoZWNrL2FyYml0cmFyeS9Bc3luY1NjaGVkdWxlckFyYml0cmFyeS50cw==) | `73.27% <0.00%> (-25.00%)` | :arrow_down: |
| ... and [30 more](https://codecov.io/gh/dubzzz/fast-check/pull/1329/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/dubzzz/fast-check/pull/1329?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/dubzzz/fast-check/pull/1329?src=pr&el=footer). Last update [16141b0...ab150d9](https://codecov.io/gh/dubzzz/fast-check/pull/1329?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/src/check/arbitrary/RecordArbitrary.ts b/src/check/arbitrary/RecordArbitrary.ts
index 636e61cc..3c945891 100644
--- a/src/check/arbitrary/RecordArbitrary.ts
+++ b/src/check/arbitrary/RecordArbitrary.ts
@@ -127,9 +127,13 @@ function record<T>(recordModel: { [K in keyof T]: Arbitrary<T[K]> }, constraints
   const requiredKeys = ('requiredKeys' in constraints ? constraints.requiredKeys : undefined) || [];
 
   for (let idx = 0; idx !== requiredKeys.length; ++idx) {
-    if (!(requiredKeys[idx] in recordModel)) {
+    const descriptor = Object.getOwnPropertyDescriptor(recordModel, requiredKeys[idx]);
+    if (descriptor === undefined) {
       throw new Error(`requiredKeys cannot reference keys that have not been defined in recordModel`);
     }
+    if (!descriptor.enumerable) {
+      throw new Error(`requiredKeys cannot reference keys that have are enumerable in recordModel`);
+    }
   }
 
   const keys = extractAllKeys(recordModel);


Test patch:
diff --git a/test/unit/check/arbitrary/RecordArbitrary.spec.ts b/test/unit/check/arbitrary/RecordArbitrary.spec.ts
index 7e739ec8..3542d40a 100644
--- a/test/unit/check/arbitrary/RecordArbitrary.spec.ts
+++ b/test/unit/check/arbitrary/RecordArbitrary.spec.ts
@@ -88,6 +88,25 @@ describe('RecordArbitrary', () => {
           }
         )
       ));
+    it('Should accept empty keys configurations with empty requiredKeys', () => {
+      const recordModel: Record<string | symbol, Arbitrary<any>> = {};
+      expect(() => record(recordModel, { requiredKeys: [] })).not.toThrowError();
+    });
+    it.each`
+      keyName
+      ${'constructor'}
+      ${'toString'}
+      ${'__proto__'}
+    `('Should reject configurations specifying non own properties ($keyName) as requiredKeys', ({ keyName }) => {
+      const recordModel: Record<string | symbol, Arbitrary<any>> = {};
+      expect(() => record(recordModel, { requiredKeys: [keyName] })).toThrowError();
+    });
+    it('Should reject configurations specifying non enumerable properties as requiredKeys', () => {
+      const keyName = 'k';
+      const recordModel: Record<string | symbol, Arbitrary<any>> = {};
+      Object.defineProperty(recordModel, keyName, { value: fc.boolean(), enumerable: false });
+      expect(() => record(recordModel, { requiredKeys: [keyName] })).toThrowError();
+    });
 
     type Meta = { key: any; valueStart: number; kept: boolean };
     const metaArbitrary = fc.set(