go/analysis: several little fixes

internal/checker
- don't display "[name.category]" in diagnostic
  output. Most users don't care which analyzer reported the message.
  The -json output retains this information.
- print better log messages for analyze -debug=f.
- print (non-JSON) text output to standard error, like a compiler would.

passes/pkgfact
- fix a nil deref panic when encountering non-renaming imports.
- require names to have underscores before and after (_x_)
  as this avoids a huge number of spurious matches in (e.g.) the
  syscall package.
- don't export empty facts.

Change-Id: I86c003b96521334e371f9d5fcea1323cd779d7f0
Reviewed-on: https://go-review.googlesource.com/c/139657
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

go/analysis/internal/checker/checker.go

@@ -311,7 +311,7 @@ func printDiagnostics(roots []*action) {
 		type key struct {
 			token.Position
 			*analysis.Analyzer
-			message, class string
+			message string
 		}
 		seen := make(map[key]bool)
 
@@ -322,19 +322,18 @@ func printDiagnostics(roots []*action) {
 			}
 			if act.isroot {
 				for _, f := range act.diagnostics {
-					class := act.a.Name
-					if f.Category != "" {
-						class += "." + f.Category
-					}
+					// We don't display a.Name/f.Category
+					// as most users don't care.
+
 					posn := act.pkg.Fset.Position(f.Pos)
 
-					k := key{posn, act.a, class, f.Message}
+					k := key{posn, act.a, f.Message}
 					if seen[k] {
 						continue // duplicate
 					}
 					seen[k] = true
 
-					fmt.Printf("%s: [%s] %s\n", posn, class, f.Message)
+					fmt.Fprintf(os.Stderr, "%s: %s\n", posn, f.Message)
 
 					// -c=0: show offending line of code in context.
 					if Context >= 0 {
@@ -342,7 +341,7 @@ func printDiagnostics(roots []*action) {
 						lines := strings.Split(string(data), "\n")
 						for i := posn.Line - Context; i <= posn.Line+Context; i++ {
 							if 1 <= i && i <= len(lines) {
-								fmt.Printf("%d\t%s\n", i, lines[i-1])
+								fmt.Fprintf(os.Stderr, "%d\t%s\n", i, lines[i-1])
 							}
 						}
 					}
@@ -680,7 +679,8 @@ func (act *action) exportObjectFact(obj types.Object, fact analysis.Fact) {
 	act.objectFacts[key] = fact // clobber any existing entry
 	if dbg('f') {
 		objstr := types.ObjectString(obj, (*types.Package).Name)
-		log.Printf("fact %#v on %s", fact, objstr)
+		fmt.Fprintf(os.Stderr, "%s: object %s has fact %s\n",
+			act.pkg.Fset.Position(obj.Pos()), objstr, fact)
 	}
 }
 
@@ -708,7 +708,8 @@ func (act *action) exportPackageFact(fact analysis.Fact) {
 	key := packageFactKey{act.pass.Pkg, factType(fact)}
 	act.packageFacts[key] = fact // clobber any existing entry
 	if dbg('f') {
-		log.Printf("fact %#v on %s", fact, act.pass.Pkg)
+		fmt.Fprintf(os.Stderr, "%s: package %s has fact %s\n",
+			act.pkg.Fset.Position(act.pass.Files[0].Pos()), act.pass.Pkg.Path(), fact)
 	}
 }
 

go/analysis/passes/pkgfact/pkgfact.go

@@ -4,12 +4,12 @@
 // The output of the pkgfact analysis is a set of key/values pairs
 // gathered from the analyzed package and its imported dependencies.
 // Each key/value pair comes from a top-level constant declaration
-// whose name starts with "_".  For example:
+// whose name starts and ends with "_".  For example:
 //
 //      package p
 //
-// 	const _greeting  = "hello"
-// 	const _audience  = "world"
+// 	const _greeting_  = "hello"
+// 	const _audience_  = "world"
 //
 // the pkgfact analysis output for package p would be:
 //
@@ -55,7 +55,7 @@ func run(pass *analysis.Pass) (interface{}, error) {
 	// package and accumulate its information into the result.
 	// (Warning: accumulation leads to quadratic growth of work.)
 	doImport := func(spec *ast.ImportSpec) {
-		pkg := pass.TypesInfo.Defs[spec.Name].(*types.PkgName).Imported()
+		pkg := imported(pass.TypesInfo, spec)
 		var fact pairsFact
 		if pass.ImportPackageFact(pkg, &fact) {
 			for _, pair := range fact {
@@ -71,10 +71,12 @@ func run(pass *analysis.Pass) (interface{}, error) {
 		if len(spec.Names) == len(spec.Values) {
 			for i := range spec.Names {
 				name := spec.Names[i].Name
-				if strings.HasPrefix(name, "_") {
-					key := name[1:]
-					value := pass.TypesInfo.Types[spec.Values[i]].Value.String()
-					result[key] = value
+				if strings.HasPrefix(name, "_") && strings.HasSuffix(name, "_") {
+
+					if key := strings.Trim(name[1:], "_"); key != "" {
+						value := pass.TypesInfo.Types[spec.Values[i]].Value.String()
+						result[key] = value
+					}
 				}
 			}
 		}
@@ -105,7 +107,17 @@ func run(pass *analysis.Pass) (interface{}, error) {
 	for _, key := range keys {
 		fact = append(fact, fmt.Sprintf("%s=%s", key, result[key]))
 	}
-	pass.ExportPackageFact(&fact)
+	if len(fact) > 0 {
+		pass.ExportPackageFact(&fact)
+	}
 
 	return result, nil
 }
+
+func imported(info *types.Info, spec *ast.ImportSpec) *types.Package {
+	obj, ok := info.Implicits[spec]
+	if !ok {
+		obj = info.Defs[spec.Name] // renaming import
+	}
+	return obj.(*types.PkgName).Imported()
+}

go/analysis/passes/pkgfact/testdata/src/a/a.go

@@ -1,4 +1,4 @@
 package a
 
-const _greeting = "hello"
-const _audience = "world"
+const _greeting_ = "hello"
+const _audience_ = "world"

go/analysis/passes/pkgfact/testdata/src/b/b.go

@@ -2,4 +2,4 @@ package b
 
 import _ "a"
 
-const _pi = 3.14159
+const _pi_ = 3.14159