[1.0.x] DonorPerfect

Thank you for applying!

Please read Review process for security advisory coverage: What to expect for more details and Security advisory coverage application checklist to understand what reviewers look for. Tips for ensuring a smooth review gives some hints for a smoother review.

The important notes are the following.

• If you have not done it yet, you should run phpcs --standard=Drupal,DrupalPractice on the project, which alone fixes most of what reviewers would report.
• For the time this application is open, only your commits are allowed.
• The purpose of this application is giving you a new drupal.org role that allows you to opt projects into security advisory coverage, either projects you already created, or projects you will create. The project status won't be changed by this application and no other user will be able to opt projects into security advisory policy.
• We only accept an application per user. If you change your mind about the project to use for this application, or it is necessary to use a different project for the application, please update the issue summary with the link to the correct project and the issue title with the project name and the branch to review.

To the reviewers

Please read How to review security advisory coverage applications, Application workflow, What to cover in an application review, and Tools to use for reviews.

The important notes are the following.

• It is preferable to wait for a Code Review Administrator before commenting on newly created applications. Code Review Administrators will do some preliminary checks that are necessary before any change on the project files is suggested.
• Reviewers should show the output of a CLI tool only once per application.
• It may be best to have the applicant fix things before further review.

For new reviewers, I would also suggest to first read In which way the issue queue for coverage applications is different from other project queues.

1. Fix phpcs issues.

phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml donorperfect/

FILE: donorperfect/modules/gift/src/Form/GiftForm.php
-------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
-------------------------------------------------------------------------------
 51 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
-------------------------------------------------------------------------------

FILE: donorperfect/modules/other/src/Form/OtherForm.php
-------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
-------------------------------------------------------------------------------
 51 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
-------------------------------------------------------------------------------

FILE: donorperfect/modules/contact/src/Form/ContactForm.php
-------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
-------------------------------------------------------------------------------
 51 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
-------------------------------------------------------------------------------

FILE: donorperfect/modules/donor/src/Form/DonorForm.php
-------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
-------------------------------------------------------------------------------
 51 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
-------------------------------------------------------------------------------

FILE: donorperfect/modules/donor/src/Entity/DonorController.php
-------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-------------------------------------------------------------------------------
 71 | ERROR | The array declaration extends to column 97 (the limit is 80). The array content should be split up over multiple lines
-------------------------------------------------------------------------------

FILE: donorperfect/modules/donor/src/DonorNameAlphaFieldItemList.php
-------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-------------------------------------------------------------------------------
 10 | ERROR | Doc comment short description must be on a single line, further text should be a separate paragraph
-------------------------------------------------------------------------------

FILE: donorperfect/donorperfect.routing.yml
---------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
---------------------------------------------------------------------------
 32 | ERROR | [x] Expected 1 newline at end of file; 0 found
---------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
---------------------------------------------------------------------------

FILE: donorperfect/donorperfect.module
-------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
-------------------------------------------------------------------------------
 9 | WARNING | Format should be "* Implements hook_foo().", "* Implements hook_foo_BAR_ID_bar() for xyz_bar().",, "* Implements hook_foo_BAR_ID_bar() for
   |         | xyz-bar.html.twig.", "* Implements hook_foo_BAR_ID_bar() for xyz-bar.tpl.php.", or "* Implements hook_foo_BAR_ID_bar() for block templates."
-------------------------------------------------------------------------------

FILE: donorperfect/README.md
----------------------------------------------------------------------
FOUND 0 ERRORS AND 7 WARNINGS AFFECTING 7 LINES
----------------------------------------------------------------------
 18 | WARNING | Line exceeds 80 characters; contains 81 characters
 25 | WARNING | Line exceeds 80 characters; contains 83 characters
 37 | WARNING | Line exceeds 80 characters; contains 81 characters
 57 | WARNING | Line exceeds 80 characters; contains 81 characters
 58 | WARNING | Line exceeds 80 characters; contains 82 characters
 59 | WARNING | Line exceeds 80 characters; contains 81 characters
 70 | WARNING | Line exceeds 80 characters; contains 81 characters
----------------------------------------------------------------------

