Fix #568, don't output empty files without error

Now when an error in formatting happens, it never outputs anything and
doesn't override the file when working inplace.

Additionally dfmt is no longer able to fail in the middle of a file, as
now we first write everything to a buffer and only if everything was
successful, that buffer is printed to stdout or written to the inplace
file.

This should also guard against segfaults with inplace file formatting
erasing parts of the file, as well as the user thinking it was
successful, even though dfmt didn't finish properly.

.travis.sh

@@ -12,4 +12,4 @@ else
     make debug -j2
 fi
 
-cd tests && ./test.sh
+cd tests && ./test.d

makefile

@@ -31,7 +31,7 @@ gdc:githash
 	$(GDC) $(SRC) $(GDC_FLAGS) -obin/dfmt
 
 test: debug
-	cd tests && ./test.sh
+	cd tests && ./test.d
 
 bin/dfmt-test: githash $(SRC)
 	$(DC) $(DMD_TEST_FLAGS) $^ -of$@

src/dfmt/main.d

@@ -38,13 +38,13 @@ version (NoMain)
 }
 else
 {
-    import std.array : front, popFront;
-    import std.stdio : stdout, stdin, stderr, writeln, File;
     import dfmt.config : Config;
-    import dfmt.formatter : format;
-    import std.path : buildPath, dirName, expandTilde;
     import dfmt.editorconfig : getConfigFor;
+    import dfmt.formatter : format;
+    import std.array : appender, front, popFront;
     import std.getopt : getopt, GetOptException;
+    import std.path : buildPath, dirName, expandTilde;
+    import std.stdio : File, stderr, stdin, stdout, writeln;
 
     int main(string[] args)
     {
@@ -153,14 +153,13 @@ else
         args.popFront();
         immutable bool readFromStdin = args.length == 0;
 
-        File output = stdout;
         version (Windows)
         {
             // On Windows, set stdout to binary mode (needed for correct EOL writing)
             // See Phobos' stdio.File.rawWrite
             {
                 import std.stdio : _O_BINARY;
-                immutable fd = output.fileno;
+                immutable fd = stdout.fileno;
                 _setmode(fd, _O_BINARY);
                 version (CRuntime_DigitalMars)
                 {
@@ -220,7 +219,7 @@ else
                     break;
             }
             immutable bool formatSuccess = format("stdin", buffer,
-                output.lockingTextWriter(), &config);
+                stdout.lockingTextWriter(), &config);
             return formatSuccess ? 0 : 1;
         }
         else
@@ -262,10 +261,16 @@ else
                 {
                     buffer = new ubyte[](cast(size_t) f.size);
                     f.rawRead(buffer);
-                    if (inplace)
+                    auto output = appender!string;
-                        output = File(path, "wb");
+                    immutable bool formatSuccess = format(path, buffer, output, &config);
-                    immutable bool formatSuccess = format(path, buffer, output.lockingTextWriter(), &config);
+                    if (formatSuccess)
-                    if (!formatSuccess)
+                    {
+                        if (inplace)
+                            File(path, "wb").rawWrite(output.data);
+                        else
+                            stdout.rawWrite(output.data);
+                    }
+                    else
                         retVal = 1;
                 }
             }

tests/expected_failures/issue0568.d

@@ -0,0 +1 @@
+### This file should not be erased just because it is not formatted correctly

tests/test.d

@@ -29,39 +29,55 @@ int main()
             if (const result = spawnProcess(dfmtCommand, stdin, File(outFileName, "w")).wait)
                 return result;
 
-            const outText = outFileName.readText;
+            if (int ret = diff(refFileName, outFileName))
-            const refText = refFileName.readText;
+                return ret;
-            const outLines = outText.splitLines(Yes.keepTerminator);
-            const refLines = refText.splitLines(Yes.keepTerminator);
-            foreach (i; 0 .. min(refLines.length, outLines.length))
-                if (outLines[i] != refLines[i])
-                {
-                    writeln("Found difference between ", outFileName, " and ", refFileName, " on line ", i + 1, ":");
-                    writefln("out: %(%s%)", [outLines[i]]); // Wrapping in array shows line endings.
-                    writefln("ref: %(%s%)", [refLines[i]]);
-                    return 1;
-                }
-            if (outLines.length < refLines.length)
-            {
-                writeln("Line ", outLines.length + 1, " in ", refFileName, " not found in ", outFileName, ":");
-                writefln("%(%s%)", [refLines[outLines.length]]);
-                return 1;
-            }
-            if (outLines.length > refLines.length)
-            {
-                writeln("Line ", outLines.length + 1, " in ", outFileName, " not present in ", refFileName, ":");
-                writefln("%(%s%)", [outLines[refLines.length]]);
-                return 1;
-            }
         }
 
     foreach (entry; dirEntries("expected_failures", "*.d", SpanMode.shallow))
-        if (execute([dfmt, entry]).status == 0)
+    {
+        string copied = entry ~ ".out.d";
+        copy(entry, copied);
+        scope (exit)
+            remove(copied);
+        if (execute([dfmt, copied, "--inplace"]).status == 0)
         {
             stderr.writeln("Expected failure on test ", entry, " but passed.");
             return 1;
         }
 
+        if (int ret = diff(entry, copied))
+            return ret;
+    }
+
     writeln("All tests succeeded.");
     return 0;
 }
+
+int diff(string refFileName, string outFileName)
+{
+    const outText = outFileName.readText;
+    const refText = refFileName.readText;
+    const outLines = outText.splitLines(Yes.keepTerminator);
+    const refLines = refText.splitLines(Yes.keepTerminator);
+    foreach (i; 0 .. min(refLines.length, outLines.length))
+        if (outLines[i] != refLines[i])
+        {
+            writeln("Found difference between ", outFileName, " and ", refFileName, " on line ", i + 1, ":");
+            writefln("out: %(%s%)", [outLines[i]]); // Wrapping in array shows line endings.
+            writefln("ref: %(%s%)", [refLines[i]]);
+            return 1;
+        }
+    if (outLines.length < refLines.length)
+    {
+        writeln("Line ", outLines.length + 1, " in ", refFileName, " not found in ", outFileName, ":");
+        writefln("%(%s%)", [refLines[outLines.length]]);
+        return 1;
+    }
+    if (outLines.length > refLines.length)
+    {
+        writeln("Line ", outLines.length + 1, " in ", outFileName, " not present in ", refFileName, ":");
+        writefln("%(%s%)", [outLines[refLines.length]]);
+        return 1;
+    }
+    return 0;
+}

tests/test.sh

@@ -1,32 +0,0 @@
-#!/usr/bin/env bash
-set -e
-
-for braceStyle in allman otbs knr
-do
-	for source in *.d
-	do
-		test "$(basename $source '.d')" = 'test' && continue
-
-		echo "${source}.ref" "${braceStyle}/${source}.out"
-		argsFile=$(basename "${source}" .d).args
-		if [ -e "${argsFile}" ]; then
-			args=$(cat "${argsFile}")
-		else
-			args=
-		fi
-		../bin/dfmt --brace_style=${braceStyle} ${args} "${source}" > "${braceStyle}/${source}.out"
-		diff -u "${braceStyle}/${source}.ref" "${braceStyle}/${source}.out"
-	done
-done
-
-set +e
-
-for source in expected_failures/*.d
-do
-	if ../bin/dfmt "${source}" > /dev/null; then
-		echo "Expected failure on test ${source} but passed"
-		exit 1
-	fi
-done
-
-echo "This script is superseded by test.d."