Skip to content

Implement FilterVarDynamicReturnTypeExtension#1900

Merged
ondrejmirtes merged 1 commit intophpstan:masterfrom
iluuu1994:feature/filter-var-dynamic-return-type-extension
Feb 27, 2019
Merged

Implement FilterVarDynamicReturnTypeExtension#1900
ondrejmirtes merged 1 commit intophpstan:masterfrom
iluuu1994:feature/filter-var-dynamic-return-type-extension

Conversation

@iluuu1994
Copy link
Copy Markdown
Contributor

No description provided.

@iluuu1994 iluuu1994 force-pushed the feature/filter-var-dynamic-return-type-extension branch 2 times, most recently from b2f367b to 95607be Compare February 14, 2019 12:08
composer.json Outdated
],
"require": {
"php": "~7.1",
"ext-filter": "*",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this should be in require-dev, as it is not required for analysis.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Nope, it's required. The constants are used in the type extension rule in the src directory. Remember that this doesn't mean your application will require it in production.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

as it is not required for analysis

To elaborate on this, it is required since I use the FILTER_ constants directly. I could probably do an AST string comparison.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

And what if I don't use filter_var function in my application? Then phpstan will not use this extension. Is it required then?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes. Remember that ext-filter is enabled by default. We also don't check if you have pcre enabled, although you could technically disable it. Most major libraries depend on it. The only reason I added it is because the build fails without it (composer-require-checker).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I got rid of the dependency.

@iluuu1994 iluuu1994 force-pushed the feature/filter-var-dynamic-return-type-extension branch 2 times, most recently from fc4226a to 0f01583 Compare February 14, 2019 13:03
@iluuu1994
Copy link
Copy Markdown
Contributor Author

@ondrejmirtes Does this have to wait for 0.12? This could break builds because mixed is ignored in lots of type checks while string|false etc. isn't.

@iluuu1994 iluuu1994 force-pushed the feature/filter-var-dynamic-return-type-extension branch from 0f01583 to 1332300 Compare February 17, 2019 13:33
@ondrejmirtes
Copy link
Copy Markdown
Member

Looks great, thank you! These kinds of changes don't have to wait for major version.

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.

3 participants