Rename PasswordEnhancementsExceptionInterface to PasswordEnhancementsException

[ycecube]

It was suggested by the PhpStorm to add the Interface for the name of the interface.

Also it is in the Drupal's coding standard that interfaces should have the suffix "Interface": https://www.drupal.org/docs/develop/standards/object-oriented-code#naming

[mxr576]

Yes, but again, do not believe everything to PHPstorm and in this case, it makes more sense to call and catch PasswordEnhancementsException instead of PasswordEnhancementsExceptionInterface. It is like \Throwable and phpcs does not complain about this.

[ycecube]

But still, the Drupal coding standard requires this.

[mxr576]

If it would require it then PHPCS would complain about it :) Also, Drupal 8 does not leverage PHP 7 features yet, because exceptions can be thrown by interface only in PHP 7.

[ycecube]

You mean catch by interface? You cannot instantiate an interface that should be thrown. In this case it is still makes sense to call it interface, and also because Drupal CS requires it, I wouldn't go against it, because otherwise we could take lightly every CS in the documentation, let's keep it consistent.

[mxr576]

You mean catch by interface? You cannot instantiate an interface that should be thrown.

<?php
interface Foo extends \Throwable {
}

class Bar extends \RuntimeException implements Foo {
}

class Command {
  public static function baz() {
      throw new Bar(...)
  } 
}

try {
   Command::baz();
} catch (Foo $e) {
   // Catch all exceptions that implements Foo.
}

You still would like to catch (FooInterface $ex)? :)

[mxr576]

otherwise we could take lightly every CS in the documentation, let's keep it consistent.

I'd do not want to take this lightly but PHPCS does not cover some PHP 7 features. I do not think Drupal core maintainers would like to the call an exception base interface as FoobarExceptionInterface or FooBarInterface...

[ycecube]

Well, you could look at it from another view, so if it is an interface called FooInterface and you are catching the exception like the following:

<?php
try {
  do_somthing();
}
catch (FooInterface $e) {
  handle_exception($e);
}

Then you could say that it is catching an exception which has a type of FooInterface or implements the FooInterface, so by doing this you won't confuse the developer either because he/she will know that is an interface and it could be implemented by multiple exceptions thus it may need further check.

But if you check the PSR-11 for example (https://www.php-fig.org/psr/psr-11/#12-exceptions) it is also suffixes the exception interface with the "Interface" word, because it is what it is.

[mxr576]

I think this is something that we should vote again... :D

Btw:

• https://github.com/php-fig/container/issues/13
• https://github.com/php-fig/container/issues/21
• https://github.com/php-fig/simple-cache/blob/1.0.1/src/CacheException.php

IOW, PSR supposed to be a good thing, but it is inconsistent 🙄

[ycecube]

Well, they are clearly state here that all interfaces must be suffixed: https://www.php-fig.org/bylaws/psr-naming-conventions/

[mxr576]

Yes, this is a naming convention for PSR libraries which it seems they clearly fail to apply in some cases.

[ycecube]

So what should be then? Even if PSR examples using it wrong it is an accepted ruleset, and Drupal CS as well, both states that all interfaces should be suffixed by Interface.

In this case I would still leave it as it is, until it changes, because if we won't follow any ruleset because it looks uncomfortable (based on our personal opinion) then we would end up in chaos :)

Although to be honest I don't care how it is called, I just want to be consistent based on the used rulesets, and keep ourselfs to that, it's also easier to point anyone to these than explaining why we have used LittleCatException as the base exception for our module.

[mxr576]

ok, let's have them suffixed with "Interface".