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

Concrete implementations should be private #2704

Open
retailcoder opened this Issue Feb 17, 2017 · 3 comments

Comments

Projects
None yet
2 participants
@retailcoder
Member

retailcoder commented Feb 17, 2017

By default, the VBE makes interface member implementations (/event handlers) Private, which leaves the member invisible to consumers of the concrete type, but visible to consumers of the interface.

We need an inspection that finds Public interface implementations, with a quick-fix that makes them Private.

The catch is that doing that might break callers if the callers are written against the concrete type, so the inspection's meta description should mention that.

@retailcoder

This comment has been minimized.

Member

retailcoder commented Feb 17, 2017

The quick-fix should be disabled if the member has direct callers. This makes it a little bit more difficult to implement, but not too much.

@Vogel612

This comment has been minimized.

Member

Vogel612 commented Feb 17, 2017

On that note: We should flag such direct callers in another inspection. "Explicit call to interface member on concrete class instance" seems like a good idea. The quickfix would then declare the accessed variable As Interface instead of As ConcreteClass (assuming that's possible)...

For instances where both call types are intermingled, we could have another related inspection (but that might be asking too much).

@retailcoder

This comment has been minimized.

Member

retailcoder commented Feb 17, 2017

@Vogel612 we know which interface(s) the concrete class is implementing, so there could be a quick-fix for changing the declaration to either. e.g. "Declare As ISomething" and "Declare As ISomethingElse", assuming the concrete type implements both ISomething and ISomethingElse.

Hmm.. that feels like R#'s "(thing) can be declared as (interface)"...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment