The test for the Twig version in bootstrap.php is loading the Environment class early, even when just parsing for code analysis. I'm just starting to look at this, and I think I'm right the test doesn't happen early in the v2 (master) branch, only v1 used by Drush? So am wondering if there is already a good alternative?

I stumbled onto the issue because of phpactor/phpactor#1043

Currently, content entity generates this kind of permissions:

Example:

…
{% if bundle %}
 *   admin_permission = "administer {{ entity_type_label|lower }} types",
{% elseif fieldable %}
 *   admin_permission = "administer {{ entity_type_label|lower }}",
{% else %}
…

Is that safe and works as intended or this is a bug?

If user tries to generate custom entity and gives it label "Node" it will share permissions with core "node" module. This is not very obvious and can lead to a lot of problems in the future.

Config entity type on the other hand, uses entity_type_id.

I propose to change content entity type permission from using this {{ entity_type_label|lower }} to that {{ entity_type_label }}. This will make generation across entity types consistent and safer for common entity labels.

When providing answers via the --answers option, is it possible to allow the default values to be used non-interactively?

For example, dcg twig-extension -a '{"name": "Example", "machine_name": "example", "class": "ExampleTwigExtension"}' Leaving off class prompts for input. Other values like true, false and null don't seem to work either.

Thanks!

I was able to install this package with symfony/filesystem v6, but I also had to allow the dev-master version of friendsoftwig/twigcs as there is not yet a compatible release of that package.

I am using a generated preloading script which looks something like this:

$loader = require_once '/app/web/autoload.php';
// more lines to load PSR-4 directories (irrelevant to this issue)

$files = [
    '/app/vendor/autoload.php',
    '/app/load.environment.php',
    '/app/vendor/composer/autoload_static.php',
    // more files to be preloaded
];

foreach ($files as $file) {
    try {
        if (!(is_file($file) && is_readable($file))) {
            throw new \Exception("{$file} does not exist or is unreadable.");
        }
        require_once $file;
    } catch (\Throwable $e) {
        // ...
        throw $e;
    }
}

This script is set to opcache.preload setting. The script works and I see that the preload is making a difference, but there is a warning on all pages.

Warning: Cannot declare class DrupalCodeGenerator\Twig\TwigEnvironment, because the name is already in use in /app/vendor/chi-teck/drupal-code-generator/src/bootstrap.php on line 44

Warning: Cannot declare class DrupalCodeGenerator\TwigEnvironment, because the name is already in use in /app/vendor/chi-teck/drupal-code-generator/src/bootstrap.php on line 47

After debugging, I realized this is because bootstrap.php is loaded by composer (it's part of composer.json's autoload.files option). As the warnings suggest, this is due to the class_alias line.

After a lot of debugging and research, I conclude that the problem is that the preloading works somewhat differently for class_alias. In other words, this could be a PHP bug and I do some hints for this in https://github.com/php/php-src/pull/3538 and https://github.com/symfony/symfony/issues/29105.

My theory is that while preloading correctly handles all the regular classes and functions, it cannot handle class_alias properly. Since class_alias runs both while preloading (as we are requiring the autoloader) and during the execution, it seems that the internal PHP entry for the class name persists during the runtime causing the warning.

The fix in my case was to wrap the class_alias call inside an if block with class_exists condition. I don't see any warnings after that. I'm going to create a PR against 1.x branch for that shortly.

Footnote: This is tangentially related to #46 but I don't know if this change would help static analyzers.

A bit confusing to use cancel url here IMO. https://github.com/Chi-teck/drupal-code-generator/blob/e3ce7b84f46f767ebb6405e5bfa8af3237f6ef89/templates/form/confirm/form.twig#L41

Drupal 10 will require PHP 8.1 which means we can make use of all new PHP features available at this moment. However, Drupal coding standards weren't updated yet to reflect this change and will unlikely be updated in the foreseeable future. Furthermore, they don't cover PHP 7 features yet. So far, all code generated by DCG follows Drupal core coding standards. Which means if something is not covered in Drupal coding standards it uses Drupal core code base as a referencing point. Nowadays, the generated code looks very outdated as most of Drupal core code is written using PHP 5 syntax only.

For custom projects it's not a problem to establish own standards for features that are missing in the Drupal coding standards. Example: https://github.com/Chi-teck/drupal-coder-extension

However, for generated code that is going to be part of other projects it needs sort of community approval.

Here is an example of the currently generated code. https://github.com/Chi-teck/drupal-code-generator/blob/3.0.0-alpha2/tests/functional/Generator/_controller/src/Controller/FooController.php

The following example represents proposed updates.

<?php declare(strict_types = 1);

namespace Drupal\example\Controller;

use Drupal\Core\Controller\ControllerBase;
use Drupal\Core\Entity\EntityTypeManagerInterface;
use Symfony\Component\DependencyInjection\ContainerInterface;

/**
 * Returns responses for Example routes.
 */
final class ExampleController extends ControllerBase {

  /**
   * The controller constructor.
   */
  public function __construct(
    private readonly EntityTypeManagerInterface $entity_type_manager,
  ) {}

  /**
   * {@inheritdoc}
   */
  public static function create(ContainerInterface $container): self {
    return new self($container->get('entity_type.manager'));
  }

  /**
   * Builds the response.
   */
  public function build(): array {
    $build['content'] = [
      '#type' => 'item',
      '#markup' => $this->t('It works!'),
    ];
    return $build;
  }

}

Here is an overview of the changes:

1. Declare strict types. #3250827
2. Use final/private/self keywords to define non-abstract classes. #3019332
3. Use constructor property promotion. #3278431
4. Use type hints wherever it is possible. #3154762
5. Omit @var, @param, @return tags for strictly typed values. #3238192
6. Mark class properties as readonly if they are not ever updated.
7. Use trailing commas for multiline arrays, methods and annotations.

This affects running multiple commands from one

Running commands from other commands, HelperSet is initialised at Application level, so InputHandler is filled with questions from prior commands.

I have a generator that:

• reads in csv data
• calls another command to generate entity types with that data (repeatedly)

The input from the last command is still present because the InputHandler is not re-initialised each command only at application level.

Potential solutions:

• Initialise a command initiated HelperSet instead of at Application
• Re-initialise InputHandler within the command so it is clean when the command is run.

Happy to implement a solution, would either of the above solutions be preferred or accepted?

I was thinking about adding a template to generate the links menu, but I found that links.menu.twig already exists. Still, the file is not generated. Is this a known issue?

Did not dig into the code.

$ drush generate module

 Welcome to module-standard generator!
–––––––––––––––––––––––––––––––––––––––

 Module name:
 ➤ Guest Upload

 Module machine name [guest_upload]:
 ➤ 

 Module description [The description.]:
 ➤ Allow anonymous users to upload images

 Package [Custom]:
 ➤ Other

 Dependencies (comma separated):
 ➤ block_upload

 Would you like to create install file? [Yes]:
 ➤ 

 Would you like to create libraries.yml file? [Yes]:
 ➤ no

 Would you like to create permissions.yml file? [Yes]:
 ➤ 

 Would you like to create event subscriber? [Yes]:
 ➤ 

 Would you like to create block plugin? [Yes]:
 ➤ no

 Would you like to create a controller? [Yes]:
 ➤ 

 Would you like to create settings form? [Yes]:
 ➤ 

 The following directories and files have been created or updated:
–––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––––
 • modules/custom/guest_upload/guest_upload.info.yml
 • modules/custom/guest_upload/guest_upload.install
 • modules/custom/guest_upload/guest_upload.module
 • modules/custom/guest_upload/guest_upload.permissions.yml
 • modules/custom/guest_upload/guest_upload.routing.yml
 • modules/custom/guest_upload/guest_upload.services.yml
 • modules/custom/guest_upload/src/Controller/GuestUploadController.php
 • modules/custom/guest_upload/src/EventSubscriber/GuestUploadSubscriber.php
 • modules/custom/guest_upload/src/Form/SettingsForm.php

Please create generator phpstorm meta file. https://confluence.jetbrains.com/display/PhpStorm/PhpStorm+Advanced+Metadata

###EXAMPLE

<?php

namespace PHPSTORM_META {

  use Drupal\node\NodeStorageInterface;

