Newlines at file endings

Implemented in DrupalCodingStandard_Sniffs_WhiteSpace_EmptyLinesSniff, see http://drupalcode.org/project/drupalcs.git/commit/2b17f23

Sorry to reopen a closed issue in a closed project, but I can't find any other trace.

- This issue was committed on Dec 5th http://drupalcode.org/project/drupalcs.git/commitdiff/2b17f23
- Then 3 days later, the file was splitted into another file, and the code changed completely http://drupalcode.org/project/coder.git/commitdiff/3c4f2fa, now does not allow blank ending line.

Then which one is correct? Most Drupal core files don't have blank ending line, but they cause the verbose "\ No newline at end of file" without the blank ending line.

Drupal Code Sniffer has been merged into Coder 7.x-2.x. Please move this issue to the Coder queue and reopen it if the problem still exists.

"The problem" is in fact an open question.

The current code should work as expected and as defined in our coding standards: https://drupal.org/coding-standards#indenting

Please note that some editors display the last "\n" in a file as separate line (e.g. Eclipse) and some not (e.g. Vim).

I don't understand why I'm the only one who complains that. Currently, in core, there is no file that ends with a new line. And if files end with a new line, Coder makes a warning (see screenshot). I'm actually using vim.

If I remove the last empty line (so that my file ends with "\n}" (or 0a7d) and not "\n}\n"), Coder says ok.

It is not what described in the coding standards https://drupal.org/coding-standards#indenting (the last paragraph).

It is single new line character, which is "\n". Vim does not display that as separate line.

Vim does display. In my example above, the last 3 characters are "\n}\n" (or "0a7d0a" by hexdump), and Vim does display the last character "\n" (line 2038).

In other IDE, like PHPStorm or Eclipse, a new "virtual" line is added (I'll see line 2039 even there is only 2038 lines).

I believe this to be erroneously or at least inadvertently marked closed given the last couple replies. Can we verify whether this is working correctly according to the docs? A failure on something this simple would understandably discourage new contributors so I think it important we be sure to get it right.

This is working fine since a long time now, please provide a test case/file where you think it reports a false positive.

I'm seeing the same erroneous behaviour. It's complaining about there being no newline at the end of my test file, but there definitely is one.

❯ phpcs --standard=/Users/psynaptic/repos/drupal/coder/7.x-2.x/coder_sniffer/Drupal ./scratch.php

FILE: /Users/psynaptic/repos/drupal/coder/7.x-2.x/scratch.php
--------------------------------------------------------------------------------
FOUND 2 ERRORS AND 1 WARNING AFFECTING 3 LINES
--------------------------------------------------------------------------------
  4 | WARNING | Format should be "* Implements hook_foo().", "* Implements
    |         | hook_foo_BAR_ID_bar() for xyz_bar().", or "* Implements
    |         | hook_foo_BAR_ID_bar() for xyz_bar.tpl.php.".
  5 | ERROR   | There must be exactly one blank line after the file comment
 14 | ERROR   | Files must end in a single new line character
--------------------------------------------------------------------------------

It also displays an erroneous error about the file docblock.

I've attached the file so it can be tested by others. I'm on the phpcs-2.0 branch btw.

I'll happily make a patch for this but wanted to post it here now, since I don't know which branch to commit it to yet.

diff --git a/coder_sniffer/Drupal/Sniffs/WhiteSpace/FileEndSniff.php b/coder_sniffer/Drupal/Sniffs/WhiteSpace/FileEndSniff.php
index 1c3f447..2a3b396 100644
--- a/coder_sniffer/Drupal/Sniffs/WhiteSpace/FileEndSniff.php
+++ b/coder_sniffer/Drupal/Sniffs/WhiteSpace/FileEndSniff.php
@@ -79,13 +79,12 @@ class Drupal_Sniffs_WhiteSpace_FileEndSniff implements PHP_CodeSniffer_Sniff
             }
             $content = file_get_contents($filename);
             $error = false;
