mirror of
https://github.com/dlang-community/dfmt.git
synced 2025-04-25 21:00:03 +03:00
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.
This commit is contained in:
parent
e251e4343a
commit
4a6078779a
6 changed files with 60 additions and 70 deletions
|
@ -12,4 +12,4 @@ else
|
|||
make debug -j2
|
||||
fi
|
||||
|
||||
cd tests && ./test.sh
|
||||
cd tests && ./test.d
|
||||
|
|
2
makefile
2
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$@
|
||||
|
|
|
@ -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)
|
||||
output = File(path, "wb");
|
||||
immutable bool formatSuccess = format(path, buffer, output.lockingTextWriter(), &config);
|
||||
if (!formatSuccess)
|
||||
auto output = appender!string;
|
||||
immutable bool formatSuccess = format(path, buffer, output, &config);
|
||||
if (formatSuccess)
|
||||
{
|
||||
if (inplace)
|
||||
File(path, "wb").rawWrite(output.data);
|
||||
else
|
||||
stdout.rawWrite(output.data);
|
||||
}
|
||||
else
|
||||
retVal = 1;
|
||||
}
|
||||
}
|
||||
|
|
1
tests/expected_failures/issue0568.d
Normal file
1
tests/expected_failures/issue0568.d
Normal file
|
@ -0,0 +1 @@
|
|||
### This file should not be erased just because it is not formatted correctly
|
66
tests/test.d
66
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;
|
||||
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;
|
||||
}
|
||||
if (int ret = diff(refFileName, outFileName))
|
||||
return ret;
|
||||
}
|
||||
|
||||
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;
|
||||
}
|
||||
|
|
|
@ -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."
|
Loading…
Add table
Add a link
Reference in a new issue