-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Config] Add type-hints to public interfaces and classes #32201
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
00875f5
to
0254139
Compare
Please also update those implementations of the updated interfaces that are also part of the Config component. |
@@ -26,7 +26,7 @@ interface LoaderInterface | |||
* | |||
* @throws \Exception If something went wrong | |||
*/ | |||
public function load($resource, $type = null); | |||
public function load($resource, ?string $type = null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
should be removed (same below when null is the default), per our CS
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
@@ -115,6 +115,6 @@ public function testLocateEmpty() | |||
{ | |||
$loader = new FileLocator([__DIR__.'/Fixtures']); | |||
|
|||
$loader->locate(null, __DIR__); | |||
$loader->locate('', __DIR__); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test expected an InvalidArgumentException
but gets a TypeError
now.
ee3536a
to
ae54f82
Compare
ae54f82
to
e069dc4
Compare
Thank you @jschaedl 8000 . |
… (jschaedl) This PR was squashed before being merged into the 5.0-dev branch (closes #32201). Discussion ---------- [Config] Add type-hints to public interfaces and classes | Q | A | ------------- | --- | Branch? | master <!-- see below --> | Bug fix? | no | New feature? | no <!-- please update src/**/CHANGELOG.md files --> | BC breaks? | no <!-- see https://symfony.com/bc --> | Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Tests pass? | yes <!-- please add some, will be required by reviewers --> | Fixed tickets | #32179 <!-- #-prefixed issue number(s), if any --> | License | MIT | Doc PR | N/A <!-- required for new features --> This PR adds type hints to all public interfaces of the Config component. Commits ------- e069dc4 [Config] Add type-hints to public interfaces and classes
*/ | ||
public function setPerformDeepMerging($boolean) | ||
public function setPerformDeepMerging(bool $boolean) | ||
{ | ||
$this->performDeepMerging = (bool) $boolean; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
forgotten cast
@@ -23,5 +23,5 @@ interface PrototypeNodeInterface extends NodeInterface | |||
* | |||
* @param string $name The name of the node |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
useless doc
@@ -33,7 +33,7 @@ public function __construct($paths = []) | |||
/** | |||
* {@inheritdoc} | |||
*/ | |||
public function locate($name, $currentPath = null, $first = true) | |||
public function locate(string $name, string $currentPath = null, $first = true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
forgotten bool
@@ -28,5 +28,5 @@ interface ConfigCacheFactoryInterface | |||
* | |||
* @return ConfigCacheInterface The cache instance | |||
*/ | |||
public function cache($file, $callable); | |||
public function cache(string $file, $callable); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing callable type
@Tobion Thanks. I will create a follow-up PR to address your comments. |
This PR was merged into the 5.0-dev branch. Discussion ---------- [Config] finish adding parameter types | Q | A | ------------- | --- | Branch? | master | Bug fix? | yno | New feature? | no <!-- please update src/**/CHANGELOG.md files --> | BC breaks? | no <!-- see https://symfony.com/bc --> | Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Tests pass? | yes <!-- please add some, will be required by reviewers --> | Fixed tickets | #32179 | License | MIT | Doc PR | symfony/symfony-docs#... <!-- required for new features --> Completes some missing things in #32201 Commits ------- 2ca8354 [Config] finish adding parameter types
This PR adds type hints to all public interfaces of the Config component.