Skip to content
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

GetFirstDerivedAttributeOrDefault implementation isn't consistent with doc comment #4333

Open
Youssef1313 opened this issue Dec 13, 2024 · 0 comments

Comments

@Youssef1313
Copy link
Member

Doc says:

/// Throws when multiple attributes are found (the attribute must allow multiple).

Implementation:

        Attribute[] cachedAttributes = GetCustomAttributesCached(attributeProvider, inherit);

        foreach (Attribute cachedAttribute in cachedAttributes)
        {
            if (AttributeComparer.IsDerived<TAttribute>(cachedAttribute))
            {
                return (TAttribute)cachedAttribute;
            }
        }

        return null;

My suggestion:

  1. Remove the doc comment about throwing an exception and leave the method as-is
  2. Introduce a new method that throws an exception and name it GetSingleDerivedAttributeOrDefault. Note that throwing exception may still be relevant even if AllowMultiple is not true (ONLY IF the attribute is not sealed). See UTA_MultipleExpectedExceptionsOnTestMethod is unused #4331 for details.
  3. Revise all usages, and switch from GetFirstDerivedAttributeOrDefault to GetSingleDerivedAttributeOrDefault if needed.
  4. My expectation is that after we do these three steps, we will end up having GetFirstDerivedAttributeOrDefault called with sealed attributes only. If this happened, we can switch all these to GetFirstNonDerivedAttributeOrDefault and delete GetFirstDerivedAttributeOrDefault
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant