Page MenuHomePhabricator

Made some classes final.
ClosedPublic

Authored by joshuaspence on Feb 26 2014, 7:45 AM.
Tags
None
Referenced Files
F13206231: D8347.id19859.diff
Wed, May 15, 6:22 AM
F13194528: D8347.diff
Sun, May 12, 9:38 PM
F13177957: D8347.diff
Wed, May 8, 8:04 PM
Unknown Object (File)
Sun, Apr 28, 4:23 AM
Unknown Object (File)
Sun, Apr 28, 4:22 AM
Unknown Object (File)
Sun, Apr 28, 4:22 AM
Unknown Object (File)
Sat, Apr 27, 9:31 PM
Unknown Object (File)
Sat, Apr 27, 12:02 PM

Details

Reviewers
epriestley
Group Reviewers
Blessed Reviewers
Commits
Restricted Diffusion Commit
rP7f5b15c6faaf: Made some classes final.
Summary

Currently, the linter raises XHP29 warnings for these files because they are not abstract or final.

I guess there are two possibly solutions, either making the classes final or marking them as @concrete-extensible. Given that there are no subclasses of these classes in the phabricator, arcanist and libphutil repositories... I opted to declare the classes as final.

Test Plan

The following linter warnings are gone:

>>> Lint for src/aphront/configuration/AphrontDefaultApplicationConfiguration.php:


   Warning  (XHP29) Class Not abstract Or final
    This class is neither 'final' nor 'abstract', and does not have a
    docblock marking it '@concrete-extensible'.

               3 /**
               4  * @group aphront
               5  */
    >>>        6 class AphrontDefaultApplicationConfiguration
               7   extends AphrontApplicationConfiguration {
               8
               9   public function __construct() {

>>> Lint for src/applications/differential/mail/DifferentialReplyHandler.php:


   Warning  (XHP29) Class Not abstract Or final
    This class is neither 'final' nor 'abstract', and does not have a
    docblock marking it '@concrete-extensible'.

               1 <?php
               2
    >>>        3 class DifferentialReplyHandler extends PhabricatorMailReplyHandler {
               4
               5   private $receivedMail;
               6

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

The context here is:

  • Facebook subclasses both of these, and we don't want to break them.
  • But we don't really want anyone else subclassing these classes, since Facebook's integrations are really old and gross.
  • Eventually, we'll theoretically get Facebook to move off their subclasses and then we can final these.

I'll accept @concrete-extensible plus a "NOTE: Do not extend this!" in the comment or similar to deal with lint.

joshuaspence updated this revision to Unknown Object (????).Feb 26 2014, 8:56 PM
  • Used @concrete-extensible instead of final.