PHPCS errors found after creating sub-theme

Hey @saltednut, I have added the patch please verify it.

Hello! I tested this, first confirming that I saw the PHPCS errors mentioned when the patch was not yet applied. After applying the MR as a patch, the PHPCS errors are no longer present. Marking RTBC as a result.

(That said, I should note that line 60 in the .theme file—the comment starting with "Calculate the minimum and maximum"—warns me, both before and after, that it's lengthier than the 80-character limit in Drupal's standards. Not sure why it doesn't appear in your testing, so I didn't flip back to Needs Work in case it's just something I'm doing differently on my end.)

Since I am picking this up cold, I ran phpcs before attempting to apply the fixes in the MR and found a bunch of other errors and warnings. (So @kurttrowbridge, it wasn't just a local quirk!) I'm curious to know why the same test is showing people different results, but not enough to dig into that part. Here's what I am seeing:

drupal_sandbox > code-review:

     [echo] $> vendor/bin/phpcs
     [exec] ...............................EW.E. 36 / 36 (100%)
     [exec] 
     [exec] 
     [exec] 
     [exec] FILE: /var/www/html/docroot/themes/custom/governor/governor.theme
     [exec] ------------------------------------------------------------------------------------------
     [exec] FOUND 3 ERRORS AND 1 WARNING AFFECTING 3 LINES
     [exec] ------------------------------------------------------------------------------------------
     [exec]  43 | ERROR   | [x] There must be exactly one blank line before the tags in a doc comment
     [exec]  43 | ERROR   | [x] Trailing punctuation for @see references is not allowed.
     [exec]  45 | ERROR   | [ ] Parameter tags must be defined first in a doc comment
     [exec]  59 | WARNING | [ ] Line exceeds 80 characters; contains 84 characters
     [exec] ------------------------------------------------------------------------------------------
     [exec] PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
     [exec] ------------------------------------------------------------------------------------------
     [exec] 
     [exec] 
     [exec] FILE: /var/www/html/docroot/themes/custom/governor/governor.info.yml
     [exec] -----------------------------------------------------------------------------------------------------------
     [exec] FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
     [exec] -----------------------------------------------------------------------------------------------------------
     [exec]  1 | WARNING | Remove "version" from the info file, it will be added by drupal.org packaging automatically
     [exec] -----------------------------------------------------------------------------------------------------------
     [exec] 
     [exec] 
     [exec] FILE: /var/www/html/docroot/themes/custom/governor/src/StarterKit.php
     [exec] -------------------------------------------------------------------------
     [exec] FOUND 4 ERRORS AND 2 WARNINGS AFFECTING 6 LINES
     [exec] -------------------------------------------------------------------------
     [exec]   10 | ERROR   | [x] Missing class doc comment
     [exec]   43 | ERROR   | [ ] Description for the @return value is missing
     [exec]   65 | ERROR   | [x] Expected 1 blank line after function; 2 found
     [exec]   78 | WARNING | [ ] Line exceeds 80 characters; contains 95 characters
     [exec]  153 | ERROR   | [ ] Parameter $machine_name is not described in comment
     [exec]  154 | WARNING | [ ] Line exceeds 80 characters; contains 115 characters
     [exec] -------------------------------------------------------------------------
     [exec] PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
     [exec] -------------------------------------------------------------------------
     [exec] 
     [exec] Time: 72ms; Memory: 6MB
     [exec] 

BUILD FAILED

as @saltednut pointed out, we need an MR, not a patch, so I'm going to fix the rest of these errors, and get it all merged.

Point of clarification: I am running code tests both on the starterkit itself, and an instance of the theme that gets spun up from running the starterkit script.

I am not able to replicate the PHPStan error mentioned in #5 but that is probably because my test config isn't set up to check /themes/contrib/. It seems like if it did, this error would happen for every helper function, or functions that don't contain the theme name, as they would slip passed being renamed by the starterkit script.

I can't think of a use case where you would have both the starterkit, and the generated project theme installed at the same time, so I can't think of an instance where you'd actually hit this error in the wild. That said, if I change the name of the function in the starterkit to include "governer" after the underscore like this: _governor_is_page_link_visible when I run the script the function name is updated as expected, so even if one did have both the starterkit and the generated theme installed at the same time, you wouldn't get this error. I'll add this update to the MR.

Update:
The errors and warnings above coming from src/StarterKit.php can probably be ignored. Given all the work that has been done to spin up new custom themes using the Core generate-theme script with just the starterkit.yml file, it looks as though we might not need that file at all. And it is removed from the generated theme, so isn't problematic there, since, you know, it doesn't exist there.

The error thrown by the missing description in the generated theme can be avoided by adding a default theme description in the starterkit.yml file. Something like:

info:
  hidden: null
  starterkit: null
  version: 1.0.0
  description: Generated by the Governor starterkit. Please update this description for your specific project.

The warning getting thrown about the "Version", however, appears to be a Core bug relating to how the generate-theme script handles Version tracking. I am able to replicate the error using the Core starterkit_theme. In order for the generate-theme script to run at all, a version must be specified in the starterkit theme's info.yml file. However, the script adds a generated version as well. As a result, the generated theme's info.yml file will have both. e.g.

name: gov_theme_test
...
version: 1.0.0
...
generator: 'governor:1.0.0'

The generated version number gets pulled from governor.starterkit.yml:

info:
  version: 1.0.0

Remove the Version key, the script won't run, leave it in, and it throws an error. Don't specify a version in code>governor.starterkit.yml and an error is thrown in the generated info.yml file.

The easiest way to get rid of the error is to manually delete "Version: 1.0.0" from the generated info.yml file in the newly minted custom theme. But the 'correct' solution is for the generate-theme script not to add the Version key to the generated info.yml file in the first place, since, "version" is special: we want to allow it in core, but not anywhere else." (See vendor/drupal/coder/coder_sniffer/Drupal/Sniffs/InfoFiles/AutoAddedKeysSniff.php line 78).
I'll open a core issue to address this. I just wanted to get all my thoughts out here first.