theseer / Autoload

A lightweight php namespace aware autoload generator and phar archive builder
Other
388 stars 47 forks source link

sttolower with special characters in class names breaks autoload on some systems #7

Closed kore closed 13 years ago

kore commented 13 years ago

If your class name contains special characters the autoloading breaks on some systems, like MacOS.

The problem is that strtolower("ö") on MacOS returns "?". This breaks the autoloading definitions if you are not using only one system.

I would prefer to be able to disable the lowercasing for class names for those who do not need case-insensitive class names but have special characters in their class names.

jaykelly450 commented 13 years ago

I second this issue. Forcing lowercase on class names makes this otherwise awesome tool unusable for me.

davidfuhr commented 13 years ago

Wouldn't it help to use mb_strtolower()?

jaykelly450 commented 13 years ago

That may fix the issue with special characters. Though, the issue I'm having is that class names are CamelCase. So, the autoloader is not matching the classnames in the array and I'm getting a fatal error.

theseer commented 13 years ago

Using mb_* would require to a) know the source encoding and b) to have mbstring extension enabled - which may or may not be the case by default and would thus add another dependency to the Autoload. Not per se a deadly thing but I don't exactly like to impose potentially unneeded dependencies.

The bigger issue is that PHP does not impose case sensitivity on classnames at code level so having a case sensitive key in the map will not work reliably - not to mention cause a bc break if silently changed within phpab.

It would even affect the doublicate class detection unless i keep a 2nd list and do an case insensitive lookup.

On the other hand having non ascii chars within classnames is imho a very bad practice to begin with and thus I still don't really see that as an issue. It's way more likely that people screw up on the case of the classname when using it versus the way it was originally defined.

I agree that coding guidelines should disallow the later but by all practical means it's not enforced technically within php.

The following options exist:

As a reminder: The problem only exist if classnames contain non ascii chars. Something, I believe should be banned by coding guidelines.

theseer commented 13 years ago

I decided to implement a simple switch (-n or --nolower) to disable the automatic lowercase, thus putting the burden of choosing between breaking PHP's case insensitivity versus using broken classnames with non-ascii chars on the developer.

This change has one sideeffect: With case sensitivity enabled the autoload builder will not be able to detect re-declarations of classnames if the spelling has case differences. This would lead to an pretty unpredictable autoload behavior, loading the implementation which is used first only. But since that would pretty much blow up on production anyway - even with out autoload -, I guess that's an acceptable drawback.

The change has been implemented with commit 1056faff4fd7b7c0a98acd55190235099c9ece45 and is part of the 1.5.0 release.

The default behavior remains the same, e.g. the automatic lowercase conversion will be done.