This closes qax-os#2259, add value check for prevent using invalid fill type when create styles (qax-os#2260)

- Add new exported 5 error variables ErrFillType, ErrFillGradientColor, ErrFillGradientShading, ErrFillPatternColor and ErrFillPattern
- Update unit tests

Author: ohauer

errors.go

@@ -38,7 +38,8 @@ var (
 	// ErrCoordinates defined the error message on invalid coordinates tuples
 	// length.
 	ErrCoordinates = errors.New("coordinates length must be 4")
-	// ErrCustomNumFmt defined the error message on receive the empty custom number format.
+	// ErrCustomNumFmt defined the error message on receive the empty custom 
+	// number format.
 	ErrCustomNumFmt = errors.New("custom number format can not be empty")
 	// ErrDataValidationFormulaLength defined the error message for receiving a
 	// data validation formula length that exceeds the limit.
@@ -54,8 +55,23 @@ var (
 	ErrDefinedNameScope = errors.New("no defined name on the scope")
 	// ErrExistsSheet defined the error message on given sheet already exists.
 	ErrExistsSheet = errors.New("the same name sheet already exists")
-	// ErrExistsTableName defined the error message on given table already exists.
+	// ErrExistsTableName defined the error message on given table already 
+	// exists.
 	ErrExistsTableName = errors.New("the same name table already exists")
+	// ErrFillType defined the error message on receive an invalid fill type.
+	ErrFillType = errors.New("fill type value must be one of 'gradient' or 'pattern'")
+	// ErrFillGradientColor defined the error message on receive an invalid fill
+	// color for 'gradient' type.
+	ErrFillGradientColor = errors.New("fill color value must be an array of two colors for 'gradient' type")
+	// ErrFillGradientShading defined the error message on receive an invalid 
+	// fill shading for 'gradient' type.
+	ErrFillGradientShading = errors.New("fill shading value must be between 0 and 16 for 'gradient' type")
+	// ErrFillPatternColor defined the error message on receive an invalid fill
+	// color for 'pattern' type.
+	ErrFillPatternColor = errors.New("fill color value must be empty or an array of one color for 'pattern' type")
+	// ErrFillPattern defined the error message on receive an invalid fill 
+	// pattern.
+	ErrFillPattern = errors.New("fill pattern value must be between 0 and 18")
 	// ErrFontLength defined the error message on the length of the font
 	// family name overflow.
 	ErrFontLength = fmt.Errorf("the length of the font family name must be less than or equal to %d", MaxFontFamilyLength)
@@ -78,7 +94,8 @@ var (
 	// ErrMaxRowHeight defined the error message on receive an invalid row
 	// height.
 	ErrMaxRowHeight = fmt.Errorf("the height of the row must be less than or equal to %d points", MaxRowHeight)
-	// ErrMaxRows defined the error message on receive a row number exceeds maximum limit.
+	// ErrMaxRows defined the error message on receive a row number exceeds 
+	// maximum limit.
 	ErrMaxRows = errors.New("row number exceeds maximum limit")
 	// ErrNameLength defined the error message on receiving the defined name or
 	// table name length exceeds the limit.
@@ -169,8 +186,8 @@ var (
 	// ErrUnsupportedHashAlgorithm defined the error message on unsupported
 	// hash algorithm.
 	ErrUnsupportedHashAlgorithm = errors.New("unsupported hash algorithm")
-	// ErrUnsupportedNumberFormat defined the error message on unsupported number format
-	// expression.
+	// ErrUnsupportedNumberFormat defined the error message on unsupported 
+	// number format expression.
 	ErrUnsupportedNumberFormat = errors.New("unsupported number format token")
 	// ErrWorkbookFileFormat defined the error message on receive an
 	// unsupported workbook file format.

excelize_test.go

@@ -970,18 +970,10 @@ func TestSetCellStyleFill(t *testing.T) {
 	assert.NoError(t, err)
 	assert.NoError(t, f.SetCellStyle("Sheet1", "O23", "O23", style))
 
-	style, err = f.NewStyle(&Style{Fill: Fill{Type: "gradient", Color: []string{"FFFFFF"}, Shading: 1}})
-	assert.NoError(t, err)
-	assert.NoError(t, f.SetCellStyle("Sheet1", "O23", "O23", style))
-
 	style, err = f.NewStyle(&Style{Fill: Fill{Type: "pattern", Color: []string{}, Shading: 1}})
 	assert.NoError(t, err)
 	assert.NoError(t, f.SetCellStyle("Sheet1", "O23", "O23", style))
 
-	style, err = f.NewStyle(&Style{Fill: Fill{Type: "pattern", Color: []string{"E0EBF5"}, Pattern: 19}})
-	assert.NoError(t, err)
-	assert.NoError(t, f.SetCellStyle("Sheet1", "O23", "O23", style))
-
 	assert.NoError(t, f.SaveAs(filepath.Join("test", "TestSetCellStyleFill.xlsx")))
 }
 

styles.go

@@ -134,6 +134,25 @@ func parseFormatStyleSet(style *Style) (*Style, error) {
 			return style, ErrFontSize
 		}
 	}
+	switch style.Fill.Type {
+	case "gradient":
+		if len(style.Fill.Color) != 2 {
+			return style, ErrFillGradientColor
+		}
+		if style.Fill.Shading < 0 || style.Fill.Shading > 16 {
+			return style, ErrFillGradientShading
+		}
+	case "pattern":
+		if len(style.Fill.Color) > 1 {
+			return style, ErrFillPatternColor
+		}
+		if style.Fill.Pattern < 0 || style.Fill.Pattern > 18 {
+			return style, ErrFillPattern
+		}
+	case "":
+	default:
+		return style, ErrFillType
+	}
 	if style.CustomNumFmt != nil && len(*style.CustomNumFmt) == 0 {
 		err = ErrCustomNumFmt
 	}
@@ -2054,9 +2073,6 @@ func newFills(style *Style, fg bool) *xlsxFill {
 	var fill xlsxFill
 	switch style.Fill.Type {
 	case "gradient":
-		if len(style.Fill.Color) != 2 || style.Fill.Shading < 0 || style.Fill.Shading > 16 {
-			break
-		}
 		gradient := styleFillVariants()[style.Fill.Shading]
 		gradient.Stop[0].Color.RGB = getPaletteColor(style.Fill.Color[0])
 		gradient.Stop[1].Color.RGB = getPaletteColor(style.Fill.Color[1])
@@ -2065,9 +2081,6 @@ func newFills(style *Style, fg bool) *xlsxFill {
 		}
 		fill.GradientFill = &gradient
 	case "pattern":
-		if style.Fill.Pattern > 18 || style.Fill.Pattern < 0 {
-			break
-		}
 		var pattern xlsxPatternFill
 		pattern.PatternType = styleFillPatterns[style.Fill.Pattern]
 		if len(style.Fill.Color) < 1 {

styles_test.go

@@ -408,12 +408,32 @@ func TestUnsetConditionalFormat(t *testing.T) {
 
 func TestNewStyle(t *testing.T) {
 	f := NewFile()
-	for i := range 18 {
+	for i := range 16 {
 		_, err := f.NewStyle(&Style{
 			Fill: Fill{Type: "gradient", Color: []string{"FFFFFF", "4E71BE"}, Shading: i},
 		})
 		assert.NoError(t, err)
 	}
+	_, err := f.NewStyle(&Style{
+		Fill: Fill{Type: "solid"},
+	})
+	assert.Equal(t, ErrFillType, err)
+	_, err = f.NewStyle(&Style{
+		Fill: Fill{Type: "gradient", Color: []string{"FFFFFF"}, Shading: 0},
+	})
+	assert.Equal(t, ErrFillGradientColor, err)
+	_, err = f.NewStyle(&Style{
+		Fill: Fill{Type: "gradient", Color: []string{"FFFFFF", "4E71BE"}, Shading: 17},
+	})
+	assert.Equal(t, ErrFillGradientShading, err)
+	_, err = f.NewStyle(&Style{
+		Fill: Fill{Type: "pattern", Color: []string{"FFFFFF", "4E71BE"}, Shading: 0},
+	})
+	assert.Equal(t, ErrFillPatternColor, err)
+	_, err = f.NewStyle(&Style{
+		Fill: Fill{Type: "pattern", Pattern: 19},
+	})
+	assert.Equal(t, ErrFillPattern, err)
 	f = NewFile()
 	styleID, err := f.NewStyle(&Style{Font: &Font{Bold: true, Italic: true, Family: "Times New Roman", Size: 36, Color: "777777"}})
 	assert.NoError(t, err)