Skip to content

Commit

Permalink
parser: drop hard dependency on glog
Browse files Browse the repository at this point in the history 
All calls go through a logger in the config.

The logger may be nil, so we check that it's non-nil everywhere.

We also keep the behavior in cmd/txtpbfmt unchanged.
Calls to Errorf still end up going to glog.Errorf,
and calls to Infof still end up going to glog.V(2).Infof.

Note that the library used the boolean value of glog.V(2) to tell
whether the "info" log level was enabled or not.
When disabled, this allowed us to not compute some log arguments,
such as calls to strings.Join or string copies.
Since we can't assume an interface to be of boolean type,
add an extra InfoLevel method to perform the same duty.

Fixes #70.
  • Loading branch information
@mvdan
mvdan committed Jan 7, 2023
1 parent 762d86c commit d8aa537
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 32 deletions.
14 changes: 14 additions & 0 deletions cmd/txtpbfmt/fmt.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,19 @@ func contentForLogging(content []byte) string {
return res
}

// parserLog satisfies parser.Logger. Note that glog only exposes Errorf as a
// top-level function, and not as a method.
type parserLog struct {
log.Verbose
}

func (l parserLog) Errorf(format string, args ...interface{}) {
log.Errorf(format, args...)
}
func (l parserLog) InfoLevel() bool {
return bool(l.Verbose)
}

