internal/lsp/cache: fix leaking wg.Done() and <-ioLimit

There were several returns in this function that could leak both the
wg.Done() and <-ioLimit. This refactors them into defers close to their
counterparts.

Additionally I refactored the function to favor early returns over
nested if statements.

Fixes golang/go#32368

Change-Id: I5357d11ee526c1cb7a6bd1a0f652c61d574c10ab
GitHub-Last-Rev: bfa160b9fd75c4464d2e0a2820c04ddae01de21f
GitHub-Pull-Request: golang/tools#107
Reviewed-on: https://go-review.googlesource.com/c/tools/+/179878
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Run-TryBot: Ian Cottrell <iancottrell@google.com>

internal/lsp/cache/parse.go

@@ -61,20 +61,29 @@ func (imp *importer) parseFiles(filenames []string) ([]*ast.File, []error) {
 		wg.Add(1)
 		go func(i int, filename string) {
 			ioLimit <- true // wait
+			defer func() {
+				<-ioLimit // signal done
+				wg.Done()
+			}()
 
-			if gof.ast != nil {
+			if gof.ast != nil { // already have an ast
 				parsed[i], errors[i] = gof.ast, nil
-			} else {
-				// We don't have a cached AST for this file.
-				gof.read(imp.ctx)
-				if gof.fc.Error != nil {
 				return
 			}
+
+			// No cached AST for this file, so try parsing it.
+			gof.read(imp.ctx)
+			if gof.fc.Error != nil { // file content error, so abort
+				return
+			}
+
 			src := gof.fc.Data
-				if src == nil {
+			if src == nil { // no source
 				parsed[i], errors[i] = nil, fmt.Errorf("No source for %v", filename)
-				} else {
+				return
-					// ParseFile may return both an AST and an error.
+			}
+
+			// ParseFile may return a partial AST AND an error.
 			parsed[i], errors[i] = parseFile(imp.fset, filename, src)
 
 			// Fix any badly parsed parts of the AST.
@@ -82,11 +91,6 @@ func (imp *importer) parseFiles(filenames []string) ([]*ast.File, []error) {
 				tok := imp.fset.File(file.Pos())
 				imp.view.fix(imp.ctx, parsed[i], tok, src)
 			}
-				}
-			}
-
-			<-ioLimit // signal
-			wg.Done()
 		}(i, filename)
 	}
 	wg.Wait()