Skip to content

Fix golangci-lint config and implement minor stylistic changes #111

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 18, 2023
Merged

Fix golangci-lint config and implement minor stylistic changes #111

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6111b52
Improve field alignment
oschwald Jun 18, 2023
eeae704
Minor style changes
oschwald Jun 18, 2023
7ce21e8
Update golangci-lint config
oschwald Jun 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 69 additions & 45 deletions .golangci.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,25 +1,26 @@
[run]
deadline = "10m"

tests = true

[linters]
disable-all = true
enable = [
"asasalint",
"asciicheck",
"bidichk",
"bodyclose",
"containedctx",
"contextcheck",
"depguard",
"dupword",
"durationcheck",
"errcheck",
"errchkjson",
"errname",
"errorlint",
# "exhaustive",
"exportloopref",
"forbidigo",
#"forcetypeassert",
"goconst",
"gocyclo",
"gocritic",
Expand All @@ -42,6 +43,7 @@
"nosprintfhostport",
"predeclared",
"revive",
"rowserrcheck",
"sqlclosecheck",
"staticcheck",
"stylecheck",
Expand All @@ -51,10 +53,17 @@
"unconvert",
"unparam",
"unused",
"usestdlibvars",
"vetshadow",
"wastedassign",
]

[[linters-settings.depguard.rules.main.deny]]
pkg = "io/ioutil"
desc = "Deprecated. Functions have been moved elsewhere."

[linters-settings.errcheck]
check-blank = true
# Ignoring Close so that we don't have to have a bunch of
# `defer func() { _ = r.Close() }()` constructs when we
# don't actually care about the error.
Expand All @@ -68,6 +77,15 @@
[linters-settings.exhaustive]
default-signifies-exhaustive = true

[linters-settings.forbidigo]
# Forbid the following identifiers
forbid = [
"Geoip", # use "GeoIP"
"^geoIP", # use "geoip"
"Maxmind", # use "MaxMind"
"^maxMind", # use "maxmind"
]

[linters-settings.gocritic]
enabled-checks = [
"appendAssign",
Expand All @@ -89,8 +107,7 @@
"commentedOutImport",
"commentFormatting",
"defaultCaseOrder",
# Revive's defer rule already captures this. This caught no extra cases.
# "deferInLoop",
"deferInLoop",
"deferUnlambda",
"deprecatedComment",
"docStub",
Expand All @@ -109,12 +126,12 @@
"exitAfterDefer",
"exposedSyncMutex",
"externalErrorReassign",
# Given that all of our code runs on Linux and the / separate should
# work fine, this seems less important.
# "filepathJoin",
"filepathJoin",
"flagDeref",
"flagName",
"hexLiteral",
"httpNoBody",
"hugeParam",
"ifElseChain",
"importShadow",
"indexAlloc",
Expand All @@ -138,22 +155,20 @@
"redundantSprint",
"regexpMust",
"regexpPattern",
# This might be good, but I don't think we want to encourage
# significant changes to regexes as we port stuff from Perl.
# "regexpSimplify",
"regexpSimplify",
"returnAfterHttpError",
"ruleguard",
"singleCaseSwitch",
"sliceClear",
"sloppyLen",
# This seems like it might also be good, but a lot of existing code
# fails.
# "sloppyReassign",
"returnAfterHttpError",
"sloppyReassign",
"sloppyTestFuncName",
"sloppyTypeAssert",
"sortSlice",
"sprintfQuotedString",
"sqlQuery",
"stringsCompare",
"stringConcatSimplify",
"stringXbytes",
"switchTrue",
"syncMapLoadAndDelete",
Expand All @@ -168,28 +183,40 @@
"underef",
"unlabelStmt",
"unlambda",
# I am not sure we would want this linter and a lot of existing
# code fails.
# "unnamedResult",
"unnecessaryBlock",
"unnecessaryDefer",
"unslice",
"valSwap",
"weakCond",
# Covered by nolintlint
# "whyNoLint"
"wrapperFunc",
"yodaStyleExpr",
# This requires explanations for "nolint" directives. This would be
# nice for gosec ones, but I am not sure we want it generally unless
# we can get the false positive rate lower.
# "whyNoLint"
]

[linters-settings.gofumpt]
extra-rules = true
lang-version = "1.19"

[linters-settings.gosec]
excludes = [
# G104 - "Audit errors not checked." We use errcheck for this.
"G104",

# G304 - "Potential file inclusion via variable"
"G304",

# G306 - "Expect WriteFile permissions to be 0600 or less".
"G306",

# Prohibits defer (*os.File).Close, which we allow when reading from file.
"G307",
]

[linters-settings.govet]
"enable-all" = true
disable = ["shadow"]

[linters-settings.lll]
line-length = 120
Expand All @@ -206,8 +233,6 @@
ignore-generated-header = true
severity = "warning"

# This might be nice but it is so common that it is hard
# to enable.
# [[linters-settings.revive.rules]]
# name = "add-constant"

Expand All @@ -232,8 +257,10 @@
# [[linters-settings.revive.rules]]
# name = "cognitive-complexity"

# Probably a good rule, but we have a lot of names that
# only have case differences.
[[linters-settings.revive.rules]]
name = "comment-spacings"
arguments = ["easyjson", "nolint"]

# [[linters-settings.revive.rules]]
# name = "confusing-naming"

Expand All @@ -252,6 +279,12 @@
# [[linters-settings.revive.rules]]
# name = "cyclomatic"

[[linters-settings.revive.rules]]
name = "datarace"

# [[linters-settings.revive.rules]]
# name = "deep-exit"

[[linters-settings.revive.rules]]
name = "defer"

Expand Down Expand Up @@ -288,8 +321,6 @@
# [[linters-settings.revive.rules]]
# name = "file-header"

