tinylib / msgp

A Go code generator for MessagePack / msgpack.org[Go]
MIT License
1.77k stars 189 forks source link

all: remove dead code and fix linting issues #320

Closed thaJeztah closed 1 year ago

thaJeztah commented 1 year ago

gen: remove strtoMeth() (unused)

This function was added in 9b8e5157921bd8b18ebbd15b121e383379d0c209, but looks to be unused.

gen/spec.go:65:6: func `strtoMeth` is unused (unused)
func strtoMeth(s string) Method {
     ^

msgp: remove (Writer).writeVal, (Writer).writeStruct (unused)

msgp: remove (*Writer).writeVal

This method was added in c1ffc473f483bc8c1e9619d0a86eaee0237759d9, when it was called by MsgWriter.Encode().

This method was refactored in 8fb918a939a73cd46449037189c826778548885b, which inlined the logic, and renamed it to MsgWriter.WriteIntf.

msgp/write.go:763:19: func `(*Writer).writeVal` is unused (unused)
func (mw *Writer) writeVal(v reflect.Value) error {
                  ^

msgp: remove (*Writer).writeStruct (unused)

This method was also added in c1ffc473f483bc8c1e9619d0a86eaee0237759d9, when it was used by (Writer).writeVal and MsgWriter.WriteIntf. Commit 8fb918a939a73cd46449037189c826778548885b removed its use from WriteIntf, and with the removal of (Writer).writeVal, there are no remaining uses.

msgp/write.go:756:19: func `(*Writer).writeStruct` is unused (unused)
func (mw *Writer) writeStruct(v reflect.Value) error {
                  ^

msgp: remove rwFloatBytes (unused)

This function was added in a62e40321a91c38cdcb58138ec58db28f52572c4, but later split to dedicated rwFloat32Bytes and rwFloat64Bytes functions in 293864c19beb3d1bb4dfb04a55d0da43aab6a14e.

msgp/json_bytes.go:226:6: func `rwFloatBytes` is unused (unused)
func rwFloatBytes(w jsWriter, msg []byte, f64 bool, scratch []byte) ([]byte, []byte, error) {
     ^

all: fix linting issues (gosimple, staticcheck)

Fixing some minor linting issues

msgp/read_test.go:367:3: S1008: should use 'return bits == failBits' instead of 'if bits == failBits { return true }; return false' (gosimple)
        if bits == failBits {
        ^
gen/decode.go:66:2: S1023: redundant `return` statement (gosimple)
    return
    ^
gen/encode.go:81:2: S1023: redundant `return` statement (gosimple)
    return
    ^
gen/marshal.go:87:2: S1023: redundant `return` statement (gosimple)
    return
    ^
msgp/elsize.go:122:2: SA9004: only the first constant in this group has an explicit type (staticcheck)
    constsize varmode = 0  // constant size (size bytes + uint8(varmode) objects)
    ^
msgp/file_test.go:51:12: SA1019: os.SEEK_SET has been deprecated since Go 1.7: Use io.SeekStart, io.SeekCurrent, and io.SeekEnd. (staticcheck)
    f.Seek(0, os.SEEK_SET)
              ^

msgp: BenchmarkWriteReadFile: ignore SA3001 (staticcheck)

Perhaps it's not "good" practice to do this, but from the looks of it, it looks like not doing this would be worse. Alternatively, we could panic() in case the chosen value would potentially DOS the system.

From https://staticcheck.io/docs/checks#SA3001

SA3001 - Assigning to b.N in benchmarks distorts the results

The testing package dynamically sets b.N to improve the reliability of benchmarks and uses it in computations to determine the duration of a single operation. Benchmark code must not alter b.N as this would falsify results.

msgp/file_test.go:71:3: SA3001: should not assign to b.N (staticcheck)
        b.N = 10000000
        ^
thaJeztah commented 1 year ago

/cc @philhofer ptal 🤗 (got these when I was trying to set up github actions)