Skip to content

Commit

Permalink
fix #1821: disable unsafe "calc()" simplification
Browse files Browse the repository at this point in the history
  • Loading branch information
@evanw
evanw committed Dec 3, 2021
1 parent e9340eb commit 4093d0d
Show file tree
Hide file tree
Showing 3 changed files with 131 additions and 44 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Expand Up @@ -20,6 +20,10 @@
x.z();
```

* Disable `calc()` transform for numbers with many fractional digits ([#1821](https://github.com/evanw/esbuild/issues/1821))

Version 0.13.12 introduced simplification of `calc()` expressions in CSS when minifying. For example, `calc(100% / 4)` turns into `25%`. However, this is problematic for numbers with many fractional digits because either the number is printed with reduced precision, which is inaccurate, or the number is printed with full precision, which could be longer than the original expression. For example, turning `calc(100% / 3)` into `33.33333%` is inaccurate and turning it into `33.333333333333336%` likely isn't desired. In this release, minification of `calc()` is now disabled when any number in the result cannot be represented to full precision with at most five fractional digits.

## 0.14.1

* Fix `imports` in `package.json` ([#1807](https://github.com/evanw/esbuild/issues/1807))
Expand Down
21 changes: 21 additions & 0 deletions internal/css_parser/css_parser_test.go
View file
Expand Up @@ -1301,6 +1301,27 @@ func TestMangleCalc(t *testing.T) {
// Using "var()" should bail because it can expand to any number of tokens
expectPrintedMangle(t, "a { b: calc(1px - x + 2px) }", "a {\n b: calc(3px - x);\n}\n")
expectPrintedMangle(t, "a { b: calc(1px - var(x) + 2px) }", "a {\n b: calc(1px - var(x) + 2px);\n}\n")

// Test values that can't be accurately represented as decimals
expectPrintedMangle(t, "a { b: calc(100% / 1) }", "a {\n b: 100%;\n}\n")
expectPrintedMangle(t, "a { b: calc(100% / 2) }", "a {\n b: 50%;\n}\n")
expectPrintedMangle(t, "a { b: calc(100% / 3) }", "a {\n b: calc(100% / 3);\n}\n")
expectPrintedMangle(t, "a { b: calc(100% / 4) }", "a {\n b: 25%;\n}\n")
expectPrintedMangle(t, "a { b: calc(100% / 5) }", "a {\n b: 20%;\n}\n")
expectPrintedMangle(t, "a { b: calc(100% / 6) }", "a {\n b: calc(100% / 6);\n}\n")
expectPrintedMangle(t, "a { b: calc(100% / 7) }", "a {\n b: calc(100% / 7);\n}\n")
expectPrintedMangle(t, "a { b: calc(100% / 8) }", "a {\n b: 12.5%;\n}\n")
expectPrintedMangle(t, "a { b: calc(100% / 9) }", "a {\n b: calc(100% / 9);\n}\n")
expectPrintedMangle(t, "a { b: calc(100% / 10) }", "a {\n b: 10%;\n}\n")
expectPrintedMangle(t, "a { b: calc(100% / 100) }", "a {\n b: 1%;\n}\n")
expectPrintedMangle(t, "a { b: calc(100% / 1000) }", "a {\n b: .1%;\n}\n")
expectPrintedMangle(t, "a { b: calc(100% / 10000) }", "a {\n b: .01%;\n}\n")
expectPrintedMangle(t, "a { b: calc(100% / 100000) }", "a {\n b: .001%;\n}\n")
expectPrintedMangle(t, "a { b: calc(100% / 1000000) }", "a {\n b: calc(100% / 1000000);\n}\n") // This actually ends up as "100% * (1 / 1000000)" which is less precise
expectPrintedMangle(t, "a { b: calc(100% / -1000000) }", "a {\n b: calc(100% / -1000000);\n}\n")
expectPrintedMangle(t, "a { b: calc(100% / -100000) }", "a {\n b: -.001%;\n}\n")
expectPrintedMangle(t, "a { b: calc(3 * (2px + 1em / 7)) }", "a {\n b: calc(3 * (2px + 1em / 7));\n}\n")
expectPrintedMangle(t, "a { b: calc(3 * (2px + 1em / 8)) }", "a {\n b: calc(3 * (2px + .125em));\n}\n")
}

func TestTransform(t *testing.T) {
Expand Down
150 changes: 106 additions & 44 deletions internal/css_parser/css_reduce_calc.go
View file
Expand Up @@ -17,19 +17,20 @@ func (p *parser) tryToReduceCalcExpression(token css_ast.Token) css_ast.Token {
whitespace = 0
}
term = term.partiallySimplify()
result := term.convertToToken(whitespace)
if result.Kind == css_lexer.TOpenParen {
result.Kind = css_lexer.TFunction
result.Text = "calc"
if result, ok := term.convertToToken(whitespace); ok {
if result.Kind == css_lexer.TOpenParen {
result.Kind = css_lexer.TFunction
result.Text = "calc"
}
return result
}
return result
}
return token
}

// See: https://www.w3.org/TR/css-values-4/#calc-internal
type calcTerm interface {
convertToToken(whitespace css_ast.WhitespaceFlags) css_ast.Token
convertToToken(whitespace css_ast.WhitespaceFlags) (css_ast.Token, bool)
partiallySimplify() calcTerm
}

Expand Down Expand Up @@ -59,57 +60,87 @@ type calcValue struct {
isInvalidPlusOrMinus bool
}

func floatToStringForCalc(a float64) string {
func floatToStringForCalc(a float64) (string, bool) {
// Handle non-finite cases
if math.IsNaN(a) {
return "NaN"
return "NaN", true
}
if math.IsInf(a, 1) {
return "Infinity"
return "Infinity", true
}
if math.IsInf(a, -1) {
return "-Infinity"
return "-Infinity", true
}

// Print the number as a string
text := fmt.Sprintf("%.05f", a)
for text[len(text)-1] == '0' {
text = text[:len(text)-1]
}
if text[len(text)-1] == '.' {
text = text[:len(text)-1]
}
return text
if strings.HasPrefix(text, "0.") {
text = text[1:]
} else if strings.HasPrefix(text, "-0.") {
text = "-" + text[2:]
}

// Bail if the number is not exactly represented
if number, err := strconv.ParseFloat(text, 64); err != nil || number != a {
return "", false
}

return text, true
}

func (c *calcSum) convertToToken(whitespace css_ast.WhitespaceFlags) css_ast.Token {
func (c *calcSum) convertToToken(whitespace css_ast.WhitespaceFlags) (css_ast.Token, bool) {
// Specification: https://www.w3.org/TR/css-values-4/#calc-serialize
tokens := make([]css_ast.Token, 0, len(c.terms)*2)

// ALGORITHM DEVIATION: Avoid parenthesizing product nodes inside sum nodes
if product, ok := c.terms[0].(*calcProduct); ok {
tokens = append(tokens, *product.convertToToken(whitespace).Children...)
token, ok := product.convertToToken(whitespace)
if !ok {
return css_ast.Token{}, false
}
tokens = append(tokens, *token.Children...)
} else {
tokens = append(tokens, c.terms[0].convertToToken(whitespace))
token, ok := c.terms[0].convertToToken(whitespace)
if !ok {
return css_ast.Token{}, false
}
tokens = append(tokens, token)
}

for _, term := range c.terms[1:] {
// If child is a Negate node, append " - " to s, then serialize the Negate’s child and append the result to s.
if negate, ok := term.(*calcNegate); ok {
token, ok := negate.term.convertToToken(whitespace)
if !ok {
return css_ast.Token{}, false
}
tokens = append(tokens, css_ast.Token{
Kind: css_lexer.TDelimMinus,
Text: "-",
Whitespace: css_ast.WhitespaceBefore | css_ast.WhitespaceAfter,
}, negate.term.convertToToken(whitespace))
}, token)
continue
}

// If child is a negative numeric value, append " - " to s, then serialize the negation of child as normal and append the result to s.
if numeric, ok := term.(*calcNumeric); ok && numeric.number < 0 {
clone := *numeric
clone.number = -clone.number
token, ok := clone.convertToToken(whitespace)
if !ok {
return css_ast.Token{}, false
}
tokens = append(tokens, css_ast.Token{
Kind: css_lexer.TDelimMinus,
Text: "-",
Whitespace: css_ast.WhitespaceBefore | css_ast.WhitespaceAfter,
}, clone.convertToToken(whitespace))
}, token)
continue
}

Expand All @@ -122,101 +153,132 @@ func (c *calcSum) convertToToken(whitespace css_ast.WhitespaceFlags) css_ast.Tok

// ALGORITHM DEVIATION: Avoid parenthesizing product nodes inside sum nodes
if product, ok := term.(*calcProduct); ok {
tokens = append(tokens, *product.convertToToken(whitespace).Children...)
token, ok := product.convertToToken(whitespace)
if !ok {
return css_ast.Token{}, false
}
tokens = append(tokens, *token.Children...)
} else {
tokens = append(tokens, term.convertToToken(whitespace))
token, ok := term.convertToToken(whitespace)
if !ok {
return css_ast.Token{}, false
}
tokens = append(tokens, token)
}
}

return css_ast.Token{
Kind: css_lexer.TOpenParen,
Text: "(",
Children: &tokens,
}
}, true
}

func (c *calcProduct) convertToToken(whitespace css_ast.WhitespaceFlags) css_ast.Token {
func (c *calcProduct) convertToToken(whitespace css_ast.WhitespaceFlags) (css_ast.Token, bool) {
// Specification: https://www.w3.org/TR/css-values-4/#calc-serialize
tokens := make([]css_ast.Token, 0, len(c.terms)*2)
tokens = append(tokens, c.terms[0].convertToToken(whitespace))
token, ok := c.terms[0].convertToToken(whitespace)
if !ok {
return css_ast.Token{}, false
}
tokens = append(tokens, token)

for _, term := range c.terms[1:] {
// If child is an Invert node, append " / " to s, then serialize the Invert’s child and append the result to s.
if invert, ok := term.(*calcInvert); ok {
token, ok := invert.term.convertToToken(whitespace)
if !ok {
return css_ast.Token{}, false
}
tokens = append(tokens, css_ast.Token{
Kind: css_lexer.TDelimSlash,
Text: "/",
Whitespace: whitespace,
}, invert.term.convertToToken(whitespace))
}, token)
continue
}

// Otherwise, append " * " to s, then serialize child and append the result to s.
token, ok := term.convertToToken(whitespace)
if !ok {
return css_ast.Token{}, false
}
tokens = append(tokens, css_ast.Token{
Kind: css_lexer.TDelimAsterisk,
Text: "*",
Whitespace: whitespace,
}, term.convertToToken(whitespace))
}, token)
}

return css_ast.Token{
Kind: css_lexer.TOpenParen,
Text: "(",
Children: &tokens,
}
}, true
}

func (c *calcNegate) convertToToken(whitespace css_ast.WhitespaceFlags) css_ast.Token {
func (c *calcNegate) convertToToken(whitespace css_ast.WhitespaceFlags) (css_ast.Token, bool) {
// Specification: https://www.w3.org/TR/css-values-4/#calc-serialize
token, ok := c.term.convertToToken(whitespace)
if !ok {
return css_ast.Token{}, false
}
return css_ast.Token{
Kind: css_lexer.TOpenParen,
Text: "(",
Children: &[]css_ast.Token{
{Kind: css_lexer.TNumber, Text: "-1"},
{Kind: css_lexer.TDelimSlash, Text: "*", Whitespace: css_ast.WhitespaceBefore | css_ast.WhitespaceAfter},
c.term.convertToToken(whitespace),
token,
},
}
}, true
}

func (c *calcInvert) convertToToken(whitespace css_ast.WhitespaceFlags) css_ast.Token {
func (c *calcInvert) convertToToken(whitespace css_ast.WhitespaceFlags) (css_ast.Token, bool) {
// Specification: https://www.w3.org/TR/css-values-4/#calc-serialize
token, ok := c.term.convertToToken(whitespace)
if !ok {
return css_ast.Token{}, false
}
return css_ast.Token{
Kind: css_lexer.TOpenParen,
Text: "(",
Children: &[]css_ast.Token{
{Kind: css_lexer.TNumber, Text: "1"},
{Kind: css_lexer.TDelimSlash, Text: "/", Whitespace: css_ast.WhitespaceBefore | css_ast.WhitespaceAfter},
c.term.convertToToken(whitespace),
token,
},
}
}, true
}

func (c *calcNumeric) convertToToken(whitespace css_ast.WhitespaceFlags) css_ast.Token {
func (c *calcNumeric) convertToToken(whitespace css_ast.WhitespaceFlags) (css_ast.Token, bool) {
text, ok := floatToStringForCalc(c.number)
if !ok {
return css_ast.Token{}, false
}
if c.unit == "" {
return css_ast.Token{
Kind: css_lexer.TNumber,
Text: floatToStringForCalc(c.number),
}
Text: text,
}, true
} else if c.unit == "%" {
return css_ast.Token{
Kind: css_lexer.TPercentage,
Text: floatToStringForCalc(c.number) + "%",
}
Text: text + "%",
}, true
} else {
text := floatToStringForCalc(c.number)
return css_ast.Token{
Kind: css_lexer.TDimension,
Text: text + c.unit,
UnitOffset: uint16(len(text)),
}
}, true
}
}

func (c *calcValue) convertToToken(whitespace css_ast.WhitespaceFlags) css_ast.Token {
func (c *calcValue) convertToToken(whitespace css_ast.WhitespaceFlags) (css_ast.Token, bool) {
t := c.token
t.Whitespace = 0
return t
return t, true
}

func (c *calcSum) partiallySimplify() calcTerm {
Expand Down Expand Up @@ -323,11 +385,11 @@ func (c *calcProduct) partiallySimplify() calcTerm {
for i := 1; i < len(terms); i++ {
if numeric, ok := terms[i].(*calcNumeric); ok {
reciprocal := 1 / numeric.number
multiply := floatToStringForCalc(numeric.number)
divide := floatToStringForCalc(reciprocal)
if len(divide) < len(multiply) {
numeric.number = reciprocal
terms[i] = &calcInvert{term: numeric}
if multiply, ok := floatToStringForCalc(numeric.number); ok {
if divide, ok := floatToStringForCalc(reciprocal); ok && len(divide) < len(multiply) {
numeric.number = reciprocal
terms[i] = &calcInvert{term: numeric}
}
}
}
}
Expand Down

0 comments on commit 4093d0d

Please sign in to comment.