Drop forbidAssignmentNotMatchingVarDoc & allowNamedArgumentOnlyInAttr…

…ibutes
shipmonk-rnd · May 14, 2024 · 8a16521 · 8a16521
1 parent c1619ed
commit 8a16521
Show file tree

Hide file tree

Showing 17 changed files with 53 additions and 843 deletions.
diff --git a/README.md b/README.md
@@ -23,8 +23,6 @@ parameters:
     shipmonkRules:
         allowComparingOnlyComparableTypes:
             enabled: true
-        allowNamedArgumentOnlyInAttributes:
-            enabled: true
         backedEnumGenerics:
             enabled: true
         classSuffixNaming:
@@ -46,9 +44,6 @@ parameters:
         forbidArithmeticOperationOnNonNumber:
             enabled: true
             allowNumericString: false
-        forbidAssignmentNotMatchingVarDoc:
-            enabled: true
-            allowNarrowing: false
         forbidCast:
             enabled: true
             blacklist: ['(array)', '(object)', '(unset)']
@@ -113,7 +108,7 @@ parameters:
             enabled: true
         forbidReturnValueInYieldingMethod:
             enabled: true
-            reportRegardlessOfReturnType: false
+            reportRegardlessOfReturnType: true
         forbidVariableTypeOverwriting:
             enabled: true
         forbidUnsetClassField:
@@ -162,27 +157,6 @@ new DateTime() > '2040-01-02'; // comparing different types is denied
 200 > '1e2'; // comparing different types is denied
 ```
 
-### allowNamedArgumentOnlyInAttributes
-- Allows usage of named arguments only in native attributes
-- Before native attributes, we used [DisallowNamedArguments](https://github.com/slevomat/coding-standard/blob/master/doc/functions.md#slevomatcodingstandardfunctionsdisallownamedarguments) sniff. But we used Doctrine annotations, which almost "require" named arguments when converted to native attributes.
-```php
-class User {
-    #[Column(type: Types::STRING, nullable: false)] // allowed
-    private string $email;
-
-    public function __construct(string $email) {
-        $this->setEmail(email: $email); // forbidden
-    }
-}
-```
-- This one is highly opinionated and will probably be disabled/dropped next major version as it does not provide any extra strictness, you can disable it by:
-```neon
-parameters:
-    shipmonkRules:
-        allowNamedArgumentOnlyInAttributes:
-            enabled: false
-```
-
 ### backedEnumGenerics *
 - Ensures that every BackedEnum child defines generic type
 - This rule makes sense only when BackedEnum was hacked to be generic by stub as described in [this article](https://rnd.shipmonk.com/hacking-generics-into-backedenum-in-php-8-1/)
@@ -352,58 +326,6 @@ function add(string $a, string $b) {
 }
 ```
 
