[gopls-release-branch.0.5] all: merge master into gopls-release-branch.0.5

8a9a893 internal/lsp/cache: don't set tempModFile is view's modURI is empty
0f1c539 internal/lsp: show orphaned file warnings as diagnostics
7b6ac5b internal/lsp/cache: slightly more useful comment
24570c0 internal/lsp: handle initial workspace load failure per module
04afef3 internal/lsp: don't search for workspace modules by default
7bb30d1 internal/lsp: fix go generate command for subdirs
06f3a46 gopls/internal/regtest: add benchmarks for completions
463111b internal/lsp: add a command to generate the gopls.mod file
03f0cc1 internal/lsp: interpolate variables in the sandbox
ba800b1 internal/lsp: ignore errors when finding workspace modules
f5e916c internal/lsp/source/completion: move search path to candidate
a1f7ab6 internal/lsp/source/completion: move deep search to end of completion
82fe25c gopls/doc: improve enum value documentation
aeb8e36 internal/lsp: remove multi-module support in tests
fa01252 all: replace 'the the ' with 'the '
a20a5e7 internal/lsp: rewrite createView to populate fields independently
4aa5c68 internal/lsp: remove multi-module test cases
0f52b63 internal/lsp/tests: use NeedsGo1Point
ccabf82 internal/lsp/cache: populate URI on files with errors
d647fc2 cmd/benchcmp: mention deprecation in docs
78fed78 internal/lsp: handle modifications to the workspace module
e94ab72 internal/lsp/source/completion: improve import suggestion labels
d56e4e4 internal/lsp/cache: set GO111MODULE=auto in 1.16
3791637 all: fix tests in preparation for GO111MODULE=on by default
d148ae1 internal/lsp/source/completion: convert deep completion to bfs
0511c4c internal/imports: use ProcessEnv to filter files
75ebdcb gopls/internal/regtest: add expected ranges in completion tests
60aba8a internal/lsp: always show errors from running commands
63098cc internal/imports: don't set Context.WorkingDir, which was renamed
655488c internal/lsp: fix concurrency issues in view initialization
c537a34 internal/lsp: pass options by reference instead of by value
c9a70fc internal/lsp/cache: fix release tag parsing
f128e62 internal/memoize: show key type in panics
587cf23 internal/lsp/mod: handle nil pointers in code lenses
56d9a0c cover: fix sorting of profile segment boundaries
c8d9e05 gopls/internal/regtest: simplify expectation return values
6ec2cde internal/lsp/source: add some additional symbol downranking
e20053b gopls/internal/regtest: remove ExpectNow
797bd0f gopls/internal/regtest: simplify regtest EditorConfig
cbbbe62 gopls/internal/regtest: add an InitialWorkspaceLoad expectation
bf5c620 gopls/internal/regtest: move expectations to their own file
f4cefd1 internal/lsp: enable multi-module workspace mode by default in tests
2db8f0f internal/lsp/cache: fix -mod=mod for workspace module setups
6498648 internal/lsp: check file kind before showing warning
3c2036f go/packages: support overlays for replaced modules
2b477fa tools, gopls: update Go versions in go.mod files to 1.12
8f9ed77 go/packages: add roots for overlaid packages for all query types
17fc728 internal/lsp: handle staticcheck in code actions
2350fe8 internal/lsp/source: fix panic in addFieldItems
97363e2 internal/lsp: handle staticcheck in didChangeConfiguration
6422fca report cycle when visiting a grey analyzer
b43031a internal/lsp/cache: fix for default -mod=readonly
7ad463c gopls/internal/regtest: add a test for new symbols in test variants
d49a6cb internal/lsp/protocol: add support for upcoming LSP 3.16
d179df3 internal/lsp/cache: automatically construct the workspace module
571a207 internal/lsp/source: fix duplicates in workspaceSymbols
53e29e9 internal/lsp: show a warning message when opening an "orphaned" file
d6107fe gopls/internal/regtest: skip failing test for go1.13 and below
b484961 x/gools/gopls: document settings for working on source distribution
44a2922 internal/lsp, go/packages: fix Go 1.15-related overlay bug
6a8222e gopls/doc: generate settings JSON, docs
201d438 internal/lsp: add more tests for package completion

Change-Id: I9e52247c70e94cbedf2ebc47a6e9b2f6dabdb8c6

Author: stamblerre

cmd/benchcmp/doc.go

@@ -4,6 +4,8 @@
 
 /*
 
+Deprecated: benchcmp is deprecated in favor of benchstat: golang.org/x/perf/cmd/benchstat
+
 The benchcmp command displays performance changes between benchmarks.
 
 Benchcmp parses the output of two 'go test' benchmark runs,

cmd/callgraph/main.go

@@ -63,7 +63,7 @@ func init() {
 	flag.Var((*buildutil.TagsFlag)(&build.Default.BuildTags), "tags", buildutil.TagsFlagDoc)
 }
 
-const Usage = `callgraph: display the the call graph of a Go program.
+const Usage = `callgraph: display the call graph of a Go program.
 
 Usage:
 

cmd/fiximports/main.go

@@ -266,7 +266,7 @@ func fiximports(packages ...string) bool {
 
 					// TODO(adonovan): should we make an HTTP request to
 					// see if there's an HTTP redirect, a "go-import" meta tag,
-					// or an import comment in the the latest revision?
+					// or an import comment in the latest revision?
 					// It would duplicate a lot of logic from "go get".
 				}
 				break

cmd/guru/referrers.go

@@ -95,7 +95,7 @@ func referrers(q *Query) error {
 		if pkglevel {
 			return globalReferrersPkgLevel(q, obj, fset)
 		}
-		// We'll use the the object's position to identify it in the larger program.
+		// We'll use the object's position to identify it in the larger program.
 		objposn := fset.Position(obj.Pos())
 		defpkg := obj.Pkg().Path() // defining package
 		return globalReferrers(q, qpos.info.Pkg.Path(), defpkg, objposn)

cover/profile.go

@@ -194,6 +194,7 @@ type Boundary struct {
 	Start  bool    // Is this the start of a block?
 	Count  int     // Event count from the cover profile.
 	Norm   float64 // Count normalized to [0..1].
+	Index  int     // Order in input file.
 }
 
 // Boundaries returns a Profile as a set of Boundary objects within the provided src.
@@ -209,8 +210,10 @@ func (p *Profile) Boundaries(src []byte) (boundaries []Boundary) {
 	divisor := math.Log(float64(max))
 
 	// boundary returns a Boundary, populating the Norm field with a normalized Count.
+	index := 0
 	boundary := func(offset int, start bool, count int) Boundary {
-		b := Boundary{Offset: offset, Start: start, Count: count}
+		b := Boundary{Offset: offset, Start: start, Count: count, Index: index}
+		index++
 		if !start || count == 0 {
 			return b
 		}
@@ -250,7 +253,9 @@ func (b boundariesByPos) Len() int      { return len(b) }
 func (b boundariesByPos) Swap(i, j int) { b[i], b[j] = b[j], b[i] }
 func (b boundariesByPos) Less(i, j int) bool {
 	if b[i].Offset == b[j].Offset {
-		return !b[i].Start && b[j].Start
+		// Boundaries at the same offset should be ordered according to
+		// their original position.
+		return b[i].Index < b[j].Index
 	}
 	return b[i].Offset < b[j].Offset
 }

go.mod

@@ -1,6 +1,6 @@
 module golang.org/x/tools
 
-go 1.11
+go 1.12
 
 require (
 	github.com/yuin/goldmark v1.2.1

go/analysis/validate.go

@@ -3,6 +3,7 @@ package analysis
 import (
 	"fmt"
 	"reflect"
+	"strings"
 	"unicode"
 )
 
@@ -58,14 +59,28 @@ func Validate(analyzers []*Analyzer) error {
 			}
 
 			// recursion
-			for i, req := range a.Requires {
+			for _, req := range a.Requires {
 				if err := visit(req); err != nil {
-					return fmt.Errorf("%s.Requires[%d]: %v", a.Name, i, err)
+					return err
 				}
 			}
 			color[a] = black
 		}
 
+		if color[a] == grey {
+			stack := []*Analyzer{a}
+			inCycle := map[string]bool{}
+			for len(stack) > 0 {
+				current := stack[len(stack)-1]
+				stack = stack[:len(stack)-1]
+				if color[current] == grey && !inCycle[current.Name] {
+					inCycle[current.Name] = true
+					stack = append(stack, current.Requires...)
+				}
+			}
+			return &CycleInRequiresGraphError{AnalyzerNames: inCycle}
+		}
+
 		return nil
 	}
 	for _, a := range analyzers {
@@ -95,3 +110,17 @@ func validIdent(name string) bool {
 	}
 	return name != ""
 }
+
+type CycleInRequiresGraphError struct {
+	AnalyzerNames map[string]bool
+}
+
+func (e *CycleInRequiresGraphError) Error() string {
+	var b strings.Builder
+	b.WriteString("cycle detected involving the following analyzers:")
+	for n := range e.AnalyzerNames {
+		b.WriteByte(' ')
+		b.WriteString(n)
+	}
+	return b.String()
+}

go/analysis/validate_test.go

@@ -0,0 +1,118 @@
+// Copyright 2020 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package analysis
+
+import (
+	"strings"
+	"testing"
+)
+
+func TestValidate(t *testing.T) {
+	var (
+		dependsOnSelf = &Analyzer{
+			Name: "dependsOnSelf",
+			Doc:  "this analyzer depends on itself",
+		}
+		inCycleA = &Analyzer{
+			Name: "inCycleA",
+			Doc:  "this analyzer depends on inCycleB",
+		}
+		inCycleB = &Analyzer{
+			Name: "inCycleB",
+			Doc:  "this analyzer depends on inCycleA and notInCycleA",
+		}
+		pointsToCycle = &Analyzer{
+			Name: "pointsToCycle",
+			Doc:  "this analyzer depends on inCycleA",
+		}
+		notInCycleA = &Analyzer{
+			Name: "notInCycleA",
+			Doc:  "this analyzer depends on notInCycleB and notInCycleC",
+		}
+		notInCycleB = &Analyzer{
+			Name: "notInCycleB",
+			Doc:  "this analyzer depends on notInCycleC",
+		}
+		notInCycleC = &Analyzer{
+			Name: "notInCycleC",
+			Doc:  "this analyzer has no dependencies",
+		}
+	)
+
+	dependsOnSelf.Requires = append(dependsOnSelf.Requires, dependsOnSelf)
+	inCycleA.Requires = append(inCycleA.Requires, inCycleB)
+	inCycleB.Requires = append(inCycleB.Requires, inCycleA, notInCycleA)
+	pointsToCycle.Requires = append(pointsToCycle.Requires, inCycleA)
+	notInCycleA.Requires = append(notInCycleA.Requires, notInCycleB, notInCycleC)
+	notInCycleB.Requires = append(notInCycleB.Requires, notInCycleC)
+	notInCycleC.Requires = []*Analyzer{}
+
+	cases := []struct {
+		analyzers        []*Analyzer
+		wantErr          bool
+		analyzersInCycle map[string]bool
+	}{
+		{
+			[]*Analyzer{dependsOnSelf},
+			true,
+			map[string]bool{"dependsOnSelf": true},
+		},
+		{
+			[]*Analyzer{inCycleA, inCycleB},
+			true,
+			map[string]bool{"inCycleA": true, "inCycleB": true},
+		},
+		{
+			[]*Analyzer{pointsToCycle},
+			true,
+			map[string]bool{"inCycleA": true, "inCycleB": true},
+		},
+		{
+			[]*Analyzer{notInCycleA},
+			false,
+			map[string]bool{},
+		},
+	}
+
+	for _, c := range cases {
+		got := Validate(c.analyzers)
+
+		if !c.wantErr {
+			if got == nil {
+				continue
+			}
+			t.Errorf("got unexpected error while validating analyzers %v: %v", c.analyzers, got)
+		}
+
+		if got == nil {
+			t.Errorf("expected error while validating analyzers %v, but got nil", c.analyzers)
+		}
+
+		err, ok := got.(*CycleInRequiresGraphError)
+		if !ok {
+			t.Errorf("want CycleInRequiresGraphError, got %T", err)
+		}
+
+		for a := range c.analyzersInCycle {
+			if !err.AnalyzerNames[a] {
+				t.Errorf("analyzer %s should be in cycle", a)
+			}
+		}
+		for a := range err.AnalyzerNames {
+			if !c.analyzersInCycle[a] {
+				t.Errorf("analyzer %s should not be in cycle", a)
+			}
+		}
+	}
+}
+
+func TestCycleInRequiresGraphErrorMessage(t *testing.T) {
+	err := CycleInRequiresGraphError{}
+	errMsg := err.Error()
+	wantSubstring := "cycle detected"
+	if !strings.Contains(errMsg, wantSubstring) {
+		t.Errorf("error string %s does not contain expected substring %q", errMsg, wantSubstring)
+	}
+}

go/internal/gcimporter/gcimporter.go

@@ -491,7 +491,7 @@ func (p *parser) parseMapType(parent *types.Package) types.Type {
 //
 // For unqualified and anonymous names, the returned package is the parent
 // package unless parent == nil, in which case the returned package is the
-// package being imported. (The parent package is not nil if the the name
+// package being imported. (The parent package is not nil if the name
 // is an unqualified struct field or interface method name belonging to a
 // type declared in another package.)
 //

go/packages/golist.go

@@ -246,6 +246,21 @@ extractQueries:
 			}
 		}
 	}
+	// Add root for any package that matches a pattern. This applies only to
+	// packages that are modified by overlays, since they are not added as
+	// roots automatically.
+	for _, pattern := range restPatterns {
+		match := matchPattern(pattern)
+		for _, pkgID := range modifiedPkgs {
+			pkg, ok := response.seenPackages[pkgID]
+			if !ok {
+				continue
+			}
+			if match(pkg.PkgPath) {
+				response.addRoot(pkg.ID)
+			}
+		}
+	}
 
 	sizeswg.Wait()
 	if sizeserr != nil {
@@ -753,7 +768,8 @@ func (state *golistState) getGoVersion() (string, error) {
 	return state.goVersion, state.goVersionError
 }
 
-// getPkgPath finds the package path of a directory if it's relative to a root directory.
+// getPkgPath finds the package path of a directory if it's relative to a root
+// directory.
 func (state *golistState) getPkgPath(dir string) (string, bool, error) {
 	absDir, err := filepath.Abs(dir)
 	if err != nil {