Report a compilation error for invalid placeholders [Part II]#217
Report a compilation error for invalid placeholders [Part II]#217yevhenii-nadtochii merged 20 commits intomasterfrom
Conversation
yevhenii-nadtochii
changed the base branch from
master
to
report-illegal-placeholders
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #217 +/- ##
============================================
+ Coverage 26.14% 28.21% +2.07%
- Complexity 241 242 +1
============================================
Files 137 138 +1
Lines 3236 3314 +78
Branches 247 247
============================================
+ Hits 846 935 +89
+ Misses 2351 2327 -24
- Partials 39 52 +13 🚀 New features to boost your workflow:
|
# Conflicts: # model/src/main/kotlin/io/spine/validation/required/RequiredOption.kt
armiol
left a comment
armiol
left a comment
There was a problem hiding this comment.
@yevhenii-nadtochii please see my comments.
| @@ -1,5 +1,5 @@ | |||
| /* | |||
| ` * Copyright 2024, TeamDev. All rights reserved. | |||
| * Copyright 2024, TeamDev. All rights reserved. | |||
| string string_value = 2; | ||
|
|
||
| bool bool_value = 3; | ||
|
|
|
@armiol PTAL |
alexander-yevsyukov
left a comment
alexander-yevsyukov
left a comment
There was a problem hiding this comment.
Please see my comment.
alexander-yevsyukov
left a comment
alexander-yevsyukov
left a comment
There was a problem hiding this comment.
LGTM, with the comments regarding the code layout to consider.
It is better to have the sets given as vertical lists for easier reading.
| public val FieldType.isSingularString: Boolean | ||
| get() = primitive == TYPE_STRING | ||
|
|
||
| private val SUPPORTED_PLACEHOLDERS = |
There was a problem hiding this comment.
Please have the values line by line, and alphabetically sorted.
| } | ||
|
|
||
| private val SUPPORTED_PLACEHOLDERS = | ||
| setOf(FIELD_PATH, FIELD_VALUE, FIELD_TYPE, PARENT_TYPE, WHEN_IN) |
There was a problem hiding this comment.
Please have the values line by line, and alphabetically sorted.
|
|
||
| private val DELIMITER = Regex("""(?<=\d)\s?\.\.\s?(?=[\d-+])""") | ||
|
|
||
| private val SUPPORTED_PLACEHOLDERS = |
There was a problem hiding this comment.
Please have the values line by line, and alphabetically sorted.
| } | ||
| } | ||
|
|
||
| private val SUPPORTED_PLACEHOLDERS = setOf(MESSAGE_TYPE, REQUIRE_FIELDS) |
There was a problem hiding this comment.
Please have the values line by line, and alphabetically sorted.
3 participants
This PR is a continuation of #216.
It completes #213 by adding the requested check to the remaining options.
Now,
checkPlaceholders()function is shared among all options. It has three overloadings due to the fact that we have options for fields,oneofs and messages.For some options, I have also added basic tests to check compilation errors upon unsupported field types.