search

joomla / joomla-platform

[READ-ONLY] This repo is no longer in active development. Please see https://github.com/joomla/joomla-cms
http://www.joomla.org
GNU General Public License v2.0
541 stars 298 forks source link

RFC 1787: Native PHP Namespacing #1787

Closed dongilbert closed 11 years ago

dongilbert commented 11 years ago

Summary

This RFC outlines a change to the way the Joomla Platform handles code structure and class naming. Currently, Joomla uses an ad-hoc namespacing standard based on the technology available at the time it was written. Since PHP 5.3 (released in June of 2009), PHP has native support for namespacing.

More information on the "why" behind namespacing can be found at these resources:

All existing platform code will be reworked to utilize native PHP namespaces according to the PSR-0 namespace specification. There are certain requirements for PSR-0. As such, please read and understand the following document when reviewing these specifications: https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-0.md

We are choosing PSR-0 as it is the "accepted standard" of other large PHP frameworks that are targeting the same user base that we are targeting with the Joomla Platform. This allows for developers to use the Joomla Platform with a low barrier to entry. It will feel familiar to them out of the box.

Proposal

*Note: In some cases, such as in the crypt package, we cannot maintain directory structure and have namespace structure follow it. This is because of the JCryptCipher3DES class which is in the joomla/crypt/cipher/3des.php file. If we were to mirror exactly, then the class name would have to be 3DES, and class names can't start with a number. In cases like this, we will structure the class name using the _ convention. All files within a package will be named this way, to make it easier to track exceptions to the rule. To that end, JCryptCipher3des will translate to Joomla\Crypt\Cipher_3des. Any exceptions like this will be clearly noted.

Benefits of Namespacing

Self-Documenting Code use statements at the top of class files clearly show file dependencies, increase readability, and eliminates ambiguity as to what code is being used.

Follows Modern OOP Practices Namespacing has been around since PHP 5.3, released in 2008. With most web hosts now widely supporting PHP 5.3, it's time to make this transition to stay relevant in a changing PHP ecosystem.

Easily Integrate 3rd Party Libraries Namespacing may not be for you if you're developing the entire code base for a small application that's for your own use, but as soon as you introduce 3rd party code, you increase the chances of naming collisions and as a result, problems. Properly namespaced code eliminates that issue.

Community Perception of Standard Compliance Everyone likes to have an "approved" badge if they can. Some very good developers won't even consider code if it is not "standards compliant" or if it requires extra effort on their part to integrate.

Composer and Packagist These tools facilitate integration between platforms, which allows for broad use of the joomla platform packages and thus increased contribution to the project. You as a developer will see the benefits

Importing and aliasing external classes and namespaces Namespacing allows you to import whole packages and libraries from other namespaces if you want. Instead of writing out JApplicationWebRouterRest when you need it, you could simply import the Joomla\Application\Web namespace, and then use Web\Router\Rest. Or, if you wanted to use two classes of the same name from two different libraries, you could import them and alias them to another name.

Overrides

Currently, as a user of the platform, you are able to override platform classes simply by creating a new JApplicationWeb (or new whatever class you want to override) and registering that class location in the loader via JLoader::register('JApplicationWeb', 'path/to/your/copy.php');. Alternatively, if you are going to be overriding many files, you can use JLoader::registerPrefix('J', 'path/to/your/joomla/library'); and the autoloader will search your path first for classes before falling back to search the platform.

