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

IncomingRequest property $uri is public and should be protected #5344

Closed
pawelkg opened this issue Nov 18, 2021 · 5 comments
Closed

IncomingRequest property $uri is public and should be protected #5344

pawelkg opened this issue Nov 18, 2021 · 5 comments
Labels
breaking change Pull requests that may break existing functionalities bug Verified issues on the current code behavior or pull requests that will fix them next major version? Read this for a relevant v5 idea

Comments

@pawelkg
Copy link
Contributor

pawelkg commented Nov 18, 2021

PHP Version

8.0

CodeIgniter4 Version

4.1.5

Which operating systems have you tested for this bug?

Linux

Which server did you use?

apache

Database

No response

What happened?

It's not a bug, but an inconsistency. Class Request have property $uri which is protected, but IncomingRequest class change this access level to public. It's should be protected also, especially that there is getUri() method to get it. Unfortunetly I don't see this method in CodeIniter documentation. This is why I assume that most of people get this by property name, not method (as I done it first), so changing it can couse problems in projects.

Steps to Reproduce

In eg. Controller usage of $this->request->uri Should not be available. Should use this: $this->request->getUri()

Expected Output

$this->request->uri
Cannot access protected property CodeIgniter\HTTP\IncomingRequest::$uri

Anything else?

No response

@pawelkg pawelkg added the bug Verified issues on the current code behavior or pull requests that will fix them label Nov 18, 2021
kenjis added a commit to kenjis/CodeIgniter4 that referenced this issue Nov 18, 2021
kenjis added a commit to kenjis/CodeIgniter4 that referenced this issue Nov 18, 2021
@kenjis kenjis changed the title IncomingRequest property is public and should (?) be protected IncomingRequest property $uri is public and should (?) be protected Nov 18, 2021
@kenjis kenjis changed the title IncomingRequest property $uri is public and should (?) be protected IncomingRequest property $uri is public and should (?) be protected Nov 18, 2021
@kenjis
Copy link
Member

kenjis commented Nov 18, 2021

Thank you!
I agree with you.

Probably it seems $uri is public because of historical reasons.
And we can't change the visibility in short term, because it is a BC break.

I sent a PR #5345 for now.

kenjis added a commit to kenjis/CodeIgniter4 that referenced this issue Nov 18, 2021
@kenjis kenjis added breaking change Pull requests that may break existing functionalities and removed bug Verified issues on the current code behavior or pull requests that will fix them labels Nov 20, 2021
@kenjis kenjis changed the title IncomingRequest property $uri is public and should (?) be protected IncomingRequest property $uri is public and should be protected Nov 20, 2021
@kenjis kenjis added the next major version? Read this for a relevant v5 idea label Nov 20, 2021
@kenjis
Copy link
Member

kenjis commented Nov 20, 2021

Ref #3230

@MGatner
Copy link
Member

MGatner commented Nov 27, 2021

Yes as @kenjis pointed out we are stuck with this for now for backwards-compatibility but very much would like it changed. Version 5 already has plans on refactoring the HTTP layer to be PSR-compliant so that will address this. Safe to close this issue.

@kenjis kenjis added the bug Verified issues on the current code behavior or pull requests that will fix them label Sep 30, 2023
@kenjis
Copy link
Member

kenjis commented Sep 30, 2023

We deprecated $uri in v4.3.0. #6662
So we can change it in 4.5.

The deprecated items are not covered by backwards compatibility (BC) promise. It may be removed in the next next minor version or later. For example, if an item has been deprecated since 4.3.x, it may be removed in 4.5.0.
https://codeigniter4.github.io/CodeIgniter4/installation/backward_compatibility_notes.html#what-are-not-breaking-changes

@kenjis
Copy link
Member

kenjis commented Oct 27, 2023

Closed by #8067

@kenjis kenjis closed this as completed Oct 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change Pull requests that may break existing functionalities bug Verified issues on the current code behavior or pull requests that will fix them next major version? Read this for a relevant v5 idea
Projects
None yet
Development

No branches or pull requests

3 participants