FILE: donorperfect/src/DPQuery.php
-------------------------------------------------------------------------------
FOUND 33 ERRORS AND 9 WARNINGS AFFECTING 42 LINES
-------------------------------------------------------------------------------
    9 | ERROR   | Doc comment short description must be on a single line, further text should be a separate paragraph
   64 | ERROR   | Doc comment short description must be on a single line, further text should be a separate paragraph
  103 | ERROR   | Doc comment short description must be on a single line, further text should be a separate paragraph
  118 | ERROR   | Doc comment short description must be on a single line, further text should be a separate paragraph
  126 | ERROR   | Doc comment short description must be on a single line, further text should be a separate paragraph
  169 | ERROR   | Doc comment short description must be on a single line, further text should be a separate paragraph
  226 | ERROR   | Doc comment short description must be on a single line, further text should be a separate paragraph
  287 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
  323 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
  329 | ERROR   | Doc comment short description must be on a single line, further text should be a separate paragraph
  376 | ERROR   | The array declaration extends to column 97 (the limit is 80). The array content should be split up over multiple lines
  409 | ERROR   | The array declaration extends to column 100 (the limit is 80). The array content should be split up over multiple lines
  443 | ERROR   | Doc comment short description must be on a single line, further text should be a separate paragraph
  526 | ERROR   | Doc comment short description must be on a single line, further text should be a separate paragraph
  545 | ERROR   | Doc comment short description must be on a single line, further text should be a separate paragraph
  564 | ERROR   | Doc comment short description must be on a single line, further text should be a separate paragraph
  641 | ERROR   | Doc comment short description must be on a single line, further text should be a separate paragraph
  723 | ERROR   | Doc comment short description must be on a single line, further text should be a separate paragraph
  744 | ERROR   | Doc comment short description must be on a single line, further text should be a separate paragraph
  970 | ERROR   | Doc comment short description must be on a single line, further text should be a separate paragraph
 1060 | ERROR   | Doc comment short description must be on a single line, further text should be a separate paragraph
 1142 | ERROR   | Doc comment short description must be on a single line, further text should be a separate paragraph
 1186 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
 1239 | ERROR   | Doc comment short description must be on a single line, further text should be a separate paragraph
 1618 | ERROR   | Doc comment short description must be on a single line, further text should be a separate paragraph
 1791 | ERROR   | Doc comment short description must be on a single line, further text should be a separate paragraph
 1847 | ERROR   | Doc comment short description must be on a single line, further text should be a separate paragraph
 1953 | ERROR   | Doc comment short description must be on a single line, further text should be a separate paragraph
 1969 | ERROR   | Doc comment short description must be on a single line, further text should be a separate paragraph
 1985 | ERROR   | Doc comment short description must be on a single line, further text should be a separate paragraph
 2017 | ERROR   | Doc comment short description must be on a single line, further text should be a separate paragraph
 2027 | ERROR   | Doc comment short description must be on a single line, further text should be a separate paragraph
 2030 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
 2031 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
 2125 | ERROR   | The array declaration extends to column 90 (the limit is 80). The array content should be split up over multiple lines
 2213 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
 2245 | ERROR   | The array declaration extends to column 102 (the limit is 80). The array content should be split up over multiple lines
 2302 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
 2310 | ERROR   | Doc comment short description must be on a single line, further text should be a separate paragraph
 2343 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
 2351 | ERROR   | Doc comment short description must be on a single line, further text should be a separate paragraph
 2368 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
-------------------------------------------------------------------------------

FILE: donorperfect/src/Plugin/views/query/DonorPerfect.php
-------------------------------------------------------------------------------
FOUND 7 ERRORS AFFECTING 7 LINES
-------------------------------------------------------------------------------
  34 | ERROR | Doc comment short description must be on a single line, further text should be a separate paragraph
  84 | ERROR | Doc comment short description must be on a single line, further text should be a separate paragraph
 142 | ERROR | Doc comment short description must be on a single line, further text should be a separate paragraph
 216 | ERROR | Doc comment short description must be on a single line, further text should be a separate paragraph
 244 | ERROR | Doc comment short description must be on a single line, further text should be a separate paragraph
 258 | ERROR | The array declaration extends to column 85 (the limit is 80). The array content should be split up over multiple lines
 334 | ERROR | Doc comment short description must be on a single line, further text should be a separate paragraph
-------------------------------------------------------------------------------

FILE: donorperfect/src/Form/EntitySettingsFormBase.php
-------------------------------------------------------------------------------
FOUND 0 ERRORS AND 3 WARNINGS AFFECTING 3 LINES
-------------------------------------------------------------------------------
  50 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
 126 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
 127 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
