Skip to content

Commit

Permalink
chore(fmt/linters): Add gofumpt formatter and linter (celestiaorg#3331)
Browse files Browse the repository at this point in the history
This PR enables stricter formatting by integrating `gofumpt`, which
imposes more rules than the default gofmt, allowing for improved code
formatting. The results, visible in this PR, include minor yet
significant cleanups such as the removal of extra new lines and the
consistent declaration of variables and constants.Integrating `gofumpt`
will be a valuable addition to our code formatting automation, ensuring
higher code quality and consistency.Additionally, the PR updates the
Makefile and linter rules to include this enhanced formatting and
applies it to the codebase.

(cherry picked from commit e53e612)
  • Loading branch information
walldiss committed May 7, 2024
1 parent c0698c2 commit 96d7957
Show file tree
Hide file tree
Showing 55 changed files with 107 additions and 115 deletions.
3 changes: 3 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ linters:
# - gocyclo
# - godox
- gofmt
- gofumpt
- goimports
# - golint - deprecated since v1.41. revive will be used instead
- revive
Expand Down Expand Up @@ -67,3 +68,5 @@ linters-settings:
local-prefixes: github.com/celestiaorg/celestia-node
dupl:
threshold: 200
gofumpt:
extra-rules: true
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,9 @@ install-key:
## fmt: Formats only *.go (excluding *.pb.go *pb_test.go). Runs `gofmt & goimports` internally.
fmt: sort-imports
@find . -name '*.go' -type f -not -path "*.git*" -not -name '*.pb.go' -not -name '*pb_test.go' | xargs gofmt -w -s
@find . -name '*.go' -type f -not -path "*.git*" -not -name '*.pb.go' -not -name '*pb_test.go' | xargs goimports -w -local github.com/celestiaorg
@go mod tidy -compat=1.20
@cfmt -w -m=100 ./...
@gofumpt -w -extra .
@markdownlint --fix --quiet --config .markdownlint.yaml .
.PHONY: fmt

Expand Down
2 changes: 1 addition & 1 deletion api/docgen/openrpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ func ParseCommentsFromNodebuilderModules(moduleNames ...string) (Comments, Comme
return nodeComments, permComments
}

func NewOpenRPCDocument(comments Comments, permissions Comments) *go_openrpc_reflect.Document {
func NewOpenRPCDocument(comments, permissions Comments) *go_openrpc_reflect.Document {
d := &go_openrpc_reflect.Document{}

d.WithMeta(&go_openrpc_reflect.MetaT{
Expand Down
3 changes: 1 addition & 2 deletions api/gateway/bindings_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ func TestRegisterEndpoints(t *testing.T) {
}
}

func hasEndpointRegistered(router *mux.Router, path string, method string) bool {
func hasEndpointRegistered(router *mux.Router, path, method string) bool {
var registered bool
err := router.Walk(func(route *mux.Route, router *mux.Router, ancestors []*mux.Route) error {
template, err := route.GetPathTemplate()
Expand All @@ -109,7 +109,6 @@ func hasEndpointRegistered(router *mux.Router, path string, method string) bool
}
return nil
})

if err != nil {
fmt.Println("Error walking through routes:", err)
return false
Expand Down
4 changes: 1 addition & 3 deletions api/gateway/header.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,7 @@ const (
headerByHeightEndpoint = "/header"
)

var (
heightKey = "height"
)
var heightKey = "height"

func (h *Handler) handleHeadRequest(w http.ResponseWriter, r *http.Request) {
head, err := h.header.LocalHead(r.Context())
Expand Down
2 changes: 1 addition & 1 deletion api/rpc/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func (c *Client) Close() {

// NewClient creates a new Client with one connection per namespace with the
// given token as the authorization token.
func NewClient(ctx context.Context, addr string, token string) (*Client, error) {
func NewClient(ctx context.Context, addr, token string) (*Client, error) {
authHeader := http.Header{perms.AuthKey: []string{fmt.Sprintf("Bearer %s", token)}}
return newClient(ctx, addr, authHeader)
}
Expand Down
2 changes: 1 addition & 1 deletion api/rpc/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func (s *Server) verifyAuth(_ context.Context, token string) ([]auth.Permission,

// RegisterService registers a service onto the RPC server. All methods on the service will then be
// exposed over the RPC.
func (s *Server) RegisterService(namespace string, service interface{}, out interface{}) {
func (s *Server) RegisterService(namespace string, service, out interface{}) {
if s.authDisabled {
s.rpc.Register(namespace, service)
return
Expand Down
3 changes: 1 addition & 2 deletions api/rpc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ func TestAuthedRPC(t *testing.T) {
adminToken, err := perms.NewTokenWithPerms(signer, perms.AllPerms)
require.NoError(t, err)

var tests = []struct {
tests := []struct {
perm int
token string
}{
Expand Down Expand Up @@ -280,7 +280,6 @@ func implementsMarshaler(t *testing.T, typ reflect.Type) {
default:
return
}

}

// setupNodeWithAuthedRPC sets up a node and overrides its JWT
Expand Down
2 changes: 1 addition & 1 deletion blob/blob_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func TestBlob(t *testing.T) {
blob, err := convertBlobs(appBlobs...)
require.NoError(t, err)

var test = []struct {
test := []struct {
name string
expectedRes func(t *testing.T)
}{
Expand Down
3 changes: 2 additions & 1 deletion blob/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ type parser struct {
verifyFn func(blob *Blob) bool
}

// NOTE: passing shares here needed to detect padding shares(as we do not need this check in addShares)
// NOTE: passing shares here needed to detect padding shares(as we do not need this check in
// addShares)
func (p *parser) set(index int, shrs []shares.Share) ([]shares.Share, error) {
if len(shrs) == 0 {
return nil, errEmptyShares
Expand Down
3 changes: 1 addition & 2 deletions blob/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func TestBlobService_Get(t *testing.T) {
require.NoError(t, err)

service := createService(ctx, t, append(blobs0, blobs1...))
var test = []struct {
test := []struct {
name string
doFn func() (interface{}, error)
expectedResult func(interface{}, error)
Expand Down Expand Up @@ -304,7 +304,6 @@ func TestBlobService_Get(t *testing.T) {
assert.Empty(t, blobs)
require.Error(t, err)
require.ErrorIs(t, err, ErrBlobNotFound)

},
},
{
Expand Down
3 changes: 1 addition & 2 deletions cmd/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (
)

func AuthCmd(fsets ...*flag.FlagSet) *cobra.Command {
var cmd = &cobra.Command{
cmd := &cobra.Command{
Use: "auth [permission-level (e.g. read || write || admin)]",
Short: "Signs and outputs a hex-encoded JWT token with the given permissions.",
Long: "Signs and outputs a hex-encoded JWT token with the given permissions. NOTE: only use this command when " +
Expand All @@ -37,7 +37,6 @@ func AuthCmd(fsets ...*flag.FlagSet) *cobra.Command {
ks, err := newKeystore(StorePath(cmd.Context()))
if err != nil {
return err

}

key, err := ks.Get(nodemod.SecretName)
Expand Down
2 changes: 1 addition & 1 deletion core/fetcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ func (f *BlockFetcher) Commit(ctx context.Context, height *int64) (*types.Commit
// ValidatorSet queries Core for the ValidatorSet from the
// block at the given height.
func (f *BlockFetcher) ValidatorSet(ctx context.Context, height *int64) (*types.ValidatorSet, error) {
var perPage = 100
perPage := 100

vals, total := make([]*types.Validator, 0), -1
for page := 1; len(vals) != total; page++ {
Expand Down
2 changes: 1 addition & 1 deletion core/listener.go
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ func (cl *Listener) handleNewSignedBlock(ctx context.Context, b types.EventDataS
if err != nil && !errors.Is(err, context.Canceled) {
log.Errorw("listener: broadcasting data hash",
"height", b.Header.Height,
"hash", b.Header.Hash(), "err", err) //TODO: hash or datahash?
"hash", b.Header.Hash(), "err", err) // TODO: hash or datahash?
}
}

Expand Down
3 changes: 2 additions & 1 deletion das/backoff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ func Test_exponentialBackoff(t *testing.T) {
4 * time.Minute,
16 * time.Minute,
64 * time.Minute,
}},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down
6 changes: 4 additions & 2 deletions das/daser.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,10 @@ type DASer struct {
running int32
}

type listenFn func(context.Context, *header.ExtendedHeader)
type sampleFn func(context.Context, *header.ExtendedHeader) error
type (
listenFn func(context.Context, *header.ExtendedHeader)
sampleFn func(context.Context, *header.ExtendedHeader) error
)

// NewDASer creates a new DASer.
func NewDASer(
Expand Down
9 changes: 5 additions & 4 deletions das/daser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ func TestDASer_SamplingWindow(t *testing.T) {
WithSamplingWindow(time.Second))
require.NoError(t, err)

var tests = []struct {
tests := []struct {
timestamp time.Time
withinWindow bool
}{
Expand All @@ -282,7 +282,6 @@ func TestDASer_SamplingWindow(t *testing.T) {
assert.Equal(t, tt.withinWindow, daser.isWithinSamplingWindow(eh))
})
}

}

// createDASerSubcomponents takes numGetter (number of headers
Expand Down Expand Up @@ -403,7 +402,8 @@ type benchGetterStub struct {

func newBenchGetter() benchGetterStub {
return benchGetterStub{header: &header.ExtendedHeader{
DAH: &share.Root{RowRoots: make([][]byte, 0)}}}
DAH: &share.Root{RowRoots: make([][]byte, 0)},
}}
}

func (m benchGetterStub) GetByHeight(context.Context, uint64) (*header.ExtendedHeader, error) {
Expand All @@ -423,7 +423,8 @@ func (m getterStub) GetByHeight(_ context.Context, height uint64) (*header.Exten
return &header.ExtendedHeader{
Commit: &types.Commit{},
RawHeader: header.RawHeader{Height: int64(height)},
DAH: &share.Root{RowRoots: make([][]byte, 0)}}, nil
DAH: &share.Root{RowRoots: make([][]byte, 0)},
}, nil
}

func (m getterStub) GetRangeByHeight(
Expand Down
2 changes: 1 addition & 1 deletion das/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ var ErrInvalidOption = errors.New("das: invalid option")

// errInvalidOptionValue is a utility function to dedup code for error-returning
// when dealing with invalid parameter values
func errInvalidOptionValue(optionName string, value string) error {
func errInvalidOptionValue(optionName, value string) error {
return fmt.Errorf("%w: value %s cannot be %s", ErrInvalidOption, optionName, value)
}

Expand Down
6 changes: 4 additions & 2 deletions das/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ type checkpointStore struct {
func newCheckpointStore(ds datastore.Datastore) checkpointStore {
return checkpointStore{
namespace.Wrap(ds, storePrefix),
newDone("checkpoint store")}
newDone("checkpoint store"),
}
}

// load loads the DAS checkpoint from disk and returns it.
Expand Down Expand Up @@ -65,7 +66,8 @@ func (s *checkpointStore) store(ctx context.Context, cp checkpoint) error {
func (s *checkpointStore) runBackgroundStore(
ctx context.Context,
storeInterval time.Duration,
getCheckpoint func(ctx context.Context) (checkpoint, error)) {
getCheckpoint func(ctx context.Context) (checkpoint, error),
) {
defer s.indicateDone()

// runBackgroundStore could be disabled by setting storeInterval = 0
Expand Down
1 change: 1 addition & 0 deletions header/headertest/fraud/testing.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ func (f *FraudMaker) MakeExtendedHeader(odsSize int, edsStore *eds.Store) header
return header.MakeExtendedHeader(h, comm, vals, eds)
}
}

func CreateFraudExtHeader(
t *testing.T,
eh *header.ExtendedHeader,
Expand Down
6 changes: 3 additions & 3 deletions libs/keystore/fs_keystore.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ type fsKeystore struct {
// NewFSKeystore creates a new Keystore over OS filesystem.
// The path must point to a directory. It is created automatically if necessary.
func NewFSKeystore(path string, ring keyring.Keyring) (Keystore, error) {
err := os.Mkdir(path, 0755)
err := os.Mkdir(path, 0o755)
if err != nil && !os.IsExist(err) {
return nil, fmt.Errorf("keystore: failed to make a dir: %w", err)
}
Expand All @@ -49,7 +49,7 @@ func (f *fsKeystore) Put(n KeyName, pk PrivKey) error {
return fmt.Errorf("keystore: failed to marshal key '%s': %w", n, err)
}

err = os.WriteFile(path, data, 0600)
err = os.WriteFile(path, data, 0o600)
if err != nil {
return fmt.Errorf("keystore: failed to write key '%s': %w", n, err)
}
Expand Down Expand Up @@ -138,7 +138,7 @@ func (f *fsKeystore) pathTo(file string) string {
}

func checkPerms(perms os.FileMode) error {
if perms&0077 != 0 {
if perms&0o077 != 0 {
return fmt.Errorf("required: 0600, got: %#o", perms)
}
return nil
Expand Down
4 changes: 2 additions & 2 deletions libs/utils/address_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
)

func TestSanitizeAddr(t *testing.T) {
var tests = []struct {
tests := []struct {
addr string
want string
err error
Expand Down Expand Up @@ -39,7 +39,7 @@ func TestValidateAddr(t *testing.T) {
addr string
unresolved bool
}
var tests = []struct {
tests := []struct {
addr string
want want
}{
Expand Down
2 changes: 1 addition & 1 deletion nodebuilder/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ func UpdateConfig(tp node.Type, path string) (err error) {

// updateConfig merges new values from the new config into the old
// config, returning the updated old config.
func updateConfig(oldCfg *Config, newCfg *Config) (*Config, error) {
func updateConfig(oldCfg, newCfg *Config) (*Config, error) {
err := mergo.Merge(oldCfg, newCfg, mergo.WithOverrideEmptySlice)
return oldCfg, err
}
Expand Down
2 changes: 1 addition & 1 deletion nodebuilder/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ func IsInit(path string) bool {
return false
}

const perms = 0755
const perms = 0o755

// initRoot initializes(creates) directory if not created and check if it is writable
func initRoot(path string) error {
Expand Down
7 changes: 3 additions & 4 deletions nodebuilder/node_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import (
)

func TestLifecycle(t *testing.T) {
var test = []struct {
test := []struct {
tp node.Type
}{
{tp: node.Bridge},
Expand Down Expand Up @@ -58,7 +58,7 @@ func TestLifecycle_WithMetrics(t *testing.T) {

otelCollectorURL := strings.ReplaceAll(url, "http://", "")

var test = []struct {
test := []struct {
tp node.Type
coreExpected bool
}{
Expand Down Expand Up @@ -130,7 +130,7 @@ func TestEmptyBlockExists(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

var test = []struct {
test := []struct {
tp node.Type
}{
{tp: node.Bridge},
Expand All @@ -155,5 +155,4 @@ func TestEmptyBlockExists(t *testing.T) {
require.NoError(t, err)
})
}

}
2 changes: 1 addition & 1 deletion nodebuilder/p2p/addrs.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ func Listen(listen []string) func(h hst.Host) (err error) {
}

// addrsFactory returns a constructor for AddrsFactory.
func addrsFactory(announce []string, noAnnounce []string) func() (_ p2pconfig.AddrsFactory, err error) {
func addrsFactory(announce, noAnnounce []string) func() (_ p2pconfig.AddrsFactory, err error) {
return func() (_ p2pconfig.AddrsFactory, err error) {
// Convert maAnnounce strings to Multiaddresses
maAnnounce := make([]ma.Multiaddr, len(announce))
Expand Down
Loading

0 comments on commit 96d7957

Please sign in to comment.