Skip to content

[OffsetAccessOnStringRule] Initial implementation#1310

Closed
iluuu1994 wants to merge 1 commit into
phpstan:masterfrom
iluuu1994:feature/offset-access-on-string
Closed

[OffsetAccessOnStringRule] Initial implementation#1310
iluuu1994 wants to merge 1 commit into
phpstan:masterfrom
iluuu1994:feature/offset-access-on-string

Conversation

@iluuu1994

Copy link
Copy Markdown
Contributor

Closes #1308

@iluuu1994 iluuu1994 force-pushed the feature/offset-access-on-string branch from 9c8fa0a to 9be9be9 Compare July 29, 2018 18:34
@ondrejmirtes

Copy link
Copy Markdown
Member

I believe part of this is already covered by https://github.com/phpstan/phpstan/blob/master/src/Rules/Arrays/NonexistentOffsetInArrayDimFetchRule.php and the other part would be covered by my suggestion in #1307. But it's always good to at least recycle these tests to corresponding rules once they're implemented so I'm leaving this open.

@ondrejmirtes

Copy link
Copy Markdown
Member

See https://phpstan.org/r/4b9affb1252f4ccab55ef797703101cb - it's already handled.

It does not show up here https://phpstan.org/r/2445f50bdd5edf1fd8cee57bf45fd38d because $foo becomes an array on line 7.

@iluuu1994

Copy link
Copy Markdown
Contributor Author

I don't understand. This should report and error:

https://phpstan.org/r/dbda208449aa9798e2901be22bfba409

So should this:

https://phpstan.org/r/9f1b618077bf1b9ca1db1c6051378eb1

@ondrejmirtes

Copy link
Copy Markdown
Member

There's no rule for checking that setOffsetValueType returns ErrorType. I suggested that here:
#1307 (comment)

@ondrejmirtes

Copy link
Copy Markdown
Member

To summarize - there's already a rule for hasOffsetValueType. It can be improved by grabbing relevant testcases from this PR and fixing those cases.

I also suggested in #1307 to add a rule that checks whether setOffsetValueType return ErrorType in cases of $foo['bar'] = 'baz';

@iluuu1994

Copy link
Copy Markdown
Contributor Author

@ondrejmirtes I see. Sorry for the confusion 😆

@ondrejmirtes

Copy link
Copy Markdown
Member

I've recycled your tests and used them with the current rules and typesystem in this commit: 7a630b8

Thanks!

@iluuu1994 iluuu1994 deleted the feature/offset-access-on-string branch November 7, 2018 12:49
@iluuu1994

Copy link
Copy Markdown
Contributor Author

@ondrejmirtes Cool, thanks 🙂

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