-------------------------------------------------------------------------------

FILE: donorperfect/src/Form/SettingsForm.php
-------------------------------------------------------------------------------
FOUND 3 ERRORS AND 1 WARNING AFFECTING 1 LINE
-------------------------------------------------------------------------------
 82 | ERROR   | The array declaration extends to column 108 (the limit is 80). The array content should be split up over multiple lines
 82 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
 82 | ERROR   | The array declaration extends to column 107 (the limit is 80). The array content should be split up over multiple lines
 82 | ERROR   | The array declaration extends to column 106 (the limit is 80). The array content should be split up over multiple lines
-------------------------------------------------------------------------------

FILE: donorperfect/src/Entity/Query/QueryAggregate.php
-------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-------------------------------------------------------------------------------
 108 | ERROR | The array declaration extends to column 110 (the limit is 80). The array content should be split up over multiple lines
-------------------------------------------------------------------------------

FILE: donorperfect/src/Entity/ControllerInterface.php
-------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-------------------------------------------------------------------------------
 14 | ERROR | Doc comment short description must be on a single line, further text should be a separate paragraph
-------------------------------------------------------------------------------

FILE: donorperfect/src/Entity/ControllerServiceInterface.php
-------------------------------------------------------------------------------
FOUND 6 ERRORS AFFECTING 6 LINES
-------------------------------------------------------------------------------
 24 | ERROR | Doc comment short description must be on a single line, further text should be a separate paragraph
 33 | ERROR | Doc comment short description must be on a single line, further text should be a separate paragraph
 42 | ERROR | Doc comment short description must be on a single line, further text should be a separate paragraph
 56 | ERROR | Doc comment short description must be on a single line, further text should be a separate paragraph
 67 | ERROR | Doc comment short description must be on a single line, further text should be a separate paragraph
 78 | ERROR | Doc comment short description must be on a single line, further text should be a separate paragraph
-------------------------------------------------------------------------------

FILE: donorperfect/src/Entity/ControllerService.php
-------------------------------------------------------------------------------
FOUND 1 ERROR AND 1 WARNING AFFECTING 2 LINES
-------------------------------------------------------------------------------
 30 | ERROR   | Doc comment short description must be on a single line, further text should be a separate paragraph
 67 | WARNING | \Drupal calls should be avoided in classes, use dependency injection instead
-------------------------------------------------------------------------------

FILE: donorperfect/src/Entity/DefaultTableMapping.php
-------------------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
-------------------------------------------------------------------------------
  76 | ERROR | The array declaration extends to column 107 (the limit is 80). The array content should be split up over multiple lines
 146 | ERROR | The array declaration extends to column 81 (the limit is 80). The array content should be split up over multiple lines
-------------------------------------------------------------------------------

FILE: donorperfect/src/DPUtility.php
-------------------------------------------------------------------------------
FOUND 4 ERRORS AFFECTING 4 LINES
-------------------------------------------------------------------------------
 194 | ERROR | Doc comment short description must be on a single line, further text should be a separate paragraph
 245 | ERROR | Doc comment short description must be on a single line, further text should be a separate paragraph
 287 | ERROR | Doc comment short description must be on a single line, further text should be a separate paragraph
 418 | ERROR | The array declaration extends to column 94 (the limit is 80). The array content should be split up over multiple lines
-------------------------------------------------------------------------------

FILE: donorperfect/src/Element/Name.php
-------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-------------------------------------------------------------------------------
 174 | ERROR | The array declaration extends to column 104 (the limit is 80). The array content should be split up over multiple lines
-------------------------------------------------------------------------------

FILE: donorperfect/src/Element/Email.php
-------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-------------------------------------------------------------------------------
 71 | ERROR | The array declaration extends to column 106 (the limit is 80). The array content should be split up over multiple lines
-------------------------------------------------------------------------------

FILE: donorperfect/src/Controller/DonorPerfectController.php
-------------------------------------------------------------------------------
FOUND 4 ERRORS AND 1 WARNING AFFECTING 5 LINES
-------------------------------------------------------------------------------
  55 | WARNING | [ ] \Drupal calls should be avoided in classes, use dependency injection instead
  84 | ERROR   | [x] Namespaced classes/interfaces/traits should be referenced with use statements
  95 | ERROR   | [ ] The array declaration extends to column 89 (the limit is 80). The array content should be split up over multiple lines
 126 | ERROR   | [ ] The array declaration extends to column 92 (the limit is 80). The array content should be split up over multiple lines
 145 | ERROR   | [ ] The array declaration extends to column 93 (the limit is 80). The array content should be split up over multiple lines
-------------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-------------------------------------------------------------------------------

FILE: donorperfect/donorperfect.api.php
-------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
-------------------------------------------------------------------------------
 29 | ERROR | Doc comment short description must be on a single line, further text should be a separate paragraph
-------------------------------------------------------------------------------

2. FILE: donorperfect.module

/**
 * @file
 * Code for the DonorPerfect module.
 */

The usual description for a .module file is Hook implementations for the [module name] module. where [module name] is the module name given in the .info.yml file.

Rest looks fine to me.

Let’s wait for a Code Review Administrator to take a look and if everything goes fine, you will get the role.

There are changes that need to be done. I will make a review in the next 30 minutes.

• The following points are just a start and don't necessarily encompass all of the changes that may be necessary
• A specific point may just be an example and may apply in other places
• A review is about code that doesn't follow the coding standards, contains possible security issue, or does not correctly use the Drupal API; the single points are not ordered, not even by importance

src/Controller/ControllerBase.php

The parent class already defines container() and it is not necessary to define configFactory(), since there is already config() in the parent class.

src/Controller/DonorPerfectController.php

      $request = $this->container()->get('request_stack')->getCurrentRequest();
      foreach (['element_token', 'donor_type', 'last_name', 'first_name'] as $request_key) {
        $request_values[$request_key] = $request->get($request_key);
      }

Dependencies must be added using the create() method, like done from the AdminController class, the ContactController class, or the ShortcutController class, which all extend the ControllerBase class.

      $result_content[] = ['#markup' => Markup::create('<h3>Possible Matches in DonorPerfect</h3>')];
      $caption = (count($donors) == 1) ? '1 possible match' : count($donors) . ' possible matches';

The Markup class is marked @internal. It must not be used by contributed modules, and there should not be any reason to use it, since Drupal core allows to use a render array or a string containing markup.
Strings shown in the user interface must be translatable. Drupal core has also a class/method for strings that depend on a dynamic value (a string when the value is singular; a string when the value is plural).

$link = Markup::create('<a href="#" result-data=' . str_replace(' ', '&nbsp;', Json::encode($donor_data)) . '>' . $this->t('Match') . '</a>');

There is a class to generate links.

src/Entity/Storage.php

        if ($field_name === 'flag') {
          $sql = "SELECT donor_id AS matching_id, flag AS code from dpflags WHERE donor_id IN (" . implode(',', $ids) . ")";
        }
        else {
          $sql = "SELECT matching_id, code FROM dpusermultivalues WHERE (matching_id IN (" . implode(',', $ids) . ")) AND (field_name='" . strtoupper($field_name) . "')";
        }

SQL queries are not built concatenating strings. Placeholders must be used.

src/Entity/Query/Condition.php

Drupal\Core\Entity\Query\Sql\Condition is not part of the public API and cannot be extended from contributed modules. Drupal\Core\Entity\Query\ConditionBase is a base class and can be extended.

src/Entity/Query/QueryFactory.php

The Drupal\Core\Entity\Query\Sql\QueryFactory is not part of the public API too. In this case, it does not even make sense to extend a class and not change any defined method.

src/DPQuery.php

ControllerBase is a class for controllers. It cannot be used for a generic class that is not used for controllers.

src/DPUtility.php

If a class uses \Drupal's methods, then it should probably be converted to a service class.

No, the absence of the @internal tag does not mean the class is public API.
Although I have summarized what can be considered public API (what is marked with @api or base classes), that is essentially correct.
In fact, not even the public or protected class properties are public API. In the latter case, the exception is protected properties on a base class marked with @api, or properties that are accessed with __get() or __set() (for the classes that implement those magic methods). That means, for example that I could use $object['name_of_a_property_that_really_is_protected'], where $object is an instance of a class implementing those magic methods, without to worry in the case the code returns the value of a protected property.

The only method that Drupal 10 marks with @api is ConfigEntityUpdater::update(). That does not mean that what reported about classes/interfaces/methods marked with @api is not true.

@brandonpost
https://www.drupal.org/about/core/policies/core-change-policies/bc-policy Is the backend “API” policy.

Short version: very little of Drupal’s code is actually considered API, a significant portion of code is @internal by default. Even for code(interfaces) that is API in one scenario(caller) may not be API in another scenario (implementer).

I believe the last caveat of that statement is where my module falls, and I am prepared to update my code if anything changes within the core classes I have extended.

Except that the quoted part applies when nothing else in the page applies.
If only interfaces can be relied on as the public API, then I could not access a protected property via __get(), nor set it via __set(), since those methods are not defined in an interface, contrary to what the following sentence says.

Public properties on a class are always considered @internal. This does not include the use of __get() and __set() to provide access to protected properties.

The part you quote does not even limit what said in other parts of the page. It is not that controller classes can be used as parent class only when they implement an interface; they cannot used as parent class in any case. Similarly, it is not that classes that implement a plugin can be used as parent class when they implement an interface; they cannot be used as parent class in any case.

If the PHP and JavaScript classes part were always applicable, other parts in that page should not be present.

• Protected properties: Interfaces do not define properties; there is no need to say they are internal API because that is covered from only interfaces can be relied on as the public API
• Public properties: Interfaces do not define properties; there is no need to say they are internal API for the same reason.
• Public methods not specified in an interface: Those methods are not covered by an interface; there is no need to say they are internal API when it is already said I cannot rely on them.

The reason I extended the classes I did is to avoid a lot of code duplication.

The class in src/Controller/ControllerBase.php contained code that was not necessary; the class in src/Controller/DonorPerfectController.php still contains code that is not necessary, especially because it defines a property that is already defined from the parent class. That is duplication too.

Apart from the parent classes, there are other changes that have not been done.

I quoted the # PHP and JavaScript Classes section from the documentation. In comment #14, you seem to focus on the first line of that section which says, "In general, only interfaces can be relied on as the public API."

No, I focus on the fact that part applies when other parts do not apply. Otherwise, that full documentation page would be much shorter: It would need to just contain the PHP and JavaScript Classes section, instead of having all those parts about classes, interfaces, and properties.

Since the purpose of that section does not seem clear, let me ask this: Are there classes implemented by Drupal core or Drupal core modules, which are not usually used as parent class from classes implemented by contributed modules, for which the constructor is not called (not even from Drupal core), and for which none of the properties are used by a contributed module?

I believe you are aware that this file no longer exists, but it seems you are mentioning it to make a point that I have duplicated code

If the aim is not duplicating code, creating a class just to duplicate a method should not be done. Yes, ControllerBase::container() is private, but that does not mean that overring that method just to make it protected or public is not duplicating code.
Furthermore, the reason for which that method is private is documented in that method.

This method is marked private to prevent sub-classes from retrieving services from the container through it. Instead, \Drupal\Core\DependencyInjection\ContainerInjectionInterface should be used for injecting services.

That is exactly the opposite of what DonorPerfectController did. I have reported that in comment #8. I reviewed the code further two times, to discover there are still changes that need to be done for that class.

(Hint: Needs review is not for letting reviewers answer any question applicants have; on this queue, that status means I have changed the code as requested and the project is ready for another review.)

src/Entity/ControllerService.php

Services do not need a property containing the dependency container: Each dependency a service has is defined in the service definition. The class is also using the container for the wrong purpose: Forms are instantiated using the form_builder service.

src/Entity/Storage.php

      $entity = new $entity_class($entity_values, $this->entityTypeId);
      $entities[$entity_id] = $entity;

Entity objects are created calling the create() method.

  /**
   * Gets the DPQuery service.
   *
   * Using \Drupal::service() here, following the example set in this class's
   * parent class \Drupal\Core\Entity\EntityHandlerBase::moduleHandler().
   *
   * @return \Drupal\donorperfect\DPQuery
   *   The DPQuery service.
   */
  protected function dpQuery() {
    if (!$this->dpQuery) {
      $this->dpQuery = \Drupal::service('donorperfect.dpquery');
    }
    return $this->dpQuery;
  }

As a side note, it is correct to use \Drupal methods in that case, but not because Drupal\Core\Entity\EntityHandlerBase::moduleHandler() does it. It is because classes that implement Drupal\Core\Entity\EntityStorageInterface cannot also implement Drupal\Core\DependencyInjection\ContainerInjectionInterface. The only way they have to get dependency injected is using \Drupal methods.

src/Form/EntitySettingsFormBase.php

$entity_type_label = ucfirst($this->getEntityType()->getSingularLabel());

