packer: Clean up some of the JSON syntax stuff

CHANGELOG.md

@@ -1,6 +1,8 @@
 ## 0.1.4 (unreleased)
 
+IMPROVEMENTS:
 
+* core: Template syntax errors now show line and character number. [GH-56]
 
 ## 0.1.3 (July 1, 2013)
 

packer/template.go

@@ -57,29 +57,6 @@ type rawProvisionerConfig struct {
 	rawConfig interface{}
 }
 
-// displaySyntaxError returns a location for the json syntax error
-// Adapted from:
-// https://groups.google.com/forum/#!topic/golang-nuts/fizimmXtVfc
-func displaySyntaxError(js []byte, syntaxError error) (err error) {
-	syntax, ok := syntaxError.(*json.SyntaxError)
-	if !ok {
-		err = syntaxError
-		return
-	}
-	newline := []byte{'\x0a'}
-	space := []byte{' '}
-
-	start, end := bytes.LastIndex(js[:syntax.Offset], newline)+1, len(js)
-	if idx := bytes.Index(js[start:], newline); idx >= 0 {
-		end = start + idx
-	}
-	
-	line, pos := bytes.Count(js[:start], newline)+1, int(syntax.Offset) - start - 1
-	
-	err = fmt.Errorf("\nError in line %d: %s \n%s\n%s^", line, syntaxError, js[start:end], bytes.Repeat(space, pos))
-	return
-}
-
 // ParseTemplate takes a byte slice and parses a Template from it, returning
 // the template and possibly errors while loading the template. The error
 // could potentially be a MultiError, representing multiple errors. Knowing
@@ -89,7 +66,27 @@ func ParseTemplate(data []byte) (t *Template, err error) {
 	var rawTpl rawTemplate
 	err = json.Unmarshal(data, &rawTpl)
 	if err != nil {
-		err = displaySyntaxError(data, err)
+		syntaxErr, ok := err.(*json.SyntaxError)
+		if !ok {
+			return
+		}
+
+		// We have a syntax error. Extract out the line number and friends.
+		// https://groups.google.com/forum/#!topic/golang-nuts/fizimmXtVfc
+		newline := []byte{'\x0a'}
+
+		start := bytes.LastIndex(data[:syntaxErr.Offset], newline)+1
+		end := len(data)
+		if idx := bytes.Index(data[start:], newline); idx >= 0 {
+			end = start + idx
+		}
+
+		line := bytes.Count(data[:start], newline)+1
+		pos := int(syntaxErr.Offset) - start - 1
+
+		err = fmt.Errorf("Error in line %d, char %d: %s\n%s",
+			line, pos, syntaxErr, data[start:end])
+
 		return
 	}
 
@@ -231,151 +228,151 @@ func parsePostProvisioner(i int, rawV interface{}) (result []map[string]interfac
 				errors = append(
 					errors,
 					fmt.Errorf("Post-processor %d.%d: sequences not allowed to be nested in sequences", i+1, j+1))
-			default:
-				errors = append(errors, fmt.Errorf("Post-processor %d.%d is in a bad format.", i+1, j+1))
+				default:
+					errors = append(errors, fmt.Errorf("Post-processor %d.%d is in a bad format.", i+1, j+1))
+				}
 			}
-		}
 
-		if len(errors) == 0 {
-			errors = nil
+			if len(errors) == 0 {
+				errors = nil
+			}
+		default:
+			result = nil
+			errors = []error{fmt.Errorf("Post-processor %d is in a bad format.", i+1)}
 		}
-	default:
-		result = nil
-		errors = []error{fmt.Errorf("Post-processor %d is in a bad format.", i+1)}
+
+		return
 	}
 
-	return
-}
+	// BuildNames returns a slice of the available names of builds that
+	// this template represents.
+	func (t *Template) BuildNames() []string {
+		names := make([]string, 0, len(t.Builders))
+		for name, _ := range t.Builders {
+			names = append(names, name)
+		}
 
-// BuildNames returns a slice of the available names of builds that
-// this template represents.
-func (t *Template) BuildNames() []string {
-	names := make([]string, 0, len(t.Builders))
-	for name, _ := range t.Builders {
-		names = append(names, name)
+		return names
 	}
 
-	return names
-}
+	// Build returns a Build for the given name.
+	//
+	// If the build does not exist as part of this template, an error is
+	// returned.
+	func (t *Template) Build(name string, components *ComponentFinder) (b Build, err error) {
+		// Setup the Builder
+		builderConfig, ok := t.Builders[name]
+		if !ok {
+			err = fmt.Errorf("No such build found in template: %s", name)
+			return
+		}
 
-// Build returns a Build for the given name.
-//
-// If the build does not exist as part of this template, an error is
-// returned.
-func (t *Template) Build(name string, components *ComponentFinder) (b Build, err error) {
-	// Setup the Builder
-	builderConfig, ok := t.Builders[name]
-	if !ok {
-		err = fmt.Errorf("No such build found in template: %s", name)
-		return
-	}
+		// We panic if there is no builder function because this is really
+		// an internal bug that always needs to be fixed, not an error.
+		if components.Builder == nil {
+			panic("no builder function")
+		}
 
-	// We panic if there is no builder function because this is really
-	// an internal bug that always needs to be fixed, not an error.
-	if components.Builder == nil {
-		panic("no builder function")
-	}
+		// Panic if there are provisioners on the template but no provisioner
+		// component finder. This is always an internal error, so we panic.
+		if len(t.Provisioners) > 0 && components.Provisioner == nil {
+			panic("no provisioner function")
+		}
 
-	// Panic if there are provisioners on the template but no provisioner
-	// component finder. This is always an internal error, so we panic.
-	if len(t.Provisioners) > 0 && components.Provisioner == nil {
-		panic("no provisioner function")
-	}
+		builder, err := components.Builder(builderConfig.Type)
+		if err != nil {
+			return
+		}
 
-	builder, err := components.Builder(builderConfig.Type)
-	if err != nil {
-		return
-	}
+		if builder == nil {
+			err = fmt.Errorf("Builder type not found: %s", builderConfig.Type)
+			return
+		}
 
-	if builder == nil {
-		err = fmt.Errorf("Builder type not found: %s", builderConfig.Type)
-		return
-	}
+		// Gather the Hooks
+		hooks := make(map[string][]Hook)
+		for tplEvent, tplHooks := range t.Hooks {
+			curHooks := make([]Hook, 0, len(tplHooks))
 
-	// Gather the Hooks
-	hooks := make(map[string][]Hook)
-	for tplEvent, tplHooks := range t.Hooks {
-		curHooks := make([]Hook, 0, len(tplHooks))
+			for _, hookName := range tplHooks {
+				var hook Hook
+				hook, err = components.Hook(hookName)
+				if err != nil {
+					return
+				}
 
-		for _, hookName := range tplHooks {
-			var hook Hook
-			hook, err = components.Hook(hookName)
-			if err != nil {
-				return
-			}
+				if hook == nil {
+					err = fmt.Errorf("Hook not found: %s", hookName)
+					return
+				}
 
-			if hook == nil {
-				err = fmt.Errorf("Hook not found: %s", hookName)
-				return
+				curHooks = append(curHooks, hook)
 			}
 
-			curHooks = append(curHooks, hook)
+			hooks[tplEvent] = curHooks
 		}
 
-		hooks[tplEvent] = curHooks
-	}
+		// Prepare the post-processors
+		postProcessors := make([][]coreBuildPostProcessor, 0, len(t.PostProcessors))
+		for _, rawPPs := range t.PostProcessors {
+			current := make([]coreBuildPostProcessor, len(rawPPs))
+			for i, rawPP := range rawPPs {
+				pp, err := components.PostProcessor(rawPP.Type)
+				if err != nil {
+					return nil, err
+				}
 
-	// Prepare the post-processors
-	postProcessors := make([][]coreBuildPostProcessor, 0, len(t.PostProcessors))
-	for _, rawPPs := range t.PostProcessors {
-		current := make([]coreBuildPostProcessor, len(rawPPs))
-		for i, rawPP := range rawPPs {
-			pp, err := components.PostProcessor(rawPP.Type)
-			if err != nil {
-				return nil, err
-			}
+				if pp == nil {
+					return nil, fmt.Errorf("PostProcessor type not found: %s", rawPP.Type)
+				}
 
-			if pp == nil {
-				return nil, fmt.Errorf("PostProcessor type not found: %s", rawPP.Type)
+				current[i] = coreBuildPostProcessor{
+					processor:         pp,
+					processorType:     rawPP.Type,
+					config:            rawPP.rawConfig,
+					keepInputArtifact: rawPP.KeepInputArtifact,
+				}
 			}
 
-			current[i] = coreBuildPostProcessor{
-				processor:         pp,
-				processorType:     rawPP.Type,
-				config:            rawPP.rawConfig,
-				keepInputArtifact: rawPP.KeepInputArtifact,
-			}
+			postProcessors = append(postProcessors, current)
 		}
 
-		postProcessors = append(postProcessors, current)
-	}
-
-	// Prepare the provisioners
-	provisioners := make([]coreBuildProvisioner, 0, len(t.Provisioners))
-	for _, rawProvisioner := range t.Provisioners {
-		var provisioner Provisioner
-		provisioner, err = components.Provisioner(rawProvisioner.Type)
-		if err != nil {
-			return
-		}
+		// Prepare the provisioners
+		provisioners := make([]coreBuildProvisioner, 0, len(t.Provisioners))
+		for _, rawProvisioner := range t.Provisioners {
+			var provisioner Provisioner
+			provisioner, err = components.Provisioner(rawProvisioner.Type)
+			if err != nil {
+				return
+			}
 
-		if provisioner == nil {
-			err = fmt.Errorf("Provisioner type not found: %s", rawProvisioner.Type)
-			return
-		}
+			if provisioner == nil {
+				err = fmt.Errorf("Provisioner type not found: %s", rawProvisioner.Type)
+				return
+			}
 
-		configs := make([]interface{}, 1, 2)
-		configs[0] = rawProvisioner.rawConfig
+			configs := make([]interface{}, 1, 2)
+			configs[0] = rawProvisioner.rawConfig
 
-		if rawProvisioner.Override != nil {
-			if override, ok := rawProvisioner.Override[name]; ok {
-				configs = append(configs, override)
+			if rawProvisioner.Override != nil {
+				if override, ok := rawProvisioner.Override[name]; ok {
+					configs = append(configs, override)
+				}
 			}
+
+			coreProv := coreBuildProvisioner{provisioner, configs}
+			provisioners = append(provisioners, coreProv)
 		}
 
-		coreProv := coreBuildProvisioner{provisioner, configs}
-		provisioners = append(provisioners, coreProv)
-	}
+		b = &coreBuild{
+			name:           name,
+			builder:        builder,
+			builderConfig:  builderConfig.rawConfig,
+			builderType:    builderConfig.Type,
+			hooks:          hooks,
+			postProcessors: postProcessors,
+			provisioners:   provisioners,
+		}
 
-	b = &coreBuild{
-		name:           name,
-		builder:        builder,
-		builderConfig:  builderConfig.rawConfig,
-		builderType:    builderConfig.Type,
-		hooks:          hooks,
-		postProcessors: postProcessors,
-		provisioners:   provisioners,
+		return
 	}
-
-	return
-}