Add support for multiple config files based on a file pattern (different to order of precendence)

[Sectimus]

This issue exists to diverge this discussion to it's own issue

The problem:

With a very large project, PHPStan eats through your memory faster than you can say segfault. So if you were limited on memory, splitting your configs so that you stay within the limit of your hardware is a very valid choice. Especially in the context of CI/CD.

There is currently no way to allow this extension to use a different config file based on directory of the file that was saved.

PHPStan does already support this via the parameters.paths config. If you were wanting to scan multiple paths / config files at the same time I could just call PHPStan multiple times.

phpstan analyse path/to/directory --configuration path/to/phpstan.neon & phpstan analyse path/to/another/directory --configuration path/to/another/phpstan.neon &

This is possible in the CLI of course, but not via this extension (I would have to awkwardly change the phpstan.configFile setting every time I switch between working on src/ vs test/ in my example. and this is exactly what I do, it is quite frustrating

Solution 1:

I propose a new setting where we can map our configs to directories.

this example would be mutually exclusive with phpstan.configFile

"phpstan.configFiles": {
    "${workspaceFolder}/src/*": "${workspaceFolder}/src.phpstan.neon",
    "${workspaceFolder}/test/*": "${workspaceFolder}/test.phpstan.neon",
},

Solution 2:

We could instead just read the parameters.paths & paramerers.excludePaths values in the PHPStan config files, since it is just using the fnmatch() function to interpret them, we could do the same process to receive the same result without needing to define a new setting.

Each entry is used as a pattern for the fnmatch() function.

• https://phpstan.org/config-reference#analysed-files

[SanderRonde]

I do like solution 2 a lot more since it requires a lot less user effort and is less likely to go wrong. But I do have to say that I'm not super opposed to the idea anymore. A few challenges still that I'll have to think about and that this kind of hinges on:

• Currently there is the phpstan.configFile setting. This already allows for a comma-separated list of config files that the extension will try in order. I'm kind of wondering how this change would fit into that. I guess it could be deprecated and phpstan.configFiles could be an array. Although then you'd have an array of comma-separated lists... Or the already-existing comma-separated list could be turned into an array of config files through some phpstan.multipleConfigs boolean. I guess this is solvable though.
• Getting the list of files in the paths config isn't too easy for a few reasons:
  • There is currently no JS implementation of a .neon file parser. There is one in PHP and in Python though so I guess I could build a new one using those as reference. Quite a hassle to build that though...
  • I'd have to also implement resolving of included config files. Not undoable but does add to the complexity and list of things that can go wrong

These are kind of the two biggest implementation issues. If it turns out that one of them is just really hard to find a solution for (or it is possible but really complex) then it's probably not worth it to implement this. If they turn out to be doable then it should be rather easy to implement.

So TLDR: I am open to the idea but will have to think about it a bit. Might take a while before this is implemented though since I am a bit busy in the coming weeks and since bug-fixes obviously take priority.

[Sectimus]

I'd prefer the mutually exclusive option to not impact current user setups (I'm pretty sure I have seen other extensions do this, so that your settings.json would report an issue should you try to use both.)

I would say phpstan.configFiles could just take an array with the same comma separated string parameters, that way you can still have your order of precedence defined yourself on a per-directory level and it wouldn't require a whole lot of rework.

"phpstan.configFiles": [
    "${workspaceFolder}/src.phpstan.neon, ${workspaceFolder}/src.phpstan.neon.dist, ${workspaceFolder}/src.phpstan.dist.neon",
    "${workspaceFolder}/test.phpstan.neon, ${workspaceFolder}/test.phpstan.neon.dist, ${workspaceFolder}/test.phpstan.dist.neon"
],

IE. having the below configured in settings.json is disallowed, only one option may be selected.

"phpstan.configFile": "${workspaceFolder}/src.phpstan.neon, ${workspaceFolder}/test.phpstan.neon",
"phpstan.configFiles": [
    "${workspaceFolder}/src.phpstan.neon",
    "${workspaceFolder}/test.phpstan.neon"
],