In the first instance, there are no changes. You will still be able to use JLoader::register() exactly as you do today. (Well, the class name will be Joomla\Loader::register(), but that's minor.)

In the second instance there are minor changes as well. Since we are no longer using Prefix based (read: old style) class structure, the JLoader::registerPrefix() method will be deprecated. However, you may still override namespace classes in the same fashion. However, instead of using JLoader::registerPrefix(), you would use Joomla\Loader::registerNamespace().

Examples

https://gist.github.com/4520559

Side Effects of the Change

Good

Potentially Negative

Along with this change comes a compatibility layer that will allow current code written on the Joomla Platform to continue to function as usual on the new namespaced codebase. This will be ready for 13.1 provided all other namespace work is done as well.

This layer will be frozen at platform version 13.3. After that, all new class names will need to referenced by their fully qualified name. Existing classes may be referenced for as long as the compatibility layer is around.

The layer is opt in, and must be enabled by downstream users. For example, the CMS would need to explicitly enable it. A $enableCompatLayer argument has been added to JLoader::setup() to accomplish this. When true (it defaults to true), the JLoader::compatLayer() method is registered to the spl autoloader. As you can see from the method, it receives a $class param from the autoloader and checks the internal $nsMap to see if there is a match found. If a match is found, the old style class name gets aliased to the new namespaced name via class_alias.

The class map for the compatibility layer can be found here: https://github.com/dongilbert/joomla-platform/blob/PlatformNamespace/libraries/compat/NamespaceMap.php

Tasks

Namespacing the platform is going to take a LOT of work. Here are some of the tasks required:

The entire platform has been namespaced thanks to the efforts of @florianv and @dongilbert. Unit tests are 100% passing and it is ready for broader testing.

Testing the Namespaced Platform

The repo where all this is happening: https://github.com/dongilbert/joomla-platform/tree/PlatformNamespace

Todo: detailed testing instructions.

AmyStephen commented 11 years ago

Full support. Already, very encouraged by comments on Twitter, not only from Joomla devs who will certainly welcome this, but maybe even more importantly from some of the hard-core PHP devs whose involvement would be awesome.

eddieajau commented 11 years ago

Don, first off, thanks for putting the work in so far. However, understand we must put this through the refiners fire so I'm just going to say stick with it as we field comments. This is also, probably, going to be an iterative process. So, a couple of points.

I have personally committed to maintaining

I get what you are saying but this is an RFC and we should keep the text second or third person. I think it's best that you change split that block of text and create a new section called "Backward Compatibility". Then you propose to introduce the change en masse in 13.1 to be deprecate the compatibility layer in 14.1. Just thinking out loud though, another way to handle this is to say that the compatibility layer will be introduced in 13.1 but will be "frozen" after 13.3. That would mean that any new work must be referenced by namespace; and old classes could referenced by the old class names ad infinitum. The compatibility layer could also be opt in, you have to explicitly turn it on much like you have to explicitly add the legacy tree.

You haven't mentioned anything about how file names work. If there is no change to file and folder structure, this should be mentioned explicitly. If change is required, you also need to map out what the changes are.

I think the experienced Platform contributors will tend to get the impact of change from the examples you've given. However, I don't think the average CMS component developer is going to get it. It's probably worth adding several examples revolving around how this would affect CMS developers IF the CMS decided to adopt 13.1+ at some time in the future. Obviously we can't predict what the CMS would do, but we need to have a go at providing all the information we can so that any downstream user can make an informed decision (it will certainly affect what I do at work this year big time).

I think the dot points under "Benefits of Namespacing" need a sentence or paragraph after each point to further explain what is meant by, for example, "Eliminate ambiguity". What does that mean to an average downstream users that has never heard of namespacing. Remember, we are dealing with developers that kicked up a huge stink over dropping the DS constant - this is going to blow their mind.

Currently we have a system of overrides in the loader. I can create my own JApplicationWeb and have that override the Platform's core class. It's worth explaining how that feature would be supported under name spacing or how that feature has to change.

Obviously the task breakdown is going to be huge for this effort (the unit testing for this is probably going to be daunting, and we may have quite a number of dependencies like absolutely getting rid of lingering CMS code). Hopefully David will have Jira up by then :) In the absence of the Jira, I think it's worth starting the "task list" in the RFC just so we know the full impact of what we are signing up for. I know you are going to shoulder a lot of the work, but you are going to need help :)

Also, once we are happy with the state of the RFC, we are going to need to publish this widely and will probably need to do a voting response survey like we did for the LGPL.

dongilbert commented 11 years ago

Andrew,

Thanks for the pointers - I'll rework some of the language in the RFC and get this right. It's going to get lengthy. :)

As for a task list, it definitely is greater than just throwing some namespace and use constructs at the tops of files and calling it a day. I initially thought it could be a simple exercise - but I see it is definitely more than that now. No worries - I'm in this for the long haul.