func main() {
flag.Parse()
paths := flag.Args()
Expand All @@ -72,6 +85,7 @@ func main() {
ExpandAllChildren: *expandAllChildren,
SkipAllColons: *skipAllColons,
AllowTripleQuotedStrings: *allowTripleQuotedStrings,
Logger: parserLog{log.V(2)},
})
if err != nil {
errorf("parser.Format for path %v with content %q returned err %v", path, contentForLogging(content), err)
Expand Down
1 change: 1 addition & 0 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,5 @@ github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0
github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw=
github.com/mitchellh/go-wordwrap v1.0.1 h1:TLuKupo69TCn6TQSyGxwI1EblZZEsQ0vMlAFQflz0v0=
github.com/mitchellh/go-wordwrap v1.0.1/go.mod h1:R62XHJLzvMFRBbcrT7m7WgmE1eOyTSsCt+hzestvNj0=
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4=
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
91 changes: 59 additions & 32 deletions parser/parser.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (
"strconv"
"strings"

log "github.com/golang/glog"
"github.com/mitchellh/go-wordwrap"
"github.com/protocolbuffers/txtpbfmt/ast"
"github.com/protocolbuffers/txtpbfmt/unquote"
Expand Down Expand Up @@ -78,6 +77,36 @@ type Config struct {

// Use single quotes around strings that contain double but not single quotes.
SmartQuotes bool

// Logger enables logging when it is non-nil.
Logger Logger
}

func (c *Config) errorf(format string, args ...interface{}) {
if c.Logger != nil {
c.Logger.Errorf(format, args...)
}
}
func (c *Config) infof(format string, args ...interface{}) {
if c.Logger != nil {
c.Logger.Infof(format, args...)
}
}
func (c *Config) infoLevel() bool {
return c.Logger != nil && c.Logger.InfoLevel()
}

// Logger is a small glog-like interface.
type Logger interface {
// Infof is used for informative messages, for testing or debugging.
Infof(format string, args ...interface{})

// Errorf is used for non-fatal errors, such as invalid meta comments.
Errorf(format string, args ...interface{})

// InfoLevel reports whether Infof messages will be logged.
// When it returns false, the caller can avoid constructing its log messages.
InfoLevel() bool
}

// RootName contains a constant that can be used to identify the root of all Nodes.
Expand Down Expand Up @@ -119,7 +148,6 @@ type parser struct {
in []byte
index int
length int
log log.Verbose
// Maps the index of '{' characters on 'in' that have the matching '}' on
// the same line to 'true'.
bracketSameLine map[int]bool
Expand Down Expand Up @@ -163,7 +191,7 @@ func Format(in []byte) ([]byte, error) {
func FormatWithConfig(in []byte, c Config) ([]byte, error) {
addMetaCommentsToConfig(in, &c)
if c.Disable {
log.Infoln("Ignored file with 'disable' comment.")
c.infof("Ignored file with 'disable' comment.")
return in, nil
}
nodes, err := parseWithMetaCommentConfig(in, c)
Expand Down Expand Up @@ -322,9 +350,9 @@ func parseWithMetaCommentConfig(in []byte, c Config) ([]*ast.Node, error) {
if err != nil {
return nil, err
}
if p.log {
p.log.Infof("p.in: %q", string(p.in))
p.log.Infof("p.length: %v", p.length)
if p.config.infoLevel() {
p.config.infof("p.in: %q", string(p.in))
p.config.infof("p.length: %v", p.length)
}
// Although unnamed nodes aren't strictly allowed, some formats represent a
// list of protos as a list of unnamed top-level nodes.
Expand Down Expand Up @@ -375,21 +403,21 @@ func addMetaCommentsToConfig(in []byte, c *Config) {
if metaComments["sort_repeated_fields_by_content"] {
c.SortRepeatedFieldsByContent = true
}
for _, sf := range getMetaCommentStringValues("sort_repeated_fields_by_subfield", metaComments) {
for _, sf := range getMetaCommentStringValues(c, "sort_repeated_fields_by_subfield", metaComments) {
c.SortRepeatedFieldsBySubfield = append(c.SortRepeatedFieldsBySubfield, sf)
}
if metaComments["wrap_html_strings"] {
c.WrapHTMLStrings = true
}
setMetaCommentIntValue("wrap_strings_at_column", metaComments, &c.WrapStringsAtColumn)
setMetaCommentIntValue(c, "wrap_strings_at_column", metaComments, &c.WrapStringsAtColumn)
}

// For a MetaComment in the form comment_name=<int> set *int to the value. Will not change
// the *int if the comment name isn't present.
func setMetaCommentIntValue(name string, metaComments map[string]bool, value *int) {
func setMetaCommentIntValue(c *Config, name string, metaComments map[string]bool, value *int) {
for mc := range metaComments {
if strings.HasPrefix(mc, fmt.Sprintf("%s ", name)) {
log.Errorf("format should be %s=<int>, got: %s", name, mc)
c.errorf("format should be %s=<int>, got: %s", name, mc)
return
}
prefix := fmt.Sprintf("%s=", name)
Expand All @@ -398,7 +426,7 @@ func setMetaCommentIntValue(name string, metaComments map[string]bool, value *in
var err error
i, err := strconv.Atoi(strings.TrimSpace(intStr))
if err != nil {
log.Errorf("error parsing %s value %q (skipping): %v", name, intStr, err)
c.errorf("error parsing %s value %q (skipping): %v", name, intStr, err)
return
}
*value = i
Expand All @@ -407,11 +435,11 @@ func setMetaCommentIntValue(name string, metaComments map[string]bool, value *in
}

// For a MetaComment in the form comment_name=<string> returns the strings.
func getMetaCommentStringValues(name string, metaComments map[string]bool) []string {
func getMetaCommentStringValues(c *Config, name string, metaComments map[string]bool) []string {
var vs []string
for mc := range metaComments {
if strings.HasPrefix(mc, fmt.Sprintf("%s ", name)) {
log.Errorf("format should be %s=<string>, got: %s", name, mc)
c.errorf("format should be %s=<string>, got: %s", name, mc)
continue
}
prefix := fmt.Sprintf("%s=", name)
Expand Down Expand Up @@ -439,7 +467,6 @@ func newParser(in []byte, c Config) (*parser, error) {
in: in,
index: 0,
length: len(in),
log: log.V(2),
bracketSameLine: bracketSameLine,
config: c,
line: 1,
Expand Down Expand Up @@ -572,8 +599,8 @@ func (p *parser) parse(isRoot bool) (result []*ast.Node, endPos ast.Position, er

// Handle blank lines.
if blankLines > 1 {
if p.log {
p.log.Infof("blankLines: %v", blankLines)
if p.config.infoLevel() {
p.config.infof("blankLines: %v", blankLines)
}
comments = append([]string{""}, comments...)
}
Expand Down Expand Up @@ -610,8 +637,8 @@ func (p *parser) parse(isRoot bool) (result []*ast.Node, endPos ast.Position, er
Start: startPos,
PreComments: comments,
}
if p.log {
p.log.Infof("PreComments: %q", strings.Join(nd.PreComments, "\n"))
if p.config.infoLevel() {
p.config.infof("PreComments: %q", strings.Join(nd.PreComments, "\n"))
}

// Skip white-space other than '\n', which is handled below.
Expand Down Expand Up @@ -653,8 +680,8 @@ func (p *parser) parse(isRoot bool) (result []*ast.Node, endPos ast.Position, er
return nil, ast.Position{}, fmt.Errorf("Failed to find a FieldName at %s", p.errorContext())
}
}
if p.log {
p.log.Infof("name: %q", nd.Name)
if p.config.infoLevel() {
p.config.infof("name: %q", nd.Name)
}
// Skip separator.
_, _ = p.skipWhiteSpaceAndReadComments(true /* multiLine */)
Expand Down Expand Up @@ -756,8 +783,8 @@ func (p *parser) parse(isRoot bool) (result []*ast.Node, endPos ast.Position, er
return nil, ast.Position{}, err
}
}
if p.log && p.index < p.length {
p.log.Infof("p.in[p.index]: %q", string(p.in[p.index]))
if p.config.infoLevel() && p.index < p.length {
p.config.infof("p.in[p.index]: %q", string(p.in[p.index]))
}
res = append(res, nd)
}
Expand Down Expand Up @@ -839,10 +866,10 @@ func (p *parser) skipWhiteSpaceAndReadComments(multiLine bool) ([]string, int) {
}
}
sep := p.advance(i)
if p.log {
p.log.Infof("sep: %q\np.index: %v", string(sep), p.index)
if p.config.infoLevel() {
p.config.infof("sep: %q\np.index: %v", string(sep), p.index)
if p.index < p.length {
p.log.Infof("p.in[p.index]: %q", string(p.in[p.index]))
p.config.infof("p.in[p.index]: %q", string(p.in[p.index]))
}
}
return comments, blankLines
Expand Down Expand Up @@ -940,8 +967,8 @@ func (p *parser) readValues() ([]*ast.Value, error) {
vl := p.advance(i)
values = append(values, p.populateValue(vl, nil))
}
if p.log {
p.log.Infof("values: %v", values)
if p.config.infoLevel() {
p.config.infof("values: %v", values)
}
return values, nil
}
Expand Down Expand Up @@ -974,8 +1001,8 @@ func (p *parser) readTripleQuotedString() (*ast.Value, error) {
}

func (p *parser) populateValue(vl string, preComments []string) *ast.Value {
if p.log {
p.log.Infof("value: %q", vl)
if p.config.infoLevel() {
p.config.infof("value: %q", vl)
}
return &ast.Value{
Value: vl,
Expand All @@ -986,8 +1013,8 @@ func (p *parser) populateValue(vl string, preComments []string) *ast.Value {

func (p *parser) readInlineComment() string {
inlineComment, _ := p.skipWhiteSpaceAndReadComments(false /* multiLine */)
if p.log {
p.log.Infof("inlineComment: %q", strings.Join(inlineComment, "\n"))
if p.config.infoLevel() {
p.config.infof("inlineComment: %q", strings.Join(inlineComment, "\n"))
}
if len(inlineComment) > 0 {
return inlineComment[0]
Expand Down Expand Up @@ -1124,7 +1151,7 @@ func wrapLines(nd *ast.Node, depth int, c Config) {

str, err := unquote.Raw(nd)
if err != nil {
log.Errorf("skipping string wrapping on node %q (error unquoting string): %v", nd.Name, err)
c.errorf("skipping string wrapping on node %q (error unquoting string): %v", nd.Name, err)
return
}

Expand Down

0 comments on commit d8aa537

Please sign in to comment.