Merge pull request #16123 from owen-mc/go/misc-trivial-fixes

Go: miscellaneous trivial fixes

Author: owen-mc

go/extractor/extractor.go

@@ -132,9 +132,7 @@ func ExtractWithFlags(buildFlags []string, patterns []string) error {
 	pkgsNotFound := make([]string, 0, len(pkgs))
 
 	// Do a post-order traversal and extract the package scope of each package
-	packages.Visit(pkgs, func(pkg *packages.Package) bool {
-		return true
-	}, func(pkg *packages.Package) {
+	packages.Visit(pkgs, nil, func(pkg *packages.Package) {
 		log.Printf("Processing package %s.", pkg.PkgPath)
 
 		if _, ok := pkgInfos[pkg.PkgPath]; !ok {
@@ -200,10 +198,8 @@ func ExtractWithFlags(buildFlags []string, patterns []string) error {
 	noExtractRe := regexp.MustCompile(`.*(^|` + sep + `)(\.\.|vendor)($|` + sep + `).*`)
 
 	// extract AST information for all packages
-	packages.Visit(pkgs, func(pkg *packages.Package) bool {
-		return true
-	}, func(pkg *packages.Package) {
-		for root, _ := range wantedRoots {
+	packages.Visit(pkgs, nil, func(pkg *packages.Package) {
+		for root := range wantedRoots {
 			pkgInfo := pkgInfos[pkg.PkgPath]
 			relDir, err := filepath.Rel(root, pkgInfo.PkgDir)
 			if err != nil || noExtractRe.MatchString(relDir) {
@@ -401,15 +397,15 @@ func extractObjects(tw *trap.Writer, scope *types.Scope, scopeLabel trap.Label)
 			// do not appear as objects in any scope, so they have to be dealt
 			// with separately in extractMethods.
 			if funcObj, ok := obj.(*types.Func); ok {
-				populateTypeParamParents(tw, funcObj.Type().(*types.Signature).TypeParams(), obj)
-				populateTypeParamParents(tw, funcObj.Type().(*types.Signature).RecvTypeParams(), obj)
+				populateTypeParamParents(funcObj.Type().(*types.Signature).TypeParams(), obj)
+				populateTypeParamParents(funcObj.Type().(*types.Signature).RecvTypeParams(), obj)
 			}
 			// Populate type parameter parents for named types. Note that we
 			// skip type aliases as the original type should be the parent
 			// of any type parameters.
 			if typeNameObj, ok := obj.(*types.TypeName); ok && !typeNameObj.IsAlias() {
 				if tp, ok := typeNameObj.Type().(*types.Named); ok {
-					populateTypeParamParents(tw, tp.TypeParams(), obj)
+					populateTypeParamParents(tp.TypeParams(), obj)
 				}
 			}
 			extractObject(tw, obj, lbl)
@@ -435,8 +431,8 @@ func extractMethod(tw *trap.Writer, meth *types.Func) trap.Label {
 	if !exists {
 		// Populate type parameter parents for methods. They do not appear as
 		// objects in any scope, so they have to be dealt with separately here.
-		populateTypeParamParents(tw, meth.Type().(*types.Signature).TypeParams(), meth)
-		populateTypeParamParents(tw, meth.Type().(*types.Signature).RecvTypeParams(), meth)
+		populateTypeParamParents(meth.Type().(*types.Signature).TypeParams(), meth)
+		populateTypeParamParents(meth.Type().(*types.Signature).RecvTypeParams(), meth)
 		extractObject(tw, meth, methlbl)
 	}
 
@@ -494,7 +490,7 @@ func extractObject(tw *trap.Writer, obj types.Object, lbl trap.Label) {
 func extractObjectTypes(tw *trap.Writer) {
 	// calling `extractType` on a named type will extract all methods defined
 	// on it, which will add new objects. Therefore we need to do this first
-	// before we loops over all objects and emit them.
+	// before we loop over all objects and emit them.
 	changed := true
 	for changed {
 		changed = tw.ForEachObject(extractObjectType)
@@ -870,6 +866,10 @@ func extractExpr(tw *trap.Writer, expr ast.Expr, parent trap.Label, idx int) {
 		kind = dbscheme.IdentExpr.Index()
 		dbscheme.LiteralsTable.Emit(tw, lbl, expr.Name, expr.Name)
 		def := tw.Package.TypesInfo.Defs[expr]
+		// Note that there are some cases where `expr` is in the map but `def`
+		// is nil. The docs for `tw.Package.TypesInfo.Defs` give the following
+		// examples: the package name in package clauses, or symbolic variables
+		// `t` in `t := x.(type)` of type switch headers.
 		if def != nil {
 			defTyp := extractType(tw, def.Type())
 			objlbl, exists := tw.Labeler.LookupObjectID(def, defTyp)
@@ -1010,7 +1010,10 @@ func extractExpr(tw *trap.Writer, expr ast.Expr, parent trap.Label, idx int) {
 		}
 		kind = dbscheme.TypeAssertExpr.Index()
 		extractExpr(tw, expr.X, lbl, 0)
-		extractExpr(tw, expr.Type, lbl, 1)
+		// expr.Type can be `nil` if this is the `x.(type)` in a type switch.
+		if expr.Type != nil {
+			extractExpr(tw, expr.Type, lbl, 1)
+		}
 	case *ast.CallExpr:
 		if expr == nil {
 			return
@@ -1126,11 +1129,9 @@ func extractExpr(tw *trap.Writer, expr ast.Expr, parent trap.Label, idx int) {
 // each child over its preceding child (usually either 1 for assigning increasing indices, or
 // -1 for decreasing indices)
 func extractExprs(tw *trap.Writer, exprs []ast.Expr, parent trap.Label, idx int, dir int) {
-	if exprs != nil {
-		for _, expr := range exprs {
-			extractExpr(tw, expr, parent, idx)
-			idx += dir
-		}
+	for _, expr := range exprs {
+		extractExpr(tw, expr, parent, idx)
+		idx += dir
 	}
 }
 
@@ -1386,13 +1387,10 @@ func extractStmt(tw *trap.Writer, stmt ast.Stmt, parent trap.Label, idx int) {
 // each child over its preceding child (usually either 1 for assigning increasing indices, or
 // -1 for decreasing indices)
 func extractStmts(tw *trap.Writer, stmts []ast.Stmt, parent trap.Label, idx int, dir int) {
-	if stmts != nil {
-		for _, stmt := range stmts {
-			extractStmt(tw, stmt, parent, idx)
-			idx += dir
-		}
+	for _, stmt := range stmts {
+		extractStmt(tw, stmt, parent, idx)
+		idx += dir
 	}
-
 }
 
 // extractDecl extracts AST information for the given declaration
@@ -1941,7 +1939,7 @@ func extractTypeParamDecls(tw *trap.Writer, fields *ast.FieldList, parent trap.L
 }
 
 // populateTypeParamParents sets `parent` as the parent of the elements of `typeparams`
-func populateTypeParamParents(tw *trap.Writer, typeparams *types.TypeParamList, parent types.Object) {
+func populateTypeParamParents(typeparams *types.TypeParamList, parent types.Object) {
 	if typeparams != nil {
 		for idx := 0; idx < typeparams.Len(); idx++ {
 			setTypeParamParent(typeparams.At(idx), parent)
@@ -1963,16 +1961,6 @@ func getObjectBeingUsed(tw *trap.Writer, ident *ast.Ident) types.Object {
 	}
 }
 
-// tryGetGenericType returns the generic type of `tp`, and a boolean indicating
-// whether it is the same as `tp`.
-func tryGetGenericType(tp types.Type) (*types.Named, bool) {
-	if namedType, ok := tp.(*types.Named); ok {
-		originType := namedType.Origin()
-		return originType, namedType == originType
-	}
-	return nil, false
-}
-
 // trackInstantiatedStructFields tries to give the fields of an instantiated
 // struct type underlying `tp` the same labels as the corresponding fields of
 // the generic struct type. This is so that when we come across the

go/ql/lib/semmle/go/Expr.qll

@@ -754,13 +754,19 @@ class SliceExpr extends @sliceexpr, Expr {
  *
  * ```go
  * x.(T)
+ * x.(type)
  * ```
  */
 class TypeAssertExpr extends @typeassertexpr, Expr {
   /** Gets the base expression whose type is being asserted. */
   Expr getExpr() { result = this.getChildExpr(0) }
 
-  /** Gets the expression representing the asserted type. */
+  /**
+   * Gets the expression representing the asserted type.
+   *
+   * Note that this is not defined when the type assertion is of the form
+   * `x.(type)`, as found in type switches.
+   */
   Expr getTypeExpr() { result = this.getChildExpr(1) }
 
   override predicate mayHaveOwnSideEffects() { any() }

go/ql/lib/semmle/go/Scopes.qll

@@ -119,7 +119,13 @@ class Entity extends @object {
    */
   Scope getScope() { objectscopes(this, result) }
 
-  /** Gets the declaring identifier for this entity. */
+  /**
+   * Gets the declaring identifier for this entity, if any.
+   *
+   * Note that type switch statements which define a new variable in the guard
+   * actually have a new variable (of the right type) implicitly declared at
+   * the beginning of each case clause, and these do not have a declaration.
+   */
   Ident getDeclaration() { result.declares(this) }
 
   /** Gets a reference to this entity. */

go/ql/lib/semmle/go/Stmt.qll

@@ -747,13 +747,25 @@ class IfStmt extends @ifstmt, Stmt, ScopeNode {
  * ```
  */
 class CaseClause extends @caseclause, Stmt, ScopeNode {
-  /** Gets the `i`th expression of this `case` clause (0-based). */
+  /**
+   * Gets the `i`th expression of this `case` clause (0-based).
+   *
+   * Note that the default clause does not have any expressions.
+   */
   Expr getExpr(int i) { result = this.getChildExpr(-(i + 1)) }
 
-  /** Gets an expression of this `case` clause. */
+  /**
+   * Gets an expression of this `case` clause, if any.
+   *
+   * Note that the default clause does not have any expressions.
+   */
   Expr getAnExpr() { result = this.getAChildExpr() }
 
-  /** Gets the number of expressions of this `case` clause. */
+  /**
+   * Gets the number of expressions of this `case` clause.
+   *
+   * Note that the default clause does not have any expressions.
+   */
   int getNumExpr() { result = this.getNumChildExpr() }
 
   /** Gets the `i`th statement of this `case` clause (0-based). */