How namespaces are structured and how we import classes between packages and such has a great impact on the flexibility of the platform in the future.

One of the bigger tasks at this point is clearly defining and separating packages to make them independent.

AmyStephen commented 11 years ago

All good points @eddieajau - as an aside, it's a lot easier to complain about DS because it's easier to understand. Given your observation, I think this effort might just get ignored. ;-)

Don and I were just talking about how one approach to implementing NS can result in flexible overrides using DI, or dependency chains that are very hard to override. https://gist.github.com/4458255/ The approach does indeed make a difference - glad you raised that point.

The other issue to work out is a decision on whether or not (or how) a services container might fit in as it can provide some relief with NSing. Andrew - Do you think it's the right time to decide on this? In conjunction with the NSing? Or, do you think it's better to wait on that? Dealing with JFactory in some way will be impossible to ignore.

This effort will drive a lot of good direction, really glad to see you guys have been working on this.

I'll ping @dukeofgaming this weekend, see if he's around.

eddieajau commented 11 years ago

Task I see should be in the mix:

Amy, yes, a standing goal should be to design the Platform with dependency injection in mind. However, I don't know what you mean by "services container".

dongilbert commented 11 years ago

Edit: Seemless interoperability requires file structure to match namespace casing as well.

Ex: Joomla\Application\Web\Router would map to Joomla/Application/Web/Router.php

eddieajau commented 11 years ago

Bugga. It's worth noting that will be a required change in the file system. That's not a particularly bad one, but one we need to make clear.

eddieajau commented 11 years ago

On, and might be better to include a more translatable word instead of "anathema". Fitting, but not common :)

dongilbert commented 11 years ago

Ya, I'm not happy about it either - but I think the revitalized interest in the platform is worth the extra effort. I got used to camelCased method names, I can get used to the same for structure.

dongilbert commented 11 years ago

Good news is - using git mv we can easily retain all file history. That's a plus.

AmyStephen commented 11 years ago

@eddieajau Good resource is from Fowler

http://martinfowler.com/articles/injection.html#ServiceLocatorVsDependencyInjection

Mainly a question of what you want to do with JFactory, thinking there is plenty already to do, as it is.

eddieajau commented 11 years ago

JFactory is a separate discussion in my book. I think it's scope creep for this discussion.

Regards, Andrew Eddie http://learn.theartofjoomla.com - training videos for Joomla developers

On 5 January 2013 14:26, AmyStephen notifications@github.com wrote:

@eddieajau https://github.com/eddieajau Good resource is from Fowler

http://martinfowler.com/articles/injection.html#ServiceLocatorVsDependencyInjection

Mainly a question of what you want to do with JFactory, thinking there is plenty already to do, as it is.

— Reply to this email directly or view it on GitHubhttps://github.com/joomla/joomla-platform/issues/1787#issuecomment-11909815.

dongilbert commented 11 years ago

The first 27 slides here are relevant to IoC, dependency injection, etc (how I think we could use it) - http://www.slideshare.net/neraath/ioc-with-php-8833643

But, as you said, outside scope.

eddieajau commented 11 years ago

So, correct me if I'm wrong, but PSR-0 requires that underscore must be converted to a directory separator, but it doesn't preclude you from having any other rules that you desire. You could, for example, make each character of the class name a folder and you'd still comply with PSR-0, no?

AmyStephen commented 11 years ago

I'm not so sure that's true. The idea is that a common class loader be usable for work from various authors/projects.

