Skip to content

Commit dd65a57

Browse files
reid-spencerclaude
andcommitted
Add validation for predefined type name conflicts
Error when a type name exactly matches a built-in type (e.g., type Currency is Decimal(10,2)) since the parser won't handle the redefined name correctly. Style warning when a type name is a case-variant of a built-in (e.g., type Timestamp is TimeStamp) as it's redundant. Add test cases for both checks. Update .check files for existing test inputs that trigger the new validations. Add 6 riddl-models that redefine built-in types to pendingModels. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent eabd988 commit dd65a57

5 files changed

Lines changed: 110 additions & 4 deletions

File tree

commands/jvm/src/test/scala/com/ossuminc/riddl/commands/RiddlModelsRoundTripTest.scala

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,16 @@ class RiddlModelsRoundTripTest extends AnyWordSpec with Matchers with BeforeAndA
7171
"--show-usage-warnings=false"
7272
)
7373

74-
private val pendingModels: Set[String] = Set.empty
74+
// Models that redefine built-in type names (UserId, Location,
75+
// Currency). Remove after fixing in riddl-models.
76+
private val pendingModels: Set[String] = Set(
77+
"technology/saas/tenant-provisioning",
78+
"entertainment/media/content-management",
79+
"construction/field-operations/equipment-tracking",
80+
"construction/field-operations/job-site-management",
81+
"finance/payments/digital-wallet",
82+
"telecommunications/billing/usage-billing"
83+
)
7584

7685
"BAST round-trip" should {
7786
val confFiles = discoverModels(riddlModelsDir)

passes/input/check/everything/everything.check

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,9 @@ passes/input/check/everything/everything.riddl(24:10->15):
4646
passes/input/check/everything/everything.riddl(25:10->13):
4747
Type 'url' should start with a capital letter:
4848
type url is URL
49+
passes/input/check/everything/everything.riddl(25:10->13):
50+
Type 'url' is a redundant case-variant of built-in type 'URL':
51+
type url is URL
4952
passes/input/check/everything/everything.riddl(27:25->26):
5053
Field identifier 'a' is too short. The minimum length is 3:
5154
type PeachType is { a: Integer }

passes/input/check/streaming/streaming.check

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
passes/input/check/streaming/streaming.riddl(3:8->19):
2+
Type 'Temperature' redefines built-in type 'Temperature':
3+
type Temperature is Real type Forecast is any of { Rainy, Cloudy, Sunny }
14
passes/input/check/streaming/streaming.riddl(4:3->28):
25
Source 'GetWeatherForecast' should have a handler:
36
source GetWeatherForecast is {

passes/jvm-native/src/test/scala/com/ossuminc/riddl/passes/validate/TypeValidatorTest.scala

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ package com.ossuminc.riddl.passes.validate
99
import com.ossuminc.riddl.language.AST.*
1010
import com.ossuminc.riddl.language.{Contents, Messages, *}
1111
import com.ossuminc.riddl.language.Messages.*
12-
import com.ossuminc.riddl.language.parsing.RiddlParserInput
12+
import com.ossuminc.riddl.language.parsing.{PredefType, RiddlParserInput}
1313
import com.ossuminc.riddl.utils.{CommonOptions, pc}
1414
import org.scalatest.TestData
1515

@@ -156,5 +156,72 @@ class TypeValidatorTest extends AbstractValidatingTest {
156156
}
157157
}
158158
}
159+
160+
"error when type name matches a predefined type" in { (td: TestData) =>
161+
// Test every predefined type name that can be used as a type
162+
// identifier without conflicting with the parser. Some names
163+
// (like String, Boolean, Integer, etc.) are keywords that the
164+
// parser consumes as type expressions, so they can't appear
165+
// as user-defined type names. We test those that parse as
166+
// identifiers followed by "is".
167+
val testableTypes = PredefType.allPredefTypes.filterNot { name =>
168+
// These are consumed by the parser as type expression
169+
// keywords and won't parse as "type X is ..." definitions
170+
scala.collection.immutable.Set(
171+
"Abstract", "Boolean", "Current", "Currency", "Date",
172+
"DateTime", "Decimal", "Duration", "Id", "Integer",
173+
"Location", "Length", "Luminosity", "Mass", "Mole",
174+
"Nothing", "Natural", "Number", "Pattern", "Range",
175+
"Real", "String", "Temperature", "Time", "TimeStamp",
176+
"Unknown", "URI", "UserId", "UUID", "Whole",
177+
"ZonedDate", "ZonedDateTime"
178+
).contains(name)
179+
}
180+
// If all names are keywords, test with at least one that we
181+
// know works by wrapping in a context (where type keywords
182+
// are not consumed as expressions at the identifier position)
183+
// Actually: predefined type keywords ARE consumed by the
184+
// parser before reaching the identifier, so we test using
185+
// the names that don't clash. For full coverage, we verify
186+
// the check exists and works with "Blob" which is not a
187+
// keyword used in type expressions.
188+
val input = RiddlParserInput(
189+
"""domain foo is {
190+
| type Blob is Number
191+
|}
192+
|""".stripMargin,
193+
td
194+
)
195+
pc.withOptions(CommonOptions.default) { _ =>
196+
parseAndValidateDomain(input, shouldFailOnErrors = false) {
197+
case (_: Domain, _, msgs: Messages) =>
198+
assertValidationMessage(
199+
msgs,
200+
Error,
201+
"redefines built-in type 'Blob'"
202+
)
203+
}
204+
}
205+
}
206+
207+
"warn when type name is case-variant of predefined type" in { (td: TestData) =>
208+
val input = RiddlParserInput(
209+
"""domain foo is {
210+
| type Timestamp is TimeStamp
211+
|}
212+
|""".stripMargin,
213+
td
214+
)
215+
pc.withOptions(CommonOptions.default) { _ =>
216+
parseAndValidateDomain(input, shouldFailOnErrors = false) {
217+
case (_: Domain, _, msgs: Messages) =>
218+
assertValidationMessage(
219+
msgs,
220+
StyleWarning,
221+
"redundant case-variant of built-in type 'TimeStamp'"
222+
)
223+
}
224+
}
225+
}
159226
}
160227
}

passes/shared/src/main/scala/com/ossuminc/riddl/passes/validate/ValidationPass.scala

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ package com.ossuminc.riddl.passes.validate
88

99
import com.ossuminc.riddl.language.AST.*
1010
import com.ossuminc.riddl.language.Messages.*
11+
import com.ossuminc.riddl.language.parsing.PredefType
1112
import com.ossuminc.riddl.language.{Contents, Finder, Messages, *}
1213
import com.ossuminc.riddl.passes.resolve.{ResolutionOutput, ResolutionPass}
1314
import com.ossuminc.riddl.passes.symbols.{SymbolsOutput, SymbolsPass}
@@ -455,12 +456,35 @@ case class ValidationPass(
455456
parents: Parents
456457
): Unit = {
457458
checkDefinition(parents, t)
459+
val typeName = t.id.value
458460
check(
459-
t.id.value.head.isUpper,
461+
typeName.head.isUpper,
460462
s"${t.identify} should start with a capital letter",
461463
StyleWarning,
462-
t.id.loc // Fixed: use identifier location, not type keyword location
464+
t.id.loc
463465
)
466+
// Check if the type name exactly matches a predefined type name
467+
check(
468+
!PredefType.allPredefTypes.contains(typeName),
469+
s"${t.identify} redefines built-in type '$typeName'",
470+
Error,
471+
t.id.loc
472+
)
473+
// Check if the type name is a case-variant of a predefined type
474+
if !PredefType.allPredefTypes.contains(typeName) then
475+
val caseMatch = PredefType.allPredefTypes.find(
476+
pt => pt.equalsIgnoreCase(typeName) && pt != typeName
477+
)
478+
caseMatch.foreach { predef =>
479+
check(
480+
false,
481+
s"${t.identify} is a redundant case-variant of " +
482+
s"built-in type '$predef'",
483+
StyleWarning,
484+
t.id.loc
485+
)
486+
}
487+
end if
464488
if !t.typEx.isInstanceOf[AggregateTypeExpression] then {
465489
checkTypeExpression(t.typEx, t, parents)
466490
}

0 commit comments

Comments
 (0)