request #13038 Introduce static analysis of the PHP codebase with vimeo/psalm
    Thomas Gerbet (tgerbet)
    2019-04-03 10:14
    2019-02-26 15:45
    Introduce static analysis of the PHP codebase with vimeo/psalm
    Why introducing a static analysis tool?

    Static analysis tools can help to:
    - catch bugs (e.g. null reference)
    - maintain a certain level of code quality especially when the code base is large or when multiple contributors are implicated

    Why Psalm [0] over any other static analysis tools available?

    Psalm is not the only active static analysis tools you can find in the PHP community. PHPStan [1] and Phan [2] are also frequently encountered.

    Psalm a bit easier to introduce to existing codebase thanks to its "baseline" feature [3] which allows to ignore existing errors. It also allows to select exactly which rules must applied.

    Introduction into the Tuleap pipelines

    Proposal is to start small and only adds Psalm in the nightly test pipeline with a configuration quite permissive to see how well it plays with the code currently being integrated.
    Depending on the results, the configuration could be tweaked to match our actual needs and Psalm and then integrated into the main Tuleap test pipeline.

    Over time, the configuration should be made stricter and the issues in the existing code could be fixed (or the code removed, that works too).
    Some plugins could be developed to improve our confidence on some security sensitive tasks. For example we could ensure that the only thing concatenated to SQL queries are instances of ParagonIE\EasyDB\EasyStatement which would make hard to introduce SQL injections.

    [0] https://psalm.dev/
    [1] https://github.com/phpstan/phpstan
    [2] https://github.com/phan/phan
    [3] https://psalm.dev/docs/dealing_with_code_issues/#using-a-baseline-file
    Dev tools
    • [ ] enhancement
    • [x] internal improvement
    Thomas Gerbet (tgerbet)


    • User avatar
      Thomas Gerbet (tgerbet)2019-04-03 10:14
      Let's close this one, next stage will be dealt with in a dedicated request.

      I'm quite satisfied with the result after a short month:
      * bugs has been spotted thanks to it (request #13081)
      * I have found it quite helpful when doing contribution involving sizable refactoring (e.g. git #tuleap/stable/cc20e8a3ad7a4926023ed5d820d196159ed0382d)
      * the currently used profile (described by Psalm as "level 3") seems to be a good match for the code merged those days. It's a bit of a surprise, I would have more difficulties on this point. Once the usual contributors are used to the current settings it looks like we might be able to go to stricter settings.
      * no new crash or completely unexpected behavior of Psalm has been found while running in the nightly pipeline
      * helpers to launch it are available for Tuleap developers

      Next stage will mostly be about stabilization. Ultimately the goal should be to add it into the pipeline played on each patchset but only analyze modified files.

      • Status changed from Under implementation to Closed
      • Close date set to 2019-04-03
    • User avatar
      gerrit #14420 integrated into Tuleap
    • User avatar
      Thomas Gerbet (tgerbet)2019-03-19 11:16
      The helper `make psalm-with-info` does not do what it is supposed to do. It is expected that when using this helper, we got all the findings at the INFO level (that include findings covered by the baseline file) but we currently get the same thing than with `make psalm` (only the WARN level).

      Patch to fix this can be reviewed, see gerrit #14420.
    • User avatar
      gerrit #14415 integrated into Tuleap
    • User avatar
      Thomas Gerbet (tgerbet)2019-03-19 09:12
      I spotted an issue making the PHPUnit test suite brittle due to a test introduced by gerrit #14179.
    • User avatar
      gerrit #14309 integrated into Tuleap
    • User avatar

      gerrit #14179 integrated in Tuleap

      I let the request open to attach patch that will come from the nightly failures during 11.0 cycle.