diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 1ce272e..0d64a2c 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -11,7 +11,7 @@ jobs: build: strategy: matrix: - go-version: [1.19.x, 1.20.x] + go-version: [1.21.x, 1.22.x] platform: [ubuntu-latest, macos-latest, windows-latest] runs-on: ${{ matrix.platform }} name: "Build ${{ matrix.go-version }} test on ${{ matrix.platform }}" diff --git a/.golangci.toml b/.golangci.toml index 09b1486..f9b7c79 100644 --- a/.golangci.toml +++ b/.golangci.toml @@ -1,708 +1,641 @@ [run] - deadline = "10m" - tests = true +# This is needed for precious, which may run multiple instances +# in parallel +allow-parallel-runners = true +go = "1.21" +tests = true +timeout = "10m" [linters] - disable-all = true - enable = [ - "asasalint", - "asciicheck", - "bidichk", - "bodyclose", - "containedctx", - "contextcheck", - "depguard", +enable-all = true +disable = [ + "cyclop", + "dogsled", + "dupl", + # This is probably worthwhile, but there are a number of false positives # that would need to be addressed. - # "dupword", - "durationcheck", - "errcheck", - "errchkjson", - "errname", - "errorlint", + "dupword", + # This doesn't seem to know about CTEs or DELETEs with RETURNING - # "execinquery", - "exhaustive", + "execinquery", + # We often don't initialize all of the struct fields. This is fine # generally - # "exhaustruct", + "exhaustruct", + + # Not relevant after 1.22 "exportloopref", - "forbidigo", + # We tried this linter but most places we do forced type asserts are # pretty safe, e.g., an atomic.Value when everything is encapsulated # in a small package. - # "forcetypeassert", - "goconst", - "gocyclo", - "gocritic", - "godot", - "gofumpt", - "gomodguard", - "gosec", - "gosimple", + "forcetypeassert", + + "funlen", + "gochecknoglobals", + "gochecknoinits", + + # Similar to the exhaustive linter and I don't know that we use these + # sorts of sum types + "gochecksumtype", + + "gocognit", + "goerr113", + "godox", + "gomnd", + # This only "caught" one thing, and it seemed like a reasonable use # of Han script. Generally, I don't think we want to prevent the use - # of particulr scripts. The time.Local checks might be useful, but + # of particular scripts. The time.Local checks might be useful, but # this didn't actually catch anything of note there. - # "gosmopolitan", - "govet", - "grouper", - "ineffassign", - "lll", + "gosmopolitan", + + # Seems too opinionated or at least would require going through all the + # interfaces we have. + "inamedparam", + + "ireturn", + # We don't use these loggers - # "loggercheck", - "makezero", + "loggercheck", + # Maintainability Index. Seems like it could be a good idea, but a # lot of things fail and we would need to make some decisions about # what to allow. - # "maintidx", - "misspell", + "maintidx", + # Causes panics, e.g., when processing mmerrors - # "musttag", - "nakedret", - "nilerr", + "musttag", + + "nestif", + # Perhaps too opinionated. We do have some legitimate uses of "return nil, nil" - # "nilnil", - "noctx", - "nolintlint", + "nilnil", + + "nlreturn", + # We occasionally use named returns for documentation, which is helpful. # Named returns are only really a problem when used in conjunction with # a bare return statement. I _think_ Revive's bare-return covers that # case. - # "nonamedreturns", - "nosprintfhostport", - "predeclared", - "revive", - "rowserrcheck", - # https://github.com/golangci/golangci-lint/issues/287 - # "safesql", - "sqlclosecheck", - "staticcheck", - "stylecheck", + "nonamedreturns", + + "paralleltest", + "prealloc", + # We have very few structs with multiple tags and for the couple we had, this # actually made it harder to read. - # "tagalign", - "tenv", - "tparallel", - "typecheck", - "unconvert", - "unparam", - "unused", - "usestdlibvars", - "vetshadow", - "wastedassign", - # We don't currently wrap external errors in this module. - # "wrapcheck", - ] - -# Please note that we only use depguard for stdlib as gomodguard only + "tagalign", + + # We probably _should_ be doing this! + "thelper", + + # We don't follow this. Sometimes we test internal code. + "testpackage", + + "varnamelen", + + # This would probably be good, but we would need to configure it. + "wsl", + + # These are all deprecated + "deadcode", + "exhaustivestruct", + "golint", + "ifshort", + "interfacer", + "maligned", + "nosnakecase", + "scopelint", + "structcheck", + "varcheck", + + # Require Go 1.22 + "copyloopvar", + "intrange", +] + +# Please note that we only use depguard for blocking packages and +# gomodguard for blocking modules. # supports modules currently. See https://github.com/ryancurrah/gomodguard/issues/12 +[[linters-settings.depguard.rules.main.deny]] +pkg = "golang.org/x/exp/slog" +desc = "Use log/slog instead." + [[linters-settings.depguard.rules.main.deny]] pkg = "io/ioutil" desc = "Deprecated. Functions have been moved elsewhere." [[linters-settings.depguard.rules.main.deny]] -# golang.org/x/exp/slices has better alternatives. The proposal to add this -# to Go has been accepted, https://github.com/golang/go/issues/57433 +pkg = "k8s.io/utils/strings/slices" +desc = "Use slices" + +[[linters-settings.depguard.rules.main.deny]] +# slices has better alternatives. pkg = "sort" -desc = "Use golang.org/x/exp/slices instead" +desc = "Use slices instead" [linters-settings.errcheck] - # Don't allow setting of error to the blank identifier. If there is a legitimate - # reason, there should be a nolint with an explanation. - check-blank = true +# Don't allow setting of error to the blank identifier. If there is a legitimate +# reason, there should be a nolint with an explanation. +check-blank = true - exclude-functions = [ - # If we are rolling back a transaction, we are often already in an error - # state. - '(*database/sql.Tx).Rollback', +exclude-functions = [ + # If we are rolling back a transaction, we are often already in an error + # state. + '(*database/sql.Tx).Rollback', - # It is reasonable to ignore errors if Cleanup fails in most cases. - '(*github.com/google/renameio/v2.PendingFile).Cleanup', + # It is reasonable to ignore errors if Cleanup fails in most cases. + '(*github.com/google/renameio/v2.PendingFile).Cleanup', - # We often don't care if removing a file failed (e.g., it doesn't exist) - 'os.Remove', - 'os.RemoveAll', - ] + # We often don't care if removing a file failed (e.g., it doesn't exist) + 'os.Remove', + 'os.RemoveAll', +] - # 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. - ignore = "Close,fmt:.*" +# 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. +ignore = "Close,fmt:.*" [linters-settings.errorlint] - errorf = true - asserts = true - comparison = true +errorf = true +asserts = true +comparison = true [linters-settings.exhaustive] - default-signifies-exhaustive = true +default-signifies-exhaustive = true [linters-settings.forbidigo] - # Forbid the following identifiers - forbid = [ - "Geoip", # use "GeoIP" - "^geoIP", # use "geoip" - "^hubSpot", # use "hubspot" - "Maxmind", # use "MaxMind" - "^maxMind", # use "maxmind" - "Minfraud", # use "MinFraud" - "^minFraud", # use "minfraud" - "[Uu]ser[iI][dD]", # use "accountID" or "AccountID" - - # use netip.ParsePrefix unless you really need a *net.IPNet - "^net.ParseCIDR", - - # use netip.ParseAddr unless you really need a net.IP - "^net.ParseIP", - ] +# Forbid the following identifiers +forbid = [ + { p = "Geoip", msg = "you should use `GeoIP`" }, + { p = "geoIP", msg = "you should use `geoip`" }, + { p = "^hubSpot", msg = "you should use `hubspot`" }, + { p = "Maxmind", msg = "you should use `MaxMind`" }, + { p = "^maxMind", msg = "you should use `maxmind`" }, + { p = "Minfraud", msg = "you should use `MinFraud`" }, + { p = "^minFraud", msg = "you should use `minfraud`" }, + { p = "[Uu]ser[iI][dD]", msg = "you should use `accountID` or `AccountID`" }, + { p = "WithEnterpriseURLs", msg = "Use ghe.NewClient instead." }, + { p = "^bigquery.NewClient", msg = "you should use mmgcloud.NewBigQueryClient instead." }, + { p = "^cloudresourcemanager.NewService", msg = "you should use mmgcloud.NewCloudResourceManagerService instead." }, + { p = "^compute.NewService", msg = "you should use mmgcloud.NewComputeService instead." }, + { p = "^drive.NewService", msg = "you should use mmgdrive.NewGDrive instead." }, + { p = "^math.Max$", msg = "you should use the max built-in instead." }, + { p = "^math.Min$", msg = "you should use the min built-in instead." }, + { p = "^net.ParseCIDR", msg = "you should use netip.ParsePrefix unless you really need a *net.IPNet" }, + { p = "^net.ParseIP", msg = "you should use netip.ParseAddr unless you really need a net.IP" }, + { p = "^pgtype.NewMap", msg = "you should use mmdatabase.NewTypeMap instead" }, + { p = "^serviceusage.NewService", msg = "you should use mmgcloud.NewServiceUsageService instead." }, + { p = "^sheets.NewService", msg = "you should use mmgcloud.NewSheetsService instead." }, + { p = "^storage.NewClient", msg = "you should use gstorage.NewClient instead. This sets the HTTP client settings that we need for internal use." }, + { p = "^os.IsNotExist", msg = "As per their docs, new code should use errors.Is(err, fs.ErrNotExist)." }, + { p = "^os.IsExist", msg = "As per their docs, new code should use errors.Is(err, fs.ErrExist)" }, + { p = "^net.LookupIP", msg = "You should use net.Resolver functions instead." }, + { p = "^net.LookupCNAME", msg = "You should use net.Resolver functions instead." }, + { p = "^net.LookupHost", msg = "You should use net.Resolver functions instead." }, + { p = "^net.LookupPort", msg = "You should use net.Resolver functions instead." }, + { p = "^net.LookupTXT", msg = "You should use net.Resolver functions instead." }, + { p = "^net.LookupAddr", msg = "You should use net.Resolver functions instead." }, + { p = "^net.LookupMX", msg = "You should use net.Resolver functions instead." }, + { p = "^net.LookupNS", msg = "You should use net.Resolver functions instead." }, + { p = "^net.LookupSRV", msg = "You should use net.Resolver functions instead." }, +] + +[linters-settings.gci] +sections = ["standard", "default", "prefix(github.com/maxmind/xgbshap)"] [linters-settings.gocritic] - enabled-checks = [ - "appendAssign", - "appendCombine", - "argOrder", - "assignOp", - "badCall", - "badCond", - "badLock", - "badRegexp", - "badSorting", - "boolExprSimplify", - "builtinShadow", - "builtinShadowDecl", - "captLocal", - "caseOrder", - "codegenComment", - "commentedOutCode", - "commentedOutImport", - "commentFormatting", - "defaultCaseOrder", - # Revive's defer rule already captures this. This caught no extra cases. - # "deferInLoop", - "deferUnlambda", - "deprecatedComment", - "docStub", - "dupArg", - "dupBranchBody", - "dupCase", - "dupImport", - "dupSubExpr", - "dynamicFmtString", - "elseif", - "emptyDecl", - "emptyFallthrough", - "emptyStringTest", - "equalFold", - "evalOrder", - "exitAfterDefer", - "exposedSyncMutex", - "externalErrorReassign", - # Given that all of our code runs on Linux and the / separate should - # work fine, this seems less important. - # "filepathJoin", - "flagDeref", - "flagName", - "hexLiteral", - # This seems like it could be good, but we would need to update current - # uses. It supports "--fix", but the fixing is a bit broken. - # "httpNoBody", - # This might be good, but we would have to revist a lot of code. - # "hugeParam", - "ifElseChain", - "importShadow", - "indexAlloc", - "initClause", - "mapKey", - "methodExprCall", - "nestingReduce", - "newDeref", - "nilValReturn", - "octalLiteral", - "offBy1", - "paramTypeCombine", - "preferDecodeRune", - "preferFilepathJoin", - "preferFprint", - "preferStringWriter", - "preferWriteByte", - "ptrToRefParam", - "rangeExprCopy", - "rangeValCopy", - "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", - "returnAfterHttpError", - "ruleguard", - "singleCaseSwitch", - "sliceClear", - "sloppyLen", - # This seems like it might also be good, but a lot of existing code - # fails. - # "sloppyReassign", - # This complains about helper functions in tests. - # "sloppyTestFuncName", - "sloppyTypeAssert", - "sortSlice", - "sprintfQuotedString", - "sqlQuery", - "stringsCompare", - "stringConcatSimplify", - "stringXbytes", - "switchTrue", - "syncMapLoadAndDelete", - "timeExprSimplify", - "todoCommentWithoutDetail", - "tooManyResultsChecker", - "truncateCmp", - "typeAssertChain", - "typeDefFirst", - "typeSwitchVar", - "typeUnparen", - "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", - ] +enable-all = true +disabled-checks = [ + # Revive's defer rule already captures this. This caught no extra cases. + "deferInLoop", + # Given that all of our code runs on Linux and the / separate should + # work fine, this seems less important. + "filepathJoin", + # This seems like it could be good, but we would need to update current + # uses. It supports "--fix", but the fixing is a bit broken. + "httpNoBody", + # This might be good, but we would have to revisit a lot of code. + "hugeParam", + # This might be good, but I don't think we want to encourage + # significant changes to regexes as we port stuff from Perl. + "regexpSimplify", + # This seems like it might also be good, but a lot of existing code + # fails. + "sloppyReassign", + # I am not sure we would want this linter and a lot of existing + # code fails. + "unnamedResult", + # Covered by nolintlint + "whyNoLint", +] [linters-settings.gofumpt] - extra-rules = true - lang-version = "1.18" +extra-rules = true +# IMPORTANT: gomodguard blocks _modules_, not arbitrary packages. Be +# sure to use the module path from the go.mod file for these. [linters-settings.gomodguard] - [[linters-settings.gomodguard.blocked.modules]] - [linters-settings.gomodguard.blocked.modules."github.com/avct/uasurfer"] - recommendations = ["github.com/xavivars/uasurfer"] - reason = "The original avct module appears abandoned." - - [[linters-settings.gomodguard.blocked.modules]] - [linters-settings.gomodguard.blocked.modules."github.com/BurntSushi/toml"] - recommendations = ["github.com/pelletier/go-toml/v2"] - reason = "This library panics frequently on invalid input." - - [[linters-settings.gomodguard.blocked.modules]] - [linters-settings.gomodguard.blocked.modules."github.com/gofrs/uuid"] - recommendations = ["github.com/google/uuid"] - - [[linters-settings.gomodguard.blocked.modules]] - [linters-settings.gomodguard.blocked.modules."github.com/pelletier/go-toml"] - recommendations = ["github.com/pelletier/go-toml/v2"] - reason = "This is an outdated version." - - [[linters-settings.gomodguard.blocked.modules]] - [linters-settings.gomodguard.blocked.modules."github.com/satori/go.uuid"] - recommendations = ["github.com/google/uuid"] - - [[linters-settings.gomodguard.blocked.modules]] - [linters-settings.gomodguard.blocked.modules."github.com/lib/pq"] - recommendations = ["github.com/jackc/pgx"] - reason = "This library is no longer actively maintained." - - [[linters-settings.gomodguard.blocked.modules]] - [linters-settings.gomodguard.blocked.modules."github.com/neilotoole/errgroup"] - recommendations = ["golang.org/x/sync/errgroup"] - reason = "This library can lead to subtle deadlocks in certain use cases." - - [[linters-settings.gomodguard.blocked.modules]] - [linters-settings.gomodguard.blocked.modules."github.com/pariz/gountries"] - reason = "This library's data is not actively maintained. Use GeoInfo data." - - [linters-settings.gomodguard.blocked.modules."github.com/pkg/errors"] - reason = "pkg/errors is no longer maintained." - - [[linters-settings.gomodguard.blocked.modules]] - [linters-settings.gomodguard.blocked.modules."github.com/RackSec/srslog"] - recommendations = ["github.com/RackSec/srslog"] - reason = "This library's data is not actively maintained." - - [[linters-settings.gomodguard.blocked.modules]] - [linters-settings.gomodguard.blocked.modules."github.com/ua-parser/uap-go/uaparser"] - recommendations = ["github.com/xavivars/uasurfer"] - reason = "The performance of this library is absolutely abysmal." - - [[linters-settings.gomodguard.blocked.modules]] - [linters-settings.gomodguard.blocked.modules."github.com/ugorji/go/codec"] - recommendations = ["encoding/json", "github.com/mailru/easyjson"] - reason = "This library is poorly maintained. We should default to using encoding/json and use easyjson where performance really matters." - - [[linters-settings.gomodguard.blocked.modules]] - [linters-settings.gomodguard.blocked.modules."io/ioutil"] - - [[linters-settings.gomodguard.blocked.versions]] - [linters-settings.gomodguard.blocked.versions."github.com/jackc/pgconn"] - reason = "Use github.com/jackc/pgx/v5" - - [[linters-settings.gomodguard.blocked.versions]] - [linters-settings.gomodguard.blocked.versions."github.com/jackc/pgtype"] - reason = "Use github.com/jackc/pgx/v5" - - [[linters-settings.gomodguard.blocked.versions]] - [linters-settings.gomodguard.blocked.versions."github.com/jackc/pgx"] - version = "< 5.0.0" - reason = "Use github.com/jackc/pgx/v5" - - [[linters-settings.gomodguard.blocked.versions]] - [linters-settings.gomodguard.blocked.versions."github.com/stretchr/testify/assert"] - reason = "Use github.com/stretchr/testify/assert" - - [[linters-settings.gomodguard.blocked.modules]] - [linters-settings.gomodguard.blocked.modules."inet.af/netaddr"] - recommendations = ["go4.org/netipx"] - reason = "inet.af/netaddr has been deprecated." +[[linters-settings.gomodguard.blocked.modules]] +[linters-settings.gomodguard.blocked.modules."github.com/avct/uasurfer"] +recommendations = ["github.com/xavivars/uasurfer"] +reason = "The original avct module appears abandoned." + +[[linters-settings.gomodguard.blocked.modules]] +[linters-settings.gomodguard.blocked.modules."github.com/BurntSushi/toml"] +recommendations = ["github.com/pelletier/go-toml/v2"] +reason = "This library panics frequently on invalid input." + +[[linters-settings.gomodguard.blocked.modules]] +[linters-settings.gomodguard.blocked.modules."github.com/pelletier/go-toml"] +recommendations = ["github.com/pelletier/go-toml/v2"] +reason = "This is an outdated version." + +[[linters-settings.gomodguard.blocked.modules]] +[linters-settings.gomodguard.blocked.modules."github.com/gofrs/uuid"] +recommendations = ["github.com/google/uuid"] + +[[linters-settings.gomodguard.blocked.modules]] +[linters-settings.gomodguard.blocked.modules."github.com/gofrs/uuid/v5"] +recommendations = ["github.com/google/uuid"] + +[[linters-settings.gomodguard.blocked.modules]] +[linters-settings.gomodguard.blocked.modules."github.com/satori/go.uuid"] +recommendations = ["github.com/google/uuid"] + +[[linters-settings.gomodguard.blocked.modules]] +[linters-settings.gomodguard.blocked.modules."github.com/lib/pq"] +recommendations = ["github.com/jackc/pgx"] +reason = "This library is no longer actively maintained." + +[[linters-settings.gomodguard.blocked.modules]] +[linters-settings.gomodguard.blocked.modules."github.com/neilotoole/errgroup"] +recommendations = ["golang.org/x/sync/errgroup"] +reason = "This library can lead to subtle deadlocks in certain use cases." + +[[linters-settings.gomodguard.blocked.modules]] +[linters-settings.gomodguard.blocked.modules."github.com/pariz/gountries"] +reason = "This library's data is not actively maintained. Use GeoInfo data." + +[linters-settings.gomodguard.blocked.modules."github.com/pkg/errors"] +recommendations = ["github.maxmind.com/maxmind/mm_website/go/pkg/mmerrors"] +reason = "pkg/errors is no longer maintained." + +[[linters-settings.gomodguard.blocked.modules]] +[linters-settings.gomodguard.blocked.modules."github.com/RackSec/srslog"] +recommendations = ["log/syslog"] +reason = "This library's data is not actively maintained." + +[[linters-settings.gomodguard.blocked.modules]] +[linters-settings.gomodguard.blocked.modules."github.com/ua-parser/uap-go"] +recommendations = ["github.com/xavivars/uasurfer"] +reason = "The performance of this library is absolutely abysmal." + +[[linters-settings.gomodguard.blocked.modules]] +[linters-settings.gomodguard.blocked.modules."github.com/ugorji/go"] +recommendations = ["encoding/json", "github.com/mailru/easyjson"] +reason = "This library is poorly maintained. We should default to using encoding/json and use easyjson where performance really matters." + +[[linters-settings.gomodguard.blocked.modules]] +[linters-settings.gomodguard.blocked.modules."gotest.tools/v3"] +recommendations = ["github.com/stretchr/testify/assert"] +reason = "Use github.com/stretchr/testify/assert" + +[[linters-settings.gomodguard.blocked.modules]] +[linters-settings.gomodguard.blocked.modules."inet.af/netaddr"] +recommendations = ["net/netip", "go4.org/netipx"] +reason = "inet.af/netaddr has been deprecated." + +[[linters-settings.gomodguard.blocked.versions]] +[linters-settings.gomodguard.blocked.versions."github.com/jackc/pgconn"] +reason = "Use github.com/jackc/pgx/v5" + +[[linters-settings.gomodguard.blocked.versions]] +[linters-settings.gomodguard.blocked.versions."github.com/jackc/pgtype"] +reason = "Use github.com/jackc/pgx/v5" + +[[linters-settings.gomodguard.blocked.versions]] +[linters-settings.gomodguard.blocked.versions."github.com/jackc/pgx"] +version = "< 5.0.0" +reason = "Use github.com/jackc/pgx/v5" [linters-settings.gosec] - excludes = [ - # G104 - "Audit errors not checked." We use errcheck for this. - "G104", +excludes = [ + # G104 - "Audit errors not checked." We use errcheck for this. + "G104", + + # G306 - "Expect WriteFile permissions to be 0600 or less". + "G306", - # G306 - "Expect WriteFile permissions to be 0600 or less". - "G306", + # Prohibits defer (*os.File).Close, which we allow when reading from file. + "G307", - # Prohibits defer (*os.File).Close, which we allow when reading from file. - "G307", - ] + # no longer relevant with 1.22 + "G601", + # xgbshap uses md5 + "G401", + "G501", +] [linters-settings.govet] - # This seems to be duplicate setting, but enable it for good measure. - check-shadowing = true - "enable-all" = true +"enable-all" = true - # Although it is very useful in particular cases where we are trying to - # use as little memory as possible, there are even more cases where - # other organizations may make more sense. - disable = ["fieldalignment"] +# Although it is very useful in particular cases where we are trying to +# use as little memory as possible, there are even more cases where +# other organizations may make more sense. +disable = ["fieldalignment"] - [linters-settings.govet.settings.shadow] - strict = true +[linters-settings.govet.settings.shadow] +strict = true [linters-settings.lll] - line-length = 120 - tab-width = 4 - -[linters-settings.nolintlint] - allow-leading-space = false - allow-unused = false - allow-no-explanation = ["lll", "misspell"] - require-explanation = true - require-specific = true - -[linters-settings.revive] - 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" - - # [[linters-settings.revive.rules]] - # name = "argument-limit" - - [[linters-settings.revive.rules]] - name = "atomic" - - [[linters-settings.revive.rules]] - name = "bare-return" - - [[linters-settings.revive.rules]] - name = "blank-imports" - - [[linters-settings.revive.rules]] - name = "bool-literal-in-expr" - - [[linters-settings.revive.rules]] - name = "call-to-gc" - - # [[linters-settings.revive.rules]] - # name = "cognitive-complexity" - - [[linters-settings.revive.rules]] - name = "comment-spacings" - arguments = ["easyjson", "nolint"] - - # Probably a good rule, but we have a lot of names that - # only have case differences. - # [[linters-settings.revive.rules]] - # name = "confusing-naming" - - [[linters-settings.revive.rules]] - name = "confusing-results" - - [[linters-settings.revive.rules]] - name = "constant-logical-expr" - - [[linters-settings.revive.rules]] - name = "context-as-argument" - - [[linters-settings.revive.rules]] - name = "context-keys-type" - - # [[linters-settings.revive.rules]] - # name = "cyclomatic" - - [[linters-settings.revive.rules]] - name = "datarace" +line-length = 120 +tab-width = 4 - [[linters-settings.revive.rules]] - name = "deep-exit" +[linters-settings.misspell] +locale = "US" - [[linters-settings.revive.rules]] - name = "defer" +[[linters-settings.misspell.extra-words]] +typo = "marshall" +correction = "marshal" - [[linters-settings.revive.rules]] - name = "dot-imports" +[[linters-settings.misspell.extra-words]] +typo = "marshalling" +correction = "marshaling" - [[linters-settings.revive.rules]] - name = "duplicated-imports" +[[linters-settings.misspell.extra-words]] +typo = "marshalls" +correction = "marshals" - [[linters-settings.revive.rules]] - name = "early-return" +[[linters-settings.misspell.extra-words]] +typo = "unmarshall" +correction = "unmarshal" - [[linters-settings.revive.rules]] - name = "empty-block" +[[linters-settings.misspell.extra-words]] +typo = "unmarshalling" +correction = "unmarshaling" - [[linters-settings.revive.rules]] - name = "empty-lines" +[[linters-settings.misspell.extra-words]] +typo = "unmarshalls" +correction = "unmarshals" - [[linters-settings.revive.rules]] - name = "errorf" - - [[linters-settings.revive.rules]] - name = "error-naming" - - [[linters-settings.revive.rules]] - name = "error-return" - - [[linters-settings.revive.rules]] - name = "error-strings" - - [[linters-settings.revive.rules]] - name = "exported" - - # [[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" - - # [[linters-settings.revive.rules]] - # name = "function-result-limit" - - [[linters-settings.revive.rules]] - name = "get-return" - - [[linters-settings.revive.rules]] - name = "identical-branches" - - [[linters-settings.revive.rules]] - name = "if-return" - - [[linters-settings.revive.rules]] - name = "imports-blacklist" - - [[linters-settings.revive.rules]] - name = "import-shadowing" - - [[linters-settings.revive.rules]] - name = "increment-decrement" - - [[linters-settings.revive.rules]] - name = "indent-error-flow" - - # [[linters-settings.revive.rules]] - # name = "line-length-limit" - - # [[linters-settings.revive.rules]] - # name = "max-public-structs" - - [[linters-settings.revive.rules]] - name = "modifies-parameter" - - [[linters-settings.revive.rules]] - name = "modifies-value-receiver" - - # We frequently use nested structs, particularly in tests. - # [[linters-settings.revive.rules]] - # name = "nested-structs" - - [[linters-settings.revive.rules]] - name = "optimize-operands-order" - - [[linters-settings.revive.rules]] - name = "package-comments" - - [[linters-settings.revive.rules]] - name = "range" - - [[linters-settings.revive.rules]] - name = "range-val-address" - - [[linters-settings.revive.rules]] - name = "range-val-in-closure" - - [[linters-settings.revive.rules]] - name = "receiver-naming" - - [[linters-settings.revive.rules]] - name = "redefines-builtin-id" - - [[linters-settings.revive.rules]] - name = "string-of-int" - - [[linters-settings.revive.rules]] - name = "struct-tag" - - [[linters-settings.revive.rules]] - name = "superfluous-else" - - [[linters-settings.revive.rules]] - name = "time-equal" - - [[linters-settings.revive.rules]] - name = "time-naming" - - [[linters-settings.revive.rules]] - name = "unconditional-recursion" - - [[linters-settings.revive.rules]] - name = "unexported-naming" - - [[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" - - [[linters-settings.revive.rules]] - name = "unnecessary-stmt" +[linters-settings.nolintlint] +allow-unused = false +allow-no-explanation = ["lll", "misspell"] +require-explanation = true +require-specific = true - [[linters-settings.revive.rules]] - name = "unreachable-code" +[linters-settings.revive] +enable-all-rules = true +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" +disabled = true + +[[linters-settings.revive.rules]] +name = "argument-limit" +disabled = true + +[[linters-settings.revive.rules]] +name = "cognitive-complexity" +disabled = true + +[[linters-settings.revive.rules]] +name = "comment-spacings" +arguments = ["easyjson", "nolint"] +disabled = false + +# Probably a good rule, but we have a lot of names that +# only have case differences. +[[linters-settings.revive.rules]] +name = "confusing-naming" +disabled = true + +[[linters-settings.revive.rules]] +name = "cyclomatic" +disabled = true + +# Although being consistent might be nice, I don't know that it +# is worth the effort enabling this rule. It doesn't have an +# autofix option. +[[linters-settings.revive.rules]] +name = "enforce-repeated-arg-type-style" +arguments = ["short"] +disabled = true + +[[linters-settings.revive.rules]] +name = "enforce-map-style" +arguments = ["literal"] +disabled = false + +# We have very few of these as we force nil slices in most places, +# but there are a couple of cases. +[[linters-settings.revive.rules]] +name = "enforce-slice-style" +arguments = ["literal"] +disabled = false + +[[linters-settings.revive.rules]] +name = "file-header" +disabled = true + +# 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" +disabled = true + +[[linters-settings.revive.rules]] +name = "function-length" +disabled = true + +[[linters-settings.revive.rules]] +name = "function-result-limit" +disabled = true + +[[linters-settings.revive.rules]] +name = "line-length-limit" +disabled = true + +[[linters-settings.revive.rules]] +name = "max-public-structs" +disabled = true + +# We frequently use nested structs, particularly in tests. +[[linters-settings.revive.rules]] +name = "nested-structs" +disabled = true + +# This doesn't make sense with 1.22 loop var changes. +[[linters-settings.revive.rules]] +name = "range-val-address" +disabled = true + +# This causes a ton of failures. Many are fairly safe. It might be nice to +# enable, but probably not worth the effort. +[[linters-settings.revive.rules]] +name = "unchecked-type-assertion" +disabled = true + +# This is covered elsewhere and we want to ignore some +# functions such as fmt.Fprintf. +[[linters-settings.revive.rules]] +name = "unhandled-error" +disabled = true + +# We generally have unused receivers in tests for meeting the +# requirements of an interface. +[[linters-settings.revive.rules]] +name = "unused-receiver" +disabled = true + +[linters-settings.tagliatelle.case.rules] +avro = "snake" +bson = "snake" +env = "upperSnake" +envconfig = "upperSnake" +json = "snake" +mapstructure = "snake" +xml = "snake" +yaml = "snake" - [[linters-settings.revive.rules]] - name = "unused-parameter" +[linters-settings.unparam] +check-exported = true + +[linters-settings.wrapcheck] +"ignoreSigs" = [ + ".Errorf(", + "errgroup.NewMultiError(", + "errors.Join(", + "errors.New(", + ".Wait(", + ".WithStack(", + ".Wrap(", + ".Wrapf(", + "v4.Retry(", + "v4.RetryNotify(", +] - # We generally have unused receivers in tests for meeting the - # requirements of an interface. - # [[linters-settings.revive.rules]] - # name = "unused-receiver" +[issues] +exclude-use-default = false - [[linters-settings.revive.rules]] - name = "use-any" +exclude-dirs = [ + "geoip-build/mmcsv", +] - [[linters-settings.revive.rules]] - name = "useless-break" +exclude-files = [ + "_easyjson\\.go$", + "_easyjson_test\\.go$", + "_xgb2code\\.go$", + "_json2vector\\.go$", +] - [[linters-settings.revive.rules]] - name = "var-declaration" +[[issues.exclude-rules]] +linters = [ + "bodyclose", +] +# This rule doesn't really make sense for tests where we don't have an open +# connection and we might be passing around the response for other reasons. +path = "_test.go" - [[linters-settings.revive.rules]] - name = "var-naming" +[[issues.exclude-rules]] +linters = [ + "forbidigo", +] +# This refers to a minFraud field, not the MaxMind Account ID +source = "AccountUserID|Account\\.UserID" + +# we include both a source and text exclusion as the source exclusion +# misses matches where forbidigo reports the error on the first line +# of a chunk of a function call even though the use is on a later line. +[[issues.exclude-rules]] +linters = [ + "forbidigo", +] +text = "AccountUserID|Account\\.UserID" - [[linters-settings.revive.rules]] - name = "waitgroup-by-value" +[[issues.exclude-rules]] +linters = [ + "gocritic", +] +# For some reason the imports stuff in ruleguard doesn't work in golangci-lint. +# Perhaps it has an outdated version or something +path = "_test.go" +text = "ruleguard: Prefer the alternative Context method instead" + +[[issues.exclude-rules]] +linters = [ + "gocritic", +] +# The nolintlint linter behaves oddly with ruleguard rules +source = "// *no-ruleguard" -[linters-settings.unparam] - check-exported = true +[[issues.exclude-rules]] +linters = [ + "nolintlint", +] +# The contextcheck linter also uses "nolint" in a slightly different way, +# leading to falso positives from nolintlint. +source = "//nolint:contextcheck //.*" -[issues] -exclude-use-default = false +[[issues.exclude-rules]] +linters = [ + "govet", +] +# These are usually fine to shadow and not allowing shadowing for them can +# make the code unnecessarily verbose. +text = 'shadow: declaration of "(ctx|err|ok)" shadows declaration' - # This goes off for MD5 usage, which we use heavily - [[issues.exclude-rules]] - text = "weak cryptographic primitive" - linters = ["gosec"] - - [[issues.exclude-rules]] - linters = [ - "bodyclose" - ] - # This rule doesn't really make sense for tests where we don't have an open - # connection and we might be passing around the response for other reasons. - path = "_test.go" - - [[issues.exclude-rules]] - linters = [ - "forbidigo" - ] - # This refers to a minFraud field, not the MaxMind Account ID - text = "AccountUserID|Account\\.UserID" - - [[issues.exclude-rules]] - linters = [ - "gocritic" - ] - # For some reason the imports stuff in ruleguard doesn't work in golangci-lint. - # Perhaps it has an outdated version or something - path = "_test.go" - text = "ruleguard: Prefer the alternative Context method instead" - - [[issues.exclude-rules]] - linters = [ - "gocritic" - ] - # The nolintlint linter behaves oddly with ruleguard rules - source = "// *no-ruleguard" - - [[issues.exclude-rules]] - linters = [ - "govet" - ] - # These are usually fine to shadow and not allowing shadowing for them can - # make the code unnecessarily verbose. - text = 'shadow: declaration of "(ctx|err|ok)" shadows declaration' - - [[issues.exclude-rules]] - linters = [ +[[issues.exclude-rules]] +linters = [ "contextcheck", + # With recent changes to the linter, there were a lot of failures in + # the tests and it wasn't clear to me that fixing them would actually + # improve the readability. + "goconst", "nilerr", "wrapcheck", - ] - path = "_test.go" +] +path = "_test.go" - [[issues.exclude-rules]] - linters = [ +[[issues.exclude-rules]] +linters = [ "stylecheck", - ] - # ST1016 - methods on the same type should have the same receiver name. - # easyjson doesn't interact well with this. - text = "ST1016" +] +# ST1016 - methods on the same type should have the same receiver name. +# easyjson doesn't interact well with this. +text = "ST1016" - [[issues.exclude-rules]] - linters = [ +[[issues.exclude-rules]] +linters = [ "staticcheck", - ] - # SA5008: unknown JSON option "intern" - easyjson specific option. - text = 'SA5008: unknown JSON option "intern"' +] +# SA5008: unknown JSON option "intern" - easyjson specific option. +text = 'SA5008: unknown JSON option "intern"' + +[[issues.exclude-rules]] +linters = [ + "wrapcheck", +] +text = "github.com/maxmind/xgbshap" - [[issues.exclude-rules]] - linters = [ +[[issues.exclude-rules]] +linters = [ "wrapcheck", - ] - path = "_easyjson.go" +] +path = "_easyjson.go" - [[issues.exclude-rules]] - linters = [ +[[issues.exclude-rules]] +linters = [ "gocritic", - ] - source = "Chmod|WriteFile" - text = "octalLiteral" +] +source = "Chmod|WriteFile" +text = "octalLiteral" diff --git a/cmd/calculate-contributions/main_test.go b/cmd/calculate-contributions/main_test.go index 0c4bfd4..279f4ba 100644 --- a/cmd/calculate-contributions/main_test.go +++ b/cmd/calculate-contributions/main_test.go @@ -15,7 +15,7 @@ func TestPredictContributions(t *testing.T) { ) require.NoError(t, err) - require.Equal(t, 2, len(contributionsSets)) + require.Len(t, contributionsSets, 2) assert.Equal( t, diff --git a/go.mod b/go.mod index 692ce98..89b0788 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/maxmind/xgbshap -go 1.20 +go 1.21 require github.com/stretchr/testify v1.9.0 diff --git a/parse.go b/parse.go index d2ad5c7..1135336 100644 --- a/parse.go +++ b/parse.go @@ -3,7 +3,6 @@ package xgbshap import ( "encoding/json" "fmt" - "math" "os" "path/filepath" ) @@ -99,7 +98,7 @@ func (n *Node) MaxDepth() int { leftDepth := n.Left.MaxDepth() + 1 rightDepth := n.Right.MaxDepth() + 1 - return int(math.Max(float64(leftDepth), float64(rightDepth))) + return max(leftDepth, rightDepth) } func parseModel(