Incorrect documentation for \Drupal\views\Plugin\views\join\JoinPluginInterface

@Hardik_Patel_12: You must add: use Drupal\Core\Database\Query\SelectInterface; for your patch to work. Without it PHP thinks that SelectInterface is in the current namespace.

#11 looks good to me

Some more clean up can be done here while we are touching this

1. +++ b/core/modules/views/src/Plugin/views/join/JoinPluginBase.php
   @@ -4,7 +4,7 @@
   +use Drupal\views\Plugin\views\query\QueryPluginBase;
    /**
   

   Needs a newline between the use statement and the docblock

2. +++ b/core/modules/views/src/Plugin/views/join/JoinPluginInterface.php
   @@ -14,13 +17,13 @@
       *   An select query object.
   

   Can we fix this typo while we are touching this? 'an select' => 'a select'

3. +++ b/core/modules/views/src/Plugin/views/join/JoinPluginInterface.php
   --- a/core/modules/views/src/Plugin/views/join/Subquery.php
   +++ b/core/modules/views/src/Plugin/views/join/Subquery.php
   
   +++ b/core/modules/views/src/Plugin/views/join/Subquery.php
   @@ -38,7 +41,7 @@ public function __construct(array $configuration, $plugin_id, $plugin_definition
       * @param \Drupal\views\Plugin\views\query\QueryPluginBase $view_query
       *   The source views query.
       */
   -  public function buildJoin($select_query, $table, $view_query) {
   +  public function buildJoin(SelectInterface $select_query, $table, QueryPluginBase $view_query) {
   

   Can we update the docblock for Subquery to just be {@inheritdoc}?

@lendude: Do you know if there are contrib modules that extend the buildJoin method? If so, then that would result in a BC-break.

I have just one remark:

+++ b/core/modules/views/src/Plugin/views/join/JoinPluginBase.php
@@ -109,10 +109,10 @@
- *     parent::buildJoin($select_query, $table, $view_query);
+ *     parent::buildJoin(SelectInterface $select_query, $table, QueryPluginBase $view_query);

You do not need to add type hints in method calls.

@lendude: Do you know if there are contrib modules that extend the buildJoin method?

Yeah was thinking about that too. Quick check in my contrib folder:
SearchAPI => \Drupal\search_api\Plugin\views\join\SearchApiJoin

So this a no-go in D8 and since it probably needs a deprecation or something, it probably would need to be announced/deprecated in D9 and then go into D10 :/

So I would probably just advice to leave this and when we start adding return type hints and break everything anyway, we can break this too ¯\_(ツ)_/¯

So this a no-go in D8 and since it probably needs a deprecation or something, it probably would need to be announced/deprecated in D9 and then go into D10 :/

So I would probably just advice to leave this and when we start adding return type hints and break everything anyway, we can break this too ¯\_(ツ)_/¯

The adding of parameter type hints need to go, but what we still can do is the docblock parts.

+++ b/core/modules/views/src/Plugin/views/join/JoinPluginInterface.php
@@ -14,13 +17,13 @@
-   * @param $select_query
-   *   An select query object.
+   * @param \Drupal\Core\Database\Query\SelectInterface $select_query
+   *   A select query object.
    * @param $table

IMO fixing this typo is out of scope here.

Besides, the docs need more detail -- what about the select query object? What is it here for, what does this method do with it?

I came here because the type for the parameter $table is incorrect. In the interface it is documented as a string, but \Drupal\views\Plugin\views\join\JoinPluginBase uses it as if it is an array:

$select_query->addJoin($this->type, $right_table, $table['alias'], $condition, $arguments);

On a local project I use a custom ViewsJoin plugin, and now PHPStan complains that $table cannot be an array.

Since I think that also the type of parameter $select_query would better be \Drupal\Core\Database\Query\SelectInterface than \Drupal\Core\Database\Query\Select, I thought the changes to the $table parameter would fit within this issue.

In the MR, the descriptions of the parameters are generated by AI, as I'm not an expert on the Views code. I did attempt to verify if the generated documentation is correct and removed a part generated by AI that looked more like a reaction to my prompt.

Looks good overall, just a few small suggestions.

I've attempted to address the feedback by @joachim.

Thanks! Left a few comments.

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

This looks great, just needs a rebase according to the bot.