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

Allow to change the behavior for interfaces for rule IDE0040: Accessibility modifiers required #77253

Open
m-gasser opened this issue Feb 17, 2025 · 4 comments
Labels
Area-IDE Feature Request untriaged Issues and PRs which have not yet been triaged by a lead

Comments

@m-gasser
Copy link

m-gasser commented Feb 17, 2025

Brief description:

In #74245 IDE0040 was fixed to also apply to interfaces. However it does so in a way that prohibits the configuration we actually want. We ideally would like this behavior:

  1. Accessibility modifiers for members of non interface types are mandatory
  2. Accessibility modifiers for nested types inside interface members are mandatory
  3. Accessibility modifiers for members (methods, properties...) are not enforced but allowed.

This could be achieved by either changing the behavior of the for_non_interface_members option or by providing another option or making a separate rule for interfaces than for other types. I am unsure which option is the best way to go.

Languages applicable:

C# (and probably also VB)

Code example that the analyzer should report:

public interface IFoo
{
    int SomeMethod1(); // This is allowed as "customary right" because in older C# versions access modifiers could not be specified
    public int SomeMethod2(); // This is allowed for additional clarity or to be consistent with members of non interface types

    record Bar; // <- diagnostics applies here. For nested types, access modifiers should be applied no matter whether they are nested in an interface or another type
}

Documentation requirements:

When this analyzer is implemented, it must be documented by following the steps at Documentation for IDE CodeStyle analyzers.

@CyrusNajmabadi
Copy link
Member

The first two items seem reasonable. The last is very odd. We don't generally have a rule that conditionally enforces in that manner. The idea with these rules is to ensure total consistency. The third rule would allow different parts of the codebase to differ with accessibility modifiers. We will discuss it, but we'd need a strong argument as to why it's desirable to enforce for everything else, but carve out this subset to not enforce.

@m-gasser
Copy link
Author

Now that I thought more about it, yes it is a bit weird. Especially if there still would be code fixes in the IDE to add/remove the modifiers. It would probably be more reasonable to specify a preference for that case but with a separate severity. So we would configure the general rule to warning and the non-type interface members to information with a preference of no accessibility modifier.

@tj-concentrix
Copy link

Again I feel like the wrinkle on this rule which doesn't exist for most others is the fact that modifiers were not allowed in the past. If it were possible to say "enforce this for new code but not old code" that would be another solution but there's no way for the analyzer to know what's "new" code.

Sometimes it's helpful when a new analyzer finds "old" issues in code, but to me this is not one of those cases & I feel like there are likely many people in this boat.

But to be honest we could also just create an .editorconfig in the "old" code to selectively disable the rule in those projects.

@CyrusNajmabadi
Copy link
Member

CyrusNajmabadi commented Feb 18, 2025

Again I feel like the wrinkle on this rule which doesn't exist for most others is the fact that modifiers were not allowed in the past

Correct. That's why there are two options here. One to keep code looking like how it was written for like 20 years (for_non_interface_members). And one to be more restrictive and consistent (always).

The intent of both though is the same as all our other rules. Namely that within a codebase everything be written consistently.

Wrt these two option values, you decide for your codebase:

  1. Do we want to require modifiers everywhere? Or,
  2. Do we want a special carve out for public interface members for historical reasons.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE Feature Request untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

No branches or pull requests

3 participants