Fix the issues reported by phpcs

I have provided a patch to fix this issue.
Please review it.

I reviewed the patch #2. It applied cleanly and no error/warning related to phpcs Drupal are produced. How ever this issue is about fixing phpcs issues so all issues can be taken care in single issue no need to create different issues. like https://www.drupal.org/project/menu_trail_by_path/issues/3371326 📌 Replace the t() calls to $this->t() in classes. Needs review

Thanks!

Hi @Harshita mehna, Verified and Tested #2 📌 Fix the issues reported by phpcs Needs work .
It's working fine.

Can be moved to RTBC.

   /**
+   * Template File Doc Comment.
+   *
    * @var \Drupal\Core\Path\PathValidatorInterface
    */
-   protected $pathValidator;

   /**
+   * Template File Doc Comment.
+   *
    * @var \Drupal\Core\Routing\RequestContext
    */
   protected $context;

   /**
+   * Template File Doc Comment.
+   *
    * @var \Drupal\Core\Language\LanguageManagerInterface
    */

Those are not property descriptions. (The fact the same phrase is repeated for three different properties of the same class should suggest that phrase does not describe those properties.)

    * @param \Drupal\Core\Menu\MenuLinkManagerInterface $menu_link_manager
+   *   Managing menu links and storing their definitions.
    * @param \Drupal\Core\Routing\RouteMatchInterface $route_match
+   *   Provides an interface for classes representing the result of routing.
    * @param \Drupal\Core\Cache\CacheBackendInterface $cache
+   *   Defines an interface for cache implementations.
    * @param \Drupal\Core\Lock\LockBackendInterface $lock
+   *   Lock backend interface.
    * @param \Drupal\Core\Path\PathValidatorInterface $path_validator
+   *   Provides an interface for URL path validators.
    * @param \Drupal\Core\Routing\RequestContext $context
+   *   Provides a fluent interface.
    * @param \Drupal\Core\Language\LanguageManagerInterface $languageManager
+   *   Common interface for the language manager service.
    * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory
+   *   Defines the interface for a configuration object factory.

Those are not correct descriptions to use with parameters. I would suggest to check the parameter descriptions used for Drupal core classes. In most of the cases, there is no need to use a different description.
For example, it is not Defines the interface for a configuration object factory. (since an object does not define an interface, but implements an interface), but The config factory.

-   * @return \Drupal\Core\Menu\MenuLinkInterface|NULL
-   *   The menu link for the given menu, or NULL if there is no matching menu link.
+   * @return \Drupal\Core\Menu\MenuLinkInterface|null
+   *   The menu link refers to the given menu, or NULL if none exists.
    */

The existing description is already correct.

-   * NOTE: There is a difference between $this->routeMatch->getRouteName and $this->context->getPathInfo()
-   * for now it seems more logical to prefer the latter, because that's the "real" url that visitors enter in their browser..
+   * NOTE: There is a difference between:
+   * $this->routeMatch->getRouteName and $this->context->getPathInfo()

The period at the end is missing. NOTE must be spelled Note and the colons after Note and between must be removed.

+   * for now it seems more logical to prefer the latter,
+   * because that's the "real" url that visitors enter
+   * in their browser.

For must be capitalized and url spelled URL.

    * @return \Drupal\Core\Url|null
+   *   A string containing a URL that may be used to access the file.

The description must also say when NULL is returned.

   /**
-   * Array key's should be the menu title's, if multi-level than separated by ' » '.
+   * Array key's should be the menu title's.
    *
-   * @var Url[]
+   * If multi-level than separated by ' » '.
+   *
+   * @var \Drupal\Core\Url[]
    */
   protected $menuUrls = [];

The description does not describe the property.

   /**
+   * Provides an interface defining a user entity.
+   *
    * @var \Drupal\user\UserInterface
    */
   protected $authenticatedUser;

A property does not provide interfaces; eventually, an object implements interfaces, but that does not say what that property contains.

-    // Create user
+    // Create user.
     $this->authenticatedUser = $this->drupalCreateUser();

That comment can be removed, since there is no need to explain what $this->drupalCreateUser() does; the method name explains itself.

-    // Create content type
-    $this->drupalCreateContentType(array('type' => 'page', 'name' => 'Basic page'));
+    // Create content type.
+    $this->drupalCreateContentType(['type' => 'page', 'name' => 'Basic page']);
 
-    // Create nodes
+    // Create nodes.
     $home = $this->drupalCreateNode();
     $this->createPathAlias('/node/' . $home->id(), '/home');

