Functional options, Optional seconds, Overridable parser

1. Allow specifying the Parser that Cron uses for interpreting job specs.
2. Implement functional options for Location, Parser, and ErrorLogger
3. Implement optional seconds (again)
4. Update errors to follow the recommended style of starting with lowercase

Implementation notes:

- Centralize the handling of default and optional fields into a
  single "normalizeFields" function and add a test suite for it.

- Remove the NewWithLocation constructor since it is superseded

- Remove the defaultParser which accepts seconds. Update tests to use a custom
  parser instead.

- Update docs to reflect these changes.

README.md

@@ -3,24 +3,38 @@
 
 # cron
 
-Documentation here: https://godoc.org/github.com/robfig/cron
-
 ## DRAFT - Upgrading to v3
 
 cron v3 is a major upgrade to the library that addresses all outstanding bugs,
 feature requests, and clarifications around usage. It is based on a merge of
-master (containing various fixes) and the v2 branch (containing a couple new
-features), with the addition of Go Modules support. It is currently in
-development.
+master which contains various fixes to issues found over the years and the v2
+branch which contains some backwards-incompatible features like removing cron
+jobs. In addition, it adds support for Go Modules and cleans up rough edges like
+the timezone support.
+
+It is in development and will be considered released once a 3.0 version is
+tagged. It is backwards incompatible with both the v1 and v2 branches.
 
-These are the updates required:
+Updates required:
 
 - The v1 branch accepted an optional seconds field at the beginning of the cron
   spec. This is non-standard and has led to a lot of confusion. The new default
-  parser conforms to the standard as described by
-  [the Cron wikipedia page]. This behavior is not currently supported in v3.
+  parser conforms to the standard as described by [the Cron wikipedia page].
+
+  UPDATING: To retain the old behavior, construct your Cron with a custom
+  parser:
+
+      cron.New(
+          cron.WithParser(
+              cron.SecondOptional | cron.Hour | cron.Dom | cron.Month | cron.Dow | cron.Descriptor))
+
+- The Cron type now accepts functional options on construction rather than the
+  ad-hoc behavior modification mechanisms before (setting a field, calling a setter).
+
+  UPDATING: Code that sets Cron.ErrorLogger or calls Cron.SetLocation must be
+  updated to provide those values on construction.
 
-### Cron spec format
+### Background - Cron spec format
 
 There are two cron spec formats in common usage:
 

cron.go

