Merge pull request #14 from PDOK/linting

Update linting rules as discussed

.github/workflows/lint-go.yml

@@ -26,7 +26,7 @@ jobs:
         uses: golangci/golangci-lint-action@v3
         with:
           # Optional: version of golangci-lint to use in form of v1.2 or v1.2.3 or `latest` to use the latest version
-          version: v1.54
+          version: latest
 
           # Optional: working directory, useful for monorepos
           # working-directory: somedir

.golangci.yaml

@@ -33,10 +33,12 @@ linters-settings:
             desc: Should be replaced by standard lib errors package
   cyclop:
     # The maximal code complexity to report.
-    max-complexity: 12
+    max-complexity: 15
     skip-tests: true
   funlen:
     lines: 100
+  nestif:
+    min-complexity: 6
 
 linters:
   disable-all: true
@@ -77,16 +79,16 @@ linters:
     - nakedret  # finds naked returns in functions greater than a specified function length
     - nestif  # reports deeply nested if statements
     - nilerr  # finds the code that returns nil even if it checks that the error is not nil
-    - nilnil  # checks that there is no simultaneous return of nil error and an invalid value
     - nolintlint  # reports ill-formed or insufficient nolint directives
-    - nonamedreturns  # reports all named returns
     - nosprintfhostport  # checks for misuse of Sprintf to construct a host with port in a URL
+    - perfsprint  # Golang linter for performance, aiming at usages of fmt.Sprintf which have faster alternatives
     - predeclared  # finds code that shadows one of Go's predeclared identifiers
     - promlinter  # checks Prometheus metrics naming via promlint
     - reassign  # checks that package variables are not reassigned
     - revive  # fast, configurable, extensible, flexible, and beautiful linter for Go, drop-in replacement of golint
     - rowserrcheck  # checks whether Err of rows is checked successfully
     - sqlclosecheck  # checks that sql.Rows and sql.Stmt are closed
+    - sloglint  # A Go linter that ensures consistent code style when using log/slog
     - tenv  # detects using os.Setenv instead of t.Setenv since Go1.17
     - testableexamples  # checks if examples are testable (have an expected output)
     - tparallel  # detects inappropriate usage of t.Parallel() method in your Go test codes

intgeom/line.go

@@ -1,8 +1,9 @@
 package intgeom
 
 import (
-	"github.com/go-spatial/geom"
 	"log"
+
+	"github.com/go-spatial/geom"
 )
 
 // Line has exactly two points

main.go

@@ -115,7 +115,7 @@ func main() {
 			log.Printf("  snapping %s", table.Name)
 			source.Table = table
 			target.Table = table
-			snap.SnapToPointCloud(source, &target, tileMatrix)
+			snap.ToPointCloud(source, &target, tileMatrix)
 			log.Printf("  finished %s", table.Name)
 		}
 

processing/gpkg/gpkg.go

@@ -82,6 +82,7 @@ func (source SourceGeopackage) Close() {
 	source.handle.Close()
 }
 
+//nolint:funlen,cyclop
 func (source SourceGeopackage) ReadFeatures(features chan<- processing.Feature) {
 
 	rows, err := source.handle.Query(source.Table.selectSQL())
@@ -119,9 +120,7 @@ func (source SourceGeopackage) ReadFeatures(features chan<- processing.Feature)
 				switch v := vals[i].(type) {
 				case []uint8:
 					asBytes := make([]byte, len(v))
-					for j := 0; j < len(v); j++ {
-						asBytes[j] = v[j]
-					}
+					copy(asBytes, v)
 					c = append(c, string(asBytes))
 				case int64:
 					c = append(c, v)
@@ -152,7 +151,7 @@ func (source SourceGeopackage) ReadFeatures(features chan<- processing.Feature)
 
 func (source SourceGeopackage) GetTableInfo() []Table {
 	query := `SELECT table_name, column_name, geometry_type_name, srs_id FROM gpkg_geometry_columns;`
-	rows, err := source.handle.Query(query)
+	rows, err := source.handle.Query(query) //nolint:rowserrcheck
 	if err != nil {
 		log.Fatalf("error during closing rows: %v - %v", query, err)
 	}
@@ -215,13 +214,12 @@ func (target *TargetGeopackage) WriteFeatures(inFeatures <-chan processing.Featu
 		if !hasMore {
 			target.writeFeatures(features)
 			break
-		} else {
-			features = append(features, feature)
+		}
+		features = append(features, feature)
 
-			if len(features)%target.pagesize == 0 {
-				target.writeFeatures(features)
-				features = nil
-			}
+		if len(features)%target.pagesize == 0 {
+			target.writeFeatures(features)
+			features = nil
 		}
 	}
 }
@@ -266,11 +264,11 @@ func (target *TargetGeopackage) writeFeatures(features []interface{}) {
 				continue
 			}
 		} else {
-			ext.AddGeometry(f.geometry)
+			_ = ext.AddGeometry(f.geometry)
 		}
 	}
-	stmt.Close()
-	tx.Commit()
+	stmt.Close() //nolint:sqlclosecheck
+	_ = tx.Commit()
 
 	err = target.handle.UpdateGeometryExtent(target.Table.Name, ext)
 	if err != nil {
@@ -294,10 +292,10 @@ func (t Table) createSQL() string {
 	for _, column := range t.columns {
 		columnpart := column.name + ` ` + column.ctype
 		if column.notnull == 1 {
-			columnpart = columnpart + ` NOT NULL`
+			columnpart += ` NOT NULL`
 		}
 		if column.pk == 1 {
-			columnpart = columnpart + ` PRIMARY KEY`
+			columnpart += ` PRIMARY KEY`
 		}
 
 		columnparts = append(columnparts, columnpart)
@@ -341,7 +339,7 @@ func getSpatialReferenceSystem(h *gpkg.Handle, id int) gpkg.SpatialReferenceSyst
 
 	row := h.QueryRow(fmt.Sprintf(query, id))
 	var description *string
-	row.Scan(&srs.Name, &srs.ID, &srs.Organization, &srs.OrganizationCoordsysID, &srs.Definition, &description)
+	_ = row.Scan(&srs.Name, &srs.ID, &srs.Organization, &srs.OrganizationCoordsysID, &srs.Definition, &description)
 	if description != nil {
 		srs.Description = *description
 	}
@@ -353,7 +351,7 @@ func getSpatialReferenceSystem(h *gpkg.Handle, id int) gpkg.SpatialReferenceSyst
 func getTableColumns(h *gpkg.Handle, table string) []column {
 	var columns []column
 	query := `PRAGMA table_info('%v');`
-	rows, err := h.Query(fmt.Sprintf(query, table))
+	rows, err := h.Query(fmt.Sprintf(query, table)) //nolint:rowserrcheck
 
 	if err != nil {
 		log.Fatalf("err during closing rows: %v - %v", query, err)

snap/pointindex.go

@@ -185,16 +185,15 @@ func (ix *PointIndex) snapClosestPoints(intLine intgeom.Line, depth uint, certai
 	pt2InfiniteQuadrantI := ix.getInfiniteQuadrant(intLine[1])
 	pt2IsInsideQuadrant := ix.containsPoint(intLine[1])
 
-	if pt1InfiniteQuadrantI == pt2InfiniteQuadrantI {
-		// line intersects at most this quadrant
+	switch {
+	case pt1InfiniteQuadrantI == pt2InfiniteQuadrantI:
 		if pt1IsInsideQuadrant && pt2IsInsideQuadrant {
 			// line intersects for sure this quadrant (only)
 			quadrantsToCheck = []quadrantToCheck{{pt1InfiniteQuadrantI, true, false}}
 		} else {
 			quadrantsToCheck = []quadrantToCheck{{pt1InfiniteQuadrantI, false, false}} // TODO check only outside edges
 		}
-	} else if quadrantsAreAdjacent(pt1InfiniteQuadrantI, pt2InfiniteQuadrantI) {
-		// line intersects at most these two quadrants
+	case quadrantsAreAdjacent(pt1InfiniteQuadrantI, pt2InfiniteQuadrantI):
 		if pt1IsInsideQuadrant && pt2IsInsideQuadrant {
 			quadrantsToCheck = []quadrantToCheck{
 				{pt1InfiniteQuadrantI, true, false},
@@ -206,9 +205,9 @@ func (ix *PointIndex) snapClosestPoints(intLine intgeom.Line, depth uint, certai
 				{pt2InfiniteQuadrantI, false, false},
 			} // TODO only need to check the outside edges of the quadrant
 		}
-	} else {
-		// line intersects at most three quadrants, but don't know which ones
-		if pt1IsInsideQuadrant {
+	default:
+		switch {
+		case pt1IsInsideQuadrant:
 			if pt2IsInsideQuadrant { // both points inside quadrant
 				quadrantsToCheck = []quadrantToCheck{
 					{pt1InfiniteQuadrantI, true, false},
@@ -224,14 +223,14 @@ func (ix *PointIndex) snapClosestPoints(intLine intgeom.Line, depth uint, certai
 					{pt2InfiniteQuadrantI, false, false},
 				}
 			}
-		} else if pt2IsInsideQuadrant { // pt1 outside, pt2 inside
+		case pt2IsInsideQuadrant:
 			quadrantsToCheck = []quadrantToCheck{
 				{pt1InfiniteQuadrantI, false, false},
 				{adjacentQuadrantX(pt1InfiniteQuadrantI), false, true},
 				{adjacentQuadrantY(pt1InfiniteQuadrantI), false, true},
 				{pt2InfiniteQuadrantI, true, false},
 			}
-		} else { // neither inside (worst case)
+		default:
 			quadrantsToCheck = []quadrantToCheck{
 				{pt1InfiniteQuadrantI, false, false},
 				{adjacentQuadrantX(pt1InfiniteQuadrantI), false, true},
@@ -394,15 +393,10 @@ func bool2int(b bool) int {
 	return 0
 }
 
-func oneIfLeft(quadrantI int) int {
-	return oneIfRight(quadrantI) ^ 1
-}
 func oneIfRight(quadrantI int) int {
 	return quadrantI & right
 }
-func oneIfBottom(quadrantI int) int {
-	return oneIfTop(quadrantI) ^ 1
-}
+
 func oneIfTop(quadrantI int) int {
 	return (quadrantI & top) >> 1
 }

snap/pointindex_test.go

@@ -1,16 +1,16 @@
 package snap
 
 import (
-	"github.com/go-spatial/geom/encoding/wkt"
-	"github.com/pdok/texel/intgeom"
 	"os"
 	"testing"
 
+	"github.com/go-spatial/geom/encoding/wkt"
+	"github.com/pdok/texel/intgeom"
+
 	"github.com/go-spatial/geom"
 	"github.com/stretchr/testify/assert"
 )
 
-//nolint:funlen
 func TestPointIndex_containsPoint(t *testing.T) {
 	tests := []struct {
 		name string
@@ -103,7 +103,7 @@ func TestPointIndex_getQuadrantExtentAndCentroid(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			extent, centroid := getQuadrantExtentAndCentroid(&tt.matrix, 0, 0, 0)
+			extent, centroid := getQuadrantExtentAndCentroid(&tt.matrix, 0, 0, 0) //nolint:gosec
 			if !assert.EqualValues(t, tt.want.extent, extent) {
 				t.Errorf("getQuadrantExtentAndCentroid() = %v, want %v", extent, tt.want.extent)
 			}
@@ -114,7 +114,6 @@ func TestPointIndex_getQuadrantExtentAndCentroid(t *testing.T) {
 	}
 }
 
-//nolint:funlen
 func TestPointIndex_InsertPoint(t *testing.T) {
 	tests := []struct {
 		name   string
@@ -298,7 +297,7 @@ func TestPointIndex_InsertPoint(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			ix := NewPointIndexFromTileMatrix(tt.matrix)
 			ix.InsertPoint(tt.point)
-			setRootMatrices(&tt.want, &tt.matrix)
+			setRootMatrices(&tt.want, &tt.matrix) //nolint:gosec
 			assert.EqualValues(t, tt.want, *ix)
 		})
 	}

snap/snap.go

@@ -10,6 +10,14 @@ import (
 	"github.com/umpc/go-sortedmap"
 )
 
+// ToPointCloud snaps polygons' points to a tile's internal pixel grid
+// and adds points to lines to prevent intersections.
+func ToPointCloud(source processing.Source, target processing.Target, tileMatrix TileMatrix) {
+	processing.ProcessFeatures(source, target, func(p geom.Polygon) *geom.Polygon {
+		return snapPolygon(&p, tileMatrix)
+	})
+}
+
 func snapPolygon(polygon *geom.Polygon, tileMatrix TileMatrix) *geom.Polygon {
 	ix := NewPointIndexFromTileMatrix(tileMatrix)
 	ix.InsertPolygon(polygon)
@@ -18,7 +26,6 @@ func snapPolygon(polygon *geom.Polygon, tileMatrix TileMatrix) *geom.Polygon {
 	return newPolygon
 }
 
-//nolint:cyclop
 func addPointsAndSnap(ix *PointIndex, polygon *geom.Polygon) *geom.Polygon {
 	newPolygon := make([][][2]float64, 0, len(*polygon))
 	// Could use polygon.AsSegments(), but it skips rings with <3 segments and starts with the last segment.
@@ -333,11 +340,3 @@ func kmpTable(find [][2]float64, table []int) {
 		}
 	}
 }
-
-// SnapToPointCloud snaps polygons' points to a tile's internal pixel grid
-// and adds points to lines to prevent intersections.
-func SnapToPointCloud(source processing.Source, target processing.Target, tileMatrix TileMatrix) {
-	processing.ProcessFeatures(source, target, func(p geom.Polygon) *geom.Polygon {
-		return snapPolygon(&p, tileMatrix)
-	})
-}