Merge pull request #26 from mewkiz/golangci

Add golangci metalinter
2018-05-28 10:48:39 +09:00 · 2018-05-28 10:48:39 +09:00 · fa00a54cd7
parent fcab6ed7a2 8e870c4eed
commit fa00a54cd7
7 changed files with 242 additions and 193 deletions
--- a/.travis.yml
+++ b/.travis.yml
@ -11,10 +11,11 @@ env:
    - PATH=$HOME/gopath/bin:$PATH

 before_install:
-  - go get golang.org/x/tools/cmd/cover
-  - go get golang.org/x/tools/cmd/goimports
-  - go get github.com/golang/lint/golint
-  - go get github.com/mattn/goveralls
+  - go get -u golang.org/x/tools/cmd/cover
+  - go get -u golang.org/x/tools/cmd/goimports
+  - go get -u github.com/golang/lint/golint
+  - go get -u github.com/mattn/goveralls
+  - go get -u github.com/golangci/golangci-lint/cmd/golangci-lint

 install:
  - go get ./...
@ -25,3 +26,6 @@ before_script:

 script:
  - ./goclean.sh
+  - golangci-lint run --enable-all --disable maligned --exclude "(G501:|Error return value of .md5sum\.Write. is not checked)"
+
+# skip G501: Import blacklist: crypto/md5
--- a/cmd/go-metaflac/metaflac.go
+++ b/cmd/go-metaflac/metaflac.go
@ -79,22 +79,8 @@ func list(path string) error {
 		return err
 	}

-	if blockNums != nil {
-		// Only list blocks specified in the "--block-number" command line flag.
-		for _, blockNum := range blockNums {
-			if blockNum == 0 {
-				listStreamInfo(stream.Info)
-			} else {
-				// strea.Blocks doesn't contain StreamInfo, therefore the blockNum
-				// is one less.
-				blockNum--
-			}
-			if blockNum < len(stream.Blocks) {
-				listBlock(stream.Blocks[blockNum], blockNum)
-			}
-		}
-	} else {
-		// List all blocks.
+	// List all blocks.
+	if blockNums == nil {
 		var isLast bool
 		if len(stream.Blocks) == 0 {
 			isLast = true
@ -107,8 +93,22 @@ func list(path string) error {
 			blockNum--
 			listBlock(block, blockNum)
 		}
+		return nil
 	}

+	// Only list blocks specified in the "--block-number" command line flag.
+	for _, blockNum := range blockNums {
+		if blockNum == 0 {
+			listStreamInfo(stream.Info)
+		} else {
+			// strea.Blocks doesn't contain StreamInfo, therefore the blockNum
+			// is one less.
+			blockNum--
+		}
+		if blockNum < len(stream.Blocks) {
+			listBlock(stream.Blocks[blockNum], blockNum)
+		}
+	}
 	return nil
 }

--- a/enc.go
+++ b/enc.go
@ -175,7 +175,7 @@ func (enc *encoder) writeStreamInfo(hdr meta.Header, si *meta.StreamInfo) error
 	}

 	// 36 bits: NSamples.
-	if err := enc.bw.WriteBits(uint64(si.NSamples), 36); err != nil {
+	if err := enc.bw.WriteBits(si.NSamples, 36); err != nil {
 		return errutil.Err(err)
 	}

@ -540,7 +540,7 @@ func (enc *encoder) writePicture(hdr meta.Header, pic *meta.Picture) error {
 	}

 	// (data length) bytes: Data.
-	if _, err := enc.bw.Write([]byte(pic.Data)); err != nil {
+	if _, err := enc.bw.Write(pic.Data); err != nil {
 		return errutil.Err(err)
 	}

--- a/frame/frame.go
+++ b/frame/frame.go
@ -121,7 +121,7 @@ func (frame *Frame) Parse() error {
 		}

 		// Parse subframe.
-		frame.Subframes[channel], err = frame.parseSubframe(bps)
+		frame.Subframes[channel], err = frame.parseSubframe(frame.br, bps)
 		if err != nil {
 			return err
 		}
@ -215,9 +215,11 @@ func (frame *Frame) parseHeader() error {
 	h := crc8.NewATM()
 	hr := io.TeeReader(frame.hr, h)

-	// 14 bits: sync-code (11111111111110)
+	// Create bit reader.
 	br := bits.NewReader(hr)
 	frame.br = br
+
+	// 14 bits: sync-code (11111111111110)
 	x, err := br.Read(14)
 	if err != nil {
 		// This is the only place an audio frame may return io.EOF, which signals
@ -260,38 +262,65 @@ func (frame *Frame) parseHeader() error {
 		return unexpected(err)
 	}

-	// 4 bits: Channels.
-	//
-	// The 4 bits are used to specify the channels as follows:
-	//    0000: (1 channel) mono.
-	//    0001: (2 channels) left, right.
-	//    0010: (3 channels) left, right, center.
-	//    0011: (4 channels) left, right, left surround, right surround.
-	//    0100: (5 channels) left, right, center, left surround, right surround.
-	//    0101: (6 channels) left, right, center, LFE, left surround, right surround.
-	//    0110: (7 channels) left, right, center, LFE, center surround, side left, side right.
-	//    0111: (8 channels) left, right, center, LFE, left surround, right surround, side left, side right.
-	//    1000: (2 channels) left, side; using inter-channel decorrelation.
-	//    1001: (2 channels) side, right; using inter-channel decorrelation.
-	//    1010: (2 channels) mid, side; using inter-channel decorrelation.
-	//    1011: reserved.
-	//    1100: reserved.
-	//    1101: reserved.
-	//    1111: reserved.
-	x, err = br.Read(4)
-	if err != nil {
-		return unexpected(err)
+	// Parse channels.
+	if err := frame.parseChannels(br); err != nil {
+		return err
 	}
-	if x >= 0xB {
-		return fmt.Errorf("frame.Frame.parseHeader: reserved channels bit pattern (%04b)", x)
-	}
-	frame.Channels = Channels(x)

-	// 3 bits: BitsPerSample.
-	x, err = br.Read(3)
+	// Parse bits per sample.
+	if err := frame.parseBitsPerSample(br); err != nil {
+		return err
+	}
+
+	// 1 bit: reserved.
+	x, err = br.Read(1)
 	if err != nil {
 		return unexpected(err)
 	}
+	if x != 0 {
+		return errors.New("frame.Frame.parseHeader: non-zero reserved value")
+	}
+
+	// if (fixed block size)
+	//    1-6 bytes: UTF-8 encoded frame number.
+	// else
+	//    1-7 bytes: UTF-8 encoded sample number.
+	frame.Num, err = decodeUTF8Int(hr)
+	if err != nil {
+		return unexpected(err)
+	}
+
+	// Parse block size.
+	if err := frame.parseBlockSize(br, blockSize); err != nil {
+		return err
+	}
+
+	// Parse sample rate.
+	if err := frame.parseSampleRate(br, sampleRate); err != nil {
+		return err
+	}
+
+	// 1 byte: CRC-8 checksum.
+	var want uint8
+	if err = binary.Read(frame.hr, binary.BigEndian, &want); err != nil {
+		return unexpected(err)
+	}
+	got := h.Sum8()
+	if want != got {
+		return fmt.Errorf("frame.Frame.parseHeader: CRC-8 checksum mismatch; expected %v, got %v", want, got)
+	}
+
+	return nil
+}
+
+// parseBitsPerSample parses the bits per sample of the header.
+func (frame *Frame) parseBitsPerSample(br *bits.Reader) error {
+	// 3 bits: BitsPerSample.
+	x, err := br.Read(3)
+	if err != nil {
+		return unexpected(err)
+	}
+
 	// The 3 bits are used to specify the sample size as follows:
 	//    000: unknown sample size; get from StreamInfo.
 	//    001: 8 bits-per-sample.
@ -328,27 +357,42 @@ func (frame *Frame) parseHeader() error {
 		// 111: reserved.
 		return fmt.Errorf("frame.Frame.parseHeader: reserved sample size bit pattern (%03b)", x)
 	}
+	return nil
+}

-	// 1 bit: reserved.
-	x, err = br.Read(1)
-	if err != nil {
-		return unexpected(err)
-	}
-	if x != 0 {
-		return errors.New("frame.Frame.parseHeader: non-zero reserved value")
-	}
-
-	// if (fixed block size)
-	//    1-6 bytes: UTF-8 encoded frame number.
-	// else
-	//    1-7 bytes: UTF-8 encoded sample number.
-	frame.Num, err = decodeUTF8Int(hr)
-	if err != nil {
-		return unexpected(err)
-	}
-
-	// Parse block size.
+// parseChannels parses the channels of the header.
+func (frame *Frame) parseChannels(br *bits.Reader) error {
+	// 4 bits: Channels.
 	//
+	// The 4 bits are used to specify the channels as follows:
+	//    0000: (1 channel) mono.
+	//    0001: (2 channels) left, right.
+	//    0010: (3 channels) left, right, center.
+	//    0011: (4 channels) left, right, left surround, right surround.
+	//    0100: (5 channels) left, right, center, left surround, right surround.
+	//    0101: (6 channels) left, right, center, LFE, left surround, right surround.
+	//    0110: (7 channels) left, right, center, LFE, center surround, side left, side right.
+	//    0111: (8 channels) left, right, center, LFE, left surround, right surround, side left, side right.
+	//    1000: (2 channels) left, side; using inter-channel decorrelation.
+	//    1001: (2 channels) side, right; using inter-channel decorrelation.
+	//    1010: (2 channels) mid, side; using inter-channel decorrelation.
+	//    1011: reserved.
+	//    1100: reserved.
+	//    1101: reserved.
+	//    1111: reserved.
+	x, err := br.Read(4)
+	if err != nil {
+		return unexpected(err)
+	}
+	if x >= 0xB {
+		return fmt.Errorf("frame.Frame.parseHeader: reserved channels bit pattern (%04b)", x)
+	}
+	frame.Channels = Channels(x)
+	return nil
+}
+
+// parseBlockSize parses the block size of the header.
+func (frame *Frame) parseBlockSize(br *bits.Reader, blockSize uint64) error {
 	// The 4 bits of n are used to specify the block size as follows:
 	//    0000: reserved.
 	//    0001: 192 samples.
@ -371,14 +415,14 @@ func (frame *Frame) parseHeader() error {
 		frame.BlockSize = 576 * (1 << (n - 2))
 	case n == 0x6:
 		// 0110: get 8 bit (block size)-1 from the end of the header.
-		x, err = br.Read(8)
+		x, err := br.Read(8)
 		if err != nil {
 			return unexpected(err)
 		}
 		frame.BlockSize = uint16(x + 1)
 	case n == 0x7:
 		// 0111: get 16 bit (block size)-1 from the end of the header.
-		x, err = br.Read(16)
+		x, err := br.Read(16)
 		if err != nil {
 			return unexpected(err)
 		}
@ -387,9 +431,11 @@ func (frame *Frame) parseHeader() error {
 		//    1000-1111: 256 * 2^(n-8) samples.
 		frame.BlockSize = 256 * (1 << (n - 8))
 	}
+	return nil
+}

-	// Parse sample rate.
-	//
+// parseSampleRate parses the sample rate of the header.
+func (frame *Frame) parseSampleRate(br *bits.Reader, sampleRate uint64) error {
 	// The 4 bits are used to specify the sample rate as follows:
 	//    0000: unknown sample rate; get from StreamInfo.
 	//    0001: 88.2 kHz.
@ -449,7 +495,7 @@ func (frame *Frame) parseHeader() error {
 		frame.SampleRate = 96000
 	case 0xC:
 		// 1100: get 8 bit sample rate (in kHz) from the end of the header.
-		x, err = br.Read(8)
+		x, err := br.Read(8)
 		if err != nil {
 			return unexpected(err)
 		}
@ -458,14 +504,14 @@ func (frame *Frame) parseHeader() error {
 		log.Printf("frame.Frame.parseHeader: The flac library test cases do not yet include any audio files with sample rate %d. If possible please consider contributing this audio sample to improve the reliability of the test cases.", frame.SampleRate)
 	case 0xD:
 		// 1101: get 16 bit sample rate (in Hz) from the end of the header.
-		x, err = br.Read(16)
+		x, err := br.Read(16)
 		if err != nil {
 			return unexpected(err)
 		}
 		frame.SampleRate = uint32(x)
 	case 0xE:
 		// 1110: get 16 bit sample rate (in daHz) from the end of the header.
-		x, err = br.Read(16)
+		x, err := br.Read(16)
 		if err != nil {
 			return unexpected(err)
 		}
@ -476,17 +522,6 @@ func (frame *Frame) parseHeader() error {
 		// 1111: invalid.
 		return errors.New("frame.Frame.parseHeader: invalid sample rate bit pattern (1111)")
 	}
-
-	// 1 byte: CRC-8 checksum.
-	var want uint8
-	if err = binary.Read(frame.hr, binary.BigEndian, &want); err != nil {
-		return unexpected(err)
-	}
-	got := h.Sum8()
-	if got != want {
-		return fmt.Errorf("frame.Frame.parseHeader: CRC-8 checksum mismatch; expected %v, got %v", want, got)
-	}
-
 	return nil
 }

--- a/frame/subframe.go
+++ b/frame/subframe.go
@ -28,10 +28,10 @@ type Subframe struct {

 // parseSubframe reads and parses the header, and the audio samples of a
 // subframe.
-func (frame *Frame) parseSubframe(bps uint) (subframe *Subframe, err error) {
+func (frame *Frame) parseSubframe(br *bits.Reader, bps uint) (subframe *Subframe, err error) {
 	// Parse subframe header.
 	subframe = new(Subframe)
-	if err = subframe.parseHeader(frame.br); err != nil {
+	if err = subframe.parseHeader(br); err != nil {
 		return subframe, err
 	}
 	// Adjust bps of subframe for wasted bits-per-sample.
@ -42,13 +42,13 @@ func (frame *Frame) parseSubframe(bps uint) (subframe *Subframe, err error) {
 	subframe.Samples = make([]int32, 0, subframe.NSamples)
 	switch subframe.Pred {
 	case PredConstant:
-		err = subframe.decodeConstant(frame.br, bps)
+		err = subframe.decodeConstant(br, bps)
 	case PredVerbatim:
-		err = subframe.decodeVerbatim(frame.br, bps)
+		err = subframe.decodeVerbatim(br, bps)
 	case PredFixed:
-		err = subframe.decodeFixed(frame.br, bps)
+		err = subframe.decodeFixed(br, bps)
 	case PredFIR:
-		err = subframe.decodeFIR(frame.br, bps)
+		err = subframe.decodeFIR(br, bps)
 	}

 	// Left shift to accout for wasted bits-per-sample.
--- a/frame/utf8.go
+++ b/frame/utf8.go
@ -8,7 +8,6 @@ import (
 )

 const (
-	t1 = 0x00 // 0000 0000
 	tx = 0x80 // 1000 0000
 	t2 = 0xC0 // 1100 0000
 	t3 = 0xE0 // 1110 0000
--- a/meta/cuesheet.go
+++ b/meta/cuesheet.go
@ -34,9 +34,10 @@ func (block *Block) parseCueSheet() error {
 	if err != nil {
 		return unexpected(err)
 	}
-	cs := new(CueSheet)
+	cs := &CueSheet{
+		MCN: stringFromSZ(buf),
+	}
 	block.Body = cs
-	cs.MCN = stringFromSZ(buf)

 	// 64 bits: NLeadInSamples.
 	if err = binary.Read(block.lr, binary.BigEndian, &cs.NLeadInSamples); err != nil {
@ -45,7 +46,7 @@ func (block *Block) parseCueSheet() error {

 	// 1 bit: IsCompactDisc.
 	var x uint8
-	if err = binary.Read(block.lr, binary.BigEndian, &x); err != nil {
+	if err := binary.Read(block.lr, binary.BigEndian, &x); err != nil {
 		return unexpected(err)
 	}
 	// mask = 10000000
@ -60,14 +61,13 @@ func (block *Block) parseCueSheet() error {
 	}
 	lr := io.LimitReader(block.lr, 258)
 	zr := zeros{r: lr}
-	_, err = io.Copy(ioutil.Discard, zr)
-	if err != nil {
+	if _, err := io.Copy(ioutil.Discard, zr); err != nil {
 		return err
 	}

 	// Parse cue sheet tracks.
 	// 8 bits: (number of tracks)
-	if err = binary.Read(block.lr, binary.BigEndian, &x); err != nil {
+	if err := binary.Read(block.lr, binary.BigEndian, &x); err != nil {
 		return unexpected(err)
 	}
 	if x < 1 {
@ -81,111 +81,122 @@ func (block *Block) parseCueSheet() error {
 	// track.
 	uniq := make(map[uint8]struct{})
 	for i := range cs.Tracks {
-		// 64 bits: Offset.
-		track := &cs.Tracks[i]
-		if err = binary.Read(block.lr, binary.BigEndian, &track.Offset); err != nil {
-			return unexpected(err)
+		if err := block.parseTrack(cs, i, uniq); err != nil {
+			return err
 		}
-		if cs.IsCompactDisc && track.Offset%588 != 0 {
-			return fmt.Errorf("meta.Block.parseCueSheet: CD-DA track offset (%d) must be evenly divisible by 588", track.Offset)
+	}
+
+	return nil
+}
+
+// parseTrack parses the i:th cue sheet track, and ensures that its track number
+// is unique.
+func (block *Block) parseTrack(cs *CueSheet, i int, uniq map[uint8]struct{}) error {
+	track := &cs.Tracks[i]
+	// 64 bits: Offset.
+	if err := binary.Read(block.lr, binary.BigEndian, &track.Offset); err != nil {
+		return unexpected(err)
+	}
+	if cs.IsCompactDisc && track.Offset%588 != 0 {
+		return fmt.Errorf("meta.Block.parseCueSheet: CD-DA track offset (%d) must be evenly divisible by 588", track.Offset)
+	}
+
+	// 8 bits: Num.
+	if err := binary.Read(block.lr, binary.BigEndian, &track.Num); err != nil {
+		return unexpected(err)
+	}
+	if _, ok := uniq[track.Num]; ok {
+		return fmt.Errorf("meta.Block.parseCueSheet: duplicated track number %d", track.Num)
+	}
+	uniq[track.Num] = struct{}{}
+	if track.Num == 0 {
+		return errors.New("meta.Block.parseCueSheet: invalid track number (0)")
+	}
+	isLeadOut := i == len(cs.Tracks)-1
+	if cs.IsCompactDisc {
+		if !isLeadOut {
+			if track.Num >= 100 {
+				return fmt.Errorf("meta.Block.parseCueSheet: CD-DA track number (%d) exceeds 99", track.Num)
+			}
+		} else {
+			if track.Num != 170 {
+				return fmt.Errorf("meta.Block.parseCueSheet: invalid lead-out CD-DA track number; expected 170, got %d", track.Num)
+			}
+		}
+	} else {
+		if isLeadOut && track.Num != 255 {
+			return fmt.Errorf("meta.Block.parseCueSheet: invalid lead-out track number; expected 255, got %d", track.Num)
+		}
+	}
+
+	// 12 bytes: ISRC.
+	buf, err := readBytes(block.lr, 12)
+	if err != nil {
+		return unexpected(err)
+	}
+	track.ISRC = stringFromSZ(buf)
+
+	// 1 bit: IsAudio.
+	var x uint8
+	if err = binary.Read(block.lr, binary.BigEndian, &x); err != nil {
+		return unexpected(err)
+	}
+	// mask = 10000000
+	if x&0x80 == 0 {
+		track.IsAudio = true
+	}
+
+	// 1 bit: HasPreEmphasis.
+	// mask = 01000000
+	if x&0x40 != 0 {
+		track.HasPreEmphasis = true
+	}
+
+	// 6 bits and 13 bytes: reserved.
+	// mask = 00111111
+	if x&0x3F != 0 {
+		return ErrInvalidPadding
+	}
+	lr := io.LimitReader(block.lr, 13)
+	zr := zeros{r: lr}
+	_, err = io.Copy(ioutil.Discard, zr)
+	if err != nil {
+		return err
+	}
+
+	// Parse indicies.
+	// 8 bits: (number of indicies)
+	if err = binary.Read(block.lr, binary.BigEndian, &x); err != nil {
+		return unexpected(err)
+	}
+	if x < 1 {
+		if !isLeadOut {
+			return errors.New("meta.Block.parseCueSheet: at least one track index required")
+		}
+		// Lead-out track has no track indices to parse; return early.
+		return nil
+	}
+	track.Indicies = make([]CueSheetTrackIndex, x)
+	for i := range track.Indicies {
+		index := &track.Indicies[i]
+		// 64 bits: Offset.
+		if err = binary.Read(block.lr, binary.BigEndian, &index.Offset); err != nil {
+			return unexpected(err)
 		}

 		// 8 bits: Num.
-		if err = binary.Read(block.lr, binary.BigEndian, &track.Num); err != nil {
+		if err = binary.Read(block.lr, binary.BigEndian, &index.Num); err != nil {
 			return unexpected(err)
 		}
-		if _, ok := uniq[track.Num]; ok {
-			return fmt.Errorf("meta.Block.parseCueSheet: duplicated track number %d", track.Num)
-		}
-		uniq[track.Num] = struct{}{}
-		if track.Num == 0 {
-			return errors.New("meta.Block.parseCueSheet: invalid track number (0)")
-		}
-		isLeadOut := i == len(cs.Tracks)-1
-		if cs.IsCompactDisc {
-			if !isLeadOut {
-				if track.Num >= 100 {
-					return fmt.Errorf("meta.Block.parseCueSheet: CD-DA track number (%d) exceeds 99", track.Num)
-				}
-			} else {
-				if track.Num != 170 {
-					return fmt.Errorf("meta.Block.parseCueSheet: invalid lead-out CD-DA track number; expected 170, got %d", track.Num)
-				}
-			}
-		} else {
-			if isLeadOut && track.Num != 255 {
-				return fmt.Errorf("meta.Block.parseCueSheet: invalid lead-out track number; expected 255, got %d", track.Num)
-			}
-		}

-		// 12 bytes: ISRC.
-		buf, err = readBytes(block.lr, 12)
-		if err != nil {
-			return unexpected(err)
-		}
-		track.ISRC = stringFromSZ(buf)
-
-		// 1 bit: IsAudio.
-		if err = binary.Read(block.lr, binary.BigEndian, &x); err != nil {
-			return unexpected(err)
-		}
-		// mask = 10000000
-		if x&0x80 == 0 {
-			track.IsAudio = true
-		}
-
-		// 1 bit: HasPreEmphasis.
-		// mask = 01000000
-		if x&0x40 != 0 {
-			track.HasPreEmphasis = true
-		}
-
-		// 6 bits and 13 bytes: reserved.
-		// mask = 00111111
-		if x&0x3F != 0 {
-			return ErrInvalidPadding
-		}
-		lr = io.LimitReader(block.lr, 13)
+		// 3 bytes: reserved.
+		lr = io.LimitReader(block.lr, 3)
 		zr = zeros{r: lr}
 		_, err = io.Copy(ioutil.Discard, zr)
 		if err != nil {
 			return err
 		}
-
-		// Parse indicies.
-		// 8 bits: (number of indicies)
-		if err = binary.Read(block.lr, binary.BigEndian, &x); err != nil {
-			return unexpected(err)
-		}
-		if x < 1 {
-			if !isLeadOut {
-				return errors.New("meta.Block.parseCueSheet: at least one track index required")
-			}
-			continue
-		}
-		track.Indicies = make([]CueSheetTrackIndex, x)
-		for i := range track.Indicies {
-			index := &track.Indicies[i]
-			// 64 bits: Offset.
-			if err = binary.Read(block.lr, binary.BigEndian, &index.Offset); err != nil {
-				return unexpected(err)
-			}
-
-			// 8 bits: Num.
-			if err = binary.Read(block.lr, binary.BigEndian, &index.Num); err != nil {
-				return unexpected(err)
-			}
-
-			// 3 bytes: reserved.
-			lr = io.LimitReader(block.lr, 3)
-			zr = zeros{r: lr}
-			_, err = io.Copy(ioutil.Discard, zr)
-			if err != nil {
-				return err
-			}
-		}
 	}
-
 	return nil
 }