Lint mcap cli

go/Makefile

@@ -7,7 +7,7 @@ test:
 lint:
 	make -C mcap lint
 	make -C ros lint
-	make -C mcap lint
+	make -C cli/mcap lint
 
 build-conformance-binaries:
 	make -C conformance build

go/cli/mcap/cmd/attachment.go

@@ -95,11 +95,13 @@ var getAttachmentCmd = &cobra.Command{
 			case len(attachments[getAttachmentName]) == 0:
 				die("attachment %s not found", getAttachmentName)
 			case len(attachments[getAttachmentName]) == 1:
-				getAttachment(output, rs, attachments[getAttachmentName][0])
+				if err := getAttachment(output, rs, attachments[getAttachmentName][0]); err != nil {
+					die("failed to get attachment: %s", err)
+				}
 			case len(attachments[getAttachmentName]) > 1:
 				if getAttachmentOffset == 0 {
 					return fmt.Errorf(
-						"multiple attachments named %s exist. Specify an offset.",
+						"multiple attachments named %s exist (specify an offset)",
 						getAttachmentName,
 					)
 				}
@@ -179,15 +181,35 @@ var addAttachmentCmd = &cobra.Command{
 func init() {
 	addCmd.AddCommand(addAttachmentCmd)
 	addAttachmentCmd.PersistentFlags().StringVarP(&addAttachmentFilename, "file", "f", "", "filename of attachment to add")
-	addAttachmentCmd.PersistentFlags().StringVarP(&addAttachmentName, "name", "n", "", "name of attachment to add (defaults to filename)")
-	addAttachmentCmd.PersistentFlags().StringVarP(&addAttachmentMediaType, "content-type", "", "application/octet-stream", "content type of attachment")
-	addAttachmentCmd.PersistentFlags().Uint64VarP(&addAttachmentLogTime, "log-time", "", 0, "attachment log time in nanoseconds (defaults to current timestamp)")
-	addAttachmentCmd.PersistentFlags().Uint64VarP(&addAttachmentLogTime, "creation-time", "", 0, "attachment creation time in nanoseconds (defaults to ctime)")
-	addAttachmentCmd.MarkPersistentFlagRequired("file")
+	addAttachmentCmd.PersistentFlags().StringVarP(
+		&addAttachmentName, "name", "n", "", "name of attachment to add (defaults to filename)",
+	)
+	addAttachmentCmd.PersistentFlags().StringVarP(
+		&addAttachmentMediaType, "content-type", "", "application/octet-stream", "content type of attachment",
+	)
+	addAttachmentCmd.PersistentFlags().Uint64VarP(
+		&addAttachmentLogTime, "log-time", "", 0, "attachment log time in nanoseconds (defaults to current timestamp)",
+	)
+	addAttachmentCmd.PersistentFlags().Uint64VarP(
+		&addAttachmentLogTime, "creation-time", "", 0, "attachment creation time in nanoseconds (defaults to ctime)",
+	)
+	err := addAttachmentCmd.MarkPersistentFlagRequired("file")
+	if err != nil {
+		die("failed to mark --file flag as required: %s", err)
+	}
 
 	getCmd.AddCommand(getAttachmentCmd)
 	getAttachmentCmd.PersistentFlags().StringVarP(&getAttachmentName, "name", "n", "", "name of attachment to extract")
 	getAttachmentCmd.PersistentFlags().Uint64VarP(&getAttachmentOffset, "offset", "", 0, "offset of attachment to extract")
-	getAttachmentCmd.PersistentFlags().StringVarP(&getAttachmentOutput, "output", "o", "", "location to write attachment to")
-	getAttachmentCmd.MarkPersistentFlagRequired("name")
+	getAttachmentCmd.PersistentFlags().StringVarP(
+		&getAttachmentOutput,
+		"output",
+		"o",
+		"",
+		"location to write attachment to",
+	)
+	err = getAttachmentCmd.MarkPersistentFlagRequired("name")
+	if err != nil {
+		die("failed to mark --name flag as required: %s", err)
+	}
 }

go/cli/mcap/cmd/cat.go

@@ -39,7 +39,7 @@ func digits(n uint64) int {
 	}
 	count := 0
 	for n != 0 {
-		n = n / 10
+		n /= 10
 		count++
 	}
 	return count
@@ -65,7 +65,6 @@ type Message struct {
 	LogTime     uint64          `json:"log_time"`
 	PublishTime uint64          `json:"publish_time"`
 	Data        json.RawMessage `json:"data"`
-	buf         *bytes.Buffer
 }
 
 type jsonOutputWriter struct {
@@ -88,42 +87,42 @@ func (w *jsonOutputWriter) writeMessage(
 	data []byte,
 ) error {
 	w.buf.Reset()
-	_, err := w.buf.Write([]byte("{"))
+	_, err := w.buf.WriteString("{")
 	if err != nil {
 		return err
 	}
 
-	_, err = w.buf.Write([]byte(`"topic":`))
+	_, err = w.buf.WriteString(`"topic":`)
 	if err != nil {
 		return err
 	}
 
-	_, err = w.buf.Write([]byte(`"`))
+	_, err = w.buf.WriteString(`"`)
 	if err != nil {
 		return err
 	}
 
-	_, err = w.buf.Write([]byte(topic))
+	_, err = w.buf.WriteString(topic)
 	if err != nil {
 		return err
 	}
 
-	_, err = w.buf.Write([]byte(`",`))
+	_, err = w.buf.WriteString(`",`)
 	if err != nil {
 		return err
 	}
 
-	_, err = w.buf.Write([]byte(`"sequence":`))
+	_, err = w.buf.WriteString(`"sequence":`)
 	if err != nil {
 		return err
 	}
 
-	_, err = w.buf.Write([]byte(strconv.FormatUint(uint64(sequence), 10)))
+	_, err = w.buf.WriteString(strconv.FormatUint(uint64(sequence), 10))
 	if err != nil {
 		return err
 	}
 
-	_, err = w.buf.Write([]byte(`,"log_time":`))
+	_, err = w.buf.WriteString(`,"log_time":`)
 	if err != nil {
 		return err
 	}
@@ -133,7 +132,7 @@ func (w *jsonOutputWriter) writeMessage(
 		return err
 	}
 
-	_, err = w.buf.Write([]byte(`,"publish_time":`))
+	_, err = w.buf.WriteString(`,"publish_time":`)
 	if err != nil {
 		return err
 	}
@@ -143,7 +142,7 @@ func (w *jsonOutputWriter) writeMessage(
 		return err
 	}
 
-	_, err = w.buf.Write([]byte(`,"data":`))
+	_, err = w.buf.WriteString(`,"data":`)
 	if err != nil {
 		return err
 	}
@@ -153,7 +152,7 @@ func (w *jsonOutputWriter) writeMessage(
 		return err
 	}
 
-	_, err = w.buf.Write([]byte("}\n"))
+	_, err = w.buf.WriteString("}\n")
 	if err != nil {
 		return err
 	}
@@ -179,7 +178,6 @@ func getReadOpts(useIndex bool) []readopts.ReadOpt {
 }
 
 func printMessages(
-	ctx context.Context,
 	w io.Writer,
 	it mcap.MessageIterator,
 	formatJSON bool,
@@ -213,7 +211,10 @@ func printMessages(
 					return fmt.Errorf("failed to write message bytes: %w", err)
 				}
 			default:
-				return fmt.Errorf("For schema-less channels, JSON output is only supported with 'json' message encoding. Found: %s", channel.MessageEncoding)
+				return fmt.Errorf(
+					"for schema-less channels, JSON output is only supported with 'json' message encoding. found: %s",
+					channel.MessageEncoding,
+				)
 			}
 		} else {
 			switch schema.Encoding {
@@ -250,7 +251,7 @@ func printMessages(
 					messageDescriptor = descriptor.(protoreflect.MessageDescriptor)
 					descriptors[channel.SchemaID] = messageDescriptor
 				}
-				protoMsg := dynamicpb.NewMessage(messageDescriptor.(protoreflect.MessageDescriptor))
+				protoMsg := dynamicpb.NewMessage(messageDescriptor)
 				if err := proto.Unmarshal(message.Data, protoMsg); err != nil {
 					return fmt.Errorf("failed to parse message: %w", err)
 				}
@@ -266,7 +267,10 @@ func printMessages(
 					return fmt.Errorf("failed to write message bytes: %w", err)
 				}
 			default:
-				return fmt.Errorf("JSON output only supported for ros1msg, protobuf, and jsonschema schemas. Found: %s", schema.Encoding)
+				return fmt.Errorf(
+					"JSON output only supported for ros1msg, protobuf, and jsonschema schemas. Found: %s",
+					schema.Encoding,
+				)
 			}
 		}
 		err = jsonWriter.writeMessage(
@@ -277,7 +281,7 @@ func printMessages(
 			msg.Bytes(),
 		)
 		if err != nil {
-			return fmt.Errorf("failed to write encoded message: %s", err)
+			return fmt.Errorf("failed to write encoded message: %w", err)
 		}
 		msg.Reset()
 	}
@@ -310,7 +314,7 @@ var catCmd = &cobra.Command{
 			if err != nil {
 				die("Failed to read messages: %s", err)
 			}
-			err = printMessages(ctx, output, it, catFormatJSON)
+			err = printMessages(output, it, catFormatJSON)
 			if err != nil {
 				die("Failed to print messages: %s", err)
 			}
@@ -332,7 +336,7 @@ var catCmd = &cobra.Command{
 			if err != nil {
 				return fmt.Errorf("failed to read messages: %w", err)
 			}
-			err = printMessages(ctx, output, it, catFormatJSON)
+			err = printMessages(output, it, catFormatJSON)
 			if err != nil {
 				return fmt.Errorf("failed to print messages: %w", err)
 			}

go/cli/mcap/cmd/cat_test.go

@@ -2,17 +2,15 @@ package cmd
 
 import (
 	"bytes"
-	"context"
 	"io"
-	"io/ioutil"
+	"os"
 	"testing"
 
 	"github.com/foxglove/mcap/go/mcap"
 	"github.com/stretchr/testify/assert"
 )
 
 func TestCat(t *testing.T) {
-	ctx := context.Background()
 	cases := []struct {
 		assertion string
 		inputfile string
@@ -25,7 +23,7 @@ func TestCat(t *testing.T) {
 		},
 	}
 	for _, c := range cases {
-		input, err := ioutil.ReadFile(c.inputfile)
+		input, err := os.ReadFile(c.inputfile)
 		assert.Nil(t, err)
 		w := new(bytes.Buffer)
 		r := bytes.NewReader(input)
@@ -35,7 +33,7 @@ func TestCat(t *testing.T) {
 			defer reader.Close()
 			it, err := reader.Messages()
 			assert.Nil(t, err)
-			err = printMessages(ctx, w, it, false)
+			err = printMessages(w, it, false)
 			assert.Nil(t, err)
 			r.Reset(input)
 			assert.Equal(t, c.expected, w.String())
@@ -44,7 +42,6 @@ func TestCat(t *testing.T) {
 }
 
 func BenchmarkCat(b *testing.B) {
-	ctx := context.Background()
 	cases := []struct {
 		assertion  string
 		inputfile  string
@@ -57,20 +54,22 @@ func BenchmarkCat(b *testing.B) {
 		},
 	}
 	for _, c := range cases {
-		input, err := ioutil.ReadFile(c.inputfile)
+		input, err := os.ReadFile(c.inputfile)
 		assert.Nil(b, err)
 		w := io.Discard
 		r := bytes.NewReader(input)
 		b.Run(c.assertion, func(b *testing.B) {
 			for i := 0; i < b.N; i++ {
-				reader, err := mcap.NewReader(r)
-				assert.Nil(b, err)
-				defer reader.Close()
-				it, err := reader.Messages()
-				assert.Nil(b, err)
-				err = printMessages(ctx, w, it, c.formatJSON)
-				assert.Nil(b, err)
-				r.Reset(input)
+				func() {
+					reader, err := mcap.NewReader(r)
+					assert.Nil(b, err)
+					defer reader.Close()
+					it, err := reader.Messages()
+					assert.Nil(b, err)
+					err = printMessages(w, it, c.formatJSON)
+					assert.Nil(b, err)
+					r.Reset(input)
+				}()
 			}
 		})
 	}

go/cli/mcap/cmd/channels.go

@@ -25,7 +25,7 @@ func printChannels(w io.Writer, channels []*mcap.Channel) error {
 	for _, channel := range channels {
 		metadata, err := json.Marshal(channel.Metadata)
 		if err != nil {
-			return fmt.Errorf("failed to marshal channel metadata: %v", err)
+			return fmt.Errorf("failed to marshal channel metadata: %w", err)
 		}
 		row := []string{
 			fmt.Sprintf("%d", channel.ID),
@@ -40,7 +40,7 @@ func printChannels(w io.Writer, channels []*mcap.Channel) error {
 	return nil
 }
 
-// channelsCmd represents the channels command
+// channelsCmd represents the channels command.
 var channelsCmd = &cobra.Command{
 	Use:   "channels",
 	Short: "List channels in an MCAP file",

go/cli/mcap/cmd/chunks.go

@@ -42,7 +42,7 @@ func printChunks(w io.Writer, chunkIndexes []*mcap.ChunkIndex) {
 	utils.FormatTable(w, rows)
 }
 
-// chunksCmd represents the chunks command
+// chunksCmd represents the chunks command.
 var chunksCmd = &cobra.Command{
 	Use:   "chunks",
 	Short: "List chunks in an MCAP file",

go/cli/mcap/cmd/convert.go

@@ -127,7 +127,10 @@ var convertCmd = &cobra.Command{
 			}
 
 			err = ros.Bag2MCAP(bw, f, opts, func(data []byte) error {
-				progressBar.Add64(1)
+				progressBarErr := progressBar.Add64(1)
+				if progressBarErr != nil {
+					die("failed to increment progressbar: %s", err)
+				}
 				return nil
 			})
 			if err != nil && !errors.Is(err, io.EOF) {
@@ -154,7 +157,10 @@ var convertCmd = &cobra.Command{
 			}
 			dirs := strings.FieldsFunc(amentPath, func(c rune) bool { return (c == os.PathListSeparator) })
 			err = ros.DB3ToMCAP(bw, db, opts, dirs, func(b []byte) error {
-				progressBar.Add(1)
+				progressBarErr := progressBar.Add64(1)
+				if progressBarErr != nil {
+					die("failed to increment progressbar: %s", err)
+				}
 				return nil
 			})
 			if err != nil {