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

[5.2] Non-static methods called as static from object context #41661

Conversation

Denitz
Copy link
Contributor

@Denitz Denitz commented Sep 8, 2023

Summary of Changes

Non-static methods called as static from object context

Testing Instructions

Apply patch

Actual result BEFORE applying this Pull Request

See IDE warnings

Expected result AFTER applying this Pull Request

No IDE warnings.

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

@HLeithner HLeithner changed the title 5.0 Non-static methods called as static from object context [5.0] Non-static methods called as static from object context Sep 12, 2023
@HLeithner
Copy link
Member

This Pull request changes the behavior. self calls the function defined by it "self" or "parent" class but not if defined in child class.
Example: https://3v4l.org/TtuJ3

<?php

class a {
    function x() {
        echo 'x' . chr(10);
    }
}

class b extends a {
    function y() {
        echo 'y' . chr(10);
        self::x();    
        $this->x();    
    }
    
}

class c extends b {
    function z() {
        echo 'z' . chr(10);
        self::y();
    }

    function x() {
        echo 'x2' . chr(10);
    }
}

echo (new c)->z();

Output:

z
y
x
x2

if someone has time to evaluate what we really want it would be great.

@HLeithner HLeithner marked this pull request as draft September 24, 2023 08:08
@HLeithner HLeithner added Feature b/c break This item changes the behavior in an incompatible why. HEADS UP labels Sep 24, 2023
@HLeithner HLeithner mentioned this pull request Sep 24, 2023
4 tasks
@HLeithner HLeithner changed the base branch from 5.0-dev to 5.1-dev September 30, 2023 22:49
@HLeithner
Copy link
Member

This pull request has been automatically rebased to 5.1-dev.

@Denitz Denitz changed the title [5.0] Non-static methods called as static from object context [5.1] Non-static methods called as static from object context Oct 3, 2023
@Hackwar
Copy link
Member

Hackwar commented Feb 21, 2024

I disagree that this is a b/c break. Yes, in theory the behavior can be different, but in reality this is simply super old code (The line in the banner table for example is at least 14 years old.) and at that time the code quality wasn't the best... With a codereview this could simply be merged.

@Hackwar Hackwar added bug and removed b/c break This item changes the behavior in an incompatible why. HEADS UP Feature labels Feb 21, 2024
@Hackwar
Copy link
Member

Hackwar commented Feb 21, 2024

Ok, I finally found it. The line in the banner table was introduced close to 15 years ago in one of those major rewrites: 81e0102#diff-220ad14bb0e7e21591b2575dfb72a5edb7bfbffff4d8069de75e7bcaf8874acdR118

@crommie
Copy link

crommie commented Aug 24, 2024

In what situation(s) should we see the warnings?


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/41661.

@HLeithner HLeithner changed the base branch from 5.1-dev to 5.2-dev September 2, 2024 08:28
@HLeithner
Copy link
Member

This pull request has been automatically rebased to 5.2-dev.

@HLeithner HLeithner changed the title [5.1] Non-static methods called as static from object context [5.2] Non-static methods called as static from object context Sep 2, 2024
@Hackwar Hackwar marked this pull request as ready for review September 11, 2024 14:06
@pe7er pe7er self-assigned this Sep 11, 2024
@Hackwar Hackwar merged commit e704b9a into joomla:5.2-dev Sep 11, 2024
0 of 2 checks passed
@Hackwar
Copy link
Member

Hackwar commented Sep 11, 2024

Thank you for your contribution @Denitz!

@Hackwar Hackwar added this to the Joomla! 5.2.0 milestone Sep 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants