This is a spin-off from #2621486: Fixes to migrate/tests/src/Unit/*.php files. In that issue, the constructor() method of core/modules/migrate/tests/src/Unit/TestSqlIdMap.php was corrected by hand for the missing @param directive for the final variable $event_dispatcher. For some reason, the FunctionCommentSniff.php sniff is not catching this missing @param directive.

Good example:

  /**
   * Constructs a TestSqlIdMap object.
   *
   * @param \Drupal\Core\Database\Connection $database
   *   The database.
   * @param array $configuration
   *   The configuration.
   * @param string $plugin_id
   *   The plugin ID for the migration process to do.
   * @param mixed $plugin_definition
   *   The configuration for the plugin.
   * @param \Drupal\migrate\Entity\MigrationInterface $migration
   *   The migration to do.
   * @param \Symfony\Component\EventDispatcher\EventDispatcherInterface $event_dispatcher
   *   The event dispatcher service.
   */
  public function __construct(Connection $database, array $configuration, $plugin_id, $plugin_definition, MigrationInterface $migration, EventDispatcherInterface $event_dispatcher) {
  }

Bad example:

  /**
   * Constructs a TestSqlIdMap object.
   *
   * @param \Drupal\Core\Database\Connection $database
   *   The database.
   * @param array $configuration
   *   The configuration.
   * @param string $plugin_id
   *   The plugin ID for the migration process to do.
   * @param mixed $plugin_definition
   *   The configuration for the plugin.
   * @param \Drupal\migrate\Entity\MigrationInterface $migration
   *   The migration to do.
   */
  public function __construct(Connection $database, array $configuration, $plugin_id, $plugin_definition, MigrationInterface $migration, EventDispatcherInterface $event_dispatcher) {
  }
CommentFileSizeAuthor
#4 create_sniff_for-2624558-4.patch2.81 KBYurkinPark

Comments

Lars Toomre created an issue. See original summary.

anoopjohn’s picture

Title: Incorrect sniff for missing final @param directive » Create sniff for missing @param directive

I checked this. It does not look like we have a rule that catches a missing @param directive at all. What we have is a rule that checks if the actual comment in a @param directive is missing and that gives the MissingParamComment error.

andypost’s picture

Confirm bug, functions that have patams but does not declare them in doc block pass.
Same time we still have hooks that should omit params - that maybe really tricky to handle

YurkinPark’s picture

StatusFileSize
new2.81 KB

This could be a solution

YurkinPark’s picture

Status: Active » Needs review
klausi’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests

Cool, can you file a pull request against https://github.com/klausi/coder/pulls and add a test case?

YurkinPark’s picture

  • klausi committed bba7132 on 8.x-2.x authored by YurkinPark
    fix(FunctionCommentSniff): Add check for missing param doc comments (#...

  • klausi committed 4756f95 on 8.x-2.x
    fix(FunctionCommentSniff): @param comments are allowed to be omitted...
klausi’s picture

Title: Create sniff for missing @param directive » Create sniff for missing @param directive when other @param tags exist
Status: Needs work » Fixed
Issue tags: -Needs tests

Thanks!

So this was a bit tricky because we allow omission of param tags:

// Missing parameters only apply to methods and not function because on
// functions it is allowed to leave out param comments for form constructors
// for example.
// It is also allowed to omit pram tags completely, in which case we don't
// throw errors. Only throw errors if param comments exists but are
// incomplete on class methods.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.