Collision of interface constant and trait constant doesn't produce fatal error

Description

The following code:

<?php

trait MyTrait{
    const A = 1;
}
interface I{
    const A = 2;
}

class D implements I{
    use MyTrait;
}

echo D::A;

Resulted in this output:

But I expected this output instead:

Fatal error: I and MyTrait define the same constant (A) in the composition of D. However, the definition differs and is considered incompatible. Class was composed in /in/EVuKh on line 10

PHP Version

8.2

Operating System

No response

@sj-i

@sj-i FYI. Can you take a look? This is an example from the RFC:

trait T1 {
    public const CONSTANT = 42;
}

interface I {
    public const CONSTANT = 43;
}
 
// Fatal Error
class C4 implements I {
    use T1;
}

However, there is no error here.

I'm pretty sure this is expected behaviour since the change introduced by https://wiki.php.net/rfc/final_class_const where Interface constants are now override-able by default, and the trait overrides the constants.

@Girgias

@Girgias That makes sense. Then the example in the RFC was incorrect (the constants in traits RFC was voted on AFTER the final const RFC).

There are two possible interpretations here.

One is that class constants inherited from interfaces conflict with trait constants, and the other is that constants introduced into classes from traits can override constants defined in interfaces.

The RFC for trait constants adopts the former, in line with the following behavior for trait properties and class inheritances.
https://3v4l.org/84abA

So this is indeed a bug. I have a fix on my end and will PR it later.

I think it shouldn't matter if an interface constant is overridden by a constant defined in an implementing class or in a trait which is used by an implementing class, so I'm fine with the situation we have now (except if final interface constants are overridable).

I would rather recommend a more restrictive behavior as described in the RFC for now.

In the current implementation, trait constants can conflict with constants inherited from parent classes. If trait constants are allowed to override interface constants, then the behavior with inheritance needs to be changed as well.

And if we do so, it would become inconsistent with the current behavior for properties. In other words, the order of application for inheritance and trait usage would be different between constants and properties.

Unless there is a strong demand, I think it is easier and preferable to make the behavior as described in the RFC via merging #9274

Note that I would have no problem if trait constants could override constants in parent classes, as long as the behavior reflects further discussions and considerations in internals.

Although this is a comparatively trivial issue, I think it would be better to have a separate RFC for this change, as it should be consistent with the existing behavior for properties.

@adoy

And if we do so, it would become inconsistent with the current behavior for properties. In other words, the order of application for inheritance and trait usage would be different between constants and properties.

Yes, we have to choose which we want to be consistent with: in case of properties, the more restrictive behavior makes sense, as they cannot be defined in interfaces. On the other hand, constants can be, and IMO we should continue to allow classes/traits to override them, otherwise we'll have different behavior if a piece of code is added to a class via a trait or directly, which would seem like an unfortunate thing to do:

interface I {
    public const CONSTANT = 1;
}

trait T {
    public const CONSTANT = 2;
}

// Will trigger a fatal error after https://github.com/php/php-src/pull/9274 is merged
class C1 implements I {
    use T;
}

// Will continue to work
class C2 implements I {
    public const CONSTANT = 2;
}

Just like in case of #9285, I think the reasoning behind the alteration from the RFC is straightforward and doesn't need too much debate (in the sense of a full RFC process). I'd appreciate the opinion of the release managers though @adoy @TimWolla @ramsey

@kocsismate

@kocsismate You likely meant to mention @saundefined, not me.

Ahh, yes, sorry! :/

I would still recommend sticking with the behavior as described in the RFC for the time being, and changing it later if necessary, in a consistent way with the behavior for properties :)

otherwise we'll have different behavior if a piece of code is added to a class via a trait or directly, which would seem like an unfortunate thing to do

The breakage in that situation itself can be also considered as a valid result of a feature keeping consistency of the definitions, which we know as the compatibility checking.

Anyway, if RMs decide to allow trait constants to override constants from interfaces or parents of composing classes, I would like to see a decision on whether to fix the inconsistency with properties or not as well.

As now it's an error related to constant for extending a class: https://3v4l.org/14L0Z/rfc#vgit.master

interface I {
    public const CONSTANT = 1;
}

abstract class A {
    public const CONSTANT = 1;
}

trait T {
    public const CONSTANT = 2;
}


// works well, as it should
class C1 implements I {
    public const CONSTANT = 2;
}

// works well, as it should
class C2 extends A {
    public const CONSTANT = 2;
}

// works well now
class C3 implements I {
    use T;
}

// fatal error
class C4 extends A {
    use T;
}

I believe it's good to keep it in sync for interface as well. That is what https://wiki.php.net/rfc/final_class_const was fixing as well, the inconsistencies for constants overriding between class extending and interface implementing.

A common RFC to allow them both would be good later, to have the code in sync. Otherwise, if we don't fix them and RFC to allow it for class inheritance with trait overriding would never be approved, we would have an inconsistency forever.

Just to make it clear, I don't think it's bad to keep the behavior we have for interfaces but I don't like the inconsistency it brings in comparison with parent classes. So another option would be to allow Traits implemented in child classes to override non final constants from parent classes and to have this in PHP 8.2.
For inconsistency with properties, that's another story and I don't think that needs to be in sync with constants.

Someone has any update about this bug?

<?php
trait T1
{
	final public const string C1 = 'T1';
}

interface I1
{
	public const ?string C1 = null;
	public const ?string C2 = null;
}

final class O1 implements I1
{
	final public const string C2 = 'O1';
}

final class O2 implements I1
{
	use T1;
}

abstract class A1 implements I1
{
}

final class O3 extends A1
{
	final public const string C2 = 'O3';
}

final class O4 extends A1
{
	use T1;
}

We don't have the same behavior with trait for the class O4, returns:

Fatal error: I1 and T1 define the same constant (C1) in the composition of O4. However, the definition differs and is considered incompatible. Class was composed in /home/user/scripts/code.php on line 32

When I change the trait T1 with the following code, I get the same error:

trait T1
{
	public const ?string C1 = 'T1';
}

Thank you

(Tested with PHP v8.3.10)

Navigation Menu

Repository navigation

Collision of interface constant and trait constant doesn't produce fatal error #9271

Description

PHP Version

Operating System

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Collision of interface constant and trait constant doesn't produce fatal error #9271

Description

Description

PHP Version

Operating System

Activity

iluuu1994 commented on Aug 8, 2022

Girgias commented on Aug 8, 2022

iluuu1994 commented on Aug 8, 2022

sj-i commented on Aug 8, 2022

kocsismate commented on Aug 21, 2022

sj-i commented on Aug 21, 2022

sj-i commented on Aug 21, 2022

kocsismate commented on Aug 22, 2022

TimWolla commented on Aug 22, 2022

kocsismate commented on Aug 22, 2022

sj-i commented on Aug 22, 2022

drealecs commented on Aug 22, 2022

Renji-FR commented on Oct 3, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions