Skip to content

Commit

Permalink
Bump golangci to 1.51.1 and fix various linting errors (#132)
Browse files Browse the repository at this point in the history
* Bump golangci to 1.51.1 to try and fix fmt load bug

* Various spacing fixes, still need to fix a few more

* fix gci things

* Extract styledcell instead of forcing potentially unsafe type check

* Fix fuzz tests gated behind go version
  • Loading branch information
Evertras authored Feb 13, 2023
1 parent 6973eda commit 235f169
Show file tree
Hide file tree
Showing 20 changed files with 53 additions and 27 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,5 @@ jobs:
- name: golangci-lint
uses: golangci/golangci-lint-action@v3
with:
version: v1.44.2
version: v1.51.1

8 changes: 6 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,13 @@ coverage.out: table/*.go go.*
@go test -coverprofile=coverage.out ./table

.PHONY: fmt
fmt:
fmt: ./bin/gci
@go fmt ./...
@./bin/gci write --skip-generated ./table/*.go

./bin/golangci-lint:
curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b ./bin v1.44.2
curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b ./bin v1.51.1

./bin/gci:
GOBIN=$(shell pwd)/bin go install github.com/daixiang0/[email protected]

6 changes: 4 additions & 2 deletions table/border.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,8 @@ func (b *Border) styleBothWithFooter(original lipgloss.Style) lipgloss.Style {
}

// This function is long, but it's just repetitive...
// nolint:funlen
//
//nolint:funlen
func (b *Border) generateMultiStyles() {
b.styleMultiTopLeft = lipgloss.NewStyle().BorderStyle(
lipgloss.Border{
Expand Down Expand Up @@ -354,7 +355,8 @@ func (b *borderStyleRow) inherit(s lipgloss.Style) {

// There's a lot of branches here, but splitting it up further would make it
// harder to follow. So just be careful with comments and make sure it's tested!
// nolint:nestif
//
//nolint:nestif
func (m Model) styleHeaders() borderStyleRow {
hasRows := len(m.GetVisibleRows()) > 0
singleColumn := len(m.columns) == 1
Expand Down
4 changes: 3 additions & 1 deletion table/calc.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ func max(x, y int) int {
return y
}

// nolint: varnamelen
// These var names are fine for this little function
//
//nolint:varnamelen
func gcd(x, y int) int {
if x == 0 {
return y
Expand Down
3 changes: 2 additions & 1 deletion table/data.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ package table
import "time"

// This is just a bunch of data type checks, so... no linting here
// nolint: cyclop
//
//nolint:cyclop
func asInt(data interface{}) (int64, bool) {
switch val := data.(type) {
case int:
Expand Down
3 changes: 2 additions & 1 deletion table/dimensions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ import (
)

// This function is only long because of repetitive test definitions, this is fine
// nolint: funlen
//
//nolint:funlen
func TestColumnUpdateWidths(t *testing.T) {
tests := []struct {
name string
Expand Down
4 changes: 2 additions & 2 deletions table/events_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func TestUserEventsEmptyWhenNothingHappens(t *testing.T) {
assert.Len(t, events, 0, "Should be empty when no changes made in Update")
}

// nolint: funlen // This is a bunch of checks in a row, this is fine
//nolint:funlen // This is a bunch of checks in a row, this is fine
func TestUserEventHighlightedIndexChanged(t *testing.T) {
// Don't need any actual row data for this
empty := RowData{}
Expand Down Expand Up @@ -98,7 +98,7 @@ func TestUserEventHighlightedIndexChanged(t *testing.T) {
assert.Len(t, events, 0, "There's no row to change to for an empty table, event shouldn't exist")
}

// nolint: funlen // This is a bunch of checks in a row, this is fine
//nolint:funlen // This is a bunch of checks in a row, this is fine
func TestUserEventRowSelectToggled(t *testing.T) {
// Don't need any actual row data for this
empty := RowData{}
Expand Down
10 changes: 7 additions & 3 deletions table/filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,17 +41,21 @@ func isRowMatched(columns []Column, row Row, filter string) bool {
continue
}

// Extract internal StyledCell data
switch dataV := data.(type) {
case StyledCell:
data = dataV.Data
}

target := ""
switch dataV := data.(type) {
case string:
target = dataV

case fmt.Stringer:
target = dataV.String()

case StyledCell:
target = dataV.Data.(string)
}

if strings.Contains(strings.ToLower(target), strings.ToLower(filter)) {
return true
}
Expand Down
2 changes: 1 addition & 1 deletion table/filter_test.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
package table

import (
"github.com/charmbracelet/lipgloss"
"testing"
"time"

"github.com/charmbracelet/bubbles/textinput"
tea "github.com/charmbracelet/bubbletea"
"github.com/charmbracelet/lipgloss"
"github.com/stretchr/testify/assert"
)

Expand Down
3 changes: 2 additions & 1 deletion table/header.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ import "github.com/charmbracelet/lipgloss"

// This is long and could use some refactoring in the future, but unsure of how
// to pick it apart right now.
// nolint: funlen, cyclop
//
//nolint:funlen,cyclop
func (m Model) renderHeaders() string {
headerStrings := []string{}

Expand Down
3 changes: 2 additions & 1 deletion table/options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,8 @@ func TestWithHighlightedRowSetTooHigh(t *testing.T) {
}

// This is long only because it's a lot of repetitive test cases
// nolint: funlen
//
//nolint:funlen
func TestPageOptions(t *testing.T) {
const (
pageSize = 5
Expand Down
3 changes: 2 additions & 1 deletion table/pagination_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,8 @@ func TestPaginationHighlightFirstOnPageDown(t *testing.T) {
}

// This is long because of various test cases, not because of logic
// nolint: funlen
//
//nolint:funlen
func TestExpectedPageForRowIndex(t *testing.T) {
tests := []struct {
name string
Expand Down
5 changes: 3 additions & 2 deletions table/row.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func (r Row) WithStyle(style lipgloss.Style) Row {
return r
}

// nolint: nestif // This has many ifs, but they're short
//nolint:nestif // This has many ifs, but they're short
func (m Model) renderRowColumnData(row Row, column Column, rowStyle lipgloss.Style, borderStyle lipgloss.Style) string {
cellStyle := rowStyle.Copy().Inherit(column.style).Inherit(m.baseStyle)

Expand Down Expand Up @@ -94,7 +94,8 @@ func (m Model) renderRowColumnData(row Row, column Column, rowStyle lipgloss.Sty

// This is long and could use some refactoring in the future, but not quite sure
// how to pick it apart yet.
// nolint: funlen, cyclop, gocognit
//
//nolint:funlen, cyclop, gocognit
func (m Model) renderRow(rowIndex int, last bool) string {
numColumns := len(m.columns)
row := m.GetVisibleRows()[rowIndex]
Expand Down
3 changes: 2 additions & 1 deletion table/scrolling_fuzz_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ import (
)

// This is long because of test cases
// nolint: funlen
//
//nolint:funlen,gocognit,cyclop
func FuzzHorizontalScrollingStopEdgeCases(f *testing.F) {
const (
minNameWidth = 2
Expand Down
6 changes: 4 additions & 2 deletions table/scrolling_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,8 @@ func TestHorizontalScrollingWithFooterAndFrozenCols(t *testing.T) {
}

// This is long due to literal strings
// nolint: funlen
//
//nolint:funlen
func TestHorizontalScrollStopsAtLastColumnBeingVisible(t *testing.T) {
model := New([]Column{
NewColumn("Name", "Name", 4),
Expand Down Expand Up @@ -285,7 +286,8 @@ func TestNoScrollingWhenEntireTableIsVisible(t *testing.T) {
}

// This is long because of test cases
// nolint: funlen
//
//nolint:funlen
func TestHorizontalScrollingStopEdgeCases(t *testing.T) {
tests := []struct {
numCols int
Expand Down
3 changes: 2 additions & 1 deletion table/strlimit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ import (
)

// This function is only long because of repetitive test definitions, this is fine
// nolint: funlen
//
//nolint:funlen
func TestLimitStr(t *testing.T) {
tests := []struct {
name string
Expand Down
4 changes: 3 additions & 1 deletion table/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,9 @@ func (m Model) updateFilterTextInput(msg tea.Msg) (Model, tea.Cmd) {
return m, cmd
}

// nolint: cyclop // This is a series of Matches tests with minimal logic
// This is a series of Matches tests with minimal logic
//
//nolint:cyclop
func (m *Model) handleKeypress(msg tea.KeyMsg) {
previousRowIndex := m.rowCursorIndex

Expand Down
3 changes: 2 additions & 1 deletion table/update_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ func TestPageKeysDoNothingWhenNoPages(t *testing.T) {

// This is a long test with a lot of movement keys pressed, that's okay because
// it's simply repetitive and tracking the same kind of state change many times
// nolint: funlen
//
//nolint:funlen
func TestFocusedMovesWhenMoveKeysPressedPaged(t *testing.T) {
cols := []Column{
NewColumn("id", "ID", 3),
Expand Down
2 changes: 1 addition & 1 deletion table/view.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func (m Model) View() string {
if m.headerVisible {
rowStrs = append(rowStrs, headers)
} else if endRowIndex-startRowIndex > 0 {
// nolint: gomnd // This is just getting the first newlined substring
//nolint: gomnd // This is just getting the first newlined substring
split := strings.SplitN(headers, "\n", 2)
rowStrs = append(rowStrs, split[0])
}
Expand Down
3 changes: 2 additions & 1 deletion table/view_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -647,7 +647,8 @@ func TestSimple3x3StyleOverridesAsBaseColumnRowCell(t *testing.T) {
}

// This is a long test due to typing and multiple big table strings, that's okay
// nolint: funlen
//
//nolint:funlen
func Test3x3WithFilterFooter(t *testing.T) {
model := New([]Column{
NewColumn("1", "1", 4).WithFiltered(true),
Expand Down

0 comments on commit 235f169

Please sign in to comment.