The same holds true for those comments.

   /**
-   * Test url: Home
+   * Test url: Home.
    */
   public function testUrlHome() {
     $this->drupalGet(clone $this->menuUrls['Home']);
@@ -111,7 +115,7 @@ class MenuTrailByPathActiveTrailHtmlClassTest extends BrowserTestBase {
   }
 
   /**
-   * Test url: User password
+   * Test url: User password.
    */
   public function testUrlUserPassword() {
     $this->drupalGet(clone $this->menuUrls['User password']);
@@ -156,7 +160,7 @@ class MenuTrailByPathActiveTrailHtmlClassTest extends BrowserTestBase {
   }
 
   /**
-   * Test url: User login
+   * Test url: User login.

That is the minimum required change. Test url: User login should be Test the user login URL. Similar change must be done for the other comments.

-    // Also test the parent item, due to the tree url key construction of assertMenuActiveTrail we need two separate calls
+    // Also test the parent item, due to the tree url key
+    // construction of assertMenuActiveTrail we need two separate calls.

The correct sentence is Also test the parent item; due to the tree URL key construction of assertMenuActiveTrail, we need two separate calls.

   /**
-   * Build a menu with the data of $this->menuUrls
+   * Build a menu with the data of $this->menuUrls.
    *
    * @param string $menu_name
+   *   Build a menu.
    */
   protected function buildMenu($menu_name = 'main') {

Build in the method description must be Builds.
Build a menu is not a parameter description: A parameter does not build anything.
The return value description is missing.

   /**
-   * Helper for getting the base: "link_path" that assertMenuActiveTrail expects.
+   * Helper for getting the base:"link_path" that assertMenuActiveTrail expects.

It is the base link path that.

+   * @param string $name
+   *   Specifies the URL for all relative URLs in the page.

Specifies is not necessary.

Addressed all the suggestion mentioned in #8 📌 Fix the issues reported by phpcs Needs work .
Please review the MR.

Hi,

I applied the latest MR !8 to the Menu Trail By Path module against 2.x-dev on Drupal 9.5.10 and confirmed all PHPCS errors have been fixed.

I ran this command on the module:
phpcs --standard=Drupal --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml menu_trail_by_path/

Please see the attached files for reference.

I'm moving this now to RTBC.

Thank you!

Hi @everyone,

The changes made on MR!9 was applied not-so successfully, might be the reason there are errors reported. Please see below:

menu_trail_by_path git:(2.x) curl https://git.drupalcode.org/project/menu_trail_by_path/-/merge_requests/9.diff | patch -p1
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 13989    0 13989    0     0  26403      0 --:--:-- --:--:-- --:--:-- 27005
patching file src/Form/MenuTrailByPathSettingsForm.php
patching file src/MenuTrailByPathActiveTrail.php
Hunk #6 succeeded at 235 (offset 5 lines).
patching file src/MenuTrailByPathServiceProvider.php
patching file tests/src/Functional/MenuTrailByPathActiveTrailHtmlClassTest.php
Hunk #1 succeeded at 38 with fuzz 1.
➜  menu_trail_by_path git:(2.x) ✗ ..
➜  contrib git:(main) ✗ phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,twig menu_trail_by_path

FILE: ...l_by_path/tests/src/Functional/MenuTrailByPathActiveTrailHtmlClassTest.php
--------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
--------------------------------------------------------------------------------
 294 | ERROR | [x] Separate the @param and @return sections by a blank line.
 295 | ERROR | [x] Return comment indentation must be 3 spaces, found 2 spaces
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------


FILE: ...Demo-site/drupal-orgissue/web/modules/contrib/menu_trail_by_path/README.md
--------------------------------------------------------------------------------
FOUND 1 ERROR AND 2 WARNINGS AFFECTING 3 LINES
--------------------------------------------------------------------------------
  3 | WARNING | [ ] Line exceeds 80 characters; contains 84 characters
  6 | WARNING | [ ] Line exceeds 80 characters; contains 91 characters
 52 | ERROR   | [x] Expected 1 newline at end of file; 0 found
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
--------------------------------------------------------------------------------

Time: 279ms; Memory: 12MB

Kindly check

Thanks,
Jake

There are still PHP_CodeSniffer errors to fix.

avpaderno → changed the visibility of the branch 3371317-fix-the-issues to hidden.

avpaderno → changed the visibility of the branch 3371317-fix-the-issues to active.

avpaderno → changed the visibility of the branch 3371317-gitlab-ci-exports to hidden.

MR !9 fixes all the reported PHP_CodeSniffer errors/warnings.

Conflicted on this one, large chunks of code being reformatted here are completely rewritten in 📌 Use yield/generator to provide active trail URL candidates and other performance improvements Needs review , which will likely become a new 3.x version in the near future. Will hold out on this for now.

As usual, there is also at least one duplicate: 📌 Coding Standards Needs review .

Hi @avpaderno,

Confirmed PHPCS errors no longer exist: https://git.drupalcode.org/issue/menu_trail_by_path-3371317/-/jobs/2505527.

Please see the attached file for reference.

I'm moving this now to ‘RTBC’.

Thank you!

See the previous comment by berdir. The code is being rewritten, which means (at best) that this issue is postponed.