Add stylecheck linter to golangci-lint CI runs

.github/workflows/ci.yml

@@ -19,8 +19,8 @@ jobs:
       - name: golangci-lint
         uses: golangci/golangci-lint-action@v2
         with:
-          version: v1.38.0
-          args: --timeout=5m
+          version: v1.42.0 # Has fixes for stylecheck configuration https://github.com/golangci/golangci-lint/pull/2017/files
+          args: --timeout=5m -v
 
   test:
     runs-on: 'windows-2019'

.golangci.yml

@@ -0,0 +1,96 @@
+linters:
+  enable:
+    - stylecheck
+
+linters-settings:
+  stylecheck:
+    # https://staticcheck.io/docs/checks
+    checks: ["all"]
+
+
+issues:
+  # This repo has a LOT of generated schema files, operating system bindings, and other things that ST1003 from stylecheck won't like
+  # (screaming case Windows api constants for example). There's also some structs that we *could* change the initialisms to be Go
+  # friendly (Id -> ID) but they're exported and it would be a breaking change. This makes it so that most new code, code that isn't
+  # supposed to be a pretty faithful mapping to an OS call/constants, or non-generated code still checks if we're following idioms,
+  # while ignoring the things that are just noise or would be more of a hassle than it'd be worth to change.
+  exclude-rules:
+    - path: layer.go
+      linters:
+        - stylecheck
+      Text: "ST1003:"
+
+    - path: hcsshim.go
+      linters:
+        - stylecheck
+      Text: "ST1003:"
+
+    - path: internal\\hcs\\schema2\\
+      linters:
+        - stylecheck
+      Text: "ST1003:"
+
+    - path: internal\\wclayer\\
+      linters:
+        - stylecheck
+      Text: "ST1003:"
+
+    - path: hcn\\
+      linters:
+        - stylecheck
+      Text: "ST1003:"
+
+    - path: internal\\hcs\\schema1\\
+      linters:
+        - stylecheck
+      Text: "ST1003:"
+
+    - path: internal\\hns\\
+      linters:
+        - stylecheck
+      Text: "ST1003:"
+
+    - path: ext4\\internal\\compactext4\\
+      linters:
+        - stylecheck
+      Text: "ST1003:"
+
+    - path: ext4\\internal\\format\\
+      linters:
+        - stylecheck
+      Text: "ST1003:"
+
+    - path: internal\\guestrequest\\
+      linters:
+        - stylecheck
+      Text: "ST1003:"
+
+    - path: internal\\guest\\prot\\
+      linters:
+        - stylecheck
+      Text: "ST1003:"
+
+    - path: internal\\windevice\\
+      linters:
+        - stylecheck
+      Text: "ST1003:"
+
+    - path: internal\\winapi\\
+      linters:
+        - stylecheck
+      Text: "ST1003:"
+
+    - path: internal\\vmcompute\\
+      linters:
+        - stylecheck
+      Text: "ST1003:"
+
+    - path: internal\\regstate\\
+      linters:
+        - stylecheck
+      Text: "ST1003:"
+
+    - path: internal\\hcserror\\
+      linters:
+        - stylecheck
+      Text: "ST1003:"

cmd/containerd-shim-runhcs-v1/task_hcs.go

