yamlfmt: added and implemented functional features (#43)

* yamlfmt: added and implemented functional features

While adding functionality for small edge cases or formatting
differences, I found that the Formatter code was becoming harder to
reason about. It would grow linearly with the number of little
formatting things I had to do. I have adjusted the pattern to instead be
a collection of functional features that can be added at-will.

Created a new `Feature` type that has a Before and After action. Also
added the FeatureList type, with code to apply the before or after
actions in order on byte slice input.

I converted all existing candidates in the formatter code to use this
functional feature pattern instead.

* hotfix: make hotfix package create feature

* yamlfmt: wrap feature apply errors

This wraps feature apply errors with the name and type of action.

* basic: change feature configuration pattern

Instead of a receiver method that configures the features in place on
the formatter, make it so the features are returned from a function that
reads the config. Avoids deliberate mutability.

formatters/basic/README.md

@@ -10,3 +10,4 @@ The basic formatter is a barebones formatter that simply takes the data provided
 | `include_document_start` | bool           | false   | Include `---` at document start |
 | `line_ending`            | `lf` or `crlf` | `crlf` on Windows, `lf` otherwise | Parse and write the file with "lf" or "crlf" line endings |
 | `emoji_support`          | bool           | false   | Support encoding utf-8 emojis |
+| `retain_line_breaks`     | bool           | false   | Retain line breaks in formatted yaml |

formatters/basic/config.go

@@ -25,6 +25,7 @@ type Config struct {
 	IncludeDocumentStart bool   `mapstructure:"include_document_start"`
 	EmojiSupport         bool   `mapstructure:"emoji_support"`
 	LineEnding           string `mapstructure:"line_ending"`
+	RetainLineBreaks     bool   `mapstructure:"retain_line_breaks"`
 }
 
 func DefaultConfig() *Config {

formatters/basic/factory.go

@@ -26,7 +26,7 @@ func (f *BasicFormatterFactory) Type() string {
 }
 
 func (f *BasicFormatterFactory) NewDefault() yamlfmt.Formatter {
-	return &BasicFormatter{Config: DefaultConfig()}
+	return newFormatter(DefaultConfig())
 }
 
 func (f *BasicFormatterFactory) NewWithConfig(configData map[string]interface{}) (yamlfmt.Formatter, error) {
@@ -35,5 +35,12 @@ func (f *BasicFormatterFactory) NewWithConfig(configData map[string]interface{})
 	if err != nil {
 		return nil, err
 	}
-	return &BasicFormatter{Config: config}, nil
+	return newFormatter(config), nil
+}
+
+func newFormatter(config *Config) yamlfmt.Formatter {
+	return &BasicFormatter{
+		Config:   config,
+		Features: ConfigureFeaturesFromConfig(config),
+	}
 }

formatters/basic/features.go

@@ -0,0 +1,61 @@
+// Copyright 2022 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package basic
+
+import (
+	"github.com/google/yamlfmt"
+	"github.com/google/yamlfmt/internal/hotfix"
+)
+
+var (
+	featIncludeDocumentStart = yamlfmt.Feature{
+		Name: "Include Document Start",
+		AfterAction: func(content []byte) ([]byte, error) {
+			documentStart := "---\n"
+			return append([]byte(documentStart), content...), nil
+		},
+	}
+	featEmojiSupport = yamlfmt.Feature{
+		Name:        "Emoji Support",
+		AfterAction: hotfix.ParseUnicodePoints,
+	}
+	featCRLFSupport = yamlfmt.Feature{
+		Name:         "CRLF Support",
+		BeforeAction: hotfix.StripCRBytes,
+		AfterAction:  hotfix.WriteCRLFBytes,
+	}
+)
+
+func ConfigureFeaturesFromConfig(config *Config) yamlfmt.FeatureList {
+	features := []yamlfmt.Feature{}
+	if config.EmojiSupport {
+		features = append(features, featEmojiSupport)
+	}
+	if config.IncludeDocumentStart {
+		features = append(features, featIncludeDocumentStart)
+	}
+	if config.LineEnding == yamlfmt.LineBreakStyleCRLF {
+		features = append(features, featCRLFSupport)
+	}
+	if config.RetainLineBreaks {
+		linebreakStr := "\n"
+		if config.LineEnding == yamlfmt.LineBreakStyleCRLF {
+			linebreakStr = "\r\n"
+		}
+		featLineBreak := hotfix.MakeFeatureRetainLineBreak(linebreakStr, config.Indent)
+		features = append(features, featLineBreak)
+	}
+	return features
+}

formatters/basic/formatter.go

@@ -20,48 +20,32 @@ import (
 	"io"
 
 	"github.com/google/yamlfmt"
-	"github.com/google/yamlfmt/internal/hotfix"
 	"gopkg.in/yaml.v3"
 )
 
 const BasicFormatterType string = "basic"
 
 type BasicFormatter struct {
-	Config *Config
+	Config   *Config
+	Features yamlfmt.FeatureList
 }
 
+// yamlfmt.Formatter interface
+
 func (f *BasicFormatter) Type() string {
 	return BasicFormatterType
 }
 
-func (f *BasicFormatter) Format(yamlContent []byte) ([]byte, error) {
-	var reader *bytes.Reader
-	if f.Config.LineEnding == yamlfmt.LineBreakStyleCRLF {
-		crStrippedContent := hotfix.StripCRBytes(yamlContent)
-		reader = bytes.NewReader(crStrippedContent)
-	} else {
-		reader = bytes.NewReader(yamlContent)
-	}
-
-	encodedContent, err := retainLineBreaks(reader, f.format)
+func (f *BasicFormatter) Format(input []byte) ([]byte, error) {
+	// Run all featurres with BeforeActions
+	yamlContent, err := f.Features.ApplyFeatures(input, yamlfmt.FeatureApplyBefore)
 	if err != nil {
 		return nil, err
 	}
 
-	if f.Config.IncludeDocumentStart {
-		encodedContent = withDocumentStart(encodedContent)
-	}
-	if f.Config.EmojiSupport {
-		encodedContent = hotfix.ParseUnicodePoints(encodedContent)
-	}
-	if f.Config.LineEnding == yamlfmt.LineBreakStyleCRLF {
-		encodedContent = hotfix.WriteCRLFBytes(encodedContent)
-	}
-	return encodedContent, nil
-}
-
-func (f *BasicFormatter) format(in io.Reader) (io.Reader, error) {
-	decoder := yaml.NewDecoder(in)
+	// Format the yaml content
+	reader := bytes.NewReader(yamlContent)
+	decoder := yaml.NewDecoder(reader)
 	documents := []yaml.Node{}
 	for {
 		var docNode yaml.Node
@@ -84,10 +68,12 @@ func (f *BasicFormatter) format(in io.Reader) (io.Reader, error) {
 			return nil, err
 		}
 	}
-	return &b, nil
-}
 
-func withDocumentStart(document []byte) []byte {
-	documentStart := "---\n"
-	return append([]byte(documentStart), document...)
+	// Run all features with AfterActions
+	resultYaml, err := f.Features.ApplyFeatures(b.Bytes(), yamlfmt.FeatureApplyAfter)
+	if err != nil {
+		return nil, err
+	}
+
+	return resultYaml, nil
 }

formatters/basic/formatter_test.go

@@ -22,8 +22,15 @@ import (
 	"github.com/google/yamlfmt/formatters/basic"
 )
 
+func newFormatter(config *basic.Config) *basic.BasicFormatter {
+	return &basic.BasicFormatter{
+		Config:   config,
+		Features: basic.ConfigureFeaturesFromConfig(config),
+	}
+}
+
 func TestFormatterRetainsComments(t *testing.T) {
-	f := &basic.BasicFormatter{Config: basic.DefaultConfig()}
+	f := newFormatter(basic.DefaultConfig())
 
 	yaml := `x: "y" # foo comment`
 
@@ -72,8 +79,9 @@ a:
 }
 
 func TestWithDocumentStart(t *testing.T) {
-	f := &basic.BasicFormatter{Config: basic.DefaultConfig()}
-	f.Config.IncludeDocumentStart = true
+	config := basic.DefaultConfig()
+	config.IncludeDocumentStart = true
+	f := newFormatter(config)
 
 	yaml := "a:"
 	s, err := f.Format([]byte(yaml))
@@ -86,8 +94,9 @@ func TestWithDocumentStart(t *testing.T) {
 }
 
 func TestCRLFLineEnding(t *testing.T) {
-	f := &basic.BasicFormatter{Config: basic.DefaultConfig()}
-	f.Config.LineEnding = yamlfmt.LineBreakStyleCRLF
+	config := basic.DefaultConfig()
+	config.LineEnding = yamlfmt.LineBreakStyleCRLF
+	f := newFormatter(config)
 
 	yaml := "# comment\r\na:\r\n"
 	result, err := f.Format([]byte(yaml))
@@ -100,8 +109,9 @@ func TestCRLFLineEnding(t *testing.T) {
 }
 
 func TestEmojiSupport(t *testing.T) {
-	f := &basic.BasicFormatter{Config: basic.DefaultConfig()}
-	f.Config.EmojiSupport = true
+	config := basic.DefaultConfig()
+	config.EmojiSupport = true
+	f := newFormatter(config)
 
 	yaml := "a: 😊"
 	result, err := f.Format([]byte(yaml))
@@ -166,15 +176,17 @@ shell: |
 `,
 		},
 	}
-	f := &basic.BasicFormatter{Config: basic.DefaultConfig()}
+	config := basic.DefaultConfig()
+	config.RetainLineBreaks = true
+	f := newFormatter(config)
 	for _, c := range testCases {
 		t.Run(c.desc, func(t *testing.T) {
 			got, err := f.Format([]byte(c.input))
 			if err != nil {
 				t.Fatalf("expected formatting to pass, returned error: %v", err)
 			}
 			if string(got) != c.expect {
-				t.Fatalf("didn't retain line breaks result: %v, expect %s", string(got), c.expect)
+				t.Fatalf("didn't retain line breaks\nresult: %v\nexpect %s", string(got), c.expect)
 			}
 		})
 	}

internal/hotfix/crlf.go

@@ -1,22 +1,38 @@
+// Copyright 2022 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
 package hotfix
 
-func StripCRBytes(crlfContent []byte) []byte {
+// yamlfmt.FeatureFunc
+func StripCRBytes(crlfContent []byte) ([]byte, error) {
 	onlyLf := []byte{}
 	for _, b := range crlfContent {
 		if b != '\r' {
 			onlyLf = append(onlyLf, b)
 		}
 	}
-	return onlyLf
+	return onlyLf, nil
 }
 
-func WriteCRLFBytes(lfContent []byte) []byte {
+// yamlfmt.FeatureFunc
+func WriteCRLFBytes(lfContent []byte) ([]byte, error) {
 	crlfContent := []byte{}
 	for _, b := range lfContent {
 		if b == '\n' {
 			crlfContent = append(crlfContent, '\r')
 		}
 		crlfContent = append(crlfContent, b)
 	}
-	return crlfContent
+	return crlfContent, nil
 }