Page MenuHomePhabricator

Fix param documentation of TabPanelLayout::__construct
Closed, ResolvedPublic

Description

In OOUI-php the class TabPanelLayout is defined to take a string as first argument:

	/**
	 * @param string $name Unique symbolic name of tab panel
	 * @param array $config Configuration options
	 *      - string|HtmlSnippet $config['label'] Label for tab panel's tab
	 * @param-taint $config escapes_htmlnoent
	 */
	public function __construct( $name, array $config = [] ) {
		// Allow passing positional parameters inside the config array
		if ( is_array( $name ) && isset( $name['name'] ) ) {
			$config = $name;
			$name = $config['name'];
		}

But the class also accept arrays for the first argument.

Please adjust the documentation to make phan happy in mediawiki/core

<file name="includes\specials\forms\PreferencesFormOOUI.php">
  <error line="150" severity="warning" message="Argument 1 (name) is array{classes:array{0:'mw-htmlform-autoinfuse-lazy'},name:string,label:string,content:\OOUI\Element|\OOUI\FieldsetLayout|\OOUI\GroupElement|\OOUI\IconElement|\OOUI\LabelElement|\OOUI\Layout|\OOUI\Tag|string,expanded:false,framed:true} but \OOUI\TabPanelLayout::__construct() takes string defined at vendor\oojs\oojs-ui\php\layouts\TabPanelLayout.php:34" source="PhanTypeMismatchArgument"/>
</file>

It seems this is also wrong on other classes like ActionFieldLayout or FieldLayout

Thanks.

Details

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 2 2019, 8:17 PM

I think the way we would really want to document it is as an optional positional parameter. Or in other words, it's valid to call TabPanelLayout(string) or TabPanelLayout(string, array) or TabPanelLayout(array) but not TabPanelLayout(array, array). As far as I know there is no way to do this, in PHP or in Phan.

We added this in https://gerrit.wikimedia.org/r/c/oojs/ui/+/192722, because it simplified some internal code, where we generate the constructor parameters for arbitrary widgets (e.g. in infusion code, and in unit tests). Handling just objects instead of arrays of objects makes things a lot simpler. Perhaps this was a bad idea, but we can't easily remove it now.

I suggest just changing the calls whenever Phan complains about them instead.

Change 520796 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/core@master] PreferencesFormOOUI: Avoid Phan warning with weird signature of TabPanelLayout

https://gerrit.wikimedia.org/r/520796

Change 520796 merged by jenkins-bot:
[mediawiki/core@master] PreferencesFormOOUI: Avoid Phan warning with weird signature of TabPanelLayout

https://gerrit.wikimedia.org/r/520796

Umherirrender closed this task as Resolved.Jul 4 2019, 8:32 PM
Umherirrender assigned this task to MaxSem.

Thanks for the patch