Page MenuHomePhabricator

Made some classes final.
ClosedPublic

Authored by joshuaspence on Feb 26 2014, 7:45 AM.
Tags
None
Referenced Files
F18808537: D8347.diff
Sun, Oct 19, 7:02 AM
F18743141: D8347.diff
Thu, Oct 2, 10:31 PM
F18627239: D8347.diff
Sep 16 2025, 3:32 AM
F18616134: D8347.id.diff
Sep 14 2025, 4:23 PM
F18604263: D8347.diff
Sep 13 2025, 6:23 PM
F18585316: D8347.id.diff
Sep 11 2025, 2:57 PM
F17854846: D8347.id.diff
Jul 27 2025, 3:38 PM
F17853989: D8347.id19858.diff
Jul 27 2025, 2:34 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.