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

Add support for @internal #103

Open
mindplay-dk opened this issue Sep 18, 2018 · 4 comments
Open

Add support for @internal #103

mindplay-dk opened this issue Sep 18, 2018 · 4 comments

Comments

@mindplay-dk
Copy link

Per #34 you can manually exclude files/folders from analysis - this is time-consuming, impractical for existing projects, and places meta-information about the API and source-code in a tool-specific configuration file.

#8 proposes to add support for the @api annotation - this is once again time-consuming and impractical if most of your API is public. It's also inconsistent with PHP itself, where every source member is public by default - adding a single @api annotation would mean that the tool needs to make the opposite assumption: that everything is internal by default. (Adding a even a single @api annotation would effectively need to mark your entire codebase as internal.)

I propose to add support for @internal instead, providing a means of excluding classes, interfaces, traits and methods/functions from affecting the version number.

Explicitly marking specific members of your codebase as internal makes it easier to progressively flag portions of your API as internal. With everything being public by default, it's more in tune with how PHP works, and more consistent with how doc-blocks work in general - narrowing types for static analysis, adding tigher constraints than is possible with PHP; similarly, this will narrow the scope of the public API.

To support the @api annotation, you would need to turn the visibility rules upside-down, effectively rendering the @internal annotation useless - since the presence of a single @api annotation would implicitly flag the entire codebase as internal.

I was very involved in the development of PSR-5 and was strongly opposed to the definition of the @api annotation for these reasons - a single annotation-type should not be able to nullify another annotation-type entirely.

I plan on taking this up with the new working group as soon as possible.

In the mean time, I'd encourage you to stick to established PHP and doc-block paradigms - starting with support for @internal as opposed to @api addresses the need to control public/internal visibility just fine, and in a more explicit and predictable manner.

I wouldn't put my money on @api until PSR-5 is officially approved, and I will do my best to make sure it's definition is changed before that.

@Ocramius
Copy link
Contributor

Cross-linking Roave/BackwardCompatibilityCheck#51

Generally ACK on @internal, while @api is an outlier only used within the Symfony packages: we have public for that for a reason :-)

@mindplay-dk
Copy link
Author

we have public for that for a reason

Not sure what you mean?

The public keyword doesn't work on classes, interfaces, etc.

From my understanding, that's the main reason the idea of something being "internal" even exists?

@Ocramius
Copy link
Contributor

I'm just saying that @api makes little sense - easy to forget to add it, too late to do it after a release anyway.

I'll likely work on supporting both on the other package, although only @internal support will be enabled by default.

@mindplay-dk
Copy link
Author

Gotcha :-)

I've started a thread on the FIG board proposing to change the definition of @api in the annotation-types PSR which will likely split from PSR-5.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants