Skip to content

Avoid reporting CA1033 for interfaces#50339

Open
Alex-Sob wants to merge 2 commits intodotnet:mainfrom
Alex-Sob:dev/alex-sob/CA1033-fix
Open

Avoid reporting CA1033 for interfaces#50339
Alex-Sob wants to merge 2 commits intodotnet:mainfrom
Alex-Sob:dev/alex-sob/CA1033-fix

Conversation

@Alex-Sob
Copy link

@Alex-Sob Alex-Sob commented Aug 18, 2025 •
edited
Loading

This PR fixes #50341

Changes

  • Fixed InterfaceMethodsShouldBeCallableByChildTypesAnalyzer so that CA1033 is not reported for interfaces with default implementations, for example:

    public interface IBase
{
    void M();
}

public interface IDerived : IBase
{
    // Causes CA1033: Make 'IDerived' sealed (a breaking change if this class has previously shipped), implement the method non-explicitly, or implement a new method that exposes the functionality of 'ConsoleApp.IBase.M' and is visible to derived classes
    void IBase.M() => Console.WriteLine();
}

  • Added unit tests.

@Alex-Sob Alex-Sob requested a review from a team as a code owner August 18, 2025 12:28
@Alex-Sob Alex-Sob mentioned this pull request Aug 18, 2025
Open
Alex-Sob
Alex-Sob commented Aug 18, 2025
View reviewed changes
...deQuality.Analyzers/ApiDesignGuidelines/InterfaceMethodsShouldBeCallableByChildTypesTests.cs
[Fact]
public async Task CA1033InterfaceWithDefaultImplementationsCSharpAsync()
{
await VerifyCS.Test.Create("""
Copy link
Author

@Alex-Sob Alex-Sob Aug 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about using raw string literals? I think it makes tests more readable. Is it acceptable to update other tests only in this file for now? Or should all tests in the project be updated at once?

Alex-Sob
Alex-Sob commented Aug 18, 2025
View reviewed changes
src/Microsoft.CodeAnalysis.NetAnalyzers/tests/Test.Utilities/CSharpCodeFixVerifier`2+Test.cs
public LanguageVersion LanguageVersion { get; set; } = LanguageVersion.CSharp7_3;
public LanguageVersion LanguageVersion { get; set; } = DefaultLanguageVersion;

public static Test Create([StringSyntax("C#-test")] string source, LanguageVersion languageVersion = DefaultLanguageVersion, params ReadOnlySpan<DiagnosticResult> expected)
Copy link
Author

@Alex-Sob Alex-Sob Aug 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also proposed these changes a separate PR #50342 with a detailed description

This was referenced Aug 22, 2025
Open
Closed
@Alex-Sob Alex-Sob changed the title CA1033 should not report errors for interfaces CA1033 should not be reported for interfaces Aug 26, 2025
@Alex-Sob Alex-Sob changed the title CA1033 should not be reported for interfaces Avoid reporting CA1033 for interfaces Aug 26, 2025
@Alex-Sob
Copy link
Author

Alex-Sob commented Sep 11, 2025

@dotnet/dotnet-analyzers

@github-actions
Copy link
Contributor

github-actions bot commented Mar 10, 2026

Due to lack of recent activity, this PR has been labeled as 'Stale'. It will be closed if no further activity occurs within 7 more days. Any new comment will remove the label.

@github-actions github-actions bot added the stale label Mar 10, 2026
@Frulfump
Copy link

Frulfump commented Mar 10, 2026

Due to lack of recent activity, this PR has been labeled as 'Stale'. It will be closed if no further activity occurs within 7 more days. Any new comment will remove the label.

The lack of activity is just waiting for a review so the bot just adds noise.

@github-actions github-actions bot removed the stale label Mar 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

Area-Infrastructure

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

CA1033 should not report errors for interfaces

2 participants

@Alex-Sob @Frulfump