Skip to content

Add codespell support (config, workflow to detect/not fix) and make it fix few typos #6872

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.

Sign up for GitHub

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
+39 −7
Open

Add codespell support (config, workflow to detect/not fix) and make it fix few typos #6872

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6d40251
Add github action to codespell master on push and PRs
yarikoptic Mar 4, 2025
8b02800
Add rudimentary codespell config with some skips
yarikoptic Mar 4, 2025
430910e
Annotate to ignore a line with forced first 5 symbols
yarikoptic Mar 4, 2025
362cdaa
Ignore some words pointed out in PR review
yarikoptic Mar 6, 2025
2eb49c4
[DATALAD RUNCMD] run codespell throughout fixing few left typos autom…
yarikoptic Mar 6, 2025
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
7 changes: 7 additions & 0 deletions .codespellrc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[codespell]
# Ref: https://github.com/codespell-project/codespell#using-a-config-file
skip = .git*,go.sum,.codespellrc,.golangci.yml
check-hidden = true
# ignore-regex =
# "idiosyncracies" or reflections of some config values like pass_thru
ignore-words-list = ot,te,ond,fo,tread,noes,pass-thru,pass-thrus,keep-alives
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are ot, te, ond, fo, and why is tread on there? (Tread is a dictionary word.)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

an example of hits for `ot` 
❯ git grep -i '\<ot\>'
.codespellrc:ignore-words-list = ot,te,ond,fo,tread,noes,pass-thru,pass-thrus,keep-alives
go.mod:     go.opentelemetry.io/contrib/propagators/ot v1.17.0 // indirect
go.sum:go.opentelemetry.io/contrib/propagators/ot v1.17.0 h1:ufo2Vsz8l76eI47jFjuVyjyB3Ae2DmfiCV/o6Vc8ii0=
go.sum:go.opentelemetry.io/contrib/propagators/ot v1.17.0/go.mod h1:SbKPj5XGp8K/sGm05XblaIABgMgw2jDczP8gGeuaVLk=
modules/caddyhttp/tracing/module.go:func (ot *Tracing) Provision(ctx caddy.Context) error {
modules/caddyhttp/tracing/module.go:        ot.logger = ctx.Logger()
modules/caddyhttp/tracing/module.go:        ot.otel, err = newOpenTelemetryWrapper(ctx, ot.SpanName)
modules/caddyhttp/tracing/module.go:func (ot *Tracing) Cleanup() error {
modules/caddyhttp/tracing/module.go:        if err := ot.otel.cleanup(ot.logger); err != nil {
modules/caddyhttp/tracing/module.go:func (ot *Tracing) ServeHTTP(w http.ResponseWriter, r *http.Request, next caddyhttp.Handler) error {
modules/caddyhttp/tracing/module.go:        return ot.otel.ServeHTTP(w, r, next)
modules/caddyhttp/tracing/module.go:func (ot *Tracing) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
modules/caddyhttp/tracing/module.go:                "span": &ot.SpanName,
modules/caddyhttp/tracing/module_test.go:                   ot := &Tracing{}
modules/caddyhttp/tracing/module_test.go:                   if err := ot.UnmarshalCaddyfile(tt.d); (err != nil) != tt.wantErr {
modules/caddyhttp/tracing/module_test.go:                   if ot.SpanName != tt.spanName {
modules/caddyhttp/tracing/module_test.go:                           t.Errorf("UnmarshalCaddyfile() SpanName = %v, want SpanName %v", ot.SpanName, tt.spanName)
modules/caddyhttp/tracing/module_test.go:                   ot := &Tracing{}
modules/caddyhttp/tracing/module_test.go:                   if err := ot.UnmarshalCaddyfile(tt.d); (err != nil) != tt.wantErr {
modules/caddyhttp/tracing/module_test.go:   ot := &Tracing{
modules/caddyhttp/tracing/module_test.go:   if err := ot.Provision(ctx); err != nil {
modules/caddyhttp/tracing/module_test.go:   if err := ot.ServeHTTP(w, req, handler); err != nil {
modules/caddyhttp/tracing/module_test.go:   ot := &Tracing{
modules/caddyhttp/tracing/module_test.go:   if err := ot.Provision(ctx); err != nil {
modules/caddyhttp/tracing/module_test.go:   if err := ot.ServeHTTP(w, req, handler); err != nil {
modules/caddyhttp/tracing/module_test.go:   ot := &Tracing{
modules/caddyhttp/tracing/module_test.go:   if err := ot.Provision(ctx); err != nil {
modules/caddyhttp/tracing/module_test.go:   if err := ot.ServeHTTP(w, req, handler); err == nil || !errors.Is(err, expectErr) {
modules/caddyhttp/tracing/tracer.go:        ot := openTelemetryWrapper{
modules/caddyhttp/tracing/tracer.go:        res, err := ot.newResource(webEngineName, version)
modules/caddyhttp/tracing/tracer.go:                return ot, fmt.Errorf("creating resource error: %w", err)
modules/caddyhttp/tracing/tracer.go:                return ot, fmt.Errorf("creating trace exporter error: %w", err)
modules/caddyhttp/tracing/tracer.go:        ot.propagators = autoprop.NewTextMapPropagator()
modules/caddyhttp/tracing/tracer.go:        ot.handler = otelhttp.NewHandler(http.HandlerFunc(ot.serveHTTP),
modules/caddyhttp/tracing/tracer.go:                ot.spanName,
modules/caddyhttp/tracing/tracer.go:                otelhttp.WithPropagators(ot.propagators),
modules/caddyhttp/tracing/tracer.go:                otelhttp.WithSpanNameFormatter(ot.spanNameFormatter),
modules/caddyhttp/tracing/tracer.go:        return ot, nil
modules/caddyhttp/tracing/tracer.go:func (ot *openTelemetryWrapper) serveHTTP(w http.ResponseWriter, r *http.Request) {
modules/caddyhttp/tracing/tracer.go:        ot.propagators.Inject(ctx, propagation.HeaderCarrier(r.Header))
modules/caddyhttp/tracing/tracer.go:func (ot *openTelemetryWrapper) ServeHTTP(w http.ResponseWriter, r *http.Request, next caddyhttp.Handler) error {
modules/caddyhttp/tracing/tracer.go:        ot.handler.ServeHTTP(w, r.WithContext(context.WithValue(r.Context(), nextCallCtxKey, n)))
modules/caddyhttp/tracing/tracer.go:func (ot *openTelemetryWrapper) cleanup(logger *zap.Logger) error {
modules/caddyhttp/tracing/tracer.go:func (ot *openTelemetryWrapper) newResource(
modules/caddyhttp/tracing/tracer.go:func (ot *openTelemetryWrapper) spanNameFormatter(operation string, r *http.Request) string {

tread is indeed a correct word but rare to encounter in code, more often a typo from thread, hence here (I guess).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh ... this checks spelling of variable names too?

I was kinda hoping it would just do comments and maybe strings.

I dunno. I'm still not super convinced that this is a net help for the project. I like the idea but in practice I can see it requires maintaining exceptions more than the occasional typo fix.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's somewhat of a beauty of the codespell that it is a very basic tool which identifies words (regardless of the programming language etc) and see which ones are close hits to typos. I do once-a-while run into fixing bugs where variable in a scripted (e.g. python or JS) program was mistyped and never covered by tests, thus never detected... Of cause YMMV, but you might appreciate the low number of false positives (besides those non-descript short variable names) it ran into in caddy's entire codebase.

25 changes: 25 additions & 0 deletions .github/workflows/codespell.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
# Codespell configuration is within .codespellrc
---
name: Codespell

on:
push:
branches: [master]
pull_request:
branches: [master]

permissions:
contents: read

jobs:
codespell:
name: Check for spelling errors
runs-on: ubuntu-latest

steps:
- name: Checkout
uses: actions/checkout@v4
- name: Annotate locations with typos
uses: codespell-project/codespell-problem-matcher@v1
- name: Codespell
uses: codespell-project/actions-codespell@v2
4 changes: 2 additions & 2 deletions caddyconfig/caddyfile/lexer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ func (l *lexer) next() (bool, error) {
// want to keep.
if ch == '\n' {
if len(val) == 2 {
return false, fmt.Errorf("missing opening heredoc marker on line #%d; must contain only alpha-numeric characters, dashes and underscores; got empty string", l.line)
return false, fmt.Errorf("missing opening heredoc marker on line #%d; must contain only alphanumeric characters, dashes and underscores; got empty string", l.line)
}

// check if there's too many <
Expand All @@ -165,7 +165,7 @@ func (l *lexer) next() (bool, error) {

heredocMarker = string(val[2:])
if !heredocMarkerRegexp.Match([]byte(heredocMarker)) {
return false, fmt.Errorf("heredoc marker on line #%d must contain only alpha-numeric characters, dashes and underscores; got '%s'", l.line, heredocMarker)
return false, fmt.Errorf("heredoc marker on line #%d must contain only alphanumeric characters, dashes and underscores; got '%s'", l.line, heredocMarker)
}

inHeredoc = true
Expand Down
2 changes: 1 addition & 1 deletion caddyconfig/caddyfile/lexer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,7 @@ EOF
{
input: []byte("not-a-heredoc <<\n"),
expectErr: true,
errorMessage: "missing opening heredoc marker on line #1; must contain only alpha-numeric characters, dashes and underscores; got empty string",
errorMessage: "missing opening heredoc marker on line #1; must contain only alphanumeric characters, dashes and underscores; got empty string",
},
{
input: []byte(`heredoc <<<EOF
Expand Down
2 changes: 1 addition & 1 deletion caddyconfig/httpcaddyfile/tlsapp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ func (st ServerType) buildTLSApp(
tlsApp.Automation.Policies = append(tlsApp.Automation.Policies, catchAllAP)
}

// collect all hosts that have a wildcard in them, and arent HTTP
// collect all hosts that have a wildcard in them, and aren't HTTP
wildcardHosts := []string{}
// hosts that have been explicitly marked to be automated,
// even if covered by another wildcard
Expand Down
2 changes: 1 addition & 1 deletion modules/caddyhttp/httpredirectlistener.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ func (c *httpRedirectConn) Read(p []byte) (int, error) {
// looks like it might've been a misdirected plaintext HTTP request.
func firstBytesLookLikeHTTP(hdr []byte) bool {
switch string(hdr[:5]) {
case "GET /", "HEAD ", "POST ", "PUT /", "OPTIO":
case "GET /", "HEAD ", "POST ", "PUT /", "OPTIO": // codespell:ignore
return true
}
return false
Expand Down
2 changes: 1 addition & 1 deletion modules/caddyhttp/reverseproxy/healthchecks.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -510,7 +510,7 @@ func (h *Handler) doActiveHealthCheck(dialInfo DialInfo, hostAddr string, networ
body = io.LimitReader(body, h.HealthChecks.Active.MaxSize)
}
defer func() {
// drain any remaining body so connection could be re-used
// drain any remaining body so connection could be reused
_, _ = io.Copy(io.Discard, body)
resp.Body.Close()
}()
Expand Down
2 changes: 1 addition & 1 deletion modules/caddyhttp/reverseproxy/streaming.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ func (h *Handler) handleUpgradeResponse(logger *zap.Logger, wg *sync.WaitGroup,
// adopted from https://github.com/golang/go/commit/8bcf2834afdf6a1f7937390903a41518715ef6f5
backConnCloseCh := make(chan struct{})
go func() {
// Ensure that the cancelation of a request closes the backend.
// Ensure that the cancellation of a request closes the backend.
// See issue https://golang.org/issue/35559.
select {
case <-req.Context().Done():
Expand Down