css: attempt to warn about undefined composes

internal/bundler_tests/bundler_css_test.go

@@ -855,6 +855,101 @@ func TestImportCSSFromJSComposesFromCircular(t *testing.T) {
 	})
 }
 
+func TestImportCSSFromJSComposesFromUndefined(t *testing.T) {
+	note := "NOTE: The specification of \"composes\" does not define an order when class declarations from separate files are composed together. " +
+		"The value of the \"zoom\" property for \"foo\" may change unpredictably as the code is edited. " +
+		"Make sure that all definitions of \"zoom\" for \"foo\" are in a single file."
+	css_suite.expectBundled(t, bundled{
+		files: map[string]string{
+			"/entry.js": `
+				import styles from "./styles.css"
+				console.log(styles)
+			`,
+			"/styles.css": `
+				@import "well-defined.css";
+				@import "undefined/case1.css";
+				@import "undefined/case2.css";
+				@import "undefined/case3.css";
+				@import "undefined/case4.css";
+				@import "undefined/case5.css";
+			`,
+			"/well-defined.css": `
+				.z1 { composes: z2; zoom: 1; }
+				.z2 { zoom: 2; }
+
+				.z4 { zoom: 4; }
+				.z3 { composes: z4; zoom: 3; }
+
+				.z5 { composes: foo bar from "file-1.css"; }
+			`,
+			"/undefined/case1.css": `
+				.foo {
+					composes: foo from "../file-1.css";
+					zoom: 2;
+				}
+			`,
+			"/undefined/case2.css": `
+				.foo {
+					composes: foo from "../file-1.css";
+					composes: foo from "../file-2.css";
+				}
+			`,
+			"/undefined/case3.css": `
+				.foo { composes: nested1 nested2; }
+				.nested1 { zoom: 3; }
+				.nested2 { composes: foo from "../file-2.css"; }
+			`,
+			"/undefined/case4.css": `
+				.foo { composes: nested1 nested2; }
+				.nested1 { composes: foo from "../file-1.css"; }
+				.nested2 { zoom: 3; }
+			`,
+			"/undefined/case5.css": `
+				.foo { composes: nested1 nested2; }
+				.nested1 { composes: foo from "../file-1.css"; }
+				.nested2 { composes: foo from "../file-2.css"; }
+			`,
+			"/file-1.css": `
+				.foo { zoom: 1; }
+				.bar { zoom: 2; }
+			`,
+			"/file-2.css": `
+				.foo { zoom: 2; }
+			`,
+		},
+		entryPaths: []string{"/entry.js"},
+		options: config.Options{
+			Mode:         config.ModeBundle,
+			AbsOutputDir: "/out",
+			ExtensionToLoader: map[string]config.Loader{
+				".js":  config.LoaderJS,
+				".css": config.LoaderLocalCSS,
+			},
+		},
+		expectedCompileLog: `undefined/case1.css: WARNING: The value of "zoom" in the "foo" class is undefined
+file-1.css: NOTE: The first definition of "zoom" is here:
+undefined/case1.css: NOTE: The second definition of "zoom" is here:
+` + note + `
+undefined/case2.css: WARNING: The value of "zoom" in the "foo" class is undefined
+file-1.css: NOTE: The first definition of "zoom" is here:
+file-2.css: NOTE: The second definition of "zoom" is here:
+` + note + `
+undefined/case3.css: WARNING: The value of "zoom" in the "foo" class is undefined
+undefined/case3.css: NOTE: The first definition of "zoom" is here:
+file-2.css: NOTE: The second definition of "zoom" is here:
+` + note + `
+undefined/case4.css: WARNING: The value of "zoom" in the "foo" class is undefined
+file-1.css: NOTE: The first definition of "zoom" is here:
+undefined/case4.css: NOTE: The second definition of "zoom" is here:
+` + note + `
+undefined/case5.css: WARNING: The value of "zoom" in the "foo" class is undefined
+file-1.css: NOTE: The first definition of "zoom" is here:
+file-2.css: NOTE: The second definition of "zoom" is here:
+` + note + `
+`,
+	})
+}
+
 func TestImportCSSFromJSWriteToStdout(t *testing.T) {
 	css_suite.expectBundled(t, bundled{
 		files: map[string]string{

internal/bundler_tests/snapshots/snapshots_css.txt

@@ -821,6 +821,82 @@ console.log(styles_default);
 .styles_bar {
 }
 
+================================================================================
+TestImportCSSFromJSComposesFromUndefined
+---------- /out/entry.js ----------
+// styles.css
+var styles_default = {};
+
+// entry.js
+console.log(styles_default);
+
+---------- /out/entry.css ----------
+/* well-defined.css */
+.well_defined_z1 {
+  zoom: 1;
+}
+.well_defined_z2 {
+  zoom: 2;
+}
+.well_defined_z4 {
+  zoom: 4;
+}
+.well_defined_z3 {
+  zoom: 3;
+}
+.well_defined_z5 {
+}
+
+/* undefined/case1.css */
+.case1_foo {
+  zoom: 2;
+}
+
+/* undefined/case2.css */
+.case2_foo {
+}
+
+/* undefined/case3.css */
+.case3_foo {
+}
+.case3_nested1 {
+  zoom: 3;
+}
+.case3_nested2 {
+}
+
+/* undefined/case4.css */
+.case4_foo {
+}
+.case4_nested1 {
+}
+.case4_nested2 {
+  zoom: 3;
+}
+
+/* file-1.css */
+.file_1_foo {
+  zoom: 1;
+}
+.file_1_bar {
+  zoom: 2;
+}
+
+/* file-2.css */
+.file_2_foo {
+  zoom: 2;
+}
+
+/* undefined/case5.css */
+.case5_foo {
+}
+.case5_nested1 {
+}
+.case5_nested2 {
+}
+
+/* styles.css */
+
 ================================================================================
 TestImportCSSFromJSLocalAtContainer
 ---------- /out/entry.js ----------

internal/css_ast/css_ast.go

@@ -57,6 +57,11 @@ type Composes struct {
 	//   .foo { composes: bar from url(bar.css) }
 	//
 	ImportedNames []ImportedComposesName
+
+	// This tracks what CSS properties each class uses so that we can warn when
+	// "composes" is used incorrectly to compose two classes from separate files
+	// that declare the same CSS properties.
+	Properties map[string]logger.Loc
 }
 
 type ImportedComposesName struct {

internal/css_parser/css_decls.go

@@ -94,6 +94,28 @@ func (p *parser) processDeclarations(rules []css_ast.Rule, composesContext *comp
 		inset.keyText = ""
 	}
 
+	// If this is a local class selector, track which CSS properties it declares.
+	// This is used to warn when CSS "composes" is used incorrectly.
+	if composesContext != nil {
+		for _, ref := range composesContext.parentRefs {
+			composes, ok := p.composes[ref]
+			if !ok {
+				composes = &css_ast.Composes{}
+				p.composes[ref] = composes
+			}
+			properties := composes.Properties
+			if properties == nil {
+				properties = make(map[string]logger.Loc)
+				composes.Properties = properties
+			}
+			for _, rule := range rules {
+				if decl, ok := rule.Data.(*css_ast.RDeclaration); ok && decl.Key != css_ast.DComposes {
+					properties[decl.KeyText] = decl.KeyRange.Loc
+				}
+			}
+		}
+	}
+
 	for _, rule := range rules {
 		rewrittenRules = append(rewrittenRules, rule)
 		decl, ok := rule.Data.(*css_ast.RDeclaration)

internal/css_parser/css_decls_composes.go

@@ -23,10 +23,6 @@ func (p *parser) handleComposesPragma(context composesContext, tokens []css_ast.
 	var names []nameWithLoc
 	fromGlobal := false
 
-	if p.composes == nil {
-		p.composes = make(map[ast.Ref]*css_ast.Composes)
-	}
-
 	for i, t := range tokens {
 		if t.Kind == css_lexer.TIdent {
 			// Check for a "from" clause at the end
@@ -49,10 +45,6 @@ func (p *parser) handleComposesPragma(context composesContext, tokens []css_ast.
 					}
 					for _, parentRef := range context.parentRefs {
 						composes := p.composes[parentRef]
-						if composes == nil {
-							composes = &css_ast.Composes{}
-							p.composes[parentRef] = composes
-						}
 						for _, name := range names {
 							composes.ImportedNames = append(composes.ImportedNames, css_ast.ImportedComposesName{
 								ImportRecordIndex: importRecordIndex,
@@ -102,10 +94,6 @@ func (p *parser) handleComposesPragma(context composesContext, tokens []css_ast.
 	}
 	for _, parentRef := range context.parentRefs {
 		composes := p.composes[parentRef]
-		if composes == nil {
-			composes = &css_ast.Composes{}
-			p.composes[parentRef] = composes
-		}
 		for _, name := range names {
 			composes.Names = append(composes.Names, p.symbolForName(name.loc, name.text))
 		}

internal/css_parser/css_parser.go

@@ -127,6 +127,7 @@ func Parse(log logger.Log, source logger.Source, options Options) css_ast.AST {
 		allComments:      result.AllComments,
 		legalComments:    result.LegalComments,
 		prevError:        logger.Loc{Start: -1},
+		composes:         make(map[ast.Ref]*css_ast.Composes),
 		localScope:       make(map[string]ast.LocRef),
 		globalScope:      make(map[string]ast.LocRef),
 		makeLocalSymbols: options.symbolMode == symbolModeLocal,

internal/linker/linker.go

@@ -1359,6 +1359,8 @@ func (c *linkerContext) scanImportsAndExports() {
 				}
 			}
 
+			c.validateComposesFromProperties(file, repr)
+
 		case *graph.JSRepr:
 			for importRecordIndex := range repr.AST.ImportRecords {
 				record := &repr.AST.ImportRecords[importRecordIndex]
@@ -2003,6 +2005,86 @@ func (c *linkerContext) scanImportsAndExports() {
 	c.timer.End("Step 6")
 }
 
+func (c *linkerContext) validateComposesFromProperties(rootFile *graph.LinkerFile, rootRepr *graph.CSSRepr) {
+	for _, local := range rootRepr.AST.LocalSymbols {
+		type propertyInFile struct {
+			file *graph.LinkerFile
+			loc  logger.Loc
+		}
+
+		visited := make(map[ast.Ref]bool)
+		properties := make(map[string]propertyInFile)
+		var visit func(*graph.LinkerFile, *graph.CSSRepr, ast.Ref)
+
+		visit = func(file *graph.LinkerFile, repr *graph.CSSRepr, ref ast.Ref) {
+			if visited[ref] {
+				return
+			}
+			visited[ref] = true
+
+			composes, ok := repr.AST.Composes[ref]
+			if !ok {
+				return
+			}
+
+			for _, name := range composes.ImportedNames {
+				if record := repr.AST.ImportRecords[name.ImportRecordIndex]; record.SourceIndex.IsValid() {
+					otherFile := &c.graph.Files[record.SourceIndex.GetIndex()]
+					if otherRepr, ok := otherFile.InputFile.Repr.(*graph.CSSRepr); ok {
+						if otherName, ok := otherRepr.AST.LocalScope[name.Alias]; ok {
+							visit(otherFile, otherRepr, otherName.Ref)
+						}
+					}
+				}
+			}
+
+			for _, name := range composes.Names {
+				visit(file, repr, name.Ref)
+			}
+
+			// Warn about cross-file composition with the same CSS properties
+			for keyText, keyLoc := range composes.Properties {
+				property, ok := properties[keyText]
+				if !ok {
+					properties[keyText] = propertyInFile{file, keyLoc}
+					continue
+				}
+				if property.file == file || property.file == nil {
+					continue
+				}
+
+				localOriginalName := c.graph.Symbols.Get(local.Ref).OriginalName
+				c.log.AddMsgID(logger.MsgID_CSS_UndefinedComposesFrom, logger.Msg{
+					Kind: logger.Warning,
+					Data: rootFile.LineColumnTracker().MsgData(
+						css_lexer.RangeOfIdentifier(rootFile.InputFile.Source, local.Loc),
+						fmt.Sprintf("The value of %q in the %q class is undefined", keyText, localOriginalName),
+					),
+					Notes: []logger.MsgData{
+						property.file.LineColumnTracker().MsgData(
+							css_lexer.RangeOfIdentifier(property.file.InputFile.Source, property.loc),
+							fmt.Sprintf("The first definition of %q is here:", keyText),
+						),
+						file.LineColumnTracker().MsgData(
+							css_lexer.RangeOfIdentifier(file.InputFile.Source, keyLoc),
+							fmt.Sprintf("The second definition of %q is here:", keyText),
+						),
+						{Text: fmt.Sprintf("The specification of \"composes\" does not define an order when class declarations from separate files are composed together. "+
+							"The value of the %q property for %q may change unpredictably as the code is edited. "+
+							"Make sure that all definitions of %q for %q are in a single file.", keyText, localOriginalName, keyText, localOriginalName)},
+					},
+				})
+
+				// Don't warn more than once
+				property.file = nil
+				properties[keyText] = property
+			}
+		}
+
+		visit(rootFile, rootRepr, local.Ref)
+	}
+}
+
 func (c *linkerContext) generateCodeForLazyExport(sourceIndex uint32) {
 	file := &c.graph.Files[sourceIndex]
 	repr := file.InputFile.Repr.(*graph.JSRepr)

internal/logger/msg_ids.go

@@ -48,6 +48,7 @@ const (
 	MsgID_CSS_InvalidAtLayer
 	MsgID_CSS_InvalidCalc
 	MsgID_CSS_JSCommentInCSS
+	MsgID_CSS_UndefinedComposesFrom
 	MsgID_CSS_UnsupportedAtCharset
 	MsgID_CSS_UnsupportedAtNamespace
 	MsgID_CSS_UnsupportedCSSProperty
@@ -161,6 +162,8 @@ func StringToMsgIDs(str string, logLevel LogLevel, overrides map[MsgID]LogLevel)
 		overrides[MsgID_CSS_InvalidCalc] = logLevel
 	case "js-comment-in-css":
 		overrides[MsgID_CSS_JSCommentInCSS] = logLevel
+	case "undefined-composes-from":
+		overrides[MsgID_CSS_UndefinedComposesFrom] = logLevel
 	case "unsupported-@charset":
 		overrides[MsgID_CSS_UnsupportedAtCharset] = logLevel
 	case "unsupported-@namespace":
@@ -283,6 +286,8 @@ func MsgIDToString(id MsgID) string {
 		return "invalid-calc"
 	case MsgID_CSS_JSCommentInCSS:
 		return "js-comment-in-css"
+	case MsgID_CSS_UndefinedComposesFrom:
+		return "undefined-composes-from"
 	case MsgID_CSS_UnsupportedAtCharset:
 		return "unsupported-@charset"
 	case MsgID_CSS_UnsupportedAtNamespace: