Format generated Go code to adhere to golang coding standards
Golang expects generated code to be properly go fmt'ed (see https://github.com/golang/go/issues/73181#issuecomment-2780661979).
This PR tries to implement the changes so the code will be compatible and go fmt will not change anything in the generated code.
Changes:
- indent using tabs instead of spaces
- when there is only one return argument, omit the parentheses
- removes trailing spaces on comments without a content
- fix import ordering
- correctly indent case statements inside a switch
Test: To test we will create a test client. Next go to the golangtest directory, init a git repo, and run go fmt to see the differences
rm -rf ./golangtest/ && cp -r ./it/go golangtest && dotnet build && ./src/kiota/bin/Debug/net9.0/kiota generate --openapi https://aka.ms/graph/v1.0/openapi.yaml --output ./golangtest/client --language go --exclude-backward-compatible --clean-output -i '/groups/getByIds#POST' -n 'integrationtest/client'
cd golangtest
git init
git add .
go fmt ./...
git diff
Ideally the git diff will show no changes
Still to do
- trailing spaces on setter methods
- some missing spaces between declarations
@baywet this is almost fnished but I'm struggling with one of the last pieces. There is a trailing space on all setter methods in interface declarations (Setxxxx(value xxx)). I can't find the code where this property is introduced, do you happen to know where to look exactly for this?
Example of an interface:
@@ -434,15 +435,15 @@
type AccessPackageable interface {
GetIsHidden() *bool
GetModifiedDateTime() *i336074805fc853987abe6f7fe3ad97a6a6f3077a16391fec744f671a015fbd7e.Time
GetResourceRoleScopes() []AccessPackageResourceRoleScopeable
- SetAccessPackagesIncompatibleWith(value []AccessPackageable)
- SetAssignmentPolicies(value []AccessPackageAssignmentPolicyable)
- SetCatalog(value AccessPackageCatalogable)
- SetCreatedDateTime(value *i336074805fc853987abe6f7fe3ad97a6a6f3077a16391fec744f671a015fbd7e.Time)
- SetDescription(value *string)
- SetDisplayName(value *string)
- SetIncompatibleAccessPackages(value []AccessPackageable)
- SetIncompatibleGroups(value []Groupable)
- SetIsHidden(value *bool)
- SetModifiedDateTime(value *i336074805fc853987abe6f7fe3ad97a6a6f3077a16391fec744f671a015fbd7e.Time)
- SetResourceRoleScopes(value []AccessPackageResourceRoleScopeable)
+ SetAccessPackagesIncompatibleWith(value []AccessPackageable)
+ SetAssignmentPolicies(value []AccessPackageAssignmentPolicyable)
+ SetCatalog(value AccessPackageCatalogable)
+ SetCreatedDateTime(value *i336074805fc853987abe6f7fe3ad97a6a6f3077a16391fec744f671a015fbd7e.Time)
+ SetDescription(value *string)
+ SetDisplayName(value *string)
+ SetIncompatibleAccessPackages(value []AccessPackageable)
+ SetIncompatibleGroups(value []Groupable)
+ SetIsHidden(value *bool)
+ SetModifiedDateTime(value *i336074805fc853987abe6f7fe3ad97a6a6f3077a16391fec744f671a015fbd7e.Time)
+ SetResourceRoleScopes(value []AccessPackageResourceRoleScopeable)
The first version is ready for review @baywet . There are still some minor issues which I was not able to fix as I did not find the code that generates the corresponding code. I will open an issue with the additional minor issues that need to be adressed
@firefart thanks for the extended work on this. As for your question, have you looked here? https://github.com/microsoft/kiota/blob/e23ac20b0aef66454376deee667714f0ab50ca1f/src/Kiota.Builder/Writers/Go/CodePropertyWriter.cs#L31 It seems to me that if both the suffix and the return type are null or empty, we should not add the space.
@firefart thanks for the extended work on this. As for your question, have you looked here?
https://github.com/microsoft/kiota/blob/e23ac20b0aef66454376deee667714f0ab50ca1f/src/Kiota.Builder/Writers/Go/CodePropertyWriter.cs#L31
It seems to me that if both the suffix and the return type are null or empty, we should not add the space.
@baywet thanks for the hint but this particular place it not the provblem. Added some checks, but the trailing spaces are still generated, so it must be elsewhere (but I suspect the same pattern)
Thank you for the additional information.
First, I'm not seeing the trailing space behaviour from the main branch. Second, I spent more time looking into this, and it's actually here.
https://github.com/microsoft/kiota/blob/3491bb7f1943427e3c1e5fca5cef289a9ea8497a/src/Kiota.Builder/Writers/Go/CodeMethodWriter.cs#L476
Which breaks with the following conditional breakpoint.
code.IsOfKind(CodeMethodKind.Setter) && code.Parent is CodeInterface
Let me know if you have any additional comments or questions.
@baywet thanks, that did the trick, the trailing whitespaces are now gone. I also tried to add an integration test to github actions to run go fmt ./... on the generated code and error out if there are formatted files. Could you please approve the github actions run so I can check if the test works? (running it locally is not really possible)
@baywet fixed the unit tests, so would appreciate a new approval for all the tests.
PS: The used xunit version is really old, you should consider upgrading to xunit.v3. I needed to upgrade the local csproj files to this version so see the full strings from the Assert calls which is not possible in the current used version (I used XUNIT_PRINT_MAX_STRING_LENGTH=99999 dotnet test to test)
The used xunit version is really old, you should consider upgrading to xunit.v3
Thanks for bringing this up. From reading the migration documentation, it seems there might be quite some work for this migration. If this is something you feel like you would like to submit a pull request for, I'd suggest starting a new issue.
gentle reminder on this one @firefart
@baywet sorry haven't found time yet to look into the tests, but hopefully by the end of the week to get this PR done