# We have a lot of flag parameters. This linter probably makes
# a good point, but we would need some cleanup or a lot of nolints.
# [[linters-settings.revive.rules]]
# name = "flag-parameter"

Expand Down Expand Up @@ -329,7 +360,6 @@
[[linters-settings.revive.rules]]
name = "modifies-value-receiver"

# We frequently use nested structs, particularly in tests.
# [[linters-settings.revive.rules]]
# name = "nested-structs"

Expand Down Expand Up @@ -363,6 +393,9 @@
[[linters-settings.revive.rules]]
name = "superfluous-else"

[[linters-settings.revive.rules]]
name = "time-equal"

[[linters-settings.revive.rules]]
name = "time-naming"

Expand All @@ -375,8 +408,6 @@
[[linters-settings.revive.rules]]
name = "unexported-return"

# This is covered elsewhere and we want to ignore some
# functions such as fmt.Fprintf.
# [[linters-settings.revive.rules]]
# name = "unhandled-error"

Expand All @@ -389,14 +420,11 @@
[[linters-settings.revive.rules]]
name = "unused-parameter"

# We generally have unused receivers in tests for meeting the
# requirements of an interface.
# [[linters-settings.revive.rules]]
# name = "unused-receiver"
[[linters-settings.revive.rules]]
name = "unused-receiver"

# This probably makes sense after we upgrade to 1.18
# [[linters-settings.revive.rules]]
# name = "use-any"
[[linters-settings.revive.rules]]
name = "use-any"

[[linters-settings.revive.rules]]
name = "useless-break"
Expand All @@ -413,16 +441,12 @@
[linters-settings.unparam]
check-exported = true

[issues]
exclude-use-default = false

[[issues.exclude-rules]]
linters = [
"govet"
]
# we want to enable almost all govet rules. It is easier to just filter out
# the ones we don't want:
#
# * fieldalignment - way too noisy. Although it is very useful in particular
# cases where we are trying to use as little memory as possible, having
# it go off on every struct isn't helpful.
# * shadow - although often useful, it complains about _many_ err
# shadowing assignments and some others where shadowing is clear.
text = "^(fieldalignment|shadow)"
path = "_test.go"
text = "^fieldalignment"
20 changes: 10 additions & 10 deletions decoder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,12 +151,12 @@ func (d *decoder) decodeFromType(
result reflect.Value,
depth int,
) (uint, error) {
result = d.indirect(result)
result = indirect(result)

// For these types, size has a special meaning
switch dtype {
case _Bool:
return d.unmarshalBool(size, offset, result)
return unmarshalBool(size, offset, result)
case _Map:
return d.unmarshalMap(size, offset, result, depth)
case _Pointer:
Expand Down Expand Up @@ -203,7 +203,7 @@ func (d *decoder) decodeFromTypeToDeserializer(
// For these types, size has a special meaning
switch dtype {
case _Bool:
v, offset := d.decodeBool(size, offset)
v, offset := decodeBool(size, offset)
return offset, dser.Bool(v)
case _Map:
return d.decodeMapToDeserializer(size, offset, dser, depth)
Expand Down Expand Up @@ -255,14 +255,14 @@ func (d *decoder) decodeFromTypeToDeserializer(
}
}

func (d *decoder) unmarshalBool(size, offset uint, result reflect.Value) (uint, error) {
func unmarshalBool(size, offset uint, result reflect.Value) (uint, error) {
if size > 1 {
return 0, newInvalidDatabaseError(
"the MaxMind DB file's data section contains bad data (bool size of %v)",
size,
)
}
value, newOffset := d.decodeBool(size, offset)
value, newOffset := decodeBool(size, offset)

switch result.Kind() {
case reflect.Bool:
Expand All @@ -281,7 +281,7 @@ func (d *decoder) unmarshalBool(size, offset uint, result reflect.Value) (uint,
// heavily based on encoding/json as my original version had a subtle
// bug. This method should be considered to be licensed under
// https://golang.org/LICENSE
func (d *decoder) indirect(result reflect.Value) reflect.Value {
func indirect(result reflect.Value) reflect.Value {
for {
// Load value from interface, but only if the result will be
// usefully addressable.
Expand Down Expand Up @@ -415,7 +415,7 @@ func (d *decoder) unmarshalMap(
result reflect.Value,
depth int,
) (uint, error) {
result = d.indirect(result)
result = indirect(result)
switch result.Kind() {
default:
return 0, newUnmarshalTypeError("map", result.Type())
Expand All @@ -425,7 +425,7 @@ func (d *decoder) unmarshalMap(
return d.decodeMap(size, offset, result, depth)
case reflect.Interface:
if result.NumMethod() == 0 {
rv := reflect.ValueOf(make(map[string]interface{}, size))
rv := reflect.ValueOf(make(map[string]any, size))
newOffset, err := d.decodeMap(size, offset, rv, depth)
result.Set(rv)
return newOffset, err
Expand Down Expand Up @@ -458,7 +458,7 @@ func (d *decoder) unmarshalSlice(
return d.decodeSlice(size, offset, result, depth)
case reflect.Interface:
if result.NumMethod() == 0 {
a := []interface{}{}
a := []any{}
rv := reflect.ValueOf(&a).Elem()
newOffset, err := d.decodeSlice(size, offset, rv, depth)
result.Set(rv)
Expand Down Expand Up @@ -551,7 +551,7 @@ func (d *decoder) unmarshalUint128(size, offset uint, result reflect.Value) (uin
return newOffset, newUnmarshalTypeError(value, result.Type())
}

func (d *decoder) decodeBool(size, offset uint) (bool, uint) {
func decodeBool(size, offset uint) (bool, uint) {
return size != 0, offset
}

Expand Down
Loading