  override(\Drupal::service(0), map([
    'date.formatter' => \Drupal\Core\Datetime\DateFormatterInterface::class,
  ]));
  override(\Drupal::entityTypeManager()->getStorage(0), map([
    'node' => NodeStorageInterface::class,
  ]));
}

The generated edit form for a content entity currently copies the logic from the parent method. It is a better practice to instead call the parent method.

As per the Issue #2350939 on Drupal.org we can have Revision UI for Revisionable Entities.

Update Content Entity Generator class to include Revision UI annotations in Entity Class .

The CR has additional details for the syntaxes

Edit:

We might need to consider the changes that might be introduced in the issue #2976861 as well. Thanks.

While generating a content entity and creating a new module in the process there won't be a generated *.info.yml It seems to me this is because the command is extending the src/Command/ModuleGenerator.php Class, and the yml is created by the src/Command/Module.php class.

If I'm not mistaken we don't have this one yet: https://drupal.org/docs/drupal-apis/configuration-api/configuration-override-system#s-providing-overrides-from-modules.

Just logging my need for it. I do not have time to make a pull request right now.

For now, entity bundle classes generated by DCG 2 are placed in the namespace Drupal\mymodule\Entity\Bundle:

<?php

namespace Drupal\mymodule\Entity\Bundle;

use Drupal\node\Entity\Node;

/**
 * A bundle class for node entities.
 */
class ArticleBundle extends Node {
...
}

However, most documentation and blog posts I've seen for now favor putting them directly in the 'Entity' namespace. Here's an excellent example: https://www.hashbangcode.com/article/drupal-9-entity-bundle-classes.

This is highly opinionated, but IMHO DCG should follow established best practices, so I'd suggest changing the namespace.

Problem/Motivation

According to the Drupal 8 Backward Compatibility Policy, constructors for service objects are considered internal, which means they are subject to change on a minor release:

The constructor for a service object (one in the container), plugin, or controller is considered internal unless otherwise marked, and may change in a minor release. These are objects where developers are not expected to call the constructor directly in the first place. Constructors for value objects where a developer will be calling the constructor directly are excluded from this statement.

The implication is that if we override their constructors, and they happen to change their signature on a minor release, it would require us to update our current code for all instances where we're overriding their constructors, to conform to the new signature. This shouldn't have to happen to conform to a minor release.

There is a good blog post from jrockwitz (maintainer of Webform for D8), that goes into more detail: https://www.jrockowitz.com/blog/webform-road-to-drupal-9

Solution

We can implement a new design pattern that would fix the problem.

As an example, from the controller command, injecting 3 services as an example.

Before:

...
  public function __construct(
    RequestStack $request_stack,
    LoggerInterface $logger,
    Client $httpClient) {
    $this->request = $request_stack->getCurrentRequest();
    $this->logger = $logger;
    $this->httpClient = $httpClient;
  }

  public static function create(ContainerInterface $container) {
    return new static(
      $container->get('request_stack'),
      $container->get('logger.factory')->get('foo'),
      $container->get('http_client')
    );
  }
...

After:

...
  public static function create(ContainerInterface $container) {
    $instance = parent::create($container);
    $instance->request = $container->get('request_stack')->getCurrentRequest();
    $instance->logger = $container->get('logger.factory')->get('foo');
    $instance->httpClient = $container->get('http_client');
    return $instance;
  }
...

Some of the most used Drupal contrib modules are already using this design pattern, like the aforementioned Webform, Migrate Plus, Search API, etc.

Some extra benefits from this approach also include less code bloat:

• We no longer need a __construct() method, since the __construct() and create() methods are effectively merged into one.
• We no longer need a use statement per service injected. Their only purpose was so we could typehint on the constructor.

That reduces the final code lines by quite a bit. Also due to the above, adding a new service dependency injection after already running the generate command is no longer a major PITA.

I think this solution would include:

• A change in di.twig's container(services) macro that would use this new design pattern.
• A change to the generator twig template files, to use this new function, and also remove the now unnecessary use statements as well as the constructor.

I have a PR ready for these changes.

This solution could be applied to the following generators:

• [x] controller
• [x] block
• [x] views-argument-default
• [x] views-field