Merge pull request #1729 from calmh/lint-clean

Run vet and lint. Make us lint clean.

build.go

@@ -78,6 +78,11 @@ func main() {
 			tags = []string{"noupgrade"}
 		}
 		install("./cmd/...", tags)
+
+		vet("./cmd/syncthing")
+		vet("./internal/...")
+		lint("./cmd/syncthing")
+		lint("./internal/...")
 		return
 	}
 
@@ -103,8 +108,7 @@ func main() {
 			build(pkg, tags)
 
 		case "test":
-			pkg := "./..."
+			test("./...")
-			test(pkg)
 
 		case "assets":
 			assets()
@@ -130,6 +134,14 @@ func main() {
 		case "clean":
 			clean()
 
+		case "vet":
+			vet("./cmd/syncthing")
+			vet("./internal/...")
+
+		case "lint":
+			lint("./cmd/syncthing")
+			lint("./internal/...")
+
 		default:
 			log.Fatalf("Unknown command %q", cmd)
 		}
@@ -437,10 +449,7 @@ func run(cmd string, args ...string) []byte {
 func runError(cmd string, args ...string) ([]byte, error) {
 	ecmd := exec.Command(cmd, args...)
 	bs, err := ecmd.CombinedOutput()
-	if err != nil {
+	return bytes.TrimSpace(bs), err
-		return nil, err
-	}
-	return bytes.TrimSpace(bs), nil
 }
 
 func runPrint(cmd string, args ...string) {
@@ -615,3 +624,37 @@ func md5File(file string) error {
 
 	return out.Close()
 }
+
+func vet(pkg string) {
+	bs, err := runError("go", "vet", pkg)
+	if err != nil && err.Error() == "exit status 3" || bytes.Contains(bs, []byte("no such tool \"vet\"")) {
+		// Go said there is no go vet
+		log.Println(`- No go vet, no vetting. Try "go get -u golang.org/x/tools/cmd/vet".`)
+		return
+	}
+
+	falseAlarmComposites := regexp.MustCompile("composite literal uses unkeyed fields")
+	exitStatus := regexp.MustCompile("exit status 1")
+	for _, line := range bytes.Split(bs, []byte("\n")) {
+		if falseAlarmComposites.Match(line) || exitStatus.Match(line) {
+			continue
+		}
+		log.Printf("%s", line)
+	}
+}
+
+func lint(pkg string) {
+	bs, err := runError("golint", pkg)
+	if err != nil {
+		log.Println(`- No golint, not linting. Try "go get -u github.com/golang/lint/golint".`)
+		return
+	}
+
+	analCommentPolicy := regexp.MustCompile(`exported (function|method|const|type|var) [^\s]+ should have comment`)
+	for _, line := range bytes.Split(bs, []byte("\n")) {
+		if analCommentPolicy.Match(line) {
+			continue
+		}
+		log.Printf("%s", line)
+	}
+}

build.sh

@@ -118,8 +118,6 @@ case "${1:-default}" in
 			-e "STTRACE=$STTRACE" \
 			syncthing/build:latest \
 			sh -c './build.sh clean \
-				&& go tool vet -composites=false cmd/*/*.go internal/*/*.go \
-				&& ( golint ./cmd/... ; golint ./internal/... ) | egrep -v "comment on exported|should have comment" \
 				&& ./build.sh all \
 				&& STTRACE=all ./build.sh test-cov'
 		;;
@@ -138,6 +136,25 @@ case "${1:-default}" in
 				&& git clean -fxd .'
 		;;
 
+	docker-lint)
+		docker run --rm -h syncthing-builder -u $(id -u) -t \
+			-v $(pwd):/go/src/github.com/syncthing/syncthing \
+			-w /go/src/github.com/syncthing/syncthing \
+			-e "STTRACE=$STTRACE" \
+			syncthing/build:latest \
+			sh -euxc 'go run build.go lint'
+		;;
+
+
+	docker-vet)
+		docker run --rm -h syncthing-builder -u $(id -u) -t \
+			-v $(pwd):/go/src/github.com/syncthing/syncthing \
+			-w /go/src/github.com/syncthing/syncthing \
+			-e "STTRACE=$STTRACE" \
+			syncthing/build:latest \
+			sh -euxc 'go run build.go vet'
+		;;
+
 	*)
 		echo "Unknown build command $1"
 		;;

cmd/syncthing/gui.go

@@ -45,16 +45,16 @@ type guiError struct {
 }
 
 var (
-	configInSync            = true
+	configInSync = true
-	guiErrors               = []guiError{}
+	guiErrors    = []guiError{}
-	guiErrorsMut sync.Mutex = sync.NewMutex()
+	guiErrorsMut = sync.NewMutex()
-	startTime               = time.Now()
+	startTime    = time.Now()
 	eventSub     *events.BufferedSubscription
 )
 
 var (
 	lastEventRequest    time.Time
-	lastEventRequestMut sync.Mutex = sync.NewMutex()
+	lastEventRequestMut = sync.NewMutex()
 )
 
 func startGUI(cfg config.GUIConfiguration, assetDir string, m *model.Model) error {
@@ -538,7 +538,7 @@ func flushResponse(s string, w http.ResponseWriter) {
 }
 
 var cpuUsagePercent [10]float64 // The last ten seconds
-var cpuUsageLock sync.RWMutex = sync.NewRWMutex()
+var cpuUsageLock = sync.NewRWMutex()
 
 func restGetSystemStatus(w http.ResponseWriter, r *http.Request) {
 	var m runtime.MemStats

cmd/syncthing/gui_auth.go

@@ -20,8 +20,8 @@ import (
 )
 
 var (
-	sessions               = make(map[string]bool)
+	sessions    = make(map[string]bool)
-	sessionsMut sync.Mutex = sync.NewMutex()
+	sessionsMut = sync.NewMutex()
 )
 
 func basicAuthAndSessionMiddleware(cfg config.GUIConfiguration, next http.Handler) http.Handler {

cmd/syncthing/gui_csrf.go

@@ -19,7 +19,7 @@ import (
 )
 
 var csrfTokens []string
-var csrfMut sync.Mutex = sync.NewMutex()
+var csrfMut = sync.NewMutex()
 
 // Check for CSRF token on /rest/ URLs. If a correct one is not given, reject
 // the request with 403. For / and /index.html, set a new CSRF cookie if none

cmd/syncthing/monitor.go

@@ -22,9 +22,9 @@ import (
 )
 
 var (
-	stdoutFirstLines []string   // The first 10 lines of stdout
+	stdoutFirstLines []string // The first 10 lines of stdout
-	stdoutLastLines  []string   // The last 50 lines of stdout
+	stdoutLastLines  []string // The last 50 lines of stdout
-	stdoutMut        sync.Mutex = sync.NewMutex()
+	stdoutMut        = sync.NewMutex()
 )
 
 const (

internal/config/config.go

@@ -45,28 +45,28 @@ type Configuration struct {
 	OriginalVersion int `xml:"-" json:"-"` // The version we read from disk, before any conversion
 }
 
-func (orig Configuration) Copy() Configuration {
+func (cfg Configuration) Copy() Configuration {
-	c := orig
+	newCfg := cfg
 
 	// Deep copy FolderConfigurations
-	c.Folders = make([]FolderConfiguration, len(orig.Folders))
+	newCfg.Folders = make([]FolderConfiguration, len(cfg.Folders))
-	for i := range c.Folders {
+	for i := range newCfg.Folders {
-		c.Folders[i] = orig.Folders[i].Copy()
+		newCfg.Folders[i] = cfg.Folders[i].Copy()
 	}
 
 	// Deep copy DeviceConfigurations
-	c.Devices = make([]DeviceConfiguration, len(orig.Devices))
+	newCfg.Devices = make([]DeviceConfiguration, len(cfg.Devices))
-	for i := range c.Devices {
+	for i := range newCfg.Devices {
-		c.Devices[i] = orig.Devices[i].Copy()
+		newCfg.Devices[i] = cfg.Devices[i].Copy()
 	}
 
-	c.Options = orig.Options.Copy()
+	newCfg.Options = cfg.Options.Copy()
 
 	// DeviceIDs are values
-	c.IgnoredDevices = make([]protocol.DeviceID, len(orig.IgnoredDevices))
+	newCfg.IgnoredDevices = make([]protocol.DeviceID, len(cfg.IgnoredDevices))
-	copy(c.IgnoredDevices, orig.IgnoredDevices)
+	copy(newCfg.IgnoredDevices, cfg.IgnoredDevices)
 
-	return c
+	return newCfg
 }
 
 type FolderConfiguration struct {
@@ -89,10 +89,10 @@ type FolderConfiguration struct {
 	deviceIDs []protocol.DeviceID
 }
 
-func (orig FolderConfiguration) Copy() FolderConfiguration {
+func (f FolderConfiguration) Copy() FolderConfiguration {
-	c := orig
+	c := f
-	c.Devices = make([]FolderDeviceConfiguration, len(orig.Devices))
+	c.Devices = make([]FolderDeviceConfiguration, len(f.Devices))
-	copy(c.Devices, orig.Devices)
+	copy(c.Devices, f.Devices)
 	return c
 }
 

internal/config/wrapper.go

@@ -156,7 +156,7 @@ func (w *Wrapper) SetDevice(dev DeviceConfiguration) {
 	w.replaces <- w.cfg.Copy()
 }
 
-// Devices returns a map of folders. Folder structures should not be changed,
+// Folders returns a map of folders. Folder structures should not be changed,
 // other than for the purpose of updating via SetFolder().
 func (w *Wrapper) Folders() map[string]FolderConfiguration {
 	w.mut.Lock()
@@ -220,8 +220,8 @@ func (w *Wrapper) SetGUI(gui GUIConfiguration) {
 	w.replaces <- w.cfg.Copy()
 }
 
-// Returns whether or not connection attempts from the given device should be
+// IgnoredDevice returns whether or not connection attempts from the given
-// silently ignored.
+// device should be silently ignored.
 func (w *Wrapper) IgnoredDevice(id protocol.DeviceID) bool {
 	w.mut.Lock()
 	defer w.mut.Unlock()

internal/db/blockmap.go

@@ -10,7 +10,6 @@
 // depending on who calls us. We transform paths to wire-format (NFC and
 // slashes) on the way to the database, and transform to native format
 // (varying separator and encoding) on the way back out.
-
 package db
 
 import (
@@ -131,7 +130,7 @@ func NewBlockFinder(db *leveldb.DB, cfg *config.Wrapper) *BlockFinder {
 	return f
 }
 
-// Implements config.Handler interface
+// Changed implements config.Handler interface
 func (f *BlockFinder) Changed(cfg config.Configuration) error {
 	folders := make([]string, len(cfg.Folders))
 	for i, folder := range cfg.Folders {
@@ -147,11 +146,11 @@ func (f *BlockFinder) Changed(cfg config.Configuration) error {
 	return nil
 }
 
-// An iterator function which iterates over all matching blocks for the given
+// Iterate takes an iterator function which iterates over all matching blocks
-// hash. The iterator function has to return either true (if they are happy with
+// for the given hash. The iterator function has to return either true (if
-// the block) or false to continue iterating for whatever reason.
+// they are happy with the block) or false to continue iterating for whatever
-// The iterator finally returns the result, whether or not a satisfying block
+// reason. The iterator finally returns the result, whether or not a
-// was eventually found.
+// satisfying block was eventually found.
 func (f *BlockFinder) Iterate(hash []byte, iterFn func(string, string, int32) bool) bool {
 	f.mut.RLock()
 	folders := f.folders
@@ -172,8 +171,8 @@ func (f *BlockFinder) Iterate(hash []byte, iterFn func(string, string, int32) bo
 	return false
 }
 
-// A method for repairing incorrect blockmap entries, removes the old entry
+// Fix repairs incorrect blockmap entries, removing the old entry and
-// and replaces it with a new entry for the given block
+// replacing it with a new entry for the given block
 func (f *BlockFinder) Fix(folder, file string, index int32, oldHash, newHash []byte) error {
 	buf := make([]byte, 4)
 	binary.BigEndian.PutUint32(buf, uint32(index))

internal/db/leveldb.go

@@ -25,7 +25,7 @@ import (
 
 var (
 	clockTick int64
-	clockMut  sync.Mutex = sync.NewMutex()
+	clockMut  = sync.NewMutex()
 )
 
 func clock(v int64) int64 {
@@ -976,11 +976,11 @@ func unmarshalTrunc(bs []byte, truncate bool) (FileIntf, error) {
 		var tf FileInfoTruncated
 		err := tf.UnmarshalXDR(bs)
 		return tf, err
-	} else {
-		var tf protocol.FileInfo
-		err := tf.UnmarshalXDR(bs)
-		return tf, err
 	}
+
+	var tf protocol.FileInfo
+	err := tf.UnmarshalXDR(bs)
+	return tf, err
 }
 
 func ldbCheckGlobals(db *leveldb.DB, folder []byte) {

internal/fnmatch/fnmatch.go

@@ -75,8 +75,8 @@ func Convert(pattern string, flags int) (*regexp.Regexp, error) {
 	return regexp.Compile(pattern)
 }
 
-// Matches the pattern against the string, with the given flags,
+// Match matches the pattern against the string, with the given flags, and
-// and returns true if the match is successful.
+// returns true if the match is successful.
 func Match(pattern, s string, flags int) (bool, error) {
 	exp, err := Convert(pattern, flags)
 	if err != nil {

internal/ignore/ignore.go

@@ -30,9 +30,8 @@ type Pattern struct {
 func (p Pattern) String() string {
 	if p.include {
 		return p.match.String()
-	} else {
-		return "(?exclude)" + p.match.String()
 	}
+	return "(?exclude)" + p.match.String()
 }
 
 type Matcher struct {

internal/model/model.go

@@ -125,15 +125,16 @@ func NewModel(cfg *config.Wrapper, id protocol.DeviceID, deviceName, clientName,
 	return m
 }
 
-// Starts deadlock detector on the models locks which causes panics in case
+// StartDeadlockDetector starts a deadlock detector on the models locks which
-// the locks cannot be acquired in the given timeout period.
+// causes panics in case the locks cannot be acquired in the given timeout
+// period.
 func (m *Model) StartDeadlockDetector(timeout time.Duration) {
 	l.Infof("Starting deadlock detector with %v timeout", timeout)
 	deadlockDetect(m.fmut, timeout)
 	deadlockDetect(m.pmut, timeout)
 }
 
-// StartRW starts read/write processing on the current model. When in
+// StartFolderRW starts read/write processing on the current model. When in
 // read/write mode the model will attempt to keep in sync with the cluster by
 // pulling needed files from peer devices.
 func (m *Model) StartFolderRW(folder string) {
@@ -166,9 +167,9 @@ func (m *Model) StartFolderRW(folder string) {
 	go p.Serve()
 }
 
-// StartRO starts read only processing on the current model. When in
+// StartFolderRO starts read only processing on the current model. When in
-// read only mode the model will announce files to the cluster but not
+// read only mode the model will announce files to the cluster but not pull in
-// pull in any external changes.
+// any external changes.
 func (m *Model) StartFolderRO(folder string) {
 	m.fmut.Lock()
 	cfg, ok := m.folderCfgs[folder]
@@ -243,7 +244,7 @@ func (m *Model) ConnectionStats() map[string]interface{} {
 	return res
 }
 
-// Returns statistics about each device
+// DeviceStatistics returns statistics about each device
 func (m *Model) DeviceStatistics() map[string]stats.DeviceStatistics {
 	var res = make(map[string]stats.DeviceStatistics)
 	for id := range m.cfg.Devices() {
@@ -252,7 +253,7 @@ func (m *Model) DeviceStatistics() map[string]stats.DeviceStatistics {
 	return res
 }
 
-// Returns statistics about each folder
+// FolderStatistics returns statistics about each folder
 func (m *Model) FolderStatistics() map[string]stats.FolderStatistics {
 	var res = make(map[string]stats.FolderStatistics)
 	for id := range m.cfg.Folders() {
@@ -261,7 +262,8 @@ func (m *Model) FolderStatistics() map[string]stats.FolderStatistics {
 	return res
 }
 
-// Returns the completion status, in percent, for the given device and folder.
+// Completion returns the completion status, in percent, for the given device
+// and folder.
 func (m *Model) Completion(device protocol.DeviceID, folder string) float64 {
 	var tot int64
 
@@ -375,9 +377,9 @@ func (m *Model) NeedSize(folder string) (nfiles int, bytes int64) {
 	return
 }
 
-// NeedFiles returns paginated list of currently needed files in progress, queued,
+// NeedFolderFiles returns paginated list of currently needed files in
-// and to be queued on next puller iteration, as well as the total number of
+// progress, queued, and to be queued on next puller iteration, as well as the
-// files currently needed.
+// total number of files currently needed.
 func (m *Model) NeedFolderFiles(folder string, page, perpage int) ([]db.FileInfoTruncated, []db.FileInfoTruncated, []db.FileInfoTruncated, int) {
 	m.fmut.RLock()
 	defer m.fmut.RUnlock()
@@ -1535,7 +1537,7 @@ func (m *Model) Availability(folder, file string) []protocol.DeviceID {
 	return availableDevices
 }
 
-// Bump the given files priority in the job queue
+// BringToFront bumps the given files priority in the job queue.
 func (m *Model) BringToFront(folder, file string) {
 	m.pmut.RLock()
 	defer m.pmut.RUnlock()
@@ -1546,9 +1548,8 @@ func (m *Model) BringToFront(folder, file string) {
 	}
 }
 
-// Returns current folder error, or nil if the folder is healthy.
+// CheckFolderHealth checks the folder for common errors and returns the
-// Updates the Invalid field on the folder configuration struct, and emits a
+// current folder error, or nil if the folder is healthy.
-// ConfigSaved event which causes a GUI refresh.
 func (m *Model) CheckFolderHealth(id string) error {
 	folder, ok := m.cfg.Folders()[id]
 	if !ok {

internal/model/model_test.go

@@ -760,7 +760,7 @@ func TestGlobalDirectoryTree(t *testing.T) {
 	m.AddFolder(defaultFolderConfig)
 
 	b := func(isfile bool, path ...string) protocol.FileInfo {
-		var flags uint32 = protocol.FlagDirectory
+		flags := uint32(protocol.FlagDirectory)
 		blocks := []protocol.BlockInfo{}
 		if isfile {
 			flags = 0
@@ -1009,7 +1009,7 @@ func TestGlobalDirectorySelfFixing(t *testing.T) {
 	m.AddFolder(defaultFolderConfig)
 
 	b := func(isfile bool, path ...string) protocol.FileInfo {
-		var flags uint32 = protocol.FlagDirectory
+		flags := uint32(protocol.FlagDirectory)
 		blocks := []protocol.BlockInfo{}
 		if isfile {
 			flags = 0

internal/model/progressemitter.go

@@ -27,8 +27,8 @@ type ProgressEmitter struct {
 	stop chan struct{}
 }
 
-// Creates a new progress emitter which emits DownloadProgress events every
+// NewProgressEmitter creates a new progress emitter which emits
-// interval.
+// DownloadProgress events every interval.
 func NewProgressEmitter(cfg *config.Wrapper) *ProgressEmitter {
 	t := &ProgressEmitter{
 		stop:     make(chan struct{}),
@@ -42,8 +42,8 @@ func NewProgressEmitter(cfg *config.Wrapper) *ProgressEmitter {
 	return t
 }
 
-// Starts progress emitter which starts emitting DownloadProgress events as
+// Serve starts the progress emitter which starts emitting DownloadProgress
-// the progress happens.
+// events as the progress happens.
 func (t *ProgressEmitter) Serve() {
 	for {
 		select {
@@ -81,7 +81,8 @@ func (t *ProgressEmitter) Serve() {
 	}
 }
 
-// Interface method to handle configuration changes
+// Changed implements the config.Handler Interface to handle configuration
+// changes
 func (t *ProgressEmitter) Changed(cfg config.Configuration) error {
 	t.mut.Lock()
 	defer t.mut.Unlock()
@@ -93,7 +94,7 @@ func (t *ProgressEmitter) Changed(cfg config.Configuration) error {
 	return nil
 }
 
-// Stops the emitter.
+// Stop stops the emitter.
 func (t *ProgressEmitter) Stop() {
 	t.stop <- struct{}{}
 }
@@ -122,7 +123,7 @@ func (t *ProgressEmitter) Deregister(s *sharedPullerState) {
 	delete(t.registry, filepath.Join(s.folder, s.file.Name))
 }
 
-// Returns number of bytes completed in the given folder.
+// BytesCompleted returns the number of bytes completed in the given folder.
 func (t *ProgressEmitter) BytesCompleted(folder string) (bytes int64) {
 	t.mut.Lock()
 	defer t.mut.Unlock()

internal/osutil/osutil.go

@@ -23,7 +23,7 @@ var ErrNoHome = errors.New("No home directory found - set $HOME (or the platform
 
 // Try to keep this entire operation atomic-like. We shouldn't be doing this
 // often enough that there is any contention on this lock.
-var renameLock sync.Mutex = sync.NewMutex()
+var renameLock = sync.NewMutex()
 
 // TryRename renames a file, leaving source file intact in case of failure.
 // Tries hard to succeed on various systems by temporarily tweaking directory
@@ -89,7 +89,8 @@ func InWritableDir(fn func(string) error, path string) error {
 	return fn(path)
 }
 
-// On Windows, removes the read-only attribute from the target prior deletion.
+// Remove removes the given path. On Windows, removes the read-only attribute
+// from the target prior to deletion.
 func Remove(path string) error {
 	if runtime.GOOS == "windows" {
 		info, err := os.Stat(path)

internal/scanner/blocks.go

@@ -59,7 +59,7 @@ func Blocks(r io.Reader, blocksize int, sizehint int64) ([]protocol.BlockInfo, e
 	return blocks, nil
 }
 
-// Set the Offset field on each block
+// PopulateOffsets sets the Offset field on each block
 func PopulateOffsets(blocks []protocol.BlockInfo) {
 	var offset int64
 	for i := range blocks {
@@ -139,7 +139,7 @@ func VerifyBuffer(buf []byte, block protocol.BlockInfo) ([]byte, error) {
 	return hash, nil
 }
 
-// BlockEqual returns whether two slices of blocks are exactly the same hash
+// BlocksEqual returns whether two slices of blocks are exactly the same hash
 // and index pair wise.
 func BlocksEqual(src, tgt []protocol.BlockInfo) bool {
 	if len(tgt) != len(src) {

internal/scanner/walk.go

@@ -379,15 +379,15 @@ func PermsEqual(a, b uint32) bool {
 	}
 }
 
-// If the target is missing, Unix never knows what type of symlink it is
-// and Windows always knows even if there is no target.
-// Which means that without this special check a Unix node would be fighting
-// with a Windows node about whether or not the target is known.
-// Basically, if you don't know and someone else knows, just accept it.
-// The fact that you don't know means you are on Unix, and on Unix you don't
-// really care what the target type is. The moment you do know, and if something
-// doesn't match, that will propogate throught the cluster.
 func SymlinkTypeEqual(disk, index uint32) bool {
+	// If the target is missing, Unix never knows what type of symlink it is
+	// and Windows always knows even if there is no target. Which means that
+	// without this special check a Unix node would be fighting with a Windows
+	// node about whether or not the target is known. Basically, if you don't
+	// know and someone else knows, just accept it. The fact that you don't
+	// know means you are on Unix, and on Unix you don't really care what the
+	// target type is. The moment you do know, and if something doesn't match,
+	// that will propogate throught the cluster.
 	if disk&protocol.FlagSymlinkMissingTarget != 0 && index&protocol.FlagSymlinkMissingTarget == 0 {
 		return true
 	}

internal/sync/sync_test.go

@@ -58,7 +58,7 @@ func TestMutex(t *testing.T) {
 	threshold = logThreshold
 
 	msgmut := sync.Mutex{}
-	messages := make([]string, 0)
+	var messages []string
 
 	l.AddHandler(logger.LevelDebug, func(_ logger.LogLevel, message string) {
 		msgmut.Lock()
@@ -91,7 +91,7 @@ func TestRWMutex(t *testing.T) {
 	threshold = logThreshold
 
 	msgmut := sync.Mutex{}
-	messages := make([]string, 0)
+	var messages []string
 
 	l.AddHandler(logger.LevelDebug, func(_ logger.LogLevel, message string) {
 		msgmut.Lock()
@@ -149,7 +149,7 @@ func TestWaitGroup(t *testing.T) {
 	threshold = logThreshold
 
 	msgmut := sync.Mutex{}
-	messages := make([]string, 0)
+	var messages []string
 
 	l.AddHandler(logger.LevelDebug, func(_ logger.LogLevel, message string) {
 		msgmut.Lock()

internal/upgrade/upgrade_common.go

@@ -40,7 +40,6 @@ func init() {
 	upgradeUnlocked <- true
 }
 
-// A wrapper around actual implementations
 func To(rel Release) error {
 	select {
 	case <-upgradeUnlocked:
@@ -60,7 +59,6 @@ func To(rel Release) error {
 	}
 }
 
-// A wrapper around actual implementations
 func ToURL(url string) error {
 	select {
 	case <-upgradeUnlocked:
@@ -90,7 +88,7 @@ const (
 	MajorNewer          = 2  // Newer by a major version (x in x.y.z or 0.x.y).
 )
 
-// Returns a relation describing how a compares to b.
+// CompareVersions returns a relation describing how a compares to b.
 func CompareVersions(a, b string) Relation {
 	arel, apre := versionParts(a)
 	brel, bpre := versionParts(b)

internal/upgrade/upgrade_supported.go

@@ -27,7 +27,8 @@ import (
 	"strings"
 )
 
-// Returns the latest releases, including prereleases or not depending on the argument
+// LatestGithubReleases returns the latest releases, including prereleases or
+// not depending on the argument
 func LatestGithubReleases(version string) ([]Release, error) {
 	resp, err := http.Get("https://api.github.com/repos/syncthing/syncthing/releases?per_page=30")
 	if err != nil {

internal/upnp/upnp.go

@@ -27,7 +27,7 @@ import (
 	"github.com/syncthing/syncthing/internal/sync"
 )
 
-// A container for relevant properties of a UPnP InternetGatewayDevice.
+// An IGD is a UPnP InternetGatewayDevice.
 type IGD struct {
 	uuid           string
 	friendlyName   string
@@ -36,27 +36,25 @@ type IGD struct {
 	localIPAddress string
 }
 
-// The InternetGatewayDevice's UUID.
 func (n *IGD) UUID() string {
 	return n.uuid
 }
 
-// The InternetGatewayDevice's friendly name.
 func (n *IGD) FriendlyName() string {
 	return n.friendlyName
 }
 
-// The InternetGatewayDevice's friendly identifier (friendly name + IP address).
+// FriendlyIdentifier returns a friendly identifier (friendly name + IP
+// address) for the IGD.
 func (n *IGD) FriendlyIdentifier() string {
 	return "'" + n.FriendlyName() + "' (" + strings.Split(n.URL().Host, ":")[0] + ")"
 }
 
-// The URL of the InternetGatewayDevice's root device description.
 func (n *IGD) URL() *url.URL {
 	return n.url
 }
 
-// A container for relevant properties of a UPnP service of an IGD.
+// An IGDService is a specific service provided by an IGD.
 type IGDService struct {
 	serviceID  string
 	serviceURL string
@@ -236,7 +234,7 @@ func parseResponse(deviceType string, resp []byte) (IGD, error) {
 
 	deviceDescriptionLocation := response.Header.Get("Location")
 	if deviceDescriptionLocation == "" {
-		return IGD{}, errors.New("invalid IGD response: no location specified.")
+		return IGD{}, errors.New("invalid IGD response: no location specified")
 	}
 
 	deviceDescriptionURL, err := url.Parse(deviceDescriptionLocation)
@@ -247,7 +245,7 @@ func parseResponse(deviceType string, resp []byte) (IGD, error) {
 
 	deviceUSN := response.Header.Get("USN")
 	if deviceUSN == "" {
-		return IGD{}, errors.New("invalid IGD response: USN not specified.")
+		return IGD{}, errors.New("invalid IGD response: USN not specified")
 	}
 
 	deviceUUID := strings.TrimLeft(strings.Split(deviceUSN, "::")[0], "uuid:")
@@ -353,9 +351,8 @@ func getServiceDescriptions(rootURL string, device upnpDevice) ([]IGDService, er
 
 	if len(result) < 1 {
 		return result, errors.New("[" + rootURL + "] Malformed device description: no compatible service descriptions found.")
-	} else {
-		return result, nil
 	}
+	return result, nil
 }
 
 func getIGDServices(rootURL string, device upnpDevice, wanDeviceURN string, wanConnectionURN string, serviceURNs []string) []IGDService {
@@ -480,9 +477,11 @@ func soapRequest(url, service, function, message string) ([]byte, error) {
 	return resp, nil
 }
 
-// Add a port mapping to all relevant services on the specified InternetGatewayDevice.
+// AddPortMapping adds a port mapping to all relevant services on the
-// Port mapping will fail and return an error if action is fails for _any_ of the relevant services.
+// specified InternetGatewayDevice. Port mapping will fail and return an error
-// For this reason, it is generally better to configure port mapping for each individual service instead.
+// if action is fails for _any_ of the relevant services. For this reason, it
+// is generally better to configure port mapping for each individual service
+// instead.
 func (n *IGD) AddPortMapping(protocol Protocol, externalPort, internalPort int, description string, timeout int) error {
 	for _, service := range n.services {
 		err := service.AddPortMapping(n.localIPAddress, protocol, externalPort, internalPort, description, timeout)
@@ -493,9 +492,11 @@ func (n *IGD) AddPortMapping(protocol Protocol, externalPort, internalPort int,
 	return nil
 }
 
-// Delete a port mapping from all relevant services on the specified InternetGatewayDevice.
+// DeletePortMapping deletes a port mapping from all relevant services on the
-// Port mapping will fail and return an error if action is fails for _any_ of the relevant services.
+// specified InternetGatewayDevice. Port mapping will fail and return an error
-// For this reason, it is generally better to configure port mapping for each individual service instead.
+// if action is fails for _any_ of the relevant services. For this reason, it
+// is generally better to configure port mapping for each individual service
+// instead.
 func (n *IGD) DeletePortMapping(protocol Protocol, externalPort int) error {
 	for _, service := range n.services {
 		err := service.DeletePortMapping(protocol, externalPort)
@@ -520,7 +521,7 @@ type getExternalIPAddressResponse struct {
 	NewExternalIPAddress string `xml:"NewExternalIPAddress"`
 }
 
-// Add a port mapping to the specified IGD service.
+// AddPortMapping adds a port mapping to the specified IGD service.
 func (s *IGDService) AddPortMapping(localIPAddress string, protocol Protocol, externalPort, internalPort int, description string, timeout int) error {
 	tpl := `<u:AddPortMapping xmlns:u="%s">
 	<NewRemoteHost></NewRemoteHost>
@@ -542,7 +543,7 @@ func (s *IGDService) AddPortMapping(localIPAddress string, protocol Protocol, ex
 	return nil
 }
 
-// Delete a port mapping from the specified IGD service.
+// DeletePortMapping deletes a port mapping from the specified IGD service.
 func (s *IGDService) DeletePortMapping(protocol Protocol, externalPort int) error {
 	tpl := `<u:DeletePortMapping xmlns:u="%s">
 	<NewRemoteHost></NewRemoteHost>
@@ -560,8 +561,9 @@ func (s *IGDService) DeletePortMapping(protocol Protocol, externalPort int) erro
 	return nil
 }
 
-// Query the IGD service for its external IP address.
+// GetExternalIPAddress queries the IGD service for its external IP address.
-// Returns nil if the external IP address is invalid or undefined, along with any relevant errors
+// Returns nil if the external IP address is invalid or undefined, along with
+// any relevant errors
 func (s *IGDService) GetExternalIPAddress() (net.IP, error) {
 	tpl := `<u:GetExternalIPAddress xmlns:u="%s" />`
 

internal/versioner/external.go

@@ -21,13 +21,11 @@ func init() {
 	Factories["external"] = NewExternal
 }
 
-// The type holds our configuration
 type External struct {
 	command    string
 	folderPath string
 }
 
-// The constructor function takes a map of parameters and creates the type.
 func NewExternal(folderID, folderPath string, params map[string]string) Versioner {
 	command := params["command"]
 
@@ -42,8 +40,8 @@ func NewExternal(folderID, folderPath string, params map[string]string) Versione
 	return s
 }
 
-// Move away the named file to a version archive. If this function returns
+// Archive moves the named file away to a version archive. If this function
-// nil, the named file does not exist any more (has been archived).
+// returns nil, the named file does not exist any more (has been archived).
 func (v External) Archive(filePath string) error {
 	_, err := osutil.Lstat(filePath)
 	if os.IsNotExist(err) {

internal/versioner/simple.go

@@ -19,13 +19,11 @@ func init() {
 	Factories["simple"] = NewSimple
 }
 
-// The type holds our configuration
 type Simple struct {
 	keep       int
 	folderPath string
 }
 
-// The constructor function takes a map of parameters and creates the type.
 func NewSimple(folderID, folderPath string, params map[string]string) Versioner {
 	keep, err := strconv.Atoi(params["keep"])
 	if err != nil {
@@ -43,8 +41,8 @@ func NewSimple(folderID, folderPath string, params map[string]string) Versioner
 	return s
 }
 
-// Move away the named file to a version archive. If this function returns
+// Archive moves the named file away to a version archive. If this function
-// nil, the named file does not exist any more (has been archived).
+// returns nil, the named file does not exist any more (has been archived).
 func (v Simple) Archive(filePath string) error {
 	fileInfo, err := osutil.Lstat(filePath)
 	if os.IsNotExist(err) {

internal/versioner/staggered.go

@@ -27,7 +27,6 @@ type Interval struct {
 	end  int64
 }
 
-// The type holds our configuration
 type Staggered struct {
 	versionsPath  string
 	cleanInterval int64
@@ -62,7 +61,6 @@ func (v Staggered) renameOld() {
 	}
 }
 
-// The constructor function takes a map of parameters and creates the type.
 func NewStaggered(folderID, folderPath string, params map[string]string) Versioner {
 	maxAge, err := strconv.ParseInt(params["maxAge"], 10, 0)
 	if err != nil {
@@ -271,8 +269,8 @@ func (v Staggered) expire(versions []string) {
 	}
 }
 
-// Move away the named file to a version archive. If this function returns
+// Archive moves the named file away to a version archive. If this function
-// nil, the named file does not exist any more (has been archived).
+// returns nil, the named file does not exist any more (has been archived).
 func (v Staggered) Archive(filePath string) error {
 	if debug {
 		l.Debugln("Waiting for lock on ", v.versionsPath)