From 09ea035b224e758ecd19477dc69e40f3bfb21654 Mon Sep 17 00:00:00 2001 From: mofant Date: Sun, 23 Feb 2025 17:00:23 +0800 Subject: [PATCH 1/2] fix: QuotedString_ISSUE65 --- config/statement.go | 14 ++--- dumper/dumper.go | 36 +++++++++---- parser/parser.go | 119 ++++++++++++++++++++---------------------- parser/parser_test.go | 28 ++++++++-- 4 files changed, 114 insertions(+), 83 deletions(-) diff --git a/config/statement.go b/config/statement.go index a8a64d2..8e3187b 100644 --- a/config/statement.go +++ b/config/statement.go @@ -25,23 +25,23 @@ type IDirective interface { // InlineCommenter represents the inline comment holder type InlineCommenter interface { - GetInlineComment() string - SetInlineComment(comment string) + GetInlineComment() []InlineComment + SetInlineComment(comment InlineComment) } // DefaultInlineComment represents the default inline comment holder type DefaultInlineComment struct { - InlineComment string + InlineComment []InlineComment } // GetInlineComment returns the inline comment -func (d *DefaultInlineComment) GetInlineComment() string { +func (d *DefaultInlineComment) GetInlineComment() []InlineComment { return d.InlineComment } // SetInlineComment sets the inline comment -func (d *DefaultInlineComment) SetInlineComment(comment string) { - d.InlineComment = comment +func (d *DefaultInlineComment) SetInlineComment(comment InlineComment) { + d.InlineComment = append(d.InlineComment, comment) } // FileDirective a statement that saves its own file @@ -84,3 +84,5 @@ func (p *Parameter) SetRelativeLineIndex(i int) { func (p *Parameter) GetRelativeLineIndex() int { return p.RelativeLineIndex } + +type InlineComment Parameter diff --git a/dumper/dumper.go b/dumper/dumper.go index ac4b485..8b18f87 100644 --- a/dumper/dumper.go +++ b/dumper/dumper.go @@ -88,31 +88,45 @@ func DumpDirective(d config.IDirective, style *Style) string { if style.SpaceBeforeBlocks && d.GetBlock() != nil { buf.WriteString("\n") } + // outline comment if len(d.GetComment()) > 0 { for _, comment := range d.GetComment() { buf.WriteString(fmt.Sprintf("%s%s\n", strings.Repeat(" ", style.StartIndent), comment)) } } buf.WriteString(fmt.Sprintf("%s%s", strings.Repeat(" ", style.StartIndent), d.GetName())) - if len(d.GetParameters()) > 0 { - // Use relative line index to handle different line number arrangements of instruction parameters - relativeLineIndex := 0 - for _, parameter := range d.GetParameters() { - // If the parameter line index is not the same as the previous one, add a new line - if parameter.GetRelativeLineIndex() != relativeLineIndex { - buf.WriteString("\n") - buf.WriteString(fmt.Sprintf("%s%s", strings.Repeat(" ", style.StartIndent), parameter.GetValue())) - relativeLineIndex = parameter.GetRelativeLineIndex() + + inlineComments := make(map[int]config.InlineComment) + for _, comment := range d.GetInlineComment() { + inlineComments[comment.RelativeLineIndex] = comment + } + + // Use relative line index to handle different line number arrangements of instruction parameters + relativeLineIndex := 0 + for _, parameter := range d.GetParameters() { + // If the parameter line index is not the same as the previous one, add a new line + if parameter.GetRelativeLineIndex() != relativeLineIndex { + // write param comment + if comment, ok := inlineComments[relativeLineIndex]; ok { + buf.WriteString(fmt.Sprintf(" %s\n", comment.Value)) } else { - buf.WriteString(fmt.Sprintf(" %s", parameter.GetValue())) + buf.WriteString("\n") } + buf.WriteString(fmt.Sprintf("%s%s", strings.Repeat(" ", style.StartIndent+style.Indent), parameter.GetValue())) + relativeLineIndex = parameter.GetRelativeLineIndex() + } else { + buf.WriteString(fmt.Sprintf(" %s", parameter.GetValue())) } } + if d.GetBlock() == nil { if d.GetName() != "" { buf.WriteRune(';') } - buf.WriteString(d.GetInlineComment()) + // the last inline comment + if comment, ok := inlineComments[relativeLineIndex]; ok { + buf.WriteString(comment.Value) + } } else { buf.WriteString(" {\n") buf.WriteString(DumpBlock(d.GetBlock(), style.Iterate())) diff --git a/parser/parser.go b/parser/parser.go index f433939..52ccf72 100644 --- a/parser/parser.go +++ b/parser/parser.go @@ -236,18 +236,8 @@ parsingLoop: if p.opts.skipComments { break } - // inline comment - if line == p.currentToken.Line { - if s == nil && len(context.Directives) > 0 { - s = context.Directives[len(context.Directives)-1] - } - s.SetInlineComment(p.currentToken.Literal) - s.SetLine(line) - p.commentBuffer = nil - } else { - p.commentBuffer = append(p.commentBuffer, p.currentToken.Literal) - } - + // outline comment + p.commentBuffer = append(p.commentBuffer, p.currentToken.Literal) } p.nextToken() } @@ -274,67 +264,74 @@ func (p *Parser) parseStatement(isSkipValidDirective bool) (config.IDirective, e return sp() } + // set outline comment if len(p.commentBuffer) > 0 { d.Comment = p.commentBuffer - p.commentBuffer = nil + p.commentBuffer = make([]string, 0) } - //parse parameters until the end. - // keep track of the line index of the directive - directiveLineIndex := p.currentToken.Line - for p.nextToken(); p.currentToken.IsParameterEligible(); p.nextToken() { - d.Parameters = append(d.Parameters, config.Parameter{ - Value: p.currentToken.Literal, - RelativeLineIndex: p.currentToken.Line - directiveLineIndex}) // save the relative line index of the parameter - if p.currentToken.Is(token.BlockEnd) { + directiveLineIndex := p.currentToken.Line // keep track of the line index of the directive + // Parse parameters until reaching the semicolon that ends the directive. + for { + p.nextToken() + if p.currentToken.IsParameterEligible() { + d.Parameters = append(d.Parameters, config.Parameter{ + Value: p.currentToken.Literal, + RelativeLineIndex: p.currentToken.Line - directiveLineIndex}) // save the relative line index of the parameter + if p.currentToken.Is(token.BlockEnd) { + return d, nil + } + } else if p.curTokenIs(token.Semicolon) { + // inline comment in following token + if !p.opts.skipComments { + if p.followingTokenIs(token.Comment) && p.followingToken.Line == p.currentToken.Line { + // if following token is a comment, then it is an inline comment, fetch next token + p.nextToken() + d.SetInlineComment(config.InlineComment{ + Value: p.currentToken.Literal, + RelativeLineIndex: p.currentToken.Line - directiveLineIndex, + }) + } + } + if iw, ok := p.includeWrappers[d.Name]; ok { + include, err := iw(d) + if err != nil { + return nil, err + } + return p.ParseInclude(include.(*config.Include)) + } else if dw, ok := p.directiveWrappers[d.Name]; ok { + return dw(d) + } return d, nil - } - } - - //if we find a semicolon it is a directive, we will check directive converters - if p.curTokenIs(token.Semicolon) { - if iw, ok := p.includeWrappers[d.Name]; ok { - include, err := iw(d) + } else if p.curTokenIs(token.Comment) { + // param comment + d.SetInlineComment(config.InlineComment{ + Value: p.currentToken.Literal, + RelativeLineIndex: p.currentToken.Line - directiveLineIndex, + }) + } else if p.curTokenIs(token.BlockStart) { + _, blockSkip1 := SkipValidBlocks[d.Name] + _, blockSkip2 := p.opts.skipValidSubDirectiveBlock[d.Name] + isSkipBlockSubDirective := blockSkip1 || blockSkip2 || isSkipValidDirective + + b, err := p.parseBlock(true, isSkipBlockSubDirective) if err != nil { return nil, err } - return p.ParseInclude(include.(*config.Include)) - } else if dw, ok := p.directiveWrappers[d.Name]; ok { - return dw(d) - } - return d, nil - } - for { - if p.curTokenIs(token.Comment) { - p.commentBuffer = append(p.commentBuffer, p.currentToken.Literal) - p.nextToken() - } else { - break - } - } + d.Block = b - //ok, it does not end with a semicolon but a block starts, we will convert that block if we have a converter - if p.curTokenIs(token.BlockStart) { - _, blockSkip1 := SkipValidBlocks[d.Name] - _, blockSkip2 := p.opts.skipValidSubDirectiveBlock[d.Name] - isSkipBlockSubDirective := blockSkip1 || blockSkip2 || isSkipValidDirective - b, err := p.parseBlock(true, isSkipBlockSubDirective) - if err != nil { - return nil, err - } - d.Block = b - - if strings.HasSuffix(d.Name, "_by_lua_block") { - return p.blockWrappers["_by_lua_block"](d) - } + if strings.HasSuffix(d.Name, "_by_lua_block") { + return p.blockWrappers["_by_lua_block"](d) + } - if bw, ok := p.blockWrappers[d.Name]; ok { - return bw(d) + if bw, ok := p.blockWrappers[d.Name]; ok { + return bw(d) + } + return d, nil + } else { + return nil, fmt.Errorf("unexpected token %s (%s) on line %d, column %d", p.currentToken.Type.String(), p.currentToken.Literal, p.currentToken.Line, p.currentToken.Column) } - return d, nil } - - return nil, fmt.Errorf("unexpected token %s (%s) on line %d, column %d", p.currentToken.Type.String(), p.currentToken.Literal, p.currentToken.Line, p.currentToken.Column) } // ParseInclude just parse include confs diff --git a/parser/parser_test.go b/parser/parser_test.go index a9749ea..31b6e64 100644 --- a/parser/parser_test.go +++ b/parser/parser_test.go @@ -574,14 +574,14 @@ stream { func TestParser_KeepDataInMultiLine01(t *testing.T) { p := NewStringParser(`log_format main '$remote_addr - $remote_user [$time_local] "$request" ' -'$status $body_bytes_sent "$http_referer" ' -'"$http_user_agent" "$http_x_forwarded_for"';`) + '$status $body_bytes_sent "$http_referer" ' + '"$http_user_agent" "$http_x_forwarded_for"';`) conf, err := p.Parse() assert.NilError(t, err, "no error expected here") s := dumper.DumpConfig(conf, dumper.IndentedStyle) assert.Equal(t, `log_format main '$remote_addr - $remote_user [$time_local] "$request" ' -'$status $body_bytes_sent "$http_referer" ' -'"$http_user_agent" "$http_x_forwarded_for"';`, s) + '$status $body_bytes_sent "$http_referer" ' + '"$http_user_agent" "$http_x_forwarded_for"';`, s) } @@ -594,7 +594,25 @@ func TestParser_KeepDataInMultiLine02(t *testing.T) { s := dumper.DumpConfig(conf, dumper.IndentedStyle) assert.Equal(t, `events { worker_connections - 4096; + 4096; }`, s) } + +func TestParser_QuotedString_ISSUE65(t *testing.T) { + p := NewStringParser(`log_format json_analytics escape=json '{' # json start + '"msec": "$msec", ' # request unixtime in seconds with a milliseconds resolution + '"connection": "$connection", ' # connection serial number + '"connection_requests": "$connection_requests", ' # number of requests made in connection + '}'; # inline comment +error_log off; # error_log inline comment`) + conf, err := p.Parse() + assert.NilError(t, err, "no error expected here") + s := dumper.DumpConfig(conf, dumper.IndentedStyle) + assert.Equal(t, `log_format json_analytics escape=json '{' # json start + '"msec": "$msec", ' # request unixtime in seconds with a milliseconds resolution + '"connection": "$connection", ' # connection serial number + '"connection_requests": "$connection_requests", ' # number of requests made in connection + '}';# inline comment +error_log off;# error_log inline comment`, s) +} From 5937f69095ba4d312042323e584a26b56181430b Mon Sep 17 00:00:00 2001 From: mofant Date: Mon, 24 Feb 2025 21:53:04 +0800 Subject: [PATCH 2/2] update comment --- config/location.go | 2 ++ config/statement.go | 1 + 2 files changed, 3 insertions(+) diff --git a/config/location.go b/config/location.go index c5a1805..9d0bfa8 100644 --- a/config/location.go +++ b/config/location.go @@ -57,6 +57,7 @@ func NewLocation(directive IDirective) (*Location, error) { return nil, errors.New("too many arguments for location directive") } +// FindDirectives find directives by name func (l *Location) FindDirectives(directiveName string) []IDirective { block := l.GetBlock() if block == nil { @@ -65,6 +66,7 @@ func (l *Location) FindDirectives(directiveName string) []IDirective { return block.FindDirectives(directiveName) } +// GetDirectives get all directives func (l *Location) GetDirectives() []IDirective { block := l.GetBlock() if block == nil { diff --git a/config/statement.go b/config/statement.go index 8e3187b..d6a195e 100644 --- a/config/statement.go +++ b/config/statement.go @@ -85,4 +85,5 @@ func (p *Parameter) GetRelativeLineIndex() int { return p.RelativeLineIndex } +// InlineComment represents an inline comment type InlineComment Parameter