Detect when a class extends a class that has been flagged as @internal

[brambaud]

See #183

In this PR, a shared namespace for Drupal starts with Drupal\project_name.

It means:

• a class inside Drupal\Core can extends an internal class from Drupal\Core
• a class inside Drupal\my_module can extends an internal class from Drupal\my_module
• a class inside Drupal\my_module cannot extends an internal class from Drupal\Core or \Drupal\another_module
• a class inside Drupal\Component cannot extends an internal class from Drupal\Core or \Drupal\another_module
• a class inside Drupal\Core cannot extends an internal class from Drupal\Component or \Drupal\another_module
• etc..

For the part detecting calls to internal methods, I think it can be handled in another PR 🤔

[mglaman]

This is amazing. I want to come back and review it more. Because we could move a lot of this logic into a shared service that can help handle #249.

Like FunctionCallParametersCheck in PHPStan. That namespace service could return errors as needed. To start it'll return errors on the usage of internal across namespaces. Later it can be enhanced to check the current namespace's extension dependency compared to called class.

[brambaud]

This is amazing. I want to come back and review it more. Because we could move a lot of this logic into a shared service that can help handle #249.

Like FunctionCallParametersCheck in PHPStan. That namespace service could return errors as needed. To start it'll return errors on the usage of internal across namespaces. Later it can be enhanced to check the current namespace's extension dependency compared to called class.

I'm not sure how a service like that could be named 🤔

Do you mean something like:

final class Foo {
  public function isClassFromAnotherExtension(\PhpParser\Node\Stmt\Class_ $node);
  public function IsClassFromADependencyExtension();
}

?

[mglaman]

Some thoughts on naming and maybe ditching the private methods added to the Rule.

In reference to #247 (I tagged the wrong issue before), we could move these two methods into static methods in a namespace helper utility class. One that checks if a namespace is in Drupal and another if it's in a shared namespace.

[brambaud]

I will try to re-work this PR this week to include the requested changes (hoping I won't have to wait this weekend)

[brambaud]

ah! Seems the fresh (3h ago) and new nikic/php-parser version 4.13.2 breaks the CI thanks to this:

The namespacedName property populated by the NameResolver is now declared on relevant nodes,
to avoid a dynamic property deprecation warning with PHP 8.2.

I think we could ignore this error because ClassLike::$namespacedName can still be null even if its true it will always be declared.
Furthermore, unless we force the version to be >=4.13.2 it could still break if we only take into account the nullable part.

I'm gonna add this error to the ignored error in the phpstan configuration.

What do you think @mglaman?

[mglaman]

@brambaud could we just add the "ignore next line" inline error? On mobile, but PHPStan has some inline comment to ignore an error on the next line

[brambaud]

@mglaman If you prefer the inline ignore tag fine by me!
I'm more used to add the errors directly to the configuration to have only one place to look. The possibility to ignore the same error for all the code source or to comment why it could be ignored is appealing to me.

[mglaman]

@brambaud Since it's one failure, I like a single ignore. If we had 30+ and it'd be a huge refactor, then I'd ignore it in the config. Either approach requires coming back to audit errors being ignored. This single ignore helps as a "reminder" if it shows again in new code

[mglaman]

I'll give this a review later this evening, I have a feeling it's ready to go :)

[mglaman]

Looks great, @brambaud ! I'll pause to merge to see your thoughts on s/ClassHelper/NamespaceCheck.

I like NamespaceCheck::isInDrupalNamespace (or just isDrupalNamespace). I think it reads well

[brambaud]

Agreed about the naming and changes have been done! Thanks!

[mglaman]

This is amazing, thanks!