Introduce Static Analysis Checking System

[AlexandruGG]

This PR aims to kickstart improvements to static analysis
Follows #89.

Overall

• Increase Psalm strictness & add baseline
• Introduce tests/static-analysis
• Fixing some issues where applicable

Checks added

• fromFile
• fromResource
• fromString

[drupol]

It seems that ::fromFile() is a new constructor.

I've always been reluctant to add it because I haven't found a proper way to do the fclose() at the end. Do you have a clue??

I'm not so keen on leaving the resource open without closing them.

[AlexandruGG]

It seems that ::fromFile() is a new constructor.

I've always been reluctant to add it because I haven't found a proper way to do the fclose() at the end. Do you have a clue??

I'm not so keen on leaving the resource open without closing them.

Challenge accepted! 😄

Let me know if this looks good: e000805

[AlexandruGG]

It seems that ::fromFile() is a new constructor.
I've always been reluctant to add it because I haven't found a proper way to do the fclose() at the end. Do you have a clue??
I'm not so keen on leaving the resource open without closing them.

Challenge accepted! 😄

Let me know if this looks good: e000805

I realised that this iterator is also used in fromResource, where the user already provides a resource. We might not want to close it in that case as it should be up to the caller, so I'm thinking to add a boolean parameter to ResourceIterator with default value false that should be used to check if the resource should be closed. Does that sound good?

[drupol]

I realised that this iterator is also used in fromResource, where the user already provides a resource. We might not want to close it in that case as it should be up to the caller, so I'm thinking to add a boolean parameter to ResourceIterator with default value false that should be used to check if the resource should be closed. Does that sound good?

This is exactly why it is done like that, I let the user close its resource.

In overall, I'm ok with the pull request, feel free to submit it!

[AlexandruGG]

I realised that this iterator is also used in fromResource, where the user already provides a resource. We might not want to close it in that case as it should be up to the caller, so I'm thinking to add a boolean parameter to ResourceIterator with default value false that should be used to check if the resource should be closed. Does that sound good?

This is exactly why it is done like that, I let the user close it's resource.

In overall, I'm ok with the pull request, feel free to submit it!

Ok so we don't want ResourceIterator to always close the resource. I will add that parameter to ensure that we only close it in fromFile and not in fromResource.

Also, I'll add the static analysis check for fromResource and after that it'll be ready for review :)

[drupol]

Maybe a new extra parameter to ResourceIterator ?

[AlexandruGG]

@drupol I see a mix of annotations in this project, but I'm not sure why that is. Should we use the psalm ones or the "normal" ones everywhere?

A. @param, @return, @template
B. @psalm-param, @psalm-return, @psalm-template

For the analysers it doesn't matter because they support both. And in an IDE like PHPStorm / IntelliJ both are properly highlighted.

I personally favour A just because of a couple small reasons:

• less characters to write. And easier to mentally parse when reading 😄
• doesn't tie the code to a specific developer tool (though, again, doesn't really matter because both are supported)

[drupol]

This is also on my Todo list! Option A for the win :)

[drupol]

Excellent!