fix #1730: crash with legal comment and @import

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## Unreleased
+
+* Fix a crash with a legal comment followed by an import ([#1730](https://github.com/evanw/esbuild/issues/1730))
+
+    Version 0.13.10 introduced parsing for CSS legal comments but caused a regression in the code that checks whether there are any rules that come before `@import`. This is not desired because browsers ignore `@import` rules after other non-`@import` rules, so esbuild warns you when you do this. However, legal comments are modeled as rules in esbuild's internal AST even though they aren't actual CSS rules, and the code that performs this check wasn't updated. This release fixes the crash.
+
 ## 0.13.11
 
 * Implement class static blocks ([#1558](https://github.com/evanw/esbuild/issues/1558))

internal/css_lexer/css_lexer.go

@@ -206,6 +206,13 @@ func Tokenize(log logger.Log, source logger.Source) TokenizeResult {
 		tokens = append(tokens, lexer.Token)
 		lexer.next()
 	}
+	if lexer.licenseCommentsBefore != nil {
+		for _, comment := range lexer.licenseCommentsBefore {
+			comment.TokenIndexAfter = uint32(len(tokens))
+			comments = append(comments, comment)
+		}
+		lexer.licenseCommentsBefore = nil
+	}
 	return TokenizeResult{
 		Tokens:               tokens,
 		LicenseComments:      comments,

internal/css_parser/css_parser.go

@@ -177,6 +177,9 @@ loop:
 				break
 			}
 			rules = append(rules, css_ast.Rule{Loc: comment.Loc, Data: &css_ast.RComment{Text: comment.Text}})
+			if context.isTopLevel {
+				locs = append(locs, comment.Loc)
+			}
 			p.licenseCommentIndex++
 		}
 
@@ -196,19 +199,23 @@ loop:
 			if context.isTopLevel {
 				switch rule.Data.(type) {
 				case *css_ast.RAtCharset:
-					if !didWarnAboutCharset && len(rules) > 0 {
-						p.log.AddRangeWarningWithNotes(&p.tracker, first, "\"@charset\" must be the first rule in the file",
-							[]logger.MsgData{logger.RangeData(&p.tracker, logger.Range{Loc: locs[len(locs)-1]},
-								"This rule cannot come before a \"@charset\" rule")})
-						didWarnAboutCharset = true
+					if !didWarnAboutCharset {
+						for i, before := range rules {
+							if _, ok := before.Data.(*css_ast.RComment); !ok {
+								p.log.AddRangeWarningWithNotes(&p.tracker, first, "\"@charset\" must be the first rule in the file",
+									[]logger.MsgData{logger.RangeData(&p.tracker, logger.Range{Loc: locs[i]},
+										"This rule cannot come before a \"@charset\" rule")})
+								didWarnAboutCharset = true
+							}
+						}
 					}
 
 				case *css_ast.RAtImport:
 					if !didWarnAboutImport {
 					importLoop:
 						for i, before := range rules {
 							switch before.Data.(type) {
-							case *css_ast.RAtCharset, *css_ast.RAtImport:
+							case *css_ast.RComment, *css_ast.RAtCharset, *css_ast.RAtImport:
 							default:
 								p.log.AddRangeWarningWithNotes(&p.tracker, first, "All \"@import\" rules must come first",
 									[]logger.MsgData{logger.RangeData(&p.tracker, logger.Range{Loc: locs[i]},

internal/css_parser/css_parser_test.go

@@ -846,6 +846,27 @@ func TestAtImport(t *testing.T) {
 	expectParseError(t, "@import \"foo.css\" {}", "<stdin>: warning: Expected \";\" but found end of file\n")
 }
 
+func TestLicenseComment(t *testing.T) {
+	expectPrinted(t, "/*!*/@import \"x\";", "/*!*/\n@import \"x\";\n")
+	expectPrinted(t, "/*!*/@charset \"UTF-8\";", "/*!*/\n@charset \"UTF-8\";\n")
+	expectPrinted(t, "/*!*/ @import \"x\";", "/*!*/\n@import \"x\";\n")
+	expectPrinted(t, "/*!*/ @charset \"UTF-8\";", "/*!*/\n@charset \"UTF-8\";\n")
+	expectPrinted(t, "/*!*/ @charset \"UTF-8\"; @import \"x\";", "/*!*/\n@charset \"UTF-8\";\n@import \"x\";\n")
+	expectPrinted(t, "/*!*/ @import \"x\"; @charset \"UTF-8\";", "/*!*/\n@import \"x\";\n@charset \"UTF-8\";\n")
+
+	expectParseError(t, "/*!*/ @import \"x\";", "")
+	expectParseError(t, "/*!*/ @charset \"UTF-8\";", "")
+	expectParseError(t, "/*!*/ @charset \"UTF-8\"; @import \"x\";", "")
+	expectParseError(t, "/*!*/ @import \"x\"; @charset \"UTF-8\";",
+		"<stdin>: warning: \"@charset\" must be the first rule in the file\n"+
+			"<stdin>: note: This rule cannot come before a \"@charset\" rule\n")
+
+	expectPrinted(t, "@import \"x\";/*!*/", "@import \"x\";\n/*!*/\n")
+	expectPrinted(t, "@charset \"UTF-8\";/*!*/", "@charset \"UTF-8\";\n/*!*/\n")
+	expectPrinted(t, "@import \"x\"; /*!*/", "@import \"x\";\n/*!*/\n")
+	expectPrinted(t, "@charset \"UTF-8\"; /*!*/", "@charset \"UTF-8\";\n/*!*/\n")
+}
+
 func TestAtKeyframes(t *testing.T) {
 	expectPrinted(t, "@keyframes {}", "@keyframes \"\" {\n}\n")
 	expectPrinted(t, "@keyframes name{}", "@keyframes name {\n}\n")