Better type inference, improved test coverage throughout locked dependencies, automated dependency upgrades

[Ocramius]

This patch is mostly raised by the upgrade of 0.3.0 to 0.4.0 breaking BC (expected, but preventable).

In practice, Valinor is probably going to be used because of TreeMapper#map(class-string<T>, mixed): T in the grand majority of cases.

To cover this scenario, and prevent regressions, I introduced:

• testing against locked dependencies, so that we can pinpoint tiny upgrades that break cuyz/valinor as they come up
• automated dependency widening (I would prefer bumping, but that's really your decision)
• vimeo/psalm type inference testing
• better types around TreeMapper#map(), so that it works without coupling cuyz/valinor to a specific vimeo/psalm version (the type declarations for the majority scenario are in the code, without any need for plugins)

[Ocramius]

Note: mutation tests fail because of no actual source diff in the patch.

Everything is done in:

• composer.json + composer.lock
• github actions manifests
• dependabot manifest
• docblocks

[romm]

Looking nice, thank you!

I'd like some explanation on the Dependabot thing, I often saw it on popular repos but I'm not sure how it works. 😅

Also, I try to keep using conventional commits for this repo (for consistency mainly). What would be your advice for this kind of PR where fixup commits were made? Should I squash it with a correct title/description when submitting the PR?

[romm]

Note: mutation tests fail because of no actual source diff in the patch.

Everything is done in:

* `composer.json` + `composer.lock`

* github actions manifests

* dependabot manifest

* docblocks

@Ocramius damn I don't get why it would work in #60 but not here. Any recommendation on how to fix this?

[Ocramius]

I'd like some explanation on the Dependabot thing, I often saw it on popular repos but I'm not sure how it works. sweat_smile

It will, once a week, look for new dependencies, composer.json and composer.lock accordingly (and also .github/* actions references), and send you PR for review for each dependency, so you don't need to chase changes in the ecosystem.

It will also rebase any PRs it currently has open, should composer.json and composer.lock change in between, so you don't need to do that work yourself.

Also, I try to keep using conventional commits for this repo (for consistency mainly).

I can squash out some commits, but no idea about what the actual commits here should be then? I keep them very much human-friendly, rather than "changed X", so if you want specific messages, please do tell.

What would be your advice for this kind of PR where fixup commits were made? Should I squash it with a correct title/description when submitting the PR?

Fixup commits will be squashed from my end, after review is through.

[Ocramius]

Note: instead of a51f8c5 , I will rebase this patch post-review.

Merges of mainline into feature branches are really to be avoided (in pretty much every environment), and are a much bigger problem than conventional commits ;-)

[Ocramius]

Note: mutation tests fail because of no actual source diff in the patch.
Everything is done in:

* `composer.json` + `composer.lock`

* github actions manifests

* dependabot manifest

* docblocks

@Ocramius damn I don't get why it would work in #60 but not here. Any recommendation on how to fix this?

This is probably because I have a diff in src/, but it doesn't involve any AST node changes, beside docblock

[romm]

I can squash out some commits, but no idea about what the actual commits here should be then? I keep them very much human-friendly, rather than "changed X", so if you want specific messages, please do tell.

The commits names are good, but what is missing is a correct prefix — see conventional commits. This allows me to use marcocesarato/php-conventional-changelog for the changelog automatic generation.

Note: instead of a51f8c5 , I will rebase this patch post-review.

Merges of mainline into feature branches are really to be avoided (in pretty much every environment), and are a much bigger problem than conventional commits ;-)

Right, I usually use rebase strategies on my own branches, but I was not sure if that would fit on a PR managed by someone else. Learning basic OSS contribution rules here :D

This is probably because I have a diff in src/, but it doesn't involve any AST node changes, beside docblock

Ah! Right, must be that. Any idea how we could fix it, or avoid this case?

[Ocramius]

I can squash out some commits, but no idea about what the actual commits here should be then? I keep them very much human-friendly, rather than "changed X", so if you want specific messages, please do tell.

The commits names are good, but what is missing is a correct prefix — see conventional commits. This allows me to use marcocesarato/php-conventional-changelog for the changelog automatic generation.

Rebased and added prefixes 💪

Note: instead of a51f8c5 , I will rebase this patch post-review.
Merges of mainline into feature branches are really to be avoided (in pretty much every environment), and are a much bigger problem than conventional commits ;-)

Right, I usually use rebase strategies on my own branches, but I was not sure if that would fit on a PR managed by someone else. Learning basic OSS contribution rules here :D

Understandable: most people probably roll their eyes when I ask them for a rebase, but I do git rebase 10~20 times a day for my own stuff ;-)

This is probably because I have a diff in src/, but it doesn't involve any AST node changes, beside docblock

Ah! Right, must be that. Any idea how we could fix it, or avoid this case?

This is potentially a bug in infection/infection - I wouldn't worry too much about it, for now.

[romm]

Rebased and added prefixes 💪

Great! Not so important but FYI:

• For consistency the first letter of the subject is lowercase
• I force myself to have commit subjects that are less than 72 chars, this eases readability of the history — for instance GitHub will "break" subjects with more than 72 chars, this makes it harder to read:
  

These are non-blocking from my POW but just wanted to share. 🙂

Understandable: most people probably roll their eyes when I ask them for a rebase, but I do git rebase 10~20 times a day for my own stuff ;-)

I also do, this is without doubt the best strategy to adopt, although rebasing on a contributor PR could also frighten the author if not used to it 😬

This is potentially a bug in infection/infection - I wouldn't worry too much about it, for now.

Alright, I'll see if I can report/fix it.

[romm]

Just though I could easily reword the commits to fit the requirements I was writing about earlier — the sense doesn't change, it just fits the 72 chars threshold. Hope you don't mind? I'm not sure if that can seem rude. 😬

Anyway, thanks a lot for the contribution @Ocramius!

[romm]

@Ocramius I'm not sure whether I should enable "Dependabot security updates" in the repository settings?

[Ocramius]

Thanks @romm!

Enabling security updates shouldn't be necessary: weekly updates are enough for a library