-            $lastChar = substr($content, -1);
-            // There must be a \n character at the end of the last token.
-            if ($lastChar !== $phpcsFile->eolChar) {
+            // There must be a blank newline at the end of the file.
+            if (substr($content, -2, 1) !== $phpcsFile->eolChar) {
                 $error = true;
             }
-            // There must be only one \n character at the end of the file.
-            else if (substr($content, -2, 1) === $phpcsFile->eolChar) {
+            // There must be only one blank newline at the end of the file.
+            else if (substr($content, -3, 1) === $phpcsFile->eolChar) {
                 $error = true;
             }

I moved the substr expression into the conditional to match the one below and make the code more readable. I also made the comments more accurate.

Aside from that, what coding standards are we following with the rules? I see else if is being used more often than elseif which is against the Drupal coding standards, but perhaps not the ones used for the phpcs rules.

Setting correct component and pushing to 8.x-2.x as it should be fixed there first and backported.

The test file from #13 has two newline characters at the end, so the sniff correctly reports that.

Oh, and we are using PHPCS coding standards for our sniffs to be more compatible with PHPCS when we copy or modify sniffs.

I see! You are right, klausi.

To prove exactly how spectacularly wrong I am, I downloaded the latest release (8.x-2.0-alpha1) and ran some tests to (hopefully) clear this up once and for all.

Here's the first example showing a file without a newline (single \n character) at the end of the file:

❯ cat --show-all file.php
<?php$
$
/**$
 * @file$
 * Provides a test case.$
 */$
$
/**$
 * Does nothing at all.$
 */$
function foo() {$
}
❯ git diff
diff --git a/file.php b/file.php
index 7e6681a..a2d2877 100644
--- a/file.php
+++ b/file.php
@@ -9,4 +9,4 @@
  * Does nothing at all.
  */
 function foo() {
-}
+}
\ No newline at end of file
❯ hexdump file.php
0000000 3c 3f 70 68 70 0a 0a 2f 2a 2a 0a 20 2a 20 40 66
0000010 69 6c 65 0a 20 2a 20 50 72 6f 76 69 64 65 73 20
0000020 61 20 74 65 73 74 20 63 61 73 65 2e 0a 20 2a 2f
0000030 0a 0a 2f 2a 2a 0a 20 2a 20 44 6f 65 73 20 6e 6f
0000040 74 68 69 6e 67 20 61 74 20 61 6c 6c 2e 0a 20 2a
0000050 2f 0a 66 75 6e 63 74 69 6f 6e 20 66 6f 6f 28 29
0000060 20 7b 0a 7d
0000064
❯ ../sites/all/modules/coder/vendor/bin/phpcs --standard=/srv/d7/sites/all/modules/coder/coder_sniffer/Drupal -- ./file.php

FILE: /srv/d7/test2/file.php
--------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------
 12 | ERROR | Files must end in a single new line character
--------------------------------------------------------------------------------

Here's the other side of the test (after committing the file):

❯ cat --show-all file.php
<?php$
$
/**$
 * @file$
 * Provides a test case.$
 */$
$
/**$
 * Does nothing at all.$
 */$
function foo() {$
}$
❯ hexdump file.php
0000000 3c 3f 70 68 70 0a 0a 2f 2a 2a 0a 20 2a 20 40 66
0000010 69 6c 65 0a 20 2a 20 50 72 6f 76 69 64 65 73 20
0000020 61 20 74 65 73 74 20 63 61 73 65 2e 0a 20 2a 2f
0000030 0a 0a 2f 2a 2a 0a 20 2a 20 44 6f 65 73 20 6e 6f
0000040 74 68 69 6e 67 20 61 74 20 61 6c 6c 2e 0a 20 2a
0000050 2f 0a 66 75 6e 63 74 69 6f 6e 20 66 6f 6f 28 29
0000060 20 7b 0a 7d 0a
0000065
❯ git diff
diff --git a/file.php b/file.php
index a2d2877..7e6681a 100644
--- a/file.php
+++ b/file.php
@@ -9,4 +9,4 @@
  * Does nothing at all.
  */
 function foo() {
-}
\ No newline at end of file
+}
❯ ../sites/all/modules/coder/vendor/bin/phpcs --standard=/srv/d7/sites/all/modules/coder/coder_sniffer/Drupal -- ./file.php

We can see from 3 different sources how wrong I am (oh and also that and that phpcs didn't throw the error).

Conclusion

First of all, there is nothing wrong with the sniff, so if that's all you're interested in you can stop reading now.

Why this is confusing people (myself included) has become clear to me.

The confusion is caused by different editors handling the display of the newline in different ways. In vim you don't see the newline as an actual blank line, whereas in other editors e.g. TextMate you do see a whole blank line. Seeing the blank line in the editor makes it easier, for better or worse, to be able to remove the final \n character from a file.

I've been duped into thinking a newline at the end of a file meant a whole blank line when really this was just a product of the way the editor I used long ago handled the display of the final newline character. I've actually been putting blank lines at the end of files all this time!

Anyway, I think that's about all I can say on the no-newline-at-end-of-file issue.