@@ -2,6 +2,7 @@ package cron
 
 import (
 	"log"
+	"os"
 	"runtime"
 	"sort"
 	"time"
@@ -17,8 +18,9 @@ type Cron struct {
 	remove   chan EntryID
 	snapshot chan []Entry
 	running  bool
-	ErrorLog *log.Logger
+	logger   *log.Logger
 	location *time.Location
+	parser   Parser
 	nextID   EntryID
 }
 
@@ -79,25 +81,39 @@ func (s byTime) Less(i, j int) bool {
 	return s[i].Next.Before(s[j].Next)
 }
 
-// New returns a new Cron job runner.
-// Jobs added to this cron are interpreted in the Local time zone by default.
-func New() *Cron {
-	return NewWithLocation(time.Local)
-}
-
-// NewWithLocation returns a new Cron job runner in the given time zone.
-// Jobs added to this cron are interpreted in this time zone unless overridden.
-func NewWithLocation(location *time.Location) *Cron {
-	return &Cron{
+// New returns a new Cron job runner, modified by the given options.
+//
+// Available Settings
+//
+//   Time Zone
+//     Description: The time zone in which schedules are interpreted
+//     Default:     time.Local
+//
+//   PanicLogger
+//     Description: How to log Jobs that panic
+//     Default:     Log the panic to os.Stderr
+//
+//   Parser
+//     Description:
+//     Default:     Parser that accepts the spec described here: https://en.wikipedia.org/wiki/Cron
+//
+// See "cron.With*" to modify the default behavior.
+func New(opts ...Option) *Cron {
+	c := &Cron{
 		entries:  nil,
 		add:      make(chan *Entry),
 		stop:     make(chan struct{}),
 		snapshot: make(chan []Entry),
 		remove:   make(chan EntryID),
 		running:  false,
-		ErrorLog: nil,
-		location: location,
+		logger:   log.New(os.Stderr, "", log.LstdFlags),
+		location: time.Local,
+		parser:   standardParser,
+	}
+	for _, opt := range opts {
+		opt(c)
 	}
+	return c
 }
 
 // FuncJob is a wrapper that turns a func() into a cron.Job
@@ -116,7 +132,7 @@ func (c *Cron) AddFunc(spec string, cmd func()) (EntryID, error) {
 // The spec is parsed using the time zone of this Cron instance as the default.
 // An opaque ID is returned that can be used to later remove it.
 func (c *Cron) AddJob(spec string, cmd Job) (EntryID, error) {
-	schedule, err := Parse(spec)
+	schedule, err := c.parser.Parse(spec)
 	if err != nil {
 		return 0, err
 	}
@@ -172,7 +188,7 @@ func (c *Cron) Remove(id EntryID) {
 	}
 }
 
-// Start the cron scheduler in its own go-routine, or no-op if already started.
+// Start the cron scheduler in its own goroutine, or no-op if already started.
 func (c *Cron) Start() {
 	if c.running {
 		return
@@ -269,11 +285,7 @@ func (c *Cron) now() time.Time {
 
 // Logs an error to stderr or to the configured error log
 func (c *Cron) logf(format string, args ...interface{}) {
-	if c.ErrorLog != nil {
-		c.ErrorLog.Printf(format, args...)
-	} else {
-		log.Printf(format, args...)
-	}
+	c.logger.Printf(format, args...)
 }
 
 // Stop stops the cron scheduler if it is running; otherwise it does nothing.

cron_test.go

@@ -1,9 +1,12 @@
 package cron
 
 import (
+	"bytes"
 	"fmt"
+	"log"
 	"sync"
 	"testing"
+	"strings"
 	"time"
 )
 
@@ -12,14 +15,24 @@ import (
 // compensate for a few milliseconds of runtime.
 const OneSecond = 1*time.Second + 10*time.Millisecond
 
+func newBufLogger(buf *bytes.Buffer) *log.Logger {
+	return log.New(buf, "", log.LstdFlags)
+}
+
 func TestFuncPanicRecovery(t *testing.T) {
-	cron := New()
+	var buf bytes.Buffer
+	cron := New(WithParser(secondParser), WithPanicLogger(newBufLogger(&buf)))
 	cron.Start()
 	defer cron.Stop()
-	cron.AddFunc("* * * * * ?", func() { panic("YOLO") })
+	cron.AddFunc("* * * * * ?", func() {
+		panic("YOLO")
+	})
 
 	select {
 	case <-time.After(OneSecond):
+		if !strings.Contains(buf.String(), "YOLO") {
+			t.Error("expected a panic to be logged, got none")
+		}
 		return
 	}
 }
@@ -33,20 +46,24 @@ func (d DummyJob) Run() {
 func TestJobPanicRecovery(t *testing.T) {
 	var job DummyJob
 
-	cron := New()
+	var buf bytes.Buffer
+	cron := New(WithParser(secondParser), WithPanicLogger(newBufLogger(&buf)))
 	cron.Start()
 	defer cron.Stop()
 	cron.AddJob("* * * * * ?", job)
 
 	select {
 	case <-time.After(OneSecond):
+		if !strings.Contains(buf.String(), "YOLO") {
+			t.Error("expected a panic to be logged, got none")
+		}
 		return
 	}
 }
 
 // Start and stop cron with no entries.
 func TestNoEntries(t *testing.T) {
-	cron := New()
+	cron := newWithSeconds()
 	cron.Start()
 
 	select {
@@ -61,7 +78,7 @@ func TestStopCausesJobsToNotRun(t *testing.T) {
 	wg := &sync.WaitGroup{}
 	wg.Add(1)
 
-	cron := New()
+	cron := newWithSeconds()
 	cron.Start()
 	cron.Stop()
 	cron.AddFunc("* * * * * ?", func() { wg.Done() })
@@ -79,7 +96,7 @@ func TestAddBeforeRunning(t *testing.T) {
 	wg := &sync.WaitGroup{}
 	wg.Add(1)
 
-	cron := New()
+	cron := newWithSeconds()
 	cron.AddFunc("* * * * * ?", func() { wg.Done() })
 	cron.Start()
 	defer cron.Stop()
@@ -97,7 +114,7 @@ func TestAddWhileRunning(t *testing.T) {
 	wg := &sync.WaitGroup{}
 	wg.Add(1)
 
-	cron := New()
+	cron := newWithSeconds()
 	cron.Start()
 	defer cron.Stop()
 	cron.AddFunc("* * * * * ?", func() { wg.Done() })
@@ -111,7 +128,7 @@ func TestAddWhileRunning(t *testing.T) {
 
 // Test for #34. Adding a job after calling start results in multiple job invocations
 func TestAddWhileRunningWithDelay(t *testing.T) {
-	cron := New()
+	cron := newWithSeconds()
 	cron.Start()
 	defer cron.Stop()
 	time.Sleep(5 * time.Second)
@@ -129,7 +146,7 @@ func TestRemoveBeforeRunning(t *testing.T) {
 	wg := &sync.WaitGroup{}
 	wg.Add(1)
 
-	cron := New()
+	cron := newWithSeconds()
 	id, _ := cron.AddFunc("* * * * * ?", func() { wg.Done() })
 	cron.Remove(id)
 	cron.Start()
@@ -148,7 +165,7 @@ func TestRemoveWhileRunning(t *testing.T) {
 	wg := &sync.WaitGroup{}
 	wg.Add(1)
 
-	cron := New()
+	cron := newWithSeconds()
 	cron.Start()
 	defer cron.Stop()
 	id, _ := cron.AddFunc("* * * * * ?", func() { wg.Done() })
@@ -193,7 +210,7 @@ func TestMultipleEntries(t *testing.T) {
 	wg := &sync.WaitGroup{}
 	wg.Add(2)
 
-	cron := New()
+	cron := newWithSeconds()
 	cron.AddFunc("0 0 0 1 1 ?", func() {})
 	cron.AddFunc("* * * * * ?", func() { wg.Done() })
 	id1, _ := cron.AddFunc("* * * * * ?", func() { t.Fatal() })
@@ -218,7 +235,7 @@ func TestRunningJobTwice(t *testing.T) {
 	wg := &sync.WaitGroup{}
 	wg.Add(2)
 
-	cron := New()
+	cron := newWithSeconds()
 	cron.AddFunc("0 0 0 1 1 ?", func() {})
 	cron.AddFunc("0 0 0 31 12 ?", func() {})
 	cron.AddFunc("* * * * * ?", func() { wg.Done() })
@@ -237,7 +254,7 @@ func TestRunningMultipleSchedules(t *testing.T) {
 	wg := &sync.WaitGroup{}
 	wg.Add(2)
 
-	cron := New()
+	cron := newWithSeconds()
 	cron.AddFunc("0 0 0 1 1 ?", func() {})
 	cron.AddFunc("0 0 0 31 12 ?", func() {})
 	cron.AddFunc("* * * * * ?", func() { wg.Done() })
@@ -264,7 +281,7 @@ func TestLocalTimezone(t *testing.T) {
 	spec := fmt.Sprintf("%d,%d %d %d %d %d ?",
 		now.Second()+1, now.Second()+2, now.Minute(), now.Hour(), now.Day(), now.Month())
 
-	cron := New()
+	cron := newWithSeconds()
 	cron.AddFunc(spec, func() { wg.Done() })
 	cron.Start()
 	defer cron.Stop()
@@ -291,7 +308,7 @@ func TestNonLocalTimezone(t *testing.T) {
 	spec := fmt.Sprintf("%d,%d %d %d %d %d ?",
 		now.Second()+1, now.Second()+2, now.Minute(), now.Hour(), now.Day(), now.Month())
 
-	cron := NewWithLocation(loc)
+	cron := New(WithLocation(loc), WithParser(secondParser))
 	cron.AddFunc(spec, func() { wg.Done() })
 	cron.Start()
 	defer cron.Stop()
@@ -333,7 +350,7 @@ func TestBlockingRun(t *testing.T) {
 	wg := &sync.WaitGroup{}
 	wg.Add(1)
 
-	cron := New()
+	cron := newWithSeconds()
 	cron.AddFunc("* * * * * ?", func() { wg.Done() })
 
 	var unblockChan = make(chan struct{})
@@ -357,7 +374,7 @@ func TestBlockingRun(t *testing.T) {
 func TestStartNoop(t *testing.T) {
 	var tickChan = make(chan struct{}, 2)
 
-	cron := New()
+	cron := newWithSeconds()
 	cron.AddFunc("* * * * * ?", func() {
 		tickChan <- struct{}{}
 	})
@@ -385,7 +402,7 @@ func TestJob(t *testing.T) {
 	wg := &sync.WaitGroup{}
 	wg.Add(1)
 
-	cron := New()
+	cron := newWithSeconds()
 	cron.AddJob("0 0 0 30 Feb ?", testJob{wg, "job0"})
 	cron.AddJob("0 0 0 1 1 ?", testJob{wg, "job1"})
 	cron.AddJob("* * * * * ?", testJob{wg, "job2"})
@@ -425,7 +442,7 @@ func (*ZeroSchedule) Next(time.Time) time.Time {
 
 // Tests that job without time does not run
 func TestJobWithZeroTimeDoesNotRun(t *testing.T) {
-	cron := New()
+	cron := newWithSeconds()
 	calls := 0
 	cron.AddFunc("* * * * * *", func() { calls += 1 })
 	cron.Schedule(new(ZeroSchedule), FuncJob(func() { t.Error("expected zero task will not run") }))
@@ -454,3 +471,8 @@ func stop(cron *Cron) chan bool {
 	}()
 	return ch
 }
+
+// newWithSeconds returns a Cron with the seconds field enabled.
+func newWithSeconds() *Cron {
+	return New(WithParser(secondParser))
+}

doc.go

@@ -44,8 +44,12 @@ The specific interpretation of the format is based on [the Cron Wikipedia page].
 
 Alternative Formats
 
-Alternative Cron expression formats (like [Quartz]) support other fields (like
-seconds). You can implement that by [creating a custom Parser](#NewParser).
+Alternative Cron expression formats (like [Quartz]) support other fields like
+seconds. You can implement that by [creating a custom Parser](#NewParser), e.g.
+
+      cron.New(
+          cron.WithParser(
+              cron.SecondOptional | cron.Hour | cron.Dom | cron.Month | cron.Dow | cron.Descriptor))
 
 [Quartz]: http://www.quartz-scheduler.org/documentation/quartz-2.x/tutorials/crontrigger.html
 
@@ -114,7 +118,10 @@ Time zones
 
 By default, all interpretation and scheduling is done in the machine's local
 time zone ([time.Local](https://golang.org/pkg/time/#Location)). You can change
-that default using [Cron.SetLocation](#Cron.SetLocation).
+that default using the [WithLocation](#WithLocation) option:
+
+      cron.New(
+          cron.WithLocation(time.UTC))
 
 Individual cron schedules may also override the time zone they are to be
 interpreted in by providing an additional space-separated field at the beginning
@@ -126,15 +133,15 @@ For example:
 	cron.New().AddFunc("0 6 * * ?", ...)
 
 	# Runs at 6am in America/New_York
-	c := cron.New()
-	c.SetLocation("America/New_York")
+	nyc, _ := time.LoadLocation("America/New_York")
+	c := cron.New(cron.WithLocation(nyc))
 	c.AddFunc("0 6 * * ?", ...)
 
 	# Runs at 6am in Asia/Tokyo
 	cron.New().AddFunc("TZ=Asia/Tokyo 0 6 * * ?", ...)
 
 	# Runs at 6am in Asia/Tokyo
-	c := cron.New()
+	c := cron.New(cron.WithLocation(nyc))
 	c.SetLocation("America/New_York")
 	c.AddFunc("TZ=Asia/Tokyo 0 6 * * ?", ...)
 

option.go

@@ -0,0 +1,30 @@
+package cron
+
+import (
+	"log"
+	"time"
+)
+
+// Option represents a modification to the default behavior of a Cron.
+type Option func(*Cron)
+
+// WithLocation overrides the timezone of the cron instance.
+func WithLocation(loc *time.Location) Option {
+	return func(c *Cron) {
+		c.location = loc
+	}
+}
+
+// WithParser overrides the parser used for interpreting job schedules.
+func WithParser(p Parser) Option {
+	return func(c *Cron) {
+		c.parser = p
+	}
+}
+
+// WithPanicLogger overrides the logger used for logging job panics.
+func WithPanicLogger(l *log.Logger) Option {
+	return func(c *Cron) {
+		c.logger = l
+	}
+}

option_test.go

@@ -0,0 +1,32 @@
+package cron
+
+import (
+	"bytes"
+	"log"
+	"testing"
+	"time"
+)
+
+func TestWithLocation(t *testing.T) {
+	c := New(WithLocation(time.UTC))
+	if c.location != time.UTC {
+		t.Errorf("expected UTC, got %v", c.location)
+	}
+}
+
+func TestWithParser(t *testing.T) {
+	var parser = NewParser(Dow)
+	c := New(WithParser(parser))
+	if c.parser != parser {
+		t.Error("expected provided parser")
+	}
+}
+
+func TestWithPanicLogger(t *testing.T) {
+	var b bytes.Buffer
+	var logger = log.New(&b, "", log.LstdFlags)
+	c := New(WithPanicLogger(logger))
+	if c.logger != logger {
+		t.Error("expected provided logger")
+	}
+}

parser.go

@@ -15,14 +15,15 @@ import (
 type ParseOption int
 
 const (
-	Second      ParseOption = 1 << iota // Seconds field, default 0
-	Minute                              // Minutes field, default 0
-	Hour                                // Hours field, default 0
-	Dom                                 // Day of month field, default *
-	Month                               // Month field, default *
-	Dow                                 // Day of week field, default *
-	DowOptional                         // Optional day of week field, default *
-	Descriptor                          // Allow descriptors such as @monthly, @weekly, etc.
+	Second         ParseOption = 1 << iota // Seconds field, default 0
+	SecondOptional                         // Optional seconds field, default 0
+	Minute                                 // Minutes field, default 0
+	Hour                                   // Hours field, default 0
+	Dom                                    // Day of month field, default *
+	Month                                  // Month field, default *
+	Dow                                    // Day of week field, default *
+	DowOptional                            // Optional day of week field, default *
+	Descriptor                             // Allow descriptors such as @monthly, @weekly, etc.
 )
 
 var places = []ParseOption{
@@ -45,11 +46,15 @@ var defaults = []string{
 
 // A custom Parser that can be configured.
 type Parser struct {
-	options   ParseOption
-	optionals int
+	options ParseOption
 }
 
-// Creates a custom Parser with custom options.
+// NewParser creates a Parser with custom options.
+//
+// It panics if more than one Optional is given, since it would be impossible to
+// correctly infer which optional is provided or missing in general.
+//
+// Examples
 //
 //  // Standard parser without descriptors
 //  specParser := NewParser(Minute | Hour | Dom | Month | Dow)
@@ -66,10 +71,15 @@ type Parser struct {
 func NewParser(options ParseOption) Parser {
 	optionals := 0
 	if options&DowOptional > 0 {
-		options |= Dow
 		optionals++
 	}
-	return Parser{options, optionals}
+	if options&SecondOptional > 0 {
+		optionals++
+	}
+	if optionals > 1 {
+		panic("multiple optionals may not be configured")
+	}
+	return Parser{options}
 }
 
 // Parse returns a new crontab schedule representing the given spec.
@@ -77,7 +87,7 @@ func NewParser(options ParseOption) Parser {
 // It accepts crontab specs and features configured by NewParser.
 func (p Parser) Parse(spec string) (Schedule, error) {
 	if len(spec) == 0 {
-		return nil, fmt.Errorf("Empty spec string")
+		return nil, fmt.Errorf("empty spec string")
 	}
 
 	// Extract timezone if present
@@ -86,7 +96,7 @@ func (p Parser) Parse(spec string) (Schedule, error) {
 		var err error
 		i := strings.Index(spec, " ")
 		if loc, err = time.LoadLocation(spec[3:i]); err != nil {
-			return nil, fmt.Errorf("Provided bad location %s: %v", spec[3:i], err)
+			return nil, fmt.Errorf("provided bad location %s: %v", spec[3:i], err)
 		}
 		spec = strings.TrimSpace(spec[i:])
 	}
@@ -96,30 +106,16 @@ func (p Parser) Parse(spec string) (Schedule, error) {
 		return parseDescriptor(spec, loc)
 	}
 
-	// Figure out how many fields we need
-	max := 0
-	for _, place := range places {
-		if p.options&place > 0 {
-			max++
-		}
-	}
-	min := max - p.optionals
-
 	// Split on whitespace.
 	fields := strings.Fields(spec)
 
-	// Validate number of fields
-	if count := len(fields); count < min || count > max {
-		if min == max {
-			return nil, fmt.Errorf("Expected exactly %d fields, found %d: %s", min, count, spec)
-		}
-		return nil, fmt.Errorf("Expected %d to %d fields, found %d: %s", min, max, count, spec)
+	// Validate & fill in any omitted or optional fields
+	var err error
+	fields, err = normalizeFields(fields, p.options)
+	if err != nil {
+		return nil, err
 	}
 
-	// Fill in missing fields
-	fields = expandFields(fields, p.options)
-
-	var err error
 	field := func(field string, r bounds) uint64 {
 		if err != nil {
 			return 0
@@ -148,25 +144,98 @@ func (p Parser) Parse(spec string) (Schedule, error) {
 		Dom:      dayofmonth,
 		Month:    month,
 		Dow:      dayofweek,
-                Location: loc,
+		Location: loc,
 	}, nil
 }
 
-func expandFields(fields []string, options ParseOption) []string {
+// normalizeFields takes a subset set of the time fields and returns the full set
+// with defaults (zeroes) populated for unset fields.
+//
+// As part of performing this function, it also validates that the provided
+// fields are compatible with the configured options.
+func normalizeFields(fields []string, options ParseOption) ([]string, error) {
+	// Validate optionals & add their field to options
+	optionals := 0
+	if options&SecondOptional > 0 {
+		options |= Second
+		optionals++
+	}
+	if options&DowOptional > 0 {
+		options |= Dow
+		optionals++
+	}
+	if optionals > 1 {
+		return nil, fmt.Errorf("multiple optionals may not be configured")
+	}
+
+	// Figure out how many fields we need
+	max := 0
+	for _, place := range places {
+		if options&place > 0 {
+			max++
+		}
+	}
+	min := max - optionals
+
+	// Validate number of fields
+	if count := len(fields); count < min || count > max {
+		if min == max {
+			return nil, fmt.Errorf("expected exactly %d fields, found %d: %s", min, count, fields)
+		}
+		return nil, fmt.Errorf("expected %d to %d fields, found %d: %s", min, max, count, fields)
+	}
+
+	// Populate the optional field if not provided
+	if min < max && len(fields) == min {
+		switch {
+		case options&DowOptional > 0:
+			fields = append(fields, defaults[5]) // TODO: improve access to default
+		case options&SecondOptional > 0:
+			fields = append([]string{defaults[0]}, fields...)
+		default:
+			return nil, fmt.Errorf("unknown optional field")
+		}
+	}
+
+	// Populate all fields not part of options with their defaults
 	n := 0
-	count := len(fields)
-	expFields := make([]string, len(places))
-	copy(expFields, defaults)
+	expandedFields := make([]string, len(places))
+	copy(expandedFields, defaults)
 	for i, place := range places {
 		if options&place > 0 {
-			expFields[i] = fields[n]
+			expandedFields[i] = fields[n]
 			n++
 		}
-		if n == count {
-			break
+	}
+	return expandedFields, nil
+}
+
+// expandOptionalFields returns fields with any optional fields added in at
+// their default value, if not provided.
+//
+// It panics if the input does not fulfill the following precondition:
+//   1. (# options fields) - (1 optional field) <= len(fields) <= (# options fields)
+//   2. Any optional fields have had their field added.
+//      For example, options&SecondOptional implies options&Second)
+func expandOptionalFields(fields []string, options ParseOption) []string {
+	expectedFields := 0
+	for _, place := range places {
+		if options&place > 0 {
+			expectedFields++
+		}
+	}
+	switch {
+	case len(fields) == expectedFields:
+		return fields
+	case len(fields) == expectedFields-1:
+		switch {
+		case options&DowOptional > 0:
+			return append(fields, defaults[5]) // TODO: improve access to default
+		case options&SecondOptional > 0:
+			return append([]string{defaults[0]}, fields...)
 		}
 	}
-	return expFields
+	panic(fmt.Errorf("expected %d fields, got %d", expectedFields, len(fields)))
 }
 
 var standardParser = NewParser(
@@ -185,20 +254,6 @@ func ParseStandard(standardSpec string) (Schedule, error) {
 	return standardParser.Parse(standardSpec)
 }
 
-var defaultParser = NewParser(
-	Second | Minute | Hour | Dom | Month | DowOptional | Descriptor,
-)
-
-// Parse returns a new crontab schedule representing the given spec.
-// It returns a descriptive error if the spec is not valid.
-//
-// It accepts
-//   - Full crontab specs, e.g. "* * * * * ?"
-//   - Descriptors, e.g. "@midnight", "@every 1h30m"
-func Parse(spec string) (Schedule, error) {
-	return defaultParser.Parse(spec)
-}
-
 // getField returns an Int with the bits set representing all of the times that
 // the field represents or error parsing field value.  A "field" is a comma-separated
 // list of "ranges".
@@ -246,7 +301,7 @@ func getRange(expr string, r bounds) (uint64, error) {
 				return 0, err
 			}
 		default:
-			return 0, fmt.Errorf("Too many hyphens: %s", expr)
+			return 0, fmt.Errorf("too many hyphens: %s", expr)
 		}
 	}
 
@@ -264,20 +319,20 @@ func getRange(expr string, r bounds) (uint64, error) {
 			end = r.max
 		}
 	default:
-		return 0, fmt.Errorf("Too many slashes: %s", expr)
+		return 0, fmt.Errorf("too many slashes: %s", expr)
 	}
 
 	if start < r.min {
-		return 0, fmt.Errorf("Beginning of range (%d) below minimum (%d): %s", start, r.min, expr)
+		return 0, fmt.Errorf("beginning of range (%d) below minimum (%d): %s", start, r.min, expr)
 	}
 	if end > r.max {
-		return 0, fmt.Errorf("End of range (%d) above maximum (%d): %s", end, r.max, expr)
+		return 0, fmt.Errorf("end of range (%d) above maximum (%d): %s", end, r.max, expr)
 	}
 	if start > end {
-		return 0, fmt.Errorf("Beginning of range (%d) beyond end of range (%d): %s", start, end, expr)
+		return 0, fmt.Errorf("beginning of range (%d) beyond end of range (%d): %s", start, end, expr)
 	}
 	if step == 0 {
-		return 0, fmt.Errorf("Step of range should be a positive number: %s", expr)
+		return 0, fmt.Errorf("step of range should be a positive number: %s", expr)
 	}
 
 	return getBits(start, end, step) | extra, nil
@@ -297,10 +352,10 @@ func parseIntOrName(expr string, names map[string]uint) (uint, error) {
 func mustParseInt(expr string) (uint, error) {
 	num, err := strconv.Atoi(expr)
 	if err != nil {
-		return 0, fmt.Errorf("Failed to parse int from %s: %s", expr, err)
+		return 0, fmt.Errorf("failed to parse int from %s: %s", expr, err)
 	}
 	if num < 0 {
-		return 0, fmt.Errorf("Negative number (%d) not allowed: %s", num, expr)
+		return 0, fmt.Errorf("negative number (%d) not allowed: %s", num, expr)
 	}
 
 	return uint(num), nil
@@ -391,10 +446,10 @@ func parseDescriptor(descriptor string, loc *time.Location) (Schedule, error) {
 	if strings.HasPrefix(descriptor, every) {
 		duration, err := time.ParseDuration(descriptor[len(every):])
 		if err != nil {
-			return nil, fmt.Errorf("Failed to parse duration %s: %s", descriptor, err)
+			return nil, fmt.Errorf("failed to parse duration %s: %s", descriptor, err)
 		}
 		return Every(duration), nil
 	}
 
-	return nil, fmt.Errorf("Unrecognized descriptor: %s", descriptor)
+	return nil, fmt.Errorf("unrecognized descriptor: %s", descriptor)
 }

parser_test.go

@@ -7,6 +7,8 @@ import (
 	"time"
 )
 
+var secondParser = NewParser(Second | Minute | Hour | Dom | Month | DowOptional | Descriptor)
+
 func TestRange(t *testing.T) {
 	zero := uint64(0)
 	ranges := []struct {
@@ -30,11 +32,11 @@ func TestRange(t *testing.T) {
 		{"*", 1, 3, 1<<1 | 1<<2 | 1<<3 | starBit, ""},
 		{"*/2", 1, 3, 1<<1 | 1<<3 | starBit, ""},
 
-		{"5--5", 0, 0, zero, "Too many hyphens"},
-		{"jan-x", 0, 0, zero, "Failed to parse int from"},
-		{"2-x", 1, 5, zero, "Failed to parse int from"},
-		{"*/-12", 0, 0, zero, "Negative number"},
-		{"*//2", 0, 0, zero, "Too many slashes"},
+		{"5--5", 0, 0, zero, "too many hyphens"},
+		{"jan-x", 0, 0, zero, "failed to parse int from"},
+		{"2-x", 1, 5, zero, "failed to parse int from"},
+		{"*/-12", 0, 0, zero, "negative number"},
+		{"*//2", 0, 0, zero, "too many slashes"},
 		{"1", 3, 5, zero, "below minimum"},
 		{"6", 3, 5, zero, "above maximum"},
 		{"5-3", 3, 5, zero, "beyond end of range"},
@@ -118,14 +120,14 @@ func TestBits(t *testing.T) {
 
 func TestParseScheduleErrors(t *testing.T) {
 	var tests = []struct{ expr, err string }{
-		{"* 5 j * * *", "Failed to parse int from"},
-		{"@every Xm", "Failed to parse duration"},
-		{"@unrecognized", "Unrecognized descriptor"},
-		{"* * * *", "Expected 5 to 6 fields"},
-		{"", "Empty spec string"},
+		{"* 5 j * * *", "failed to parse int from"},
+		{"@every Xm", "failed to parse duration"},
+		{"@unrecognized", "unrecognized descriptor"},
+		{"* * * *", "expected 5 to 6 fields"},
+		{"", "empty spec string"},
 	}
 	for _, c := range tests {
-		actual, err := Parse(c.expr)
+		actual, err := secondParser.Parse(c.expr)
 		if err == nil || !strings.Contains(err.Error(), c.err) {
 			t.Errorf("%s => expected %v, got %v", c.expr, c.err, err)
 		}
@@ -138,23 +140,24 @@ func TestParseScheduleErrors(t *testing.T) {
 func TestParseSchedule(t *testing.T) {
 	tokyo, _ := time.LoadLocation("Asia/Tokyo")
 	entries := []struct {
+		parser   Parser
 		expr     string
 		expected Schedule
 	}{
-		{"0 5 * * * *", every5min(time.Local)},
-                // Relied on the "optional seconds" behavior
-		// {"5 * * * *", every5min(time.Local)},
-		{"TZ=UTC  0 5 * * * *", every5min(time.UTC)},
-		// {"TZ=UTC  5 * * * *", every5min(time.UTC)},
-		{"TZ=Asia/Tokyo 0 5 * * * *", every5min(tokyo)},
-		{"@every 5m", ConstantDelaySchedule{5 * time.Minute}},
-		{"@midnight", midnight(time.Local)},
-		{"TZ=UTC  @midnight", midnight(time.UTC)},
-		{"TZ=Asia/Tokyo @midnight", midnight(tokyo)},
-		{"@yearly", annual(time.Local)},
-		{"@annually", annual(time.Local)},
-		{
-			expr: "* 5 * * * *",
+		{secondParser, "0 5 * * * *", every5min(time.Local)},
+		{standardParser, "5 * * * *", every5min(time.Local)},
+		{secondParser, "TZ=UTC  0 5 * * * *", every5min(time.UTC)},
+		{standardParser, "TZ=UTC  5 * * * *", every5min(time.UTC)},
+		{secondParser, "TZ=Asia/Tokyo 0 5 * * * *", every5min(tokyo)},
+		{secondParser, "@every 5m", ConstantDelaySchedule{5 * time.Minute}},
+		{secondParser, "@midnight", midnight(time.Local)},
+		{secondParser, "TZ=UTC  @midnight", midnight(time.UTC)},
+		{secondParser, "TZ=Asia/Tokyo @midnight", midnight(tokyo)},
+		{secondParser, "@yearly", annual(time.Local)},
+		{secondParser, "@annually", annual(time.Local)},
+		{
+			parser: secondParser,
+			expr:   "* 5 * * * *",
 			expected: &SpecSchedule{
 				Second:   all(seconds),
 				Minute:   1 << 5,
@@ -168,7 +171,29 @@ func TestParseSchedule(t *testing.T) {
 	}
 
 	for _, c := range entries {
-		actual, err := Parse(c.expr)
+		actual, err := c.parser.Parse(c.expr)
+		if err != nil {
+			t.Errorf("%s => unexpected error %v", c.expr, err)
+		}
+		if !reflect.DeepEqual(actual, c.expected) {
+			t.Errorf("%s => expected %b, got %b", c.expr, c.expected, actual)
+		}
+	}
+}
+
+func TestOptionalSecondSchedule(t *testing.T) {
+	parser := NewParser(SecondOptional | Minute | Hour | Dom | Month | Dow | Descriptor)
+	entries := []struct {
+		expr     string
+		expected Schedule
+	}{
+		{"0 5 * * * *", every5min(time.Local)},
+		{"5 5 * * * *", every5min5s(time.Local)},
+		{"5 * * * *", every5min(time.Local)},
+	}
+
+	for _, c := range entries {
+		actual, err := parser.Parse(c.expr)
 		if err != nil {
 			t.Errorf("%s => unexpected error %v", c.expr, err)
 		}
@@ -178,6 +203,115 @@ func TestParseSchedule(t *testing.T) {
 	}
 }
 
+func TestNormalizeFields(t *testing.T) {
+	tests := []struct {
+		name     string
+		input    []string
+		options  ParseOption
+		expected []string
+	}{
+		{
+			"AllFields_NoOptional",
+			[]string{"0", "5", "*", "*", "*", "*"},
+			Second | Minute | Hour | Dom | Month | Dow | Descriptor,
+			[]string{"0", "5", "*", "*", "*", "*"},
+		},
+		{
+			"AllFields_SecondOptional_Provided",
+			[]string{"0", "5", "*", "*", "*", "*"},
+			SecondOptional | Minute | Hour | Dom | Month | Dow | Descriptor,
+			[]string{"0", "5", "*", "*", "*", "*"},
+		},
+		{
+			"AllFields_SecondOptional_NotProvided",
+			[]string{"5", "*", "*", "*", "*"},
+			SecondOptional | Minute | Hour | Dom | Month | Dow | Descriptor,
+			[]string{"0", "5", "*", "*", "*", "*"},
+		},
+		{
+			"SubsetFields_NoOptional",
+			[]string{"5", "15", "*"},
+			Hour | Dom | Month,
+			[]string{"0", "0", "5", "15", "*", "*"},
+		},
+		{
+			"SubsetFields_DowOptional_Provided",
+			[]string{"5", "15", "*", "4"},
+			Hour | Dom | Month | DowOptional,
+			[]string{"0", "0", "5", "15", "*", "4"},
+		},
+		{
+			"SubsetFields_DowOptional_NotProvided",
+			[]string{"5", "15", "*"},
+			Hour | Dom | Month | DowOptional,
+			[]string{"0", "0", "5", "15", "*", "*"},
+		},
+		{
+			"SubsetFields_SecondOptional_NotProvided",
+			[]string{"5", "15", "*"},
+			SecondOptional | Hour | Dom | Month,
+			[]string{"0", "0", "5", "15", "*", "*"},
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			actual, err := normalizeFields(test.input, test.options)
+			if err != nil {
+				t.Errorf("unexpected error: %v", err)
+			}
+			if !reflect.DeepEqual(actual, test.expected) {
+				t.Errorf("expected %v, got %v", test.expected, actual)
+			}
+		})
+	}
+}
+
+func TestNormalizeFields_Errors(t *testing.T) {
+	tests := []struct {
+		name    string
+		input   []string
+		options ParseOption
+		err     string
+	}{
+		{
+			"TwoOptionals",
+			[]string{"0", "5", "*", "*", "*", "*"},
+			SecondOptional | Minute | Hour | Dom | Month | DowOptional,
+			"",
+		},
+		{
+			"TooManyFields",
+			[]string{"0", "5", "*", "*"},
+			SecondOptional | Minute | Hour,
+			"",
+		},
+		{
+			"NoFields",
+			[]string{},
+			SecondOptional | Minute | Hour,
+			"",
+		},
+		{
+			"TooFewFields",
+			[]string{"*"},
+			SecondOptional | Minute | Hour,
+			"",
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			actual, err := normalizeFields(test.input, test.options)
+			if err == nil {
+				t.Errorf("expected an error, got none. results: %v", actual)
+			}
+			if !strings.Contains(err.Error(), test.err) {
+				t.Errorf("expected error %q, got %q", test.err, err.Error())
+			}
+		})
+	}
+}
+
 func TestStandardSpecSchedule(t *testing.T) {
 	entries := []struct {
 		expr     string
@@ -194,11 +328,11 @@ func TestStandardSpecSchedule(t *testing.T) {
 		},
 		{
 			expr: "5 j * * *",
-			err:  "Failed to parse int from",
+			err:  "failed to parse int from",
 		},
 		{
 			expr: "* * * *",
-			err:  "Expected exactly 5 fields",
+			err:  "expected exactly 5 fields",
 		},
 	}
 
@@ -220,6 +354,10 @@ func every5min(loc *time.Location) *SpecSchedule {
 	return &SpecSchedule{1 << 0, 1 << 5, all(hours), all(dom), all(months), all(dow), loc}
 }
 
+func every5min5s(loc *time.Location) *SpecSchedule {
+	return &SpecSchedule{1 << 5, 1 << 5, all(hours), all(dom), all(months), all(dow), loc}
+}
+
 func midnight(loc *time.Location) *SpecSchedule {
 	return &SpecSchedule{1, 1, 1, all(dom), all(months), all(dow), loc}
 }

spec_test.go

@@ -171,7 +171,7 @@ func TestNext(t *testing.T) {
 	}
 
 	for _, c := range runs {
-		sched, err := Parse(c.spec)
+		sched, err := secondParser.Parse(c.spec)
 		if err != nil {
 			t.Error(err)
 			continue