From 476d41c67b58f833e3f2c094ca766cad9ad2f59e Mon Sep 17 00:00:00 2001
From: Robert Griesemer <gri@golang.org>
Date: Mon, 8 Sep 2014 10:29:00 -0700
Subject: [PATCH] go.tools/go/types: better error messages

- for unused packages where base(package path) != package name
- for conflicts between imported packages or dot-imported objects
  and local declarations

Per suggestions from adonovan, inspired by the gc error messages.

LGTM=adonovan
R=adonovan, bradfitz
CC=golang-codereviews
https://golang.org/cl/135550043
---
 go/types/resolver.go               | 26 ++++++++++++++++++++++++--
 go/types/testdata/importdecl0a.src |  7 +++++--
 2 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/go/types/resolver.go b/go/types/resolver.go
index dce877bc..4e0c48f4 100644
--- a/go/types/resolver.go
+++ b/go/types/resolver.go
@@ -9,6 +9,7 @@ import (
 	"fmt"
 	"go/ast"
 	"go/token"
+	pathLib "path"
 	"strconv"
 	"strings"
 	"unicode"
@@ -207,6 +208,14 @@ func (check *Checker) collectObjects() {
 								// A package scope may contain non-exported objects,
 								// do not import them!
 								if obj.Exported() {
+									// TODO(gri) When we import a package, we create
+									// a new local package object. We should do the
+									// same for each dot-imported object. That way
+									// they can have correct position information.
+									// (We must not modify their existing position
+									// information because the same package - found
+									// via Config.Packages - may be dot-imported in
+									// another package!)
 									check.declare(fileScope, nil, obj)
 									check.recordImplicit(s, obj)
 								}
@@ -343,7 +352,14 @@ func (check *Checker) collectObjects() {
 	for _, scope := range check.fileScopes {
 		for _, obj := range scope.elems {
 			if alt := pkg.scope.Lookup(obj.Name()); alt != nil {
-				check.errorf(alt.Pos(), "%s already declared in this file through import of package %s", obj.Name(), obj.Pkg().Name())
+				if pkg, ok := obj.(*PkgName); ok {
+					check.errorf(alt.Pos(), "%s already declared through import of %s", alt.Name(), pkg.Imported())
+					check.reportAltDecl(pkg)
+				} else {
+					check.errorf(alt.Pos(), "%s already declared through dot-import of %s", alt.Name(), obj.Pkg())
+					// TODO(gri) dot-imported objects don't have a position; reportAltDecl won't print anything
+					check.reportAltDecl(obj)
+				}
 			}
 		}
 	}
@@ -397,7 +413,13 @@ func (check *Checker) unusedImports() {
 				// Unused "blank imports" are automatically ignored
 				// since _ identifiers are not entered into scopes.
 				if !obj.used {
-					check.softErrorf(obj.pos, "%q imported but not used", obj.imported.path)
+					path := obj.imported.path
+					base := pathLib.Base(path)
+					if obj.name == base {
+						check.softErrorf(obj.pos, "%q imported but not used", path)
+					} else {
+						check.softErrorf(obj.pos, "%q imported but not used as %s", path, obj.name)
+					}
 				}
 			default:
 				// All other objects in the file scope must be dot-
diff --git a/go/types/testdata/importdecl0a.src b/go/types/testdata/importdecl0a.src
index 49a35fec..463dcd08 100644
--- a/go/types/testdata/importdecl0a.src
+++ b/go/types/testdata/importdecl0a.src
@@ -17,7 +17,7 @@ import (
 )
 
 import "math" /* ERROR "imported but not used" */
-import m /* ERROR "imported but not used" */ "math"
+import m /* ERROR "imported but not used as m" */ "math"
 import _ "math"
 
 import (
@@ -34,8 +34,11 @@ import f2 "fmt"
 type flag int
 type _ reflect /* ERROR "not exported" */ .flag
 
+// imported package name may conflict with local objects
+type reflect /* ERROR "reflect already declared" */ int
+
 // dot-imported exported objects may conflict with local objects
-type Value /* ERROR "already declared in this file" */ struct{}
+type Value /* ERROR "Value already declared through dot-import of package reflect" */ struct{}
 
 var _ = fmt.Println // use "fmt"