Skip to content

Conversation

lcobucci
Copy link

This implements the necessary rules for validating namespace visibility semantics for interfaces, classes, and traits.

  • Create a test scenario for class instantiation
  • Create a test scenario for interface implementation
  • Create a test scenario for class/interface inheritance
  • Create a test scenario for trait usage
  • Create a test scenario for type references
  • Create a test scenario for declaration inconsistencies
  • Get feedback
  • Implement rules

This requires PHP 8.1+ so we can make use of features like enums.

Signed-off-by: Luís Cobucci <lcobucci@gmail.com>
@lcobucci
Copy link
Author

A thought: should we allow defining the depth for protected visibility (or another access modifier, idk)?

The use case would be to define a component that could only be accessed by the same namespace or components in the X-levels (I only think about the first level, honestly). Like:

namespace Example {
    #[NamespaceVisibility(AccessModifier::ProtectedPrivate)] // Terrible name, I know =)
    interface Testing {}
}

namespace Example\Nested {
    use Example\Testing;

    final class Test implements Testing {} // OK
}

namespace Example\Nested\Nested {
    use Example\Testing;

    final class Test implements Testing {} // ERROR
}

This relies on the proposed attribute and enum to define possible
scenarios for handling validations for namespace visibility. Most of the
cases are based on the "Namespace visibility" RFC.

More info:

- https://wiki.php.net/rfc/namespace-visibility
- DaveLiddament/php-language-extensions#3 (comment)

Signed-off-by: Luís Cobucci <lcobucci@gmail.com>
@lcobucci lcobucci force-pushed the namespace-visibility branch from 94386ec to 72d1955 Compare June 27, 2022 23:39
Comment on lines +24 to +36
#[NamespaceVisibility(AccessModifier::Public)]
interface PublicInterface
{
public function arguments(
PublicDto $public, // OK
ProtectedDto $protected, // ERROR (it doesn't make sense for public interface to receive non-public args)
PrivateDto $private, // ERROR (it doesn't make sense for public interface to receive non-public args)
): void;

public function return1(): PublicDto; // OK
public function return2(): ProtectedDto; // ERROR (it doesn't make sense for public interface to return non-public types)
public function return3(): PrivateDto; // ERROR (it doesn't make sense for public interface to return non-public types)
}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not entirely sure if these checks make sense, honestly... they can become a bit more complicated when throwing inheritance into the mix... imagine this:

namespace Example {
    #[NamespaceVisibility(AccessModifier::Protected)]
    final class ProtectedDto;

    #[NamespaceVisibility(AccessModifier::Protected)]
    interface ProtectedInterface {
        public function example(): ProtectedDto;
    }
}

namespace Example\Nested {

    #[NamespaceVisibility(AccessModifier::Public)]
    final class Test implements \Example\ProtectedInterface
    {
        public function example(): \Example\ProtectedDto
        {
            return new \Example\ProtectedDto();
        }        
    }
}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From an encapsulation perspective, I believe that example shouldn't report any error...

@DaveLiddament
Copy link
Owner

Thanks for this contribution. I'm about to start my day at work. I'll take a fuller look this evening (UK time).

@lcobucci
Copy link
Author

Thanks for this contribution. I'm about to start my day at work. I'll take a fuller look this evening (UK time).

No rush at all! It's OSS, right? Enjoy your day!

@DaveLiddament
Copy link
Owner

@lcobucci what are your thoughts on #[NamespaceVisibility] as outlined in this comment?

I think this gives maximum flexibility and is, hopefully, easy to understand.

@DaveLiddament
Copy link
Owner

Addressed by #11

@lcobucci lcobucci deleted the namespace-visibility branch May 11, 2023 18:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants