#74 Fix linter warnings (#113)

.github/workflows/ci-build.yml

@@ -75,10 +75,6 @@ jobs:
             exit 1
           fi
 
-      - name: golangci-lint
-        run: |
-          ./ci/install_and_run_golangci-lint.sh
-
       - name: Build
         run: |
           go build ./...
@@ -145,3 +141,17 @@ jobs:
             oft
       - name: Run OpenFastTrace
         run: ./ci/trace-requirements.sh
+
+  static-code-analysis-go:
+    runs-on: ubuntu-latest
+    concurrency:
+      group: ${{ github.workflow }}-${{ github.ref }}-static-code-analysis-go
+      cancel-in-progress: true
+    steps:
+      - name: Check out code
+        uses: actions/checkout@v3
+      - name: Run golangci-lint
+        uses: golangci/golangci-lint-action@v3
+        with:
+          version: v1.53.3
+          args: --timeout=15m

.golangci.yml

@@ -1,6 +1,26 @@
 # https://golangci-lint.run/usage/configuration/
 linters:
   enable-all: false
+  presets:
+    - bugs
+    - complexity
+    - format
+    - import
+    - module
+    - sql
+    - comment
+    - performance
+    - test
+    #- error
+    #- style
+    #- metalinter
+    #- unused # Deactivated because of false positives for mocks
+  disable:
+    - gofumpt # "gofmt" is OK for us, no need for stricter rules (https://github.com/mvdan/gofumpt)
+    - gci # No need to explicitly format imports using GCI (https://github.com/daixiang0/gci)
+    - depguard # No need to restrict imported packages (https://github.com/OpenPeeDeeP/depguard)
+    - exhaustruct # Listing every field of a struct is not worth the effort (https://github.com/GaijinEntertainment/go-exhaustruct)
 run:
+  tests: false
   skip-dirs:
     - node_modules

doc/changes/changes_0.4.1.md

@@ -6,11 +6,12 @@ Code name:
 
 This release improves error handling when using extensions not implementing all functions required by EM. EM now returns a helpful error message instead of failing with a `nil`-pointer error.
 
-A common scenario for an extension not implementing a required function is when the extension had been built using an older version of EM's extension intererface.
+A common scenario for an extension not implementing a required function is when the extension had been built using an older version of EM's extension interface.
 
 ## Bugfixes
 
 * #105: Ensured that EM can load and use compatible extensions
+* #74: Fixed most important linter warnings
 
 ## Dependency Updates
 

pkg/apiErrors/errors.go

