Page MenuHomePhabricator

Build an efficient methods for extensions to add phan dependencies
Open, Needs TriagePublic

Description

Currently (example), it goes by adding a list of extensions (with hardcoded path) to both $cfg['directory_list'] and $cfg['exclude_analysis_directory_list']. This has at least three problems:

  1. You have to keep the lists synced
  2. The path is hardcoded
  3. It doesn't exclude duplicated files. Locally, this is especially a problem if the other extensions have a vendor folder, as every vendor dependency is analyzed once per extension requiring it (leading to PhanRedefined* issues)

Perhaps we could have something like

$requiredExtensions = [ 'ExtA', 'ExtB' ];
require __DIR__ . '/../vendor/mediawiki/mediawiki-phan-config/src/config.php';

and the base config file would process $requiredExtensions, i.e. add path, filter vendor, etc. It would still be possible to specify dependencies manually.

Event Timeline

Or alternatively, we may have the base config return an object (e.g. PhanConfig), and call methods on that. For instance (implemented as a fluid interface, overcomplicated example):

$cfg = require __DIR__ . '/../vendor/mediawiki/mediawiki-phan-config/src/config.php';
return $cfg
    ->addExtensionDependencies( 'ExtA', 'ExtB' )
    ->suppressIssueTypes( 'NastyIssue1' )
    ->setExcludeFileRegex( '!vendor/foobar!' )
    ->setMinimumSeverity( 0 )
    ->disableStrictTypes()
    ->make();

Manual additions can happen after calling make() and before returning the result.

Of note, another drawback of the current approach which I forgot to mention: you have to handle appending to arrays, which may or may not exist.

Change 589603 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/tools/phan@master] Add a ConfigBuilder class to configure phan

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

Change 589603 merged by jenkins-bot:
[mediawiki/tools/phan@master] Add a ConfigBuilder class to configure phan

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

Change 629060 had a related patch set uploaded (by Hashar; owner: Hashar):
[mediawiki/tools/phan@master] Release 0.10.3

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

Aklapper removed a project: Patch-For-Review.

@Daimona: Patch in Gerrit has been merged. Thus I'm resolving this task. If there is more to do in this task, please reopen and elaborate. Thanks!

The patch above added a basic config framework to allow specifying dependencies in a more structured way, but it still doesn't have a specific method for loading extension dependencies that would fulfill the criteria listed in the task description (i.e., automatically exclude the directories from analysis, resolve paths, and handle duplicate composer dependencies).

I'm not working on it though.