It might even be worthwhile to use the Classloader that comes with composer (if you don't include your own). That's going to be the best way to verify compliance.

https://github.com/composer/composer/blob/master/src/Composer/Autoload/ClassLoader.php

dongilbert commented 11 years ago

Yes and no. They chose to split on _ because that's how PEAR and Zend functioned before native NS capabilities. You would still be 'compliant' to the standard if you did what you said, because the standard fails to mention that in order for the autoloader to work, it needs to specifically exclude basing your directory structure an anything else.

The goal of the RFC is for our code and others code to be able to work with the auto loader they and we are already using. JLoader::loadByNaturalCase is almost there, although I discovered a bug in it last night.

Sent from my iPhone

On Jan 5, 2013, at 3:17 AM, Andrew Eddie notifications@github.com wrote:

So, correct me if I'm wrong, but PSR-0 requires that underscore must be converted to a directory separator, but it doesn't preclude you from having any other rules that you desire. You could, for example, make each character of the class name a folder and you'd still comply with PSR-0, no?

— Reply to this email directly or view it on GitHub.

AmyStephen commented 11 years ago

Agree with the goal aspect of Don's response. Unfortunately, I don't see ambiguity in the rule. The PSR-0 also states "You can test that you are following these standards by utilizing this sample SplClassLoader implementation which is able to load PHP 5.3 classes." Following that statement is an Autoload function with code.

So, bottom line is, if that code loads our classes, given our NS strategy, we're in compliance. If it doesn't, we're not. I really do think it's as simple as that.

https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-0.md

We might consider adding unit testing that brings in the shared class loader, if JLoader is maintained. I wonder if it makes sense for the platform to simply use the class loader from Composer? If JLoader is needed for the CMS during the deprecation period, would it be better to move it into the CMS branch? An idea, anyway.

eddieajau commented 11 years ago

By inspection of the code, the RFC is not going to be PSR-0 compliant:

https://gist.github.com/221634#file-splclassloader-php-L131

IF that's what the PSR meant - and it probably did. It's not what the PSR says and this was done before the group implemented the MUST's and SHOULD's.

It just means we aren't going to fully compatible (but we will be fully compliant to the letter of the law) in terms of people wanting to consume the Platform with other PSR-0 underscore split code.

AmyStephen commented 11 years ago

@eddieajau - I can't see an issue with Joomla's compliance. I only know of one class - and that's the numeric class name in the Crypt package. Other than that, I am not seeing any challenge to compliance. Sorry to be so dense but I am not seeing the problem. What problems are you seeing?

dongilbert commented 11 years ago

The whole exercise of name spacing for me is based around 2 things. The ability for platform users to consume PSR-0 packages and for PSR-0 users to be able to seamlessly use platform code.

Sent from my iPhone

On Jan 5, 2013, at 4:00 PM, Andrew Eddie notifications@github.com wrote:

By inspection of the code, the RFC is not going to be PSR-0 compliant:

https://gist.github.com/221634#file-splclassloader-php-L131

IF that's what the PSR meant - and it probably did. It's not what the PSR says and this was done before the group implemented the MUST's and SHOULD's.

It just means we aren't going to fully compatible (but we will be fully compliant to the letter of the law) in terms of people wanting to consume the Platform with other PSR-0 underscore split code. — Reply to this email directly or view it on GitHub.

AmyStephen commented 11 years ago

It really is a little frustrating that there are some many PSR-0 class loaders that are pointed too as the "official" standard enforcer. @eddieajau - the one you are pointing to was a very early copy and it doesn't appear to have been updated in a long time. Maybe that's the code problem you are referring too? (I hope)

The one I believe we should be most focused on is the one provided with Composer. https://github.com/composer/composer/blob/master/src/Composer/Autoload/ClassLoader.php

Again, sorry to repeat, but if there are compliance issues for Joomla, let's identify very clearly why we cannot comply. If it's only a matter of providing time to deprecate old names, that is not a compliance problem. The CMS will have to have a different autoloader for a period of time. That would be expected.

But, if there is something we can't do and there is a valid reason for us doing it a certain way (meaning, not just a preference, needs to be a valid reason) then we should fork the class loader, adapt it, so it works for everyone else and for us, and have Andrew, as a voting member of the PSR group, present our case and request adaptations needed.

But, again, I just don't understand yet what the problem is. What @dongilbert has presented is in compliance, if I understand things, and if it's not, I would appreciate an explanation. I am just not getting seeing the problem.

dongilbert commented 11 years ago

OH - I apparently had a brain fart when I read your response earlier Andrew.

I think you are referring to my statement that _'s are anathema and not to be used in our code whatsoever.

The reason I made that statement is that I don't want our code to have classes with _'s in any situation. (I was considering using it earlier to solve the Cipher\3DES problem, but I decided against it.)

Our code will be compliant and able to be consumed by PSR-0 autoloaders / projects.

AmyStephen commented 11 years ago

Good.

eddieajau commented 11 years ago

@Amy, the gist I referred to is what is in PSR-0 "standard".

https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-0.md

See the very last link.

In terms of compliance, PSR-0 does not say definitively that underscores are the only way you can determine how to form the real file system path. Whether that is what is intended or not is neither here nor there, PSR-0 is silent on the matter. On that basis I think we can rightly say we will be able to "comply" to the letter of the law with PSR-0, but that does not necessarily mean we will be compatible with everyone. It's a deficiency in PSR-0. The bigger problem we have is breaking on camel case because some libraries, like Zend, have files like /Foo/Bar/GooCar.php.

As for changing the PSR, I'm happy to take a case to the group but I don't want to own the processing of making "our" case. Amy, maybe you'd like to run with doing the work of making a draft and getting our community to discuss it. If the consensus is that we need to propose a change to PSR-0, I can take it to FIG. If nobody is interested in forming a case for change, we'll just keep on doing our own thing (maybe I need to say something about that on the mailing list).

Just on that point I just want to make it clear I only have time to be a voting member of issues that interest this community. I don't have time to keep you up to date with what they are discussing etc and so on. You can subscribe to the mailing list yourself for that :) And as I've said, not every PSR is going to fit what we do - the PSR's are about normalising the average, not necessarily about being innovative and bleeding edge which is more where I'd like to see the culture of the Platform head.

dongilbert commented 11 years ago

The PSR does say that it splits on namespace separator and on underscore. It goes on to say that casing in class name has no effect. So it could be assumed that if casing in class name caused an effect, that it is in fact non-complaint to the standard.

TBH I want name spacing and LGPL so that the great code in the platform can be consumed by others, and so Joomla can be a respected platform once again. If you're set against that, then I drop my case for both items.

Sent from my iPhone

On Jan 5, 2013, at 8:22 PM, Andrew Eddie notifications@github.com wrote:

@Amy, the gist I referred to is what is in PSR-0 "standard".

https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-0.md

See the very last link.

In terms of compliance, PSR-0 does not say definitively that underscores are the only way you can determine how to form the real file system path. Whether that is what is intended or not is neither here nor there, PSR-0 is silent on the matter. On that basis I think we can rightly say we will be able to "comply" to the letter of the law with PSR-0, but that does not necessarily mean we will be compatible with everyone. It's a deficiency in PSR-0. The bigger problem we have is breaking on camel case because some libraries, like Zend, have files like /Foo/Bar/GooCar.php.

As for changing the PSR, I'm happy to take a case to the group but I don't want to own the processing of making "our" case. Amy, maybe you'd like to run with doing the work of making a draft and getting our community to discuss it. If the consensus is that we need to propose a change to PSR-0, I can take it to FIG. If nobody is interested in forming a case for change, we'll just keep on doing our own thing (maybe I need to say something about that on the mailing list).

Just on that point I just want to make it clear I only have time to be a voting member of issues that interest this community. I don't have time to keep you up to date with what they are discussing etc and so on. You can subscribe to the mailing list yourself for that :) And as I've said, not every PSR is going to fit what we do - the PSR's are about normalising the average, not necessarily about being innovative and bleeding edge which is more where I'd like to see the culture of the Platform head.

— Reply to this email directly or view it on GitHub.

eddieajau commented 11 years ago

Ok, so I've been a bit confused and that is all clear up now. In summary, what we can do is say:

a) the Platform will conform to PSR-0 in every way b) BUT, we will enforce some style rules

