Add automatic removal of interface member access modifiers about csharpier HOT 3 CLOSED

I'm a little hesitant to add this because it feels a bit outside the scope of CSharpier. The only two exceptions to the "only change whitespace" guideline were a pain to deal with due to figuring out how to get the syntax tree validation to not flag them as problems with code being lost.

Other modifiers are also unnecessary but it is pretty standard to always include them. Should CSharpier then enforce adding the following modifiers -

If no modifier is on an interface should CSharpier change it to public interface?
Should classes without modifiers be changed to internal class?
Should members of classes without modifiers have private added?

from csharpier.

If the idea is to not change behavior, then your first example should probably be changed instead to internal interface.

But, on further thought, I'm removing my vote. IMO the behavior would be less than obvious. For example, it would not make sense to change this

internal sealed class MaybeThisShouldBePublic
{
    internal string MyInternalOnlyProperty { get; set; }
}

to this

internal sealed class MaybeThisShouldBePublic
{
    public string MyInternalOnlyProperty { get; set; }
}

because even though the internal modifier on the property is currently useless, if CSharpier were to change the property accessor to public and then the user later changed the class to public, CSharpier may have just exposed a property that shouldn't have been exposed. Even worse if they accidentally change a public class to internal, and CSharpier instantly changes all their properties to public. So a case like that would need to be left alone. But to me as a user it wouldn't be obvious why CSharpier sometimes removes/adds/changes "useless/non-effective" modifiers -- but not always.

EDIT:

Back to the original proposal, it wouldn't even be quite as simple as just removing access modifiers. An interface may contain private/internal static members, class/interface definitions, or default method implementations. It would only be safe to remove public modifiers. Now some people might not like the inconsistency in only showing some access modifiers and removing others. And that's a valid preference, IMO. My vote is to leave things how the dev wrote them.

from csharpier.

Having read your arguments, I probably agree that this issue is a bit beyond the scope of CSharpier's responsibility, because it can potentially change the behavior of the source code. Yes, there are many special cases here. I think it's better for the developer to make his own decision about the access modifier, depending on the situation.

Thanks for the feedback!
Closing the issue.

from csharpier.