Merge pull request #179 from walles/johan/revive

Use Revive for finding unused code

.github/workflows/linux-ci.yml

@@ -22,7 +22,7 @@ jobs:
       # commandline below:
       # https://github.com/golangci/golangci-lint/releases/latest
       - name: Install golangci-lint
-        run: curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b "$(go env GOPATH)"/bin v1.51.2
+        run: curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b "$(go env GOPATH)"/bin v1.54.1
 
       - run: ./test.sh
       - run: GOARCH=386 ./test.sh

.golangci.yaml

@@ -1,9 +1,5 @@
 linters:
   enable:
     - gofmt
-
-    # I'd really want to use Revive for this but that doesn't work:
-    # https://github.com/golangci/golangci-lint/issues/3653
-    - unparam
-
+    - revive
     - usestdlibvars

m/ansiTokenizer.go

@@ -34,6 +34,8 @@ func NewLine(raw string) Line {
 
 // Returns a representation of the string split into styled tokens. Any regexp
 // matches are highlighted. A nil regexp means no highlighting.
+//
+//revive:disable-next-line:unexported-return
 func (line *Line) HighlightedTokens(linePrefix string, search *regexp.Regexp, lineNumberOneBased *int) cellsWithTrailer {
 	plain := line.Plain(lineNumberOneBased)
 	matchRanges := getMatchRanges(&plain, search)

m/ansiTokenizer_test.go

@@ -41,6 +41,7 @@ func TestTokenize(t *testing.T) {
 			}()
 
 			myReader := NewReaderFromStream(fileName, file)
+			//revive:disable-next-line:empty-block
 			for !myReader.done.Load() {
 			}
 

m/highlight.go

@@ -12,6 +12,8 @@ import (
 )
 
 // Files larger than this won't be highlighted
+//
+//revive:disable-next-line:var-naming
 const MAX_HIGHLIGHT_SIZE int64 = 1024 * 1024
 
 // Read and highlight a file using Chroma: https://github.com/alecthomas/chroma

m/linewrapper.go

@@ -8,6 +8,8 @@ import (
 )
 
 // From: https://www.compart.com/en/unicode/U+00A0
+//
+//revive:disable-next-line:var-naming
 const NO_BREAK_SPACE = '\xa0'
 
 func getWrapWidth(line []twin.Cell, maxWrapWidth int) int {

m/pager.go

@@ -24,16 +24,21 @@ const (
 type StatusBarOption int
 
 const (
+	//revive:disable-next-line:var-naming
 	STATUSBAR_STYLE_INVERSE StatusBarOption = iota
+	//revive:disable-next-line:var-naming
 	STATUSBAR_STYLE_PLAIN
+	//revive:disable-next-line:var-naming
 	STATUSBAR_STYLE_BOLD
 )
 
 // How do we render unprintable characters?
 type UnprintableStyle int
 
 const (
+	//revive:disable-next-line:var-naming
 	UNPRINTABLE_STYLE_HIGHLIGHT UnprintableStyle = iota
+	//revive:disable-next-line:var-naming
 	UNPRINTABLE_STYLE_WHITESPACE
 )
 
@@ -475,7 +480,7 @@ func (p *Pager) StartPaging(screen twin.Screen, chromaStyle *chroma.Style, chrom
 
 	unprintableStyle = p.UnprintableStyle
 	consumeLessTermcapEnvs(chromaStyle, chromaFormatter)
-	styleUi(chromaStyle, chromaFormatter, p.StatusBarStyle)
+	styleUI(chromaStyle, chromaFormatter, p.StatusBarStyle)
 
 	p.screen = screen
 	p.linePrefix = getLineColorPrefix(chromaStyle, chromaFormatter)

m/pager_test.go

@@ -15,6 +15,8 @@ import (
 	"gotest.tools/v3/assert"
 )
 
+//revive:disable:empty-block
+
 const blueBackgroundClearToEol0 = "\x1b[44m\x1b[0K" // With 0 before the K, should clear to EOL
 const blueBackgroundClearToEol = "\x1b[44m\x1b[K"   // No 0 before the K, should also clear to EOL
 

m/reader.go

@@ -465,58 +465,60 @@ func NewReaderFromFilename(filename string, style chroma.Style, formatter chroma
 }
 
 // createStatusUnlocked() assumes that its caller is holding the lock
-func (r *Reader) createStatusUnlocked(lastLineOneBased int) string {
+func (reader *Reader) createStatusUnlocked(lastLineOneBased int) string {
 	prefix := ""
-	if r.name != nil {
-		prefix = path.Base(*r.name) + ": "
+	if reader.name != nil {
+		prefix = path.Base(*reader.name) + ": "
 	}
 
-	if len(r.lines) == 0 {
+	if len(reader.lines) == 0 {
 		return prefix + "<empty>"
 	}
 
-	if len(r.lines) == 1 {
+	if len(reader.lines) == 1 {
 		return prefix + "1 line  100%"
 	}
 
-	percent := int(100 * float64(lastLineOneBased) / float64(len(r.lines)))
+	percent := int(100 * float64(lastLineOneBased) / float64(len(reader.lines)))
 
 	return fmt.Sprintf("%s%s lines  %d%%",
 		prefix,
-		formatNumber(uint(len(r.lines))),
+		formatNumber(uint(len(reader.lines))),
 		percent)
 }
 
 // GetLineCount returns the number of lines available for viewing
-func (r *Reader) GetLineCount() int {
-	r.Lock()
-	defer r.Unlock()
+func (reader *Reader) GetLineCount() int {
+	reader.Lock()
+	defer reader.Unlock()
 
-	return len(r.lines)
+	return len(reader.lines)
 }
 
 // GetLine gets a line. If the requested line number is out of bounds, nil is returned.
-func (r *Reader) GetLine(lineNumberOneBased int) *Line {
-	r.Lock()
-	defer r.Unlock()
+func (reader *Reader) GetLine(lineNumberOneBased int) *Line {
+	reader.Lock()
+	defer reader.Unlock()
 
 	if lineNumberOneBased < 1 {
 		return nil
 	}
-	if lineNumberOneBased > len(r.lines) {
+	if lineNumberOneBased > len(reader.lines) {
 		return nil
 	}
-	return r.lines[lineNumberOneBased-1]
+	return reader.lines[lineNumberOneBased-1]
 }
 
 // GetLines gets the indicated lines from the input
 //
 // Overflow state will be didFit if we returned all lines we currently have, or
 // didOverflow otherwise.
-func (r *Reader) GetLines(firstLineOneBased int, wantedLineCount int) (*InputLines, overflowState) {
-	r.Lock()
-	defer r.Unlock()
-	return r.getLinesUnlocked(firstLineOneBased, wantedLineCount)
+//
+//revive:disable-next-line:unexported-return
+func (reader *Reader) GetLines(firstLineOneBased int, wantedLineCount int) (*InputLines, overflowState) {
+	reader.Lock()
+	defer reader.Unlock()
+	return reader.getLinesUnlocked(firstLineOneBased, wantedLineCount)
 }
 
 func nonWrappingAdd(a int, b int) int {
@@ -531,25 +533,25 @@ func nonWrappingAdd(a int, b int) int {
 	return a + b
 }
 
-func (r *Reader) getLinesUnlocked(firstLineOneBased int, wantedLineCount int) (*InputLines, overflowState) {
+func (reader *Reader) getLinesUnlocked(firstLineOneBased int, wantedLineCount int) (*InputLines, overflowState) {
 	if firstLineOneBased < 1 {
 		firstLineOneBased = 1
 	}
 
-	if len(r.lines) == 0 || wantedLineCount == 0 {
+	if len(reader.lines) == 0 || wantedLineCount == 0 {
 		return &InputLines{
 				lines:             nil,
 				firstLineOneBased: firstLineOneBased,
-				statusText:        r.createStatusUnlocked(firstLineOneBased),
+				statusText:        reader.createStatusUnlocked(firstLineOneBased),
 			},
 			didFit // Empty files always fit
 	}
 
 	firstLineZeroBased := firstLineOneBased - 1
 	lastLineZeroBased := nonWrappingAdd(firstLineZeroBased, wantedLineCount-1)
 
-	if lastLineZeroBased >= len(r.lines) {
-		lastLineZeroBased = len(r.lines) - 1
+	if lastLineZeroBased >= len(reader.lines) {
+		lastLineZeroBased = len(reader.lines) - 1
 	}
 
 	// Prevent reading past the end of the available lines
@@ -561,19 +563,19 @@ func (r *Reader) getLinesUnlocked(firstLineOneBased int, wantedLineCount int) (*
 			firstLineOneBased = 1
 		}
 
-		return r.getLinesUnlocked(firstLineOneBased, wantedLineCount)
+		return reader.getLinesUnlocked(firstLineOneBased, wantedLineCount)
 	}
 
-	returnLines := r.lines[firstLineZeroBased : lastLineZeroBased+1]
+	returnLines := reader.lines[firstLineZeroBased : lastLineZeroBased+1]
 	overflow := didFit
-	if len(returnLines) != len(r.lines) {
+	if len(returnLines) != len(reader.lines) {
 		overflow = didOverflow // We're not returning all available lines
 	}
 
 	return &InputLines{
 			lines:             returnLines,
 			firstLineOneBased: firstLineOneBased,
-			statusText:        r.createStatusUnlocked(lastLineZeroBased + 1),
+			statusText:        reader.createStatusUnlocked(lastLineZeroBased + 1),
 		},
 		overflow
 }

m/reader_test.go

@@ -15,6 +15,8 @@ import (
 	"gotest.tools/v3/assert"
 )
 
+//revive:disable:empty-block
+
 func testGetLineCount(t *testing.T, reader *Reader) {
 	if strings.Contains(*reader.name, "compressed") {
 		// We are no good at counting lines of compressed files, never mind
@@ -139,8 +141,10 @@ func getTestFiles() []string {
 // Wait for reader to finish reading and highlighting. Used by tests.
 func (r *Reader) _wait() error {
 	// Wait for our goroutine to finish
+	//revive:disable-next-line:empty-block
 	for !r.done.Load() {
 	}
+	//revive:disable-next-line:empty-block
 	for !r.highlightingDone.Load() {
 	}
 
@@ -213,7 +217,7 @@ func testHighlightingLineCount(t *testing.T, filenameWithPath string) {
 	}
 	rawLinesCount := rawLinefeedsCount
 	if !rawFileEndsWithNewline {
-		rawLinesCount += 1
+		rawLinesCount++
 	}
 
 	// Then load the same file using one of our Readers
@@ -329,15 +333,15 @@ func TestCompressedFiles(t *testing.T) {
 }
 
 func TestFilterNotInstalled(t *testing.T) {
-	// FIXME: Test what happens if we try to use a filter that is not installed
+	t.Skip("FIXME: Test what happens if we try to use a filter that is not installed")
 }
 
 func TestFilterFailure(t *testing.T) {
-	// FIXME: Test what happens if the filter command fails because of bad command line options
+	t.Skip("FIXME: Test what happens if the filter command fails because of bad command line options")
 }
 
 func TestFilterPermissionDenied(t *testing.T) {
-	// FIXME: Test what happens if the filter command fails because it can't access the requested file
+	t.Skip("FIXME: Test what happens if the filter command fails because it can't access the requested file")
 }
 
 func TestFilterFileNotFound(t *testing.T) {
@@ -357,7 +361,7 @@ func TestFilterFileNotFound(t *testing.T) {
 }
 
 func TestFilterNotAFile(t *testing.T) {
-	// FIXME: Test what happens if the filter command fails because the target is not a file
+	t.Skip("FIXME: Test what happens if the filter command fails because the target is not a file")
 }
 
 // How long does it take to read a file?
@@ -376,6 +380,7 @@ func BenchmarkReaderDone(b *testing.B) {
 		}
 
 		// Wait for the reader to finish
+		//revive:disable-next-line:empty-block
 		for !readMe.done.Load() {
 		}
 		if readMe.err != nil {
@@ -390,8 +395,8 @@ func BenchmarkReadLargeFile(b *testing.B) {
 	const largeSizeBytes = 35_000_000
 
 	// First, create it from something...
-	input_filename := getSamplesDir() + "/../m/pager.go"
-	contents, err := os.ReadFile(input_filename)
+	inputFilename := getSamplesDir() + "/../m/pager.go"
+	contents, err := os.ReadFile(inputFilename)
 	if err != nil {
 		panic(err)
 	}
@@ -436,8 +441,8 @@ func BenchmarkReadLargeFile(b *testing.B) {
 // Count lines in pager.go
 func BenchmarkCountLines(b *testing.B) {
 	// First, get some sample lines...
-	input_filename := getSamplesDir() + "/../m/pager.go"
-	contents, err := os.ReadFile(input_filename)
+	inputFilename := getSamplesDir() + "/../m/pager.go"
+	contents, err := os.ReadFile(inputFilename)
 	if err != nil {
 		panic(err)
 	}

m/screenLines_test.go

@@ -10,6 +10,8 @@ import (
 	"gotest.tools/v3/assert"
 )
 
+//revive:disable:empty-block
+
 func testHorizontalCropping(t *testing.T, contents string, firstIndex int, lastIndex int, expected string, expectedOverflow overflowState) {
 	pager := NewPager(nil)
 	pager.ShowLineNumbers = false

m/scrollPosition.go

@@ -60,28 +60,30 @@ func canonicalFromPager(pager *Pager) scrollPositionCanonical {
 }
 
 // Create a new position, scrolled towards the end of the file
-func (s scrollPosition) PreviousLine(scrollDistance int) scrollPosition {
+func (sp scrollPosition) PreviousLine(scrollDistance int) scrollPosition {
 	return scrollPosition{
 		internalDontTouch: scrollPositionInternal{
-			name:               s.internalDontTouch.name,
-			lineNumberOneBased: s.internalDontTouch.lineNumberOneBased,
-			deltaScreenLines:   s.internalDontTouch.deltaScreenLines - scrollDistance,
+			name:               sp.internalDontTouch.name,
+			lineNumberOneBased: sp.internalDontTouch.lineNumberOneBased,
+			deltaScreenLines:   sp.internalDontTouch.deltaScreenLines - scrollDistance,
 		},
 	}
 }
 
 // Create a new position, scrolled towards the end of the file
-func (s scrollPosition) NextLine(scrollDistance int) scrollPosition {
+func (sp scrollPosition) NextLine(scrollDistance int) scrollPosition {
 	return scrollPosition{
 		internalDontTouch: scrollPositionInternal{
-			name:               s.internalDontTouch.name,
-			lineNumberOneBased: s.internalDontTouch.lineNumberOneBased,
-			deltaScreenLines:   s.internalDontTouch.deltaScreenLines + scrollDistance,
+			name:               sp.internalDontTouch.name,
+			lineNumberOneBased: sp.internalDontTouch.lineNumberOneBased,
+			deltaScreenLines:   sp.internalDontTouch.deltaScreenLines + scrollDistance,
 		},
 	}
 }
 
 // Create a new position, scrolled to the given line number
+//
+//revive:disable-next-line:unexported-return
 func NewScrollPositionFromLineNumberOneBased(lineNumberOneBased int, source string) scrollPosition {
 	return scrollPosition{
 		internalDontTouch: scrollPositionInternal{
@@ -168,7 +170,7 @@ func (si *scrollPositionInternal) emptyBottomLinesCount(pager *Pager) int {
 		}
 
 		// Move to the next line
-		lineNumberOneBased += 1
+		lineNumberOneBased++
 	}
 
 	return unclaimedViewportLines

m/search.go

@@ -70,9 +70,9 @@ func (p *Pager) findFirstHit(startPosition scrollPosition, backwards bool) *scro
 		}
 
 		if backwards {
-			searchPosition -= 1
+			searchPosition--
 		} else {
-			searchPosition += 1
+			searchPosition++
 		}
 	}
 }