Background and motivation
The README explains the API well but leaves out three behaviours that will surprise people. All three are things you can only discover by reading the source, which defeats the purpose for a package designed to be dropped into someone else's library.
1. Indexers are silently excluded from GetProperties.
OrderedPropertyCollection.Add skips them outright:
if (property.IsIndexer())
{
// We explicitly skip indexers
}This is a deliberate and probably correct decision, but it is invisible. The README documents GetProperties and, separately, FindIndexers, without ever connecting the two. A reader would reasonably assume GetProperties(MemberKind.Public) returns every public property, indexers included. Anyone building a member walker on top of this will quietly lose data and not know why.
2. Everything is cached forever in a static dictionary.
ReflectorCache is a static readonly ConcurrentDictionary keyed on (Type, MemberKind) that is never cleared. Great for performance, and there is a PerformanceSpecs file protecting it. But the consequences are not documented anywhere: the cache pins Type objects and therefore assemblies, and it grows without bound for dynamically generated types. Consumers making a decision about embedding this in their own library deserve to know. (Tracked separately as a potential code change; this issue is about documenting the current behaviour regardless of what is decided there.)
3. All types are emitted as internal.
Every type in Reflectify.cs is internal, which is exactly right for a content-only package, since it prevents two consumers from colliding. But the README never says so. The consequence a reader needs to know is that Reflectify types cannot appear in their own public API surface: you cannot expose a MemberKind parameter from a public method, and you cannot return Reflector. That is a design constraint on anyone building a library on top of it, and it is better learned from the README than from a compiler error.
A fourth, smaller one: the README says the package requires C# 12, and there is a closed issue about LangVersion. Worth confirming the stated requirement matches what the props file enforces, so the two do not drift.
Alternative Concerns
- Add a short "Things to know" or "Behaviour and limitations" section to the README covering all four points. Low effort, high value, no code changes.
- Put the indexer note in the XML documentation on
GetProperties as well, since that is where someone will actually be looking when they hit it. The doc comment currently says "Gets the public, internal, explicitly implemented and/or default properties of a type hierarchy" with no mention of the exclusion.
- Alternatively, reconsider the behaviour itself and add a
MemberKind.Indexers flag so callers can opt in. That is an API change and belongs in its own issue, but if it were done, the documentation problem largely goes away.
Are you willing help with a pull-request?
No
Background and motivation
The README explains the API well but leaves out three behaviours that will surprise people. All three are things you can only discover by reading the source, which defeats the purpose for a package designed to be dropped into someone else's library.
1. Indexers are silently excluded from
GetProperties.OrderedPropertyCollection.Addskips them outright:This is a deliberate and probably correct decision, but it is invisible. The README documents
GetPropertiesand, separately,FindIndexers, without ever connecting the two. A reader would reasonably assumeGetProperties(MemberKind.Public)returns every public property, indexers included. Anyone building a member walker on top of this will quietly lose data and not know why.2. Everything is cached forever in a static dictionary.
ReflectorCacheis astatic readonly ConcurrentDictionarykeyed on(Type, MemberKind)that is never cleared. Great for performance, and there is aPerformanceSpecsfile protecting it. But the consequences are not documented anywhere: the cache pinsTypeobjects and therefore assemblies, and it grows without bound for dynamically generated types. Consumers making a decision about embedding this in their own library deserve to know. (Tracked separately as a potential code change; this issue is about documenting the current behaviour regardless of what is decided there.)3. All types are emitted as
internal.Every type in
Reflectify.csisinternal, which is exactly right for a content-only package, since it prevents two consumers from colliding. But the README never says so. The consequence a reader needs to know is that Reflectify types cannot appear in their own public API surface: you cannot expose aMemberKindparameter from a public method, and you cannot returnReflector. That is a design constraint on anyone building a library on top of it, and it is better learned from the README than from a compiler error.A fourth, smaller one: the README says the package requires C# 12, and there is a closed issue about
LangVersion. Worth confirming the stated requirement matches what the props file enforces, so the two do not drift.Alternative Concerns
GetPropertiesas well, since that is where someone will actually be looking when they hit it. The doc comment currently says "Gets the public, internal, explicitly implemented and/or default properties of a type hierarchy" with no mention of the exclusion.MemberKind.Indexersflag so callers can opt in. That is an API change and belongs in its own issue, but if it were done, the documentation problem largely goes away.Are you willing help with a pull-request?
No