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 S1694: Remove part about protected constructor #8795

Closed
pavel-mikula-sonarsource opened this issue Feb 22, 2024 · 0 comments · Fixed by #9387
Closed

Improve S1694: Remove part about protected constructor #8795

pavel-mikula-sonarsource opened this issue Feb 22, 2024 · 0 comments · Fixed by #9387
Assignees
Labels
Area: C# C# rules related issues.
Milestone

Comments

@pavel-mikula-sonarsource
Copy link
Contributor

S1694 asks to do two things:

Change abstract class with only abstract members to an interface.
Change abstract class without abstract members to a class with protected constructor. We should do the exact opposite on this point.

Why?

If a class should be inherited by design, it should be marked as such in the class declaration. It's an explicit contract on the class level.

The rule asks to create a random member (constructor) placed somewhere inside the class (hard to find by-definition), with a specific visibility and hope that users will infer what the contract was supposed to be from that.

That is not practical from code usability and maintainability point of view.

Attempt to instantiate abstract class provides clear explanation, that is actionable (find implementing class, or create one)

CS0144 Cannot create an instance of the abstract type or interface 'Sample1'

While attempt to instantiate a class with protected constructor provides generic message that is same for private and protected constructors. Making it unclear if the class just should not be created.

CS0122 'Sample2.Sample2()' is inaccessible due to its protection level

What?

Update S1694 to limit its scope to abstract class -> interface part and remove the abstract class -> protected constructor part.

Make the rule SonarWay.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: C# C# rules related issues.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants