Page MenuHomePhabricator

Made some classes final.
ClosedPublic

Authored by joshuaspence on Feb 26 2014, 7:45 AM.
Tags
None
Referenced Files
F18959130: D8347.id19858.diff
Thu, Nov 13, 6:08 AM
F18959128: D8347.id19844.diff
Thu, Nov 13, 6:08 AM
F18944967: D8347.id19859.diff
Tue, Nov 11, 5:25 PM
F18906616: D8347.id.diff
Sat, Nov 8, 7:22 PM
F18901506: D8347.diff
Fri, Nov 7, 10:19 PM
F18808537: D8347.diff
Oct 19 2025, 7:02 AM
F18743141: D8347.diff
Oct 2 2025, 10:31 PM
F18627239: D8347.diff
Sep 16 2025, 3:32 AM

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.