Currently, the standards don't define whether there should be a blank line after a file's initial <?php line. To remedy this, we propose appending the following to the coding standards' "PHP Code Tags" section:

All blocks at the beginning of a PHP file should be separated by a blank line. This includes the /** @file */ block, the namespace declaration and the use statements as well as the following code in the file. So, for example, a file header might look as follows:

<?php

namespace This\Is\The\Namespace;

use Drupal\foo\Bar;

/**
 * Provides examples.
 */
class ExampleClassName {

Or, for a non-class file (e.g., .module):

<?php

/**
 * @file
 * Provides example functionality.
 */

use Drupal\foo\Bar;

/**
 * Implements hook_help().
 */
function example_help($route_name) {

Comments

silverwing’s picture

Project: Drupal.org site moderators » Drupal core
Version: » 7.x-dev
Component: Textual improvements » documentation
jhodgdon’s picture

Title: Moderation report for API documentation and comment standards » [policy] Should we require a blank line between <?php and @file docs?
Version: 7.x-dev » 8.x-dev
Issue summary: View changes
Issue tags: -@file, -blank line, -empty line +Coding standards

I am not aware that we have a standard for that. We can discuss it. Changing title and tags to reflect such a discussion

By the way, please do not add random tags to issues. Read the issue tag guidelines (just below the tags field).

Michael Hodge Jr’s picture

I went through a project that we recently did and looked at the 50+ contributed modules that were installed trying to see if i could ascertain if there was a sort of de facto standard (even if the sample size is really small).

I found more modules tended to use the space, rather than ones that didn't, but then again there were a handful of modules that didn't even have the @file declaration at all.

Personally, I find the space to be a little easier on the eyes, but that's just me. Having not been a part of a coding standards discussion before, I'm curious as to what arguments are made for and against the space or even if it should be included in the coding standards at all.

jhodgdon’s picture

Project: Drupal core » Drupal Technical Working Group
Version: 8.0.x-dev »
Component: documentation » Code

Coding standards changes are now part of the TWG

jhodgdon’s picture

There are several duplicates of this issue that I'm in the process of closing; will make sure they are listed as parent/related issues so they will appear on the sidebar here.

tizzo’s picture

Project: Drupal Technical Working Group » Coding Standards
pfrenssen’s picture

Component: Code » Coding Standards

I have a slight preference for having a space in between the opening PHP tag and the file docblock, but I personally think this is not of such importance that it should be a part of the standard. The code is perfectly readable either way.

tim.plunkett’s picture

Yes.

drunken monkey’s picture

I also prefer it with the blank line, it just looks a bit nicer. But maybe we can tone it down to a recommendation instead of a fixed rule. Holding up, e.g., the project application process for that wouldn't be worth it.

Liam Morland’s picture

My preference is to keep the files shorter by leaving the space out, but it's not a big deal. The most important thing for me is that there is a recommendation one way or the other.

pfrenssen’s picture

Let's do a tally for this issue as well as the duplicates.

From #1339022: Standardize yes/no on empty line before the @file docblock:

  • Webchick: 0 or +1, I think she uses the blank line herself but is neutral in enforcing it. Counting as 0.
  • Dave Reid: +1
  • Neclimdul: -1, but toning down in a second comment since it is consistent with Symfony.
  • sun: +1

From #1852152: [policy, no patch] Blank line before @file docblock in PHP file types?:

  • tim.plunkett: +1
  • Lars Toomre: +1

This issue:

  • Michael Hodge Jr: +1
  • pfrenssen: 0
  • drunken monkey: +1
  • Liam Morland: -1

Total: 6 in favor, 2 against, 2 meh.

alex.skrypnyk’s picture

Issue summary: View changes
alex.skrypnyk’s picture

Can we please make a decision on this after 2 years?

pfrenssen’s picture

It looks like it's pretty much decided. We just need to write the statement to include in the coding standards and approve it.

dawehner’s picture

IMHO its kinda pointless to discuss this issue as you can get rid of #2304909: Relax requirement for @file when using OO Class or Interface per file

pfrenssen’s picture

Title: [policy] Should we require a blank line between <?php and @file docs? » [policy] Should we require a blank line after <?php?

Yeah but then the question will be between the <?php and the namespace declaration, which is in essence the same discussion.

Maybe it's better to rename this to "require a blank line after <?php".

drunken monkey’s picture

Issue tags: +needs announcement for final discussion

@ pfrenssen: Thanks for tallying, seems this is really decided now.
Tagging for final discussion. (I hope that's the way to go, sorry otherwise.)

khusingh’s picture

Assigned: Unassigned » khusingh
khusingh’s picture

Assigned: khusingh » Unassigned
DamienMcKenna’s picture

I'd vote to "no", but I don't think there was a poll set up anywhere ;) I find it to be needless - there are already six lines (the :lt;?php line, a four-line docblock, plus one blank line) before it gets to the docblock for the first actual bit of code, plenty of breathing space, so why do we need another line?

andypost’s picture

+1 cos readabilty, just require empty line after <?php

Crell’s picture

It's not approved yet, but it appears likely that PSR-12, the new FIG coding standards that extend PSR-2, will require the header section of a file to be newline separated between each block. Vis, opening PHP tag (blank line) docblock (blank line) namespace declaration (blank line) use statements (blank line), etc.

That is, this change is a good anticipation of PSR-12. I am therefore +1.

[Edit: I'm a dope and referenced the wrong PSR number.]

dawehner’s picture

Here is a suggested addition to the coding standard page:

Each block of the beginning of the PHP file should be separated with an empty new line. This includes the /** @file */ block, the namespace and the use statements as well as the class statements. So for example a file header should look like:



/**
 * @file
 * This is the file description.
 */

namespace This\Is\The\Namespace;

use Drupal\foo\Bar;

class ExampleClassName 
Perignon’s picture

@dawehner That example doesn't include a blank line after the php declaration. That is what is primarily being discussed here.

I am on the fence about this one. I could honestly go either way. If you ask me what I do in practice, I include the blank like after <?php

dawehner’s picture

@Perignon
Well, this are some rendering issue of the syntax highlighting on drupal.org

DamienMcKenna’s picture

@dawehner: I almost LOL'd at how the email notification of your message removed all of the blank lines your comment tried to demonstrate. Technology, eh! ;-)

Crell’s picture

For reference, here's the PR where FIG is discussing this question for PSR-12: https://github.com/php-fig/fig-standards/pull/730

While I cannot of course guarantee that it won't change before PSR-12 is approved, I would be supportive of us following FIG's current trajectory on this one and adopting that header style (which includes a blank line after the opening php tag, as discussed here).

[Edit: I'm a dope and referenced the wrong PSR number.]

xjm’s picture

I think the standard as stated in the current title (blank line after <?php) improves readability and is more consistent with our other whitespace guidelines about blank linkes between members, at the beginning and end of classes, etc., so +1.

effulgentsia’s picture

Title: [policy] Should we require a blank line after <?php? » Require a blank line after <?php
Issue tags: +Needs issue summary update

+1 to announcing this for final discussion. I'll bring it up on our next call to see if the rest of the committee agrees.

Meanwhile, retitling with the proposal that has the most support so far.

Also tagging for an issue summary update. Would be good to copy the specific suggestion from #23, mention who's against it so far and why, and include the reference to the PSR-12 draft. Also, any insight on how close PSR-12 is to being adopted? If it's close, would it make sense to postpone this issue on it?

effulgentsia’s picture

Issue tags: -needs announcement for final discussion +final discussion

I'll bring it up on our next call to see if the rest of the committee agrees.

Ahem, this was actually already done: https://groups.drupal.org/node/509885. Fixing the tag. Issue summary update would still be good though.

alex.skrypnyk’s picture

Title: Require a blank line after <?php » Should we require a blank line after <?php?
alex.skrypnyk’s picture

Title: Should we require a blank line after <?php? » Require a blank line after <?php

Sorry, did not realise this was already changed on purpose.

NerOcrO’s picture

+1 for a new line like @xjm :)

drunken monkey’s picture

Updated the issue summary, also correcting the example to follow #2304909: Relax requirement for @file when using OO Class or Interface per file (and actually mentioning where this should go). Please correct me if I got anything wrong there.

alexpott’s picture

So looking at the examples on PSR-12... https://github.com/php-fig/fig-standards/blob/master/proposed/extended-c... actually having a new line after the opening PHP tag is appears against that standard from the first few examples. Looking at vendor though Symfony always has a new line here - but they always have a comment about copyright. Guzzle doesn't have a new line between php and namespace. Actually reading the proposed standard - it actually does not make any statement about what immediately follows a php open tag apart from

When the opening <?php tag is on the first line of the file, it MUST be on its own line with no other statements unless it is a file containing markup outside of PHP opening and closing tags.

And in fact the proposal has examples of both with and without.

The only time it mandates anything is this...

There MUST NOT be a blank line before declare statements such as those for strict types or ticks. They MUST be contained on the lines immediately following the opening tag (which must be on the first line when declare statement(s) are present).

But given we're not using this PHP feature atm that doesn't feel too relevant.

So, tldr, PSR-12 doesn't guide us here at all (yet - of course that could change).

alexpott’s picture

If we do mandate a blank line then we are going to need to think about how this contradicts the one thing PSR-12 does say on the subject of blank lines ie.. the bit about declare statements. And given that I actually think we should consider having no blank line because it will be easier to implement when we do have the need to declare strict types and given the @file docblocks have been removed or a collapsed by default in editors I don't really think readability suffers anyway.

Liam Morland’s picture

I suggest making a pull request against PSR-12 either adding or removing the blank lines so that all examples are consistent. Then discussion about the pull request could iron out this issue there. I think Drupal should follow whatever PSR is doing.

DamienMcKenna’s picture

+1 for adopting an existing standard, and if one doesn't exist then we push the closest one to covering this too (i.e. PSR-12).

Crell’s picture

The mandatory blank line for PSR-12 is still in a pull request; I'll have to harass the editor to see why it's not been merged yet.

At this point PSR-12 is at best advisory, as it is subject to change, as noted. However, it's also true that we can influence it. Although Drupal doesn't use PSR-2, "what are people actually doing" is a significant input to PSR-12. If Drupal approves a mandatory blank line, PSR-12 is (somewhat) more likely to do so eventually as well. And vice versa.

The alternative would be to clearly tell FIG that we want to go blank-line, but are waiting on them to figure this stuff out so we can follow their lead, at least for the "header formatting". I'm fine with that approach as well.

Disclaimer: I'm the Drupal FIG rep so telling FIG what our preference is would explicitly be part of my job.

dawehner’s picture

Well, we use the newline pretty much everywhere, so documenting that as part of the CS seems more like a technical detail at that point.

effulgentsia’s picture

If we do mandate a blank line then we are going to need to think about how this contradicts the one thing PSR-12 does say on the subject of blank lines ie.. the bit about declare statements.

@jthorson, @tizzo, and I discussed this on our last coding standards call, and were wondering why PSR-12 is currently proposing to require no blank line before declare statements? If they had a good reason for this, and if it's likely to remain this way when the standard is adopted, then perhaps we should try to keep Drupal compliant with that, and modify this issue's proposal to specifically call out an exception for when a file has a declare statement?

On the other hand, we generally prefer simpler coding standards to more complex ones (i.e., where possible, we prefer do this over do this except when that), so we're open to reading arguments about why Drupal should violate the current PSR-12 proposal with respect to a blank line before declare. Such an argument could be along the lines of:

The mandatory blank line for PSR-12 is still in a pull request

If there's a link with a discussion about that.

Crell’s picture

The PR discussion is here:

https://github.com/php-fig/fig-standards/pull/730

Disclaimer: Yes, I wrote it. :-) The main argument in favor is the same "simplicity, fewer exceptions" one as made here. The main argument against is some existing doc parsers will apparently complain because they mandate the opposite. The response to that seems to be "that's nice, they'll need to be updated anyway for PSR-12, they can update that too." The PR is basically waiting on "people doing other things and got busy".

There's an amusing chicken-and-egg here, as PSR-12 is, in theory, supposed to be based in part on what people are doing in practice, whereas we're here waiting to see what PSR-12 does before deciding what to do in practice.

My personal stance is that Drupal adopting the simpler/better "new line between each section, consistently" format now will help nudge PSR-12 to adopting that same policy, so we should do so. It's not a guarantee, but it improves the odds. And if in the end PSR-12 goes the other way and we decide to adapt to match PSR-12 in this regard, it's a fairly non-invasive change and easily scriptable across the repo.

effulgentsia’s picture

Project: Coding Standards » Drupal core
Version: » 8.2.x-dev
Component: Coding Standards » other
Status: Active » Reviewed & tested by the community
Issue tags: -final discussion +Approved Coding Standard

Thanks for #42. Given the discussion on that PR, @jthorson, @tizzo, and I agreed it makes sense to move forward with this as described and not make an exception for declare.

If PSR-12 is adopted with the requirement of no blank line before declare, then we can open a new issue at that time to discuss bringing Drupal's coding standards into alignment with that.

Moving to the core queue per step 7 of https://www.drupal.org/project/coding_standards, although I did not do a grep to see if any core code or core patches are affected.

xjm’s picture

Status: Reviewed & tested by the community » Postponed

Thanks @effulgentsia.

We'll adopt and apply this for core once #2754187: Add rule for a blank line following a PHP tag is available. Postponing on that.

klausi’s picture

Status: Postponed » Reviewed & tested by the community

All required sniffs for this are now fully implemented in Coder (dev version, not released yet). Please test!

xjm’s picture

Project: Drupal core » Coding Standards
Version: 8.2.x-dev »
Component: other » Coding Standards
Issue tags: +Core Committer Approved

Yay, thanks @klausi! I tested the rule locally and it seems to work correctly. There are a handful of files in core that are not compliant.

Filed #2755677: Fix 'Drupal.WhiteSpace.OpenTagNewline' coding standard.

This rule is committer-approved pending making core compliant, having the rule in a tagged release of coder, and enabling the rule in phpcs.xml.dist. We will do these things during the RC phase of 8.2.x which begins September 9, 2016.

alexpott’s picture

I agree with @xjm about waiting till the rc phase for 8.2 for introducing a new rule. Also doing #2744463: Add phpcs, coder 8.2.8 as --dev requirements for drupal/core will allow us to manage coding standards change better.

David_Rothstein’s picture

Approved (belatedly) for Drupal 7 core also. (I guess if these issues aren't going into the Drupal 7 RTBC queue ever, Drupal 7 maintainers will have to start keeping an eye for them over here too...)

One minor suggestion, when moving the final documentation into the coding standards, instead of this:

All blocks at the beginning of a PHP file should be separated by a blank line. This includes the /** @file */ block, the namespace declaration and the use statements as well as the following code in the file.

I would suggest this:

All blocks at the beginning of a PHP file should be separated by a blank line. This includes the /** @file */ block, the namespace declaration and the use statements (if present) as well as the following code in the file.

(emphasis added)

Since I don't think any of the previous-mentioned items are always required to be there in the first place.

Crell’s picture

Feel free to steal the exact language from the PSR-12 draft, or suggest improvements to it over in that PR.

xjm’s picture

Issue tags: +8.2.0 release notes
tizzo’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.