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.1] Non-static methods called as static from object context #41661

Draft
wants to merge 1 commit into
base: 5.1-dev
Choose a base branch
from

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

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.

None yet

4 participants