fix #3272: account for IE7 hacks in nested CSS

CHANGELOG.md

@@ -2,6 +2,32 @@
 
 ## Unreleased
 
+* Adjust CSS nesting parser for IE7 hacks ([#3272](https://github.com/evanw/esbuild/issues/3272))
+
+    This fixes a regression with esbuild's treatment of IE7 hacks in CSS. CSS nesting allows selectors to be used where declarations are expected. There's an IE7 hack where prefixing a declaration with a `*` causes that declaration to only be applied in IE7 due to a bug in IE7's CSS parser. However, it's valid for nested CSS selectors to start with `*`. So esbuild was incorrectly parsing these declarations and anything following it up until the next `{` as a selector for a nested CSS rule. This release changes esbuild's parser to terminate the parsing of selectors for nested CSS rules when a `;` is encountered to fix this edge case:
+
+    ```css
+    /* Original code */
+    .item {
+      *width: 100%;
+      height: 1px;
+    }
+
+    /* Old output */
+    .item {
+      *width: 100%; height: 1px; {
+      }
+    }
+
+    /* New output */
+    .item {
+      *width: 100%;
+      height: 1px;
+    }
+    ```
+
+    Note that the syntax for CSS nesting is [about to change again](https://github.com/w3c/csswg-drafts/issues/7961), so esbuild's CSS parser may still not be completely accurate with how browsers do and/or will interpret CSS nesting syntax. Expect additional updates to esbuild's CSS parser in the future to deal with upcoming CSS specification changes.
+
 * Adjust esbuild's warning about undefined imports for TypeScript `import` equals declarations ([#3271](https://github.com/evanw/esbuild/issues/3271))
 
     In JavaScript, accessing a missing property on an import namespace object is supposed to result in a value of `undefined` at run-time instead of an error at compile-time. This is something that esbuild warns you about by default because doing this can indicate a bug with your code. For example:

internal/css_parser/css_parser.go

@@ -446,9 +446,9 @@ loop:
 
 		var rule css_ast.Rule
 		if context.parseSelectors {
-			rule = p.parseSelectorRuleFrom(p.index, context.isTopLevel, parseSelectorOpts{})
+			rule = p.parseSelectorRule(context.isTopLevel, parseSelectorOpts{})
 		} else {
-			rule = p.parseQualifiedRuleFrom(p.index, parseQualifiedRuleOpts{isTopLevel: context.isTopLevel})
+			rule = p.parseQualifiedRule(parseQualifiedRuleOpts{isTopLevel: context.isTopLevel})
 		}
 
 		// Lower CSS nesting if it's not supported (but only at the top level)
@@ -529,7 +529,7 @@ func (p *parser) parseListOfDeclarations(opts listOfDeclarationsOpts) (list []cs
 			css_lexer.TDelimGreaterThan,
 			css_lexer.TDelimTilde:
 			p.shouldLowerNesting = true
-			list = append(list, p.parseSelectorRuleFrom(p.index, false, parseSelectorOpts{isDeclarationContext: true}))
+			list = append(list, p.parseSelectorRule(false, parseSelectorOpts{isDeclarationContext: true}))
 			foundNesting = true
 
 		default:
@@ -1830,11 +1830,12 @@ func mangleNumber(t string) (string, bool) {
 	return t, t != original
 }
 
-func (p *parser) parseSelectorRuleFrom(preludeStart int, isTopLevel bool, opts parseSelectorOpts) css_ast.Rule {
+func (p *parser) parseSelectorRule(isTopLevel bool, opts parseSelectorOpts) css_ast.Rule {
 	// Save and restore the local symbol state in case there are any bare
 	// ":global" or ":local" annotations. The effect of these should be scoped
 	// to within the selector rule.
 	local := p.makeLocalSymbols
+	preludeStart := p.index
 
 	// Try parsing the prelude as a selector list
 	if list, ok := p.parseSelectorList(opts); ok {
@@ -1868,22 +1869,27 @@ func (p *parser) parseSelectorRuleFrom(preludeStart int, isTopLevel bool, opts p
 			return css_ast.Rule{Loc: p.tokens[preludeStart].Range.Loc, Data: &selector}
 		}
 	}
+
 	p.makeLocalSymbols = local
+	p.index = preludeStart
 
 	// Otherwise, parse a generic qualified rule
-	return p.parseQualifiedRuleFrom(preludeStart, parseQualifiedRuleOpts{
-		isAlreadyInvalid: true,
-		isTopLevel:       isTopLevel,
+	return p.parseQualifiedRule(parseQualifiedRuleOpts{
+		isAlreadyInvalid:     true,
+		isTopLevel:           isTopLevel,
+		isDeclarationContext: opts.isDeclarationContext,
 	})
 }
 
 type parseQualifiedRuleOpts struct {
-	isAlreadyInvalid bool
-	isTopLevel       bool
+	isAlreadyInvalid     bool
+	isTopLevel           bool
+	isDeclarationContext bool
 }
 
-func (p *parser) parseQualifiedRuleFrom(preludeStart int, opts parseQualifiedRuleOpts) css_ast.Rule {
-	preludeLoc := p.tokens[preludeStart].Range.Loc
+func (p *parser) parseQualifiedRule(opts parseQualifiedRuleOpts) css_ast.Rule {
+	preludeStart := p.index
+	preludeLoc := p.current().Range.Loc
 
 loop:
 	for {
@@ -1895,11 +1901,16 @@ loop:
 			if !opts.isTopLevel {
 				break loop
 			}
-			p.parseComponentValue()
 
-		default:
-			p.parseComponentValue()
+		case css_lexer.TSemicolon:
+			if opts.isDeclarationContext {
+				return css_ast.Rule{Loc: preludeLoc, Data: &css_ast.RBadDeclaration{
+					Tokens: p.convertTokens(p.tokens[preludeStart:p.index]),
+				}}
+			}
 		}
+
+		p.parseComponentValue()
 	}
 
 	qualified := css_ast.RQualified{

internal/css_parser/css_parser_test.go

@@ -603,7 +603,7 @@ func TestDeclaration(t *testing.T) {
 	// See http://browserhacks.com/
 	expectPrinted(t, ".selector { (;property: value;); }", ".selector {\n  (;property: value;);\n}\n",
 		"<stdin>: WARNING: Expected identifier but found \"(\"\n")
-	expectPrinted(t, ".selector { [;property: value;]; }", ".selector {\n  [;property: value;]; {\n  }\n}\n",
+	expectPrinted(t, ".selector { [;property: value;]; }", ".selector {\n  [;property: value;];\n}\n",
 		"<stdin>: WARNING: Expected identifier but found \";\"\n") // Note: This now overlaps with CSS nesting syntax
 	expectPrinted(t, ".selector, {}", ".selector, {\n}\n", "<stdin>: WARNING: Unexpected \"{\"\n")
 	expectPrinted(t, ".selector\\ {}", ".selector\\  {\n}\n", "")
@@ -1032,6 +1032,7 @@ func TestNestedSelector(t *testing.T) {
 
 func TestBadQualifiedRules(t *testing.T) {
 	expectPrinted(t, "$bad: rule;", "$bad: rule; {\n}\n", "<stdin>: WARNING: Unexpected \"$\"\n")
+	expectPrinted(t, "$bad: rule; div { color: red }", "$bad: rule; div {\n  color: red;\n}\n", "<stdin>: WARNING: Unexpected \"$\"\n")
 	expectPrinted(t, "$bad { color: red }", "$bad {\n  color: red;\n}\n", "<stdin>: WARNING: Unexpected \"$\"\n")
 	expectPrinted(t, "a { div.major { color: blue } color: red }", "a {\n  div.major { color: blue } color: red;\n}\n",
 		"<stdin>: WARNING: A nested style rule cannot start with \"div\" because it looks like the start of a declaration\n"+
@@ -1040,6 +1041,9 @@ func TestBadQualifiedRules(t *testing.T) {
 	expectPrinted(t, "a { div:hover { color: blue }; color: red }", "a {\n  div: hover { color: blue };\n  color: red;\n}\n", "")
 	expectPrinted(t, "a { div:hover { color: blue } ; color: red }", "a {\n  div: hover { color: blue };\n  color: red;\n}\n", "")
 	expectPrinted(t, "! { x: {} }", "! {\n  x: {};\n}\n", "<stdin>: WARNING: Unexpected \"!\"\n")
+	expectPrinted(t, "a { *width: 100%; height: 1px }", "a {\n  *width: 100%;\n  height: 1px;\n}\n", "<stdin>: WARNING: Unexpected \"width\"\n")
+	expectPrinted(t, "a { garbage; height: 1px }", "a {\n  garbage;\n  height: 1px;\n}\n", "<stdin>: WARNING: Expected \":\"\n")
+	expectPrinted(t, "a { !; height: 1px }", "a {\n  !;\n  height: 1px;\n}\n", "<stdin>: WARNING: Expected identifier but found \"!\"\n")
 }
 
 func TestAtRule(t *testing.T) {