Repository navigation
Ruleset fully_qualified_strict_types is creating invalid code #17
Description
Talk integration tests have this file which imports TableNode:
https://github.com/nextcloud/spreed/blob/0d1fc13e36e40a6cd8dfa80915ea77f06b835f80/tests/integration/features/bootstrap/FeatureContext.php#L27
But the autofixing is breaking the docs by adding a leading \ because the file itself has no namespace
@@ -914,9 +914,9 @@
* @param string $user
* @param string $identifier
* @param string $apiVersion
- * @param TableNode|null $formData
+ * @param \TableNode|null $formData
*/
- public function userCreatesRoom(string $user, string $identifier, string $apiVersion, TableNode $formData = null): void {
+ public function userCreatesRoom(string $user, string $identifier, string $apiVersion, ?TableNode $formData = null): void {
$this->userCreatesRoomWith($user, $identifier, 201, $apiVersion, $formData);
}
Activity
come-nc commentedon Feb 1, 2024
We should remove
fully_qualified_strict_typesrule and report the bug upstream I thinknickvergessen commentedon Feb 1, 2024
yeah let's do that and do a "quick" 1.2.1
come-nc commentedon Feb 1, 2024
For the record, there are several issues and PRs opened regarding this rule, I think PHP-CS-Fixer/PHP-CS-Fixer#7719 would fix the issue you had.
But for now let’s just remove the rule to be able to update our coding-standard. We may try it again in some time.
fix: Remove `fully_qualified_strict_types` again
nickvergessen commentedon Feb 1, 2024
Let keep this open as reminder?
[-]1.2 ruleset is creating invalid code[/-][+]Ruleset `fully_qualified_strict_types` is creating invalid code[/+]Wirone commentedon Feb 2, 2024
IMHO you should keep
fully_qualified_strict_types, but at the same time addno_superfluous_phpdoc_tags- there's no point in keeping these@params because all the information is already in the native signature. This method should look like:Then you don't need to wait for PHP-CS-Fixer/PHP-CS-Fixer#7719 and have less code in the repo 😉.
come-nc commentedon Feb 5, 2024
We most probably have tags in places which are not superfluous and those would get messed-up by the rule as well. Also seeing several bug reports and open pull request against the same rule has made me scared to add it for now.
Wirone commentedon Feb 5, 2024
Only actually superfluous tags are removed (where type in phpDoc is exactly the same as in native signature + there's no additional description), so maybe just try how it works in practice 🙂.
In terms of
fully_qualified_strict_types: this rule was massively developed lately, I've added feature for importing symbols found during analysis, we added several new places where FQCN can be found and shortened, there were many other improvements like relative imports. It's natural there were issues reported, but you can clearly see these were resolved - almost all remaining issues are feature requests, there is only one bug related to edge case, but even this issue has 2 opened PRs (with alternative implementation). There's no need to be scared, just look at how many test cases we have for it 🙂.