Add ClassMustBeFinal issue

bin/docs/generate_issues_list_doc.php

@@ -3,7 +3,7 @@
 
 declare(strict_types=1);
 
-$docs_dir = dirname(__DIR__) . DIRECTORY_SEPARATOR . "docs"
+$docs_dir = dirname(__DIR__, 2) . DIRECTORY_SEPARATOR . "docs"
     . DIRECTORY_SEPARATOR . "running_psalm" . DIRECTORY_SEPARATOR;
 $issues_index = "{$docs_dir}issues.md";
 $issues_dir = "{$docs_dir}issues";

bin/docs/generate_levels_doc.php

@@ -5,7 +5,7 @@
 use Psalm\Config\IssueHandler;
 use Psalm\Issue\CodeIssue;
 
-require_once(dirname(__DIR__) . '/vendor/autoload.php');
+require_once(dirname(__DIR__, 2) . '/vendor/autoload.php');
 
 $issue_types = IssueHandler::getAllIssueTypes();
 
@@ -60,7 +60,7 @@
     $result .= ' - [' . $issue_type . '](issues/' . $issue_type . '.md)' . "\n";
 }
 
-$f = dirname(__DIR__).'/docs/running_psalm/error_levels.md';
+$f = dirname(__DIR__, 2).'/docs/running_psalm/error_levels.md';
 $content = file_get_contents($f);
 
 $content = explode('<!-- begin list -->', $content)[0].$result;

bin/docs/max_used_shortcode.php

@@ -5,7 +5,7 @@
 
 use Psalm\Config\IssueHandler;
 
-require_once(dirname(__DIR__) . '/vendor/autoload.php');
+require_once(dirname(__DIR__, 2) . '/vendor/autoload.php');
 
 $issue_types = IssueHandler::getAllIssueTypes();
 

bin/stubs/gen_base_callmap.php

@@ -116,6 +116,6 @@ function paramsToEntries(ReflectionFunctionAbstract $reflectionFunction, string
 $payload = '<?php // phpcs:ignoreFile
 
 return '.var_export($callmap, true).';';
-$f = __DIR__.'/../dictionaries/autogen/CallMap_'.PHP_MAJOR_VERSION.PHP_MINOR_VERSION.'.php';
+$f = __DIR__.'/../../dictionaries/autogen/CallMap_'.PHP_MAJOR_VERSION.PHP_MINOR_VERSION.'.php';
 
 file_put_contents($f, $payload);

bin/stubs/gen_callmap.php

@@ -22,7 +22,7 @@
 
 // Load+normalize autogenerated maps
 $baseMaps = [];
-foreach (glob(__DIR__."/../dictionaries/autogen/CallMap_*.php") as $file) {
+foreach (glob(__DIR__."/../../dictionaries/autogen/CallMap_*.php") as $file) {
     Assert::eq(preg_match('/_(\d+)\.php/', $file, $matches), 1);
     $version = $matches[1];
 
@@ -97,11 +97,11 @@
 
 // Load+normalize hand-written diff maps
 $customMaps = [
-    84 => require __DIR__."/../dictionaries/override/CallMap.php",
+    84 => require __DIR__."/../../dictionaries/override/CallMap.php",
 ];
 
 $customDiffs = [];
-foreach (glob(__DIR__."/../dictionaries/override/CallMap_*.php") as $file) {
+foreach (glob(__DIR__."/../../dictionaries/override/CallMap_*.php") as $file) {
     if (!preg_match('/_(\d+)_delta\.php/', $file, $matches)) {
         continue;
     }
@@ -202,11 +202,11 @@
 // Merge hand-written full maps into autogenerated full maps, write to files
 foreach ($customMaps as $version => $data) {
     $data = normalizeCallMap(array_replace($baseMaps[$version] ?? [], $data));
-    writeCallMap(__DIR__."/../dictionaries/CallMap_$version.php", $data);
+    writeCallMap(__DIR__."/../../dictionaries/CallMap_$version.php", $data);
 }
 
 // Overwrite custom diff maps
-writeCallMap(__DIR__."/../dictionaries/override/CallMap.php", $customMaps[$last]);
+writeCallMap(__DIR__."/../../dictionaries/override/CallMap.php", $customMaps[$last]);
 
 // Regenerate diff maps from custom maps
 foreach ($customMaps as $prevVersion => $prevData) {
@@ -233,7 +233,7 @@
         }
     }
 
-    writeCallMap(__DIR__."/../dictionaries/override/CallMap_{$nextVersion}_delta.php", normalizeCallMap($diff));
+    writeCallMap(__DIR__."/../../dictionaries/override/CallMap_{$nextVersion}_delta.php", normalizeCallMap($diff));
 
     $nextVersion = $prevVersion;
     $nextData = $prevData;

bin/stubs/gen_callmap.sh

@@ -10,8 +10,8 @@ done
 wait
 
 for f in $VERSIONS; do
-    docker run --rm -it -v $PWD:/app psalm_test_$f php /app/bin/gen_base_callmap.php
+    docker run --rm -it -v $PWD:/app psalm_test_$f php /app/bin/stubs/gen_base_callmap.php
 done
 
-php bin/gen_callmap.php
-php bin/gen_callmap.php
+php bin/stubs/gen_callmap.php
+php bin/stubs/gen_callmap.php

bin/stubs/update-property-map.php

@@ -33,7 +33,7 @@
 
 $stubbedClasses = [];
 foreach (new RecursiveDirectoryIterator(
-    __DIR__ . '/../stubs',
+    __DIR__ . '/../../stubs',
     FilesystemIterator::CURRENT_AS_PATHNAME|FilesystemIterator::SKIP_DOTS,
 ) as $file) {
     if (is_dir($file)) {
@@ -47,7 +47,7 @@
 }
 unset($file, $contents, $stmts);
 
-$docDir = realpath(__DIR__ . '/../build/doc-en');
+$docDir = realpath(__DIR__ . '/../../build/doc-en');
 
 if (false === $docDir) {
     echo 'PHP doc not found!' . PHP_EOL;

config.xsd

@@ -219,6 +219,7 @@
             <xs:element name="AssignmentToVoid" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="CheckType" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="CircularReference" type="IssueHandlerType" minOccurs="0" />
+            <xs:element name="ClassMustBeFinal" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="ComplexFunction" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="ComplexMethod" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="ConfigIssue" type="IssueHandlerType" minOccurs="0" />

docs/running_psalm/error_levels.md

@@ -136,6 +136,7 @@ Level 5 and above allows a more non-verifiable code, and higher levels are even
 
 These issues are treated as errors at level 2 and below.
 
+ - [ClassMustBeFinal](issues/ClassMustBeFinal.md)
  - [DeprecatedClass](issues/DeprecatedClass.md)
  - [DeprecatedConstant](issues/DeprecatedConstant.md)
  - [DeprecatedFunction](issues/DeprecatedFunction.md)

docs/running_psalm/issues.md

@@ -7,6 +7,7 @@
  - [AssignmentToVoid](issues/AssignmentToVoid.md)
  - [CheckType](issues/CheckType.md)
  - [CircularReference](issues/CircularReference.md)
+ - [ClassMustBeFinal](issues/ClassMustBeFinal.md)
  - [ComplexFunction](issues/ComplexFunction.md)
  - [ComplexMethod](issues/ComplexMethod.md)
  - [ConfigIssue](issues/ConfigIssue.md)

docs/running_psalm/issues/ClassMustBeFinal.md

@@ -0,0 +1,48 @@
+# ClassMustBeFinal
+
+Emitted when a non-final, non-abstract class with no child classes is found.  
+
+```php
+<?php
+
+class A {}
+```
+
+## Why this is bad
+
+Non-final classes are bad for multiple reasons:
+
+- They allow overriding non-final methods and properties, which can lead to unexpected behavior and bugs if implementation details are changed during inheritance of classes not explicitly part of the public API (marked using the `@api` attribute).  
+- A corollary of the above is that non-final classes increase the amount of code that must be covered by any backwards-compatibility promise.  
+  - In final classes, only public functions/properties/constants must be covered by the backwards compatibility promise.  
+  - In non-final classes, all private, protected and public functions/properties/constants must be covered by the backwards compatibility promise (private methods too, because changes to their code may not be compatible with overridden protected/public methods).
+- They are not optimized by Opcache and PHP itself, and thus are more expensive to use at runtime.  
+- Psalm type inference is more complex and not as exact for non-`final` classes.  
+
+In general, the number of non-final classes in the codebase should be reduced as much as possible, both to speed up code execution and avoid unexpected bugs.  
+
+## How to fix
+
+Recommended, make the class `final`:    
+
+```php
+<?php
+
+final class A {}
+```
+
+The above can also be automated using `vendor/bin/psalm --alter --issues=ClassMustBeFinal`.  
+
+If inheritance should still be allowed, reduce the surface covered by the backwards compatibility promise by making the class abstract (containing **only** the logic that should be overridable), and move any non-overridable logic to a new `A` class:
+
+```php
+<?php
+
+abstract class A {}
+
+final class NewA extends A {}
+```
+
+**Note**: if non-`final` classes are needed for mocking in unit tests, simply use [dg/bypass-finals](https://packagist.org/packages/dg/bypass-finals) in your unit tests to allow mocking `final` classes.  
+
+An alternative, not recommended for the [above reasons](#why-this-is-bad), is to make the class part of the public API of your library with `@api`.  

docs/running_psalm/issues/UnusedClass.md

@@ -8,7 +8,7 @@ If this class is used and part of the public API, annotate it with `@psalm-api`.
 ```php
 <?php
 
-class A {}
-class B {}
+final class A {}
+final class B {}
 $a = new A();
 ```

psalm-baseline.xml

@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<files psalm-version="6.x-dev@d5ee11cc5a050760e4ac0b0801f43c3780a66fda">
+<files psalm-version="dev-master@d577f78e60d7aff42e42af7825c0c978b094a558">
   <file src="examples/TemplateChecker.php">
     <PossiblyUndefinedIntArrayOffset>
       <code><![CDATA[$comment_block->tags['variablesfrom'][0]]]></code>
@@ -66,6 +66,7 @@
     </PossiblyUnusedMethod>
     <PropertyTypeCoercion>
       <code><![CDATA[$this]]></code>
+      <code><![CDATA[$this]]></code>
     </PropertyTypeCoercion>
     <RiskyTruthyFalsyComparison>
       <code><![CDATA[!$composer_json]]></code>

src/Psalm/Internal/Analyzer/ProjectAnalyzer.php

@@ -22,6 +22,7 @@
 use Psalm\Internal\Provider\ProjectCacheProvider;
 use Psalm\Internal\Provider\Providers;
 use Psalm\Internal\Provider\StatementsProvider;
+use Psalm\Issue\ClassMustBeFinal;
 use Psalm\Issue\CodeIssue;
 use Psalm\Issue\InvalidFalsableReturnType;
 use Psalm\Issue\InvalidNullableReturnType;
@@ -164,6 +165,7 @@ final class ProjectAnalyzer
      * @var array<int, class-string<CodeIssue>>
      */
     private const SUPPORTED_ISSUES_TO_FIX = [
+        ClassMustBeFinal::class,
         InvalidFalsableReturnType::class,
         InvalidNullableReturnType::class,
         InvalidReturnType::class,

src/Psalm/Internal/Codebase/ClassLikes.php

@@ -24,6 +24,7 @@
 use Psalm\Internal\Provider\FileReferenceProvider;
 use Psalm\Internal\Provider\StatementsProvider;
 use Psalm\Internal\Type\TypeExpander;
+use Psalm\Issue\ClassMustBeFinal;
 use Psalm\Issue\PossiblyUnusedMethod;
 use Psalm\Issue\PossiblyUnusedParam;
 use Psalm\Issue\PossiblyUnusedProperty;
@@ -861,6 +862,34 @@ public function consolidateAnalyzedData(Methods $methods, ?Progress $progress, b
                     }
                     $this->checkMethodParamReferences($classlike_storage);
                 }
+                if (!$classlike_storage->public_api
+                    && !$classlike_storage->has_children
+                    && !$classlike_storage->abstract
+                    && !$classlike_storage->final
+                    && !$classlike_storage->is_enum
+                    && !$classlike_storage->is_interface
+                ) {
+                    IssueBuffer::maybeAdd(
+                        new ClassMustBeFinal(
+                            'Class ' . $classlike_storage->name
+                                . ' is never extended and is not part of the public API, and thus must be made final.',
+                            $classlike_storage->location,
+                            $classlike_storage->name,
+                        ),
+                        $classlike_storage->suppressed_issues,
+                        true,
+                    );
+
+                    if ($codebase->alter_code
+                        && $classlike_storage->stmt_location !== null
+                        && isset($project_analyzer->getIssuesToFix()['ClassMustBeFinal'])
+                    ) {
+                        $idx = $classlike_storage->stmt_location->getSelectionBounds()[0];
+                        FileManipulationBuffer::add($classlike_storage->stmt_location->file_path, [
+                            new FileManipulation($idx, $idx, 'final ', true),
+                        ]);
+                    }
+                }
 
                 $this->findPossibleMethodParamTypes($classlike_storage);
 

src/Psalm/Internal/Codebase/Populator.php

@@ -560,6 +560,8 @@ private function populateDataFromParentClass(
                 $storage->declaring_pseudo_method_ids[$method_name] = $pseudo_method_id;
             };
         }
+
+        $parent_storage->has_children = true;
     }
 
     private function populateInterfaceData(

src/Psalm/Internal/Provider/ParamsProvider/ArrayFilterParamsProvider.php

@@ -29,7 +29,7 @@
 /**
  * @internal
  */
-class ArrayFilterParamsProvider implements FunctionParamsProviderInterface
+final class ArrayFilterParamsProvider implements FunctionParamsProviderInterface
 {
     /**
      * @return array<lowercase-string>

src/Psalm/Internal/Provider/ParamsProvider/ArrayMultisortParamsProvider.php

@@ -32,7 +32,7 @@
 /**
  * @internal
  */
-class ArrayMultisortParamsProvider implements FunctionParamsProviderInterface
+final class ArrayMultisortParamsProvider implements FunctionParamsProviderInterface
 {
     /**
      * @return array<lowercase-string>

src/Psalm/Internal/Provider/ParamsProvider/ArrayUArrayParamsProvider.php

@@ -19,7 +19,7 @@
 /**
  * @internal
  */
-class ArrayUArrayParamsProvider implements FunctionParamsProviderInterface
+final class ArrayUArrayParamsProvider implements FunctionParamsProviderInterface
 {
 
     /**

src/Psalm/Internal/Provider/ReturnTypeProvider/FilterInputReturnTypeProvider.php

@@ -34,7 +34,7 @@
 /**
  * @internal
  */
-class FilterInputReturnTypeProvider implements FunctionReturnTypeProviderInterface
+final class FilterInputReturnTypeProvider implements FunctionReturnTypeProviderInterface
 {
     /**
      * @return array<lowercase-string>

src/Psalm/Issue/ClassMustBeFinal.php

@@ -0,0 +1,11 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Psalm\Issue;
+
+final class ClassMustBeFinal extends ClassIssue
+{
+    public const ERROR_LEVEL = 2;
+    public const SHORTCODE = 361;
+}

src/Psalm/Issue/LiteralKeyUnshapedArray.php

@@ -4,7 +4,7 @@
 
 namespace Psalm\Issue;
 
-class LiteralKeyUnshapedArray extends CodeIssue
+final class LiteralKeyUnshapedArray extends CodeIssue
 {
     public const ERROR_LEVEL = -2;
     public const SHORTCODE = 360;

src/Psalm/Issue/UnusedIssueHandlerSuppression.php

@@ -4,7 +4,7 @@
 
 namespace Psalm\Issue;
 
-class UnusedIssueHandlerSuppression extends CodeIssue
+final class UnusedIssueHandlerSuppression extends CodeIssue
 {
     public const ERROR_LEVEL = -1;
     public const SHORTCODE = 326;

src/Psalm/Storage/ClassLikeStorage.php

@@ -180,6 +180,8 @@ final class ClassLikeStorage implements HasAttributesInterface
      */
     public array $pseudo_static_methods = [];
 
+    public bool $has_children = false;
+
     /**
      * Maps pseudo method names to the original declaring method identifier
      * The key is the method name in lowercase, and the value is the original `MethodIdentifier` instance

tests/AlgebraTest.php

@@ -25,7 +25,7 @@
 
 use function spl_object_id;
 
-class AlgebraTest extends TestCase
+final class AlgebraTest extends TestCase
 {
     public function testNegateFormula(): void
     {

tests/AnnotationTest.php

@@ -12,7 +12,7 @@
 
 use const DIRECTORY_SEPARATOR;
 
-class AnnotationTest extends TestCase
+final class AnnotationTest extends TestCase
 {
     use InvalidCodeAnalysisTestTrait;
     use ValidCodeAnalysisTestTrait;

tests/ArgTest.php

@@ -9,7 +9,7 @@
 
 use const DIRECTORY_SEPARATOR;
 
-class ArgTest extends TestCase
+final class ArgTest extends TestCase
 {
     use InvalidCodeAnalysisTestTrait;
     use ValidCodeAnalysisTestTrait;

tests/ArrayAccessTest.php

@@ -12,7 +12,7 @@
 
 use const DIRECTORY_SEPARATOR;
 
-class ArrayAccessTest extends TestCase
+final class ArrayAccessTest extends TestCase
 {
     use InvalidCodeAnalysisTestTrait;
     use ValidCodeAnalysisTestTrait;