Missing support for psalm/phar

[zonuexe]

Bug Report

Q	A
Version(s)	1.39.0

Summary

When Psalm is installed using psalm/phar instead of vimeo/psalm, an error occurs because vendor/vimeo/psalm/config.xsd does not exist.

refs #31, psalm/phar#5

Current behavior

Running before_script: xmllint --schema vendor/vimeo/psalm/config.xsd psalm.xml
warning: failed to load external entity "vendor/vimeo/psalm/config.xsd"
<?xml version="1.0"?>
Schemas parser error : Failed to locate the main schema resource at 'vendor/vimeo/psalm/config.xsd'.
WXS schema vendor/vimeo/psalm/config.xsd failed to compile
<psalm xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns="https://getpsalm.org/schema/config" errorLevel="1" resolveFromConfigFile="true" xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd" errorBaseline="psalm-baseline.xml" findUnusedBaselineEntry="true" findUnusedCode="false">
    <projectFiles>
        <directory name="src"/>
        <ignoreFiles>
            <directory name="vendor"/>
        </ignoreFiles>
    </projectFiles>
</psalm>

How to reproduce

In a project where psalm.xml exists, replacing vimeo/psalm with psalm/phar will reproduce the problem.

michaelpetri/phpunit-consecutive-arguments#109

Expected behavior

No errors.

[Ocramius]

Evidently, vendor/vimeo/psalm/config.xsd is an invalid declaration for your project: provide an XSD at any right location, and you should be fine.

[zonuexe]

@Ocramius

Isn't vendor/vimeo/psalm/config.xsd also hardcoded into laminas-continuous-integration-action?

https://github.com/laminas/laminas-ci-matrix-action/blob/v1.25/src/tools.ts

Both Psalm and the application need to be improved to resolve this issue.

[boesing]

It is, that is due to the fact that the CI implementation does only support psalm installed via composer as vimeo/psalm.
That was mainly as the generated phar has a bunch of problems, especially because it does not conflict with vimeo/psalm dependency and thus, you can have composer installed vimeo/psalm as v4.20 while the psalm/phar is 5.30.

I already reported that in psalm/phar#14 2 years ago and thus, for now, we do not support custom psalm config path.

Feel free to provide a PR where the XSD path for psalm can be modified but that would require you to have the XSD file somewhere available. I guess that this is not available from within psalm/phar?

[Ocramius]

@zonuexe good shout, sorry for jumping to conclusions!

[boesing]

I moved this issue to the CI matrix action as it has to be implemented here.
I also had a quick glance into the psalm/phar package, which sadly does not provide the XSD.
So you could ofc. create a copy of that in your project and extend the CI matrix to consume configurable XSD path but that feels odd. Maybe psalm/phar can pack the XSD along with the phar, that would actually ensure the correct XSD file (feel free to create a feature request there).

Until then, to make your CI work, you could add your own additional_checks entry where you omit the config validation stuff.
Via exclude you could then drop the psalm check which is auto-generated from the matrix job:
https://github.com/laminas/laminas-ci-matrix-action?tab=readme-ov-file#excluding-specific-jobs

I still have #184 pending, I could extend that to allow excluding tools as well. 🤔

[Ocramius]

@boesing I'm thinking that we should perhaps honor the XSD location, when specified as a local path: WDYT?

[boesing]

@Ocramius you mean extracting from psalm.xml?

[Ocramius]

Or generally any XML: we currently use hardcoded locations, but the locations are in the individual XML diles already, when done right 👍

[boesing]

the hardcoded location is used as that is known from psalm. but I am up for extracting the location from the config file - but usually there is no actual path located in XML files. I see either just namespaces and/or URI targeting external files via https. Even phpunit generated config is targeting remote XSD. I do not really want to implement a whole XSD detection strategy just for a niche use-case. we do not support psalm.xml in any other location than project root as well so I wanted to keep it simple. From what I can see, having a config option would probably work best (config is available where we do create the Tool job) but extracting stuff from XML might open a can of worms. but maybe I think too complicated?

[Ocramius]

do not really want to implement a whole XSD detection strategy just for a niche use-case. we do not support psalm.xml in any other location than project root as well so I wanted to keep it simple.

This is fair 👍

[boesing]

I created psalm/phar#18 to request config.xsd being bundled with upcoming versions.
Lets see how that works out. In the meantime, we could continue on working out how to have the XSD location being configured via the .laminas-ci.json.

Maybe something like this?

{
    "...": {},
   "tools": {
      "psalm": {
          "config": "pathToAlternativePsalm.xml",
          "schema": "pathToConfigSchemaDefinition.xsd"
      }
   }
}

We do already have quirks for infection where we detect roave plugin for the executable:

laminas-ci-matrix-action/src/tools.ts

Lines 40 to 46 in f25365c

	const composerJson: ComposerJson = parseJsonFile('composer.json', true) as ComposerJson;
	if (composerJson['require-dev']?.['roave/infection-static-analysis-plugin'] !== undefined) {
	return 'phpdbg -qrr ./vendor/bin/roave-infection-static-analysis-plugin';
	}
	return 'phpdbg -qrr ./vendor/bin/infection';

I would assume that we have something like this in-place for the psalm/phar detection to swap to the bundled config.xsd in that components folder (once its bundled).

---

Also created a patch in psalm to have the config.xsd bundled. vimeo/psalm#10955

[boesing]

Just to have my 2 cents dropped here regarding the usage of psalm/phar:
I wanted to use that a few years back as well, thats why I mentioned psalm/phar#14 in my 1st post.
The biggest problem with the PHAR is that u are unable to use psalm plugins at all.
IMHO, there are a bunch of decent plugins which provide more in-depth stuff such as the doctrine plugin, etc.

Thats why I ended up keeping vimeo/psalm in our projects, even tho having psalm/phar in-place would help us in reducing dependency conflicts. So depending on what the main goal of the initial change was to switch to psalm/phar, I would at least think about the plugin problematic once more.
But ofc, for most stuff, the psalm as is, is already good enough.

[zonuexe]

psalm/phar is important for michaelpetri/phpunit-consecutive-arguments because psalm is not compatible with PHP-Parser 5 and PHPUnit 11 until they release Psalm v6.

refs vimeo/psalm#10567, vimeo/psalm#10788 and vimeo/psalm#10888

[boesing]

I released v1.26.0 for the matrix action, you can - until we found a better solution - use exclude now to exclude auto-generated Psalm jobs.

Something like this is the repository you are mentioning could be implemented as .laminas-ci.json:

{
    "additional_checks": [
        {
             "name": "Static Code Analysis via psalm.phar",
             "job": {
                 "command": "./vendor/bin/psalm"
                 "php": "@lowest",
                 "dependencies": "locked"
             }
        }
    ],
    "exclude": [
       {"name": "Psalm"}
   ]
}

(I think the path is wrong so that would be another thing to consider when providing a patch for the alternative XSD path 😅)

---

Btw. I've read about your example regarding the extraction of the config.xsd. If that is something we can actually do, I would not mind having that in before_script 🤷🏼‍♂️ If that is how that has to work, could also be used for the standalone example Bruce mentioned (i.e. phive). But I'd say lets start with the psalm/phar example and see if some1 will ever need this to be supported for phive - I mean, we do not support phive installed phpunit so lets keep it simple for now.

[boesing]

Do you think you can provide a PR for that feature @zonuexe?
I'd be happy to review and create a new release for that, just hit me up!
In case you need help, you can reach out on slack, I have the same name there as on github 👍🏼