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

refactor code and add more linters #159

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
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
3 changes: 3 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,15 @@ linters:
- gosimple
- govet
- ineffassign
# - mirror # requires golangci-lint 1.53
- misspell
- staticcheck
- structcheck
- thelper
- typecheck
- unparam
- unused
- usestdlibvars
- varcheck

linters-settings:
Expand Down
1 change: 1 addition & 0 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ func main() {
// TODO(bwplotka): Move to customized better setup function.
return runner(ctx, logger)
}, func(err error) {
level.Error(logger).Log("err", fmt.Errorf("%s runner failed: %w", cmd, err))
cancel()
})

Expand Down
2 changes: 1 addition & 1 deletion pkg/cache/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ func (s *SQLite3Storage) IsCached(URL string) (bool, error) {
row := statement.QueryRow(URL)
if err = row.Scan(&timestamp); err != nil {
// If ErrNoRows then it means URL is new, so need to call Visited.
if err == sql.ErrNoRows {
if errors.Is(err, sql.ErrNoRows) {
return false, nil
}
return false, err
Expand Down
5 changes: 1 addition & 4 deletions pkg/cache/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,7 @@ func (c *Config) UnmarshalYAML(value *yaml.Node) error {
if err := value.Decode(c.cacheParser); err != nil {
return err
}
if err := c.load(); err != nil {
return err
}
return nil
return c.load()
}

// load validates the cache configuration from the parser and copy it
Expand Down
20 changes: 13 additions & 7 deletions pkg/clilog/clilog.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,10 @@ func New(w io.Writer) log.Logger {
}

func (l logger) Log(keyvals ...interface{}) error {
buf := bufPool.Get().(*buf)
buf, ok := bufPool.Get().(*buf)
if !ok {
return errors.New("invalid buffer")
}
buf.Reset()
defer bufPool.Put(buf)

Expand Down Expand Up @@ -152,14 +155,17 @@ func (enc *Encoder) EncodeKeyvals(keyvals ...interface{}) error {
for i := 0; i < len(keyvals); i += 2 {
k, v := keyvals[i], keyvals[i+1]
err := enc.EncodeKeyval(k, v)
if err == ErrUnsupportedKeyType {

var marshalError *MarshalerError
switch {
case errors.Is(err, ErrUnsupportedKeyType):
continue
}
if _, ok := err.(*MarshalerError); ok || err == ErrUnsupportedValueType {
case
errors.As(err, &marshalError),
errors.Is(err, ErrUnsupportedValueType):
v = err
err = enc.EncodeKeyval(k, v)
}
if err != nil {
return enc.EncodeKeyval(k, v)
case err != nil:
return err
}
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/mdformatter/linktransformer/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,14 +148,14 @@ func getGitHubRegex(pullsIssuesRe string, repoToken string) (*regexp.Regexp, int
client := &http.Client{}

// Check latest pull request number.
reqPull, err := http.NewRequest("GET", fmt.Sprintf(gitHubAPIURL, reponame, "pulls"), nil)
reqPull, err := http.NewRequest(http.MethodGet, fmt.Sprintf(gitHubAPIURL, reponame, "pulls"), nil)
if err != nil {
return nil, math.MaxInt64, err
}
reqPull.Header.Set("User-Agent", "mdox")

// Check latest issue number and return whichever is greater.
reqIssue, err := http.NewRequest("GET", fmt.Sprintf(gitHubAPIURL, reponame, "issues"), nil)
reqIssue, err := http.NewRequest(http.MethodGet, fmt.Sprintf(gitHubAPIURL, reponame, "issues"), nil)
if err != nil {
return nil, math.MaxInt64, err
}
Expand All @@ -170,7 +170,7 @@ func getGitHubRegex(pullsIssuesRe string, repoToken string) (*regexp.Regexp, int
if err != nil {
return nil, math.MaxInt64, err
}
if respPull.StatusCode != 200 {
if respPull.StatusCode != http.StatusOK {
return nil, math.MaxInt64, fmt.Errorf("pulls API request failed. status code: %d", respPull.StatusCode)
}
defer respPull.Body.Close()
Expand All @@ -182,7 +182,7 @@ func getGitHubRegex(pullsIssuesRe string, repoToken string) (*regexp.Regexp, int
if err != nil {
return nil, math.MaxInt64, err
}
if respIssue.StatusCode != 200 {
if respIssue.StatusCode != http.StatusOK {
return nil, math.MaxInt64, fmt.Errorf("issues API request failed. status code: %d", respIssue.StatusCode)
}
defer respIssue.Body.Close()
Expand Down
10 changes: 5 additions & 5 deletions pkg/mdformatter/linktransformer/link.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ func (l *localizer) TransformDestination(ctx mdformatter.SourceContext, destinat
return absLinkToRelLink(newDest, ctx.Filepath)
}

func (l *localizer) Close(mdformatter.SourceContext) error { return nil }
func (*localizer) Close(mdformatter.SourceContext) error { return nil }

type validator struct {
logger log.Logger
Expand Down Expand Up @@ -532,12 +532,12 @@ func (l localLinksCache) addRelLinks(localLink string) error {
reader := bufio.NewReader(file)
for {
b, err = reader.ReadBytes('\n')
if err != nil {
if err != io.EOF {
return fmt.Errorf("failed to read file %v: %w", localLink, err)
}
if errors.Is(err, io.EOF) {
break
}
if err != nil {
return fmt.Errorf("failed to read file %v: %w", localLink, err)
}

if bytes.HasPrefix(b, []byte(`#`)) {
ids = append(ids, toHeaderID(b))
Expand Down
2 changes: 2 additions & 0 deletions pkg/mdformatter/linktransformer/link_bench_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ This is a test section [link](./doc.md#this-is-a-section)`
)

func benchLinktransformer(b *testing.B) {
b.Helper()

tmpDir, err := os.MkdirTemp("", "bench-test")
testutil.Ok(b, err)
b.Cleanup(func() { testutil.Ok(b, os.RemoveAll(tmpDir)) })
Expand Down
4 changes: 2 additions & 2 deletions pkg/mdformatter/linktransformer/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func (v GitHubPullsIssuesValidator) IsValid(k futureKey, r *validator) (bool, er
}

// RoundTripValidator.IsValid returns true if URL is checked by colly or it is a valid local link.
func (v RoundTripValidator) IsValid(k futureKey, r *validator) (bool, error) {
func (RoundTripValidator) IsValid(k futureKey, r *validator) (bool, error) {
matches := remoteLinkPrefixRe.FindAllStringIndex(k.dest, 1)
if matches == nil && r.validateConfig.ExplicitLocalValidators {
r.l.localLinksChecked.Inc()
Expand Down Expand Up @@ -73,7 +73,7 @@ func (v RoundTripValidator) IsValid(k futureKey, r *validator) (bool, error) {
}

// IgnoreValidator.IsValid returns true if matched so that link in not checked.
func (v IgnoreValidator) IsValid(k futureKey, r *validator) (bool, error) {
func (IgnoreValidator) IsValid(_ futureKey, r *validator) (bool, error) {
r.l.ignoreSkippedLinks.Inc()

return true, nil
Expand Down
4 changes: 2 additions & 2 deletions pkg/mdformatter/mdformatter.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,13 +182,13 @@ func FormatFrontMatter(m map[string]interface{}) ([]byte, error) {
keys: keys,
}

b := bytes.NewBuffer([]byte("---\n"))
b := bytes.NewBufferString("---\n")
o, err := yaml.Marshal(f)
if err != nil {
return nil, fmt.Errorf("marshall front matter: %w", err)
}
_, _ = b.Write(o)
_, _ = b.Write([]byte("---\n\n"))
_, _ = b.WriteString("---\n\n")
return b.Bytes(), nil
}

Expand Down
6 changes: 4 additions & 2 deletions pkg/mdformatter/mdformatter_bench_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ import (

var testBuf bytes.Buffer

func benchMdformatter(filename string, b *testing.B) {
func benchMdformatter(b *testing.B, filename string) {
b.Helper()

file, err := os.OpenFile(filename, os.O_RDONLY, 0)
testutil.Ok(b, err)
defer file.Close()
Expand All @@ -28,4 +30,4 @@ func benchMdformatter(filename string, b *testing.B) {
})
}

func Benchmark_Mdformatter(b *testing.B) { benchMdformatter("testdata/not_formatted.md", b) }
func Benchmark_Mdformatter(b *testing.B) { benchMdformatter(b, "testdata/not_formatted.md") }
19 changes: 9 additions & 10 deletions pkg/mdformatter/mdgen/mdgen.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package mdgen

import (
"bytes"
"errors"
"fmt"
"os/exec"
"strconv"
Expand All @@ -19,17 +20,15 @@ const (
infoStringKeyExitCode = "mdox-expect-exit-code"
)

var (
newLineChar = []byte("\n")
)
var newLineChar = []byte("\n")

type genCodeBlockTransformer struct{}

func NewCodeBlockTransformer() *genCodeBlockTransformer {
return &genCodeBlockTransformer{}
}

func (t *genCodeBlockTransformer) TransformCodeBlock(ctx mdformatter.SourceContext, infoString []byte, code []byte) ([]byte, error) {
func (*genCodeBlockTransformer) TransformCodeBlock(ctx mdformatter.SourceContext, infoString []byte, code []byte) ([]byte, error) {
if len(infoString) == 0 {
return code, nil
}
Expand Down Expand Up @@ -82,13 +81,13 @@ func (t *genCodeBlockTransformer) TransformCodeBlock(ctx mdformatter.SourceConte
cmd.Stdout = &b
if err := cmd.Run(); err != nil {
expectedCode, _ := strconv.Atoi(infoStringAttr[infoStringKeyExitCode])
if exitErr, ok := err.(*exec.ExitError); ok {
if exitErr.ExitCode() != expectedCode {
return nil, fmt.Errorf("run %v, expected exit code %v, got %v, out: %v, error: %w", execCmd, expectedCode, exitErr.ExitCode(), b.String(), err)
}
} else {
var exitErr *exec.ExitError
if !errors.As(err, &exitErr) {
return nil, fmt.Errorf("run %v, out: %v, error: %w", execCmd, b.String(), err)
}
if exitErr.ExitCode() != expectedCode {
return nil, fmt.Errorf("run %v, expected exit code %v, got %v, out: %v, error: %w", execCmd, expectedCode, exitErr.ExitCode(), b.String(), err)
}
}
output := b.Bytes()
// Add newline to output if not present.
Expand All @@ -101,4 +100,4 @@ func (t *genCodeBlockTransformer) TransformCodeBlock(ctx mdformatter.SourceConte
panic("should never get here")
}

func (t *genCodeBlockTransformer) Close(ctx mdformatter.SourceContext) error { return nil }
func (*genCodeBlockTransformer) Close(_ mdformatter.SourceContext) error { return nil }
10 changes: 6 additions & 4 deletions pkg/mdformatter/mdgen/mdgen_bench_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ import (

var testBuf bytes.Buffer

func benchMdgen(filename string, b *testing.B) {
func benchMdgen(b *testing.B, filename string) {
b.Helper()

file, err := os.OpenFile(filename, os.O_RDONLY, 0)
testutil.Ok(b, err)
defer file.Close()
Expand All @@ -30,8 +32,8 @@ func benchMdgen(filename string, b *testing.B) {
})
}

func Benchmark_Mdgen_Sleep2(b *testing.B) { benchMdgen("benchdata/sleep2.md", b) }
func Benchmark_Mdgen_Sleep2(b *testing.B) { benchMdgen(b, "benchdata/sleep2.md") }

func Benchmark_Mdgen_Sleep5(b *testing.B) { benchMdgen("benchdata/sleep5.md", b) }
func Benchmark_Mdgen_Sleep5(b *testing.B) { benchMdgen(b, "benchdata/sleep5.md") }

func Benchmark_Mdgen_GoHelp(b *testing.B) { benchMdgen("benchdata/gohelp.md", b) }
func Benchmark_Mdgen_GoHelp(b *testing.B) { benchMdgen(b, "benchdata/gohelp.md") }
4 changes: 3 additions & 1 deletion pkg/mdformatter/transformer.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package mdformatter

import (
"bytes"
"errors"
"io"
"regexp"
"strconv"
Expand Down Expand Up @@ -102,7 +103,8 @@ func (t *transformer) Render(w io.Writer, source []byte, node ast.Node) error {
}
out += token.String()
}
if err := z.Err(); err != nil && err != io.EOF {

if err := z.Err(); err != nil && !errors.Is(err, io.EOF) {
return ast.WalkStop, err
}

Expand Down
8 changes: 4 additions & 4 deletions pkg/transform/transform.go
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ func (r *relLinkTransformer) TransformDestination(ctx mdformatter.SourceContext,
return []byte(newDest), nil
}

func (r *relLinkTransformer) Close(mdformatter.SourceContext) error { return nil }
func (*relLinkTransformer) Close(mdformatter.SourceContext) error { return nil }

type frontMatterTransformer struct {
localLinksStyle LocalLinksStyle
Expand Down Expand Up @@ -419,9 +419,9 @@ func (f *frontMatterTransformer) TransformFrontMatter(ctx mdformatter.SourceCont
return mdformatter.FormatFrontMatter(m)
}

func (f *frontMatterTransformer) Close(mdformatter.SourceContext) error { return nil }
func (*frontMatterTransformer) Close(mdformatter.SourceContext) error { return nil }

func (f *backMatterTransformer) TransformBackMatter(ctx mdformatter.SourceContext) ([]byte, error) {
func (f *backMatterTransformer) TransformBackMatter(_ mdformatter.SourceContext) ([]byte, error) {
b := bytes.Buffer{}
if err := f.b._template.Execute(&b, struct {
Origin MatterOrigin
Expand All @@ -442,7 +442,7 @@ func (f *backMatterTransformer) TransformBackMatter(ctx mdformatter.SourceContex
return m, nil
}

func (f *backMatterTransformer) Close(mdformatter.SourceContext) error { return nil }
func (*backMatterTransformer) Close(mdformatter.SourceContext) error { return nil }

func firstMatch(absRelPath string, trs []*TransformationConfig) (*TransformationConfig, bool) {
for _, tr := range trs {
Expand Down
4 changes: 2 additions & 2 deletions pkg/transform/transform_e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,18 +37,18 @@ func TestTransform(t *testing.T) {
testutil.Ok(t, err)
testutil.Ok(t, transform.Dir(context.Background(), logger, mdox2))
assertDirContent(t, filepath.Join(testData, "expected", "test2"), filepath.Join(tmpDir, "test2"))

})
t.Run("mdox3.yaml", func(t *testing.T) {
mdox2, err := os.ReadFile(filepath.Join(testData, "mdox3.yaml"))
testutil.Ok(t, err)
testutil.Ok(t, transform.Dir(context.Background(), logger, mdox2))
assertDirContent(t, filepath.Join(testData, "expected", "test3"), filepath.Join(tmpDir, "test3"))

})
}

func assertDirContent(t *testing.T, expectedDir string, gotDir string) {
t.Helper()

// TODO(bwplotka): Check if some garbage was not generated too!
testutil.Ok(t, filepath.Walk(expectedDir, func(path string, info os.FileInfo, err error) error {
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion scripts/copyright/copyright.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func applyLicenseToProtoAndGo() error {
return err
}

if !strings.HasPrefix(string(b), string(license)) {
if !bytes.HasPrefix(b, license) {
log.Println("file", path, "is missing Copyright header. Adding.")

var bb bytes.Buffer
Expand Down
Loading