lint: Enable errcheck, fix failures

This enables linting with errcheck on the repository.
Exclusions were added for functions that are known
to never fail, e.g. all Write methods on Zap's buffer.Buffer.

In attempting to enable exclusions for these functions,
I discovered and fixed a typo in the golangci.yml.

.golangci.yml

@@ -9,7 +9,7 @@ linters:
   disable-all: true
   enable:
     # golangci-lint defaults:
-    # - errcheck TODO: enable errcheck
+    - errcheck
     - gosimple
     - govet
     - ineffassign
@@ -21,7 +21,7 @@ linters:
     - nolintlint # lints nolint directives
     - revive
 
-linter-settings:
+linters-settings:
   govet:
     # These govet checks are disabled by default, but they're useful.
     enable:
@@ -30,6 +30,24 @@ linter-settings:
       - sortslice
       - unusedwrite
 
+  errcheck:
+    exclude-functions:
+      # These methods can not fail.
+      # They operate on an in-memory buffer.
+      - (*go.uber.org/zap/buffer.Buffer).Write
+      - (*go.uber.org/zap/buffer.Buffer).WriteByte
+      - (*go.uber.org/zap/buffer.Buffer).WriteString
+
+      - (*go.uber.org/zap/zapio.Writer).Close
+      - (*go.uber.org/zap/zapio.Writer).Sync
+      - (*go.uber.org/zap/zapio.Writer).Write
+      # Write to zapio.Writer cannot fail,
+      # so io.WriteString on it cannot fail.
+      - io.WriteString(*go.uber.org/zap/zapio.Writer)
+
+      # Writing a plain string to a fmt.State cannot fail.
+      - io.WriteString(fmt.State)
+
 issues:
   # Print all issues reported by all linters.
   max-issues-per-linter: 0
@@ -51,3 +69,9 @@ issues:
     #   for foo() { }
     - linters: [revive]
       text: 'empty-block: this block is empty, you can remove it'
+
+    # Ignore logger.Sync() errcheck failures in example_test.go
+    # since those are intended to be uncomplicated examples.
+    - linters: [errcheck]
+      path: example_test.go
+      text: 'Error return value of `logger.Sync` is not checked'

benchmarks/scenario_bench_test.go

@@ -104,7 +104,6 @@ func BenchmarkDisabledWithoutFields(b *testing.B) {
 			}
 		})
 	})
-
 }
 
 func BenchmarkDisabledAccumulatedContext(b *testing.B) {
@@ -183,7 +182,6 @@ func BenchmarkDisabledAccumulatedContext(b *testing.B) {
 			}
 		})
 	})
-
 }
 
 func BenchmarkDisabledAddingFields(b *testing.B) {
@@ -253,7 +251,6 @@ func BenchmarkDisabledAddingFields(b *testing.B) {
 			}
 		})
 	})
-
 }
 
 func BenchmarkWithoutFields(b *testing.B) {
@@ -323,7 +320,9 @@ func BenchmarkWithoutFields(b *testing.B) {
 		b.ResetTimer()
 		b.RunParallel(func(pb *testing.PB) {
 			for pb.Next() {
-				logger.Log(getMessage(0), getMessage(1))
+				if err := logger.Log(getMessage(0), getMessage(1)); err != nil {
+					b.Fatal(err)
+				}
 			}
 		})
 	})
@@ -401,7 +400,6 @@ func BenchmarkWithoutFields(b *testing.B) {
 			}
 		})
 	})
-
 }
 
 func BenchmarkAccumulatedContext(b *testing.B) {
@@ -471,7 +469,9 @@ func BenchmarkAccumulatedContext(b *testing.B) {
 		b.ResetTimer()
 		b.RunParallel(func(pb *testing.PB) {
 			for pb.Next() {
-				logger.Log(getMessage(0), getMessage(1))
+				if err := logger.Log(getMessage(0), getMessage(1)); err != nil {
+					b.Fatal(err)
+				}
 			}
 		})
 	})
@@ -531,7 +531,6 @@ func BenchmarkAccumulatedContext(b *testing.B) {
 			}
 		})
 	})
-
 }
 
 func BenchmarkAddingFields(b *testing.B) {
@@ -592,7 +591,9 @@ func BenchmarkAddingFields(b *testing.B) {
 		b.ResetTimer()
 		b.RunParallel(func(pb *testing.PB) {
 			for pb.Next() {
-				logger.Log(fakeSugarFields()...)
+				if err := logger.Log(fakeSugarFields()...); err != nil {
+					b.Fatal(err)
+				}
 			}
 		})
 	})
@@ -643,5 +644,4 @@ func BenchmarkAddingFields(b *testing.B) {
 			}
 		})
 	})
-
 }

encoder_test.go

@@ -41,7 +41,7 @@ func TestRegisterEncoder(t *testing.T) {
 
 func TestDuplicateRegisterEncoder(t *testing.T) {
 	testEncoders(func() {
-		RegisterEncoder("foo", newNilEncoder)
+		assert.NoError(t, RegisterEncoder("foo", newNilEncoder), "expected to be able to register the encoder foo")
 		assert.Error(t, RegisterEncoder("foo", newNilEncoder), "expected an error when registering an encoder with the same name twice")
 	})
 }
@@ -52,7 +52,7 @@ func TestRegisterEncoderNoName(t *testing.T) {
 
 func TestNewEncoder(t *testing.T) {
 	testEncoders(func() {
-		RegisterEncoder("foo", newNilEncoder)
+		assert.NoError(t, RegisterEncoder("foo", newNilEncoder), "expected to be able to register the encoder foo")
 		encoder, err := newEncoder("foo", zapcore.EncoderConfig{})
 		assert.NoError(t, err, "could not create an encoder for the registered name foo")
 		assert.Nil(t, encoder, "the encoder from newNilEncoder is not nil")

error.go

@@ -61,9 +61,12 @@ func (errs errArray) MarshalLogArray(arr zapcore.ArrayEncoder) error {
 		// allocating, pool the wrapper type.
 		elem := _errArrayElemPool.Get()
 		elem.error = errs[i]
-		arr.AppendObject(elem)
+		err := arr.AppendObject(elem)
 		elem.error = nil
 		_errArrayElemPool.Put(elem)
+		if err != nil {
+			return err
+		}
 	}
 	return nil
 }

http_handler.go

@@ -69,6 +69,13 @@ import (
 //
 //	curl -X PUT localhost:8080/log/level -H "Content-Type: application/json" -d '{"level":"debug"}'
 func (lvl AtomicLevel) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+	if err := lvl.serveHTTP(w, r); err != nil {
+		w.WriteHeader(http.StatusInternalServerError)
+		fmt.Fprintf(w, "internal error: %v", err)
+	}
+}
+
+func (lvl AtomicLevel) serveHTTP(w http.ResponseWriter, r *http.Request) error {
 	type errorResponse struct {
 		Error string `json:"error"`
 	}
@@ -80,19 +87,20 @@ func (lvl AtomicLevel) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 
 	switch r.Method {
 	case http.MethodGet:
-		enc.Encode(payload{Level: lvl.Level()})
+		return enc.Encode(payload{Level: lvl.Level()})
+
 	case http.MethodPut:
 		requestedLvl, err := decodePutRequest(r.Header.Get("Content-Type"), r)
 		if err != nil {
 			w.WriteHeader(http.StatusBadRequest)
-			enc.Encode(errorResponse{Error: err.Error()})
-			return
+			return enc.Encode(errorResponse{Error: err.Error()})
 		}
 		lvl.SetLevel(requestedLvl)
-		enc.Encode(payload{Level: lvl.Level()})
+		return enc.Encode(payload{Level: lvl.Level()})
+
 	default:
 		w.WriteHeader(http.StatusMethodNotAllowed)
-		enc.Encode(errorResponse{
+		return enc.Encode(errorResponse{
 			Error: "Only GET and PUT are supported.",
 		})
 	}
@@ -129,5 +137,4 @@ func decodePutJSON(body io.Reader) (zapcore.Level, error) {
 		return 0, errors.New("must specify logging level")
 	}
 	return *pld.Level, nil
-
 }

http_handler_test.go

@@ -167,7 +167,9 @@ func TestAtomicLevelServeHTTP(t *testing.T) {
 
 			res, err := http.DefaultClient.Do(req)
 			require.NoError(t, err, "Error making %s request.", req.Method)
-			defer res.Body.Close()
+			defer func() {
+				assert.NoError(t, res.Body.Close(), "Error closing response body.")
+			}()
 
 			require.Equal(t, tt.expectedCode, res.StatusCode, "Unexpected status code.")
 			if tt.expectedCode != http.StatusOK {

logger.go

@@ -374,7 +374,7 @@ func (log *Logger) check(lvl zapcore.Level, msg string) *zapcore.CheckedEntry {
 	if stack.Count() == 0 {
 		if log.addCaller {
 			fmt.Fprintf(log.errorOutput, "%v Logger.check error: failed to get caller\n", ent.Time.UTC())
-			log.errorOutput.Sync()
+			_ = log.errorOutput.Sync()
 		}
 		return ce
 	}

sink.go

@@ -66,7 +66,8 @@ func newSinkRegistry() *sinkRegistry {
 		factories: make(map[string]func(*url.URL) (Sink, error)),
 		openFile:  os.OpenFile,
 	}
-	sr.RegisterSink(schemeFile, sr.newFileSinkFromURL)
+	// Infallible operation: the registry is empty, so we can't have a conflict.
+	_ = sr.RegisterSink(schemeFile, sr.newFileSinkFromURL)
 	return sr
 }
 
@@ -154,7 +155,7 @@ func (sr *sinkRegistry) newFileSinkFromPath(path string) (Sink, error) {
 	case "stderr":
 		return nopCloserSink{os.Stderr}, nil
 	}
-	return sr.openFile(path, os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0666)
+	return sr.openFile(path, os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0o666)
 }
 
 func normalizeScheme(s string) (string, error) {

stacktrace_ext_test.go

@@ -97,7 +97,7 @@ func TestStacktraceFiltersVendorZap(t *testing.T) {
 
 		testDir := filepath.Join(goPath, "src/go.uber.org/zap_test/")
 		vendorDir := filepath.Join(testDir, "vendor")
-		require.NoError(t, os.MkdirAll(testDir, 0777), "Failed to create source director")
+		require.NoError(t, os.MkdirAll(testDir, 0o777), "Failed to create source director")
 
 		curFile := getSelfFilename(t)
 		setupSymlink(t, curFile, filepath.Join(testDir, curFile))
@@ -175,7 +175,7 @@ func getSelfFilename(t *testing.T) string {
 
 func setupSymlink(t *testing.T, src, dst string) {
 	// Make sure the destination directory exists.
-	os.MkdirAll(filepath.Dir(dst), 0777)
+	require.NoError(t, os.MkdirAll(filepath.Dir(dst), 0o777))
 
 	// Get absolute path of the source for the symlink, otherwise we can create a symlink
 	// that uses relative paths.

writer.go

@@ -62,7 +62,7 @@ func open(paths []string) ([]zapcore.WriteSyncer, func(), error) {
 	closers := make([]io.Closer, 0, len(paths))
 	closeAll := func() {
 		for _, c := range closers {
-			c.Close()
+			_ = c.Close()
 		}
 	}
 

writer_test.go

@@ -91,7 +91,6 @@ func TestOpen(t *testing.T) {
 	}
 
 	assert.True(t, fileExists(tempName))
-	os.Remove(tempName)
 }
 
 func TestOpenPathsNotFound(t *testing.T) {
@@ -255,7 +254,8 @@ func TestOpenWithErroringSinkFactory(t *testing.T) {
 func TestCombineWriteSyncers(t *testing.T) {
 	tw := &testWriter{"test", t}
 	w := CombineWriteSyncers(tw)
-	w.Write([]byte("test"))
+	_, err := w.Write([]byte("test"))
+	assert.NoError(t, err, "Unexpected write error.")
 }
 
 func fileExists(name string) bool {

zapcore/buffered_write_syncer_bench_test.go

@@ -40,11 +40,15 @@ func BenchmarkBufferedWriteSyncer(b *testing.B) {
 		w := &BufferedWriteSyncer{
 			WS: AddSync(file),
 		}
-		defer w.Stop()
+		defer func() {
+			assert.NoError(b, w.Stop(), "failed to stop buffered write syncer")
+		}()
 		b.ResetTimer()
 		b.RunParallel(func(pb *testing.PB) {
 			for pb.Next() {
-				w.Write([]byte("foobarbazbabble"))
+				if _, err := w.Write([]byte("foobarbazbabble")); err != nil {
+					b.Fatal(err)
+				}
 			}
 		})
 	})

zapcore/buffered_write_syncer_test.go

@@ -101,7 +101,8 @@ func TestBufferWriter(t *testing.T) {
 		n, err := ws.Write([]byte("foo"))
 		require.NoError(t, err, "Unexpected error writing to WriteSyncer.")
 		require.Equal(t, 3, n, "Wrote an unexpected number of bytes.")
-		ws.Write([]byte("foo"))
+		_, err = ws.Write([]byte("foo"))
+		assert.Error(t, err, "Expected error writing to WriteSyncer.")
 		assert.Error(t, ws.Stop(), "Expected stop to fail.")
 	})
 

zapcore/core.go

@@ -102,9 +102,9 @@ func (c *ioCore) Write(ent Entry, fields []Field) error {
 		return err
 	}
 	if ent.Level > ErrorLevel {
-		// Since we may be crashing the program, sync the output. Ignore Sync
-		// errors, pending a clean solution to issue #370.
-		c.Sync()
+		// Since we may be crashing the program, sync the output.
+		// Ignore Sync errors, pending a clean solution to issue #370.
+		_ = c.Sync()
 	}
 	return nil
 }

zapcore/core_test.go

@@ -148,7 +148,7 @@ func TestIOCoreSyncsOutput(t *testing.T) {
 			DebugLevel,
 		)
 
-		core.Write(tt.entry, nil)
+		assert.NoError(t, core.Write(tt.entry, nil), "Unexpected error writing entry.")
 		assert.Equal(t, tt.shouldSync, sink.Called(), "Incorrect Sync behavior.")
 	}
 }

zapcore/encoder_test.go

@@ -285,10 +285,11 @@ func TestEncoderConfiguration(t *testing.T) {
 			},
 			extra: func(enc Encoder) {
 				enc.AddTime("extra", _epoch)
-				enc.AddArray("extras", ArrayMarshalerFunc(func(enc ArrayEncoder) error {
+				err := enc.AddArray("extras", ArrayMarshalerFunc(func(enc ArrayEncoder) error {
 					enc.AppendTime(_epoch)
 					return nil
 				}))
+				assert.NoError(t, err)
 			},
 			expectedJSON: `{"L":"info","T":"1970-01-01 00:00:00 +0000 UTC","N":"main","C":"foo.go:42","F":"foo.Foo","M":"hello","extra":"1970-01-01 00:00:00 +0000 UTC","extras":["1970-01-01 00:00:00 +0000 UTC"],"S":"fake-stack"}` + "\n",
 			expectedConsole: "1970-01-01 00:00:00 +0000 UTC\tinfo\tmain\tfoo.go:42\tfoo.Foo\thello\t" + // plain-text preamble
@@ -313,10 +314,11 @@ func TestEncoderConfiguration(t *testing.T) {
 			},
 			extra: func(enc Encoder) {
 				enc.AddDuration("extra", time.Second)
-				enc.AddArray("extras", ArrayMarshalerFunc(func(enc ArrayEncoder) error {
+				err := enc.AddArray("extras", ArrayMarshalerFunc(func(enc ArrayEncoder) error {
 					enc.AppendDuration(time.Minute)
 					return nil
 				}))
+				assert.NoError(t, err)
 			},
 			expectedJSON: `{"L":"info","T":0,"N":"main","C":"foo.go:42","F":"foo.Foo","M":"hello","extra":"1s","extras":["1m0s"],"S":"fake-stack"}` + "\n",
 			expectedConsole: "0\tinfo\tmain\tfoo.go:42\tfoo.Foo\thello\t" + // preamble
@@ -720,10 +722,11 @@ func TestNameEncoders(t *testing.T) {
 
 func assertAppended(t testing.TB, expected interface{}, f func(ArrayEncoder), msgAndArgs ...interface{}) {
 	mem := NewMapObjectEncoder()
-	mem.AddArray("k", ArrayMarshalerFunc(func(arr ArrayEncoder) error {
+	err := mem.AddArray("k", ArrayMarshalerFunc(func(arr ArrayEncoder) error {
 		f(arr)
 		return nil
 	}))
+	assert.NoError(t, err, msgAndArgs...)
 	arr := mem.Fields["k"].([]interface{})
 	require.Equal(t, 1, len(arr), "Expected to append exactly one element to array.")
 	assert.Equal(t, expected, arr[0], msgAndArgs...)