added the new switch expression usage for jdk equal or greater than 13#4065
added the new switch expression usage for jdk equal or greater than 13#4065bydrim wants to merge 1 commit into
Conversation
|
It seems that the code is not compliant with the formatting rules. Working on it. |
bydrim
force-pushed
the
feat/switch-exp-jdk13
branch
from
6e20680 to
ca10fcc
Compare
|
I realized that Reformat Code on save was not active on my Intellij. I used git squash to keep the git logs clean, so these are the formatting changes:
|
|
The fail seems to be related to codecov migrating their keybase accounts. Their advice is to use v7, and currently v5 is used in this project. |
|
@bydrim yes the JDK 21 pipline failed because of the issue you referenced. We upgraded the plugin to v7.
|
| <#list valueMappings as valueMapping> | ||
| case <@writeSource source=valueMapping.source/> -> <#if valueMapping.targetAsException > throw new <@includeModel object=unexpectedValueMappingException />( "Unexpected enum constant: " + ${sourceParameter.name} );<#else><@writeTarget target=valueMapping.target/>;</#if> | ||
| </#list> | ||
| default -> <#if defaultTarget.targetAsException >throw new <@includeModel object=unexpectedValueMappingException />( "Unexpected enum constant: " + ${sourceParameter.name} )<#else><@writeTarget target=defaultTarget.target/></#if>; |
There was a problem hiding this comment.
The default branch is only necessary when not all values are explicitly covered by a case.
There was a problem hiding this comment.
I also agree that it is unnecessary to use default branch when all possible cases are covered. To figure out if all the cases are covered, i thought about checking defaultTarget.targetAsException == true. However, if I understand correctly, it is possible to leave unmatched values and the default behavior of the library is to give warning instead of throwing error during compilation, unless configured otherwise by the user. Also it is possible to choose to throw exception for specified targets. Am i wrong and how can i check if all cases are covered?
There was a problem hiding this comment.
I'm pinging in case you missed previous comment. How can i make sure that all the cases are covered?
| @Override | ||
| public boolean isSourceVersionAtLeast13() { | ||
| return sourceVersionAtLeast13; | ||
| } | ||
|
|
There was a problem hiding this comment.
Weren't switch expressions released in Java 14? https://openjdk.org/jeps/361
Java 13 needs to have preview enabled to use them.
There was a problem hiding this comment.
@stefanos-kalantzis you are right in Java 13 it was only available with –enable-preview.
There was a problem hiding this comment.
Oh, sorry that I didn't think of checking when exactly it was released. should i change it to version 14 then?
There was a problem hiding this comment.
@bydrim I think so, yes. It's available by default since Java 14.
Sorry, my analysis in the issue was incorrect.
| <#list valueMappings as valueMapping> | ||
| case <@writeSource source=valueMapping.source/> -> <#if valueMapping.targetAsException > throw new <@includeModel object=unexpectedValueMappingException />( "Unexpected enum constant: " + ${sourceParameter.name} );<#else><@writeTarget target=valueMapping.target/>;</#if> | ||
| </#list> | ||
| default -> <#if defaultTarget.targetAsException >throw new <@includeModel object=unexpectedValueMappingException />( "Unexpected enum constant: " + ${sourceParameter.name} )<#else><@writeTarget target=defaultTarget.target/></#if>; |
There was a problem hiding this comment.
expanding on @hduelme's comment,
There are 2 possibilities here:
- if the switch covers all cases, the
defaultbranch should not be there (it's unreachable/dead code). - if the switch does NOT cover all cases, then the compiler will fail with:
(switch-expressions are exhaustive)
'switch' expression does not cover all possible input values
In my opinion, if the above compiler error occurs, its a valid outcome for mapstruct, indicating that the user needs to fix something. A default case would move this error from compile-time to runtime, which is the opposite direction from what the JEP was designed for.
If mapstruct should behave differently than that, then the user should define a custom mapping.
(I hope I'm not missing something...)
There was a problem hiding this comment.
@stefanos-kalantzis currently mapstruct enforces that all constants are mapped. If not an error is given
The following constants from the source enum have no corresponding constant in the target enum and must be be mapped via adding additional mappings: C.
This behavior can be alter with map MappingConstants. For example
@ValueMapping(target = MappingConstants.THROW_EXCEPTION, source = MappingConstants.ANY_REMAINING)
results in all unmapped values being mapped as THROW_EXCEPTION.
3 participants
As described in the issue #4062 for jdk equal or greater than 13 the usage of new switch expression added. Related fixtures are updated. Also as discussed with Filip, for switch expressions with only default case, defaultTarget is directly assigned without switch expression, such as