search

silverleague / silverstripe-ideannotator

Generate docblocks for DataObjects, Page, PageControllers and (Data)Extensions
BSD 3-Clause "New" or "Revised" License
46 stars 25 forks source link

Extensions on Controller doesn't work as expected #126

Closed madmatt closed 1 year ago

madmatt commented 5 years ago

Having an extension that extends SilverStripe\Control\Controller appears to cut off classes.

An example docblock that gets generated is: @property \SilverStripe\Admin\SecurityAdmin|\SilverStripe\GraphQL\Controller|\SilverStripe\TestSession\TestSessionController|\SilverShop\Comparison\Pagetypes\ProductComparisonPageController|\SilverShop\Admin\ProductCatalogAdmin|\SilverShop\Cart\ShoppingCartController|\SilverShop\Page\AccountPageController|\SilverShop\Page\CheckoutPageController|\SilverShop\Page\ProductCategoryController|\SilverShop\Page\ProductController|\SilverShop\Discounts\Admin\DiscountModelAdmin|\SilverStripe\CMS\Controllers\CMSMain|\SilverStripe\CMS\Controllers\CMSPageSettingsController|\SilverStripe\CMS\Controllers\ContentController|\SilverStripe\CMS\Controllers\ModelAsController|\SilverStripe\Control\Controller|\SilverStripe\Dev\DevBuildController|\App\Extensions\ControllerExtension $owner

I would have expected either one of two things to happen:

I'm not sure which option is better, but this seems to be the worst of both worlds? I haven't had a chance to investigate yet, but is there thought on which solution is better?

Alternatively, maybe I'm just confused how this works, but e.g. PageController isn't there, so there are entire hierarchies missing.

Firesphere commented 5 years ago

I've noticed the same behaviour for extensions on DataObject, it takes all children, instead of what it's actually extending at the base.

Firesphere commented 5 years ago

Feel free to make a PR @madmatt :D Also, while at it, do you want to upgrade the PHPDocumentor library pretty please? :)

madmatt commented 5 years ago

I was trying to figure out what the best way to fix this is... what do you think @Firesphere?

IMO, we should look at the YML and add hints for every class that the extension directly extends (e.g. it might show more than one class, but will only ever show classes that it directly extends. I think this best signals the intent of the extension (in that if you call a method that exists on HomePageController and you only extend Page then you should rightly see a warning from your IDE...

Thoughts welcome before I go diving into this on the next hackday I get a chance to participate in 😉

Firesphere commented 4 years ago

Related: https://github.com/silverleague/silverstripe-ideannotator/pull/129/files

Firesphere commented 4 years ago

I think this has been adressed in the PR. @madmatt , can you confirm dev-master does not have the problem anymore?

Firesphere commented 1 year ago

Closing, I'm fairly certain this has been resolved, and we've not heard back in 2+ years.