-### forbidAssignmentNotMatchingVarDoc
-- Verifies if defined type in `@var` phpdoc accepts the assigned type during assignment
-- No other places except assignment are checked
-
-```php
-/** @var string $foo */
-$foo = $this->methodReturningInt(); // invalid var phpdoc
-```
-
-- For reasons of imperfect implementation of [type infering in phpstan-doctrine](https://github.com/phpstan/phpstan-doctrine#query-type-inference), there is an option to check only array-shapes and forget all other types by using `check-shape-only`
-- This is helpful for cases where field nullability is eliminated by WHERE field IS NOT NULL which is not propagated to the inferred types
-```php
-/** @var array<array{id: int}> $result check-shape-only */
-$result = $queryBuilder->select('t.id')
-    ->from(Table::class, 't')
-    ->andWhere('t.id IS NOT NULL')
-    ->getResult();
-```
-
-- It is possible to explicitly allow narrowing of types by `@var` phpdoc by using `allow-narrowing`
-```php
-/** @var SomeClass $result allow-narrowing */
-$result = $service->getSomeClassOrNull();
-```
-- Or you can enable it widely by using:
-```neon
-parameters:
-    shipmonkRules:
-        forbidAssignmentNotMatchingVarDoc:
-            allowNarrowing: true
-```
-
-#### Differences with native check:
-
-- Since `phpstan/phpstan:1.10.0` with bleedingEdge, there is a [very similar check within PHPStan itself](https://phpstan.org/blog/phpstan-1-10-comes-with-lie-detector#validate-inline-phpdoc-%40var-tag-type).
-- The main difference is that it allows only subtype (narrowing), not supertype (widening) in `@var` phpdoc.
-- This rule allows only widening, narrowing is allowed only when marked by `allow-narrowing` or configured by `allowNarrowing: true`.
-- Basically, **there are 3 ways for you to check inline `@var` phpdoc**:
-  - allow only narrowing
-    - this rule disabled, native check enabled
-  - allow narrowing and widening
-    - this rule enabled with `allowNarrowing: true`, native check disabled
-  - allow only widening
-    - this rule enabled, native check disabled
-
-- You can disable native check while keeping bleedingEdge by:
-```neon
-parameters:
-    featureToggles:
-        varTagType: false
-```
-
 ### forbidCast
 - Deny casting you configure
 - Possible values to use:

diff --git a/phpstan.neon.dist b/phpstan.neon.dist
@@ -37,8 +37,8 @@ parameters:
                 PHPStan\Rules\Rule: Rule
                 PhpParser\NodeVisitor: Visitor
                 ShipMonk\PHPStan\RuleTestCase: RuleTest
-        forbidAssignmentNotMatchingVarDoc:
-            enabled: false # native check is better now; this rule will be dropped / reworked in 3.0
+        enforceClosureParamNativeTypehint:
+            enabled: false # we support even PHP 7.4, some typehints cannot be used
 
     ignoreErrors:
         -

diff --git a/rules.neon b/rules.neon
@@ -2,8 +2,6 @@ parameters:
     shipmonkRules:
         allowComparingOnlyComparableTypes:
             enabled: true
-        allowNamedArgumentOnlyInAttributes:
-            enabled: true
         backedEnumGenerics:
             enabled: true
         classSuffixNaming:
@@ -25,9 +23,6 @@ parameters:
         forbidArithmeticOperationOnNonNumber:
             enabled: true
             allowNumericString: false
-        forbidAssignmentNotMatchingVarDoc:
-            enabled: true
-            allowNarrowing: false
         forbidCast:
             enabled: true
             blacklist: ['(array)', '(object)', '(unset)']
@@ -92,7 +87,7 @@ parameters:
             enabled: true
         forbidReturnValueInYieldingMethod:
             enabled: true
-            reportRegardlessOfReturnType: false
+            reportRegardlessOfReturnType: true
         forbidVariableTypeOverwriting:
             enabled: true
         forbidUnsetClassField:
@@ -116,9 +111,6 @@ parametersSchema:
         allowComparingOnlyComparableTypes: structure([
             enabled: bool()
         ])
-        allowNamedArgumentOnlyInAttributes: structure([
-            enabled: bool()
-        ])
         backedEnumGenerics: structure([
             enabled: bool()
         ])
@@ -149,10 +141,6 @@ parametersSchema:
             enabled: bool()
             allowNumericString: bool()
         ])
