cue/cmd: avoid overwriting formatted files in cue fmt

Currently, `cue fmt` will always write to cue files, regardless of
whether they are formatted or not. In the case of formatted files,
their contents will remain exactly the same, but their modification
time will change.
This is problematic for tools that rely on the mod time.

To fix this, we buffer the original/formatted bytes. If the result
is exactly the same, the file is not written to.

Fixes #1731, #1744

Signed-off-by: Noam Dolovich <[email protected]>
Change-Id: I67e7fc80b0e108a2cb6c7bfb4090e29bd47cb995
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1193934
TryBot-Result: CUEcueckoo <[email protected]>
Unity-Result: CUE porcuepine <[email protected]>
Reviewed-by: Daniel Martí <[email protected]>

Author: NoamTD

cmd/cue/cmd/fmt.go

@@ -85,19 +85,19 @@ func newFmtCmd(c *Command) *cobra.Command {
 						continue
 					}
 
-					// When using --check and --diff, we need to buffer the input and output bytes to compare them.
+					// We buffer the input and output bytes to compare them.
+					// This allows us to determine whether a file is already
+					// formatted, without modifying the file.
 					var original []byte
 					var formatted bytes.Buffer
-					if doDiff || check {
-						if bs, ok := file.Source.([]byte); ok {
-							original = bs
-						} else {
-							original, err = source.ReadAll(file.Filename, file.Source)
-							exitOnErr(cmd, err, true)
-							file.Source = original
-						}
-						cfg.Out = &formatted
+					if bs, ok := file.Source.([]byte); ok {
+						original = bs
+					} else {
+						original, err = source.ReadAll(file.Filename, file.Source)
+						exitOnErr(cmd, err, true)
+						file.Source = original
 					}
+					cfg.Out = &formatted
 
 					var files []*ast.File
 					d := encoding.NewDecoder(cmd.ctx, file, &cfg)
@@ -130,20 +130,32 @@ func newFmtCmd(c *Command) *cobra.Command {
 						exitOnErr(cmd, err, true)
 					}
 
-					if (doDiff || check) && !bytes.Equal(formatted.Bytes(), original) {
-						foundBadlyFormatted = true
-						var path string
-						f := file.Filename
-						path, err = filepath.Rel(cwd, f)
-						if err != nil {
-							path = f
-						}
+					// File is already well formatted; we can stop here.
+					if bytes.Equal(formatted.Bytes(), original) {
+						continue
+					}
 
-						if doDiff {
-							d := diff.Diff(path+".orig", original, path, formatted.Bytes())
-							fmt.Fprintln(stdout, string(d))
-						} else {
-							fmt.Fprintln(stdout, path)
+					foundBadlyFormatted = true
+					var path string
+					f := file.Filename
+					path, err = filepath.Rel(cwd, f)
+					if err != nil {
+						path = f
+					}
+
+					switch {
+					case doDiff:
+						d := diff.Diff(path+".orig", original, path, formatted.Bytes())
+						fmt.Fprintln(stdout, string(d))
+					case check:
+						fmt.Fprintln(stdout, path)
+					case file.Filename == "-":
+						if _, err := fmt.Fprint(stdout, formatted.String()); err != nil {
+							exitOnErr(cmd, err, false)
+						}
+					default:
+						if err := os.WriteFile(file.Filename, formatted.Bytes(), 0644); err != nil {
+							exitOnErr(cmd, err, false)
 						}
 					}
 				}

cmd/cue/cmd/script_test.go

@@ -116,6 +116,18 @@ func TestScript(t *testing.T) {
 					ts.Check(os.WriteFile(path, []byte(data), 0o666))
 				}
 			},
+			// mod-time prints the modification time of a file to stdout.
+			// The time is displayed as nanoseconds since the Unix epoch.
+			"mod-time": func(ts *testscript.TestScript, neg bool, args []string) {
+				if neg || len(args) != 1 {
+					ts.Fatalf("usage: mod-time PATH")
+				}
+				path := ts.MkAbs(args[0])
+				fi, err := os.Stat(path)
+				ts.Check(err)
+				_, err = fmt.Fprint(ts.Stdout(), fi.ModTime().UnixNano())
+				ts.Check(err)
+			},
 			// get-manifest writes the manifest for a given reference within an OCI
 			// registry to a file in JSON format.
 			"get-manifest": func(ts *testscript.TestScript, neg bool, args []string) {

cmd/cue/cmd/testdata/script/fmt.txtar

@@ -1,5 +1,13 @@
 ! exec cue fmt ./fmt
 cmp stderr expect-stderr
+
+# Issue 1744: if we encounter an error while formatting,
+# don't overwrite the input file with zero bytes.
+cp issue1744/invalid.cue issue1744/invalid.cue.orig
+! exec cue fmt issue1744/invalid.cue
+cmp stderr issue1744/stderr-golden
+cmp issue1744/invalid.cue issue1744/invalid.cue.orig
+
 -- expect-stderr --
 expected 'STRING', found '.':
     ./fmt/error.cue:1:9
@@ -9,3 +17,8 @@ import a.b "foo"
 a: 2
 bb: 3
 -- fmt/cue.mod/module.cue --
+-- issue1744/invalid.cue --
+~
+-- issue1744/stderr-golden --
+illegal character U+007E '~':
+    ./issue1744/invalid.cue:1:1

cmd/cue/cmd/testdata/script/fmt_unchanged.txtar

@@ -0,0 +1,10 @@
+# Verify that cue fmt does not write to files
+# if they are already well formatted
+mod-time formatted.cue
+cp stdout mod-time.txt
+exec cue fmt formatted.cue
+mod-time formatted.cue
+cmp stdout mod-time.txt
+
+-- formatted.cue --
+a: 1