revel/revel#1057 code improvements

harness/app.go

@@ -20,11 +20,12 @@ type App struct {
 	cmd        AppCmd // The last cmd returned.
 }
 
+// NewApp returns app instance with binary path in it
 func NewApp(binPath string) *App {
 	return &App{BinaryPath: binPath}
 }
 
-// Return a command to run the app server using the current configuration.
+// Cmd returns a command to run the app server using the current configuration.
 func (a *App) Cmd() AppCmd {
 	a.cmd = NewAppCmd(a.BinaryPath, a.Port)
 	return a.cmd
@@ -41,6 +42,7 @@ type AppCmd struct {
 	*exec.Cmd
 }
 
+// NewAppCmd returns the AppCmd with parameters initialized for running app
 func NewAppCmd(binPath string, port int) AppCmd {
 	cmd := exec.Command(binPath,
 		fmt.Sprintf("-port=%d", port),
@@ -70,6 +72,8 @@ func (cmd AppCmd) Start() error {
 	case <-listeningWriter.notifyReady:
 		return nil
 	}
+
+	// TODO remove this unreachable code and document it
 	panic("Impossible")
 }
 
@@ -81,7 +85,7 @@ func (cmd AppCmd) Run() {
 	}
 }
 
-// Terminate the app server if it's running.
+// Kill terminates the app server if it's running.
 func (cmd AppCmd) Kill() {
 	if cmd.Cmd != nil && (cmd.ProcessState == nil || !cmd.ProcessState.Exited()) {
 		revel.TRACE.Println("Killing revel server pid", cmd.Process.Pid)
@@ -96,7 +100,7 @@ func (cmd AppCmd) Kill() {
 func (cmd AppCmd) waitChan() <-chan struct{} {
 	ch := make(chan struct{}, 1)
 	go func() {
-		cmd.Wait()
+		_ = cmd.Wait()
 		ch <- struct{}{}
 	}()
 	return ch

harness/build.go

@@ -44,8 +44,8 @@ func Build(buildFlags ...string) (app *App, compileError *revel.Error) {
 		"ImportPaths":    calcImportAliases(sourceInfo),
 		"TestSuites":     sourceInfo.TestSuites(),
 	}
-	genSource("tmp", "main.go", MAIN, templateArgs)
-	genSource("routes", "routes.go", ROUTES, templateArgs)
+	genSource("tmp", "main.go", RevelMainTemplate, templateArgs)
+	genSource("routes", "routes.go", RevelRoutesTemplate, templateArgs)
 
 	// Read build config.
 	buildTags := revel.Config.StringDefault("build.tags", "")
@@ -126,6 +126,8 @@ func Build(buildFlags ...string) (app *App, compileError *revel.Error) {
 
 		// Success getting the import, attempt to build again.
 	}
+
+	// TODO remove this unreachable code and document it
 	revel.ERROR.Fatalf("Not reachable")
 	return nil, nil
 }
@@ -179,7 +181,10 @@ func cleanDir(dir string) {
 			revel.ERROR.Println("Failed to clean dir:", err)
 		}
 	} else {
-		defer f.Close()
+		defer func() {
+			_ = f.Close()
+		}()
+
 		infos, err := f.Readdir(0)
 		if err != nil {
 			if !os.IsNotExist(err) {
@@ -221,12 +226,14 @@ func genSource(dir, filename, templateSource string, args map[string]interface{}
 
 	// Create the file
 	file, err := os.Create(filepath.Join(tmpPath, filename))
-	defer file.Close()
 	if err != nil {
 		revel.ERROR.Fatalf("Failed to create file: %v", err)
 	}
-	_, err = file.WriteString(sourceCode)
-	if err != nil {
+	defer func() {
+		_ = file.Close()
+	}()
+
+	if _, err = file.WriteString(sourceCode); err != nil {
 		revel.ERROR.Fatalf("Failed to write to file: %v", err)
 	}
 }
@@ -345,7 +352,8 @@ func newCompileError(output []byte) *revel.Error {
 	return compileError
 }
 
-const MAIN = `// GENERATED CODE - DO NOT EDIT
+// RevelMainTemplate template for app/tmp/main.go
+const RevelMainTemplate = `// GENERATED CODE - DO NOT EDIT
 package main
 
 import (
@@ -399,7 +407,9 @@ func main() {
 	revel.Run(*port)
 }
 `
-const ROUTES = `// GENERATED CODE - DO NOT EDIT
+
+// RevelRoutesTemplate template for app/conf/routes
+const RevelRoutesTemplate = `// GENERATED CODE - DO NOT EDIT
 package routes
 
 import "github.com/revel/revel"
@@ -415,7 +425,7 @@ func (_ t{{$c.StructName}}) {{.Name}}({{range .Args}}
 	args := make(map[string]string)
 	{{range .Args}}
 	revel.Unbind(args, "{{.Name}}", {{.Name}}){{end}}
-	return revel.MainRouter.Reverse("{{$c.StructName}}.{{.Name}}", args).Url
+	return revel.MainRouter.Reverse("{{$c.StructName}}.{{.Name}}", args).URL
 }
 {{end}}
 {{end}}

harness/harness.go

@@ -84,12 +84,14 @@ func NewHarness() *Harness {
 	// Prefer the app's views/errors directory, and fall back to the stock error pages.
 	revel.MainTemplateLoader = revel.NewTemplateLoader(
 		[]string{filepath.Join(revel.RevelPath, "templates")})
-	revel.MainTemplateLoader.Refresh()
+	if err := revel.MainTemplateLoader.Refresh(); err != nil {
+		revel.ERROR.Println(err)
+	}
 
-	addr := revel.HttpAddr
+	addr := revel.HTTPAddr
 	port := revel.Config.IntDefault("harness.port", 0)
 	scheme := "http"
-	if revel.HttpSsl {
+	if revel.HTTPSsl {
 		scheme = "https"
 	}
 
@@ -110,7 +112,7 @@ func NewHarness() *Harness {
 		proxy:      httputil.NewSingleHostReverseProxy(serverURL),
 	}
 
-	if revel.HttpSsl {
+	if revel.HTTPSsl {
 		harness.proxy.Transport = &http.Transport{
 			TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
 		}
@@ -166,13 +168,16 @@ func (h *Harness) Run() {
 	watcher.Listen(h, paths...)
 
 	go func() {
-		addr := fmt.Sprintf("%s:%d", revel.HttpAddr, revel.HttpPort)
+		addr := fmt.Sprintf("%s:%d", revel.HTTPAddr, revel.HTTPPort)
 		revel.INFO.Printf("Listening on %s", addr)
 
 		var err error
-		if revel.HttpSsl {
-			err = http.ListenAndServeTLS(addr, revel.HttpSslCert,
-				revel.HttpSslKey, h)
+		if revel.HTTPSsl {
+			err = http.ListenAndServeTLS(
+				addr,
+				revel.HTTPSslCert,
+				revel.HTTPSslKey,
+				h)
 		} else {
 			err = http.ListenAndServe(addr, h)
 		}
@@ -213,7 +218,7 @@ func proxyWebsocket(w http.ResponseWriter, r *http.Request, host string) {
 		d   net.Conn
 		err error
 	)
-	if revel.HttpSsl {
+	if revel.HTTPSsl {
 		// since this proxy isn't used in production,
 		// it's OK to set InsecureSkipVerify to true
 		// no need to add another configuration option.
@@ -236,8 +241,14 @@ func proxyWebsocket(w http.ResponseWriter, r *http.Request, host string) {
 		revel.ERROR.Printf("Hijack error: %v", err)
 		return
 	}
-	defer nc.Close()
-	defer d.Close()
+	defer func() {
+		if err = nc.Close(); err != nil {
+			revel.ERROR.Println(err)
+		}
+		if err = d.Close(); err != nil {
+			revel.ERROR.Println(err)
+		}
+	}()
 
 	err = r.Write(d)
 	if err != nil {

harness/reflect.go

@@ -60,12 +60,14 @@ type methodCall struct {
 	Names []string
 }
 
+// MethodSpec holds the information of one Method
 type MethodSpec struct {
 	Name        string        // Name of the method, e.g. "Index"
 	Args        []*MethodArg  // Argument descriptors
 	RenderCalls []*methodCall // Descriptions of Render() invocations from this Method.
 }
 
+// MethodArg holds the information of one argument
 type MethodArg struct {
 	Name       string   // Name of the argument.
 	TypeExpr   TypeExpr // The name of the type, e.g. "int", "*pkg.UserType"
@@ -80,8 +82,9 @@ type embeddedTypeName struct {
 // receiver.
 type methodMap map[string][]*MethodSpec
 
-// Parse the app controllers directory and return a list of the controller types found.
-// Returns a CompileError if the parsing fails.
+// ProcessSource parses the app controllers directory and
+// returns a list of the controller types found.
+// Otherwise CompileError if the parsing fails.
 func ProcessSource(roots []string) (*SourceInfo, *revel.Error) {
 	var (
 		srcInfo      *SourceInfo
@@ -120,7 +123,7 @@ func ProcessSource(roots []string) (*SourceInfo, *revel.Error) {
 			}, 0)
 			if err != nil {
 				if errList, ok := err.(scanner.ErrorList); ok {
-					var pos token.Position = errList[0].Pos
+					var pos = errList[0].Pos
 					compileError = &revel.Error{
 						SourceType:  ".go source",
 						Title:       "Go Compilation Error",
@@ -139,6 +142,8 @@ func ProcessSource(roots []string) (*SourceInfo, *revel.Error) {
 
 					return compileError
 				}
+
+				// This is exception, err alredy checked above. Here just a print
 				ast.Print(nil, err)
 				log.Fatalf("Failed to parse dir: %s", err)
 			}
@@ -490,7 +495,7 @@ func appendAction(fset *token.FileSet, mm methodMap, decl ast.Decl, pkgImportPat
 	})
 
 	var recvTypeName string
-	var recvType ast.Expr = funcDecl.Recv.List[0].Type
+	var recvType = funcDecl.Recv.List[0].Type
 	if recvStarType, ok := recvType.(*ast.StarExpr); ok {
 		recvTypeName = recvStarType.X.(*ast.Ident).Name
 	} else {
@@ -673,6 +678,8 @@ func (s *SourceInfo) TypesThatEmbed(targetType string) (filtered []*TypeInfo) {
 	return
 }
 
+// ControllerSpecs returns the all the contollers that embeds
+// `revel.Controller`
 func (s *SourceInfo) ControllerSpecs() []*TypeInfo {
 	if s.controllerSpecs == nil {
 		s.controllerSpecs = s.TypesThatEmbed(revel.RevelImportPath + ".Controller")
@@ -680,6 +687,8 @@ func (s *SourceInfo) ControllerSpecs() []*TypeInfo {
 	return s.controllerSpecs
 }
 
+// TestSuites returns the all the Application tests that embeds
+// `testing.TestSuite`
 func (s *SourceInfo) TestSuites() []*TypeInfo {
 	if s.testSuites == nil {
 		s.testSuites = s.TypesThatEmbed(revel.RevelImportPath + "/testing.TestSuite")
@@ -705,7 +714,7 @@ func (e TypeExpr) TypeName(pkgOverride string) string {
 	return e.Expr[:e.pkgIndex] + pkgName + "." + e.Expr[e.pkgIndex:]
 }
 
-// This returns the syntactic expression for referencing this type in Go.
+// NewTypeExpr returns the syntactic expression for referencing this type in Go.
 func NewTypeExpr(pkgName string, expr ast.Expr) TypeExpr {
 	switch t := expr.(type) {
 	case *ast.Ident:
@@ -731,31 +740,32 @@ func NewTypeExpr(pkgName string, expr ast.Expr) TypeExpr {
 	return TypeExpr{Valid: false}
 }
 
-var _BUILTIN_TYPES = map[string]struct{}{
-	"bool":       struct{}{},
-	"byte":       struct{}{},
-	"complex128": struct{}{},
-	"complex64":  struct{}{},
-	"error":      struct{}{},
-	"float32":    struct{}{},
-	"float64":    struct{}{},
-	"int":        struct{}{},
-	"int16":      struct{}{},
-	"int32":      struct{}{},
-	"int64":      struct{}{},
-	"int8":       struct{}{},
-	"rune":       struct{}{},
-	"string":     struct{}{},
-	"uint":       struct{}{},
-	"uint16":     struct{}{},
-	"uint32":     struct{}{},
-	"uint64":     struct{}{},
-	"uint8":      struct{}{},
-	"uintptr":    struct{}{},
+var builtInTypes = map[string]struct{}{
+	"bool":       {},
+	"byte":       {},
+	"complex128": {},
+	"complex64":  {},
+	"error":      {},
+	"float32":    {},
+	"float64":    {},
+	"int":        {},
+	"int16":      {},
+	"int32":      {},
+	"int64":      {},
+	"int8":       {},
+	"rune":       {},
+	"string":     {},
+	"uint":       {},
+	"uint16":     {},
+	"uint32":     {},
+	"uint64":     {},
+	"uint8":      {},
+	"uintptr":    {},
 }
 
+// IsBuiltinType checks the given type is built-in types of Go
 func IsBuiltinType(name string) bool {
-	_, ok := _BUILTIN_TYPES[name]
+	_, ok := builtInTypes[name]
 	return ok
 }
 

harness/reflect_test.go

@@ -92,18 +92,18 @@ func TestGetValidationKeys(t *testing.T) {
 }
 
 var TypeExprs = map[string]TypeExpr{
-	"int":        TypeExpr{"int", "", 0, true},
-	"*int":       TypeExpr{"*int", "", 1, true},
-	"[]int":      TypeExpr{"[]int", "", 2, true},
-	"...int":     TypeExpr{"[]int", "", 2, true},
-	"[]*int":     TypeExpr{"[]*int", "", 3, true},
-	"...*int":    TypeExpr{"[]*int", "", 3, true},
-	"MyType":     TypeExpr{"MyType", "pkg", 0, true},
-	"*MyType":    TypeExpr{"*MyType", "pkg", 1, true},
-	"[]MyType":   TypeExpr{"[]MyType", "pkg", 2, true},
-	"...MyType":  TypeExpr{"[]MyType", "pkg", 2, true},
-	"[]*MyType":  TypeExpr{"[]*MyType", "pkg", 3, true},
-	"...*MyType": TypeExpr{"[]*MyType", "pkg", 3, true},
+	"int":        {"int", "", 0, true},
+	"*int":       {"*int", "", 1, true},
+	"[]int":      {"[]int", "", 2, true},
+	"...int":     {"[]int", "", 2, true},
+	"[]*int":     {"[]*int", "", 3, true},
+	"...*int":    {"[]*int", "", 3, true},
+	"MyType":     {"MyType", "pkg", 0, true},
+	"*MyType":    {"*MyType", "pkg", 1, true},
+	"[]MyType":   {"[]MyType", "pkg", 2, true},
+	"...MyType":  {"[]MyType", "pkg", 2, true},
+	"[]*MyType":  {"[]*MyType", "pkg", 3, true},
+	"...*MyType": {"[]*MyType", "pkg", 3, true},
 }
 
 func TestTypeExpr(t *testing.T) {
@@ -126,10 +126,10 @@ func TestTypeExpr(t *testing.T) {
 		}
 
 		if array {
-			expr = &ast.ArrayType{expr.Pos(), nil, expr}
+			expr = &ast.ArrayType{Lbrack: expr.Pos(), Len: nil, Elt: expr}
 		}
 		if ellipsis {
-			expr = &ast.Ellipsis{expr.Pos(), expr}
+			expr = &ast.Ellipsis{Ellipsis: expr.Pos(), Elt: expr}
 		}
 
 		actual := NewTypeExpr("pkg", expr)
@@ -146,11 +146,11 @@ func TestProcessBookingSource(t *testing.T) {
 		t.Fatal("Failed to process booking source with error:", err)
 	}
 
-	CONTROLLER_PKG := "github.com/revel/samples/booking/app/controllers"
+	controllerPackage := "github.com/revel/samples/booking/app/controllers"
 	expectedControllerSpecs := []*TypeInfo{
-		{"GorpController", CONTROLLER_PKG, "controllers", nil, nil},
-		{"Application", CONTROLLER_PKG, "controllers", nil, nil},
-		{"Hotels", CONTROLLER_PKG, "controllers", nil, nil},
+		{"GorpController", controllerPackage, "controllers", nil, nil},
+		{"Application", controllerPackage, "controllers", nil, nil},
+		{"Hotels", controllerPackage, "controllers", nil, nil},
 	}
 	if len(sourceInfo.ControllerSpecs()) != len(expectedControllerSpecs) {
 		t.Errorf("Unexpected number of controllers found.  Expected %d, Found %d",