PropertiesMustHavePublic(Getters/Setters) only inspects public properties about conventional HOT 8 CLOSED

Excellent, thank you for your assistance!

from conventional.

@AtomicBlom interesting! So your scenario is implying that the property needs to be explicitly marked public, so that the getters and setters can be public?

I can see a couple of options here:

1. Modify the current convention to enforce the property level modifier. This is a reasonable path, but is a breaking behavioural change.
2. Add an overload to the constructor of PropertiesMustHavePublic* that allows you to supply the binding flags you would like to work with. This is a non-breaking change, as we would leave the default behaviour as-is.
3. Rename the current convention to PublicPropertiesMustHavePublicGetters and introduce an AllPropertiesMustHavePublicGetters convention. This is a breaking API change.

I'm partial to not making breaking changes on this convention, as it is one of the most widely used (Generally used to ensure contracts are serializable).

What are your thoughts on the above options?

from conventional.

I think that an overload would be completely fine, as I believe I've seen you modify the BindingFlags via a constructor parameter in another Specification, so that would certainly be consistent.

Our particular use case is that we have an assembly that defines the contracts used between services, and one of the things our unit tests were checking for is how usable the contract DTOs actually are. In my particular case, I had failed to mark a property as public, so even though it was passing the public getter and setter convention checks, when I pulled down the NuGet package to consume the contract, the property wasn't accessible as it was actually private, which was certainly not the behaviour I was expecting from the specification.

My expectation was that PropertiesMustHavePublicGetters meant that the property was publicly accessible, and a constructor would work nicely to create this expected behaviour.

from conventional.

@AtomicBlom I've realised that option 2 is still going to be a breaking change due to the way the API surfaces conventions as properties if they do not require input parameters.

Not a big problem, it should be easy enough for people upgrading to see what needs to change - they will just need to change .MustConformTo(Convention.PropertiesMustHavePublicGetters) to .MustConformTo(Convention.PropertiesMustHavePublicGetters()).

from conventional.

@AtomicBlom let me know if the PR addresses your use case.

I don't entirely love it, as there is some implicit behavior in the convention when supplied with the private BindingFlag - private properties are never going to have public gets and sets, so it feels a little odd to scoop them in to this convention.

However I think it is more useful than creating a convention per visibility modifier 👍

from conventional.

Well, the PR certainly addresses my use cause.

However, your comments are certainly reasonable. Perhaps then a better solution would be to have a dedicated specification .MustConformTo(Convention.PropertiesMustBePublic), or even using an Enum for the visibility .MustConformTo(Convention.PropertiesMustBe(Visibility.Public))

from conventional.

I certainly like .MustConformTo(Convention.PropertiesMustBePublic) more having stewed on it a little. Means a non-breaking change, and it is more explicit, and you can combine it with the other conventions to get the outcome you want. Going to close down that PR and start another 👍

from conventional.

I think I like that solution better @AtomicBlom - if you're happy here I'll merge it in once the build is green

from conventional.