-        forbidAssignmentNotMatchingVarDoc: structure([
-            enabled: bool()
-            allowNarrowing: bool()
-        ])
         forbidCast: structure([
             enabled: bool()
             blacklist: arrayOf(string())
@@ -243,8 +231,6 @@ parametersSchema:
 conditionalTags:
     ShipMonk\PHPStan\Rule\AllowComparingOnlyComparableTypesRule:
         phpstan.rules.rule: %shipmonkRules.allowComparingOnlyComparableTypes.enabled%
-    ShipMonk\PHPStan\Rule\AllowNamedArgumentOnlyInAttributesRule:
-        phpstan.rules.rule: %shipmonkRules.allowNamedArgumentOnlyInAttributes.enabled%
     ShipMonk\PHPStan\Rule\BackedEnumGenericsRule:
         phpstan.rules.rule: %shipmonkRules.backedEnumGenerics.enabled%
     ShipMonk\PHPStan\Rule\ClassSuffixNamingRule:
@@ -263,8 +249,6 @@ conditionalTags:
         phpstan.rules.rule: %shipmonkRules.enforceReadonlyPublicProperty.enabled%
     ShipMonk\PHPStan\Rule\ForbidArithmeticOperationOnNonNumberRule:
         phpstan.rules.rule: %shipmonkRules.forbidArithmeticOperationOnNonNumber.enabled%
-    ShipMonk\PHPStan\Rule\ForbidAssignmentNotMatchingVarDocRule:
-        phpstan.rules.rule: %shipmonkRules.forbidAssignmentNotMatchingVarDoc.enabled%
     ShipMonk\PHPStan\Rule\ForbidCastRule:
         phpstan.rules.rule: %shipmonkRules.forbidCast.enabled%
     ShipMonk\PHPStan\Rule\ForbidCheckedExceptionInCallableRule:
@@ -320,8 +304,6 @@ conditionalTags:
 
     ShipMonk\PHPStan\Visitor\ImmediatelyCalledCallableVisitor:
         phpstan.parser.richParserNodeVisitor: %shipmonkRules.forbidCheckedExceptionInCallable.enabled%
-    ShipMonk\PHPStan\Visitor\NamedArgumentSourceVisitor:
-        phpstan.parser.richParserNodeVisitor: %shipmonkRules.allowNamedArgumentOnlyInAttributes.enabled%
     ShipMonk\PHPStan\Visitor\UnusedExceptionVisitor:
         phpstan.parser.richParserNodeVisitor: %shipmonkRules.forbidUnusedException.enabled%
     ShipMonk\PHPStan\Visitor\UnusedMatchVisitor:
@@ -339,8 +321,6 @@ conditionalTags:
 services:
     -
         class: ShipMonk\PHPStan\Rule\AllowComparingOnlyComparableTypesRule
-    -
-        class: ShipMonk\PHPStan\Rule\AllowNamedArgumentOnlyInAttributesRule
     -
         class: ShipMonk\PHPStan\Rule\BackedEnumGenericsRule
     -
@@ -367,10 +347,6 @@ services:
         class: ShipMonk\PHPStan\Rule\ForbidArithmeticOperationOnNonNumberRule
         arguments:
             allowNumericString: %shipmonkRules.forbidArithmeticOperationOnNonNumber.allowNumericString%
-    -
-        class: ShipMonk\PHPStan\Rule\ForbidAssignmentNotMatchingVarDocRule
-        arguments:
-            allowNarrowing: %shipmonkRules.forbidAssignmentNotMatchingVarDoc.allowNarrowing%
     -
         class: ShipMonk\PHPStan\Rule\ForbidCastRule
     -
@@ -449,10 +425,8 @@ services:
     -
         class: ShipMonk\PHPStan\Visitor\ImmediatelyCalledCallableVisitor
         arguments:
-                    immediatelyCalledCallables: %shipmonkRules.forbidCheckedExceptionInCallable.immediatelyCalledCallables%
-                    allowedCheckedExceptionCallables: %shipmonkRules.forbidCheckedExceptionInCallable.allowedCheckedExceptionCallables%
-    -
-        class: ShipMonk\PHPStan\Visitor\NamedArgumentSourceVisitor
+            immediatelyCalledCallables: %shipmonkRules.forbidCheckedExceptionInCallable.immediatelyCalledCallables%
+            allowedCheckedExceptionCallables: %shipmonkRules.forbidCheckedExceptionInCallable.allowedCheckedExceptionCallables%
     -
         class: ShipMonk\PHPStan\Visitor\UnusedExceptionVisitor
     -

diff --git a/src/Rule/AllowNamedArgumentOnlyInAttributesRule.php b/src/Rule/AllowNamedArgumentOnlyInAttributesRule.php
diff --git a/src/Rule/ClassSuffixNamingRule.php b/src/Rule/ClassSuffixNamingRule.php
@@ -2,9 +2,11 @@
 
 namespace ShipMonk\PHPStan\Rule;
 
+use LogicException;
 use PhpParser\Node;
 use PHPStan\Analyser\Scope;
 use PHPStan\Node\InClassNode;
+use PHPStan\Reflection\ReflectionProvider;
 use PHPStan\Rules\IdentifierRuleError;
 use PHPStan\Rules\Rule;
 use PHPStan\Rules\RuleErrorBuilder;
@@ -25,8 +27,14 @@ class ClassSuffixNamingRule implements Rule
     /**
      * @param array<class-string, string> $superclassToSuffixMapping
      */
-    public function __construct(array $superclassToSuffixMapping = [])
+    public function __construct(ReflectionProvider $reflectionProvider, array $superclassToSuffixMapping = [])
     {
+        foreach ($superclassToSuffixMapping as $className => $suffix) {
+            if (!$reflectionProvider->hasClass($className)) {
+                throw new LogicException("Class $className used in 'superclassToSuffixMapping' does not exist");
+            }
+        }
+
         $this->superclassToSuffixMapping = $superclassToSuffixMapping;
     }