@@ -18,7 +18,6 @@ import (
 	"github.com/sirupsen/logrus"
 	"go.opencensus.io/trace"
 
-	"github.com/Microsoft/hcsshim/cmd/containerd-shim-runhcs-v1/options"
 	runhcsopts "github.com/Microsoft/hcsshim/cmd/containerd-shim-runhcs-v1/options"
 	"github.com/Microsoft/hcsshim/cmd/containerd-shim-runhcs-v1/stats"
 	"github.com/Microsoft/hcsshim/internal/cmd"
@@ -587,7 +586,7 @@ func (ht *hcsTask) DeleteExec(ctx context.Context, eid string) (int, uint32, tim
 	return int(status.Pid), status.ExitStatus, status.ExitedAt, nil
 }
 
-func (ht *hcsTask) Pids(ctx context.Context) ([]options.ProcessDetails, error) {
+func (ht *hcsTask) Pids(ctx context.Context) ([]runhcsopts.ProcessDetails, error) {
 	// Map all user created exec's to pid/exec-id
 	pidMap := make(map[int]string)
 	ht.execs.Range(func(key, value interface{}) bool {
@@ -606,7 +605,7 @@ func (ht *hcsTask) Pids(ctx context.Context) ([]options.ProcessDetails, error) {
 	}
 
 	// Copy to pid/exec-id pair's
-	pairs := make([]options.ProcessDetails, len(props.ProcessList))
+	pairs := make([]runhcsopts.ProcessDetails, len(props.ProcessList))
 	for i, p := range props.ProcessList {
 		pairs[i].ImageName = p.ImageName
 		pairs[i].CreatedAt = p.CreateTimestamp

cmd/gcstools/commoncli/common.go

@@ -20,7 +20,7 @@ func SetFlagsForLogging() []*string {
 // SetupLogging creates the logger from the command line parameters.
 func SetupLogging(args ...*string) error {
 	if len(args) < 1 {
-		return fmt.Errorf("Invalid log params")
+		return fmt.Errorf("invalid log params")
 	}
 	level, err := logrus.ParseLevel(*args[1])
 	if err != nil {

cmd/runhcs/delete.go

@@ -57,7 +57,7 @@ status of "ubuntu01" as "stopped" the following will delete resources held for
 			kill = true
 		default:
 			if !force {
-				return fmt.Errorf("cannot delete container %s that is not stopped: %s\n", id, s)
+				return fmt.Errorf("cannot delete container %s that is not stopped: %s", id, s)
 			}
 			kill = true
 		}

cmd/runhcs/exec.go

@@ -146,12 +146,12 @@ func getProcessSpec(context *cli.Context, c *container) (*specs.Process, error)
 
 func validateProcessSpec(spec *specs.Process) error {
 	if spec.Cwd == "" {
-		return fmt.Errorf("Cwd property must not be empty")
+		return fmt.Errorf("cwd property must not be empty")
 	}
 	// IsAbs doesnt recognize Unix paths on Windows builds so handle that case
 	// here.
 	if !filepath.IsAbs(spec.Cwd) && !strings.HasPrefix(spec.Cwd, "/") {
-		return fmt.Errorf("Cwd must be an absolute path")
+		return fmt.Errorf("cwd must be an absolute path")
 	}
 	if len(spec.Args) == 0 {
 		return fmt.Errorf("args must not be empty")

hcn/hcn.go

@@ -122,7 +122,7 @@ func defaultQuery() HostComputeQuery {
 
 // PlatformDoesNotSupportError happens when users are attempting to use a newer shim on an older OS
 func platformDoesNotSupportError(featureName string) error {
-	return fmt.Errorf("Platform does not support feature %s", featureName)
+	return fmt.Errorf("platform does not support feature %s", featureName)
 }
 
 // V2ApiSupported returns an error if the HCN version does not support the V2 Apis.

hcn/hcnroute.go

@@ -137,7 +137,7 @@ func AddRoute(endpoints []HostComputeEndpoint, destinationPrefix string, nextHop
 	logrus.Debugf("hcn::HostComputeRoute::AddRoute endpointId=%v, destinationPrefix=%v, nextHop=%v, needEncapsulation=%v", endpoints, destinationPrefix, nextHop, needEncapsulation)
 
 	if len(endpoints) <= 0 {
-		return nil, errors.New("Missing endpoints")
+		return nil, errors.New("missing endpoints")
 	}
 
 	route := &HostComputeRoute{

internal/hcsoci/create.go

@@ -135,7 +135,7 @@ func validateContainerConfig(ctx context.Context, coi *createOptionsInternal) er
 	}
 
 	if coi.HostingSystem != nil && coi.templateID != "" && !coi.HostingSystem.IsClone {
-		return fmt.Errorf("A container can not be cloned inside a non cloned POD")
+		return fmt.Errorf("a container can not be cloned inside a non cloned POD")
 	}
 
 	if coi.templateID != "" {
@@ -152,11 +152,11 @@ func validateContainerConfig(ctx context.Context, coi *createOptionsInternal) er
 
 	if coi.HostingSystem != nil && coi.HostingSystem.IsTemplate {
 		if len(coi.Spec.Windows.Devices) != 0 {
-			return fmt.Errorf("Mapped Devices are not supported for template containers")
+			return fmt.Errorf("mapped Devices are not supported for template containers")
 		}
 
 		if _, ok := coi.Spec.Windows.CredentialSpec.(string); ok {
-			return fmt.Errorf("gMSA specifications are not supported for template containers")
+			return fmt.Errorf("gmsa specifications are not supported for template containers")
 		}
 
 		if coi.Spec.Windows.Servicing {
@@ -179,7 +179,7 @@ func initializeCreateOptions(ctx context.Context, createOptions *CreateOptions)
 	}
 
 	if coi.Spec == nil {
-		return nil, fmt.Errorf("Spec must be supplied")
+		return nil, fmt.Errorf("spec must be supplied")
 	}
 
 	// Defaults if omitted by caller.

internal/hcsoci/hcsdoc_wcow.go

@@ -269,7 +269,7 @@ func createWindowsContainerDocument(ctx context.Context, coi *createOptionsInter
 		// Use the reserved network namespace for containers created inside
 		// cloned or template UVMs.
 		if coi.HostingSystem != nil && (coi.HostingSystem.IsTemplate || coi.HostingSystem.IsClone) {
-			v2Container.Networking.Namespace = uvm.DEFAULT_CLONE_NETWORK_NAMESPACE_ID
+			v2Container.Networking.Namespace = uvm.DefaultCloneNetworkNamespaceID
 		} else {
 			v2Container.Networking.Namespace = coi.actualNetworkNamespace
 		}

internal/jobcontainers/storage.go

@@ -15,7 +15,7 @@ import (
 // Trailing backslash required for SetVolumeMountPoint and DeleteVolumeMountPoint
 const sandboxMountFormat = `C:\C\%s\`
 
-func mountLayers(ctx context.Context, containerId string, s *specs.Spec, volumeMountPath string) error {
+func mountLayers(ctx context.Context, containerID string, s *specs.Spec, volumeMountPath string) error {
 	if s == nil || s.Windows == nil || s.Windows.LayerFolders == nil {
 		return errors.New("field 'Spec.Windows.Layerfolders' is not populated")
 	}
@@ -41,7 +41,7 @@ func mountLayers(ctx context.Context, containerId string, s *specs.Spec, volumeM
 
 	if s.Root.Path == "" {
 		log.G(ctx).Debug("mounting job container storage")
-		containerRootPath, err := layers.MountContainerLayers(ctx, containerId, s.Windows.LayerFolders, "", volumeMountPath, nil)
+		containerRootPath, err := layers.MountContainerLayers(ctx, containerID, s.Windows.LayerFolders, "", volumeMountPath, nil)
 		if err != nil {
 			return errors.Wrap(err, "failed to mount container storage")
 		}