Add a custom mechanism for looking up comments.

[misberner]

This PR adds a custom mechanism to Reflector for looking up comments, in addition to CommentsMap.

A LookupComment(reflect.Type, string) function can be defined, that, when set, gets invoked with the same arguments as the internal lookupComment method of Reflector. If it returns a non-empty string, that will be used as the type/field comment; otherwise, if the returned string is emtpy or the LookupComment function is not set, the CommentsMap will be consulted. The precedence of an explicit description defined via struct tags remains unaffected.

---

The motivation for this is that we cannot use the mechanism provided by the package for parsing Go comments, as it would require access to the source at runtime. Instead, we rely on code generation to provide Go comments at runtime. The data structure that we use is different from how the CommentsMap is structured. While we could create a CommentsMap in the required format, this would require a priori knowledge of all the types that will be referenced in the JSON schema (or we'd have to generate it for all our types, which is a much larger set as the ones we want to obtain JSON schemas for.

[samlown]

@misberner this is a great idea! Thanks for submitting! Just by chance, I've also been working on the comments handling and incorporated a CommentOption function type def for handling more complex configurations. PR #158.

Do you think it would be possible to match that approach instead of adding yet more options to the Reflect struct? Thanks!

[misberner]

@samlown thanks for the quick response! I have to admit I am not fully sure I understand the ask:

• LookupComment is in my view the most general form. Anything that the CommentOption does (and more) could be implemented by a custom logic in LookupComment, which is totally flexible.
• CommentOption makes sense when the source of comments is "fixed", like from Go source: it applies to the extraction process. LookupComment is intentionally decoupled from how the comments are extracted.
• I don't think I can add a WithLookupComment CommentOption or similar (which is what I understand would be the one way to avoid adding the LookupComment field to reflector), because as said above, CommentOption only applies to extraction from source which I'm looking to entirely circumvent. It also is used for populating the CommentMap, which requires an a priori knowledge of all the types that will be referenced in the JSON schema - something I'm also looking to avoid. Also, CommentOptions can only be passed to AddGoComments, so what would be passed as base and path if I don't want to do extraction from source?
• Refactoring the CommentOption functionality to circumvent all that is probably possible, but I think it would require a major refactor, so I'd like to double-check if that's what you had in mind.

[samlown]

@misberner you're right, sorry about that. I looked at the example and assumed that the LookupComment method needed to be used with the AddGoComments as per the test code, but clearly that's not the case. If anything, the LookupComment could in fact by an array of functions one of which could be a method to use the comments array generated by the AddGoComments method. I'd prefer that to the current public CommentMap.

I'm super-short on time for refactors. If you feel up for an experiment, great! Otherwise if you could sync with the current main branch and move as much of the logic to the reflect_comments.go file and ensure test coverage is at least maintained, that'd be great. (You may have noticed most of the PRs are rejected due to lack of tests, hopefully the code coverage action will help developers figure out what they need to add.)

[misberner]

@samlown no worries! I also don't have a ton of time on my hands so I went with the simple option.

And yes, agreed on (a) using LookupComment over CommentMap (though that would break backwards-compatibility) and (b) potentially allowing multiple lookup functions. However, I think a single variable is sufficient as the general case. It's easy enough to do something like

func CombineCommentLookups(lookupFns ...func(reflect.Type, string) string) func(reflect.Type, string) string {
  return func(t reflect.Type, field string) string {
    for _, lookupFn := range lookupFns {
      if comment := lookupFn(t, field); comment != "" {
        return comment
      }
    }
    return ""
  }
}

if the need for this arrives. I'd like to keep it simple for now. If you disagree, I can easily change the option to be of slice type directly.

[misberner]

Locally getting the same 92.1% coverage number on my branch as on main

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.39%. Comparing base (42b1bb0) to head (83557f4).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #159      +/-   ##
==========================================
+ Coverage   88.34%   88.39%   +0.05%     
==========================================
  Files           4        4              
  Lines         858      862       +4     
==========================================
+ Hits          758      762       +4     
  Misses         80       80              
  Partials       20       20              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[samlown]

Awesome, thanks for this!