For a label that starts with a capital letter, there is getLabel(). getSingularLabel() takes the value returned from getLabel(), passes it to mb_strtolower(), and then this code passes that value to ucfirst(). It is not necessary, since getLabel() returns strings like 'Content', 'User', 'Taxonomy term', or 'Taxonomy vocabulary' (just to make a few examples with Drupal core entities).

src/FormValidator.php

  public static function filterCheckPlain(string $input) {
    if (!empty($string)) {
      return Html::escape($input);
    }
    return '';
  }

Just to point out that method will always return an empty string.

No, I focus on the fact that part applies when other parts do not apply. Otherwise, that full documentation page would be much shorter: It would need to just contain the PHP and JavaScript Classes section, instead of having all those parts about classes, interfaces, and properties.

Respectfully I disagree, the section must be used in context of the other sections, since the entire page is about PHP classes this section acts as a general clarification that is binding over the entire page unless explicitly revoked.

Addtionaly the majority of that page is actually clarification for Core Committers (and core contributors) to understand the BC policy rather than direction regarding the use of Drupal's methods.

There are many should not, and no must not phrases listed in the BC policy page.

OOP standards indicate that if the Drupal Core Team is to list as a class to never be extended it MUST be final and for a property to never be accessed it must be private. Lacking final/private the it may not be recommended to do so however the extender is free to accept that risk.

The Drupal Core team has a history of not using final or private in order to allow code to be extended even if it is off API (see SLACK #contribute there were recent discussions on this regarding final and Access Policies). Furthermore the Drupal Core Team routinely and intentionally goes out of its way to maintain BC with non API classes, methods and properties (to the detriment of Cores code base) under the assumption that non API classes will be used directly.

Extending a non-final class is not on its own a sign of insecure programming or misuse of the Drupal API, only that the program has accepted additional support burden.

More importantly, IIRC this queue has approved applications extending non API classes in the past year.

That said to the applicant:
My suggestion is to not extend core classes, and do not use ServiceContollerBase. Extending classes will just make life more painful and tie your code to specific patch releases of core while ServiceControllerBase breaches the standard of Dependency Injection. Both of these make your code brittle and harder to test. Code duplication is not always bad.

I have not reviewed the code in detail, this message is solely in response to @apaderno's assertions regarding the BC policy and its impact on Code Review.

I am changing priority as per Issue priorities.

Respectfully I disagree, the section must be used in context of the other sections

Exactly, but that means that if a section says that the parent class can only be a base class, what follows must be read considering that part.

As for that policy, it clearly states:

This document defines what parts of the Drupal code base are considered stable and reliable APIs, and which parts module and theme developers should not rely on.

Reviewing for the correct Drupal API usage, I consider using as parent class a class that is not a base class a not correct usage of the Drupal API.

I also asked a question in my previous comment, but that has not been answered from the applicant.

Since the purpose of that section does not seem clear, let me ask this: Are there classes implemented by Drupal core or Drupal core modules, which are not usually used as parent class from classes implemented by contributed modules, for which the constructor is not called (not even from Drupal core), and for which none of the properties are used by a contributed module?

More importantly, IIRC this queue has approved applications extending non API classes in the past year.

The only time I recall I did was because Drupal core did not define any base class that should be extended. That is not the case here.

The \Drupal class, like the \Drupal\Component\Utility\Byte, \Drupal\Component\Utility\Color, \Drupal\Component\Utility\Crypt, \Drupal\Component\Utility\DiffArray, \Drupal\Component\Utility\Environment, and the \Drupal\Component\Utility\Html classes, just to list some of them, defines only static methods. It means there is a set of classes for which the following text applies and applies only to them.

In general, only interfaces can be relied on as the public API.
With the exception of base classes, developers should generally assume that implementations of classes will change, and that they should not inherit from classes in the system, or be prepared to update code for changes if they do.

Those classes do not implement interfaces, and they are not base classes. Basing on the rest of Drupal core backend backwards compatibility and internal API policy (Drupal 8 and above), those classes should not be used. The quoted text is saying Maintainers can use them, but be prepared to update their code because of changes in those classes.

As a side note, Drupal 12 will rename \Drupal\Core\Render\Element\FormElement to \Drupal\Core\Render\Element\FormElementBase making clear it is a base class.
(See also RenderElement and FormElement base plugin classes are deprecated and renamed to RenderElementBase and FormElementBase.)