Skip to content

Commit

Permalink
cli: fmt -check should return early on diff
Browse files Browse the repository at this point in the history
The `nomad fmt -check` command incorrectly writes to file because we didn't
return before writing the file on a diff. Fix this bug and update the command
internals to differentiate between the write-to-file and write-to-stdout code
paths, which are activated by different combinations of options and flags.

The docstring for the `-list` and `-write` flags is also unclear and can be
easily misread to be the opposite of the actual behavior. Clarify this and fix
up the docs to match.

This changeset also refactors the tests quite a bit so as to make the test
outputs clear when something is incorrect.
  • Loading branch information
tgross committed Feb 14, 2023
1 parent 1154c05 commit d9888d5
Show file tree
Hide file tree
Showing 4 changed files with 204 additions and 138 deletions.
3 changes: 3 additions & 0 deletions .changelog/16174.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
cli: Fixed a bug where `nomad fmt -check` would overwrite the file being checked
```
59 changes: 36 additions & 23 deletions command/fmt.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ type FormatCommand struct {
check bool
checkSuccess bool
recursive bool
write bool
writeFile bool
writeStdout bool
paths []string

stdin io.Reader
Expand All @@ -47,27 +48,31 @@ func (*FormatCommand) Help() string {
helpText := `
Usage: nomad fmt [flags] paths ...
Formats Nomad agent configuration and job file to a canonical format.
If a path is a directory, it will recursively format all files
with .nomad and .hcl extensions in the directory.
If you provide a single dash (-) as argument, fmt will read from standard
input (STDIN) and output the processed output to standard output (STDOUT).
Formats Nomad agent configuration and job file to a canonical format.
If a path is a directory, it will recursively format all files
with .nomad and .hcl extensions in the directory.
If you provide a single dash (-) as argument, fmt will read from standard
input (STDIN) and output the processed output to standard output (STDOUT).
Format Options:
-list=false
Don't list the files, which contain formatting inconsistencies.
-check
Check if the files are valid HCL files. If not, exit status of the
command will be 1 and the incorrect files will not be formatted. This
flag overrides any -write flag value.
-check
Check if the files are valid HCL files. If not, exit status of the command
will be 1 and the incorrect files will not be formatted.
-list
List the files which contain formatting inconsistencies. Defaults
to -list=true.
-write=false
Don't overwrite the input files.
-recursive
Process files in subdirectories. By default only the given (or current)
directory is processed.
-recursive
Process also files in subdirectories. By default only the given (or current) directory is processed.
-write
Overwrite the input files. Defaults to -write=true. Ignored if the input
comes from STDIN.
`

return strings.TrimSpace(helpText)
Expand Down Expand Up @@ -100,7 +105,7 @@ func (f *FormatCommand) Run(args []string) int {
flags := f.Meta.FlagSet(f.Name(), FlagSetClient)
flags.Usage = func() { f.Ui.Output(f.Help()) }
flags.BoolVar(&f.check, "check", false, "")
flags.BoolVar(&f.write, "write", true, "")
flags.BoolVar(&f.writeFile, "write", true, "")
flags.BoolVar(&f.list, "list", true, "")
flags.BoolVar(&f.recursive, "recursive", false, "")

Expand All @@ -121,11 +126,18 @@ func (f *FormatCommand) Run(args []string) int {
if len(flags.Args()) == 0 {
f.paths = []string{"."}
} else if flags.Args()[0] == stdinArg {
f.write = false
f.writeStdout = true
f.writeFile = false
f.list = false
} else {
f.paths = flags.Args()
}
if f.check {
f.writeFile = false
}
if !f.list && !f.writeFile {
f.writeStdout = true
}

f.fmt()

Expand Down Expand Up @@ -251,19 +263,20 @@ func (f *FormatCommand) processFile(path string, r io.Reader) {
f.Ui.Output(path)
}

if f.write {
if f.check {
f.checkSuccess = false
}

if f.writeFile {
if err := os.WriteFile(path, out, 0644); err != nil {
f.appendError(fmt.Errorf("Failed to write file %s: %w", path, err))
return
}
}

if f.check {
f.checkSuccess = false
}
}

if !f.list && !f.write {
if f.writeStdout {
f.Ui.Output(string(out))
}
}
Expand Down
Loading

0 comments on commit d9888d5

Please sign in to comment.