With very rare exceptions, classes in a phutil/Phabricator codebase should be abstract or final (probably, we should make this rule "always" instead of "with very rare exceptions", since I think all the exception cases turned out to be bad ideas and we no longer introduce them).

Ancient history on this is approximately T795, but my recollection is:

• Sometimes users took the absence of final to mean "I can extend this".
• T795 was originally going to mark things which could be extended as @stable but not use final, but we ended up just final-ing everything instead.
• Removing final is a clear "void the warranty" step that makes it obvious to users that they're in "fork" territory, not "the upstream is happy to support you" territory.
• Earlier we had some sub-rules around this (@stable, @concrete-extensible) but as the codebase evolved these became effectively unnecessary. The modern "modular" pattern (abstract base class, final subclasses, use runtime introspection to load them with PhutilClassMapQuery) hit its stride after these changes and proved much better than all the stuff that came before it, so we just use that everywhere now. We could eventually remove any vestigial traces of @stable or @concrete-extensible.

The existing ArcanistRaggedClassTreeEdgeXHPASTLinterRule in arcanist/ enforces these rules, but it is disabled by default. libphutil-specific rules like this are normally disabled by default so that the XHPAST linter's default settings are more reasonable for PHP in general.

The ArcanistPhutilXHPASTLinterStandard ("phutil.xhpast") ruleset reconfigures the linter's default settings to be appropriate for libphutil development, but doesn't enable this rule.

To fix this:

• Add this rule to ArcanistPhutilXHPASTLinterStandard->getLinterSeverityMap(), probably as a "WARNING" (we could make it an error after getting rid of @concrete-extensible completely, maybe) in arcanist/.

To test:

• Create a new change in Phabricator or libphutil which adds a class that is not abstract or final.
• Run arc lint.

The rule itself already has a unit test and I'm not sure trying to integration test the default behavior of arc lint is necessarily too useful (lots of work, issues like this are very rare) so I don't think we need any expansion of test coverage.