Console create:plugin - PascalCase namespace

[WebVPF]

Winter CMS Build

dev-develop

PHP Version

8.0

Database engine

MySQL/MariaDB

Plugins installed

No response

Issue description

Winter CMS 1.2
Console command create:plugin incorrectly changes namespace. Camelcase names are changed to capitalized names.
Example:

php artisan create:plugin WebVPF.MyName

Plugin.php file generation result:

<?php namespace Webvpf\Myname;
...

Must be:

<?php namespace WebVPF\MyName;

Steps to replicate

Create a plugin

Workaround

No response

[arvislacis]

Some comments for clarification:

1. Issue name and description should contain PascalCase not camelCase;
2. Issue is probably more relevant in wintercms/storm repository but that's just side node;
3. If we look at https://wintercms.com/docs/help/developer/guide#vendor-naming then it states - "The vendor or author code in a namespace must begin with an uppercase character and should not contain underscores or dashes." - nothing much about Pascal case here.
4. Pascal Case is mentioned under https://wintercms.com/docs/help/developer/guide#class-naming
5. If we look into WinterCMS own plugins then it looks like mostly PascalCase is used in the namespace names, for example:

• https://github.com/wintercms/wn-pwnedpasswords-plugin/blob/main/Plugin.php
• https://github.com/wintercms/wn-mailchimp-plugin/blob/main/Plugin.php
• https://github.com/wintercms/wn-drivermandrill-plugin/blob/main/Plugin.php
• etc.

So, yes, could be an issue.

[LukeTowers]

Just need to change a single line here: https://github.com/wintercms/winter/blob/wip%2F1.2/modules/system/console/BaseScaffoldCommand.php#L47. PRs welcome.

[RomainMazB]

@LukeTowers Actually, to be synced with the scaffold methods' code style, we'll need another modification here to add the ucfirst case: https://github.com/wintercms/storm/blob/develop/src/Scaffold/GeneratorCommand.php#L144-L146

[LukeTowers]

@RomainMazB I'm not sure I understand, could you expand on that?

[RomainMazB]

Sure @LukeTowers.

To convert "WebVPF" to "WebVPF" and ensuring the first letter is uppercase to keep the PascalCase style, we need to use the php native ucfirst function, or the Str::ucfirst laravel helper.

So, to me, we have two ways to fix this:
The cheap:

// Using native ucfirst function or Str::ucfirst directly inside the method you mentioned
$ucfirst_author = ucfirst($vars['author']);
$ucfirst_plugin = ucfirst($vars['plugin']);
$vars['plugin_namespace'] = "{$ucfirst_author}\\{$ucfirst_plugin}";

The good (to me):