The style rules will be that we must manually namespace every class to generate the effect of the camel case being a folder/file break.

We generally won't allow underscores in the class names UNLESS the class name would start with a number. In that case, the crypt ciphers would be Cipher_3des, Cipher_Blowfish - a one-in-all-in deal. We could support Cipher3des or CipherBlowfish but, in my opinion, it's better to have those in their own folder.

End result, I'm all good with PSR-0, aside from the work it obviously generates :)

dongilbert commented 11 years ago

YAY

AmyStephen commented 11 years ago

@eddieajau This has been confusing to me, as well, I've used a lot of Don's time trying to follow. I am concerned you might still have a slightly different understanding when you say this:

"The style rules will be that we must manually namespace every class to generate the effect of the camel case being a folder/file break."

I guess I'm not clear on what you mean by that. In reality, every class is namespaced, but how you feed the class loader is by registering a namespace and associating it with a folder location. Then, those registered namespaces are used to search for classes.

I believe Don's plan is to register the namespace for each package. In my Autoload, I would have statements like the following for each package I want to use from JPlatform:

require_once VENDOR . '/symfony/Component/ClassLoader/UniversalClassLoader.php'; use symfony\Component\ClassLoader\UniversalClassLoader;

$s = new UniversalClassLoader();

$s->registerNamespace('JPlatform\Crypt', BASE_FOLDER . '/Vendor'); $s->registerNamespace('JPlatform\Database', BASE_FOLDER . '/Vendor'); $s->registerNamespace('JPlatform\Log', BASE_FOLDER . '/Vendor');

