Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow native PHP functions to not be used #817

Closed
jbrooksuk opened this issue May 17, 2023 · 6 comments · Fixed by pestphp/pest-plugin-arch#4
Closed

Allow native PHP functions to not be used #817

jbrooksuk opened this issue May 17, 2023 · 6 comments · Fixed by pestphp/pest-plugin-arch#4
Assignees
Labels

Comments

@jbrooksuk
Copy link

Given the following architecture test:

test('globals')
    ->expect(['sleep', 'usleep'])
    ->not->toBeUsed();

I'd expect it to fail in this file:

<?php

namespace App\Jobs;

class JobWithSleep
{
    public function handle()
    {
        sleep(10);
    }
}

But it doesn't, as Pest considers these native functions.

@fabio-ivona
Copy link
Collaborator

fabio-ivona commented May 18, 2023

@nunomaduro it seems that the issue is related to https://github.com/ta-tikoma/phpunit-architecture-test

as DependenciesAsserts::assertDoesNotDependOn() is called correctly from pest arch plugin by passing a layerA containing all project classes and a layerB containing a sleep FunctionDescription:

image

I'm further investigating why it doesn't detect the function usage, will update this thread soon

Update: it seems the issue is related to how ObjectDescription::uses is built, as it does include standard functions but skips built-in ones

Update: the issue depends on how Pest Arch Plugin builds the layer that is passed to DependenciesAsserts, see following comment for more info

@fabio-ivona
Copy link
Collaborator

@nunomaduro I found where the issue is generated:

in ObjectDescriptionFactory.php#L52 the layer uses is filtered by removing all items that are not user-defined (so, basically, it removes all built-in functions):

image

by removing self::isUserDefined($use) statement, built-in functions are detected

 if (! $isFromVendor) {
      $object->uses = new ObjectUses(array_values(
          array_filter(
              iterator_to_array($object->uses->getIterator()),
-              static fn (string $use): bool => self::isUserDefined($use) && ! self::isSameLayer($object, $use),
+              static fn (string $use): bool =>  ! self::isSameLayer($object, $use),
          )
      ));
  }

image

@fabio-ivona
Copy link
Collaborator

@nunomaduro before opening a PR to fix this, I'm wondering why you added self::isUserDefined($use)

any idea?

@devajmeireles
Copy link
Member

Hey, @jbrooksuk . Thanks for your feedback.

I'm checking with Pest team if we have plans to do that. I'll update this issue soon.

@fabio-ivona
Copy link
Collaborator

@devajmeireles I have a working implementation for this, will clean it up and open a PR soon

@fabio-ivona
Copy link
Collaborator

a fix is proposed in pestphp/pest-plugin-arch#4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants