As crell already suggested at #363802-25: Document class methods with class, factory and defining interfaces I'd also like to have the return type specified, so modern IDEs like eclipse pick them up for auto completion. When working with the DB TNG and not being that familiar with the new system it helps a lot when the IDE comes up with auto-completion support - but currently that doesn't work.

I'm using eclipse and the attached patch allows it to do auto-completion for DB-TNG. As crell has pointed out most modern IDEs should pick that up.
With auto-completion using DB TNG is much more easier, so let's help IDEs to be able to offer it!

CommentFileSizeAuthor
#6 return-types.patch25.14 KBCrell
dbtng.patch4.59 KBfago
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Crell’s picture

I am majorly +1 on this, and in fact believe we should go further than the wrapper functions listed here to all of the methods of those classes, too. Hell, I want it to be Drupal-wide, not just DBTNG.

The catch is that I don't know how it will interact with the API module: #608124: Link function parameter types and return types to class/interface pages

If the API module is OK with it, let's do. It's not an API change IMO.

Dries’s picture

Committed to CVS HEAD. Let's see what API module does ... and follow-up as necessary. ;-) Leaving at 'needs review'.

Berdir’s picture

This just added @return SomeClass for the API functions like db_select() and so on. Since we provide a fluent interface ($object->doStuff()->doMoreStuff()) and try to encourage developers to use it, we should also add that to all methods that are fluent. Especially since not all of them all and in some cases, it's not obvious from the method name.

I will probably write patch for this sooner or later (have to get back to business ;)) but I wanted to write it down so that it doesn't get lost/forgotten.

Crell’s picture

Right then!

If we're going to do it, let's outright do it. There's a lot of code that returns objects now, both DBTNG and otherwise, where we can/should add these tags. I guess we start doing that and let API module decide how it's going to respond. :-) (Last I heard it still didn't deal with classes in the first place, which has to get fixed anyway one way or another.)

I'll update the coding standards page as well.

moshe weitzman’s picture

Hurray! Finally a win for IDE users. Drupal has gone so far down the path of magical array keys [e.g. $options in url(), FAPI properties, ...] that we have quite some clawing back to do.

Crell’s picture

Title: DB TNG auto completion in IDEs » @return type definitions for auto-completion in IDEs
Component: database system » base system
FileSize
25.14 KB

OK, I went through and tagged other methods and functions as I could find them. I think I got them all. This patch shouldn't do anything outside of docblocks. :-)

It turns out, actually, some of our other non-DB OO code was already defining a return type. Some of it was even buggy. Some of it even defined parameter types! And here I thought I was being a good little coder by not doing that in the DB layer. Ah well.

fago’s picture

Awesome!!! Yep API module needs some improvements nevertheless.

@crell:
Patch looks good, but also squashes some empty lines with white-spaces. Not really topic of this issue, but they should be squashed anyway.

Crell’s picture

My IDE automatically kills ending whitespace. Yours should to. If you submit a patch that adds trailing whitespace, there's a bug in the patch. :-)

fago’s picture

Mine does too, but it's nasty if the patch contains hunks unrelated to the real changes. So I turned that off for core.

Dries’s picture

Status: Needs review » Fixed

Committed to CVS HEAD. Thanks Crell.

moshe weitzman’s picture

Man, this is very very nice. Terrific DX improvement.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.