Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

CS Fixes - Replaced "array of type" by "Type[]" in PHPDoc block #5888

Closed
wants to merge 21 commits into
from

Conversation

Projects
None yet
6 participants
Contributor

raziel057 commented Nov 1, 2012

Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: no (but tests doesn't pass on master too). See Travis.
License of the code: MIT
Documentation PR: Not Applicable
Status: Finished

To improve support of the eclipse PDT pluggin (for autocompletion), I propose to change the array notation in PHPDoc blocks to match the phpDocumentor notation for "array of type".

Modifications are made for the following components:

  • BrowserKit
  • ClassLoader
  • Config
  • Console
  • CssSelector
  • DependencyInjection
  • DomCrawler
  • EventDispatcher (no changes)
  • Filesystem (no changes)
  • Finder
  • Form
  • HttpFoundation
  • HttpKernel
  • Locale
  • OptionResolver (no changes)
  • Process (no changes)
  • Routing (no changes)
  • Serializer (no changes)
  • Templating
  • Translation
  • Validator
  • Yaml (no changes)
  • Security
  • Stopwatch (no changes)

See Proposal #5852

Contributor

pborreli commented Nov 1, 2012

will you make a PR for each component ? why not only one PR with one commit for each component instead ?

Contributor

raziel057 commented Nov 1, 2012

Ok, I'm going try to do it.

Contributor

raziel057 commented Nov 1, 2012

I would like to rename my branch from COMPONENT_Form to changes-phpdoc (as all modifications would be commited in only one branch), so I tried to execute the following command but I have an error.

git remote rename COMPONENT_Form changes-phpdoc
error: Could not rename config section 'remote.COMPONENT_Form' to 'remote.changes-phpdoc'

Do you know how to do it?

Contributor

pborreli commented Nov 1, 2012

don't rename it, you will have to close and make another PR which is useless here, just edit the title.

Member

stof commented Nov 1, 2012

and git remote rename is about renaming a remote repo, not a branch

Contributor

raziel057 commented Nov 3, 2012

Is it normal that all my commit are duplicated? I would like just update my master and merge with my branch.

Owner

fabpot commented Nov 6, 2012

@raziel057 Can you rebase on master? That should fix your problem.

@webmozart webmozart commented on an outdated diff Nov 6, 2012

...mfony/Component/ClassLoader/ClassCollectionLoader.php
@@ -210,7 +210,7 @@ private static function writeCacheFile($file, $content)
*
* @param array $classes
*
- * @return array An array of sorted \ReflectionClass instances (dependencies added if needed)
+ * @return ReflectionClass[] An array of sorted \ReflectionClass instances (dependencies added if needed)
@webmozart

webmozart Nov 6, 2012

Contributor

should be \ReflectionClass[]

@webmozart webmozart commented on the diff Nov 6, 2012

src/Symfony/Component/Config/ConfigCache.php
@@ -83,8 +83,8 @@ public function isFresh()
/**
* Writes cache.
*
- * @param string $content The content to write in the cache
- * @param array $metadata An array of ResourceInterface instances
+ * @param string $content The content to write in the cache
+ * @param ResourceInterface[] $metadata An array of ResourceInterface instances
@webmozart

webmozart Nov 6, 2012

Contributor

should be Resource\ResourceInterface[]

@stof

stof Nov 6, 2012

Member

in fact, it should have a use statement (to be consistent with other phpdoc in Symfony)

@raziel057

raziel057 Nov 9, 2012

Contributor

@stof But the use statement is just usefull for the PHPDoc. Doesn't you think that it is memory use for nothing on execution? Even if it's it's negligible.

@stof

stof Nov 9, 2012

Member

@raziel057 if you use the way suggested by @bschussek, there is a high chance @fabpot would simply change it in a "Fixed CS" or "Tweaked previous merge" commit after merging the PR :)

@Tobion Tobion commented on an outdated diff Nov 6, 2012

...mponent/DependencyInjection/Compiler/RepeatedPass.php
@@ -21,13 +21,20 @@
*/
class RepeatedPass implements CompilerPassInterface
{
+ /**
+ * @var Boolean
+ */
private $repeat;
@Tobion

Tobion Nov 6, 2012

Member

should be initialized, otherwise it's a Boolean|null

@Tobion Tobion and 2 others commented on an outdated diff Nov 6, 2012

...ny/Component/DependencyInjection/ContainerBuilder.php
private $extensionConfigs = array();
+
+ /**
+ * @var Compiler
@Tobion

Tobion Nov 6, 2012

Member

Compiler|null

@raziel057

raziel057 Nov 10, 2012

Contributor

@Tobion If I add |null for this one, we need to change on a lot of other parts. Is it really needed? The goal is to be able to have a support for autocompletion in the IDE, considering that object can be nullable.

@fabpot

fabpot Nov 10, 2012

Owner

Keep it like this.

Owner

fabpot commented Nov 9, 2012

@raziel057 Can you finish this PR?

Member

Tobion commented Nov 9, 2012

I'll do it for the routing component this evening because I know it by heart. ^^

Contributor

raziel057 commented Nov 9, 2012

@Tobion ok Thanks!

@fabpot Yes, I will try to finish it this week end.

Contributor

raziel057 commented Nov 11, 2012

@Tobion Did you already change PHPDoc in the Routing component?

Member

Tobion commented Nov 11, 2012

@raziel057 Yes I'm working on it.

Member

Tobion commented Nov 12, 2012

@raziel057 Done. See #5994

@fabpot fabpot closed this in 6e8115a Nov 19, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment