laminas / technical-steering-committee

Laminas Project Technical Steering Committee organization and processes.
BSD 3-Clause "New" or "Revised" License
39 stars 23 forks source link

RFC: Standardise Psalm Configuration #127

Open gsteel opened 2 years ago

gsteel commented 2 years ago

Thanks to @boesing, I recently discovered some psalm configuration options that'll be potentially helpful to improving code quality.

I'd like to start discussion about which of those options we should switch on or off by default for Laminas and Mezzio projects so that we can define a standard configuration.

Once a standard has been decided, it should be applied everywhere with an expanded baseline so that new code will need to pass a stricter set of requirements as enforced by CI.

To start discussion off, I'm thinking something like this:

<?xml version="1.0"?>
<psalm
    errorLevel="1"
    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
    xmlns="https://getpsalm.org/schema/config"
    xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd"
    errorBaseline="psalm-baseline.xml"
    ignoreInternalFunctionFalseReturn="false"
    ignoreInternalFunctionNullReturn="false"
    allowInternalNamedArgumentCalls="false"
    allowNamedArgumentCalls="false"
    strictBinaryOperands="true"
    disableSuppressAll="true"
    memoizeMethodCallResults="true"
    checkForThrowsDocblock="false"
    findUnusedVariablesAndParams="true"
    findUnusedCode="false"
    findUnusedPsalmSuppress="true"
>
    <projectFiles>
        <directory name="src"/>
        <directory name="test"/>
        <ignoreFiles>
            <directory name="vendor"/>
        </ignoreFiles>
    </projectFiles>

    <plugins>
        <pluginClass class="Psalm\PhpUnitPlugin\Plugin"/>
    </plugins>
</psalm>

Reference: https://psalm.dev/docs/running_psalm/configuration/

Perhaps, once decided, a template can be stored in the Laminas .github organisation repo?

Ocramius commented 2 years ago

Before going down this line, perhaps we could ask for psalm itself to have this sort of "stricter" defaults with a single flag? Maybe errorLevel="0"? :P

/cc @orklah

orklah commented 2 years ago

Yeah, that'd be great, this has been suggested in Psalm issue tracker also: https://github.com/vimeo/psalm/issues/7630

/cc @AndrolGenhald

boesing commented 2 years ago

I'd like to propose these two as well.

https://psalm.dev/docs/running_psalm/configuration/#ensurearraystringoffsetsexist https://psalm.dev/docs/running_psalm/configuration/#ensurearrayintoffsetsexist

But I'd expect them to be enabled with the strictest option provided by https://github.com/vimeo/psalm/issues/7630