Include operators in api generation #49
Conversation
| @@ -512,7 +512,8 @@ static void AddCtorToTypeDeclaration(CodeTypeDeclaration typeDeclaration, Method | |||
|
|
|||
| static void AddMethodToTypeDeclaration(CodeTypeDeclaration typeDeclaration, MethodDefinition member, HashSet<string> excludeAttributes) | |||
| { | |||
| if (member.IsAssembly || member.IsPrivate || member.IsSpecialName) | |||
| var isOperator = member.Name.StartsWith("op_"); | |||
There was a problem hiding this comment.
Could we inline this so that it is only evaluated with IsSpecialName?
| { | ||
| public ClassWithUnaryOperators() { } | ||
| public static PublicApiGeneratorTests.Examples.ClassWithUnaryOperators op_Addition(PublicApiGeneratorTests.Examples.ClassWithUnaryOperators first, PublicApiGeneratorTests.Examples.ClassWithUnaryOperators second) { } | ||
| public static PublicApiGeneratorTests.Examples.ClassWithUnaryOperators op_Decrement(PublicApiGeneratorTests.Examples.ClassWithUnaryOperators first) { } |
There was a problem hiding this comment.
Would it make sense to have a translation table that translates comnon well known ops into the corresponding operator code? Would be a bit more readable in the output I think
There was a problem hiding this comment.
I agree. I looked into why the full type name was being generated but, likely due to operators being static, it appears they are seen as "unnested" types which causes the FullName to be used instead of just the specific type's name.
Handling the operators in a more special manner would definitely allow me to more cleanly format them (since the problematic area only knows the Type without any context of its use). Do you have a preference of such output? To riff on something:
public static ClassWithUnaryOperator +(ClassWithUnaryOperator first, ClassWithUnaryOperator second) { }
Or are you looking to further condense the signature?
|
Awesome stuff. Made a few comments |
|
I think it would be good enough to handle known operators except implicit casts as their code represention like your sample shows. Doesnt have to be fancy. We could always improve in a next major after releasing the first initial version (will also be a major)
|
|
@ryanwersal do you need help to get this going? |
|
@danielmarbach Sorry, I admit I somewhat forgot about this... I'll look to getting it updated over the weekend. |
|
No worries. Good stuff takes time 💃 |
|
Apologies for the near month (!!!) delay but I have the PR updated with a mapping as requested. I did the operators I could find on the Overloadable Operators documentation except for the true/false operators as I found it a bit ambiguous in the final output (since it looks so similar to a boolean value/output). Let me know if you'd like further modifications and/or add more tests. Thanks! |
|
@ryanwersal scandalous :) Don't be so hard to yourself. You did awesome work here and every contribution is very much appreciated! The packages are being indexed on nuget. In a few minutes your work is on the interwebs Thanks again! |
I noticed that operators are not currently included in the generated public api surface whereas I would expect operators to be included. This was the result of
IsSpecialNamereturningtruefor operators.Unfortunately there doesn't appear to be a good way to tell whether this is a "special name" that should be included or not (such as the case for operators). I confess I'm not sure what all falls under the "special name" category. The best differentiator I could find was sources claiming that all operator names are formatted as "op_*" so I leveraged that for determining if a given method is an operator. I'm very open to alternatives that can be more precise without needing to rely on string comparisons.
I'm willing to write more tests but I wasn't sure what would be overkill. I tried to mirror the
Method_*.cstest files to a degree.