• In the above example when using the phpstan.configFile setting: only src.phpstan.neon OR test.phpstan.neon will be evaluated. In order of precedence.
• Whereas using the phpstan.configFiles setting: both src.phpstan.neon AND test.phpstan.neon will be evaluated. it may be wise to stagger these such that test.phpstan.neon runs after src.phpstan.neon to prevent hammering the machine

There is currently no JS implementation of a .neon file parser. There is one in PHP

It could be worth just having a small PHP script that will take a .neon config file, parse the parameters.paths & paramerers.excludePaths keys and run them through the fnmatch() function. That way should this function change behaviour in any way, this extension behaviour will match that of PHPStan itself since we are following the exact same process. Presumably you already have the PHP side of things set up as this is how the extension is able to call PHPStan so you'd just need to json_encode() all the information you need, spit it out, and parse that in JS.

The compute shouldn't be that taxing at all for this, so shouldn't impact run time and could probably be done as a pre-task to every run of the extension.

As much as it pains me, I do understand the low priority. Thank you for all the effort, know that it is very much appreciated.

[SanderRonde]

Hmm yeah I agree that using an array of comma-separated values is probably the best. I'm also considering just switching to configFiles but still reading from configFile as a form of backwards compatibility, but still deprecating the setting. Mutually exclusive settings can be a bit confusing, especially since VSCode settings don't give a great overview of the entire picture. Then new users can move to that config while existing users don't need to change anything and nothing will break.

Whereas using the phpstan.configFiles setting: both src.phpstan.neon AND test.phpstan.neon will be evaluated.

Maybe good to get the scenarios as to which one is evaluated when clear:

• I'd say that if I just made a change in src/ then only that one will be evaluated and when I make a change in test/ only that one should be evaluated right?
• I would say that PHPStan: scan project for errors should evaluate both of them, indeed probably best to not do so at the same time but to queue them.
• I think it would be nice to add a PHPStan: scan **current** project for errors command (if multiple config files are used) that only scans the project belonging to the configFile that belongs to the file you currently have open. (if no file open or it's in no config, this errors)

It could be worth just having a small PHP script that will take a .neon config file, parse the parameters.paths & paramerers.excludePaths keys and run them through the fnmatch() function. That way should this function change behaviour in any way, this extension behaviour will match that of PHPStan itself since we are following the exact same process. Presumably you already have the PHP side of things set up as this is how the extension is able to call PHPStan so you'd just need to json_encode() all the information you need, spit it out, and parse that in JS.

I'm a bit wary of this for a couple of reasons:

• The extension will also need to be shipped with a vendor/ folder then. That folder will contain composer-installed packages that may or may not be compatible with the user's operating system.
  • This can be circumvented by running composer install when installing the extension, but it's a very big assumption to make that composer is installed on the user's system. Then there's issues like corporate proxies, permission issues and what not. And I probably wouldn't like it if an extension is running a bunch of PHP code on-install.
• We'd also be assuming that PHP is installed on the user's system (and in the system path). Of course this isn't that big of an assumption to make but given that quite a few users of this extension run it and PHPStan within some very purpose-built docker containers that don't contain a lot of binaries, that might not always be the case. There's also the case of windows where this is not trivial, as well as people using specific PHP binaries /usr/bin/php8.3 instead of just php, which might also cause some issues.

I completely agree that this would be ideal and would be a great match to what PHPStan does, but I'm thinking this is likely to cause a lot of edge cases with system setups to pop up. Maybe simpler to just build a JS implementation of the neon parser (possibly porting it with ✨AI✨) and keep it all in-extension.

[SanderRonde]

Well I've just found a neon parser in JS so that's no longer a problem. That's pretty nice

[SanderRonde]

I've implemented it and I think it works (tested it with the new test/multi-config-demo folder). Can you give it a try?

phpstan-vscode-3.2.11.zip

[Sectimus]

Sorry for the late response! For me whilst this does work, it doesn't really suit my purpose since I have certain folders as set up as different workspaces in vscode. I see that this extension doesn't support multi-root workspaces (and only supports the first workspace shown), but was wondering if we could have an override to enable per-workspace scans?

My phpstan config files point to these different roots, so even if we ran all of these phpstan processes on different workspaces at the same time: I wouldn't have any crosstalk. But it is not possible to do so atm.

I can see why you didn't support multi-root in the first place, with cache invalidation probably being a big drive, but if you configure your stan configs correctly to point to the right directories, then it should just work.

[SanderRonde]

Hmm that does complicate things a bit, largely because configuration right now is all relative to the current workspace. But it may be doable to fit this into the current model. As multiple configs are now supported in this branch, it uses the currently scanned file to determine the neon file. It's not a massive stretch to also use that file to determine its workspace folder to base the config off of.

Some questions though to understand:

• Is every one of your (.neon file) projects a separate workspace or do you also have some workspaces with multiple neon files in them?
• How would configuration look? Would you have workspace folder A where you configure phpstan.configFiles: ['foo/config.neon'] and then another folder B where you have different settings? Or do you foresee more of a single phpstan.configFiles: [{path: 'foo/config.neon', workspaceFolder: A}] etc-like config? (I'm not sure if the latter is even possible)

[Sectimus]

Is every one of your (.neon file) projects a separate workspace or do you also have some workspaces with multiple neon files in them?

I do have some projects with multiple neon files in them for scanning different directories of the same project. Wheras I have a separate application that is still a part of the same overall project, so it is a separate root workspace in vscode. https://code.visualstudio.com/docs/editor/multi-root-workspaces.

I could place all of my .neon config files from all of my projects in the new "phpstan.configFiles" setting, however only changes in the first workspace root will trigger this extension to start scanning. Wheras I would be looking for an override to allow it to trigger on changes to any workspace root.

in my workspace:

...
	"folders": [
		{
			"name": "primary",
			"path": "../primary"
		},
		{
			"name": "secondary",
			"path": "../secondary"
		},
	],
...

Any changes in the "../primary" directory will trigger phpstan scans, this will also scan the "../secondary" directory as expected (due to the new and shiny multi-config file support) - However nothing you ever do in the "../secondary" directory could trigger the extension to start a scan, even if your setup could handle it.

How would configuration look? Would you have workspace folder A where you configure phpstan.configFiles: ['foo/config.neon'] and then another folder B where you have different settings? Or do you foresee more of a single phpstan.configFiles: [{path: 'foo/config.neon', workspaceFolder: A}] etc-like config? (I'm not sure if the latter is even possible)

Most extension handle this with path mappings. For instance:

"phpunit.paths": {
	"${workspaceFolder:primary}": "/var/www/primary",
        "${workspaceFolder:secondary}": "/var/www/secondary",
}

It could be something like:

"phpstan.paths": {
	"${workspaceFolder:primary}": [
		"config/primary.1.neon",
		"config/primary.2.neon",
		"config/primary.3.neon"
	],
	"${workspaceFolder:secondary}": [
		"config/secondary.src.neon",
		"config/secondary.tests.neon"
	]
}

Although just allowing the scan to be triggered from any workspace root sounds like a simpler solution that would also work for me :)

[Sectimus]

The new version actually doesn't seem to work as expected in terms of this feature, the scan never completes when I use more than one config, when each works individually on their own. See my attached video

2024-10-07.11-56-43.mp4

[SanderRonde]

I've just added multi-workspace support, I think it works (I've tested it on a demo project and it worked). Could you give it a try?

Regarding the testing not succeeding when using multiple configs, could you send along the logs of the extension (Output panel, then PHPStan Language Server). I think that'll contain some more info.

Regarding my questioning of how to configure this, I did not know that VSCode allows for per-workspace settings that would then space multiple workspace roots. So indeed configuring it in the workspace settings is entirely fine. I've right now allowed for ${workspaceFolder:folderName} to be used to refer to specific workspace folder roots. You can use that to refer to config files etc.

phpstan-vscode-3.2.11.zip