else and else if
%v in format stringsrange when iterating slices, accessing slice elementsinit functionsio.Stdout to write to stdoutgotests+ for positive cases or - for negative cases.+validt.Fatalf to fail a casecmp.Diff to produce human-readable diffsreceiver.method() or function()expect field for cases when using mocks.Always adhere to returning early on error checks and negative cases. This reduces indentation and simplifies the logical flow of code.
Example
func good() (int, error) {
           // some more code
           // ...
       
           err := spawn()
           if err != nil {
               return 0, err
           }
       
           // some more code
           // ...
       
           return 200, nil
       }
       
       func bad() (int, error) {
           // some more code
           // ...
       
           err := spawn()
           if err == nil {
               // some more code
               // ...
       
               return 200, nil
           }
       
           // some more code
           // ...
       
           return 0, err
       }Go is not C, where assignments are also expressions.
Example
else and else ifElse and Else If statements are generally considered bad practice (some literature about this topic 1, 2 3 they all sort of make the same point with similar examples)
Generally almost all else and else if use cases can be refactored to not use them. Doing this usually improves readability, simplifies code, by reducing branching (usually cyclomatic complexity remains the same).
This does not mean that else and else if cases are banned, just that as a general rule avoiding them makes for cleaner code, but valid use cases do exist, like this one →
switch node := node.(type) {
       // a bunch of cases, imagine 500 lines of code with complex logic, you don't
       // want to touch
       case *ast.Image:
           part = part.AddContainer(&jsonast.AstPart{Type: "image",}, entering)
       default:
           // here is a valid use case of else, if the code want's to retain
           // the single return at the bottom, and that is the case
           // here, cause all other of the "bunch of cases~ rely on the
           // existence of the return at the bottom.
           //
           // could be refactored to not use else, but requires too much
           // work, hence "if-else" it is
           if node.AsLeaf() == nil { // container
               part = part.AddContainer(
                   &jsonast.AstPart{ Type: "unknown", },
                   entering,
               )
           } else {
               part.AddLeaf(
                   &jsonast.AstPart{ Type: "unknown", },
               )
           }
       }
       
       return part, nilelse and else ifelse use cases can be removed by just returning early or assigning early.switch statements.The use of inline ifs breaks the single purpose principle as the if statement both produces a value and checks it.
Example
// good
       err := handler()
       if err != nil {
           return errs.Wrap(err, "handler failed")
       }
       
       // bad
       if err := handler(); err != nil {
           return errs.Wrap(err, "failed to handle")
       }%v in format stringsWhere possible don't use %v, but rather exact format string directives for types. Why? - it's faster, like really noticeably faster.
Example
// good
       fmt.Printf("updated in %s secs", diff.String())
       
       // bad
       fmt.Printf("updated in %v secs", diff)Wrapping error returns provides trace of the execution flow up to the moment when something went wrong, making debugging easier, logging more verbose (if the error is logged)
Non-exhaustive list of possible exceptions to this rule are:
Handle function that internal calls handle, then the internal handle call can be not wrapped to avoid duplicate messages failed to handle: failed to handleError returns from external package functions/methods always must be wrapped.
// good
       res, err := Handler()
       if err != nil {
           return errs.Wrap(err, "failed to run handler")
       }
       
       // bad
       res, err := Handler()
       if err != nil {
           return err
       }The wrapping message must be descriptive of the failure that has occurred and where it has occurred if called from multiple locations. If the failing function operates on some data try to include some descriptors of the failing data in the error message.
Example
// good
       res, err := handler(requestID)
       if err != nil {
           return errs.Wrapf(err, "failed to handle request %d", requestID)
       }
       
       err := update(path)
       if err != nil {
           return errs.Wrapf(err, "failed to update file %q", path)
       }If a function/method produces a new error, then instead of using errs.New (or any other error creation function) directly in the function, define a constant (technically it would be a variable) and return an error wrapping the constant. This way if the error is recoverable, callers can check the returned error against the constant.
// good
       var ErrConnectionDropped = errors.New("connection dropped")
       
       func connect(c Conn) {
           if c.IsDropped() {
               return errs.Wrapf(
                   ErrConnectionDropped, "failed to connect to %q", c.String(),
               )
           }
       
           err := c.Connect()
           if err != nil {
               return errs.Wrap(err, "failed to connect")
           }
       
           return nil
       }
       
       // bad
       func connect(c Conn) {
           if c.IsDropped() {
               return errs.Errorf(
                   "failed to connect to %q, connection was dropped", c.String(),
               )
           }
       
           err := c.Connect()
           if err != nil {
               return errs.Wrap(err, "failed to connect")
           }
       
           return nil
       }To get errors with parameters, use error wrapping.
// bad
       return errs.Errorf("invalid parameter [%s]", param)
       
       // good
       var ErrInvalidParameter = errs.New("invalid parameter")
       return errs.Wrapf(ErrInvalidParameter, "parameter - %s", param)Exported objects are the public interface of a package, having descriptive comments for them saves developers time in trying to understand what a particular piece of code does.
Try to put yourself in the shoes of a developer who's unfamiliar with the codebase trying to use the function/method in question - what information you would find useful then.
Here are some tips:
When a type implements an interface, add a type check with the type against the implemented interface. This provides a compile time check that the type actually implements the desired interface.
This also provides a convenient type check that the compiler will start screaming about if you change the implementations or interfaces signature.
Example
var (
           _ io.Writer = (*customBuffer)(nil)
           _ io.Reader = (*customBuffer)(nil)
       )
       
       type customBuffer struct {}Strict types are Go's power. Using them allows catching errors, mistakes, typos quickly and at compile time. Use of exact, strict types is one of Go's idioms.
Even though use of any/interface{} types is discouraged by previous rule, there are valid usages and usages that can't be avoided. In such cases use any instead of interface{} type. interface{} is deprecated. any is just a type alias to interface{}, it provides a clearer separation of interface{}/any - the type and interface - a set of method signatures that a type can implement.
range when iterating slices, accessing slice elementsGo has a range expression, for convenience and to reduce common out-of-bounds errors. Use range.
Example
// good
       for idx, element := range slice{
           // ...
       }
       // also good
       for _, element := range slice {
           // if you don't need the index.
           // ...
       }
       
       // bad
       for idx := 0; idx < len(slice); idx++ {
           // ...
       }In short →
Named returns have two main use cases:
deferAutomatic documentation sounds really nice at first, but falls apart quickly when analyzed deeper. The thing is that only thing pretty from the use of named returns is the signature. More importantly the function/methods body becomes a mess. Data flows get obfuscated, additional variable declarations are required.
Scenario 1.
func handle() (status int, err error) {
           // big messy spaghetti function
           // ...
           // ...
       
           if (cond) {
               return
           }
       
           // big messy spaghetti function continues
           // ...
           // ...
       
           return
       }On the other hand consider the following function
func handle() (int, error) {
           // big messy spaghetti function
           // ...
           // ...
       
           if (cond) {
               return 0, err
           }
       
           // big messy spaghetti function continues
           // ...
           // ...
       
           return 200, nil
       }Catching return values with defer in methods/functions with named returns is a valid use case of named returns. The only problem is that if you are in a situation where you need to do that you are likely doing something else wrong. That something else is likely single purpose methods/functions.
Function that does one thing only can be rewritten as:
func whapHandle() error {
           err := handle()
           if err != nil {
               return zbxerr.Wrap(err, "handle failed")
           }
       
           return nil
       }from:
func wrapHandle() (err error) {
           defer func() {
               if err != nil {
                   err = zbxerr.Wrap(err, "handle failed")
               }
           }()
       
           return handle()
       }Doing some complex logic in a defer needing to use a named return is a sign that you are trying to fit in too much logic into a single scope. Split it up.
Almost all long lines can be split to abide by line length limits. Split long lines, makes the codebase consistent and easy to read for other developers.
Examples - how to split long lines
Function / Method Calls
// function/method invocations
       handler(long, many, arguments) // exceeds line length limit
       // split
       handler(
           long, many, arguments,
       )
       // if still exceeds line length limit (though at this point you should
       // reconsider the logical structure of your code, as functions with this many
       // arguments might be handling too many things in a single scope)
       handler(
           long,
           many,
           arguments,
       )
       
       // function/method definitions
       func (h *handler) handle(long, many, params) (long, many, returns) {} // exceeds line length limit
       // split
       func (h *handler) handle(
           long, many, params,
       ) (long, many, returns) {}
       // if still exceeds line length limit
       func (h *handler) handle(
           long,
           many,
           params,
       ) (long, many, returns) {}
       // if still, still exceeds line length limit (consider restructuring logic)
       func (h *handler) handle(
           long,
           many,
           params,
       ) (
           long,
           many,
           returns,
       ) {}Don't define global variables exported or un-exported. Packages that define, use or rely on global variables are unsalable, not modular and harder to maintain.
init functionsWhere possible don't use init functions for modules. The use of init functions means that there are global variables that need initialization (see rule about global variables, that explains why you shouldn't use them).
Example
// good
       package handlers
       
       func NewHandler() *Handler {
           return &Handler{
               // ...
           }
       }
       
       // in a different package
       package server
       
       func StartServer() {
           h := handlers.NewHandler()
           h.Serve()
       }
       
       // bad
       var H *Handler
       
       func init() {
           H = &Handler{
               // ...
           }
       }.go files must have the following structure:
Lint should catch any deviations of the structure.
io.Stdout to write to stdoutFunctions like fmt.Printf, fmt.Println are commonly used for debugging purposes. Use fmt.Fprint to explicitly state that you want to write to io.Stdout and that you haven't just forgotten to remove a debug print.
gotestsgotests tool is a CLI tool that generates table driven test boilerplate skeletons from predefined generation templates. Using this tool ensures that unit test base structure is consistent for every one, makes tests easier to write, saves time.
Install gotests with:
Download predefined unit test templates for Zabbix - templates.
How are Zabbix templates different from the default ones?
cmp.Diff from https://github.com/google/go-cmp to compare complex objects. Its faster and provides better diffs that reflect.DeepEqual.t.Parallel(). If a test or its cases can't be run in parallel, then the t.Parallel() calls can be removed, but by default encourage parallel test execution.t.Fatalf() to failing cases.To generate a test from the command line run
gotests \
         -template_dir '/path/to/zabbix/test/templates/dir' \
         -only 'nameOfFuncionToTest' \
         -w \
         '/path/to/source/code/file/or/dir'Flags used:
-template_dir custom unit test skeleton templates for Zabbix (read more about the custom templates below)-only the function or methods name to test-w write the generated skeleton to _test.go file.Check https://github.com/cweill/gotests for integration of your choice of editor. Don't forget to configure use of Zabbix templates.
+ for positive cases or - for negative cases.A positive case is when the function being tested produces a valid result. A negative case is when the function produces invalid result or is called with invalid input.
Example
+validThe +valid case demonstrates the successful flow of the function being tested.
Example
The cases of a test must be ordered in a way that aligns with the execution flow of the function/method, placing positive cases first, followed by all negative cases.
Example
The test target function:
func (h *handler) proxyFile(w http.ResponseWriter, path string) error {
           req, err := http.NewRequestWithContext(
               context.Background(),
               http.MethodGet,
               path,
               nil,
           )
           if err != nil {
               return errors.Wrap(err, "failed to create request")
           }
       
           resp, err := h.httpClient.Do(req)
           if err != nil {
               return errors.Wrap(err, "failed to do request")
           }
       
           defer resp.Body.Close()
       
           _, err = io.Copy(w, resp.Body)
           if err != nil {
               return errors.Wrap(err, "failed to copy source request response")
           }
       
           w.WriteHeader(resp.StatusCode)
       
           return nil
       }The test cases:
+valid-emptyPath-errCreatingNewRequest-errDoingRequest-errCopyingRespt.Fatalf to fail a casecmp.Diff to produce human-readable diffsExample
ret, err := function()
       // ... other checks ...
       if diff := cmp.Diff(tt.wantRet, ret); diff != "" {
           t.Fatalf("function() return value mismatch (-want +got):\n%s", diff)
       }Note that an inline if statement is used here to mimic a simple if x != y check and is the default in unit test generation template.
Use cmp.Diff to compare anything more complex than a int, and print the diff, describing exactly what mismatched.
Describe in failure messages, exactly what went wrong, what was the expected result.
receiver.method() or function()Having the prefix makes it easier to understand where a test failure is coming from.
Example
Here ConnCollection is the receiver and newConn is the method.
This way the case checks that the loop actually does what it's intended to.
Example
Unit test target function:
func prefix(p string, words []string) []string {
           ret := make([]string, 0, len(s))
       
           for _, w := range words {
               ret = append(ret, p + w)
           }
       
           return ret
       }Test case for prefix must contain a case that make the loop iterate twice
{
           "+valid",
           args{
               "prefix",
               // more that two element == more than two iterations
               []string{"a", "b", "c"},
           },
           []string{"prefixa", "prefixb", "prefixc"},
       }Bad example case:
{
           "+badDontDoThis",
           args{
               "prefix",
               // triggers the loop in the tested function only once. for all we know
               // from this test all that the function might be doing is
               // return []string{"prefixa"}
               []string{"a"},
           },
           []string{"prefixa"},
       }It's common that unit test input/output data can be shared between tests. If tests share data, then small changes to code have a cascading effect in tests, where changing one part of shared input/output data requires adjusting all tests that use it.
Define input and output data in each test scope.
expect field for cases when using mocks.When using mocks add a field to the cases' table expect. It should be a struct with boolean values for each mock expectation that the function has.
This way each case controls what expectation on the function should be placed.
Example
type expect struct {
           read   bool
           update bool
       }
       
       tests := []struct {
           name           string
           expect         expect
           fields         fields
           args           args
           wantErr        bool
       }{
           // ...
       }
       for _, tt := range tests {
           tt := tt
           t.Run(tt.name, func(t *testing.T) {
               t.Parallel()
       
               mock := &Mock{}
       
               if tt.expect.read {
                   mock.ExpectRead()
               }
       
               if tt.expect.update {
                   mock.ExpectUpdate()
               }
       
               err := function(mock)
               if (err != nil) != tt.wantErr {
                   t.Fatalf("function() error = %v, wantErr %v", err, tt.wantErr)
               }
               if err := mock.ExpectationsWhereMet(): err != nil {
                   t.Fatalf(
                       "function() DB mock expectations where not met: %s",
                       err.Error(),
                   )
               }
           })
       }Helper packages help in working of and keeping Zabbix agent 2 Go functionality consistent. The most commonly used packages are zbxerr, log and uri. The full list of packages utilized by Zabbix is given in the Overview section.
Zbxerr package handles error formatting. The instructions on how to create a new error message are given below:
::: noteclassic See the samples for Error messages and Zabbix naming guidelines :::
When it is necessary to add third-party libraries, it is done in a following way:
go get command from ~/zabbix/src/go directory:Any additional plugin parameter configuration is also done in plugins. See the instructions below.
go.mod file:go.sum, it might be necessary to run this one:Parameters for Zabbix agent 2 are configured utilizing the native Go flag package. See the instructions for configuration below.
    var testFlag string
           const (
               testDefault     = ""
               testDescription = "description"
           )
           flag.StringVar(&testFlag, "test", testDefault, testDescription)
           flag.StringVar(&testFlag, "t", testDefault, testDescription+" (shorthand)")::: noteclassic Parameters for Zabbix agent 2 plugins are defined and handled separately for each plugin. Typically, they are checked for the allowed number of parameters and values. :::
Follow the instructions given below.
Go code must conform to the rules specified in a configuration file:::: noteimportant In this syntax, X denotes the oldest commit number from where the changes began. :::
::: noteclassic In order to run the checks, it is required to install golangci-lint:. :::