Skip to content
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

define: case_insensitive is ignored, case-insensitive constants are no longer supported #589

Merged
merged 5 commits into from
Oct 15, 2021

Conversation

mglaman
Copy link
Contributor

@mglaman mglaman commented Jul 23, 2021

case_insensitive is ignored, case-insensitive constants are no longer supported

phpstan/phpstan#5368

@ondrejmirtes ondrejmirtes force-pushed the master branch 2 times, most recently from efd31ce to 0471f87 Compare July 29, 2021 14:08
@mglaman mglaman changed the title PHP8 signature map for define define: case_insensitive is ignored, case-insensitive constants are no longer supported Oct 7, 2021
@mglaman mglaman force-pushed the php8-define-case-insenstivie branch from 4c26321 to 41e48b4 Compare October 7, 2021 20:37
@mglaman
Copy link
Contributor Author

mglaman commented Oct 7, 2021

I've made a new rule which checks if argument 3 is provided on PHP 8.

@mglaman
Copy link
Contributor Author

mglaman commented Oct 8, 2021

@ondrejmirtes thanks for the feedback! I'll pick this up next week during my allotted phpstan dev time

@mglaman mglaman force-pushed the php8-define-case-insenstivie branch from 0e8e597 to 2085a64 Compare October 14, 2021 20:01
Comment on lines +150 to +153
public function supportsCaseInsensitiveConstantNames(): bool
{
return $this->versionId < 80000;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ondrejmirtes this handles PHP 8 not supporting it at all. What about handling the fact it was deprecated in 7.3

Per https://www.php.net/manual/en/function.define.php,

Defining case-insensitive constants is deprecated as of PHP 7.3.0.

Should we add another method that checks the PHP version ID if they're deprecated so we can report that error in our rule? Or just not supported and that's it.

Copy link
Member

Choose a reason for hiding this comment

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

Reporting something on 7.3 and below 8.0 is somewhat a different scenario that's a better fit for https://github.com/phpstan/phpstan-deprecation-rules.

@mglaman mglaman force-pushed the php8-define-case-insenstivie branch from 2085a64 to 99cacf6 Compare October 14, 2021 20:04
@mglaman mglaman force-pushed the php8-define-case-insenstivie branch from 99cacf6 to 4dbefe4 Compare October 14, 2021 20:11
@ondrejmirtes ondrejmirtes merged commit 732d033 into phpstan:master Oct 15, 2021
@ondrejmirtes
Copy link
Member

Thank you!

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