Problem/Motivation
As part of #2645010: Switch to PSR-2 as of Drupal 9 we concluded that using camelCase for local variables would help with writing libraries and adding interoperability between other libraries / projects and Drupal.
camelCase variable naming is a popular standard with many PHP projects
camelCase for variables is used by projects such as Symfony, Laravel, Zend, Flow, MediaWiki, Joomla, Composer, PEAR and many others. (However, it is not used by projects such as WordPress, or by PHP itself.)
All external libraries that Drupal depends on use camelCase variable naming.
camelCase variable naming is important for PHP coding standards interoperability
For better coding standards interoperability with those projects we want to allow camelCase variable naming. Consider this Drupal interface:
interface DiscoveryInterface {
public function getDefinition($plugin_id, $exception_on_invalid = TRUE);
}
A project with a camel case variable naming standard would have to implement this method with renamed variables:
MyDiscovery implements DiscoveryInterface {
public function getDefinition($pluginId, $exceptionOnInvalid = TRUE) {}
}
Which is very confusing, variable names of interfaces should never be changed to preserve clarity. The implementor either has the choice to violate their own coding standards or change variable names of an interface, which is a lose-lose situation.
The same applies the other way around when Drupal code implements an external interface using camelCase variables.
camelCase variable naming is important for consistency with OOP coding standards
In OOP code we are currently using different naming patterns for variables that refer to the same thing:
public function __construct(array $configuration, $plugin_id, $plugin_definition) {
$this->configuration = $configuration;
$this->pluginId = $plugin_id;
$this->pluginDefinition = $plugin_definition;
}
snake_case is inconsistent with the established class property naming standards.
Proposed resolution
Allow both snake_case and camelCase to be used as variable names. For consistency, allow that for global variables, local variables in functions and local variables in methods. That also includes names for parameter variables that are passed into functions/methods.
To allow a gradual move to camelCase variable naming the coding standards will not recommend one over the other. The coding standards will stay neutral and will not state whether snake_case or camelCase is preferred at this point. That will ensure balance between huge legacy code bases using snake_case (Drupal 7 and 8) as well as new code being written preferring camelCase (Drupal 8 contrib and Drupal 9).
Current text at https://www.drupal.org/coding-standards#naming :
Functions and variables should be named using lowercase, and words should be separated with an underscore. Functions should in addition have the grouping/module name as a prefix, to avoid name collisions between modules.
Change that to:
Variables should be named using lowercase, and words should be separated either with uppercase characters (example:
$lowerCamelCase) or with an underscore (example:$snake_case). Be consistent, do not mix camelCase and snake_case variables inside a method or function.Functions should be named using lowercase, and words should be separated with an underscore. Functions should in addition have the grouping/module name as a prefix, to avoid name collisions between modules.
Comments
Comment #2
tizzo commentedSpeaking personally (and not on behalf of the TWG) I'm very very much in favor of this and have thought this for a long time.
*Big* +1.
Comment #3
chx commentedEvery "at large" concern in #2645010: Switch to PSR-2 as of Drupal 9 applies.
Comment #4
tizzo commentedI don't disagree. I just mean that I've long preferred making all variable in a class file camelCase for aesthetics and consistency. I am not saying that it's necessarily worth the human time investment in converting everything, updating all of the patches in all of the queues, etc.
Comment #5
chx commentedComment #6
chx commentedMy proposal: allow for camelCase but not require. Gradually move over.
Comment #7
dawehnerOH yeah, for nearly none of such changes we did them immediately but rather slowly moved from one style to the other over time, just like short array syntax.
Comment #8
drunken monkeyAlso +1 for this change.
The downside is that those few remaining places with snake_case will stand out even more in contrast, but I guess there's no helping that (especially for function names).
Comment #9
dawehnerYeah, let's just apply the same strategy as we did with short array syntax. This works actually.
Comment #10
klausiMarked #2474561: Coding standard: inconsistent requirement that arguments to setters methods should be $lower_case when private variable and setter methods are camelCase() as duplicate.
I think snake_case has the advantage of quickly distinguishing between local and class variables (which are camelCase). But you can also distinguish them by the "->" in front of them, so not really an advantage.
In general I think we should do whatever PHP FIG proposes, but it seems this is not defined in PSR-2: https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-2-codi...
In that standard they are using snake_case and camelCase for variable examples, so I think it is OK if we also allow both variants.
Seems like we are all in agreement here, so let's move this further along.
Comment #11
perignon commented+1 for the change.
Comment #12
klausiGreat, we need an announcement for final discussion to move this along in the process.
Comment #13
pfrenssenIf the idea is to gradually move to camel case, shall we put in the coding standards that both are allowed but camel case properties are preferred over snake case? This will avoid a lot of discussions :)
Comment #14
gappleClosed #2411911: [policy, no patch] snake_case in coding standards makes consistency interoperability hard as a duplicate of this issue.
Comment #15
gappleWould this change apply to parameters and local variables for global functions, or is this specifically within classes?
Comment #16
pfrenssenI just checked what PSR-2 had to say about it, but actually it doesn't recommend anything regarding the casing of variables. But it builds on PSR-1 which says that it "intentionally avoids any recommendation regarding the use of $StudlyCaps, $camelCase, or $under_score property names". Interesting, so they actually don't recommend camel case at all :)
Any way it makes sense to keep the same standard for local variables everywhere. It would be stupid to have a different standard for local variables in the global scope and in classes. So let's allow camel case everywhere, as long as they are consistent within a file.
Comment #17
chx commentedThe reasoning for variables in camelCase is two fold a) classes/interfaces used for type hinting are in camelCase so you inject a route match,
RouteMatch $route_matchthat just looks sillyRouteMatch $routeMatchmuch better b) class properties are already camel case.So while b) doesn't apply outside of classes, a) does.
Comment #18
pfrenssenI'm also in favor for camel case, I just found it interesting that we are basing this whole discussion on the PSR-2 standard, while it actually doesn't mention it :)
Comment #19
klausiI think we should globally allow snake_case and lowerCamleCase for any variable name to be consistent and not have too complicated rules.
I don't think we should prefer one over the other at this point, so let's leave the coding standard neutral about that. We can change that later if we want to. The goal for this issue is to allow lowerCamleCase to allow transitioning as a first step.
Setting this back to needs review, please check the issue summary and improve or post a simple +1.
Comment #20
chx commentedtpyofix :) and looks good. And noone was arguing on PSR-2 grounds. To the contrary, initially I was against the initial proposal for all the reasons I was against PSR-2 but then twisted this around to something palatable and people liked it. bojanz in the parent issue said he doesn't care about PSR-2 and just wants his camelCase ; noone bothered checking whether the mess PSR-2 is even contains it but we certainly agreed that would be good.
tl;dr: there is no fate but what we make for ourselves. We are not blindly beholden to anyone else's codebase, standards and ideas.
Comment #21
perignon commentedI'll reiterate my +1. Just like Bojanz, just interested in the camelcase :-D
Comment #22
jhodgdonSince the question came up already of "does this also apply to param names", I think the issue summary needs an update please. Also since the question has come up a few times of "do we have a preference", I think the summary needs to state that this discussion took place. Right now it is vague and you would need to read the issue to understand the point and that means it is not a great summary yet. Thanks!
Comment #23
jhodgdonAlso please read the procedures for coding standards discussions. I don't think that we are supposed to be marking these issues "RTBC" until much later in the process.
https://www.drupal.org/project/coding_standards
Comment #24
klausiUpdated the issue summary.
The current coding standards don't mention parameter variable names explicitly, they are included in the general rules for naming variables. I think we can leave it like that.
I think the RTBC mentioned in the coding standards approval workflow is meant for the core queue once this issue is moved there. In the meantime I think we are free to use this status in the coding_standards issue queue. It gives the issue better status info when in the list view of issues. We have perfect consensus in this issue so far with buy-in from 7 well known contributors and not a single objection - I think the RTBC is well justified.
Please continue with +1 comments or any objections. Let's wait 3 more days before we put the "needs announcement for final discussion" tag on again.
Comment #25
bojanz commentedI think the wording should be stronger.
Currently it sounds like we have no preference. Our goal is to end up with camelCase only, generations from now.
But how do we say that, if core and contrib are snake_case today?
"camelCase is preferred, use snake case on existing codebases"?
Comment #26
benjy commented+1 I think this is a good step in the right direction for consistency.
Comment #27
pfrenssenYeah I also think we should state a preference towards the new syntax, this makes it clearer that this is a transitional state. At the very least we should encourage people to be consistent, similar to how we worded it for the short array syntax:
Comment #28
pfrenssenAdd this?
Comment #29
xjmAs @jhodgdon says, marking this RTBC is not following https://www.drupal.org/project/coding_standards. Thanks!
Comment #30
dawehnerAlright, the next step seems to be this:
Comment #31
mpdonadio#28++ We have similar language for arrays.
Comment #32
klausi@pfrenssen: agreed on the consistency sentence, added to the issue summary.
@bojanz: the goal of this issue is to allow camelCase for variables. Which one should be preferred is a different discussion and out of scope for this issue, IMO. As a compromise I reworded the sentence to mention camelCase first. I created #2678238: Remove prohibition on mixing snake_case and camelCase at least for constructors as a follow-up to discuss/decide the preference issue. Let's move in smaller steps to gain consensus faster and make actual change happen.
@xjm: as I said, the RTBC status is only defined in the process for the core queue, not for this one. But anyway, I'm fine leaving this as "needs review" if that satisfies the process.
Comment #33
bojanz commented@klausi
I see no point in splitting the preferred argument from this one, when it's the exact same discussion, with the exact same arguments.
I don't think we have any disagreement on the WHY, it's just a question of HOW/WHEN.
Comment #34
xjmSo in general, I have the same concern about a generic "recommendation" that is not enforceable as I described in #2135291-62: [Policy, no patch] PHP 5.4 short array syntax coding standards for Drupal 8. The difference between these two issues though is that the short array syntax can be changed throughout core or any code very easily, all at once, with little risk because it can be done with a regex and making a mistake for that will cause a syntax error. However, since renaming variables is a far bigger scope and a far riskier change, I can see the case for doing this one "transitionally".
How would we implement a rule for the required part of this? Throw an error if the two cases were mixed within the scope of a function or method? Within a class? How would we avoid contributors NWing patches over an unenforced standard if it were a "recommendation"?
Comment #35
klausiYes, discussing the consequences of having one preferred standard is the exact reason why I opened #2678238: Remove prohibition on mixing snake_case and camelCase at least for constructors. Please continue there and let's keep this issue focused on just allowing camelCase.
For the Coder implementation: I would not throw an error if snake_case and camelCase is mixed within a function in the beginning to help ease transitioning. We can enforce that later in Coder.
Comment #36
fleshgrinder commentedThere is no interoperability win in using camelCase over snake_case because one should not be exposed to the properties or variables of other code. The usage of camelCase also completely ignores the official PHP coding standard for userland code, something one might be more exposed to after all. Science also supports that snake_case is more readable plus overall it introduces less exceptions in handling of related code (which is good and the major weakness of PSR-2).[1, 2]
Associative array (dictionary) keys? snake_case
Twig variables? snake_case
GET parameters? snake_case (or dash-case)
MySQL and PostgreSQL convention? snake_case
Conversion from e.g. dash-case to snake_case is straight forward with a
str_replace('_', '-', $lemma)whereas camelCase requires complicated regular expressions and might result in a broken conversion because handling of abbreviations and acronyms is hard, same is true for single character words:deliverToUsvs.deliverToUStoHTMLvs.toHtmlUsing snake_case is also more aligned with most of the other programming languages (PHP, C, C++, Python, Pearl, Ruby [on Rails], Rust, …). It is further aggravated by the fact that variables/properties are matched in a case sensitive manner whereas methods are not. While code should be properly tested, it is still a source of possible bugs (this is highly theoretical, I never encountered this issue myself in more than ten years of camelCase coding in PHP).
I know that you guys are probably leaning towards camelCase because you are more used to it or you learned programming with Java and its style guide (hell, I did) but evidence shows that snake_case is better suited for variables, properties, and friends than camelCase so think about it again.
For instance the example that was given with
RouteMatch $route_matchvs.RouteMatch $routeMatchwhere it was said that it looks better. First off, it depends, what if we have a longer word?RouteMatcherDecoratorFactorySpecialUnicornThingy $routeMatcherDecoratorFactorySpecialUnicornThingyorRouteMatcherDecoratorFactorySpecialUnicornThingy $route_matcher_decorator_factory_special_unicorn_thingy; I think we have a clear winner here (this is the rationale behind phpspec and its usage of snake_case for method names). Secondly, the different casing tells us a lot about what the things actually are. The PascalCased word is a reference to a class whereas the snake_cased word is an argument (or variable).Big −1
Comment #37
pfrenssenIsn't that coding standard intended for people that work on the PHP code base? They're writing C, not PHP.
Comment #38
David_Rothstein commentedI think it's for people working on the PHP codebase who are writing PHP (not C) - that's why it refers to "user-level functions", right?
In other words, it's the coding standard that explains why something like http://php.net/manual/en/class.simplexmlelement.php uses camel case for method names but snake case for variable names. (Not that all PHP classes actually follow the standard exactly, though.)
Keeping Drupal's standards in line with PHP's seems like a pretty compelling argument to me. Previously I didn't have much of an opinion on this issue one way or the other but I think #36 is sounding like a good argument for why Drupal shouldn't change this.
Comment #39
bojanz commented@David_Rothstein
Let's be honest, the only standard PHP projects are following is PSR-2. And we already said we don't like that one.
Comment #40
klausiThe next step in this issue is to establish that camelCase is the PHP industry standard for variable names by now, which should be easy to prove by listing all the PHP projects doing it with references (most importantly Symfony).
Changing tags to indicate that we need a stronger case in the issue summary for that.
Comment #41
fleshgrinder commentedThe official PHP coding standard is intended for people working with PHP and yes this includes writing C. It also includes writing PHP user-level code in C that we work with everyday, exactly like the Simple XML example. I have no clue why it is always ignored by everyone but I think that nobody ever took the time to actually read the PHP source code.
PSR-2 is not a standard, it is a recommendation and I highly doubt that listing of a few projects proofs anything but lets give it a shot.
Projects and languages using camelCase:
Projects and languages using snake_case:
Projects and languages that use a mixture:
See also:
What can be said is that we see a shift towards camelCase in most projects, probably due to the fact that Java, Google, and Symfony have a big influence on the scene. That being said, science leans towards snake_case and the PHP core as well.
Comment #42
gappleAll of the packages listed in core/composer.txt as official dependencies use camelCase for method parameters and local variables:
- doctrine/common
- doctrine/annotations
- easyrdf/easyrdf
- egulias/email-validator
- fabpot/goutte
- guzzlehttp/guzzle
- masterminds/html5
- stack/builder
- symfony/class-loader
- symfony/console
- symfony/dependency-injection
- symfony/event-dispatcher
- symfony/http-foundation
- symfony/http-kernel
- symfony/routing
- symfony/serializer
- symfony/validator
- symfony/process
- symfony/yaml
- symfony-cmf/routing
- symfony/psr-http-message-bridge
- twig/twig
- zendframework/zend-diactoros
- zendframework/zend-feed
Comment #43
klausiUpdated the issue summary. I think we can ignore other programming languages, since we should be doing what the wider PHP community is doing - that's the context our code is read and maintained in most of the time.
@Fleshgrinder and @David Rothstein: does that address your concerns?
Putting the tag back on for TWG consideration.
Comment #44
klausiI don't think Coder needs to wait on final approval of this, since we are only removing errors that are reported on variable names. Committed #2303963: Allow camelCase for variable naming conventions to Coder.
Comment #45
fleshgrinder commentedI can life with camelCase especially since Drupal was using it for properties already, hence, requiring certain mapping operations that would not be necessary if snake_case would have been used. An approach to solve these mapping issues is to use camelCase everywhere: also in the DB. However, one must note the compatibility issues it adds due to case-insensitivity at some places.
I truly dislike the fact that we ignore the language's standard we are programming in but if that is what the community wants than that is how it is.
Comment #46
David_Rothstein commentedThe issue summary says this is a "PHP industry standard" but I don't see how it can be when PHP itself doesn't follow it, WordPress (the largest PHP project) doesn't follow it, a number of other PHP projects mentioned above don't follow it, etc.
As far as I can see there is no real industry standard for this one way or the other; different projects just do what they want.
It does sound like the external libraries which Drupal 8 includes in its codebase use camel case pretty consistently. Personally I'd still put (a bit) more weight on what PHP itself does though.
Comment #47
pfrenssenChanged the "PHP industry standard" wording to "popular standard" in the issue summary.
Also made some other small improvements and removed links to so-called "research" which are actually opinionated blog posts (one of them even cites research that makes the _opposite_ conclusion than the point the blog post is trying to make).
Comment #48
fleshgrinder commented+1 for the change of the issue summary.
I found it funny as well to find the two links in the summary because they actually argument towards snake_case. Here are the direct links to the actual studies and not the blog posts:
Yes it is true that this study concludes that camelCase results in a lower error rate due to the fact that people have a harder time reading the strings. I think this is very logical, you spend more time and thus make less mistakes. However, the fact that snake_case is faster to read is imho more important at that point—even if the researchers did not come to that conclusion—because spell checkers and IDEs will help you with correct typing. Additionally, I saw many huge code bases where camelCase did not help to reduce the amount of spelling mistakes.
This one is pretty clear I think.
However, for me the official PHP coding standard is the source of truth and will stay the source of truth, even if the community continues to ignore it completely because they prefer to adhere to standards of other languages; for whatever reasons.
Comment #49
donquixote commented@Fleshgrinder (#48):
I did not read the two articles, just had a quick look.
With studies like this, it is important which question they ask.
My personal guess would be that with snake case, the individual identifier, and its parts, are easier to read - which is probably what these studies confirm.
With camel case, on the other hand, the identifier's parts are more connected, so each identifier appears as a visual unit. This can be useful if you have e.g. multiple identifiers within a complex statement containing identifiers, arrays, brackets, conditions etc. It will become easier to grasp the structure and flow of the code as a whole, because the identifiers steal less attention. This might be more important than identifying or reading the identifier names.
So you could probably craft another study with a different answer, to a different question.
Comment #50
fleshgrinder commentedI actually do not think that the studies are that important, they are simply the only ones that are currently available. However, consider the example I brought earlier:
$routeMatcherDecoratorFactorySpecialUnicornThingy$route_matcher_decorator_factory_special_unicorn_thingyI think it is obvious which one is easier to read. But then again, it is not the main argument. Having different casing for different things is a good thing to have:1, 2
UpperCamelCase ~> something immutable or mutable with behavior and probably state (class, interface, trait, …)
lowerCamelCase ~> something immutable and callable (method, …)
UPPER_SNAKE_CASE ~> something immutable and constant (define, const, enum, …)
lower_snake_case ~> something mutable and assignable (variable, propertie, dictionary key, …)
Of course this is blurry in highly dynamic language like PHP (or JavaScript) but they are only hints and not laws. Using hints like these has a long tradition in programming ([App/System] Hungarian Notation, prefixes [set, get, is, has, add, …], design pattern names, …) and it makes sense because it reduces the cognitive load.
There are more arguments that I already brought up, like keeping it consistent in all layers (persistence layer [MySQL, PostgreSQL], application layer [Drupal], templating layer [Twig], rendering layer [JavaScript {not that common but some libraries out there are also changing from camelCase to snake_case; but the core library is written in camelCase, this means that it is a different situation as we have it here}]).
Comment #51
klausiThis has been announced for final discussion at https://groups.drupal.org/node/509885
We have many people that want this change and still a bit of dissent from David Rothstein. Fleshgrinder also does not agree, but would be ok if a majority accepts this change.
Please speak up if there is anything more to add or post a short +1/-1 if you have not voiced your opinion yet.
Comment #52
xanoI have no particular preference one way or another, but I am in favor of consistency. We currently have three notations and it would be helpful to bring that down to two. It makes it easier to type names by hand, and to use auto-completion by IDEs.
Comment #53
Crell commentedThe consistency with most of our 3rd party libs would be nice, but the work involved and the potential readability issues (snake_case_is_easier_to_read thanCamelCase) are not easy to dismiss, nor is the transitional inconsistency within our own codebase. So... I guess I'm +0. Neither option is really appealing. :-/
Comment #54
kim.pepper+1
Comment #55
larowlan+1
Comment #56
pfrenssen+1
Comment #57
klausi@Xano: what do you mean by 3 notations? I'm counting 2: $snake_case and $camelCase?
Comment #58
xanoAs mentioned in #50 We've got upper snake case for constants, but their values are technically different from variables and properties, so their notation is not that relevant here.
Comment #59
fleshgrinder commented-1
Comment #60
heddn+1
Comment #61
andypostIs there (on d.o) any voting content type?
Comment #62
sun-1
Comment #63
xjmThanks @klausi for the clarification; retitling to make the scope more clear.
(Edit: I have no preference either way about this so long as the scope is only what is in the title and not about "preferring" one or the other.)
Comment #64
markdorison+1 for the change.
Comment #65
klausiOver a month ago the Coding standards committee posted that they are drafting a comment: https://groups.drupal.org/node/510675 . Looks like that fell under the radar somewhere, putting this to RTBC to get their attention.
Also the final announcement for discussion already happened, removing tag.
Comment #66
jthorson commentedThis was on our agenda for the May 27th meeting, but unfortunately fell just short of fitting in before we bumped up against a hard stop due to other commitments.
I would expect this one to be officially wrapped up at our next meeting, which is scheduled for June 14th.
Comment #67
David_Rothstein commentedRemoved another instance of "PHP industry standard" from the issue summary; also mentioned projects like WordPress and PHP itself which don't follow it.
Comment #68
tizzo commentedThe Coding Standards Committee reviewed this issue and believe that it should be ratified to *allow* snake case in OOP code specifically. We would like to refer everyone to the follow up issue #2678238: Remove prohibition on mixing snake_case and camelCase at least for constructors for the implementation of a stronger preference. Moving to the core queue for core committer approval before creating a coder issue to update the relevant code sniffer.
Comment #69
klausiYou mean to allow camel case specifically?
The changes for coder are already done, it allows camel case and does not throw errors anymore.
Comment #70
xjmSince coder already allows both formats per @klausi's comment, and since it is a non-disruptive relaxation of a rule that does not require any changes to core code, this sensible standard is approved for core. Thanks everyone!
Comment #71
klausiGreat news! I updated the coding standards document with the suggestion from the issue summary: https://www.drupal.org/coding-standards#naming
Thanks everyone!
Comment #72
David_Rothstein commentedWait a second... what is being approved here exactly? #68 says this was ratified to allow it "in OOP code specifically", but that's not what the issue proposal and documentation are about (they talk about allowing it everywhere). Can someone clarify?
As an aside (which could be a followup issue) I also find it strange that the primary rationale for allowing this in the first place was never reflected in the coding standard change itself. Shouldn't there be a sentence in there that says something like "When extending a class, implementing an interface, implementing a hook, etc., use the same variable naming convention as the code you are extending"?
Comment #73
xanoI have never seen this before, anywhere. This statement effectively says several Drupal modules would have to follow PSR-2 when extending classes from generic packages, for instance.
Comment #74
David_Rothstein commentedI think it would just inherit the variable naming, not other code style conventions.
I tend to agree that matching variable names like that isn't particularly important, but a major assumption of this entire issue was that it in fact is important (see the "camelCase variable naming is important for PHP coding standards interoperability" section in the issue summary above). So if it's not, then I'm not sure what the ultimate rationale for this change actually is/was....
Comment #75
benjy commentedDoes this mean we can start using this naming in any core code right now?
Comment #76
klausiYes, this standard has been approved. We are only waiting for tizzo to clarify if it only applies to OO code or to all variables as proposed in the issue summary.
Comment #77
xjmComment #78
jthorson commented#76
The ratification applies to OO code only ... an update to the issue summary would be appreciated. :)
Comment #79
klausiThat is unfortunate - so we have to come up with completely new wording and 2 different standards on https://www.drupal.org/coding-standards#naming and https://www.drupal.org/node/608152#naming , which is IMO confusing and inconsistent.
Comment #80
pfrenssenComment #81
drunken monkeyI agree, introducing different standards for local variables and parameters depending on which file they're in seems like a bad idea.
In D8, most of the code is in OOP files anyways, so having variables that use a different naming scheme than the function names just for the few non-OOP files doesn't seem like such a problem. Better to be able to be consistent in the OOP files.
Moreover, we still allow both, so everyone is free to just use snail_case in procedural files and camelCase in OOP files.
Comment #82
David_Rothstein commented@jthorson (or anyone else who participated), what was the reason for ratifying it like that? It doesn't match the proposal, and in fact was explicitly rejected before (see #15 and onwards).
Doing it that way makes the standards more complicated, and also promotes inconsistency, e.g. if you decide that you really want camel case variables so it can match class names, you're allowed to do this:
but then aren't allowed to do this:
which is inconsistent.
Comment #83
jthorson commentedWe can flag this for further discussion at this week's meeting, but until then, I can take a stab at answering ...
First, we recognize that this topic is not fully resolved within this issue, and that the current ratification is an interim step. We anticipate further discussion and policy tweaks in #2678238: Remove prohibition on mixing snake_case and camelCase at least for constructors.
Second, we acknowledge the arguments that a lot of our included libraries in D8 leverage camelCase, and the related interoperability benefits as discussed in this thread. Most (all?) of these libraries were added in D8, and are thus assumed to be primarily referenced in object oriented code.
However, during the discussion of this issue (along with a couple of other ones), a larger concern with our overall coding standards processes was flagged. As alluded to in #2788295: How to handle and document deprecated/versioned coding standards?, our coding standards are considered version-independent, and all new code is supposed to follow current coding standards, regardless of core version. Many of the arguments which support allowing camelCase are compelling in the context of D8 and new code moving forward; but applying the same policy to D7 core and contrib would be less desireable ... We currently have a consistent pattern across the majority of our legacy code base, and there is some benefit in maintaining that consistency.
Given that more discussion on this issue is expected, and the current ratification is expected to be an interim position with further refinement coming after the 'preference' discussion, restricting the policy to object-oriented code is a compromise which allows us to move forward and allow for more flexibility within the D8 development process today, without introducing an associated negative impact on our (primarily procedural) legacy codebase.
This certainly does introduce inconsistency and complexity into the coding standards policy in the meantime; but I would anticipate this being temporary, while we further evolve our coding standards policy & process towards a model that better accommodates versioned introduction and deprecation of standards/policies.
Comment #84
David_Rothstein commentedThanks for the explanation.
However since Drupal 7 has plenty of object-oriented code and Drupal 8 has plenty of procedural code, I think making this apply only to object-oriented code just creates confusion for both versions... without actually meeting your goal.
I also don't agree with the goal. I think (like most coding standards) anything decided here should apply to both Drupal 7 and 8. That's the current policy and there are good reasons for it. But I will comment further on #2788295: How to handle and document deprecated/versioned coding standards? about that.
Comment #85
jthorson commentedAdding to the explanation above, it was recognized that all of the items described in the 'problem motivation' section of the issue summary applied primarily to object oriented code, but there was not a compelling problem motivation statement which would apply to procedural code.
Fundamentally, the committee has reservations about the more relaxed policy position, given the potential for it to lead to greater inconsistency between projects (or even within a project).
But given the inconsistency example pointed out in #82, we discussed further and agreed to the amendment to relax the restriction and stop disallowing camelCase for both OO and procedural code, provided you remain consistent within a single file.
Comment #86
tizzo commentedThe documentation has been updated and the standard has been relaxed, marking fixed.
Comment #87
David_Rothstein commentedThanks! Adding back the tags I removed above.
I guess there could still be a followup issue for #72, but not sure it's worth the effort.
Comment #88
maximpodorov commentedPlease explain whether this camelCase policy applies to D7.
Comment #89
David_Rothstein commented@maximpodorov, yes. See the "Drupal coding standards are version-independent" paragraph at the top of https://www.drupal.org/coding-standards.
However the fact that this is essentially an "optional" standard, plus the idea that you are supposed to use the same variable-naming convention throughout an entire file, probably means there won't be many camelCase variables in Drupal 7 (if any) in practice. If you want more you could try to convince the people above to make it per-function/method rather than per-file :)
Comment #91
alex.skrypnykAfter this change in Coder (https://www.drupal.org/project/coder/issues/2303963#comment-10935979), we do not have a capability to distinguish between camelCase and snake_cased arguments in methods. Moreover, these can be mixed within a single method!
I think this is not the type of the outcome we wanted: to have the codebase full of mixed types (not even within same file - within same argument list).
It would be very helpful to know if there is a Sniff that can enforce snake_case in the method arguments list that would help to ensure consistency.
Comment #92
donquixote commentedPersonally nowadays I like to put custom phpcs.xml into e.g. a contrib module, to have a relaxed version of Drupal CS.