and so on.

For the log package, only one namespace would be registered with the class loader.

Taking the entry class as an example:

  1. it would have a single namespace line at the top of the file for the namespace that represents the folder containing the file. It might be a registered namespace, but if the files are in a subfolder for the package, it will be the registered namespace + the name of the subfolder(s). That is true for each and every file -- it must have a namespace line -- that must be the first (non-comment) line - and that just PHP.

Now, this next point I was confused on and I am thinking you might be, as well.

  1. The class name defined within the file will change. Instead of having a class named JLogEntry -- which translates to joomla\log\entry, the class will be named Entry. And that's good, because the file is named entry.php. (Although we might need to uppercase the first letter of those filenames.)

So, the top of the JLogEntry file will look like this:

namespace JPlatform\log;

class Log { etc.

If I wanted to use this class in another class, I would instantiate it as:

$class = 'JPlatform\log\Log'; $instance = new $class();

or I could define a use statement on the top of the page (don't recommend doing it this way as overrides become difficult):

namespace in\my\class\folder; use Jplatform\log\Log;

Then within a method:

$instance = new Log();

Is that your understanding, too? Or, are you thinking the old JLogEntry class name will continue to be used for the class definition? (because that is not the plan - and it was a point of confusion for me until Friday.)

dongilbert commented 11 years ago

Amy, the vendor name will be Joomla, not JPlatform. Also, for the time being, we will only be registering the root namespace of Joomla, not each package.

Overrides would function much the same. From what I understand, you can register multiple locations to search, just like we currently do.

One thing I did notice though is that no part of the namespace is trimmed off when loading via the standard loader. Meaning it would be registered this way:

$loader->registerNamespace('Joomla', JPATH_LIBRARIES);

And then for the CMS to override classes would have:

$loader->registerNamespace('Joomla', JPATH_LIBRARIES . '/Cms);

Then, for classes for the CMS to override would go in /libraries/Cms/Joomla.

A name spaced JApplicationSite would go in /libraries/Cms/Joomla/Application/Site.php or it could be renamed to Cms/Application/Site.php

On Saturday, January 5, 2013, AmyStephen wrote:

@eddieajau https://github.com/eddieajau This has been confusing to me, as well, I've used a lot of Don's time trying to follow. I am concerned you might still have a slightly different understanding when you say this:

"The style rules will be that we must manually namespace every class to generate the effect of the camel case being a folder/file break."

I guess I'm not clear on what you mean by that. In reality, every class is namespaced, but how you feed the class loader is by registering a namespace and associating it with a folder location. Then, those registered namespaces are used to search for classes.

I believe Don's plan is to register the namespace for each package. In my Autoload, I would have statements like the following for each package I want to use from JPlatform:

require_once VENDOR . '/symfony/Component/ClassLoader/UniversalClassLoader.php'; use symfony\Component\ClassLoader\UniversalClassLoader;

$s = new UniversalClassLoader();

$s->registerNamespace('JPlatform\Crypt', BASE_FOLDER . '/Vendor'); $s->registerNamespace('JPlatform\Database', BASE_FOLDER . '/Vendor'); $s->registerNamespace('JPlatform\Log', BASE_FOLDER . '/Vendor');

and so on.

For the log package, only one namespace would be registered with the class loader.

Taking the entry class as an example:

  1. it would have a single namespace line at the top of the file for the namespace that represents the folder containing the file. It might be a registered namespace, but if the files are in a subfolder for the package, it will be the registered namespace + the name of the subfolder(s). That is true for each and every file -- it must have a namespace line -- that must be the first (non-comment) line - and that just PHP.

Now, this next point I was confused on and I am thinking you might be, as well.

  1. The class name defined within the file will change. Instead of having a class named JLogEntry -- which translates to joomla\log\entry, the class will be named Entry. And that's good, because the file is named entry.php. (Although we might need to uppercase the first letter of those filenames.)

So, the top of the JLogEntry file will look like this:

namespace JPlatform\log;

class Log { etc.

If I wanted to use this class in another class, I would instantiate it as:

$class = 'JPlatform\log\Log'; $instance = new $class();

or I could define a use statement on the top of the page (don't recommend doing it this way as overrides become difficult):

namespace in\my\class\folder; use Jplatform\log\Log;

Then within a method:

$instance = new Log();

Is that your understanding, too? Or, are you thinking the old JLogEntry class name will continue to be used for the class definition? (because that is not the plan - and it was a point of confusion for me until Friday.)

— Reply to this email directly or view it on GitHubhttps://github.com/joomla/joomla-platform/issues/1787#issuecomment-11924638.

dukeofgaming commented 11 years ago

Guys, I moved the existing Jira issue put by Don to an experimental workflow, I need feedback to "debug" the workflow. I'll put some details in the other mailing list post to not hijack this thread.

https://joomla.jira.com/browse/PLATFORM-4

realityking commented 11 years ago

This all looks pretty good to me - except one thing: Performance of the compatibility layer is going to stink. It will load every single platform class. In an environment not using a bytecode cache (and probably even in those using one) that won't be nice - especially when you consider you'd still be doing that on cached pages when using the Joomla CMS cache.

I haven't thought much about it but I think we could use JLoader to out advantage here. If JLoader can't find a class it could look up the class in the array, define the alias and try loading again. I'm not a 100% sure that's a better or even a good idea, but I wanna put it out there so other can think about it too ;)

dongilbert commented 11 years ago

That's a good point Rouven. I had been trying to think of a way to accomplish the compat layer without having to load every class first, and you're suggestion gave me some ideas.

On Mon, Jan 7, 2013 at 5:31 PM, Rouven Weßling notifications@github.comwrote:

This all looks pretty good to me - except one thing: Performance of the compatibility layer is going to stink. It will load every single platform class. In an environment not using a bytecode cache (and probably even in those using one) that won't be nice - especially when you consider you'd still be doing that on cached pages when using the Joomla CMS cache.

I haven't thought much about it but I think we could use JLoader to out advantage here. If JLoader can't find a class it could look up the class in the array, define the alias and try loading again. I'm not a 100% sure that's a better or even a good idea, but I wanna put it out there so other can think about it too ;)

— Reply to this email directly or view it on GitHubhttps://github.com/joomla/joomla-platform/issues/1787#issuecomment-11977533.

dongilbert commented 11 years ago

While I'm in these files, I think it may be a good time to remove all the defined('JPATH_PLATFORM') checks in the class files. There really is no need for them. I mean, I understand their original purpose, but none of the platform files have any side effects other than declaring classes, constants, functions, and interfaces. Even IF they were accessed directly, there would be no way for them to execute anything.

What say you?

realityking commented 11 years ago

It's not about executing some code but because the resulting error results in an information disclosure issue.

In a perfect world platform classes would never live in a web accessible space (there are framework with that assumption).

elkuku commented 11 years ago

E.e. class A extends B when class B isn't defined yet... Using phars would make this obsolete ;) On Jan 10, 2013 12:40 AM, "Rouven Weßling" notifications@github.com wrote:

It's not about executing some code but because the resulting error results in an information disclosure issue.

In a perfect world platform classes would never live in a web accessible space (there are framework with that assumption).

— Reply to this email directly or view it on GitHubhttps://github.com/joomla/joomla-platform/issues/1787#issuecomment-12081512.

florianv commented 11 years ago

I think we could think about changing the case of the folders and files in libraries/ to Camel Case.

If all files are affected by the namespacing, then it might be the right time.

1) This is the general convention for most frameworks and packages that you can grab via composer. 2 ) Faster autoloading because the namespace case matches the path case. 3 ) Improve files and folders readability especially when it is composed of more than one part. For example instead of having controller/base.php we can have Controller/AbstractController.php