@@ -41,7 +41,8 @@ func NewAPIError(status int, message string) error {
 }
 
 func NewAPIErrorWithCause(message string, cause error) error {
-	if apiErr, ok := cause.(*APIError); ok {
+	var apiErr *APIError
+	if errors.As(cause, &apiErr) {
 		return &APIError{
 			Status:        apiErr.Status,
 			Message:       fmt.Sprintf("%s: %s", message, apiErr.Message),
@@ -52,23 +53,19 @@ func NewAPIErrorWithCause(message string, cause error) error {
 }
 
 // UnwrapAPIError returns an API Error if one exists in the given error's chain or nil if no API Error exists in the chain.
-func UnwrapAPIError(orgErr error) *APIError {
-	err := orgErr
-	for {
-		if apiErr, ok := err.(*APIError); ok {
-			return apiErr
-		}
-		if err = errors.Unwrap(err); err == nil {
-			return NewInternalServerError(orgErr)
-		}
+func UnwrapAPIError(err error) *APIError {
+	var apiErr *APIError
+	if errors.As(err, &apiErr) {
+		return apiErr
 	}
+	return NewInternalServerError(err)
 }
 
 type APIError struct {
 	Status        int    `json:"code"`                // HTTP status code
 	Message       string `json:"message"`             // human-readable message
 	RequestID     string `json:"requestID,omitempty"` // ID to identify the request that caused this error
-	Timestamp     string `json:"timestamp,omitempty" jsonschema:"format=date-time" `
+	Timestamp     string `json:"timestamp,omitempty" jsonschema:"format=date-time"`
 	APIID         string `json:"apiID,omitempty"` // Corresponding API action
 	OriginalError error  `json:"-"`
 }

pkg/extensionAPI/ExaMetadata.go

@@ -23,7 +23,7 @@ type metaDataReaderImpl struct {
 }
 
 // ReadMetadataTables reads the metadata tables of the given schema.
-/* [impl -> dsn~extension-components~1] */
+/* [impl -> dsn~extension-components~1]. */
 func (r *metaDataReaderImpl) ReadMetadataTables(tx *sql.Tx, schemaName string) (*ExaMetadata, error) {
 	allScripts, err := readExaAllScriptTable(tx, schemaName)
 	if err != nil {
@@ -44,8 +44,12 @@ WHERE SCRIPT_SCHEMA=?`, schemaName)
 	if err != nil {
 		return nil, fmt.Errorf("failed to read SYS.EXA_ALL_SCRIPTS: %w", err)
 	}
+	defer result.Close()
 	rows := make([]ExaScriptRow, 0)
 	for result.Next() {
+		if result.Err() != nil {
+			return nil, fmt.Errorf("failed to iterate SYS.EXA_ALL_SCRIPTS: %w", err)
+		}
 		var row ExaScriptRow
 		var inputType sql.NullString
 		var resultType sql.NullString
@@ -79,9 +83,13 @@ func getExasolMajorVersion(tx *sql.Tx) (string, error) {
 	if err != nil {
 		return "", fmt.Errorf("query failed: %w", err)
 	}
+	defer result.Close()
 	if !result.Next() {
 		return "", fmt.Errorf("no result found for query")
 	}
+	if result.Err() != nil {
+		return "", fmt.Errorf("failed to iterate SYS.EXA_METADATA: %w", err)
+	}
 	var majorVersion string
 	err = result.Scan(&majorVersion)
 	if err != nil {
@@ -90,16 +98,20 @@ func getExasolMajorVersion(tx *sql.Tx) (string, error) {
 	return majorVersion, nil
 }
 
-// This reads virtual schemas from the metadata tables of Exasol version 8
+// This reads virtual schemas from the metadata tables of Exasol version 8.
 func readExaAllVirtualSchemasTableV8(tx *sql.Tx) (*ExaVirtualSchemasTable, error) {
 	result, err := tx.Query(`
 SELECT SCHEMA_NAME, SCHEMA_OWNER, ADAPTER_SCRIPT_SCHEMA, ADAPTER_SCRIPT_NAME, ADAPTER_NOTES
 FROM SYS.EXA_ALL_VIRTUAL_SCHEMAS`)
 	if err != nil {
 		return nil, fmt.Errorf("failed to read SYS.EXA_ALL_VIRTUAL_SCHEMAS: %w", err)
 	}
+	defer result.Close()
 	rows := make([]ExaVirtualSchemaRow, 0)
 	for result.Next() {
+		if result.Err() != nil {
+			return nil, fmt.Errorf("failed to iterate SYS.EXA_ALL_VIRTUAL_SCHEMAS: %w", err)
+		}
 		var row ExaVirtualSchemaRow
 		err := result.Scan(&row.Name, &row.Owner, &row.AdapterScriptSchema, &row.AdapterScriptName, &row.AdapterNotes)
 		if err != nil {
@@ -110,16 +122,20 @@ FROM SYS.EXA_ALL_VIRTUAL_SCHEMAS`)
 	return &ExaVirtualSchemasTable{Rows: rows}, nil
 }
 
-// This reads virtual schemas from the metadata tables of Exasol version 7.1
+// This reads virtual schemas from the metadata tables of Exasol version 7.1.
 func readExaAllVirtualSchemasTableV7(tx *sql.Tx) (*ExaVirtualSchemasTable, error) {
 	result, err := tx.Query(`
 SELECT SCHEMA_NAME, SCHEMA_OWNER, ADAPTER_SCRIPT, ADAPTER_NOTES
 FROM SYS.EXA_ALL_VIRTUAL_SCHEMAS`)
 	if err != nil {
 		return nil, fmt.Errorf("failed to read SYS.EXA_ALL_VIRTUAL_SCHEMAS: %w", err)
 	}
+	defer result.Close()
 	rows := make([]ExaVirtualSchemaRow, 0)
 	for result.Next() {
+		if result.Err() != nil {
+			return nil, fmt.Errorf("failed to iterate SYS.EXA_ALL_VIRTUAL_SCHEMAS: %w", err)
+		}
 		var row ExaVirtualSchemaRow
 		var adapterScriptSchemaAndName string
 		err := result.Scan(&row.Name, &row.Owner, &adapterScriptSchemaAndName, &row.AdapterNotes)

pkg/extensionAPI/extensionApi.go

@@ -12,7 +12,7 @@ import (
 )
 
 // LoadExtension loads an extension from the given file content.
-/* [impl -> dsn~extension-definition~1] */
+/* [impl -> dsn~extension-definition~1]. */
 func LoadExtension(id, content string) (*JsExtension, error) {
 	logPrefix := fmt.Sprintf("JS:%s>", id)
 	vm := newJavaScriptVm(logPrefix)

pkg/extensionAPI/version.go

@@ -8,7 +8,7 @@ import (
 
 const supportedApiVersion = "0.2.0"
 
-/* [impl -> dsn~extension-compatibility~1] */
+/* [impl -> dsn~extension-compatibility~1]. */
 func validateExtensionIsCompatibleWithApiVersion(extensionId, currentExtensionApiVersion string) error {
 	prefixedVersion := "v" + currentExtensionApiVersion
 	if !semver.IsValid(prefixedVersion) {

pkg/extensionController/BucketFsApi.go

@@ -17,7 +17,7 @@ type BucketFsAPI interface {
 	ListFiles(ctx context.Context, db *sql.DB, bucket string) ([]BfsFile, error)
 }
 
-// CreateBucketFsAPI creates an instance of BucketFsAPI
+// CreateBucketFsAPI creates an instance of BucketFsAPI.
 func CreateBucketFsAPI() BucketFsAPI {
 	return &bucketFsAPIImpl{}
 }
@@ -37,15 +37,15 @@ func (bfs bucketFsAPIImpl) ListBuckets(ctx context.Context, db *sql.DB) ([]strin
 	return names, nil
 }
 
-/* [impl -> dsn~extension-components~1] */
+/* [impl -> dsn~extension-components~1]. */
 func (bfs bucketFsAPIImpl) ListFiles(ctx context.Context, db *sql.DB, bucket string) ([]BfsFile, error) {
 	if strings.Contains(bucket, "/") {
 		return nil, fmt.Errorf("invalid bucket name. Bucket name must not contain slashes")
 	}
 	return bfs.listDirInUDF(ctx, db, "/buckets/bfsdefault/"+bucket)
 }
 
-// BfsFile represents a file in BucketFS
+// BfsFile represents a file in BucketFS.
 type BfsFile struct {
 	Name string
 	Size int
@@ -77,15 +77,20 @@ def run(ctx):
 	if err != nil {
 		return nil, fmt.Errorf("failed to create script for listing bucket. Cause: %w", err)
 	}
-	statement, err := transaction.Prepare("SELECT " + schemaName + ".LS(?)")
+	statement, err := transaction.Prepare("SELECT " + schemaName + ".LS(?)") //nolint:gosec // SQL string concatenation is safe here
 	if err != nil {
 		return nil, fmt.Errorf("failed to create prepared statement for running list files UDF. Cause: %w", err)
 	}
+	defer statement.Close()
 	result, err := statement.Query(directory)
 	if err != nil {
 		return nil, fmt.Errorf("failed to list files in BucketFS using UDF. Cause: %w", err)
 	}
+	defer result.Close()
 	for result.Next() {
+		if result.Err() != nil {
+			return nil, fmt.Errorf("failed iterating BucketFS list UDF. Cause: %w", err)
+		}
 		var file BfsFile
 		var fileSize float64
 		err = result.Scan(&file.Name, &fileSize)

pkg/extensionController/controller.go

@@ -59,7 +59,7 @@ func createImpl(extensionRegistryURL string, schema string) controller {
 	}
 }
 
-/* [impl -> dsn~list-extensions~1] */
+/* [impl -> dsn~list-extensions~1]. */
 func (c *controllerImpl) GetAllExtensions(bfsFiles []BfsFile) ([]*Extension, error) {
 	jsExtensions, err := c.getAllExtensions()
 	if err != nil {
@@ -110,7 +110,7 @@ func (c *controllerImpl) getAllExtensions() ([]*extensionAPI.JsExtension, error)
 	if err != nil {
 		return nil, err
 	}
-	var extensions []*extensionAPI.JsExtension
+	extensions := make([]*extensionAPI.JsExtension, 0, len(extensionIds))
 	for _, id := range extensionIds {
 		extension, err := c.loadExtensionById(id)
 		if err != nil {
@@ -254,7 +254,7 @@ func (c *controllerImpl) ensureSchemaExists(tx *sql.Tx) error {
 	return nil
 }
 
-/* [impl -> dsn~parameter-types~1] */
+/* [impl -> dsn~parameter-types~1]. */
 func validateParameters(parameterDefinitions []parameterValidator.ParameterDefinition, params extensionAPI.ParameterValues) error {
 	validator, err := parameterValidator.New()
 	if err != nil {

pkg/extensionController/registry/http.go

@@ -1,9 +1,11 @@
 package registry
 
 import (
+	"context"
 	"fmt"
 	"io"
 	"net/http"
+	"strings"
 
 	"github.com/exasol/extension-manager/pkg/apiErrors"
 	"github.com/exasol/extension-manager/pkg/extensionController/registry/index"
@@ -19,7 +21,7 @@ type httpRegistry struct {
 }
 
 /* [impl -> dsn~extension-registry~1] */
-/* [impl -> dsn~extension-definitions-storage~1] */
+/* [impl -> dsn~extension-definitions-storage~1]. */
 func (h *httpRegistry) FindExtensions() ([]string, error) {
 	err := h.loadIndex()
 	if err != nil {
@@ -33,6 +35,7 @@ func (h *httpRegistry) loadIndex() error {
 	if err != nil {
 		return err
 	}
+	defer response.Body.Close()
 	index, err := index.Decode(response.Body)
 	if err != nil {
 		return err
@@ -42,7 +45,11 @@ func (h *httpRegistry) loadIndex() error {
 }
 
 func getResponse(url string) (*http.Response, error) {
-	response, err := http.Get(url)
+	request, err := http.NewRequestWithContext(context.Background(), "GET", url, strings.NewReader(""))
+	if err != nil {
+		return nil, err
+	}
+	response, err := http.DefaultClient.Do(request)
 	if err != nil {
 		return nil, err
 	}
@@ -74,6 +81,7 @@ func getUrlContent(url string) (string, error) {
 	if err != nil {
 		return "", err
 	}
+	defer response.Body.Close()
 	bytes, err := io.ReadAll(response.Body)
 	if err != nil {
 		return "", fmt.Errorf("failed to read response: %w", err)

pkg/extensionController/registry/localDir.go

@@ -20,7 +20,7 @@ type localDirRegistry struct {
 }
 
 // FindExtensions searches for .js files in the local registry directory.
-/* [impl -> dsn~extension-definitions-storage~1] */
+/* [impl -> dsn~extension-definitions-storage~1]. */
 func (l *localDirRegistry) FindExtensions() ([]string, error) {
 	var files []string
 	err := filepath.Walk(l.dir, func(path string, info os.FileInfo, err error) error {

pkg/extensionController/registry/registry.go

@@ -23,7 +23,7 @@ func NewRegistry(extensionRegistryURL string) Registry {
 	return newLocalDirRegistry(extensionRegistryURL)
 }
 
-/* [impl -> dsn~extension-registry~1] */
+/* [impl -> dsn~extension-registry~1]. */
 func isHttpUrl(urlOrPath string) bool {
 	lowerCaseUrlOrPath := strings.ToLower(urlOrPath)
 	return strings.HasPrefix(lowerCaseUrlOrPath, "http://") || strings.HasPrefix(lowerCaseUrlOrPath, "https://")

pkg/extensionController/transactionController.go

@@ -60,11 +60,11 @@ type ParameterValue struct {
 	Value string
 }
 
-// ExtInstallation represents the installation of an Extension
+// ExtInstallation represents the installation of an Extension.
 type ExtInstallation struct {
 }
 
-// Create an instance of TransactionController
+// Create an instance of TransactionController.
 func Create(extensionRegistryURL string, schema string) TransactionController {
 	controller := createImpl(extensionRegistryURL, schema)
 	return &transactionControllerImpl{controller: controller, bucketFs: CreateBucketFsAPI()}

pkg/integrationTesting/testExtensionBuilder.go

@@ -87,7 +87,7 @@ func (builder *TestExtensionBuilder) WithGetInstanceParameterDefinitionFunc(tsFu
 	return builder
 }
 
-// MockFindInstallationsFunction creates a JS findInstallations function with extension name and version
+// MockFindInstallationsFunction creates a JS findInstallations function with extension name and version.
 func MockFindInstallationsFunction(extensionName string, version string) string {
 	template := `return [{name: "$NAME$", version: "$VERSION$"}]`
 	filledTemplate := strings.Replace(template, "$NAME$", extensionName, 1)