[Modifiers] Helper methods to use our custom generators by Nyholm · Pull Request #155 · FakerPHP/Faker

Nyholm · 2020-12-19T11:27:03Z

This will add real methods to use our custom generators. They are described in https://github.com/Nyholm/next-faker#modifiers

localheinz · 2020-12-20T11:57:57Z

@Nyholm

Probably best to rebase on top of main!

localheinz · 2020-12-20T12:00:57Z

src/Faker/Generator.php

+     *
+     * @return self The UniqueGenerator is a proxy
+     */
+    public function withUnique()


Suggested change

public function withUnique()

public function withUnique(): self

But this is technically not true. (And also why psalm is complaining).

The UniqueGenerator and other generators will implement the magic method __call(). They can be used and treated as they are a Generator but they are not really.

I thought we agreed to do away with magic?

Everywhere but the proxy generators. Im not sure how to implement the Modifiers without them proxying calls to Generator.

I would be super happy to get a suggestion.

Isn't the UniqueGenerator just an implementation of a generator, hence returning self would be correct, since it extends generator?

No not really. It is a proxy. It does not extend.

src/Faker/Generator.php

localheinz · 2020-12-20T12:02:54Z

src/Faker/Generator.php

+
+    public function __construct()
+    {
+        $this->uniqueGenerator = new UniqueGenerator($this);


Each Generator instance has a UniqueGenerator. So I can run:

$faker->name(); // Might be 'Alice' $faker->withUnique()->name(); // Might be 'Bob' $faker->name(); // Might be 'Alice' again $faker->withUnique()->name(); // Might be anything but 'Bob'

If we were about to create a new UniqueGenerator each time we called withUnique(), we could not guarantee it's uniqueness. Unless we used static variables, but then the uniqueness would be global and not specific to a instance of the Generator.

Instead of reimplementing the behaviour here, what do you think about moving methods from Faker\Provider\Base to Faker\Generator one by one, guided by tests?

Also see #233.

krsriq · 2020-12-22T15:50:02Z

Maybe I have overlooked something, but I think there's a bug here now when calling withMaybe() with an extension:

(new Generator())->withMaybe(0)->ext(\Faker\Extension\FileExtension::class)->mimeType()

throws Call to a member function mimeType() on null. This is because withMaybe(0) returns the DefaultGenerator, which overloads ext() to return null.

Nyholm · 2020-12-22T16:52:15Z

Excellent @krsriq, You are correct.

That is something I have not thought about. How do you suggest we fix that?

src/Faker/Generator.php

localheinz · 2020-12-22T17:20:54Z

src/Faker/Generator.php

+
+    public function __construct()
+    {
+        $this->uniqueGenerator = new UniqueGenerator($this);


Instead of reimplementing the behaviour here, what do you think about moving methods from Faker\Provider\Base to Faker\Generator one by one, guided by tests?

Also see #233.

krsriq · 2020-12-23T13:12:18Z

Excellent @krsriq, You are correct.

That is something I have not thought about. How do you suggest we fix that?

We have a related problem here in withUnique:

$generator->withUnique()->ext(\Faker\Extension\FileExtension::class)->mimeType()

may return duplicate results.

The issue here is that the UniqueGenerator stores the called method name and result in $uniques to check on a subsequent call if the result is unique. The method name in the case above, though, is ext, the result the extension class \Faker\Core\File.

Unfortunately I have not yet found a solution for these problems, I'll keep digging..

krsriq · 2021-01-07T09:43:51Z

@Nyholm okay, I found a way to do this for the UniqueGenerator, DefaultGenerator and ValidGenerator, though it's not pretty:

we basically want to be able to differentiate between a call to ext on those generators and the other method calls.
With this solution a call to ext temporarily stores the extension the method should be called on on the generator class, then returns the generator, so when finally the actual method is called on the generator, we can forward the call to the correct extension.

This is just a proof-of-(a-bad-)concept and could definitely be improved (or a cleaner solution be found), but it means that it's not impossible to use extensions with these generators.

pimjansen · 2021-02-18T14:41:00Z

What are the plans with this? Since this is crucial on the design right?

pimjansen · 2021-05-15T15:11:52Z

@Nyholm @krsriq did you already dive into this?

Nyholm · 2021-08-08T03:36:59Z

I think I've solved the extension stuff. Please have a look.

I took @krsriq idea and continued working on it.

pimjansen · 2021-12-18T15:18:55Z

I think the usecase is not resolved yet:

for ($i=0; $i<6;$i++) {
    echo $faker->unique()->ext(\Faker\Extension\BloodExtension::class)->bloodGroup().PHP_EOL;
}

A+
AB+
AB+
AB-
O+
O+

@Nyholm am i missing something here?

bram-pkg · 2022-01-18T10:49:13Z

Excellent @krsriq, You are correct.
That is something I have not thought about. How do you suggest we fix that?

We have a related problem here in withUnique:
$generator->withUnique()->ext(\Faker\Extension\FileExtension::class)->mimeType()
may return duplicate results.

The issue here is that the UniqueGenerator stores the called method name and result in $uniques to check on a subsequent call if the result is unique. The method name in the case above, though, is ext, the result the extension class \Faker\Core\File.

Unfortunately I have not yet found a solution for these problems, I'll keep digging..

This has been fixed now.

bram-pkg · 2022-01-18T10:49:32Z

The issue with ->unique()->ext() seems to have been fixed.

pimjansen · 2022-01-18T11:41:36Z

LGTM

What does the rest think? I think this is the last topic before splitting it to seperate language packs right?

…ing ->ext() method

Nyholm force-pushed the modifiers branch from c93487a to 92bbb58 Compare December 20, 2020 12:00

localheinz reviewed Dec 20, 2020

View reviewed changes

Nyholm mentioned this pull request Dec 20, 2020

Use symfony/phpunit-bridge #163

Merged

Nyholm force-pushed the modifiers branch from 6d0a20d to 16f4584 Compare December 21, 2020 19:35

localheinz mentioned this pull request Dec 22, 2020

Fix: Add test to assert unique() behaviour #233

Merged

1 task

localheinz reviewed Dec 22, 2020

View reviewed changes

pimjansen added the pinned label Jan 5, 2021

Nyholm force-pushed the modifiers branch 2 times, most recently from 4d79099 to 07f511a Compare August 8, 2021 02:30

Nyholm added 10 commits January 18, 2022 11:03

Helper functions to use our custom validators

42cd190

Update name

3870726

Make sure we support extensions

ee8c1d0

Updated some tests

ab98df5

deprecated DefaultGenerator

b43dfd7

Name it back to "optional"

b8a5b82

Fixed tests

1663cf5

fix

7396f90

cs

c22e450

cs

598bec1

bram-pkg force-pushed the modifiers branch from eefdfde to 598bec1 Compare January 18, 2022 10:03

bram-pkg force-pushed the modifiers branch from 012c00e to 88bb072 Compare January 18, 2022 10:48

pimjansen approved these changes Jan 18, 2022

View reviewed changes

Bram Ceulemans added 3 commits January 18, 2022 12:50

Fix UniqueGenerator not storing previously generated values when call…

619d572

…ing ->ext() method

Fix types in randomFloat

beaad6c

Generate baseline for proxy classes

b90deae

bram-pkg force-pushed the modifiers branch from 88bb072 to b90deae Compare January 18, 2022 13:14

Bram Ceulemans added 3 commits January 18, 2022 14:25

Add baseline for psalm

6bb85fe

Add regression test for UniqueGenerator

65111e1

Add ValidGeneratorTest.php

78f6025

bram-pkg mentioned this pull request Jan 19, 2022

Fix optional return type #422

Closed

bram-pkg approved these changes Jan 19, 2022

View reviewed changes

bram-pkg merged commit fa135b4 into FakerPHP:main Jan 19, 2022

	public function withUnique()
	public function withUnique(): self

Conversation

Nyholm commented Dec 19, 2020

Uh oh!

localheinz commented Dec 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bram-pkg Feb 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

krsriq commented Dec 22, 2020

Uh oh!

Nyholm commented Dec 22, 2020

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

krsriq commented Dec 23, 2020

Uh oh!

krsriq commented Jan 7, 2021

Uh oh!

pimjansen commented Feb 18, 2021

Uh oh!

pimjansen commented May 15, 2021

Uh oh!

Nyholm commented Aug 8, 2021

Uh oh!

pimjansen commented Dec 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bram-pkg commented Jan 18, 2022

Uh oh!

bram-pkg commented Jan 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pimjansen commented Jan 18, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

localheinz commented Dec 20, 2020 •

edited

Loading

bram-pkg Feb 4, 2021 •

edited

Loading

pimjansen commented Dec 18, 2021 •

edited

Loading

bram-pkg commented Jan 18, 2022 •

edited

Loading