-
-
Notifications
You must be signed in to change notification settings - Fork 746
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
Stub anything that looks like a Refit interface #67
Changes from all commits
a395adc
e9b2f91
3881a3f
aa6a39e
65e8a23
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 |
---|---|---|
|
@@ -14,8 +14,8 @@ | |
|
||
namespace Refit.Generator | ||
{ | ||
// * Find all calls to RestService.For<T>, extract all T's | ||
// * Search for all Interfaces, find the method definitions | ||
// * Search for all Interfaces, find the method definitions | ||
// and make sure there's at least one Refit attribute on one | ||
// * Generate the data we need for the template based on interface method | ||
// defn's | ||
// * Get this into an EXE in tools, write a targets file to beforeBuild execute it | ||
|
@@ -30,11 +30,8 @@ public class InterfaceStubGenerator | |
public string GenerateInterfaceStubs(string[] paths) | ||
{ | ||
var trees = paths.Select(x => CSharpSyntaxTree.ParseFile(x)).ToList(); | ||
var interfaceNamesToFind = trees.SelectMany(FindInterfacesToGenerate).Distinct().ToList(); | ||
|
||
var interfacesToGenerate = trees.SelectMany(x => x.GetRoot().DescendantNodes().OfType<InterfaceDeclarationSyntax>()) | ||
.Where(x => interfaceNamesToFind.Contains(x.Identifier.ValueText)) | ||
.ToList(); | ||
var interfacesToGenerate = trees.SelectMany(FindInterfacesToGenerate).ToList(); | ||
|
||
var templateInfo = GenerateTemplateInfoForInterfaceList(interfacesToGenerate); | ||
|
||
|
@@ -43,21 +40,37 @@ public string GenerateInterfaceStubs(string[] paths) | |
return text; | ||
} | ||
|
||
public List<string> FindInterfacesToGenerate(SyntaxTree tree) | ||
public List<InterfaceDeclarationSyntax> FindInterfacesToGenerate(SyntaxTree tree) | ||
{ | ||
var restServiceCalls = tree.GetRoot().DescendantNodes() | ||
.OfType<MemberAccessExpressionSyntax>() | ||
.Where(x => x.Expression is IdentifierNameSyntax && | ||
((IdentifierNameSyntax)x.Expression).Identifier.ValueText == "RestService" && | ||
x.Name.Identifier.ValueText == "For"); | ||
|
||
return restServiceCalls | ||
.SelectMany(x => ((GenericNameSyntax)x.Name).TypeArgumentList.Arguments) | ||
.Select(x => ((IdentifierNameSyntax)x).Identifier.ValueText) | ||
.Distinct() | ||
var nodes = tree.GetRoot().DescendantNodes().ToList(); | ||
|
||
// Make sure this file imports Refit. If not, we're not going to | ||
// find any Refit interfaces | ||
// NB: This falls down in the tests unless we add an explicit "using Refit;", | ||
// but we can rely on this being there in any other file | ||
if (nodes.OfType<UsingDirectiveSyntax>().All(u => u.Name.ToFullString() != "Refit")) | ||
return new List<InterfaceDeclarationSyntax>(); | ||
|
||
return nodes.OfType<InterfaceDeclarationSyntax>() | ||
.Where(i => i.Members.OfType<MethodDeclarationSyntax>().Any(HasRefitHttpMethodAttribute)) | ||
.ToList(); | ||
} | ||
|
||
static readonly HashSet<string> httpMethodAttributeNames = new HashSet<string>( | ||
new[] {"Get", "Head", "Post", "Put", "Delete"} | ||
.SelectMany(x => new[] {"{0}", "{0}Attribute"}.Select(f => string.Format(f, x)))); | ||
|
||
public bool HasRefitHttpMethodAttribute(MethodDeclarationSyntax method) | ||
{ | ||
// We could also verify that the single argument is a string, | ||
// but what if somebody is dumb and uses a constant? | ||
// Could be turtles all the way down. | ||
return method.AttributeLists.SelectMany(a => a.Attributes) | ||
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 was tempted to put also check that If we want to check for a constant, do we also need to allow for constants that reference other constants? It could get messy. (I've referenced the wrong line here - this should be on after line 70.) 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. Ignore this - I have just updated it to not allow constants. |
||
.Any(a => httpMethodAttributeNames.Contains(a.Name.ToFullString().Split('.').Last()) && | ||
a.ArgumentList.Arguments.Count == 1 && | ||
a.ArgumentList.Arguments[0].Expression.CSharpKind() == SyntaxKind.StringLiteralExpression); | ||
} | ||
|
||
public TemplateInformation GenerateTemplateInfoForInterfaceList(List<InterfaceDeclarationSyntax> interfaceList) | ||
{ | ||
var usings = interfaceList | ||
|
@@ -100,6 +113,7 @@ public ClassTemplateInfo GenerateClassInfoForInterface(InterfaceDeclarationSynta | |
.Select(y => y.Identifier.ValueText)), | ||
ArgumentListWithTypes = String.Join(",", x.ParameterList.Parameters | ||
.Select(y => String.Format("{0} {1}", y.Type.ToString(), y.Identifier.ValueText))), | ||
IsRefitMethod = HasRefitHttpMethodAttribute(x) | ||
}) | ||
.ToList(); | ||
|
||
|
@@ -142,6 +156,7 @@ public class MethodTemplateInfo | |
public string Name { get; set; } | ||
public string ArgumentListWithTypes { get; set; } | ||
public string ArgumentList { get; set; } | ||
public bool IsRefitMethod { get; set; } | ||
} | ||
|
||
public class TemplateInformation | ||
|
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.
Doing this check here instead of in
HasRefitHttpMethodAttribute
means that the method could returntrue
for something that's doesn't actually have a Refit attribute (i.e. one with a different "Get" or "Post" attribute). I'm pretty sure it doesn't matter as long as we have the check here.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.
You could roll down the "using" lists to make sure they reference Refit
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.
I'm not sure what you mean? I think that's what I'm doing here - my point was just that I'm doing it here and not in the method that looks for Refit attributes.
I could take the rolled up list all usings here and pass it in as a parameter, then it's not going to be traversing the whole tree each time the method is called. I'm conscious of the fact that this runs as part of the build so we need to make it as fast as possible. Maybe it's still micro-optimising though?
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.
I'm dumb, I just read the comment and not the code, disregard :)
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.
👌