Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add --stdin flag to 'buf format' #1345

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
`2m` (the default for all the other `buf` commands).
- Add support for experimental code generation with the `plugin:` key in `buf.gen.yaml`.
- Preserve single quotes with `buf format`.
- Add `--stdin` flag to `buf format` to format the content read from stdin.

## [v1.7.0] - 2022-06-27

Expand Down
14 changes: 14 additions & 0 deletions private/buf/bufformat/bufformat.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package bufformat

import (
"bytes"
"context"

"github.com/bufbuild/buf/private/bufpkg/bufmodule"
Expand Down Expand Up @@ -66,3 +67,16 @@ func Format(ctx context.Context, module bufmodule.Module) (_ storage.ReadBucket,
}
return readWriteBucket, nil
}

// FormatRaw formats the given raw bytes and returns the result.
func FormatRaw(ctx context.Context, raw []byte) ([]byte, error) {
fileNode, err := parser.Parse("<proto>", bytes.NewReader(raw), reporter.NewHandler(nil))
if err != nil {
return nil, err
}
buffer := bytes.NewBuffer(nil)
if err := newFormatter(buffer, fileNode).Run(); err != nil {
return nil, err
}
return buffer.Bytes(), nil
}
111 changes: 111 additions & 0 deletions private/buf/cmd/buf/buf_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1987,6 +1987,95 @@ func TestFormatExitCode(t *testing.T) {
assert.NotEmpty(t, stdout.String())
}

func TestFormatStdin(t *testing.T) {
stdin := bytes.NewBuffer(
[]byte(`
syntax = "proto3";


package foo;
`),
)
stdout := bytes.NewBuffer(nil)
testRun(
t,
0,
stdin,
stdout,
"format",
"--stdin",
)
assert.Equal(
t,
`syntax = "proto3";

package foo;
`,
stdout.String(),
)

stdin = bytes.NewBuffer(
[]byte(`
syntax = "proto3";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a test for multiple files concat'ed together, and either expect success or expect error? If error, document that --stdin can only take one file



package foo;
`),
)
stdout = bytes.NewBuffer(nil)
testRun(
t,
0,
stdin,
stdout,
"format",
"--stdin",
"--diff",
)
assert.NotEmpty(t, stdout.String())

stdin = bytes.NewBuffer(
[]byte(`
syntax = "proto3";


package foo;
`),
)
stdout = bytes.NewBuffer(nil)
testRun(
t,
bufcli.ExitCodeFileAnnotation,
stdin,
stdout,
"format",
"--stdin",
"--exit-code",
)
assert.NotEmpty(t, stdout.String())

stdin = bytes.NewBuffer(
[]byte(`
syntax = "proto3";


package foo;
`),
)
stdout = bytes.NewBuffer(nil)
testRun(
t,
bufcli.ExitCodeFileAnnotation,
stdin,
stdout,
"format",
"--stdin",
"--diff",
"--exit-code",
)
assert.NotEmpty(t, stdout.String())
}

// Tests if the image produced by the formatted result is
// equivalent to the original result.
func TestFormatEquivalence(t *testing.T) {
Expand Down Expand Up @@ -2044,6 +2133,28 @@ func TestFormatInvalidFlagCombination(t *testing.T) {
"-o",
filepath.Join(tempDir, "formatted"),
)
testRunStdoutStderr(
t,
nil,
1,
"",
`Failure: --stdin cannot be used with --write`,
"format",
"--stdin",
"--write",
filepath.Join(tempDir, "formatted"),
)
testRunStdoutStderr(
t,
nil,
1,
"",
`Failure: --stdin cannot be used with --output`,
"format",
"--stdin",
"--output",
filepath.Join(tempDir, "formatted"),
)
}

func TestFormatInvalidWriteWithModuleReference(t *testing.T) {
Expand Down
99 changes: 80 additions & 19 deletions private/buf/cmd/buf/command/format/format.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"github.com/bufbuild/buf/private/pkg/app/appcmd"
"github.com/bufbuild/buf/private/pkg/app/appflag"
"github.com/bufbuild/buf/private/pkg/command"
"github.com/bufbuild/buf/private/pkg/diff"
"github.com/bufbuild/buf/private/pkg/storage"
"github.com/bufbuild/buf/private/pkg/storage/storagemem"
"github.com/bufbuild/buf/private/pkg/storage/storageos"
Expand All @@ -52,6 +53,7 @@ const (
outputFlagName = "output"
outputFlagShortName = "o"
pathsFlagName = "path"
stdinFlagName = "stdin"
writeFlagName = "write"
writeFlagShortName = "w"
)
Expand Down Expand Up @@ -168,8 +170,9 @@ type flags struct {
ErrorFormat string
ExcludePaths []string
ExitCode bool
Paths []string
Output string
Paths []string
Stdin bool
Write bool
// special
InputHashtag string
Expand All @@ -184,26 +187,19 @@ func (f *flags) Bind(flagSet *pflag.FlagSet) {
bufcli.BindPaths(flagSet, &f.Paths, pathsFlagName)
bufcli.BindExcludePaths(flagSet, &f.ExcludePaths, excludePathsFlagName)
bufcli.BindDisableSymlinks(flagSet, &f.DisableSymlinks, disableSymlinksFlagName)
flagSet.StringVar(
&f.Config,
configFlagName,
"",
`The file or data to use for configuration.`,
)
flagSet.BoolVarP(
&f.Diff,
diffFlagName,
diffFlagShortName,
false,
"Display diffs instead of rewriting files.",
)
flagSet.BoolVar(
&f.ExitCode,
exitCodeFlagName,
false,
"Exit with a non-zero exit code if files were not already formatted.",
)
flagSet.BoolVarP(
&f.Write,
writeFlagName,
writeFlagShortName,
false,
"Rewrite files in-place.",
)
flagSet.StringVar(
&f.ErrorFormat,
errorFormatFlagName,
Expand All @@ -213,6 +209,12 @@ func (f *flags) Bind(flagSet *pflag.FlagSet) {
stringutil.SliceToString(bufanalysis.AllFormatStrings),
),
)
flagSet.BoolVar(
&f.ExitCode,
exitCodeFlagName,
false,
"Exit with a non-zero exit code if files were not already formatted.",
)
flagSet.StringVarP(
&f.Output,
outputFlagName,
Expand All @@ -223,11 +225,19 @@ func (f *flags) Bind(flagSet *pflag.FlagSet) {
buffetch.SourceFormatsString,
),
)
flagSet.StringVar(
&f.Config,
configFlagName,
flagSet.BoolVarP(
&f.Stdin,
stdinFlagName,
"",
`The file or data to use for configuration.`,
false,
"Read and format the input .proto file content from stdin.",
)
flagSet.BoolVarP(
&f.Write,
writeFlagName,
writeFlagShortName,
false,
"Rewrite files in-place.",
)
}

Expand All @@ -242,10 +252,62 @@ func run(
if flags.Output != "-" && flags.Write {
return fmt.Errorf("--%s cannot be used with --%s", outputFlagName, writeFlagName)
}
if flags.Stdin && flags.Output != "-" {
return fmt.Errorf("--%s cannot be used with --%s", stdinFlagName, outputFlagName)
}
if flags.Stdin && flags.Write {
return fmt.Errorf("--%s cannot be used with --%s", stdinFlagName, writeFlagName)
}
input, err := bufcli.GetInputValue(container, flags.InputHashtag, ".")
if err != nil {
return err
}
if flags.Stdin && container.NumArgs() > 0 {
return fmt.Errorf("--%s cannot be used with a non-empty input %s", stdinFlagName, input)
}
runner := command.NewRunner()
if flags.Stdin {
// We need to read the raw bytes from the io.Reader upfront so that
// the content can be referenced more than once: once by the formatter,
// and again during the diff.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I smell something here. --stdin is implemented with a divergent different code path because its input is a non-seekable file. Standard input is not the only non-seekable file out there: sockets, tape-archive on literal tape, named pipes, bash's named descriptors (/dev/fd/), etc.

One alternative is to implement a buffered, seeking reader. Wrap stdin with your reader and no more special code paths, I hope. I haven't groked all of the format package yet and run is particularly hard to follow.

This concept can extend further if you open a file input and attempt to seek to offset 0. That should, on POSIX systems, fail on pipes, sockets, and other file handles that are non-seekable. Commands like buf format <(cat foo.proto bar.proto) become possible.

rawBytes, err := io.ReadAll(container.Stdin())
if err != nil {
return err
}
formattedBytes, err := bufformat.FormatRaw(ctx, rawBytes)
if err != nil {
return err
}
diffBytes, err := diff.Diff(
ctx,
runner,
rawBytes,
formattedBytes,
"<original>",
"<formatted>",
)
if err != nil {
return err
}
if flags.Diff {
if len(diffBytes) > 0 {
if _, err := container.Stdout().Write(diffBytes); err != nil {
return err
}
if flags.ExitCode {
return bufcli.ErrFileAnnotation
}
}
return nil
}
if _, err := container.Stdout().Write(formattedBytes); err != nil {
return err
}
if len(diffBytes) > 0 && flags.ExitCode {
return bufcli.ErrFileAnnotation
}
return nil
}
refParser := buffetch.NewRefParser(
container.Logger(),
buffetch.RefParserWithProtoFileRefAllowed(),
Expand All @@ -265,7 +327,6 @@ func run(
if err != nil {
return err
}
runner := command.NewRunner()
storageosProvider := bufcli.NewStorageosProvider(flags.DisableSymlinks)
moduleConfigReader, err := bufcli.NewWireModuleConfigReaderForModuleReader(
container,
Expand Down