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

Add isGeneric support #39

Merged
merged 4 commits into from
Dec 17, 2016
Merged

Add isGeneric support #39

merged 4 commits into from
Dec 17, 2016

Conversation

krzysztofzablocki
Copy link
Owner

@krzysztofzablocki krzysztofzablocki commented Dec 17, 2016

@ilyapuchka Since you have been so helpful, I've invited you to the project as a collaborator, hope that's ok :)
Check this PR out

Closes #6

return "\(parentName).\(localName)"
}

/// Is this type generic? regardless whether it's this type directly or one of its parent types
var isGeneric: Bool {
return (parent?.isGeneric ?? false) || hasGenericComponent
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we really need to check parent here? As far as I know generic types can not contain other types, only the other way around.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Before Swift 3 they couldn't, now they can

Copy link
Collaborator

Choose a reason for hiding this comment

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

Really? 😯 I get a compilation error with this code:

struct MyStruct<Element> {
    struct Inner { } //Type 'Inner' can not be nested in generic type 'MyStruct'
}

Copy link
Owner Author

Choose a reason for hiding this comment

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

Uh my bad then, it was supposed to happen and I thought it was for Swift 3, I guess it's not ready yet though ;[, quote from them:
"It's an implementation limitation. We'll remove the restriction once our compiler and runtime are able to correctly handle types nested in generic contexts."

Copy link
Owner Author

Choose a reason for hiding this comment

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

Do you think we should remove this check to improve performance, or doesn't matter?

Copy link
Collaborator

Choose a reason for hiding this comment

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

For me it is more about confusion between isGeneric and hasGenericComponent as Swift does not currently support that and who knows when it will.

Copy link
Owner Author

Choose a reason for hiding this comment

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

ok, I'll get rid of it for now 👍

@@ -456,6 +457,11 @@ extension Parser {
return annotations
}

fileprivate func isGeneric(source: [String: SourceKitRepresentable]) -> Bool {
guard let substring = extract(.nameSuffix, from: source), substring.trimmingCharacters(in: .whitespacesAndNewlines).hasPrefix("<") == true else { return false }
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think there is no need to trim whitespaces as SourceKitten provides offset and tokens length not including spaces. But I might be wrong.

Copy link
Owner Author

Choose a reason for hiding this comment

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

The problem is that it's not part of scanned things by source kitten, so I'm recognising it by hand from the source code

@krzysztofzablocki krzysztofzablocki merged commit 6419dec into master Dec 17, 2016
@krzysztofzablocki krzysztofzablocki deleted the feature/generics branch December 17, 2016 14:05
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.

None yet

2 participants