New Mojom parser: Fix error message for invalid enum value initailizers.
BUG=#461
- In the case that an EnumValue is being used as an enum value initializer for an Enum of the wrong type, the error message now mentions "used as an initializer" instead of "being assigned to" because an enum value initializer declaration is not really an assignment.
- We now capture the name of the enum vale being assigned to for the sake of the error message.
- We mention the fully-qualified name of the Enum in error messages.
- Found another occurrence of "Error:" that I missed last time.
R=azani@chromium.org
Review URL: https://codereview.chromium.org/1511353002 .
diff --git a/mojom/mojom_parser/mojom/computed_data.go b/mojom/mojom_parser/mojom/computed_data.go
index f5e6fe6..9297df5 100644
--- a/mojom/mojom_parser/mojom/computed_data.go
+++ b/mojom/mojom_parser/mojom/computed_data.go
@@ -95,7 +95,7 @@
} else {
// TODO(rudominer) Allow enum values to be initialized to other enum values as long
// as the assignment chain is well-founded.
- message := fmt.Sprintf("Error: The reference %s is being used as an enum value initializer but it has resolved to a "+
+ message := fmt.Sprintf("The reference %s is being used as an enum value initializer but it has resolved to a "+
"different enum value that itself does not yet have an integer value.", specifiedValue.identifier)
message = UserErrorMessage(specifiedValue.scope.file, specifiedValue.token, message)
return 0, fmt.Errorf(message)
diff --git a/mojom/mojom_parser/mojom/types.go b/mojom/mojom_parser/mojom/types.go
index f6efc2f..89b3f29 100644
--- a/mojom/mojom_parser/mojom/types.go
+++ b/mojom/mojom_parser/mojom/types.go
@@ -953,7 +953,8 @@
message = UserErrorMessage(v.scope.file, v.token, message)
return fmt.Errorf(message)
case *EnumValue:
- // An EnumValue is being used as an EnumValue initializer. We do not check this further in this phase.
+ // An EnumValue is being used as an EnumValue initializer.
+ // Below we will check that the two EnumTypes match.
// In ComputeEnumValueIntegers() in computed_data.go we will further validate.
default:
panic(fmt.Sprintf("Unexpected type %T", concreteValue))
@@ -985,12 +986,20 @@
switch v.resolvedDeclaredValue.(type) {
case *EnumValue:
// An enum value is being assigned directly to a variable.
- message = fmt.Sprintf("Illegal assignment: The enum value %s may not be assigned to %s of type %s.",
- v.identifier, v.assigneeSpec.Name, assigneeType.TypeName())
+ message = "Illegal assignment: The enum value %s of type %s may not be assigned to %s of type %s."
+ if v.usedAsEnumValueInitializer {
+ // An enum value is being used directly as an initializer.
+ message = "Illegal assignment: The enum value %s of type %s may not be used as an initializer for %s of type %s."
+ }
+ message = fmt.Sprintf(message, v.identifier, concreteValue.enumType.fullyQualifiedName, v.assigneeSpec.Name, assigneeType.TypeName())
default:
// A user-defined constant whose value is an enum value is being assigned to a variable.
- message = fmt.Sprintf("Illegal assignment: %s with the value %v may not be assigned to %s of type %s.",
- v.identifier, concreteValue.fullyQualifiedName, v.assigneeSpec.Name, assigneeType.TypeName())
+ message = "Illegal assignment: %s with the value %v may not be assigned to %s of type %s."
+ if v.usedAsEnumValueInitializer {
+ // A user-defined constant whose value is an enum valu is being used as an initializer.
+ message = "Illegal assignment: %s with the value %v may not be used as an initializer for %s of type %s."
+ }
+ message = fmt.Sprintf(message, v.identifier, concreteValue.fullyQualifiedName, v.assigneeSpec.Name, assigneeType.TypeName())
}
default:
panic(fmt.Sprintf("Unexpected type %T", concreteValue))
diff --git a/mojom/mojom_parser/parser/parsing.go b/mojom/mojom_parser/parser/parsing.go
index 2b91cfa..d0f6879 100644
--- a/mojom/mojom_parser/parser/parsing.go
+++ b/mojom/mojom_parser/parser/parsing.go
@@ -939,7 +939,7 @@
nameToken := p.lastConsumed
var valueRef mojom.ValueRef
if p.tryMatch(lexer.Equals) {
- valueRef = p.parseEnumValueInitializer(mojomEnum)
+ valueRef = p.parseEnumValueInitializer(mojomEnum, name)
}
declData := p.DeclData(name, nameToken, attributes)
duplicateNameError = mojomEnum.AddEnumValue(declData, valueRef)
@@ -967,7 +967,7 @@
// ENUM_VAL_INITIALIZER -> INTEGER_LITERAL | ENUM_VALUE_REF
// ENUM_VALUE_REF -> IDENTIFIER {{that resolves to a declared enum value}}
-func (p *Parser) parseEnumValueInitializer(mojoEnum *mojom.MojomEnum) mojom.ValueRef {
+func (p *Parser) parseEnumValueInitializer(mojoEnum *mojom.MojomEnum, valueName string) mojom.ValueRef {
if !p.OK() {
return nil
}
@@ -982,7 +982,7 @@
enumType := mojom.NewResolvedUserTypeRef(mojoEnum.FullyQualifiedName(), mojoEnum)
valueToken := p.peekNextToken("Parsing an enum value initializer type.")
- valueRef := p.parseValue(mojom.AssigneeSpec{"enum value", enumType})
+ valueRef := p.parseValue(mojom.AssigneeSpec{valueName, enumType})
if valueRef == nil {
return nil
}
diff --git a/mojom/mojom_parser/parser/resolution_test.go b/mojom/mojom_parser/parser/resolution_test.go
index fbf2cb8..57c0cb9 100644
--- a/mojom/mojom_parser/parser/resolution_test.go
+++ b/mojom/mojom_parser/parser/resolution_test.go
@@ -245,7 +245,7 @@
`
test.addTestCase(contents, []string{
"Illegal assignment",
- "The enum value MyEnum.TWO may not be assigned to Bar of type int32"})
+ "The enum value MyEnum.TWO of type MyEnum may not be assigned to Bar of type int32"})
}
////////////////////////////////////////////////////////////
@@ -333,7 +333,7 @@
`
test.addTestCase(contents, []string{
"Illegal assignment",
- "The enum value MyEnum.TWO may not be assigned to Bar of type string"})
+ "The enum value MyEnum.TWO of type MyEnum may not be assigned to Bar of type string"})
}
////////////////////////////////////////////////////////////
@@ -452,7 +452,53 @@
`
test.addTestCase(contents, []string{
"Illegal assignment",
- "The enum value MyOtherEnum.TWO may not be assigned to Bar of type MyEnum"})
+ "The enum value MyOtherEnum.TWO of type MyOtherEnum may not be assigned to Bar of type MyEnum"})
+ }
+
+ ////////////////////////////////////////////////////////////
+ // Test Case: Use an enum value from a different enum as an enum value initializer.
+ ////////////////////////////////////////////////////////////
+ {
+ contents := `
+ enum MyEnum {
+ ONE,
+ TWO,
+ THREE
+ };
+
+ enum MyOtherEnum {
+ ONE,
+ TWO = MyEnum.TWO,
+ THREE
+ };
+ `
+ test.addTestCase(contents, []string{
+ "Illegal assignment",
+ "The enum value MyEnum.TWO of type MyEnum may not be used as an initializer for TWO of type MyOtherEnum."})
+ }
+
+ ////////////////////////////////////////////////////////////
+ // Test Case: Use an enum value from a different enum as an enum value initializer.
+ ////////////////////////////////////////////////////////////
+ {
+ contents := `
+ enum MyEnum {
+ ONE,
+ TWO,
+ THREE
+ };
+
+ const MyEnum Bar = MyEnum.TWO;
+
+ enum MyOtherEnum {
+ ONE,
+ TWO = Bar,
+ THREE
+ };
+ `
+ test.addTestCase(contents, []string{
+ "Illegal assignment",
+ "Bar with the value MyEnum.TWO may not be used as an initializer for TWO of type MyOtherEnum."})
}
////////////////////////////////////////////////////////////
