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

Adds ServerVariableToRequestFacadeRector rule #282

Merged
merged 1 commit into from
Jan 2, 2025

Conversation

peterfox
Copy link
Collaborator

Changes

  • new ServerVariableToRequestFacadeRector rule
  • adds tests
  • updates docs

Why

This converts a simple use of the $_SERVER PHP variable to use the Request facade.

-$_SERVER['VARIABLE'];
+\Illuminate\Support\Facade\Request::server('VARIABLE');

@peterfox peterfox added the enhancement New feature or request label Dec 22, 2024
@peterfox peterfox self-assigned this Dec 22, 2024
@GeniJaho
Copy link
Collaborator

@peterfox Are there any similar cases to the other PR of the $_SESSION global? I think we can include more cases in this rule, with a similar implementation.

@peterfox
Copy link
Collaborator Author

Nothing I can think of. I need to look at getallheaders but I'm not sure how 1:1 that is.

I think the only thing is possibly some key values such as:

$_SERVER['HTTP_USER_AGENT'];
$_SERVER['REQUEST_METHOD'];

Would be better replaced by dedicated method calls. That might be better off with a dedicated rule though, so you'd apply this rule and then one to transform \Illuminate\Support\Facade\Request::server('HTTP_USER_AGENT') into \Illuminate\Support\Facade\Request::userAgent().

@peterfox peterfox force-pushed the feature/convert-server-variable branch from c2226a0 to 6651ed4 Compare December 30, 2024 20:15
@peterfox peterfox requested a review from GeniJaho December 30, 2024 21:15
@GeniJaho GeniJaho merged commit b680c0f into main Jan 2, 2025
5 checks passed
@GeniJaho GeniJaho deleted the feature/convert-server-variable branch January 2, 2025 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants