dev: sync checker with x/[email protected] (golangci#5201)

Author: ldez

.golangci.next.reference.yml

@@ -1840,6 +1840,8 @@ linters-settings:
       - unusedresult
       # Checks for unused writes.
       - unusedwrite
+      # Checks for misuses of sync.WaitGroup.
+      - waitgroup
 
     # Enable all analyzers.
     # Default: false

.golangci.yml

@@ -222,6 +222,9 @@ issues:
   exclude-dirs:
     - test/testdata_etc # test files
     - internal/go # extracted from Go code
+    - internal/x # extracted from x/tools code
+  exclude-files:
+    - pkg/goanalysis/runner_checker.go # extracted from x/tools code
 
 run:
   timeout: 5m

internal/x/LICENSE

@@ -0,0 +1,27 @@
+Copyright 2009 The Go Authors.
+
+Redistribution and use in source and binary forms, with or without
+modification, are permitted provided that the following conditions are
+met:
+
+   * Redistributions of source code must retain the above copyright
+notice, this list of conditions and the following disclaimer.
+   * Redistributions in binary form must reproduce the above
+copyright notice, this list of conditions and the following disclaimer
+in the documentation and/or other materials provided with the
+distribution.
+   * Neither the name of Google LLC nor the names of its
+contributors may be used to endorse or promote products derived from
+this software without specific prior written permission.
+
+THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

internal/x/tools/analysisflags/readme.md

@@ -0,0 +1,8 @@
+# analysisflags
+
+Extracted from `/go/analysis/internal/analysisflags` (related to `checker`).
+This is just a copy of the code without any changes.
+
+## History
+
+- sync with https://github.com/golang/tools/blob/v0.28.0

internal/x/tools/analysisflags/url.go

@@ -0,0 +1,33 @@
+// Copyright 2023 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 analysisflags
+
+import (
+	"fmt"
+	"net/url"
+
+	"golang.org/x/tools/go/analysis"
+)
+
+// ResolveURL resolves the URL field for a Diagnostic from an Analyzer
+// and returns the URL. See Diagnostic.URL for details.
+func ResolveURL(a *analysis.Analyzer, d analysis.Diagnostic) (string, error) {
+	if d.URL == "" && d.Category == "" && a.URL == "" {
+		return "", nil // do nothing
+	}
+	raw := d.URL
+	if d.URL == "" && d.Category != "" {
+		raw = "#" + d.Category
+	}
+	u, err := url.Parse(raw)
+	if err != nil {
+		return "", fmt.Errorf("invalid Diagnostic.URL %q: %s", raw, err)
+	}
+	base, err := url.Parse(a.URL)
+	if err != nil {
+		return "", fmt.Errorf("invalid Analyzer.URL %q: %s", a.URL, err)
+	}
+	return base.ResolveReference(u).String(), nil
+}

internal/x/tools/analysisinternal/analysis.go

@@ -0,0 +1,48 @@
+// 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 analysisinternal provides gopls' internal analyses with a
+// number of helper functions that operate on typed syntax trees.
+package analysisinternal
+
+import (
+	"fmt"
+	"os"
+
+	"golang.org/x/tools/go/analysis"
+)
+
+// MakeReadFile returns a simple implementation of the Pass.ReadFile function.
+func MakeReadFile(pass *analysis.Pass) func(filename string) ([]byte, error) {
+	return func(filename string) ([]byte, error) {
+		if err := CheckReadable(pass, filename); err != nil {
+			return nil, err
+		}
+		return os.ReadFile(filename)
+	}
+}
+
+// CheckReadable enforces the access policy defined by the ReadFile field of [analysis.Pass].
+func CheckReadable(pass *analysis.Pass, filename string) error {
+	if slicesContains(pass.OtherFiles, filename) ||
+		slicesContains(pass.IgnoredFiles, filename) {
+		return nil
+	}
+	for _, f := range pass.Files {
+		if pass.Fset.File(f.FileStart).Name() == filename {
+			return nil
+		}
+	}
+	return fmt.Errorf("Pass.ReadFile: %s is not among OtherFiles, IgnoredFiles, or names of Files", filename)
+}
+
+// TODO(adonovan): use go1.21 slices.Contains.
+func slicesContains[S ~[]E, E comparable](slice S, x E) bool {
+	for _, elem := range slice {
+		if elem == x {
+			return true
+		}
+	}
+	return false
+}

internal/x/tools/analysisinternal/readme.md

@@ -0,0 +1,8 @@
+# analysisinternal
+
+Extracted from `/internal/analysisinternal/` (related to `checker`).
+This is just a copy of the code without any changes.
+
+## History
+
+- sync with https://github.com/golang/tools/blob/v0.28.0

jsonschema/golangci.next.jsonschema.json

@@ -210,7 +210,8 @@
         "unreachable",
         "unsafeptr",
         "unusedresult",
-        "unusedwrite"
+        "unusedwrite",
+        "waitgroup"
       ]
     },
     "revive-rules": {

pkg/goanalysis/runner.go

@@ -121,9 +121,9 @@ func (r *runner) makeAction(a *analysis.Analyzer, pkg *packages.Package,
 	}
 
 	act = actAlloc.alloc()
-	act.a = a
-	act.pkg = pkg
-	act.r = r
+	act.Analyzer = a
+	act.Package = pkg
+	act.runner = r
 	act.isInitialPkg = initialPkgs[pkg]
 	act.needAnalyzeSource = initialPkgs[pkg]
 	act.analysisDoneCh = make(chan struct{})
@@ -132,11 +132,11 @@ func (r *runner) makeAction(a *analysis.Analyzer, pkg *packages.Package,
 	if len(a.FactTypes) > 0 {
 		depsCount += len(pkg.Imports)
 	}
-	act.deps = make([]*action, 0, depsCount)
+	act.Deps = make([]*action, 0, depsCount)
 
 	// Add a dependency on each required analyzers.
 	for _, req := range a.Requires {
-		act.deps = append(act.deps, r.makeAction(req, pkg, initialPkgs, actions, actAlloc))
+		act.Deps = append(act.Deps, r.makeAction(req, pkg, initialPkgs, actions, actAlloc))
 	}
 
 	r.buildActionFactDeps(act, a, pkg, initialPkgs, actions, actAlloc)
@@ -162,7 +162,7 @@ func (r *runner) buildActionFactDeps(act *action, a *analysis.Analyzer, pkg *pac
 	sort.Strings(paths) // for determinism
 	for _, path := range paths {
 		dep := r.makeAction(a, pkg.Imports[path], initialPkgs, actions, actAlloc)
-		act.deps = append(act.deps, dep)
+		act.Deps = append(act.Deps, dep)
 	}
 
 	// Need to register fact types for pkgcache proper gob encoding.
@@ -203,7 +203,7 @@ func (r *runner) prepareAnalysis(pkgs []*packages.Package,
 	for _, a := range analyzers {
 		for _, pkg := range pkgs {
 			root := r.makeAction(a, pkg, initialPkgs, actions, actAlloc)
-			root.isroot = true
+			root.IsRoot = true
 			roots = append(roots, root)
 		}
 	}
@@ -220,7 +220,7 @@ func (r *runner) analyze(pkgs []*packages.Package, analyzers []*analysis.Analyze
 
 	actionPerPkg := map[*packages.Package][]*action{}
 	for _, act := range actions {
-		actionPerPkg[act.pkg] = append(actionPerPkg[act.pkg], act)
+		actionPerPkg[act.Package] = append(actionPerPkg[act.Package], act)
 	}
 
 	// Fill Imports field.
@@ -250,7 +250,7 @@ func (r *runner) analyze(pkgs []*packages.Package, analyzers []*analysis.Analyze
 		}
 	}
 	for _, act := range actions {
-		dfs(act.pkg)
+		dfs(act.Package)
 	}
 
 	// Limit memory and IO usage.
@@ -282,7 +282,7 @@ func extractDiagnostics(roots []*action) (retDiags []Diagnostic, retErrors []err
 		for _, act := range actions {
 			if !extracted[act] {
 				extracted[act] = true
-				visitAll(act.deps)
+				visitAll(act.Deps)
 				extract(act)
 			}
 		}
@@ -299,31 +299,31 @@ func extractDiagnostics(roots []*action) (retDiags []Diagnostic, retErrors []err
 	seen := make(map[key]bool)
 
 	extract = func(act *action) {
-		if act.err != nil {
-			if pe, ok := act.err.(*errorutil.PanicError); ok {
+		if act.Err != nil {
+			if pe, ok := act.Err.(*errorutil.PanicError); ok {
 				panic(pe)
 			}
-			retErrors = append(retErrors, fmt.Errorf("%s: %w", act.a.Name, act.err))
+			retErrors = append(retErrors, fmt.Errorf("%s: %w", act.Analyzer.Name, act.Err))
 			return
 		}
 
-		if act.isroot {
-			for _, diag := range act.diagnostics {
+		if act.IsRoot {
+			for _, diag := range act.Diagnostics {
 				// We don't display a.Name/f.Category
 				// as most users don't care.
 
-				posn := act.pkg.Fset.Position(diag.Pos)
-				k := key{posn, act.a, diag.Message}
+				posn := act.Package.Fset.Position(diag.Pos)
+				k := key{posn, act.Analyzer, diag.Message}
 				if seen[k] {
 					continue // duplicate
 				}
 				seen[k] = true
 
 				retDiag := Diagnostic{
 					Diagnostic: diag,
-					Analyzer:   act.a,
+					Analyzer:   act.Analyzer,
 					Position:   posn,
-					Pkg:        act.pkg,
+					Pkg:        act.Package,
 				}
 				retDiags = append(retDiags, retDiag)
 			}

pkg/goanalysis/runner_action.go

@@ -29,8 +29,8 @@ func (actAlloc *actionAllocator) alloc() *action {
 }
 
 func (act *action) waitUntilDependingAnalyzersWorked() {
-	for _, dep := range act.deps {
-		if dep.pkg == act.pkg {
+	for _, dep := range act.Deps {
+		if dep.Package == act.Package {
 			<-dep.analysisDoneCh
 		}
 	}
@@ -39,26 +39,26 @@ func (act *action) waitUntilDependingAnalyzersWorked() {
 func (act *action) analyzeSafe() {
 	defer func() {
 		if p := recover(); p != nil {
-			if !act.isroot {
+			if !act.IsRoot {
 				// This line allows to display "hidden" panic with analyzers like buildssa.
 				// Some linters are dependent of sub-analyzers but when a sub-analyzer fails the linter is not aware of that,
 				// this results to another panic (ex: "interface conversion: interface {} is nil, not *buildssa.SSA").
-				act.r.log.Errorf("%s: panic during analysis: %v, %s", act.a.Name, p, string(debug.Stack()))
+				act.runner.log.Errorf("%s: panic during analysis: %v, %s", act.Analyzer.Name, p, string(debug.Stack()))
 			}
 
-			act.err = errorutil.NewPanicError(fmt.Sprintf("%s: package %q (isInitialPkg: %t, needAnalyzeSource: %t): %s",
-				act.a.Name, act.pkg.Name, act.isInitialPkg, act.needAnalyzeSource, p), debug.Stack())
+			act.Err = errorutil.NewPanicError(fmt.Sprintf("%s: package %q (isInitialPkg: %t, needAnalyzeSource: %t): %s",
+				act.Analyzer.Name, act.Package.Name, act.isInitialPkg, act.needAnalyzeSource, p), debug.Stack())
 		}
 	}()
 
-	act.r.sw.TrackStage(act.a.Name, act.analyze)
+	act.runner.sw.TrackStage(act.Analyzer.Name, act.analyze)
 }
 
 func (act *action) markDepsForAnalyzingSource() {
 	// Horizontal deps (analyzer.Requires) must be loaded from source and analyzed before analyzing
 	// this action.
-	for _, dep := range act.deps {
-		if dep.pkg == act.pkg {
+	for _, dep := range act.Deps {
+		if dep.Package == act.Package {
 			// Analyze source only for horizontal dependencies, e.g. from "buildssa".
 			dep.needAnalyzeSource = true // can't be set in parallel
 		}

pkg/goanalysis/runner_action_cache.go

@@ -26,7 +26,7 @@ func (act *action) loadCachedFacts() bool {
 			return true // load cached facts only for non-initial packages
 		}
 
-		if len(act.a.FactTypes) == 0 {
+		if len(act.Analyzer.FactTypes) == 0 {
 			return true // no need to load facts
 		}
 
@@ -38,15 +38,15 @@ func (act *action) loadCachedFacts() bool {
 }
 
 func (act *action) persistFactsToCache() error {
-	analyzer := act.a
+	analyzer := act.Analyzer
 	if len(analyzer.FactTypes) == 0 {
 		return nil
 	}
 
 	// Merge new facts into the package and persist them.
 	var facts []Fact
 	for key, fact := range act.packageFacts {
-		if key.pkg != act.pkg.Types {
+		if key.pkg != act.Package.Types {
 			// The fact is from inherited facts from another package
 			continue
 		}
@@ -57,7 +57,7 @@ func (act *action) persistFactsToCache() error {
 	}
 	for key, fact := range act.objectFacts {
 		obj := key.obj
-		if obj.Pkg() != act.pkg.Types {
+		if obj.Pkg() != act.Package.Types {
 			// The fact is from inherited facts from another package
 			continue
 		}
@@ -74,33 +74,33 @@ func (act *action) persistFactsToCache() error {
 		})
 	}
 
-	factsCacheDebugf("Caching %d facts for package %q and analyzer %s", len(facts), act.pkg.Name, act.a.Name)
+	factsCacheDebugf("Caching %d facts for package %q and analyzer %s", len(facts), act.Package.Name, act.Analyzer.Name)
 
-	return act.r.pkgCache.Put(act.pkg, cache.HashModeNeedAllDeps, factCacheKey(analyzer), facts)
+	return act.runner.pkgCache.Put(act.Package, cache.HashModeNeedAllDeps, factCacheKey(analyzer), facts)
 }
 
 func (act *action) loadPersistedFacts() bool {
 	var facts []Fact
 
-	err := act.r.pkgCache.Get(act.pkg, cache.HashModeNeedAllDeps, factCacheKey(act.a), &facts)
+	err := act.runner.pkgCache.Get(act.Package, cache.HashModeNeedAllDeps, factCacheKey(act.Analyzer), &facts)
 	if err != nil {
 		if !errors.Is(err, cache.ErrMissing) && !errors.Is(err, io.EOF) {
-			act.r.log.Warnf("Failed to get persisted facts: %s", err)
+			act.runner.log.Warnf("Failed to get persisted facts: %s", err)
 		}
 
-		factsCacheDebugf("No cached facts for package %q and analyzer %s", act.pkg.Name, act.a.Name)
+		factsCacheDebugf("No cached facts for package %q and analyzer %s", act.Package.Name, act.Analyzer.Name)
 		return false
 	}
 
-	factsCacheDebugf("Loaded %d cached facts for package %q and analyzer %s", len(facts), act.pkg.Name, act.a.Name)
+	factsCacheDebugf("Loaded %d cached facts for package %q and analyzer %s", len(facts), act.Package.Name, act.Analyzer.Name)
 
 	for _, f := range facts {
 		if f.Path == "" { // this is a package fact
-			key := packageFactKey{act.pkg.Types, act.factType(f.Fact)}
+			key := packageFactKey{act.Package.Types, act.factType(f.Fact)}
 			act.packageFacts[key] = f.Fact
 			continue
 		}
-		obj, err := objectpath.Object(act.pkg.Types, objectpath.Path(f.Path))
+		obj, err := objectpath.Object(act.Package.Types, objectpath.Path(f.Path))
 		if err != nil {
 			// Be lenient about these errors. For example, when
 			// analyzing io/ioutil from source, we may get a fact