dongilbert commented 11 years ago

@florianv - see the compatibility layer gist for the proposed PSR-0 namespace structure. I'm currently working on this in my PlatformNamespace and have completed 12 of the 42 packages so far.

I'm leaving the current file / folder structure in tact to make merging interim PR's easier, until it's ready to merge. Once all the namespacing is done, I'm going to go back through and rename / move all the files to the new structure.

See the second bullet under Side Effects > Potentially Negative

florianv commented 11 years ago

Sorry, I didn't see this bullet.

It looks very good :) there is quite a bit of work. I will send you some pull requests.

dongilbert commented 11 years ago

Awesome. I'm sure you know this, but for others who'd like to contribute to the effort, this is how you work on my PlatformNamespace branch.

As for the namespace process and how to do it, check out the work done in the access through document packages so far to get an idea. I'll document the ns process later.

On Thu, Jan 10, 2013 at 3:36 PM, Florian Voutzinos <notifications@github.com

wrote:

Sorry, I didn't see this bullet.

It looks very good :) there is quite a bit of work. I will send you some pull requests.

— Reply to this email directly or view it on GitHubhttps://github.com/joomla/joomla-platform/issues/1787#issuecomment-12120016.

dongilbert commented 11 years ago

I've added a Contributing to the Effort section in the RFC. If you are interested in helping / getting things done, please check off what you'd like to be responsible and leave a comment that you are doing it, so we can track and not duplicate effort.

