Improve diagnostic quality for less accessible base type argument #80483

stbau04 · 2025-09-27T12:40:42Z

src/Compilers/CSharp/Portable/Symbols/TypeSymbolExtensions.cs

stbau04 · 2025-09-27T13:27:16Z

If wanted, i could also open a follow up pr, where i add seperate error messages for inaccessable type parameters and inaccessable parents of nested classes

src/Compilers/CSharp/Portable/CSharpResources.resx

src/Compilers/CSharp/Portable/Symbols/TypeSymbolExtensions.cs

Co-authored-by: Jan Jones <[email protected]>

stbau04 · 2025-09-29T20:45:04Z

I am not sure why the pipelines are failing in spanish, but i suspect it as something to do with the failing tests on main? Is there a way for me to trigger the pipeline again, if i have no changes to push and not commits on main to merge into the branch?

RikkiGibson · 2025-09-29T20:49:42Z

It seems doubtful the Spanish failure is related to this change. I'm kicking a rerun.

https://dev.azure.com/dnceng-public/public/_build/results?buildId=1160966&view=logs&jobId=e1af275f-8330-53ac-3037-c0a140385dde&j=e1af275f-8330-53ac-3037-c0a140385dde&t=06384551-a865-5685-cb3e-f37041bf2d8b

Failure log

  Correctas Microsoft.CodeAnalysis.MSBuild.UnitTests.NewlyCreatedProjectsFromDotNetNew.ValidateVisualBasicTemplateProjects(templateName: "wpfcustomcontrollib") [8 s]
  Mensajes de salida estándar:
 [07:31:29.544] [Trace] [dotnet.exe output] The template "WPF Custom Control Library" was created successfully.
 
 Processing post-creation actions...
 Restoring C:\h\w\A853090F\t\RoslynTests\6e5a3c86-7134-445c-808c-9a573f2cc24d\6e5a3c86-7134-445c-808c-9a573f2cc24d.vbproj:
 
   Determining projects to restore...
   Restored C:\h\w\A853090F\t\RoslynTests\6e5a3c86-7134-445c-808c-9a573f2cc24d\6e5a3c86-7134-445c-808c-9a573f2cc24d.vbproj (in 91 ms).
 Restore succeeded.
 
 
 
 [07:31:31.670] [Trace] [BuildHost PID 968] Message on stderr: info: BuildHost Runtime Version: .NET 10.0.0-rc.1.25451.107
 [07:31:31.904] [Trace] [BuildHost PID 968] Sending a Shutdown request to the BuildHost.
 [07:31:31.904] [Trace] [BuildHost PID 968] Process shut down.
 [07:31:31.920] [Trace] [BuildHost PID 968] Message on stderr: info: RPC channel closed; process exiting.
 [07:31:31.920] [Information] [Microsoft.CodeAnalysis.MSBuild.BuildHostProcessManager] .NET BuildHost started from C:\h\w\A853090F\p\dotnet-cli\sdk\10.0.100-rc.1.25451.107\TestHostNetFramework\testhost.net472.exe reloading to start from C:\h\w\A853090F\p\dotnet-cli\dotnet.exe to match necessary SDK location.
 [07:31:32.029] [Trace] [BuildHost PID 5920] Message on stderr: info: BuildHost Runtime Version: .NET 10.0.0-rc.1.25451.107
 [07:31:32.202] [Trace] [BuildHost PID 5920] Message on stderr: info: Registered MSBuild 10.0.100 instance at C:\h\w\A853090F\p\dotnet-cli\sdk\10.0.100-rc.1.25451.107
 [07:31:32.249] [Trace] [BuildHost PID 5920] Message on stderr: info: Loading C:\h\w\A853090F\t\RoslynTests\6e5a3c86-7134-445c-808c-9a573f2cc24d\6e5a3c86-7134-445c-808c-9a573f2cc24d.vbproj
 [07:31:33.530] [Trace] [BuildHost PID 5920] Sending a Shutdown request to the BuildHost.
 [07:31:33.530] [Trace] [BuildHost PID 5920] Process shut down.
 [07:31:33.530] [Trace] [BuildHost PID 5920] Message on stderr: info: RPC channel closed; process exiting.


