Skip to content

Respect phpstan level in InvalidKeyInArrayDimFetchRule #4166

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: 2.1.x
Choose a base branch
from

Conversation

VincentLanglet
Copy link
Contributor

@VincentLanglet VincentLanglet commented Jul 26, 2025

Extracted from #4012

It will be easier to review the CastArrayKeyRule after

@VincentLanglet VincentLanglet force-pushed the invalidKey branch 2 times, most recently from b32f7fe to 2caa239 Compare July 26, 2025 16:39
Copy link
Member

@ondrejmirtes ondrejmirtes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're doing something to respect levels better, you need to add some cases to LevelsIntegrationTest. You can just add the "topic" php file and run the test, it will generate JSONs with the current result.

It'd actually be great to see the current behaviour in the first commit and then see how things get corrected in the second commit with the code changes.

The expectations are:

  • level < 7 - report things that are always wrong
  • level 7 - report things that might be wrong, like int|string only where int is expected
  • level 8 - same but for int|null
  • level 9 - explicit mixed
  • level 10 - implicit (untyped) mixed

@ondrejmirtes
Copy link
Member

The best way to get LevelsIntegrationTest results fast is to comment out all the other "topics" when you're working on a single topic. Otherwise it takes a long time for the tests to run.

Also you could add some upload-artifact action call to tests-levels in tests.yml so that when PR run fails, we can download the updated JSON files and there's no need to run the test locally and slowly.

@VincentLanglet VincentLanglet force-pushed the invalidKey branch 4 times, most recently from cc2bcaf to da3e7fd Compare July 26, 2025 22:23
@VincentLanglet
Copy link
Contributor Author

Ok, so here you have

  • First commit introduce the integration test with the current behavior
    5e15e2e

  • Second commit is my fix of InvalidKeyInArrayDimFetchRule and the impact
    ae06b48

  • Funny things, the message is different between level 7 and 8, and I found the fix needed was in RuleLevelHelper:
    38c316f
    -> You should not remove null if it's an accepted type in the Union (because if you do so, you might transform a "maybe error" into a "100% sure error".

Then last commit was just to fix the lint 7.4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants