Code optimized (#493)

tools/goctl/api/parser/g4/ast/api.go

@@ -22,12 +22,6 @@ type Api struct {
 }
 
 func (v *ApiVisitor) VisitApi(ctx *api.ApiContext) interface{} {
-	defer func() {
-		if p := recover(); p != nil {
-			panic(fmt.Errorf("%+v", p))
-		}
-	}()
-
 	var final Api
 	final.importM = map[string]PlaceHolder{}
 	final.typeM = map[string]PlaceHolder{}
@@ -36,109 +30,128 @@ func (v *ApiVisitor) VisitApi(ctx *api.ApiContext) interface{} {
 	final.routeM = map[string]PlaceHolder{}
 	for _, each := range ctx.AllSpec() {
 		root := each.Accept(v).(*Api)
-		if root.Syntax != nil {
-			if final.Syntax != nil {
-				v.panic(root.Syntax.Syntax, fmt.Sprintf("mutiple syntax declaration"))
-			}
-
-			final.Syntax = root.Syntax
-		}
-
-		for _, imp := range root.Import {
-			if _, ok := final.importM[imp.Value.Text()]; ok {
-				v.panic(imp.Import, fmt.Sprintf("duplicate import '%s'", imp.Value.Text()))
-			}
-
-			final.importM[imp.Value.Text()] = Holder
-			final.Import = append(final.Import, imp)
-		}
-
-		if root.Info != nil {
-			infoM := map[string]PlaceHolder{}
-			if final.Info != nil {
-				v.panic(root.Info.Info, fmt.Sprintf("mutiple info declaration"))
-			}
-
-			for _, value := range root.Info.Kvs {
-				if _, ok := infoM[value.Key.Text()]; ok {
-					v.panic(value.Key, fmt.Sprintf("duplicate key '%s'", value.Key.Text()))
-				}
-				infoM[value.Key.Text()] = Holder
-			}
-
-			final.Info = root.Info
-		}
-
-		for _, tp := range root.Type {
-			if _, ok := final.typeM[tp.NameExpr().Text()]; ok {
-				v.panic(tp.NameExpr(), fmt.Sprintf("duplicate type '%s'", tp.NameExpr().Text()))
-			}
-
-			final.typeM[tp.NameExpr().Text()] = Holder
-			final.Type = append(final.Type, tp)
-		}
-
-		for _, service := range root.Service {
-			if _, ok := final.serviceM[service.ServiceApi.Name.Text()]; !ok && len(final.serviceM) > 0 {
-				v.panic(service.ServiceApi.Name, fmt.Sprintf("mutiple service declaration"))
-			}
-
-			if service.AtServer != nil {
-				atServerM := map[string]PlaceHolder{}
-				for _, kv := range service.AtServer.Kv {
-					if _, ok := atServerM[kv.Key.Text()]; ok {
-						v.panic(kv.Key, fmt.Sprintf("duplicate key '%s'", kv.Key.Text()))
-					}
-
-					atServerM[kv.Key.Text()] = Holder
-				}
-			}
-
-			for _, route := range service.ServiceApi.ServiceRoute {
-				uniqueRoute := fmt.Sprintf("%s %s", route.Route.Method.Text(), route.Route.Path.Text())
-				if _, ok := final.routeM[uniqueRoute]; ok {
-					v.panic(route.Route.Method, fmt.Sprintf("duplicate route '%s'", uniqueRoute))
-				}
-
-				final.routeM[uniqueRoute] = Holder
-				var handlerExpr Expr
-				if route.AtServer != nil {
-					atServerM := map[string]PlaceHolder{}
-					for _, kv := range route.AtServer.Kv {
-						if _, ok := atServerM[kv.Key.Text()]; ok {
-							v.panic(kv.Key, fmt.Sprintf("duplicate key '%s'", kv.Key.Text()))
-						}
-						atServerM[kv.Key.Text()] = Holder
-						if kv.Key.Text() == "handler" {
-							handlerExpr = kv.Value
-						}
-					}
-				}
-
-				if route.AtHandler != nil {
-					handlerExpr = route.AtHandler.Name
-				}
-
-				if handlerExpr == nil {
-					v.panic(route.Route.Method, fmt.Sprintf("mismtached handler"))
-				}
-
-				if handlerExpr.Text() == "" {
-					v.panic(handlerExpr, fmt.Sprintf("mismtached handler"))
-				}
-
-				if _, ok := final.handlerM[handlerExpr.Text()]; ok {
-					v.panic(handlerExpr, fmt.Sprintf("duplicate handler '%s'", handlerExpr.Text()))
-				}
-				final.handlerM[handlerExpr.Text()] = Holder
-			}
-			final.Service = append(final.Service, service)
-		}
+		v.acceptSyntax(root, &final)
+		v.accetpImport(root, &final)
+		v.acceptInfo(root, &final)
+		v.acceptType(root, &final)
+		v.acceptService(root, &final)
 	}
 
 	return &final
 }
 
+func (v *ApiVisitor) acceptService(root *Api, final *Api) {
+	for _, service := range root.Service {
+		if _, ok := final.serviceM[service.ServiceApi.Name.Text()]; !ok && len(final.serviceM) > 0 {
+			v.panic(service.ServiceApi.Name, fmt.Sprintf("mutiple service declaration"))
+		}
+		v.duplicateServerItemCheck(service)
+
+		for _, route := range service.ServiceApi.ServiceRoute {
+			uniqueRoute := fmt.Sprintf("%s %s", route.Route.Method.Text(), route.Route.Path.Text())
+			if _, ok := final.routeM[uniqueRoute]; ok {
+				v.panic(route.Route.Method, fmt.Sprintf("duplicate route '%s'", uniqueRoute))
+			}
+
+			final.routeM[uniqueRoute] = Holder
+			var handlerExpr Expr
+			if route.AtServer != nil {
+				atServerM := map[string]PlaceHolder{}
+				for _, kv := range route.AtServer.Kv {
+					if _, ok := atServerM[kv.Key.Text()]; ok {
+						v.panic(kv.Key, fmt.Sprintf("duplicate key '%s'", kv.Key.Text()))
+					}
+					atServerM[kv.Key.Text()] = Holder
+					if kv.Key.Text() == "handler" {
+						handlerExpr = kv.Value
+					}
+				}
+			}
+
+			if route.AtHandler != nil {
+				handlerExpr = route.AtHandler.Name
+			}
+
+			if handlerExpr == nil {
+				v.panic(route.Route.Method, fmt.Sprintf("mismtached handler"))
+			}
+
+			if handlerExpr.Text() == "" {
+				v.panic(handlerExpr, fmt.Sprintf("mismtached handler"))
+			}
+
+			if _, ok := final.handlerM[handlerExpr.Text()]; ok {
+				v.panic(handlerExpr, fmt.Sprintf("duplicate handler '%s'", handlerExpr.Text()))
+			}
+			final.handlerM[handlerExpr.Text()] = Holder
+		}
+		final.Service = append(final.Service, service)
+	}
+}
+
+func (v *ApiVisitor) duplicateServerItemCheck(service *Service) {
+	if service.AtServer != nil {
+		atServerM := map[string]PlaceHolder{}
+		for _, kv := range service.AtServer.Kv {
+			if _, ok := atServerM[kv.Key.Text()]; ok {
+				v.panic(kv.Key, fmt.Sprintf("duplicate key '%s'", kv.Key.Text()))
+			}
+
+			atServerM[kv.Key.Text()] = Holder
+		}
+	}
+}
+
+func (v *ApiVisitor) acceptType(root *Api, final *Api) {
+	for _, tp := range root.Type {
+		if _, ok := final.typeM[tp.NameExpr().Text()]; ok {
+			v.panic(tp.NameExpr(), fmt.Sprintf("duplicate type '%s'", tp.NameExpr().Text()))
+		}
+
+		final.typeM[tp.NameExpr().Text()] = Holder
+		final.Type = append(final.Type, tp)
+	}
+}
+
+func (v *ApiVisitor) acceptInfo(root *Api, final *Api) {
+	if root.Info != nil {
+		infoM := map[string]PlaceHolder{}
+		if final.Info != nil {
+			v.panic(root.Info.Info, fmt.Sprintf("mutiple info declaration"))
+		}
+
+		for _, value := range root.Info.Kvs {
+			if _, ok := infoM[value.Key.Text()]; ok {
+				v.panic(value.Key, fmt.Sprintf("duplicate key '%s'", value.Key.Text()))
+			}
+			infoM[value.Key.Text()] = Holder
+		}
+
+		final.Info = root.Info
+	}
+}
+
+func (v *ApiVisitor) accetpImport(root *Api, final *Api) {
+	for _, imp := range root.Import {
+		if _, ok := final.importM[imp.Value.Text()]; ok {
+			v.panic(imp.Import, fmt.Sprintf("duplicate import '%s'", imp.Value.Text()))
+		}
+
+		final.importM[imp.Value.Text()] = Holder
+		final.Import = append(final.Import, imp)
+	}
+}
+
+func (v *ApiVisitor) acceptSyntax(root *Api, final *Api) {
+	if root.Syntax != nil {
+		if final.Syntax != nil {
+			v.panic(root.Syntax.Syntax, fmt.Sprintf("mutiple syntax declaration"))
+		}
+
+		final.Syntax = root.Syntax
+	}
+}
+
 func (v *ApiVisitor) VisitSpec(ctx *api.SpecContext) interface{} {
 	var root Api
 	if ctx.SyntaxLit() != nil {

tools/goctl/api/parser/g4/ast/apiparser.go

@@ -156,28 +156,9 @@ func (p *Parser) invoke(linePrefix, content string) (v *Api, err error) {
 }
 
 func (p *Parser) valid(mainApi *Api, nestedApi *Api) error {
-	if len(nestedApi.Import) > 0 {
-		importToken := nestedApi.Import[0].Import
-		return fmt.Errorf("%s line %d:%d the nested api does not support import",
-			nestedApi.LinePrefix, importToken.Line(), importToken.Column())
-	}
-
-	if mainApi.Syntax != nil && nestedApi.Syntax != nil {
-		if mainApi.Syntax.Version.Text() != nestedApi.Syntax.Version.Text() {
-			syntaxToken := nestedApi.Syntax.Syntax
-			return fmt.Errorf("%s line %d:%d multiple syntax declaration, expecting syntax '%s', but found '%s'",
-				nestedApi.LinePrefix, syntaxToken.Line(), syntaxToken.Column(), mainApi.Syntax.Version.Text(), nestedApi.Syntax.Version.Text())
-		}
-	}
-
-	if len(mainApi.Service) > 0 {
-		mainService := mainApi.Service[0]
-		for _, service := range nestedApi.Service {
-			if mainService.ServiceApi.Name.Text() != service.ServiceApi.Name.Text() {
-				return fmt.Errorf("%s multiple service name declaration, expecting service name '%s', but found '%s'",
-					nestedApi.LinePrefix, mainService.ServiceApi.Name.Text(), service.ServiceApi.Name.Text())
-			}
-		}
+	err := p.nestedApiCheck(mainApi, nestedApi)
+	if err != nil {
+		return err
 	}
 
 	mainHandlerMap := make(map[string]PlaceHolder)
@@ -218,6 +199,23 @@ func (p *Parser) valid(mainApi *Api, nestedApi *Api) error {
 	}
 
 	// duplicate route check
+	err = p.duplicateRouteCheck(nestedApi, mainHandlerMap, mainRouteMap)
+	if err != nil {
+		return err
+	}
+
+	// duplicate type check
+	for _, each := range nestedApi.Type {
+		if _, ok := mainTypeMap[each.NameExpr().Text()]; ok {
+			return fmt.Errorf("%s line %d:%d duplicate type declaration '%s'",
+				nestedApi.LinePrefix, each.NameExpr().Line(), each.NameExpr().Column(), each.NameExpr().Text())
+		}
+	}
+
+	return nil
+}
+
+func (p *Parser) duplicateRouteCheck(nestedApi *Api, mainHandlerMap map[string]PlaceHolder, mainRouteMap map[string]PlaceHolder) error {
 	for _, each := range nestedApi.Service {
 		for _, r := range each.ServiceApi.ServiceRoute {
 			handler := r.GetHandler()
@@ -237,12 +235,31 @@ func (p *Parser) valid(mainApi *Api, nestedApi *Api) error {
 			}
 		}
 	}
+	return nil
+}
 
-	// duplicate type check
-	for _, each := range nestedApi.Type {
-		if _, ok := mainTypeMap[each.NameExpr().Text()]; ok {
-			return fmt.Errorf("%s line %d:%d duplicate type declaration '%s'",
-				nestedApi.LinePrefix, each.NameExpr().Line(), each.NameExpr().Column(), each.NameExpr().Text())
+func (p *Parser) nestedApiCheck(mainApi *Api, nestedApi *Api) error {
+	if len(nestedApi.Import) > 0 {
+		importToken := nestedApi.Import[0].Import
+		return fmt.Errorf("%s line %d:%d the nested api does not support import",
+			nestedApi.LinePrefix, importToken.Line(), importToken.Column())
+	}
+
+	if mainApi.Syntax != nil && nestedApi.Syntax != nil {
+		if mainApi.Syntax.Version.Text() != nestedApi.Syntax.Version.Text() {
+			syntaxToken := nestedApi.Syntax.Syntax
+			return fmt.Errorf("%s line %d:%d multiple syntax declaration, expecting syntax '%s', but found '%s'",
+				nestedApi.LinePrefix, syntaxToken.Line(), syntaxToken.Column(), mainApi.Syntax.Version.Text(), nestedApi.Syntax.Version.Text())
+		}
+	}
+
+	if len(mainApi.Service) > 0 {
+		mainService := mainApi.Service[0]
+		for _, service := range nestedApi.Service {
+			if mainService.ServiceApi.Name.Text() != service.ServiceApi.Name.Text() {
+				return fmt.Errorf("%s multiple service name declaration, expecting service name '%s', but found '%s'",
+					nestedApi.LinePrefix, mainService.ServiceApi.Name.Text(), service.ServiceApi.Name.Text())
+			}
 		}
 	}
 	return nil
@@ -276,56 +293,80 @@ func (p *Parser) checkTypeDeclaration(apiList []*Api) error {
 
 	for _, apiItem := range apiList {
 		linePrefix := apiItem.LinePrefix
-		for _, each := range apiItem.Type {
-			tp, ok := each.(*TypeStruct)
-			if !ok {
-				continue
+		err := p.checkTypes(apiItem, linePrefix, types)
+		if err != nil {
+			return err
+		}
+
+		err = p.checkServices(apiItem, types, linePrefix)
+		if err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+func (p *Parser) checkServices(apiItem *Api, types map[string]TypeExpr, linePrefix string) error {
+	for _, service := range apiItem.Service {
+		for _, each := range service.ServiceApi.ServiceRoute {
+			route := each.Route
+			err := p.checkRequestBody(route, types, linePrefix)
+			if err != nil {
+				return err
 			}
 
-			for _, member := range tp.Fields {
-				err := p.checkType(linePrefix, types, member.DataType)
-				if err != nil {
-					return err
+			if route.Reply != nil && route.Reply.Name.IsNotNil() && route.Reply.Name.Expr().IsNotNil() {
+				reply := route.Reply.Name
+				var structName string
+				switch tp := reply.(type) {
+				case *Literal:
+					structName = tp.Literal.Text()
+				case *Array:
+					switch innerTp := tp.Literal.(type) {
+					case *Literal:
+						structName = innerTp.Literal.Text()
+					case *Pointer:
+						structName = innerTp.Name.Text()
+					}
+				}
+
+				if api.IsBasicType(structName) {
+					continue
+				}
+
+				_, ok := types[structName]
+				if !ok {
+					return fmt.Errorf("%s line %d:%d can not found declaration '%s' in context",
+						linePrefix, route.Reply.Name.Expr().Line(), route.Reply.Name.Expr().Column(), structName)
 				}
 			}
 		}
+	}
+	return nil
+}
 
-		for _, service := range apiItem.Service {
-			for _, each := range service.ServiceApi.ServiceRoute {
-				route := each.Route
-				if route.Req != nil && route.Req.Name.IsNotNil() && route.Req.Name.Expr().IsNotNil() {
-					_, ok := types[route.Req.Name.Expr().Text()]
-					if !ok {
-						return fmt.Errorf("%s line %d:%d can not found declaration '%s' in context",
-							linePrefix, route.Req.Name.Expr().Line(), route.Req.Name.Expr().Column(), route.Req.Name.Expr().Text())
-					}
-				}
+func (p *Parser) checkRequestBody(route *Route, types map[string]TypeExpr, linePrefix string) error {
+	if route.Req != nil && route.Req.Name.IsNotNil() && route.Req.Name.Expr().IsNotNil() {
+		_, ok := types[route.Req.Name.Expr().Text()]
+		if !ok {
+			return fmt.Errorf("%s line %d:%d can not found declaration '%s' in context",
+				linePrefix, route.Req.Name.Expr().Line(), route.Req.Name.Expr().Column(), route.Req.Name.Expr().Text())
+		}
+	}
+	return nil
+}
 
-				if route.Reply != nil && route.Reply.Name.IsNotNil() && route.Reply.Name.Expr().IsNotNil() {
-					reply := route.Reply.Name
-					var structName string
-					switch tp := reply.(type) {
-					case *Literal:
-						structName = tp.Literal.Text()
-					case *Array:
-						switch innerTp := tp.Literal.(type) {
-						case *Literal:
-							structName = innerTp.Literal.Text()
-						case *Pointer:
-							structName = innerTp.Name.Text()
-						}
-					}
+func (p *Parser) checkTypes(apiItem *Api, linePrefix string, types map[string]TypeExpr) error {
+	for _, each := range apiItem.Type {
+		tp, ok := each.(*TypeStruct)
+		if !ok {
+			continue
+		}
 
-					if api.IsBasicType(structName) {
-						continue
-					}
-
-					_, ok := types[structName]
-					if !ok {
-						return fmt.Errorf("%s line %d:%d can not found declaration '%s' in context",
-							linePrefix, route.Reply.Name.Expr().Line(), route.Reply.Name.Expr().Column(), structName)
-					}
-				}
+		for _, member := range tp.Fields {
+			err := p.checkType(linePrefix, types, member.DataType)
+			if err != nil {
+				return err
 			}
 		}
 	}

tools/goctl/api/parser/parser.go

@@ -213,13 +213,7 @@ func (p parser) fillService() error {
 	var groups []spec.Group
 	for _, item := range p.ast.Service {
 		var group spec.Group
-		if item.AtServer != nil {
-			var properties = make(map[string]string, 0)
-			for _, kv := range item.AtServer.Kv {
-				properties[kv.Key.Text()] = kv.Value.Text()
-			}
-			group.Annotation.Properties = properties
-		}
+		p.fillAtServer(item, &group)
 
 		for _, astRoute := range item.ServiceApi.ServiceRoute {
 			route := spec.Route{
@@ -231,25 +225,9 @@ func (p parser) fillService() error {
 				route.Handler = astRoute.AtHandler.Name.Text()
 			}
 
-			if astRoute.AtServer != nil {
-				var properties = make(map[string]string, 0)
-				for _, kv := range astRoute.AtServer.Kv {
-					properties[kv.Key.Text()] = kv.Value.Text()
-				}
-				route.Annotation.Properties = properties
-				if len(route.Handler) == 0 {
-					route.Handler = properties["handler"]
-				}
-				if len(route.Handler) == 0 {
-					return fmt.Errorf("missing handler annotation for %q", route.Path)
-				}
-
-				for _, char := range route.Handler {
-					if !unicode.IsDigit(char) && !unicode.IsLetter(char) {
-						return fmt.Errorf("route [%s] handler [%s] invalid, handler name should only contains letter or digit",
-							route.Path, route.Handler)
-					}
-				}
+			err := p.fillRouteAtServer(astRoute, &route)
+			if err != nil {
+				return err
 			}
 
 			if astRoute.Route.Req != nil {
@@ -269,7 +247,7 @@ func (p parser) fillService() error {
 				}
 			}
 
-			err := p.fillRouteType(&route)
+			err = p.fillRouteType(&route)
 			if err != nil {
 				return err
 			}
@@ -289,6 +267,40 @@ func (p parser) fillService() error {
 	return nil
 }
 
+func (p parser) fillRouteAtServer(astRoute *ast.ServiceRoute, route *spec.Route) error {
+	if astRoute.AtServer != nil {
+		var properties = make(map[string]string, 0)
+		for _, kv := range astRoute.AtServer.Kv {
+			properties[kv.Key.Text()] = kv.Value.Text()
+		}
+		route.Annotation.Properties = properties
+		if len(route.Handler) == 0 {
+			route.Handler = properties["handler"]
+		}
+		if len(route.Handler) == 0 {
+			return fmt.Errorf("missing handler annotation for %q", route.Path)
+		}
+
+		for _, char := range route.Handler {
+			if !unicode.IsDigit(char) && !unicode.IsLetter(char) {
+				return fmt.Errorf("route [%s] handler [%s] invalid, handler name should only contains letter or digit",
+					route.Path, route.Handler)
+			}
+		}
+	}
+	return nil
+}
+
+func (p parser) fillAtServer(item *ast.Service, group *spec.Group) {
+	if item.AtServer != nil {
+		var properties = make(map[string]string, 0)
+		for _, kv := range item.AtServer.Kv {
+			properties[kv.Key.Text()] = kv.Value.Text()
+		}
+		group.Annotation.Properties = properties
+	}
+}
+
 func (p parser) fillRouteType(route *spec.Route) error {
 	if route.RequestType != nil {
 		switch route.RequestType.(type) {