From f7ea2cd749d178cb6c34c78c15c2f8845a82b9be Mon Sep 17 00:00:00 2001 From: Saswata Mukherjee Date: Sat, 19 Jun 2021 19:11:35 +0530 Subject: [PATCH 1/2] Add line numbers to errors Signed-off-by: Saswata Mukherjee --- pkg/mdformatter/linktransformer/link.go | 20 ++++----- pkg/mdformatter/linktransformer/link_test.go | 10 ++--- pkg/mdformatter/mdformatter.go | 3 +- pkg/mdformatter/mdformatter_test.go | 2 +- pkg/mdformatter/transformer.go | 47 +++++++++++++++++--- pkg/transform/transform.go | 2 +- 6 files changed, 59 insertions(+), 25 deletions(-) diff --git a/pkg/mdformatter/linktransformer/link.go b/pkg/mdformatter/linktransformer/link.go index b25ad21..44a2f58 100644 --- a/pkg/mdformatter/linktransformer/link.go +++ b/pkg/mdformatter/linktransformer/link.go @@ -48,9 +48,9 @@ func NewChain(c ...mdformatter.LinkTransformer) mdformatter.LinkTransformer { return &chain{chain: c} } -func (l *chain) TransformDestination(ctx mdformatter.SourceContext, destination []byte) (_ []byte, err error) { +func (l *chain) TransformDestination(ctx mdformatter.SourceContext, destination []byte, lines string) (_ []byte, err error) { for _, c := range l.chain { - destination, err = c.TransformDestination(ctx, destination) + destination, err = c.TransformDestination(ctx, destination, lines) if err != nil { return nil, err } @@ -80,7 +80,7 @@ func NewLocalizer(logger log.Logger, address *regexp.Regexp, anchorDir string) m return &localizer{logger: logger, address: address, anchorDir: anchorDir, localLinksByFile: map[string]*[]string{}} } -func (l *localizer) TransformDestination(ctx mdformatter.SourceContext, destination []byte) (_ []byte, err error) { +func (l *localizer) TransformDestination(ctx mdformatter.SourceContext, destination []byte, _ string) (_ []byte, err error) { matches := remoteLinkPrefixRe.FindAllIndex(destination, 1) if matches != nil { // URLs. Remove http/https prefix. @@ -129,7 +129,7 @@ type validator struct { } type futureKey struct { - filepath, dest string + filepath, dest, lines string } type futureResult struct { @@ -233,8 +233,8 @@ func MustNewValidator(logger log.Logger, linksValidateConfig []byte, anchorDir s return v } -func (v *validator) TransformDestination(ctx mdformatter.SourceContext, destination []byte) (_ []byte, err error) { - v.visit(ctx.Filepath, string(destination)) +func (v *validator) TransformDestination(ctx mdformatter.SourceContext, destination []byte, lines string) (_ []byte, err error) { + v.visit(ctx.Filepath, string(destination), lines) return destination, nil } @@ -266,19 +266,19 @@ func (v *validator) Close(ctx mdformatter.SourceContext) error { f := v.destFutures[k] if err := f.resultFn(); err != nil { if f.cases == 1 { - merr.Add(errors.Wrapf(err, "%v", path)) + merr.Add(errors.Wrapf(err, "%v:%v", path, k.lines)) continue } - merr.Add(errors.Wrapf(err, "%v (%v occurrences)", path, f.cases)) + merr.Add(errors.Wrapf(err, "%v:%v (%v occurrences)", path, k.lines, f.cases)) } } return merr.Err() } -func (v *validator) visit(filepath string, dest string) { +func (v *validator) visit(filepath string, dest string, lines string) { v.futureMu.Lock() defer v.futureMu.Unlock() - k := futureKey{filepath: filepath, dest: dest} + k := futureKey{filepath: filepath, dest: dest, lines: lines} if _, ok := v.destFutures[k]; ok { v.destFutures[k].cases++ return diff --git a/pkg/mdformatter/linktransformer/link_test.go b/pkg/mdformatter/linktransformer/link_test.go index 4e33a1f..5e7c405 100644 --- a/pkg/mdformatter/linktransformer/link_test.go +++ b/pkg/mdformatter/linktransformer/link_test.go @@ -225,10 +225,10 @@ func TestValidator_TransformDestination(t *testing.T) { testutil.NotOk(t, err) testutil.Equals(t, fmt.Sprintf("%v: 4 errors: "+ - "%v: link ../test2/invalid-local-links.md, normalized to: %v/repo/docs/test2/invalid-local-links.md: file not found; "+ - "%v: link ../test/invalid-local-links.md#not-yolo, normalized to: link %v/repo/docs/test/invalid-local-links.md#not-yolo, existing ids: [yolo]: file exists, but does not have such id; "+ - "%v: link ../test/doc.md, normalized to: %v/repo/docs/test/doc.md: file not found; "+ - "%v: link #not-yolo, normalized to: link %v/repo/docs/test/invalid-local-links.md#not-yolo, existing ids: [yolo]: file exists, but does not have such id", + "%v:3: link ../test2/invalid-local-links.md, normalized to: %v/repo/docs/test2/invalid-local-links.md: file not found; "+ + "%v:3: link ../test/invalid-local-links.md#not-yolo, normalized to: link %v/repo/docs/test/invalid-local-links.md#not-yolo, existing ids: [yolo]: file exists, but does not have such id; "+ + "%v:3: link ../test/doc.md, normalized to: %v/repo/docs/test/doc.md: file not found; "+ + "%v:3: link #not-yolo, normalized to: link %v/repo/docs/test/invalid-local-links.md#not-yolo, existing ids: [yolo]: file exists, but does not have such id", tmpDir+filePath, relDirPath+filePath, tmpDir, relDirPath+filePath, tmpDir, relDirPath+filePath, tmpDir, relDirPath+filePath, tmpDir), err.Error()) }) @@ -249,7 +249,7 @@ func TestValidator_TransformDestination(t *testing.T) { MustNewValidator(logger, []byte(""), anchorDir), )) testutil.NotOk(t, err) - testutil.Equals(t, fmt.Sprintf("%v%v: %v%v: \"https://bwplotka.dev/does-not-exists\" not accessible; status code 404: Not Found", tmpDir, filePath, relDirPath, filePath), err.Error()) + testutil.Equals(t, fmt.Sprintf("%v%v: %v%v:1: \"https://bwplotka.dev/does-not-exists\" not accessible; status code 404: Not Found", tmpDir, filePath, relDirPath, filePath), err.Error()) }) t.Run("check valid & 404 link with validate config", func(t *testing.T) { diff --git a/pkg/mdformatter/mdformatter.go b/pkg/mdformatter/mdformatter.go index d8915ba..1c9848d 100644 --- a/pkg/mdformatter/mdformatter.go +++ b/pkg/mdformatter/mdformatter.go @@ -36,7 +36,7 @@ type FrontMatterTransformer interface { } type LinkTransformer interface { - TransformDestination(ctx SourceContext, destination []byte) ([]byte, error) + TransformDestination(ctx SourceContext, destination []byte, lines string) ([]byte, error) Close(ctx SourceContext) error } @@ -283,6 +283,7 @@ func (f *Formatter) Format(file *os.File, out io.Writer) error { wrapped: markdown.NewRenderer(), sourceCtx: sourceCtx, link: f.link, cb: f.cb, + frontMatterLen: len(frontMatter), } if err := goldmark.New( goldmark.WithExtensions(extension.GFM), diff --git a/pkg/mdformatter/mdformatter_test.go b/pkg/mdformatter/mdformatter_test.go index 1939fb6..ce32614 100644 --- a/pkg/mdformatter/mdformatter_test.go +++ b/pkg/mdformatter/mdformatter_test.go @@ -59,7 +59,7 @@ type mockLinkTransformer struct { closed bool } -func (*mockLinkTransformer) TransformDestination(ctx SourceContext, destination []byte) ([]byte, error) { +func (*mockLinkTransformer) TransformDestination(ctx SourceContext, destination []byte, _ string) ([]byte, error) { if bytes.HasPrefix(destination, []byte("$$-")) { return destination, nil } diff --git a/pkg/mdformatter/transformer.go b/pkg/mdformatter/transformer.go index 4ec8d10..1f6e8d0 100644 --- a/pkg/mdformatter/transformer.go +++ b/pkg/mdformatter/transformer.go @@ -6,6 +6,8 @@ package mdformatter import ( "bytes" "io" + "regexp" + "strconv" "github.com/efficientgo/tools/core/pkg/merrors" "github.com/yuin/goldmark/ast" @@ -29,8 +31,9 @@ type transformer struct { sourceCtx SourceContext - link LinkTransformer - cb CodeBlockTransformer + link LinkTransformer + cb CodeBlockTransformer + frontMatterLen int } func (t *transformer) Render(w io.Writer, source []byte, node ast.Node) error { @@ -75,7 +78,8 @@ func (t *transformer) Render(w io.Writer, source []byte, node ast.Node) error { if token.Attr[i].Key != "src" { continue } - dest, err := t.link.TransformDestination(t.sourceCtx, []byte(token.Attr[i].Val)) + lines := getLinkLines(source, []byte(token.Attr[i].Val), t.frontMatterLen) + dest, err := t.link.TransformDestination(t.sourceCtx, []byte(token.Attr[i].Val), lines) if err != nil { return ast.WalkStop, err } @@ -87,7 +91,8 @@ func (t *transformer) Render(w io.Writer, source []byte, node ast.Node) error { if token.Attr[i].Key != "href" { continue } - dest, err := t.link.TransformDestination(t.sourceCtx, []byte(token.Attr[i].Val)) + lines := getLinkLines(source, []byte(token.Attr[i].Val), t.frontMatterLen) + dest, err := t.link.TransformDestination(t.sourceCtx, []byte(token.Attr[i].Val), lines) if err != nil { return ast.WalkStop, err } @@ -111,7 +116,8 @@ func (t *transformer) Render(w io.Writer, source []byte, node ast.Node) error { if !entering || t.link == nil { return ast.WalkSkipChildren, nil } - typedNode.Destination, err = t.link.TransformDestination(t.sourceCtx, typedNode.Destination) + lines := getLinkLines(source, typedNode.Destination, t.frontMatterLen) + typedNode.Destination, err = t.link.TransformDestination(t.sourceCtx, typedNode.Destination, lines) if err != nil { return ast.WalkStop, err } @@ -119,7 +125,8 @@ func (t *transformer) Render(w io.Writer, source []byte, node ast.Node) error { if !entering || t.link == nil || typedNode.AutoLinkType != ast.AutoLinkURL { return ast.WalkSkipChildren, nil } - dest, err := t.link.TransformDestination(t.sourceCtx, typedNode.URL(source)) + lines := getLinkLines(source, typedNode.URL(source), t.frontMatterLen) + dest, err := t.link.TransformDestination(t.sourceCtx, typedNode.URL(source), lines) if err != nil { return ast.WalkStop, err } @@ -135,7 +142,8 @@ func (t *transformer) Render(w io.Writer, source []byte, node ast.Node) error { if !entering || t.link == nil { return ast.WalkSkipChildren, nil } - typedNode.Destination, err = t.link.TransformDestination(t.sourceCtx, typedNode.Destination) + lines := getLinkLines(source, typedNode.Destination, t.frontMatterLen) + typedNode.Destination, err = t.link.TransformDestination(t.sourceCtx, typedNode.Destination, lines) if err != nil { return ast.WalkStop, err } @@ -178,3 +186,28 @@ func replaceContent(b *ast.BaseBlock, lastSegmentStop int, content []byte) { s.Append(text.NewSegment(lastSegmentStop, lastSegmentStop+len(content))) b.SetLines(s) } + +// getLinkLines returns line numbers in source where link is present. +func getLinkLines(source []byte, link []byte, lenfm int) string { + var targetLines string + sourceLines := bytes.Split(source, []byte("\n")) + // frontMatter is present so would need to account for `---` lines. + if lenfm > 0 { + lenfm += 2 + } + // Using regex, as two links may have same host but diff params. Same in case of local links. + linkRe := regexp.MustCompile(`(^|[^/\-~&=#?@%a-zA-Z0-9])` + string(link) + `($|[^/\-~&=#?@%a-zA-Z0-9])`) + for i, line := range sourceLines { + if linkRe.Match(line) { + // Easier to just return int slice, but then cannot use it in futureKey. + // https://golang.org/ref/spec#Map_types. + add := strconv.Itoa(i + 1 + lenfm) + if targetLines != "" { + add = "," + strconv.Itoa(i+1+lenfm) + } + targetLines += add + } + } + // If same link is found multiple times returns string like *,*,*... + return targetLines +} diff --git a/pkg/transform/transform.go b/pkg/transform/transform.go index 2871bb6..365b24a 100644 --- a/pkg/transform/transform.go +++ b/pkg/transform/transform.go @@ -211,7 +211,7 @@ type relLinkTransformer struct { newRelPath map[string]string } -func (r *relLinkTransformer) TransformDestination(ctx mdformatter.SourceContext, destination []byte) ([]byte, error) { +func (r *relLinkTransformer) TransformDestination(ctx mdformatter.SourceContext, destination []byte, _ string) ([]byte, error) { split := strings.Split(string(destination), "#") relDest := split[0] if strings.Contains(relDest, "://") || filepath.IsAbs(relDest) || strings.HasPrefix(string(destination), "#") { From 33ca83c5dc77bbcd5885b6cb493c74ba5662b5d3 Mon Sep 17 00:00:00 2001 From: Saswata Mukherjee Date: Fri, 25 Jun 2021 08:44:13 +0530 Subject: [PATCH 2/2] Implement suggestions Signed-off-by: Saswata Mukherjee --- pkg/mdformatter/linktransformer/link.go | 20 ++++++++++---------- pkg/mdformatter/mdformatter.go | 5 +++-- pkg/mdformatter/mdformatter_test.go | 2 +- pkg/mdformatter/transformer.go | 20 ++++++++++---------- pkg/transform/transform.go | 2 +- 5 files changed, 25 insertions(+), 24 deletions(-) diff --git a/pkg/mdformatter/linktransformer/link.go b/pkg/mdformatter/linktransformer/link.go index 44a2f58..4c38a45 100644 --- a/pkg/mdformatter/linktransformer/link.go +++ b/pkg/mdformatter/linktransformer/link.go @@ -48,9 +48,9 @@ func NewChain(c ...mdformatter.LinkTransformer) mdformatter.LinkTransformer { return &chain{chain: c} } -func (l *chain) TransformDestination(ctx mdformatter.SourceContext, destination []byte, lines string) (_ []byte, err error) { +func (l *chain) TransformDestination(ctx mdformatter.SourceContext, destination []byte) (_ []byte, err error) { for _, c := range l.chain { - destination, err = c.TransformDestination(ctx, destination, lines) + destination, err = c.TransformDestination(ctx, destination) if err != nil { return nil, err } @@ -80,7 +80,7 @@ func NewLocalizer(logger log.Logger, address *regexp.Regexp, anchorDir string) m return &localizer{logger: logger, address: address, anchorDir: anchorDir, localLinksByFile: map[string]*[]string{}} } -func (l *localizer) TransformDestination(ctx mdformatter.SourceContext, destination []byte, _ string) (_ []byte, err error) { +func (l *localizer) TransformDestination(ctx mdformatter.SourceContext, destination []byte) (_ []byte, err error) { matches := remoteLinkPrefixRe.FindAllIndex(destination, 1) if matches != nil { // URLs. Remove http/https prefix. @@ -129,7 +129,7 @@ type validator struct { } type futureKey struct { - filepath, dest, lines string + filepath, dest, lineNumbers string } type futureResult struct { @@ -233,8 +233,8 @@ func MustNewValidator(logger log.Logger, linksValidateConfig []byte, anchorDir s return v } -func (v *validator) TransformDestination(ctx mdformatter.SourceContext, destination []byte, lines string) (_ []byte, err error) { - v.visit(ctx.Filepath, string(destination), lines) +func (v *validator) TransformDestination(ctx mdformatter.SourceContext, destination []byte) (_ []byte, err error) { + v.visit(ctx.Filepath, string(destination), ctx.LineNumbers) return destination, nil } @@ -266,19 +266,19 @@ func (v *validator) Close(ctx mdformatter.SourceContext) error { f := v.destFutures[k] if err := f.resultFn(); err != nil { if f.cases == 1 { - merr.Add(errors.Wrapf(err, "%v:%v", path, k.lines)) + merr.Add(errors.Wrapf(err, "%v:%v", path, k.lineNumbers)) continue } - merr.Add(errors.Wrapf(err, "%v:%v (%v occurrences)", path, k.lines, f.cases)) + merr.Add(errors.Wrapf(err, "%v:%v (%v occurrences)", path, k.lineNumbers, f.cases)) } } return merr.Err() } -func (v *validator) visit(filepath string, dest string, lines string) { +func (v *validator) visit(filepath string, dest string, lineNumbers string) { v.futureMu.Lock() defer v.futureMu.Unlock() - k := futureKey{filepath: filepath, dest: dest, lines: lines} + k := futureKey{filepath: filepath, dest: dest, lineNumbers: lineNumbers} if _, ok := v.destFutures[k]; ok { v.destFutures[k].cases++ return diff --git a/pkg/mdformatter/mdformatter.go b/pkg/mdformatter/mdformatter.go index 1c9848d..ce4cf56 100644 --- a/pkg/mdformatter/mdformatter.go +++ b/pkg/mdformatter/mdformatter.go @@ -27,7 +27,8 @@ import ( type SourceContext struct { context.Context - Filepath string + Filepath string + LineNumbers string } type FrontMatterTransformer interface { @@ -36,7 +37,7 @@ type FrontMatterTransformer interface { } type LinkTransformer interface { - TransformDestination(ctx SourceContext, destination []byte, lines string) ([]byte, error) + TransformDestination(ctx SourceContext, destination []byte) ([]byte, error) Close(ctx SourceContext) error } diff --git a/pkg/mdformatter/mdformatter_test.go b/pkg/mdformatter/mdformatter_test.go index ce32614..1939fb6 100644 --- a/pkg/mdformatter/mdformatter_test.go +++ b/pkg/mdformatter/mdformatter_test.go @@ -59,7 +59,7 @@ type mockLinkTransformer struct { closed bool } -func (*mockLinkTransformer) TransformDestination(ctx SourceContext, destination []byte, _ string) ([]byte, error) { +func (*mockLinkTransformer) TransformDestination(ctx SourceContext, destination []byte) ([]byte, error) { if bytes.HasPrefix(destination, []byte("$$-")) { return destination, nil } diff --git a/pkg/mdformatter/transformer.go b/pkg/mdformatter/transformer.go index 1f6e8d0..09037fa 100644 --- a/pkg/mdformatter/transformer.go +++ b/pkg/mdformatter/transformer.go @@ -78,8 +78,8 @@ func (t *transformer) Render(w io.Writer, source []byte, node ast.Node) error { if token.Attr[i].Key != "src" { continue } - lines := getLinkLines(source, []byte(token.Attr[i].Val), t.frontMatterLen) - dest, err := t.link.TransformDestination(t.sourceCtx, []byte(token.Attr[i].Val), lines) + t.sourceCtx.LineNumbers = getLinkLines(source, []byte(token.Attr[i].Val), t.frontMatterLen) + dest, err := t.link.TransformDestination(t.sourceCtx, []byte(token.Attr[i].Val)) if err != nil { return ast.WalkStop, err } @@ -91,8 +91,8 @@ func (t *transformer) Render(w io.Writer, source []byte, node ast.Node) error { if token.Attr[i].Key != "href" { continue } - lines := getLinkLines(source, []byte(token.Attr[i].Val), t.frontMatterLen) - dest, err := t.link.TransformDestination(t.sourceCtx, []byte(token.Attr[i].Val), lines) + t.sourceCtx.LineNumbers = getLinkLines(source, []byte(token.Attr[i].Val), t.frontMatterLen) + dest, err := t.link.TransformDestination(t.sourceCtx, []byte(token.Attr[i].Val)) if err != nil { return ast.WalkStop, err } @@ -116,8 +116,8 @@ func (t *transformer) Render(w io.Writer, source []byte, node ast.Node) error { if !entering || t.link == nil { return ast.WalkSkipChildren, nil } - lines := getLinkLines(source, typedNode.Destination, t.frontMatterLen) - typedNode.Destination, err = t.link.TransformDestination(t.sourceCtx, typedNode.Destination, lines) + t.sourceCtx.LineNumbers = getLinkLines(source, typedNode.Destination, t.frontMatterLen) + typedNode.Destination, err = t.link.TransformDestination(t.sourceCtx, typedNode.Destination) if err != nil { return ast.WalkStop, err } @@ -125,8 +125,8 @@ func (t *transformer) Render(w io.Writer, source []byte, node ast.Node) error { if !entering || t.link == nil || typedNode.AutoLinkType != ast.AutoLinkURL { return ast.WalkSkipChildren, nil } - lines := getLinkLines(source, typedNode.URL(source), t.frontMatterLen) - dest, err := t.link.TransformDestination(t.sourceCtx, typedNode.URL(source), lines) + t.sourceCtx.LineNumbers = getLinkLines(source, typedNode.URL(source), t.frontMatterLen) + dest, err := t.link.TransformDestination(t.sourceCtx, typedNode.URL(source)) if err != nil { return ast.WalkStop, err } @@ -142,8 +142,8 @@ func (t *transformer) Render(w io.Writer, source []byte, node ast.Node) error { if !entering || t.link == nil { return ast.WalkSkipChildren, nil } - lines := getLinkLines(source, typedNode.Destination, t.frontMatterLen) - typedNode.Destination, err = t.link.TransformDestination(t.sourceCtx, typedNode.Destination, lines) + t.sourceCtx.LineNumbers = getLinkLines(source, typedNode.Destination, t.frontMatterLen) + typedNode.Destination, err = t.link.TransformDestination(t.sourceCtx, typedNode.Destination) if err != nil { return ast.WalkStop, err } diff --git a/pkg/transform/transform.go b/pkg/transform/transform.go index 365b24a..2871bb6 100644 --- a/pkg/transform/transform.go +++ b/pkg/transform/transform.go @@ -211,7 +211,7 @@ type relLinkTransformer struct { newRelPath map[string]string } -func (r *relLinkTransformer) TransformDestination(ctx mdformatter.SourceContext, destination []byte, _ string) ([]byte, error) { +func (r *relLinkTransformer) TransformDestination(ctx mdformatter.SourceContext, destination []byte) ([]byte, error) { split := strings.Split(string(destination), "#") relDest := split[0] if strings.Contains(relDest, "://") || filepath.IsAbs(relDest) || strings.HasPrefix(string(destination), "#") {