vimeo/psalm

Psalter with MissingPureAnnotation makes unnecessary docblock changes

Open

#8,893 opened on Dec 12, 2022

View on GitHub
 (6 comments) (0 reactions) (0 assignees)PHP (5,369 stars) (668 forks)batch import
Help wantedbug

Description

Some psalter options make a lot of unnecessary changes to the docblocks by reformatting them to have a "blank" docblock line between each annotation even if it didn't make any other changes. For example, this will do that:

psalm --no-cache --alter --issues=MissingPureAnnotation

One reproduce is when it finds something is already correctly annotated as @psalm-pure and has a @psalm-assert annotation below that. It doesn't seem to have this issue with some other annotations such as if @psalm-return were below @psalm-pure instead of @psalm-assert. Here is an example:

https://psalm.dev/r/ae01ce15ca

For isTrue() it will add an extra docblock line between the annotations. A more destructive example is that with isFalse(), it will delete the code comment describing what the method does and turn it into a "blank" docblock line.

I wanted to see what other people think about this. Do you consider it standard to have an extra line between each annotation? Are there standard PHP tools that will automatically adjust all the docblocks at once to be formatted this way? Do you think it is appropriate for Psalm to make formatting changes in this way?

Personally I think it goes a long way toward making a very powerful tool like psalter a lot less practically usable because it takes a long time to go through each change and decide if you want to keep it or not. Ideally you could configure it such that you can run it frequently and expect it to produce no changes if you already have a clean code base. When it does produce changes you should typically like the ones it makes or be able to configure it to stop making those particular changes.

Contributor guide