-
Notifications
You must be signed in to change notification settings - Fork 2
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
Parse the TargetResolver from assemblies #94
Conversation
57250be
to
4b84bf2
Compare
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.
A couple small suggestions. LGTM!
|
||
public init( | ||
name: String, | ||
registrations: [Registration], | ||
registrationsIntoCollections: [RegistrationIntoCollection], | ||
imports: [ImportDeclSyntax] = [] | ||
imports: [ImportDeclSyntax] = [], | ||
targetResolver: String = "Resolver" |
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.
Remove the default value here? (so it is not specified in multiple places)
Sources/KnitCommand/GenCommand.swift
Outdated
var useTargetResolver: Bool = false | ||
|
||
@Option(help: "Default type to extend when generating Resolver type safety methods") | ||
var defaultExtensionTarget = "Resolver" |
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.
Maybe defaultExtensionTargetResolver
to keep naming consistent
self.targetResolver = identifier.name.text | ||
} | ||
|
||
return .skipChildren |
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.
Just as a curiosity I wonder if it is possible for TypeAliasDecl nodes to have child nodes 🤔
82ba97b
to
b201b3d
Compare
self.targetResolver = identifier.name.text | ||
} | ||
|
||
return .visitChildren |
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.
Oh sorry if my other comment was confusing, I think skip children is what we want here
b201b3d
to
52a80a4
Compare
Looks for instances of
typealias TargetResolver = Foo
and generates type safe extensions underneath the targeted resolver.This allows correct scoping of generated type safety functions.
I added
useTargetResolver
as a way to disable this functionality if we upgrade knit versions but aren't ready to make the switch.defaultExtensionTarget
was also added to allow correctly setting the resolver for assemblies which aren't explicitly defining a typealias.