From db92f8e742038f7039e85cd5541d440141b47d25 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Sun, 18 Jun 2023 13:57:12 -0700 Subject: [PATCH 1/3] Improve alignment in structs --- reader.go | 152 +++++++++++++++++++++++++++--------------------------- 1 file changed, 76 insertions(+), 76 deletions(-) diff --git a/reader.go b/reader.go index cedddac..70980b2 100644 --- a/reader.go +++ b/reader.go @@ -17,117 +17,117 @@ import ( // The Enterprise struct corresponds to the data in the GeoIP2 Enterprise // database. type Enterprise struct { - City struct { - Confidence uint8 `maxminddb:"confidence"` - GeoNameID uint `maxminddb:"geoname_id"` - Names map[string]string `maxminddb:"names"` - } `maxminddb:"city"` Continent struct { + Names map[string]string `maxminddb:"names"` Code string `maxminddb:"code"` GeoNameID uint `maxminddb:"geoname_id"` - Names map[string]string `maxminddb:"names"` } `maxminddb:"continent"` - Country struct { - GeoNameID uint `maxminddb:"geoname_id"` - IsoCode string `maxminddb:"iso_code"` - Names map[string]string `maxminddb:"names"` - Confidence uint8 `maxminddb:"confidence"` - IsInEuropeanUnion bool `maxminddb:"is_in_european_union"` - } `maxminddb:"country"` - Location struct { - AccuracyRadius uint16 `maxminddb:"accuracy_radius"` - Latitude float64 `maxminddb:"latitude"` - Longitude float64 `maxminddb:"longitude"` - MetroCode uint `maxminddb:"metro_code"` - TimeZone string `maxminddb:"time_zone"` - } `maxminddb:"location"` + City struct { + Names map[string]string `maxminddb:"names"` + GeoNameID uint `maxminddb:"geoname_id"` + Confidence uint8 `maxminddb:"confidence"` + } `maxminddb:"city"` Postal struct { Code string `maxminddb:"code"` Confidence uint8 `maxminddb:"confidence"` } `maxminddb:"postal"` - RegisteredCountry struct { - GeoNameID uint `maxminddb:"geoname_id"` + Subdivisions []struct { + Names map[string]string `maxminddb:"names"` + IsoCode string `maxminddb:"iso_code"` + GeoNameID uint `maxminddb:"geoname_id"` + Confidence uint8 `maxminddb:"confidence"` + } `maxminddb:"subdivisions"` + RepresentedCountry struct { + Names map[string]string `maxminddb:"names"` IsoCode string `maxminddb:"iso_code"` + Type string `maxminddb:"type"` + GeoNameID uint `maxminddb:"geoname_id"` + IsInEuropeanUnion bool `maxminddb:"is_in_european_union"` + } `maxminddb:"represented_country"` + Country struct { Names map[string]string `maxminddb:"names"` + IsoCode string `maxminddb:"iso_code"` + GeoNameID uint `maxminddb:"geoname_id"` Confidence uint8 `maxminddb:"confidence"` IsInEuropeanUnion bool `maxminddb:"is_in_european_union"` - } `maxminddb:"registered_country"` - RepresentedCountry struct { + } `maxminddb:"country"` + RegisteredCountry struct { + Names map[string]string `maxminddb:"names"` + IsoCode string `maxminddb:"iso_code"` GeoNameID uint `maxminddb:"geoname_id"` + Confidence uint8 `maxminddb:"confidence"` IsInEuropeanUnion bool `maxminddb:"is_in_european_union"` - IsoCode string `maxminddb:"iso_code"` - Names map[string]string `maxminddb:"names"` - Type string `maxminddb:"type"` - } `maxminddb:"represented_country"` - Subdivisions []struct { - Confidence uint8 `maxminddb:"confidence"` - GeoNameID uint `maxminddb:"geoname_id"` - IsoCode string `maxminddb:"iso_code"` - Names map[string]string `maxminddb:"names"` - } `maxminddb:"subdivisions"` + } `maxminddb:"registered_country"` Traits struct { - AutonomousSystemNumber uint `maxminddb:"autonomous_system_number"` AutonomousSystemOrganization string `maxminddb:"autonomous_system_organization"` ConnectionType string `maxminddb:"connection_type"` Domain string `maxminddb:"domain"` - IsAnonymousProxy bool `maxminddb:"is_anonymous_proxy"` - IsLegitimateProxy bool `maxminddb:"is_legitimate_proxy"` - IsSatelliteProvider bool `maxminddb:"is_satellite_provider"` ISP string `maxminddb:"isp"` MobileCountryCode string `maxminddb:"mobile_country_code"` MobileNetworkCode string `maxminddb:"mobile_network_code"` Organization string `maxminddb:"organization"` - StaticIPScore float64 `maxminddb:"static_ip_score"` UserType string `maxminddb:"user_type"` + AutonomousSystemNumber uint `maxminddb:"autonomous_system_number"` + StaticIPScore float64 `maxminddb:"static_ip_score"` + IsAnonymousProxy bool `maxminddb:"is_anonymous_proxy"` + IsLegitimateProxy bool `maxminddb:"is_legitimate_proxy"` + IsSatelliteProvider bool `maxminddb:"is_satellite_provider"` } `maxminddb:"traits"` + Location struct { + TimeZone string `maxminddb:"time_zone"` + Latitude float64 `maxminddb:"latitude"` + Longitude float64 `maxminddb:"longitude"` + MetroCode uint `maxminddb:"metro_code"` + AccuracyRadius uint16 `maxminddb:"accuracy_radius"` + } `maxminddb:"location"` } // The City struct corresponds to the data in the GeoIP2/GeoLite2 City // databases. type City struct { City struct { - GeoNameID uint `maxminddb:"geoname_id"` Names map[string]string `maxminddb:"names"` + GeoNameID uint `maxminddb:"geoname_id"` } `maxminddb:"city"` + Postal struct { + Code string `maxminddb:"code"` + } `maxminddb:"postal"` Continent struct { + Names map[string]string `maxminddb:"names"` Code string `maxminddb:"code"` GeoNameID uint `maxminddb:"geoname_id"` - Names map[string]string `maxminddb:"names"` } `maxminddb:"continent"` - Country struct { + Subdivisions []struct { + Names map[string]string `maxminddb:"names"` + IsoCode string `maxminddb:"iso_code"` + GeoNameID uint `maxminddb:"geoname_id"` + } `maxminddb:"subdivisions"` + RepresentedCountry struct { + Names map[string]string `maxminddb:"names"` + IsoCode string `maxminddb:"iso_code"` + Type string `maxminddb:"type"` GeoNameID uint `maxminddb:"geoname_id"` IsInEuropeanUnion bool `maxminddb:"is_in_european_union"` - IsoCode string `maxminddb:"iso_code"` + } `maxminddb:"represented_country"` + Country struct { Names map[string]string `maxminddb:"names"` + IsoCode string `maxminddb:"iso_code"` + GeoNameID uint `maxminddb:"geoname_id"` + IsInEuropeanUnion bool `maxminddb:"is_in_european_union"` } `maxminddb:"country"` + RegisteredCountry struct { + Names map[string]string `maxminddb:"names"` + IsoCode string `maxminddb:"iso_code"` + GeoNameID uint `maxminddb:"geoname_id"` + IsInEuropeanUnion bool `maxminddb:"is_in_european_union"` + } `maxminddb:"registered_country"` Location struct { - AccuracyRadius uint16 `maxminddb:"accuracy_radius"` + TimeZone string `maxminddb:"time_zone"` Latitude float64 `maxminddb:"latitude"` Longitude float64 `maxminddb:"longitude"` MetroCode uint `maxminddb:"metro_code"` - TimeZone string `maxminddb:"time_zone"` + AccuracyRadius uint16 `maxminddb:"accuracy_radius"` } `maxminddb:"location"` - Postal struct { - Code string `maxminddb:"code"` - } `maxminddb:"postal"` - RegisteredCountry struct { - GeoNameID uint `maxminddb:"geoname_id"` - IsInEuropeanUnion bool `maxminddb:"is_in_european_union"` - IsoCode string `maxminddb:"iso_code"` - Names map[string]string `maxminddb:"names"` - } `maxminddb:"registered_country"` - RepresentedCountry struct { - GeoNameID uint `maxminddb:"geoname_id"` - IsInEuropeanUnion bool `maxminddb:"is_in_european_union"` - IsoCode string `maxminddb:"iso_code"` - Names map[string]string `maxminddb:"names"` - Type string `maxminddb:"type"` - } `maxminddb:"represented_country"` - Subdivisions []struct { - GeoNameID uint `maxminddb:"geoname_id"` - IsoCode string `maxminddb:"iso_code"` - Names map[string]string `maxminddb:"names"` - } `maxminddb:"subdivisions"` Traits struct { IsAnonymousProxy bool `maxminddb:"is_anonymous_proxy"` IsSatelliteProvider bool `maxminddb:"is_satellite_provider"` @@ -138,28 +138,28 @@ type City struct { // Country databases. type Country struct { Continent struct { + Names map[string]string `maxminddb:"names"` Code string `maxminddb:"code"` GeoNameID uint `maxminddb:"geoname_id"` - Names map[string]string `maxminddb:"names"` } `maxminddb:"continent"` Country struct { + Names map[string]string `maxminddb:"names"` + IsoCode string `maxminddb:"iso_code"` GeoNameID uint `maxminddb:"geoname_id"` IsInEuropeanUnion bool `maxminddb:"is_in_european_union"` - IsoCode string `maxminddb:"iso_code"` - Names map[string]string `maxminddb:"names"` } `maxminddb:"country"` RegisteredCountry struct { + Names map[string]string `maxminddb:"names"` + IsoCode string `maxminddb:"iso_code"` GeoNameID uint `maxminddb:"geoname_id"` IsInEuropeanUnion bool `maxminddb:"is_in_european_union"` - IsoCode string `maxminddb:"iso_code"` - Names map[string]string `maxminddb:"names"` } `maxminddb:"registered_country"` RepresentedCountry struct { - GeoNameID uint `maxminddb:"geoname_id"` - IsInEuropeanUnion bool `maxminddb:"is_in_european_union"` - IsoCode string `maxminddb:"iso_code"` Names map[string]string `maxminddb:"names"` + IsoCode string `maxminddb:"iso_code"` Type string `maxminddb:"type"` + GeoNameID uint `maxminddb:"geoname_id"` + IsInEuropeanUnion bool `maxminddb:"is_in_european_union"` } `maxminddb:"represented_country"` Traits struct { IsAnonymousProxy bool `maxminddb:"is_anonymous_proxy"` @@ -180,8 +180,8 @@ type AnonymousIP struct { // The ASN struct corresponds to the data in the GeoLite2 ASN database. type ASN struct { - AutonomousSystemNumber uint `maxminddb:"autonomous_system_number"` AutonomousSystemOrganization string `maxminddb:"autonomous_system_organization"` + AutonomousSystemNumber uint `maxminddb:"autonomous_system_number"` } // The ConnectionType struct corresponds to the data in the GeoIP2 @@ -197,12 +197,12 @@ type Domain struct { // The ISP struct corresponds to the data in the GeoIP2 ISP database. type ISP struct { - AutonomousSystemNumber uint `maxminddb:"autonomous_system_number"` AutonomousSystemOrganization string `maxminddb:"autonomous_system_organization"` ISP string `maxminddb:"isp"` MobileCountryCode string `maxminddb:"mobile_country_code"` MobileNetworkCode string `maxminddb:"mobile_network_code"` Organization string `maxminddb:"organization"` + AutonomousSystemNumber uint `maxminddb:"autonomous_system_number"` } type databaseType int From c5861217c9eb63288806e2c69c8bcec28a902587 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Sun, 18 Jun 2023 13:57:36 -0700 Subject: [PATCH 2/3] Update golangci-lint config --- .golangci.toml | 114 ++++++++++++++++++++++++++++++------------------- 1 file changed, 69 insertions(+), 45 deletions(-) diff --git a/.golangci.toml b/.golangci.toml index 2f89080..0950c1c 100644 --- a/.golangci.toml +++ b/.golangci.toml @@ -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", @@ -42,6 +43,7 @@ "nosprintfhostport", "predeclared", "revive", + "rowserrcheck", "sqlclosecheck", "staticcheck", "stylecheck", @@ -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. @@ -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", @@ -89,8 +107,7 @@ "commentedOutImport", "commentFormatting", "defaultCaseOrder", - # Revive's defer rule already captures this. This caught no extra cases. - # "deferInLoop", + "deferInLoop", "deferUnlambda", "deprecatedComment", "docStub", @@ -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", @@ -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", @@ -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 @@ -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" @@ -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" @@ -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" @@ -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" @@ -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" @@ -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" @@ -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" @@ -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" @@ -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" From 0a1d18fab277fc36caa2564a235c3c60cc35c9ec Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Sun, 18 Jun 2023 13:59:14 -0700 Subject: [PATCH 3/3] Update Go modules --- go.mod | 4 ++-- go.sum | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/go.mod b/go.mod index 704274c..13c7efc 100644 --- a/go.mod +++ b/go.mod @@ -3,13 +3,13 @@ module github.com/oschwald/geoip2-golang go 1.19 require ( - github.com/oschwald/maxminddb-golang v1.10.0 + github.com/oschwald/maxminddb-golang v1.11.0 github.com/stretchr/testify v1.8.4 ) require ( github.com/davecgh/go-spew v1.1.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect - golang.org/x/sys v0.5.0 // indirect + golang.org/x/sys v0.9.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index 6d064f9..6cdc8e5 100644 --- a/go.sum +++ b/go.sum @@ -1,13 +1,13 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/oschwald/maxminddb-golang v1.10.0 h1:Xp1u0ZhqkSuopaKmk1WwHtjF0H9Hd9181uj2MQ5Vndg= -github.com/oschwald/maxminddb-golang v1.10.0/go.mod h1:Y2ELenReaLAZ0b400URyGwvYxHV1dLIxBuyOsyYjHK0= +github.com/oschwald/maxminddb-golang v1.11.0 h1:aSXMqYR/EPNjGE8epgqwDay+P30hCBZIveY0WZbAWh0= +github.com/oschwald/maxminddb-golang v1.11.0/go.mod h1:YmVI+H0zh3ySFR3w+oz8PCfglAFj3PuCmui13+P9zDg= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= -golang.org/x/sys v0.5.0 h1:MUK/U/4lj1t1oPg0HfuXDN/Z1wv31ZJ/YcPiGccS4DU= -golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.9.0 h1:KS/R3tvhPqvJvwcKfnBHJwwthS11LRhmM5D59eEXa0s= +golang.org/x/sys v0.9.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=