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

Improve the error message shown for AG0018: #155

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

szaboopeeter
Copy link
Member

The message shown when AG0018 is violated is quite confusing:

Ensure that publicly exposed IEnumerable types

This PR will change it to the following:

Invalid return type. Publicly exposed enumerable types must be on our whitelist.

Also updated some inconsistencies in how this rule is referred to in other places.

Fixes #149

@inemtsev
Copy link

@szaboopeeter I think we should expand our list of supported interfaces. Is there a list we maintain as allowed?

@szaboopeeter
Copy link
Member Author

@inemtsev It's an option, but I wouldn't couple it together with this PR. I suggest to open an issue if you wanna have a discussion over GH.
The list of supported interfaces is here in code (plus byte array, the check for that is a bit messy, but you can find it in the same file):

private static readonly string[] AllowedTypes =
{
"System.Collections.Generic.ISet<T>",
"System.Collections.Generic.IList<T>",
"System.Collections.Generic.IDictionary<TKey, TValue>",
"System.Collections.Generic.IReadOnlyDictionary<TKey, TValue>",
"System.Collections.Generic.KeyedCollection<TKey, TValue>",
"System.Collections.Generic.IEnumerable<T>",
};
, it's based on docs: https://github.com/agoda-com/standards-c-sharp/blob/master/docs/collections/choosing-collection-implementation.md#supported-types

@inemtsev
Copy link

inemtsev commented Mar 1, 2021

@szaboopeeter Our team, would like to use IReadOnlyCollection<> which results in quite a bit of warnings. I can update the docs if we agree to allow this.

@szaboopeeter
Copy link
Member Author

@inemtsev I turned that into an issue #156 We can continue discussion there, or just create a PR :)

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

Successfully merging this pull request may close these issues.

AG0018 - Bad wording on message
3 participants