Skip to content
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

feat: adds basic generics support #1194

Conversation

kirillDanshin
Copy link

Describe the PR
This PR adds basic support for 1.18 generics.
It does not support aliases to generics, but it does support single and multi parameter generics.
The approach used in this PR inlines type parameters, so the code around the changed sections could treat these types as if it was defined without type parameters at all.

Relation issue
#1170

Additional context
Additional context provided in the issue linked above.

operation.go Outdated
matches[3] += commentLine[allMatchesLenOffset : allMatchesLenOffset+lostPartEndIdx+1]
}
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check has to be done inside parseAPIObjectSchema

swag/operation.go

Lines 981 to 984 in 65ab05e

schema, err := operation.parseAPIObjectSchema(strings.Trim(matches[2], "{}"), matches[3], astFile)
if err != nil {
return err
}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it uses the entire commentLine tho. this check could be done, if we would pass it as a parameter, but it's questionable if we want to change the function signature

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The parseAPIObjectSchema is a private method, so we can change the signature when needed.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, I just did it.

another thing to consider is that this PR relies on 1.18 ast.TypeSpec, so it won't work on older versions anymore.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, I just did it.

another thing to consider is that this PR relies on 1.18 ast.TypeSpec, so it won't work on older versions anymore.

Then this is a breaking change and it has to be implemented in v2.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might be able to take the same approach as in gopls. Have a look at golangci/golangci-lint#2649 (comment)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just added fallback implementations for older Go versions, I think it should do the trick

return typeSpecDef.FullName()
}

func (pkgDefs *PackagesDefinitions) parametrizeStruct(original *TypeSpecDef, fullGenericForm string) *TypeSpecDef {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you expect to pass the tests for older go versions if this is not implemented?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess you will have to put the TestParseGenericsBasic into a separate test file and add build tags.

The typeSpecDef.TypeSpec.TypeParams used in packages.go isn't available in < 1.18, so will have to use the same approach as in
https://github.com/golang/tools/blob/master/internal/typeparams/typeparams_go118.go
https://github.com/golang/tools/blob/master/internal/typeparams/typeparams_go117.go

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The swag generator must produce the same output no matter if it is built with go1.6 or go1.18.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kirillDanshin any update here?

@ubogdan
Copy link
Contributor

ubogdan commented Jun 16, 2022

@kirillDanshin Thanks for your contribution.
Merged: #1225

@ubogdan ubogdan closed this Jun 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants