Coder Social home page Coder Social logo

Comments (15)

boesing avatar boesing commented on June 12, 2024 1

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?

from laminas-ci-matrix-action.

Ocramius avatar Ocramius commented on June 12, 2024 1

@zonuexe good shout, sorry for jumping to conclusions!

from laminas-ci-matrix-action.

boesing avatar boesing commented on June 12, 2024 1

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. 🤔

from laminas-ci-matrix-action.

boesing avatar boesing commented on June 12, 2024 1

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?

from laminas-ci-matrix-action.

zonuexe avatar zonuexe commented on June 12, 2024 1

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

from laminas-ci-matrix-action.

boesing avatar boesing commented on June 12, 2024 1

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.

from laminas-ci-matrix-action.

Ocramius avatar Ocramius commented on June 12, 2024

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.

from laminas-ci-matrix-action.

zonuexe avatar zonuexe commented on June 12, 2024

@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.

from laminas-ci-matrix-action.

Ocramius avatar Ocramius commented on June 12, 2024

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

from laminas-ci-matrix-action.

boesing avatar boesing commented on June 12, 2024

@Ocramius you mean extracting from psalm.xml?

from laminas-ci-matrix-action.

Ocramius avatar Ocramius commented on June 12, 2024

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

from laminas-ci-matrix-action.

Ocramius avatar Ocramius commented on June 12, 2024

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 👍

from laminas-ci-matrix-action.

boesing avatar boesing commented on June 12, 2024

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:

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

from laminas-ci-matrix-action.

boesing avatar boesing commented on June 12, 2024

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.

from laminas-ci-matrix-action.

boesing avatar boesing commented on June 12, 2024

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 👍🏼

from laminas-ci-matrix-action.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.