[xUnit.net 00:00:17.86]     Microsoft.CodeAnalysis.MSBuild.UnitTests.NewlyCreatedProjectsFromDotNetNew.ValidateVisualBasicTemplateProjects(templateName: "winforms") [FAIL]
[xUnit.net 00:00:17.86]       System.InvalidOperationException : `dotnet new "winforms" -o "C:\h\w\A853090F\t\RoslynTests\c792e465-0398-4a74-87bf-f70e204eec4e" --language "VB"` returned a non-zero exit code.
[xUnit.net 00:00:17.87]       Output:
[xUnit.net 00:00:17.87]       
[xUnit.net 00:00:17.87]       
[xUnit.net 00:00:17.87]       Error:
[xUnit.net 00:00:17.87]       Template "Windows Forms App" could not be created.
[xUnit.net 00:00:17.87]       Failed to create template.
[xUnit.net 00:00:17.87]       Details: Error while processing file /WinFormsApplication-VisualBasic/Company.WinFormsApplicationSkipAppModel1.vbproj
[xUnit.net 00:00:17.87]       The process cannot access the file 'C:\h\w\A853090F\t\RoslynTests\c792e465-0398-4a74-87bf-f70e204eec4e\c792e465-0398-4a74-87bf-f70e204eec4e.vbproj' because it is being used by another process.
[xUnit.net 00:00:17.87]       
[xUnit.net 00:00:17.87]       For details on the exit code, refer to https://aka.ms/templating-exit-codes#100
[xUnit.net 00:00:17.87]       
[xUnit.net 00:00:17.87]       Stack Trace:
[xUnit.net 00:00:17.87]            en Microsoft.CodeAnalysis.MSBuild.UnitTests.NewlyCreatedProjectsFromDotNetNew.RunDotNet(String arguments, ILoggerFactory loggerFactory, String workingDirectory) en /_/src/Workspaces/MSBuild/Test/NewlyCreatedProjectsFromDotNetNew.cs:línea 264
[xUnit.net 00:00:17.87]            en Microsoft.CodeAnalysis.MSBuild.UnitTests.NewlyCreatedProjectsFromDotNetNew.<AssertTemplateProjectLoadsCleanlyAsync>g__CreateNewProject|9_1(String templateName, String outputDirectory, String languageName) en /_/src/Workspaces/MSBuild/Test/NewlyCreatedProjectsFromDotNetNew.cs:línea 185
[xUnit.net 00:00:17.87]            en Microsoft.CodeAnalysis.MSBuild.UnitTests.NewlyCreatedProjectsFromDotNetNew.<AssertTemplateProjectLoadsCleanlyAsync>d__9.MoveNext() en /_/src/Workspaces/MSBuild/Test/NewlyCreatedProjectsFromDotNetNew.cs:línea 158
[xUnit.net 00:00:17.87]         --- Fin del seguimiento de la pila de la ubicación anterior donde se produjo la excepción ---
[xUnit.net 00:00:17.87]            en System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
[xUnit.net 00:00:17.87]            en System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
[xUnit.net 00:00:17.87]            en Microsoft.CodeAnalysis.MSBuild.UnitTests.NewlyCreatedProjectsFromDotNetNew.<ValidateVisualBasicTemplateProjects>d__5.MoveNext() en /_/src/Workspaces/MSBuild/Test/NewlyCreatedProjectsFromDotNetNew.cs:línea 86
[xUnit.net 00:00:17.87]         --- Fin del seguimiento de la pila de la ubicación anterior donde se produjo la excepción ---
[xUnit.net 00:00:17.87]            en System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
[xUnit.net 00:00:17.87]            en System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
[xUnit.net 00:00:17.87]            en Xunit.Sdk.TestInvoker`1.<>c__DisplayClass47_0.<<InvokeTestMethodAsync>b__1>d.MoveNext() en /_/src/xunit.execution/Sdk/Frameworks/Runners/TestInvoker.cs:línea 259
[xUnit.net 00:00:17.87]         --- Fin del seguimiento de la pila de la ubicación anterior donde se produjo la excepción ---
[xUnit.net 00:00:17.87]            en System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
[xUnit.net 00:00:17.87]            en System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
[xUnit.net 00:00:17.87]            en Xunit.Sdk.ExecutionTimer.<AggregateAsync>d__4.MoveNext() en /_/src/xunit.execution/Sdk/Frameworks/ExecutionTimer.cs:línea 48
[xUnit.net 00:00:17.87]         --- Fin del seguimiento de la pila de la ubicación anterior donde se produjo la excepción ---
[xUnit.net 00:00:17.87]            en System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
[xUnit.net 00:00:17.87]            en System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
[xUnit.net 00:00:17.87]            en Xunit.Sdk.ExceptionAggregator.<RunAsync>d__9.MoveNext() en /_/src/xunit.core/Sdk/ExceptionAggregator.cs:línea 90

src/Compilers/CSharp/Portable/Symbols/TypeSymbolExtensions.cs

src/Compilers/CSharp/Portable/Symbols/Source/SourceNamedTypeSymbol_Bases.cs

src/Compilers/CSharp/Test/Emit3/Semantics/RecordTests.cs

src/Compilers/CSharp/Test/Symbol/Symbols/Source/BaseClassTests.cs

src/Compilers/CSharp/Portable/Symbols/Source/SourceNamedTypeSymbol_Bases.cs

…mbol_Bases.cs

stbau04 · 2025-10-06T19:00:24Z

@jjonescz Would you maybe have a quick moment to re-review?

RikkiGibson · 2025-10-06T19:16:25Z

@jjonescz feel free to hit merge if you don't have any further feedback.

jjonescz · 2025-10-07T08:45:27Z

src/Compilers/CSharp/Test/Symbol/Symbols/Source/BaseClassTests.cs

+            comp.VerifyDiagnostics(
+                // (3,14): error CS9335: Inconsistent accessibility: type 'A' is less accessible than class 'C'
+                //     public class C : A.B { }
+                Diagnostic(ErrorCode.ERR_BadVisBaseType, "C").WithArguments("C", "A").WithLocation(3, 14));


I'm wondering whether the message could be improved - since we squiggle the class C itself, the user might be confused where to search for the less accessible type (in nested types, in members, in base types, in attributes, ...) - i.e., should the message be something like this?

"Inconsistent accessibility: type A which appears in the base type is less accessible than class C"

How would you like "Inconsistent accessibility: type A used in the base type is less accessible than class C"?
Not sure why, but that somehow sounds better to me (but that might be due to my slightly lacking english skills)

I am just concerned about scalability, if we started reporting this same warning for member types and so on, would we want to have a bunch of different messages of this form? What if we just reported both errors, with the current message, in the case that an inner type of the base type is the cause of the bad accessibility?

With "both errors" you mean like this?

error CS0060: Inconsistent accessibility: base type 'B<A>' is less accessible than class 'C' error CS9335: Inconsistent accessibility: type 'A' is less accessible than class 'C'

Yep. Let's see what others think and then we can come up with a decision.

Reporting both errors works for me, thanks.

(The current state of the PR also works for me if others think that's better, this was not a blocking comment, just a thought about a possible improvement.)

I am very much not a fan of reporting both errors; that just gets us back into the confusing situation that we started at. B<A> is not less accessible than C, and any error that states that it is is incorrect. I think the current form is perfectly reasonable.

Is it something people would even be really confused about? IMHO most of the times when somebody is changing something and gets the error that A is less accessiable than C, he is probably aware, how C is related to A (as he must have done something that lets C inherit from A).

Most of the times i get the BadVisBaseClass is when i am having some internal stuff and accidently create a public subclass, so the common fix for this would be to correct the access modifier, which is also possible without knowing about the relation in detail.

So this effectively i feel like it only might be really confusing if somebody is unaware of the relation between C and A, while having a design flaw that expects an impossible inheritance. And even if that is the case, somebody who has seen Inconsistent accessibility: base class '{1}' is less accessible than class '{0}' a few times might not even recognize the different wording in Inconsistent accessibility: type '{1}' is less accessible than class '{0}' before instinctively knowing what went wrong (at least i would not expect myself to realize that the error is different).

Edit: Error could also occur if one of the access modifiers is changed, didn't think of that

B<A> is not less accessible than C

I don't agree with this statement. If this were true, it would mean that every place the compiler can currently complain about B<A> being insufficiently accessible in a signature, due to accessibility of A, is problematic. I don't believe that to be the case.

I think that when a compiler gives an error stating type '{1}' is less accessible than type '{0}', it is in reference to the accessibility domains concept in the standard, which states that accessibility domain of a constructed type is the intersection of the accessibility of the original definition and the type arguments.

That said, I am satisfied with the current behavior of the PR, and I think it's fine to move forward by resolving conflicts and merging with the current behavior.

Conflicts are resolved, so it would be ready to merge?

src/Compilers/CSharp/Portable/CSharpResources.resx

stbau04 · 2025-10-11T09:22:29Z

Gonna fix the conflict as soon as the discussion about the exact errors emitted is resolved, want to avoid doing it more often than necessary

jjonescz · 2025-10-13T08:32:25Z

Thanks @stbau04!

ascire · 2025-10-20T13:07:04Z

src/Compilers/CSharp/Portable/Symbols/Source/SourceNamedTypeSymbol_Bases.cs

+                    {
+                        // Inconsistent accessibility: type '{1}' is less accessible than class '{0}'
+                        var lessVisibleTypeLocation = lessVisibleType.GetFirstLocation();
+                        diagnostics.Add(ErrorCode.ERR_BadVisBaseType, baseTypeLocation, this, lessVisibleType);


Did you intend to use lessVisibleTypeLocation instead of baseTypeLocation in the diagnostics entry?

iirc lessVisibleTypeLocation is the location of the type definition. So it would not be intended to be used instead of the baseTypeLocation, but the variable is not used anywhere so i guess the declaration can be removed #80823

Implement

b0a0c7d

github-actions bot added the Area-Compilers label Sep 27, 2025

dotnet-policy-service bot added the Community The pull request was submitted by a contributor who is not a Microsoft employee. label Sep 27, 2025

Resolve conflicts

7e1ff18

stbau04 commented Sep 27, 2025

View reviewed changes

src/Compilers/CSharp/Portable/Symbols/TypeSymbolExtensions.cs Outdated Show resolved Hide resolved

Stephan Bauer added 4 commits September 27, 2025 15:28

Fix CSharpResources.resx

26693a7

Remove unnecessary check

7619085

Fix unit tests, Fix typo

3f59cc1

Fix for runtime with covariant return support

1571964

stbau04 marked this pull request as ready for review September 27, 2025 18:42

stbau04 requested a review from a team as a code owner September 27, 2025 18:42

jjonescz reviewed Sep 29, 2025

View reviewed changes

stbau04 and others added 5 commits September 29, 2025 18:29

Update src/Compilers/CSharp/Portable/CSharpResources.resx

c55b3cf

Co-authored-by: Jan Jones <[email protected]>

Update src/Compilers/CSharp/Portable/Symbols/TypeSymbolExtensions.cs

dd141d5

Co-authored-by: Jan Jones <[email protected]>

Track result in predicate instead

69c20ad

Fix xlf files

45c6fcc

Merge branch 'main' into feature/40977-improve-diagnostic-quality

16028bf

stbau04 requested a review from jjonescz September 29, 2025 20:41

RikkiGibson reviewed Sep 29, 2025

View reviewed changes

src/Compilers/CSharp/Portable/Symbols/TypeSymbolExtensions.cs Outdated Show resolved Hide resolved

Stephan Bauer added 3 commits October 1, 2025 18:30

Address pr review

632be43

Fix checks

57e1b52

Fix again

b3ce155

stbau04 requested review from 333fred and RikkiGibson October 1, 2025 18:12

RikkiGibson reviewed Oct 1, 2025

View reviewed changes

src/Compilers/CSharp/Portable/Symbols/Source/SourceNamedTypeSymbol_Bases.cs Outdated Show resolved Hide resolved

RikkiGibson reviewed Oct 1, 2025

View reviewed changes

src/Compilers/CSharp/Test/Emit3/Semantics/RecordTests.cs Outdated Show resolved Hide resolved

RikkiGibson reviewed Oct 1, 2025

View reviewed changes

src/Compilers/CSharp/Test/Symbol/Symbols/Source/BaseClassTests.cs Outdated Show resolved Hide resolved

Fix pr changes

5cd6798

RikkiGibson approved these changes Oct 1, 2025

View reviewed changes

333fred reviewed Oct 1, 2025

View reviewed changes

src/Compilers/CSharp/Portable/Symbols/Source/SourceNamedTypeSymbol_Bases.cs Outdated Show resolved Hide resolved

stbau04 commented Oct 2, 2025

View reviewed changes

src/Compilers/CSharp/Portable/Symbols/Source/SourceNamedTypeSymbol_Bases.cs Outdated Show resolved Hide resolved

Update src/Compilers/CSharp/Portable/Symbols/Source/SourceNamedTypeSy…

2c73ea1

…mbol_Bases.cs

stbau04 requested a review from 333fred October 2, 2025 20:11

333fred approved these changes Oct 3, 2025

View reviewed changes

jjonescz approved these changes Oct 7, 2025

View reviewed changes

jjonescz reviewed Oct 7, 2025

View reviewed changes

Merge branch 'main' into feature/40977-improve-diagnostic-quality

19e742e

Stephan Bauer added 4 commits October 12, 2025 12:57

Fix error code

4d586ee

merge main

0d26fd5

Change error code

24b3a90

Fix resources

2020478

jjonescz merged commit d2b50b6 into dotnet:main Oct 13, 2025
24 checks passed

dotnet-policy-service bot added this to the Next milestone Oct 13, 2025

dotnet-bot mentioned this pull request Oct 14, 2025

[Automated] PRs inserted in VS build main-11113.80 #80688

Closed

This was referenced Oct 14, 2025

[main] Source code updates from dotnet/roslyn dotnet/dotnet#2842

Closed

[main] Source code updates from dotnet/roslyn dotnet/dotnet#2965

Closed

This was referenced Oct 17, 2025

[Automated] PRs inserted in VS build feature.polytype-11117.05 #80797

Closed

[Automated] PRs inserted in VS build feature.debugger.main-11116.247 #80798

Closed

[Automated] PRs inserted in VS build feature.polytype-11116.236 #80799

Closed

ascire reviewed Oct 20, 2025

View reviewed changes

stbau04 mentioned this pull request Oct 20, 2025

Remove unused statement #80823

Merged

This was referenced Oct 23, 2025

[main] Source code updates from dotnet/roslyn dotnet/dotnet#3061

Closed

[main] Source code updates from dotnet/roslyn dotnet/dotnet#3147

Open

Improve diagnostic quality for less accessible base type argument #80483

Improve diagnostic quality for less accessible base type argument #80483

Uh oh!

Conversation

stbau04 commented Sep 27, 2025

Uh oh!

Uh oh!

stbau04 commented Sep 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

stbau04 commented Sep 29, 2025

Uh oh!

RikkiGibson commented Sep 29, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

stbau04 commented Oct 6, 2025

Uh oh!

RikkiGibson commented Oct 6, 2025

Uh oh!

jjonescz Oct 7, 2025

Choose a reason for hiding this comment

Uh oh!

stbau04 Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RikkiGibson Oct 7, 2025

Choose a reason for hiding this comment

Uh oh!

stbau04 Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RikkiGibson Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jjonescz Oct 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

333fred Oct 8, 2025

Choose a reason for hiding this comment

Uh oh!

stbau04 Oct 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RikkiGibson Oct 11, 2025

Choose a reason for hiding this comment

Uh oh!

stbau04 Oct 12, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

stbau04 commented Oct 11, 2025

Uh oh!

Uh oh!

jjonescz commented Oct 13, 2025

Uh oh!

ascire Oct 20, 2025

Choose a reason for hiding this comment

Uh oh!

stbau04 Oct 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

stbau04 commented Sep 27, 2025 •

edited

Loading

stbau04 Oct 7, 2025 •

edited

Loading

stbau04 Oct 7, 2025 •

edited

Loading

RikkiGibson Oct 7, 2025 •

edited

Loading

jjonescz Oct 8, 2025 •

edited

Loading

stbau04 Oct 11, 2025 •

edited

Loading

stbau04 Oct 20, 2025 •

edited

Loading