florianv commented 11 years ago

I can help you a little bit right now starting from the bottom packages 'view' etc

dongilbert commented 11 years ago

In the 2 hours since he posted that message, @florianv completely namespaced 8 packages AND gave me some pointers on an easier way to complete this task. Thanks for the help and awesome work!

florianv commented 11 years ago

We should add the task : 'creating a composer.json file in each package so they can be grabbed individually via composer'.

Once this is done we can accomplish this https://github.com/joomla/joomla-platform/issues/1799 and add the dependency on the Psr log package in the Joomla log package.

eddieajau commented 11 years ago

Slow down Florian :) Composer is another conversation we have not had yet. One step at a time please :)

florianv commented 11 years ago

ok :)

florianv commented 11 years ago

When namespacing some packages, we noticed that 2 class names are conflicting with php reserved keywords :

https://github.com/dongilbert/joomla-platform/blob/PlatformNamespace/libraries/joomla/log/logger/echo.php#L24

https://github.com/dongilbert/joomla-platform/blob/PlatformNamespace/libraries/joomla/html/list.php#L26

AmyStephen commented 11 years ago

Given all the great work that has been started on this project, it became apparent there might be benefit to further defining the approach to namespacing so that the top of the page can be used at a glance to identify the requirements for the class (outside of the current namespace) and so that the code, itself, is not cluttered with namespace data. Don and I talked about this and agreed on some guidelines. Here's a link to a gist that helps clarify those guidelines.

https://gist.github.com/4520559

eddieajau commented 11 years ago

https://gist.github.com/452055 https://gist.github.com/45205599

I think that might be better presented as a new page in the Coding Standards section of the manual. We should probably get into the habit of using the MUST/SHOULD RFC as well.

Regards, Andrew Eddie http://learn.theartofjoomla.com - training videos for Joomla developers

AmyStephen commented 11 years ago

I don't believe I can fork this RFC, can I? At least, I can't see how. Might have to have @dongilbert update the examples after downloading the gist.

As far as the Coding standards, I agree. @eddieajau Do you want that done now? Or, should we wait until we have the project somewhat complete, and have hopefully encountered all the various issues that might impact the standard? Either way, glad to help do that.

eddieajau commented 11 years ago

I think it is better to draft the standards earlier than later. We can be ratifying them while the coding effort works on in parallel.

Regards, Andrew Eddie http://learn.theartofjoomla.com - training videos for Joomla developers