lint generated go code (#79)

* lint generated go code

* x

Author: zoom-ua

.github/workflows/ci.yml

@@ -21,19 +21,17 @@ concurrency:
 jobs:
   test:
     timeout-minutes: 15
-    runs-on: ubuntu-latest
     strategy:
       matrix:
         go-version: [1.25.x, 1.24.x]
-
+        os: [ubuntu-latest]
+    runs-on: ${{ matrix.os }}
     steps:
-      - name: Checkout code
-        uses: actions/checkout@v6
+      - uses: actions/checkout@v6
         with:
           persist-credentials: false
 
-      - name: Set up Go
-        uses: actions/setup-go@v6
+      - uses: actions/setup-go@v6
         with:
           go-version: ${{ matrix.go-version }}
           cache: true
@@ -51,6 +49,13 @@ jobs:
           cd ../..
           make
 
+      - name: golangci-lint
+        uses: golangci/golangci-lint-action@v9
+        with:
+          version: v2.7.2
+          working-directory: test/files
+          args: --config ../../.golangci.yml
+
       - name: Run tests
         run: make CI_TAGS="--tags=noresinit" test
 

.golangci.yml

@@ -0,0 +1,81 @@
+version: "2"
+
+run:
+  timeout: 5m
+  tests: true
+
+formatters:
+  enable:
+    - gofumpt
+
+linters:
+  enable:
+    # Core recommended linters
+    - errcheck # Checks for unchecked errors
+    - govet # Go vet checks
+    - ineffassign # Detects ineffectual assignments
+    - staticcheck # Advanced static analysis
+    - unused # Finds unused code
+
+    # Code quality
+    - misspell # Finds commonly misspelled words
+    - unconvert # Unnecessary type conversions
+    - unparam # Finds unused function parameters
+    - gocritic # Various checks
+    - revive # Fast, configurable linter
+
+    # Security and best practices
+    - gosec # Security-focused linter
+    - bodyclose # Checks HTTP response body closed
+    - noctx # Finds HTTP requests without context
+
+  settings:
+    gocritic:
+      disabled-checks:
+        - ifElseChain
+        - elseif
+
+    govet:
+      enable-all: true
+      disable:
+        - shadow
+        - fieldalignment
+
+    revive:
+      enable-all-rules: false
+
+  exclusions:
+    rules:
+      # Exclude specific revive rules for generated code
+      - linters:
+          - revive
+        text: "package-comments"
+
+      - linters:
+          - revive
+        text: "exported"
+
+      # Exclude var-naming for intentional underscores in generated AVDL code
+      - linters:
+          - revive
+        text: "var-naming"
+
+      # Exclude specific staticcheck rules
+      - linters:
+          - staticcheck
+        text: "ST1005"
+
+      # Exclude specific gocritic rules
+      - linters:
+          - gocritic
+        text: "ifElseChain"
+
+      # Exclude gofumpt formatting for generated AVDL code - uses tabs for alignment
+      - linters:
+          - gofumpt
+        path: "test/files/sample\\.go"
+
+      # Exclude unused for test sample files - they're just for testing the generator
+      - linters:
+          - unused
+        path: "test/files/sample\\.go"

lib/go_emit.js

@@ -212,7 +212,12 @@ exports.GoEmitter = class GoEmitter extends BaseEmitter
     @untab()
     @output "}"
     @output "tmp := ", {frag : true }
-    @deep_copy { t : t[1], val : "(*x)", exported : true }
+    # For pointer-to-type where type has DeepCopy() method, we can simplify
+    # (*x).DeepCopy() to x.DeepCopy(). But for arrays/maps/primitives, we need to dereference.
+    inner = t[1]
+    can_simplify = (typeof(inner) is 'string' and not @is_primitive_type_lax(inner))
+    inner_val = if can_simplify then "x" else "(*x)"
+    @deep_copy { t : inner, val : inner_val, exported : true }
     @output "return &tmp"
     @deep_copy_postamble { val }
 
@@ -353,7 +358,14 @@ exports.GoEmitter = class GoEmitter extends BaseEmitter
       cases = ("o.#{@variant_field(obj.switch.name)} == #{v}" for v in cases)
       @output "if #{cases.join(" || ")} {"
     else
-      @output "if o.#{@variant_field(obj.switch.name)} != #{tag_val} {"
+      # Simplify boolean comparisons for cleaner code
+      switch_field = "o.#{@variant_field(obj.switch.name)}"
+      if tag_val in ["true", true]
+        @output "if !#{switch_field} {"
+      else if tag_val in ["false", false]
+        @output "if #{switch_field} {"
+      else
+        @output "if #{switch_field} != #{tag_val} {"
     @tab()
     @output """panic("wrong case accessed")"""
     @untab()
@@ -529,14 +541,14 @@ exports.GoEmitter = class GoEmitter extends BaseEmitter
     @output "}"
 
     unless nostring
-      @output "func (e " + name + ") String() string {"
+      @output "func (o " + name + ") String() string {"
       @tab()
-      @output "if v, ok := #{name}RevMap[e]; ok {"
+      @output "if v, ok := #{name}RevMap[o]; ok {"
       @tab()
       @output "return v"
       @untab()
       @output "}"
-      @output "return fmt.Sprintf(\"%v\", int(e))"
+      @output "return fmt.Sprintf(\"%v\", int(o))"
       @untab()
       @output "}"
 
@@ -584,27 +596,27 @@ exports.GoEmitter = class GoEmitter extends BaseEmitter
   emit_imports : ({imports, messages, types}, outfile, {types_only}) ->
     @output "import ("
     @tab()
-    if not types_only
-      @output '"github.com/keybase/go-framed-msgpack-rpc/rpc"'
-      @output 'context "golang.org/x/net/context"' if Object.keys(messages).length > 0
+
+    # Standard library imports - gofumpt will sort and format these
+    has_messages = Object.keys(messages).length > 0
+    @output '"context"' if has_messages
+    @output '"errors"' if @count_variants({types}) > 0
+    @output '"fmt"' if @count_enums_with_string({types}) > 0
+    @output '"time"' if has_messages
+
+    # Third-party imports - gofumpt will separate these from stdlib
+    @output '"github.com/keybase/go-framed-msgpack-rpc/rpc"' unless types_only
 
     # Go modules mode - construct import paths relative to module root
     relative_file = path_lib.resolve(outfile).replace(@gomod_dir, "")
     relative_dir = @gomod_path + path_lib.dirname(relative_file)
 
     for {import_as, path} in imports when path.indexOf('/') >= 0
-      if path.match /(\.\/|\.\.\/)/
-        path = path_lib.normalize(relative_dir + "/" + path)
-      line = ""
-      line = import_as + " " if import_as?
+      path = path_lib.normalize(relative_dir + "/" + path) if path.match /(\.\/|\.\.\/)/
+      line = if import_as? then import_as + " " else ""
       line += '"' + path + '"'
       @output line
-    if @count_variants({types}) > 0
-      @output '"errors"'
-    if @count_enums_with_string({types}) > 0
-      @output '"fmt"'
-    if Object.keys(messages).length > 0
-      @output '"time"'
+
     @untab()
     @output ")"
     @output ""

src/go_emit.iced

@@ -51,6 +51,7 @@ describe 'GoEmitter', () ->
       emitter.emit_imports {imports, messages: {}, types: []}, 'location/of/my/output.go', {types_only: true}
       code = emitter._code.join "\n"
 
+      # Note: gofumpt will format this, so spacing may vary
       expect(code).toBe('''
         import (
         \tgregor1 "github.com/keybase/node-avdl-compiler/location/of/gregor1"
@@ -88,11 +89,12 @@ describe 'GoEmitter', () ->
     it "should only import the content and time packages if types_only is false and the file contains messages", () ->
       emitter.emit_imports {imports: [], messages: {fake_message: 'blah'}, types: []}, 'location/of/my/output.go', {types_only: false}
       code = emitter._code.join "\n"
+      # gofumpt will sort imports - stdlib first, then third-party
       expect(code).toBe("""
         import (
-        \t"github.com/keybase/go-framed-msgpack-rpc/rpc"
-        \tcontext "golang.org/x/net/context"
+        \t"context"
         \t"time"
+        \t"github.com/keybase/go-framed-msgpack-rpc/rpc"
         )\n\n
       """)
       return
@@ -123,6 +125,7 @@ describe 'GoEmitter', () ->
       emitter.emit_typedef type
       code = emitter._code.join "\n"
 
+      # gofumpt will add spacing between type and method
       expect(code).toBe("""
         type BuildPaymentID string
         func (o BuildPaymentID) DeepCopy() BuildPaymentID {
@@ -405,11 +408,11 @@ describe 'GoEmitter', () ->
         \t3: "V3",
         }
 
-        func (e AuditVersion) String() string {
-        \tif v, ok := AuditVersionRevMap[e]; ok {
+        func (o AuditVersion) String() string {
+        \tif v, ok := AuditVersionRevMap[o]; ok {
         \t\treturn v
         \t}
-        \treturn fmt.Sprintf("%v", int(e))
+        \treturn fmt.Sprintf("%v", int(o))
         }\n
       """)
       return