-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: refactor to put function name generator in constant #641
Changes from 10 commits
6e15111
f161e58
4182f4b
2c98204
1ce2183
750ea96
3a3ed47
a3701df
8738b87
204d4d3
466e6a3
7c86c25
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,6 +26,8 @@ | |
import software.amazon.smithy.model.traits.ErrorTrait; | ||
import software.amazon.smithy.model.traits.UnitTypeTrait; | ||
|
||
import software.amazon.polymorph.smithygo.utils.Constants; | ||
|
||
public class DafnyAwsSdkClientTypeConversionProtocol | ||
implements ProtocolGenerator { | ||
|
||
|
@@ -599,7 +601,7 @@ func Error_ToDafny(err error)($L.Error) { | |
} | ||
""", | ||
DafnyNameResolver.dafnyTypesNamespace(serviceShape), | ||
writer.consumer(w -> { | ||
writer.consumer(w -> { | ||
for (final var error : errorShapes) { | ||
w.write( | ||
""" | ||
|
@@ -823,7 +825,7 @@ private void generateSerializerFunctions( | |
return $L | ||
} | ||
""", | ||
ShapeVisitorHelper.funcNameGenerator( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: I'd prefer fully-qualified-name / FQN over static imports. Improves readability / reasoning around where this function belongs. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
Constants.funcNameGenerator( | ||
visitingMemberShape, | ||
"ToDafny" | ||
), | ||
|
@@ -881,7 +883,7 @@ private void generateDeserializerFunctions( | |
func $L(input $L)($L) { | ||
return $L | ||
}""", | ||
ShapeVisitorHelper.funcNameGenerator( | ||
Constants.funcNameGenerator( | ||
visitingMemberShape, | ||
"FromDafny" | ||
), | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,29 @@ | ||
package software.amazon.polymorph.smithygo.utils; | ||
|
||
import software.amazon.smithy.model.shapes.MemberShape; | ||
|
||
public class Constants { | ||
|
||
public static final String DAFNY_RUNTIME_GO_LIBRARY_MODULE = | ||
"github.com/dafny-lang/DafnyRuntimeGo/v4"; | ||
|
||
// TODO: Is it possible to make this function name shorter and in camelCase? | ||
/** | ||
* Generates a function name for shape visitors for AWS SDK and localservice. | ||
* | ||
* @param memberShape The visiting MemberShape | ||
* @param suffix A string to be appended at the end of the generated function name | ||
* @return A string representing the generated function name | ||
*/ | ||
public static String funcNameGenerator( | ||
final MemberShape memberShape, | ||
final String suffix | ||
) { | ||
return memberShape | ||
.getId() | ||
.toString() | ||
.replaceAll("[.$#]", "_") | ||
.concat("_") | ||
.concat(suffix); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this different than one in the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought the name in ValidationGenerator won't collide and will look good but the name collided (Caught in CI in 8738b87) So, I made ValidationGenerator to use same function name. |
||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure how this came in diff but make format_java will fix this. So, Ignoring this for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally I'm not a fan of our java formatter - it keeps line too short and I think uses 2 white-spaces for indentation.