// GeneratorCommand.php
    protected function processVars($vars)
    {
        $cases = ['upper', 'lower', 'snake', 'studly', 'camel', 'title', 'ucfirst']; // Add ucfirst casing

// BaseScaffoldCommand.php
$vars['plugin_namespace'] = "{$vars['ucfirst_author']}\\{$vars['ucfirst_plugin']}";

---

The second one looks prettier with the rest of the method:

$vars = parent::processVars($vars);

$vars['plugin_id'] = "{$vars['lower_author']}.{$vars['lower_plugin']}";
$vars['plugin_code'] = "{$vars['studly_author']}.{$vars['studly_plugin']}";
$vars['plugin_url'] = "{$vars['lower_author']}/{$vars['lower_plugin']}";
$vars['plugin_folder'] = "{$vars['lower_author']}/{$vars['lower_plugin']}";
$vars['plugin_namespace'] = "{$vars['ucfirst_author']}\\{$vars['ucfirst_plugin']}";

return $vars;

instead of this:

$vars = parent::processVars($vars);

$ucfirst_author = ucfirst($vars['author']);
$ucfirst_plugin = ucfirst($vars['plugin']);

$vars['plugin_id'] = "{$vars['lower_author']}.{$vars['lower_plugin']}";
$vars['plugin_code'] = "{$vars['studly_author']}.{$vars['studly_plugin']}";
$vars['plugin_url'] = "{$vars['lower_author']}/{$vars['lower_plugin']}";
$vars['plugin_folder'] = "{$vars['lower_author']}/{$vars['lower_plugin']}";
$vars['plugin_namespace'] = "{$ucfirst_author}\\{$ucfirst_plugin}";

return $vars;

[arvislacis]

@LukeTowers I think that @RomainMazB was referrencing that we would need to add ucfirst modifier (https://www.php.net/manual/en/function.ucfirst.php) to make the first character of the string uppercase. However, that is not needed as studly case is basically equal to PascalCase (https://www.php-fig.org/psr/psr-12/#21-basic-coding-standard).

[RomainMazB]

@arvislacis you've got a point. So now the question is: why php artisan create:plugin WebVPF.MyName generates these studly variables:

"studly_author" => "Webvpf"
"studly_plugin" => "Myname"

and Str::studly('WebVPF') generates the good studly case 🤔

>>> Str::studly('WebVPF');
=> "WebVPF"
>>> Str::studly('MyName');
=> "MyName"

[arvislacis]

@RomainMazB Yeah, I came to same conclusion recently - that studly case for some reason doesn't work properly here.

[RomainMazB]

@arvislacis @LukeTowers , here is why.
The BaseScaffoldCommand class has the HasPluginArgument trait, which means that the arguments relies on the PluginManager's normalizeIdentifier method: https://github.com/wintercms/winter/blob/wip/1.2/modules/system/console/traits/HasPluginArgument.php#L54-L58
This method actually lowercase the plugin name and author, which fails all the rest of the process...
https://github.com/wintercms/winter/blob/wip/1.2/modules/system/classes/PluginManager.php#L575-L579

When the tiny problem becomes a "big" one 😄

[LukeTowers]

Fair point. What I would like to see is that the namespace is generated from the provided input verbatim rather than trying to do any case modifications to it. It should be up to the dev whether their namespace is Webvpf or WebVPF.

[arvislacis]

Fair point. What I would like to see is that the namespace is generated from the provided input verbatim rather than trying to do any case modifications to it. It should be up to the dev whether their namespace is Webvpf or WebVPF.

Yes, I agree on you with this - the best solution in this situation probably would be to take input verbatim..., the only modification maybe could be applied only to the first letter to make it always uppercase so it matches the coding standarts and guides (for example, if dev accidentally or by other means enter everything in the lowercase), anyways, that's also a disputable point.

[RomainMazB]

@arvislacis yes, their is chance where the dev will fail on typing, especially when creating model or controller after creating the plugin...

I agree with the verbatim stuff but this introduce possible dev errors. We should warn this into the documentation.

[arvislacis]

@RomainMazB If we look at Study/PascalCase from very basic point of view then it can also be interpreted as string in which the first character is always uppercase - all the rest characters can be interpreted verbatim from input as we don't know what acronyms dev has used for input etc. So maybe we could take user's input directly but ensure that the first character is always uppercase.

[RomainMazB]

@LukeTowers , I was about to submit a PR when discovering something that is strange but provides another way to treat two problems at once.
On the 1.2 branch, you introduced a getNormalizedIdentifier method with the parameter lower which should define if we want a lower-cased normalized identifier or not:

winter/modules/system/classes/PluginManager.php

Lines 575 to 589 in 5ccde6f

	public function normalizeIdentifier(string $code): string
	{
	$code = strtolower($code);
	return $this->normalizedMap[$code] ?? $code;
	}
	/**
	* Returns the normalized identifier (i.e. Winter.Blog) from the provided
	* string or PluginBase instance.
	*/
	public function getNormalizedIdentifier(PluginBase|string $plugin, bool $lower = false): string
	{
	$identifier = $this->normalizeIdentifier($this->getIdentifier($plugin));
	return $lower ? strtolower($identifier) : $identifier;
	}

I thought this method was our perfect solution, but actually this method can't return anything but a lower-cased normalized identifier due to the fact that itself uses the normalizeIdentifier method which - as we defined previously - forcefully return a lower-cased identifier.

I've searched into the code where this method is used and I've found 7 places where the lower parameter is set to true and 1 where the default false value is used:

As we now know, all of these usages could use the lower parameter set to true because the getNormalizedParameter basically rely on the normalizeIdentifier which return a lower cased string.

What I suggest is to introduce a lower parameter in the method which caused our initial problem, defaulted to true so we could:

• Define in our scaffold command if we want to use a lower case version for the plugin and author names
• Fix the lower parameter from the getNormalizedParameter which atm doesn't do anything (even if we actually doesn't need it atm)

The lower parameter into the getNormalizedParameter method could also be defaulted to true at this point of time where all the place where we use it work with a lower-cased string

---

Something like:

     public function normalizeIdentifier(string $code, bool $lower = true): string 
     { 
         $lowerCode = strtolower($code);

         $code = $this->normalizedMap[$lowerCode] ?? $code;

         return $lower ? strtolower($code) : $code;
     } 
  
     /** 
      * Returns the normalized identifier (i.e. Winter.Blog) from the provided 
      * string or PluginBase instance. 
      */ 
     public function getNormalizedIdentifier(PluginBase|string $plugin, bool $lower = true): string 
     { 
         return $this->normalizeIdentifier($this->getIdentifier($plugin), $lower); 
     } 

[LukeTowers]

@jaxwilko can you look at @RomainMazB's comment above when you've got a moment? Too late at night for me to fully parse it at the moment.

[jaxwilko]

Hey all, I've raised PR #588 which resolves the issue.

The problem from my perspective is that normalizeIdentifier() would check $this->normalizedMap for the code, but when the code was not found would return the lowered input rather than the actual input.

To simplify things, I've moved this logic into getNormalizedIdentifier() (which still has the additional functionality of handling plugin objects & returning a lowered identified), and then updated normalizeIdentifier() to call getNormalizedIdentifier(). This was done for backwards compatibility so we didn't have to change normalizeIdentifier()'s public api.

Now when creating a plugin the casing is preserved from the user input, i.e.

Please let me know if you have any thoughts or comments on this :)

[bennothommo]

Fixed by #588.